From 5e4de7681d0367e56e2ecf83ca37f313d152a643 Mon Sep 17 00:00:00 2001 From: JS Ng Date: Sun, 4 Oct 2026 23:27:48 +0800 Subject: [PATCH] perf(auth): cache push_context's resolved user per session (#57) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit push_context() runs as an app-wide before_request hook, so a session-carrying browser paid 1-2 synchronous auth API calls before every route — enough to wedge sync workers under load (campus-admin WORKER TIMEOUT, campus-profile#22 slow authenticated pages). The resolved user is now cached in-process keyed by session id for user_cache_ttl (default 60s): warm requests make zero upstream calls. logout() and stale-session NotFoundError paths evict immediately; the resource dict is rehydrated per hit so callers cannot poison the cache by mutating flask.g.user. Trade-off: a session revoked server-side — not through logout() — keeps resolving for up to the TTL window. --- campus_python/auth/v1/__init__.py | 87 ++++++++++- tests/unit/test_push_context_cache.py | 212 ++++++++++++++++++++++++++ 2 files changed, 295 insertions(+), 4 deletions(-) create mode 100644 tests/unit/test_push_context_cache.py diff --git a/campus_python/auth/v1/__init__.py b/campus_python/auth/v1/__init__.py index 7271908..6f4b40d 100644 --- a/campus_python/auth/v1/__init__.py +++ b/campus_python/auth/v1/__init__.py @@ -4,7 +4,8 @@ """ import logging -from typing import Literal +import time +from typing import Any, Literal import flask import werkzeug @@ -31,6 +32,12 @@ logger = logging.getLogger(__name__) +# How long push_context() may reuse a session's resolved user (#57). +# push_context runs as an app-wide before_request hook, so without the +# cache a session-carrying browser pays 1-2 synchronous auth API calls +# before every route. Warm requests within the window pay none. +USER_CACHE_TTL_SECONDS = 60.0 + class AuthRoot(ResourceRoot): """Campus Auth resource.""" @@ -48,6 +55,14 @@ def __init__(self, json_client: JsonClient): self._sessions = None self._users = None self._vaults = None + # (kind, session_id) -> (expires_at, user resource, device_id). + # kind namespaces the two session stores: "session" (auth + # sessions) and "login" (login sessions). + self._user_cache: dict[ + tuple[str, str], + tuple[float, dict[str, Any], str | None], + ] = {} + self.user_cache_ttl = USER_CACHE_TTL_SECONDS @property def broker(self) -> broker.Broker: @@ -255,6 +270,7 @@ def logout(self) -> None: try: login_session = self.logins.from_session() self.logins[login_session.id].revoke() + self._user_cache_pop("login", login_session.id) except Exception as err: logger.warning( "Login session revocation failed (continuing with local " @@ -304,31 +320,94 @@ def get_token(self) -> campus.model.OAuthToken: # For now, return the token as-is return credentials.token + def _user_cache_get( + self, kind: str, session_id: str + ) -> tuple[campus.model.User, str | None] | None: + """Return the cached (user, device_id) for a session, or None. + + Expired entries are dropped lazily on read. The resource dict is + rehydrated into a fresh User per hit so callers cannot poison + the cache by mutating flask.g.user. + """ + entry = self._user_cache.get((kind, session_id)) + if entry is None: + return None + expires_at, resource, device_id = entry + if time.monotonic() >= expires_at: + self._user_cache.pop((kind, session_id), None) + return None + return campus.model.User.from_resource(resource), device_id + + def _user_cache_put( + self, + kind: str, + session_id: str, + user: campus.model.User, + device_id: str | None, + ) -> None: + """Cache a session's resolved user for user_cache_ttl seconds.""" + self._user_cache[(kind, session_id)] = ( + time.monotonic() + self.user_cache_ttl, + user.to_resource(), + device_id, + ) + + def _user_cache_pop(self, kind: str, session_id: str) -> None: + """Drop a session's cached user (logout, stale session).""" + self._user_cache.pop((kind, session_id), None) + def push_context(self) -> None: - """Push auth/login context to flask g.""" + """Push auth/login context to flask g. + + Resolved users are cached in-process per session for + user_cache_ttl seconds (#57): a session-carrying request + otherwise pays 1-2 synchronous auth API calls before every + route, which can wedge sync workers under load. Warm requests + make zero upstream calls. Trade-off: a session revoked + server-side — not through logout(), which evicts immediately — + keeps resolving for up to the TTL window. + """ flask.g.user = None flask.g.device = None # Try to load auth session if one exists if self.sessions.has_session(): + session_id = flask.session[self.sessions._session_key] + cached = self._user_cache_get("session", session_id) + if cached is not None: + flask.g.user, flask.g.device = cached + return try: auth_session = self.sessions.from_session() if auth_session.user_id: - flask.g.user = self.users[auth_session.user_id].get() + user = self.users[auth_session.user_id].get() + flask.g.user = user + self._user_cache_put("session", session_id, user, None) except errors.NotFoundError: # Session no longer exists on server (expired, restart, etc.) + self._user_cache_pop("session", session_id) # Clear the stale session reference from Flask session if self.sessions._session_key in flask.session: del flask.session[self.sessions._session_key] # Try to load login session if one exists elif self.logins.has_session(): + login_id = flask.session[self.logins._session_key] + cached = self._user_cache_get("login", login_id) + if cached is not None: + flask.g.user, flask.g.device = cached + return try: login_session = self.logins.from_session() - flask.g.user = self.users[login_session.user_id].get() + user = self.users[login_session.user_id].get() + flask.g.user = user flask.g.device = login_session.device_id + self._user_cache_put( + "login", login_id, user, login_session.device_id + ) except errors.NotFoundError: # Login session no longer exists on server + self._user_cache_pop("login", login_id) # Clear the stale session reference from Flask session if self.logins._session_key in flask.session: del flask.session[self.logins._session_key] diff --git a/tests/unit/test_push_context_cache.py b/tests/unit/test_push_context_cache.py new file mode 100644 index 0000000..bd4bb63 --- /dev/null +++ b/tests/unit/test_push_context_cache.py @@ -0,0 +1,212 @@ +"""Unit tests for the push_context user cache (issue #57). + +push_context() runs as an app-wide before_request hook, so a +session-carrying browser paid 1-2 upstream auth calls before every +route. The resolved user is now cached in-process per session for +user_cache_ttl seconds: warm requests make zero upstream calls, and +logout() and stale sessions evict immediately. +""" + +import unittest +from unittest.mock import Mock + +import flask + +from campus_python import errors +from campus_python.auth.v1 import AuthRoot + +USER_RESOURCE = { + "id": "user-1", + "created_at": "2026-10-01T00:00:00+00:00", + "email": "student@nyjc.edu.sg", + "name": "Test Student", +} + +AUTH_SESSION = { + "id": "auth-session-1", + "created_at": "2026-10-01T00:00:00+00:00", + "expires_at": "2026-11-01T00:00:00+00:00", + "provider": "campus", + "client_id": "client-1", + "user_id": "user-1", + "redirect_uri": "https://app.example.org/finalize_login", + "scopes": [], +} + +LOGIN_SESSION = { + "id": "login-1", + "created_at": "2026-10-01T00:00:00+00:00", + "expires_at": "2026-11-01T00:00:00+00:00", + "client_id": "client-1", + "user_id": "user-1", + "device_id": "device-1", + "agent_string": "pytest", +} + + +def make_auth() -> tuple[AuthRoot, Mock]: + """Create an AuthRoot backed by a mock JSON client.""" + client = Mock() + return AuthRoot(json_client=client), client + + +def session_context(): + """A Flask request context with a writable session.""" + app = flask.Flask(__name__) + app.secret_key = "test-secret" + return app.test_request_context() + + +def ok_response(body: dict) -> Mock: + response = Mock() + response.json.return_value = body + return response + + +def not_found_response() -> Mock: + response = Mock() + response.raise_for_status.side_effect = errors.NotFoundError( + error_description="not found" + ) + return response + + +def route_gets(client: Mock, **routes: Mock) -> None: + """Route client.get calls to responses by path substring.""" + def get(path, *_args, **_kwargs): + for fragment, response in routes.items(): + if fragment in path: + return response + raise AssertionError(f"unexpected GET path: {path}") + client.get.side_effect = get + + +class TestSessionBranchCache(unittest.TestCase): + """The auth-session branch caches under ("session", id).""" + + def setUp(self): + self.auth, self.client = make_auth() + + def push(self): + with session_context(): + flask.session["campus_session_id"] = "auth-session-1" + route_gets( + self.client, + **{ + "sessions/campus": ok_response(AUTH_SESSION), + "users/": ok_response(USER_RESOURCE), + }, + ) + self.auth.push_context() + return flask.g.user, flask.g.device + + def test_first_request_resolves_via_two_calls(self): + user, device = self.push() + self.assertEqual(user.email, USER_RESOURCE["email"]) + self.assertIsNone(device) + self.assertEqual(self.client.get.call_count, 2) + + def test_warm_request_makes_no_upstream_calls(self): + self.push() + calls_after_warm = self.client.get.call_count + user, _ = self.push() + self.assertEqual(self.client.get.call_count, calls_after_warm) + self.assertEqual(user.name, USER_RESOURCE["name"]) + + def test_ttl_zero_forces_refetch_every_request(self): + self.auth.user_cache_ttl = 0 + self.push() + calls_first = self.client.get.call_count + self.push() + self.assertGreater(self.client.get.call_count, calls_first) + + def test_mutating_g_user_does_not_poison_cache(self): + user, _ = self.push() + user.name = "tampered" + user_again, _ = self.push() + self.assertEqual(user_again.name, USER_RESOURCE["name"]) + + +class TestLoginBranchCache(unittest.TestCase): + """The login-session branch caches under ("login", id) with device.""" + + def setUp(self): + self.auth, self.client = make_auth() + + def push(self): + with session_context(): + flask.session["logins_login_id"] = "login-1" + route_gets( + self.client, + **{ + "logins/": ok_response(LOGIN_SESSION), + "users/": ok_response(USER_RESOURCE), + }, + ) + self.auth.push_context() + return flask.g.user, flask.g.device + + def test_first_request_resolves_user_and_device(self): + user, device = self.push() + self.assertEqual(user.id, USER_RESOURCE["id"]) + self.assertEqual(device, "device-1") + self.assertEqual(self.client.get.call_count, 2) + + def test_warm_request_makes_no_upstream_calls(self): + self.push() + calls_after_warm = self.client.get.call_count + user, device = self.push() + self.assertEqual(self.client.get.call_count, calls_after_warm) + self.assertEqual(device, "device-1") + + def test_logout_evicts_the_cached_user(self): + self.push() + self.client.delete.return_value = ok_response({}) + with session_context(): + flask.session["logins_login_id"] = "login-1" + self.auth.logout() + self.push() + # from_session + revoke used get/delete after logout; the + # re-resolve adds two more GETs on top of the login lookup. + self.assertGreaterEqual(self.client.get.call_count, 4) + + +class TestStaleSessionEviction(unittest.TestCase): + """A session the auth service no longer knows is evicted.""" + + def test_stale_auth_session_clears_cookie_and_cache(self): + auth, client = make_auth() + with session_context(): + flask.session["campus_session_id"] = "auth-session-1" + route_gets( + client, + **{"sessions/campus": not_found_response()}, + ) + auth.push_context() + self.assertIsNone(flask.g.user) + self.assertNotIn("campus_session_id", flask.session) + # A later valid session re-resolves instead of trusting cache + flask.session["campus_session_id"] = "auth-session-2" + route_gets( + client, + **{ + "sessions/campus": ok_response(AUTH_SESSION), + "users/": ok_response(USER_RESOURCE), + }, + ) + auth.push_context() + self.assertEqual(flask.g.user.email, USER_RESOURCE["email"]) + self.assertEqual(client.get.call_count, 3) + + def test_stale_login_session_clears_cookie(self): + auth, client = make_auth() + with session_context(): + flask.session["logins_login_id"] = "login-1" + route_gets(client, **{"logins/": not_found_response()}) + auth.push_context() + self.assertIsNone(flask.g.user) + self.assertNotIn("logins_login_id", flask.session) + + +if __name__ == "__main__": + unittest.main()