diff --git a/zerver/forms.py b/zerver/forms.py index a49db22c96..2480f8cf7d 100644 --- a/zerver/forms.py +++ b/zerver/forms.py @@ -343,12 +343,7 @@ class OurAuthenticationForm(AuthenticationForm): if username is not None and password: subdomain = get_subdomain(self.request) - try: - realm = get_realm(subdomain) - except Realm.DoesNotExist: - logging.warning("User %s attempted password login to nonexistent subdomain %s", - username, subdomain) - raise ValidationError("Realm does not exist") + realm = get_realm(subdomain) return_data: Dict[str, Any] = {} try: diff --git a/zerver/middleware.py b/zerver/middleware.py index 69a6a8652b..42d9fa5b9e 100644 --- a/zerver/middleware.py +++ b/zerver/middleware.py @@ -5,7 +5,6 @@ import traceback from typing import Any, AnyStr, Callable, Dict, Iterable, List, MutableMapping, Optional, Union from django.conf import settings -from django.core.exceptions import DisallowedHost from django.core.handlers.wsgi import WSGIRequest from django.db import connection from django.http import HttpRequest, HttpResponse, StreamingHttpResponse @@ -385,32 +384,28 @@ class FlushDisplayRecipientCache(MiddlewareMixin): return response class HostDomainMiddleware(MiddlewareMixin): - def process_response(self, request: HttpRequest, response: HttpResponse) -> HttpResponse: - if getattr(response, "asynchronous", False): - # This special Tornado "asynchronous" response is - # discarded after going through this code path as Tornado - # intends to block, so we stop here to avoid unnecessary work. - return response + def process_request(self, request: HttpRequest) -> Optional[HttpResponse]: + # Match against ALLOWED_HOSTS, which is rather permissive; + # failure will raise DisallowedHost, which is a 400. + request.get_host() - try: - request.get_host() - except DisallowedHost: - # If we get a DisallowedHost exception trying to access - # the host, (1) the request is failed anyway and so the - # below code will do nothing, and (2) the below will - # trigger a recursive exception, breaking things, so we - # just return here. - return response + # This check is important to avoid doing the extra work of + # `get_realm` (which does a database query that could be + # problematic for Tornado). Also the error page below is only + # appropriate for a page visited in a browser, not the API. + # + # API authentication will end up checking for an invalid + # realm, and throw a JSON-format error if appropriate. + if request.path.startswith(("/static/", "/api/", "/json/")): + return None - if (not request.path.startswith("/static/") and not request.path.startswith("/api/") and - not request.path.startswith("/json/")): - subdomain = get_subdomain(request) - if subdomain != Realm.SUBDOMAIN_FOR_ROOT_DOMAIN: - try: - get_realm(subdomain) - except Realm.DoesNotExist: - return render(request, "zerver/invalid_realm.html", status=404) - return response + subdomain = get_subdomain(request) + if subdomain != Realm.SUBDOMAIN_FOR_ROOT_DOMAIN: + try: + request.realm = get_realm(subdomain) + except Realm.DoesNotExist: + return render(request, "zerver/invalid_realm.html", status=404) + return None class SetRemoteAddrFromForwardedFor(MiddlewareMixin): """ diff --git a/zerver/tests/test_signup.py b/zerver/tests/test_signup.py index 616f81f4a7..037134f21e 100644 --- a/zerver/tests/test_signup.py +++ b/zerver/tests/test_signup.py @@ -627,12 +627,8 @@ class LoginTest(ZulipTestCase): self.assert_logged_in_user_id(None) def test_login_invalid_subdomain(self) -> None: - with self.assertLogs(level='WARNING') as warn_log: - result = self.login_with_return(self.example_email("hamlet"), "xxx", - subdomain="invalid") - self.assertEqual(warn_log.output, [ - 'WARNING:root:User hamlet@zulip.com attempted password login to nonexistent subdomain invalid' - ]) + result = self.login_with_return(self.example_email("hamlet"), "xxx", + subdomain="invalid") self.assertEqual(result.status_code, 404) self.assert_in_response("There is no Zulip organization hosted at this subdomain.", result) self.assert_logged_in_user_id(None) @@ -2407,9 +2403,7 @@ class RealmCreationTest(ZulipTestCase): result = self.submit_reg_form_for_user(email, password, realm_subdomain = string_id, - realm_name=realm_name, - # Pass HTTP_HOST for the target subdomain - HTTP_HOST=string_id + ".testserver") + realm_name=realm_name) self.assertEqual(result.status_code, 302) result = self.client_get(result.url, subdomain=string_id) @@ -2447,8 +2441,7 @@ class RealmCreationTest(ZulipTestCase): result = self.submit_reg_form_for_user(email, password, realm_subdomain = string_id, - realm_name=realm_name, - HTTP_HOST=string_id + ".testserver") + realm_name=realm_name) self.assertEqual(result.status_code, 302) result = self.client_get(result.url, subdomain=string_id)