From 37a7d1fe7b90118f6d0a85b5a7c1adcc26a7e40e Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Thu, 14 Jul 2022 09:34:17 -0400 Subject: [PATCH] middleware: Reorder middleware to avoid hasattr checks. `request.user` gets set in Django's `AuthenticationMiddleware`, which runs after our `HostDomainMiddleware`. This makes `hasattr` checks necessary in any code path that uses the `request.user` attribute. In this case, there are functions in `context_processors` that get called in the middleware. Since neither `CsrfMiddleware` nor `HostDomainMiddleware` are required to run before `AuthenticationMiddleware`, moving it two slots up in `computed_settings` is sufficient to avoid the `hasattr` checks. Signed-off-by: Zixuan James Li --- zerver/context_processors.py | 8 ++------ zproject/computed_settings.py | 2 +- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/zerver/context_processors.py b/zerver/context_processors.py index 34776bf9b7..bebee1854b 100644 --- a/zerver/context_processors.py +++ b/zerver/context_processors.py @@ -51,7 +51,7 @@ def common_context(user: UserProfile) -> Dict[str, Any]: def get_realm_from_request(request: HttpRequest) -> Optional[Realm]: request_notes = RequestNotes.get_notes(request) - if hasattr(request, "user") and hasattr(request.user, "realm"): + if request.user.is_authenticated: return request.user.realm if not request_notes.has_fetched_realm: # We cache the realm object from this function on the request data, @@ -122,10 +122,6 @@ def zulip_default_context(request: HttpRequest) -> Dict[str, Any]: apps_page_web = settings.ROOT_DOMAIN_URI + "/accounts/go/" - user_is_authenticated = False - if hasattr(request, "user") and hasattr(request.user, "is_authenticated"): - user_is_authenticated = request.user.is_authenticated - if settings.DEVELOPMENT: secrets_path = "zproject/dev-secrets.conf" settings_path = "zproject/dev_settings.py" @@ -169,7 +165,7 @@ def zulip_default_context(request: HttpRequest) -> Dict[str, Any]: "password_min_length": settings.PASSWORD_MIN_LENGTH, "password_min_guesses": settings.PASSWORD_MIN_GUESSES, "zulip_version": ZULIP_VERSION, - "user_is_authenticated": user_is_authenticated, + "user_is_authenticated": request.user.is_authenticated, "settings_path": settings_path, "secrets_path": secrets_path, "settings_comments_path": settings_comments_path, diff --git a/zproject/computed_settings.py b/zproject/computed_settings.py index c36f93b1de..c8a5ae426a 100644 --- a/zproject/computed_settings.py +++ b/zproject/computed_settings.py @@ -178,9 +178,9 @@ MIDDLEWARE = ( "zerver.middleware.ZulipCommonMiddleware", "django.contrib.sessions.middleware.SessionMiddleware", "zerver.middleware.LocaleMiddleware", + "django.contrib.auth.middleware.AuthenticationMiddleware", "zerver.middleware.HostDomainMiddleware", "django.middleware.csrf.CsrfViewMiddleware", - "django.contrib.auth.middleware.AuthenticationMiddleware", "zerver.middleware.ZulipSCIMAuthCheckMiddleware", # Make sure 2FA middlewares come after authentication middleware. "django_otp.middleware.OTPMiddleware", # Required by two factor auth.