middleware: Make HostDomain into a process_request, not process_response.

It is more suited for `process_request`, since it should stop
execution of the request if the domain is invalid.  This code was
likely added as a process_response (in ea39fb2556) because there was
already a process_response at the time (added 7e786d5426, and no
longer necessary since dce6b4a40f).

It quiets an unnecessary warning when logging in at a non-existent
realm.

This stops performing unnecessary work when we are going to throw it
away and return a 404.  The edge case to this is if the request
_creates_ a realm, and is made using the URL of the new realm; this
change would prevent the request before it occurs. While this does
arise in tests, the tests do not reflect reality -- real requests to
/accounts/register/ are made via POST to the same (default) realm,
redirected there from `confirm-preregistrationuser`.  The tests are
adjusted to reflect real behavior.

Tweaked by tabbott to add a block comment in HostDomainMiddleware.

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:

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):
     """

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)