From d185457a595a1c0a9f44aa95ef9af24a14e4f024 Mon Sep 17 00:00:00 2001 From: JS Date: Sat, 10 Oct 2026 08:14:00 +0800 Subject: [PATCH] fix(auth): prefer the embedded session user in finalize/push_context (#879) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The auth service now embeds the session user record in session reads, finalization, and login-session reads for the session-owning client (campus companion change for campus#879). finalize() answers the user-exists check from the embed and push_context() hydrates g.user from it, falling back to the operator-gated users route on pre-#879 services or campus-suite installs without the model field. Session.finalize() returns (target, user) instead of target alone — the only in-repo caller is Auth.finalize(). --- campus_python/auth/v1/__init__.py | 30 +++- campus_python/auth/v1/sessions.py | 11 +- tests/unit/test_embedded_session_user.py | 187 +++++++++++++++++++++++ tests/unit/test_finalize_device_id.py | 3 +- tests/unit/test_sessions.py | 23 ++- 5 files changed, 242 insertions(+), 12 deletions(-) create mode 100644 tests/unit/test_embedded_session_user.py diff --git a/campus_python/auth/v1/__init__.py b/campus_python/auth/v1/__init__.py index 9f65acc..03384a7 100644 --- a/campus_python/auth/v1/__init__.py +++ b/campus_python/auth/v1/__init__.py @@ -199,12 +199,18 @@ def finalize( redirect_uri=auth_session.redirect_uri ) - # 3. Finalize session and get target - target = self.sessions[auth_session.id].finalize() + # 3. Finalize session and get target; the response embeds the + # session user for the owning client (#879). Pre-#879 services + # return no user (None). + target, session_user = self.sessions[auth_session.id].finalize() # 4. Ensure user exists (the server provisions the user record - # during verify_login; a missing user is a hard error here) - self.users[auth_session.user_id].get() + # during verify_login; a missing user is a hard error here). + # The embedded record (#879) usually answers this without the + # operator-gated users route; the explicit fetch remains as the + # fallback for older auth services. + if session_user is None: + self.users[auth_session.user_id].get() # 5. Create login session # 5. Create login session. The device id rides the auth session @@ -389,7 +395,15 @@ def push_context(self) -> None: try: auth_session = self.sessions.from_session() if auth_session.user_id: - user = self.users[auth_session.user_id].get() + # The auth service embeds the session user for the + # owning client (#879); the operator-gated users + # route remains the fallback for older services. + # getattr guards installs whose campus-suite predates + # the embedded-user model field. + user = ( + getattr(auth_session, "user", None) + or self.users[auth_session.user_id].get() + ) flask.g.user = user self._user_cache_put("session", session_id, user, None) except errors.NotFoundError: @@ -408,7 +422,11 @@ def push_context(self) -> None: return try: login_session = self.logins.from_session() - user = self.users[login_session.user_id].get() + # Embedded login user (#879) with the same fallback. + user = ( + getattr(login_session, "user", None) + or self.users[login_session.user_id].get() + ) flask.g.user = user flask.g.device = login_session.device_id self._user_cache_put( diff --git a/campus_python/auth/v1/sessions.py b/campus_python/auth/v1/sessions.py index 7f2f23e..0ca6b15 100644 --- a/campus_python/auth/v1/sessions.py +++ b/campus_python/auth/v1/sessions.py @@ -96,8 +96,9 @@ class Session(Resource): def session_id(self) -> str: return self.path.split("/")[-1] - def finalize(self) -> str: - # DELETE /sessions/{provider}/{session_id} -> {"target": } + def finalize(self) -> "tuple[str, campus.model.User | None]": + # DELETE /sessions/{provider}/{session_id} -> + # {"target": , "user": {...}|null} resp = self.client.delete( self.make_path(end_slash=True) ) @@ -109,7 +110,11 @@ def finalize(self) -> str: cast(CampusSessions, self.parent)._session_key, None ) body = resp.json() - return body["target"] + # The auth service embeds the session user for the owning + # client (#879); None from pre-#879 services. + user = campus.model.User.from_resource(body["user"]) \ + if body.get("user") else None + return body["target"], user def get(self) -> campus.model.AuthSession: resp = self.client.get( diff --git a/tests/unit/test_embedded_session_user.py b/tests/unit/test_embedded_session_user.py new file mode 100644 index 0000000..ca0127b --- /dev/null +++ b/tests/unit/test_embedded_session_user.py @@ -0,0 +1,187 @@ +"""Unit tests for the embedded session user (#879). + +The auth service embeds the session user in session reads, +finalization, and login-session reads for the session-owning client; +the SDK prefers the embed and falls back to the operator-gated users +route on older services or model versions (campus#879, companion to +the campus-side embedding change). +""" + +import os +import unittest +from types import SimpleNamespace +from unittest.mock import MagicMock, Mock, patch + +import campus.model +import flask + +from campus_python.auth.v1 import AuthRoot + +SESSION_ID = "auth-session-1" + +USER_RESOURCE = { + "id": "user-1", + "created_at": "2026-10-01T00:00:00+00:00", + "email": "student@nyjc.edu.sg", + "name": "Test Student", +} + +EMBEDDED_USER = campus.model.User.from_resource(USER_RESOURCE) + +TOKEN_RESOURCE = { + "id": "token-1", + "created_at": "2026-10-05T00:00:00+00:00", + "expires_at": "2026-10-12T00:00:00+00:00", + "expires_in": 604800, + "token_type": "Bearer", + "refresh_token": "refresh-1", + "scope": "campus.profile", + "user_id": "user-1", +} + +AUTH_SESSION_RESOURCE = { + "id": SESSION_ID, + "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": [], +} + + +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() + + +class TestFinalizeUsesEmbed(unittest.TestCase): + """finalize() answers step 4 from the embed when present.""" + + def setUp(self): + self.auth, _ = make_auth() + + def run_finalize(self, finalize_return: tuple) -> None: + sessions = MagicMock() + sessions.from_code.return_value = SimpleNamespace( + id=SESSION_ID, + user_id="user-1", + redirect_uri="https://app.example.org/finalize_login", + device_id=None, + ) + sessions[SESSION_ID].finalize.return_value = finalize_return + self.auth._sessions = sessions + users = MagicMock() + users["user-1"].get.return_value = EMBEDDED_USER + self.auth._users = users + logins = MagicMock() + self.auth._logins = logins + token_response = Mock() + token_response.json.return_value = TOKEN_RESOURCE + self.auth.client.post.return_value = token_response + + app = flask.Flask(__name__) + app.secret_key = "test-secret" + with patch.dict(os.environ, {"CLIENT_ID": "cid", "CLIENT_SECRET": "sec"}), \ + app.test_request_context( + "/finalize_login", headers={"User-Agent": "pytest"}): + self.auth.finalize( + state=SESSION_ID, code="code-1", scope="campus.profile" + ) + + def test_embedded_user_skips_users_route(self): + """The embed answers the user-exists check without users.get.""" + self.run_finalize(("https://app.example.org/landing", EMBEDDED_USER)) + self.assertFalse(self.auth._users["user-1"].get.called) + + def test_no_embed_falls_back_to_users_route(self): + """Pre-#879 services return no embed; users.get stays the check.""" + self.run_finalize(("https://app.example.org/landing", None)) + self.assertTrue(self.auth._users["user-1"].get.called) + + +class TestPushContextUsesEmbed(unittest.TestCase): + """push_context() hydrates g.user from the embed when present.""" + + def setUp(self): + self.auth, _ = make_auth() + + def push_session(self, session_obj) -> object: + sessions = MagicMock() + sessions._session_key = "campus_session_id" + sessions.has_session.return_value = True + sessions.from_session.return_value = session_obj + self.auth._sessions = sessions + users = MagicMock() + users["user-1"].get.return_value = EMBEDDED_USER + self.auth._users = users + with session_context(): + flask.session["campus_session_id"] = SESSION_ID + self.auth.push_context() + return flask.g.user + + def push_login(self, login_obj) -> object: + logins = MagicMock() + logins._session_key = "logins_login_id" + logins.has_session.return_value = True + logins.from_session.return_value = login_obj + self.auth._logins = logins + users = MagicMock() + users["user-1"].get.return_value = EMBEDDED_USER + self.auth._users = users + with session_context(): + flask.session["logins_login_id"] = "login-1" + self.auth.push_context() + return flask.g.user + + def test_session_embed_skips_users_route(self): + session = SimpleNamespace(user_id="user-1", user=EMBEDDED_USER) + user = self.push_session(session) + self.assertEqual(user.email, USER_RESOURCE["email"]) + self.assertFalse(self.auth._users["user-1"].get.called) + + def test_session_without_embed_falls_back(self): + session = SimpleNamespace(user_id="user-1", user=None) + user = self.push_session(session) + self.assertEqual(user.email, USER_RESOURCE["email"]) + self.assertTrue(self.auth._users["user-1"].get.called) + + def test_model_without_user_field_falls_back(self): + """An installed campus-suite predating the field still resolves.""" + session = campus.model.AuthSession.from_resource( + AUTH_SESSION_RESOURCE + ) + assert not hasattr(session, "user"), ( + "campus-suite now carries the field; this skew case needs a " + "pre-#879 fixture instead" + ) + user = self.push_session(session) + self.assertEqual(user.email, USER_RESOURCE["email"]) + self.assertTrue(self.auth._users["user-1"].get.called) + + def test_login_embed_skips_users_route(self): + login = SimpleNamespace( + user_id="user-1", user=EMBEDDED_USER, device_id="device-1" + ) + user = self.push_login(login) + self.assertEqual(user.email, USER_RESOURCE["email"]) + self.assertFalse(self.auth._users["user-1"].get.called) + + def test_login_without_embed_falls_back(self): + login = SimpleNamespace(user_id="user-1", user=None, device_id=None) + user = self.push_login(login) + self.assertEqual(user.email, USER_RESOURCE["email"]) + self.assertTrue(self.auth._users["user-1"].get.called) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/test_finalize_device_id.py b/tests/unit/test_finalize_device_id.py index 2f19014..1a1827f 100644 --- a/tests/unit/test_finalize_device_id.py +++ b/tests/unit/test_finalize_device_id.py @@ -53,7 +53,8 @@ def run_finalize(auth: AuthRoot, device_id: str | None) -> dict: sessions = MagicMock() sessions.from_code.return_value = make_auth_session(device_id) sessions[SESSION_ID].finalize.return_value = ( - "https://app.example.org/landing" + "https://app.example.org/landing", + None, ) auth._sessions = sessions auth._users = MagicMock() diff --git a/tests/unit/test_sessions.py b/tests/unit/test_sessions.py index 428d90e..078fabf 100644 --- a/tests/unit/test_sessions.py +++ b/tests/unit/test_sessions.py @@ -58,15 +58,34 @@ def setUp(self): def test_finalize_removes_session_key(self): with session_context(): flask.session["campus_session_id"] = "sess-1" - target = self.auth.sessions["sess-1"].finalize() + target, user = self.auth.sessions["sess-1"].finalize() self.assertEqual(target, "https://app.example.org/after-login") + self.assertIsNone(user) self.assertNotIn("campus_session_id", flask.session) def test_finalize_succeeds_without_session_key(self): with session_context(): - target = self.auth.sessions["sess-1"].finalize() + target, _ = self.auth.sessions["sess-1"].finalize() self.assertEqual(target, "https://app.example.org/after-login") + def test_finalize_parses_embedded_user(self): + """The #879 embed rides along as a User model.""" + self.client.delete.return_value = ok_response({ + "target": "https://app.example.org/after-login", + "user": { + "id": "user-1", + "created_at": "2026-10-05T00:00:00+00:00", + "email": "user@example.org", + "name": "Test User", + }, + }) + with session_context(): + target, user = self.auth.sessions["sess-1"].finalize() + self.assertEqual(target, "https://app.example.org/after-login") + self.assertIsNotNone(user) + self.assertEqual(user.id, "user-1") + self.assertEqual(user.email, "user@example.org") + class TestLoginRevoke(unittest.TestCase): """Login.revoke() must tolerate an absent Flask session key."""