fix minor authentication bug

tildes/tildes/auth.py

@@ -33,8 +33,12 @@ class DefaultRootFactory:
         pass
 
 
-def get_authenticated_user(request: Request) -> Optional[User]:
-    """Return the User object for the authed user making the request."""
+def get_unauthenticated_user(request: Request) -> Optional[User]:
+    """Return the User object for the unauthenticated user making the request.
+
+    This is used to get the user object before authentication has been processed.
+    It is cached in the request for performance reasons.
+    """
     user_id = request.unauthenticated_userid
     if not user_id:
         return None
@@ -48,25 +52,47 @@ def get_authenticated_user(request: Request) -> Optional[User]:
     return query.one_or_none()
 
 
+def get_authenticated_user(request: Request) -> Optional[User]:
+    """Return the User object for the authed user making the request."""
+
+    # Simply calling the property authenticated_userid will trigger
+    # auth_callback to be called. If that returns not None, We can
+    # trust the user object we cached earlier as authenticated.
+    user_id = request.authenticated_userid
+    if not user_id:
+        return None
+
+    return request.unauthenticated_user
+
+
 def auth_callback(user_id: int, request: Request) -> Optional[Sequence[str]]:
     """Return authorization principals for a user_id from the session.
 
-    This is a callback function needed by SessionAuthenticationPolicy. It should return
+    This is a callback function needed by our AuthenticationPolicies. It should return
     None if the user_id does not exist (such as a deleted user).
     """
-    if not request.user:
+
+    # This function needs to verify that the user exists and is not deleted/banned,
+    # This callback is called every time request.authenticated_userid is used.
+    # It is also called whenever someone calls has_permission()
+    # That means it should be fast and not do any expensive queries.
+
+    if not request.unauthenticated_user:
         return None
 
     # if the user is deleted or banned, log them out
     # (is there a better place to do this?)
-    if request.user.is_banned or request.user.is_deleted:
+    if (
+        request.unauthenticated_user.is_banned
+        or request.unauthenticated_user.is_deleted
+    ):
         request.session.invalidate()
         raise HTTPFound("/")
 
-    if user_id != request.user.user_id:
+    if user_id != request.unauthenticated_user.user_id:
         raise AssertionError("auth_callback called with different user_id")
 
-    return request.user.auth_principals
+    return request.unauthenticated_user.auth_principals
 
 
 def includeme(config: Configurator) -> None:
@@ -87,6 +113,12 @@ def includeme(config: Configurator) -> None:
     # enable CSRF checking globally by default
     config.set_default_csrf_options(require_csrf=True)
 
+    # make the partially authenticated User object available as request.user
+    # Mainly for caching during authentication, so we can avoid expensive queries
+    config.add_request_method(
+        get_unauthenticated_user, "unauthenticated_user", reify=True
+    )
+
     # make the logged-in User object available as request.user
     config.add_request_method(get_authenticated_user, "user", reify=True)