diff --git a/README.md b/README.md index 2736630..43eaa5d 100644 --- a/README.md +++ b/README.md @@ -152,7 +152,7 @@ client.auth.client.set_bearer_authorization(access_token) ### 401 Auto-Refresh -`with_user_session()` installs a one-shot 401 auto-refresh hook on the auth and api clients by default (issue #89): when a request comes back 401 because the bearer expired mid-session, the session token is force-refreshed once and the request retried with the new Authorization header. If the refresh fails, the original 401 surfaces to the caller as before. Pass `refresh_on_401=False` for the old behaviour. +`with_user_session()` installs a one-shot 401 auto-refresh hook on the auth and api clients by default (issue #89): when a request comes back 401 because the bearer expired mid-session, the session token is re-acquired once via `POST /auth/v1/logins//token` (campus#964) and the request retried with the new Authorization header. If the re-acquisition fails, the original 401 surfaces to the caller as before. Pass `refresh_on_401=False` for the old behaviour. ```python with campus.with_user_session() as client: # hook on (default) diff --git a/campus_python/__init__.py b/campus_python/__init__.py index a545d81..66e55bc 100644 --- a/campus_python/__init__.py +++ b/campus_python/__init__.py @@ -182,34 +182,19 @@ def integrations(self) -> IntegrationsRoot: ) return self._integrations - def _get_token_from_session( - self, - force_refresh=False, - refresh_if_expired=True - ) -> campus.model.OAuthToken: - """Get the token from flask session.""" - # flask session stores a session id - login_session = self.auth.logins.from_session() - user_id = login_session.user_id - if user_id is None: - raise errors.AuthenticationError( - error_description="Login session has no user id." - ) - creds_resource = self.auth.credentials["campus"][user_id] - user_creds = creds_resource.get() - if ( - force_refresh - or refresh_if_expired and user_creds.token.is_expired() - ): - token = self.auth.token( - grant_type="refresh_token", - refresh_token=user_creds.token.refresh_token - ) - self.auth.credentials["campus"][user_id].update(token=token) - # Return the refreshed token — user_creds still holds the - # pre-refresh (now rotated) access token. - return token - return user_creds.token + def _get_token_from_session(self) -> campus.model.OAuthToken: + """Get the token from flask session. + + One endpoint call (campus#963/#964, invariant A10): + POST /logins//token re-acquires the session user's + campus bearer with the client's Basic credentials. The + operator-gated credentials GET → refresh grant → credentials + PATCH legs (#854) are gone, with no fallback to them; the + server rotates an expired grant token itself, so there is no + proactive is_expired() check or write-back here — re-calling + this method (as the 401 hook does) is the refresh. + """ + return self.auth.get_token() def revoke_session(self) -> None: """Revoke the current authorization token.""" @@ -260,12 +245,13 @@ def with_user_session( By default a 401 auto-refresh hook is installed on the auth and api clients for the duration of the session (issue #89): when a request comes back 401 because the bearer expired after the - proactive refresh below, the token is force-refreshed once and - the request retried with the new Authorization header. The - retry cannot double-execute work — Campus services reject - unauthenticated requests in a before_request authenticator - before any handler runs. If the refresh itself fails, the - original 401 response surfaces to the caller as before. Pass + re-acquisition below, the token is re-acquired once via the + login-session token route (campus#964) and the request retried + with the new Authorization header. The retry cannot + double-execute work — Campus services reject unauthenticated + requests in a before_request authenticator before any handler + runs. If the re-acquisition itself fails, the original 401 + response surfaces to the caller as before. Pass refresh_on_401=False for the old behaviour. Yields: @@ -279,18 +265,17 @@ def with_user_session( refreshing = False def refresh_bearer() -> str | None: - """Force-refresh the session token once. + """Re-acquire the session token once. Returns the new bearer token for the client to retry with, or None on failure (the original 401 then - surfaces). Re-entrant calls (the refresh round-trip - itself hitting a 401) bail out immediately. - - The refresh round-trip authenticates as the client - (Basic), the same mode _get_token_from_session runs in - at session establishment — auth routes accept both, - but the session's stale bearer must not be presented - mid-rotation. + surfaces). Re-entrant calls (the re-acquisition + round-trip itself hitting a 401) bail out immediately. + + The round-trip authenticates as the client (Basic): + the login-session token route denies user principals + outright (campus#964/A10), so the session's stale + bearer must not be presented mid-rotation. """ nonlocal refreshing if refreshing: @@ -298,9 +283,7 @@ def refresh_bearer() -> str | None: refreshing = True try: self.revoke_session() - refreshed = self._get_token_from_session( - force_refresh=True - ) + refreshed = self._get_token_from_session() except errors.APIError: self.use_token(token) return None diff --git a/campus_python/auth/v1/__init__.py b/campus_python/auth/v1/__init__.py index e7895f1..cdeda30 100644 --- a/campus_python/auth/v1/__init__.py +++ b/campus_python/auth/v1/__init__.py @@ -306,9 +306,13 @@ def logout(self) -> None: def get_token(self) -> campus.model.OAuthToken: """Convenience method to get access token for a user. - This retrieves the user's credentials which includes their access token. - Allows apps to get a fresh token without re-authenticating via OAuth - if the user has a valid login session. + Re-acquires the login session user's campus bearer via + POST /logins//token (campus#963/#964, invariant + A10): one Basic-authenticated endpoint call in the + session-bound lane — never the operator-gated credentials + routes (#854). An expired grant token is rotated server-side, + so the returned token is always live; the response carries no + refresh token. Args: None @@ -317,7 +321,9 @@ def get_token(self) -> campus.model.OAuthToken: OAuthToken for the user Raises: - AuthenticationError: If no login session or no credentials found + AuthenticationError: If no login session is present + APIError: invalid_grant if the login session or the + user's campus grant is gone — re-login required Example: # In a protected view that needs the user's token @@ -325,28 +331,7 @@ def get_token(self) -> campus.model.OAuthToken: headers = {"Authorization": f"Bearer {token.id}"} response = requests.get("https://api.example.com/data", headers=headers) """ - # Get user_id from login session - if not self.logins.has_session(): - raise errors.AuthenticationError( - error_description="No login session found. User must log in first." - ) - login_session = self.logins.from_session() - user_id = login_session.user_id - if user_id is None: - raise errors.AuthenticationError( - error_description="Login session has no user id." - ) - - # Retrieve credentials for the user - credentials = self.credentials["campus"][user_id].get() - if not credentials.token: - raise errors.AuthenticationError( - error_description=f"No Campus token found for user {user_id}" - ) - - # TODO: Auto-refresh if token is expired and refresh_token is available - # For now, return the token as-is - return credentials.token + return self.logins.token() def _user_cache_get( self, kind: str, session_id: str diff --git a/campus_python/auth/v1/logins.py b/campus_python/auth/v1/logins.py index c104002..9d9fee6 100644 --- a/campus_python/auth/v1/logins.py +++ b/campus_python/auth/v1/logins.py @@ -52,6 +52,23 @@ def has_session(self) -> bool: """Check if there is a session ID stored in the flask session.""" return self._session_key in flask.session + def token(self) -> campus.model.OAuthToken: + """Re-acquire the current login session's campus bearer (#117). + + One endpoint call (campus#964, invariant A10): + POST /logins//token, authenticated by the client's + Basic credentials plus the session id — user bearers are denied + outright, so the caller must not present one (revoke_session() + first mid-rotation). An expired grant token is rotated + server-side; the response omits the refresh token. + + Raises: + AuthenticationError: If no login session is present. + APIError: invalid_grant when the session or the user's + campus grant is gone — the cue to require re-login. + """ + return self[None].token() + def new( self, *, @@ -112,3 +129,26 @@ def update(self, *, expiry_seconds: int) -> campus.model.LoginSession: # Raise error if status code is not 2XX or 3XX resp.raise_for_status() return campus.model.LoginSession.from_resource(resp.json()) + + def token(self) -> campus.model.OAuthToken: + """Re-acquire the session user's campus bearer (#117). + + POST /logins//token — the session-bound + re-acquisition lane (campus#964, invariant A10): the login + session plays the authorization code's role, authenticated + with the client's Basic credentials. Issuance mirrors the + code exchange: an unexpired grant token is reused, an + expired one is minted fresh (rotated server-side), and a + missing grant is an invalid_grant, never minted from + nothing (C5). The response omits the refresh token — + campus refresh material never leaves the auth service in + this lane. + """ + # Canonical path has no trailing slash (route rule is + # //token, unlike the CRUD subresources). + resp = self.client.post( + self.make_path("token") + ) + # Raise error if status code is not 2XX or 3XX + resp.raise_for_status() + return campus.model.OAuthToken.from_resource(resp.json()) diff --git a/poetry.lock b/poetry.lock index 2fb261d..bbc77da 100644 --- a/poetry.lock +++ b/poetry.lock @@ -154,7 +154,7 @@ werkzeug = "^3.0.0" type = "git" url = "https://github.com/nyjc-computing/campus.git" reference = "weekly" -resolved_reference = "5b85acc9c2a6eb49894b64818350a733c56964e5" +resolved_reference = "50e69c9777ff4fc8f54f9eb2738a45a76615f7f1" [[package]] name = "certifi" diff --git a/tests/unit/test_embedded_session_user.py b/tests/unit/test_embedded_session_user.py index 026bb50..8cc74e0 100644 --- a/tests/unit/test_embedded_session_user.py +++ b/tests/unit/test_embedded_session_user.py @@ -40,17 +40,6 @@ "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.""" @@ -160,14 +149,10 @@ def test_session_without_embed_falls_back(self): self.assertTrue(self.users_mock["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" - ) + """A campus-suite predating the #879 field still resolves: the + getattr fallback tolerates session objects with no `user` + attribute at all (pre-#879 model shape), not just user=None.""" + session = SimpleNamespace(user_id="user-1") user = self.push_session(session) self.assertEqual(user.email, USER_RESOURCE["email"]) self.assertTrue(self.users_mock["user-1"].get.called) diff --git a/tests/unit/test_token_refresh.py b/tests/unit/test_token_refresh.py index 996d71f..b00d0ea 100644 --- a/tests/unit/test_token_refresh.py +++ b/tests/unit/test_token_refresh.py @@ -1,8 +1,11 @@ -"""Unit test for Campus._get_token_from_session token rotation. +"""Unit tests for the session-token path. -After exchanging the refresh token, the refreshed token must be the one -returned — the credentials resource still holds the pre-refresh access -token, and refresh-token grants rotate (single-use) on the server. +Campus._get_token_from_session() re-acquires the session user's campus +bearer with ONE call to POST /auth/v1/logins/{session_id}/token +(campus#963/#964, invariant A10) — the operator-gated credentials +GET → refresh grant → credentials PATCH legs (#854) are gone, with no +fallback to them. The server rotates an expired grant token itself, so +the client sends no body and never writes back. Also covers the public-client refresh helper auth.refresh(stored) (client issue #87): it presents the stored refresh token with the @@ -12,9 +15,10 @@ import json import os import unittest -from unittest.mock import MagicMock, Mock, patch +from unittest.mock import Mock, patch import campus.model +import flask import requests from campus_python import Campus, errors @@ -29,6 +33,14 @@ "scope": "campus.profile", } +LOGIN_TOKEN_PAYLOAD = { + "access_token": "tok-live", + "token_type": "Bearer", + "expires_in": 432000, + "scope": "campus.profile", + "user_id": "user1", +} + def make_auth() -> tuple[AuthRoot, Mock]: """Create an AuthRoot backed by a mock JSON client.""" @@ -36,6 +48,16 @@ def make_auth() -> tuple[AuthRoot, Mock]: return AuthRoot(json_client=client), client +def make_campus() -> tuple[Campus, Mock]: + """Create a Campus whose auth root is backed by a mock client.""" + client = Mock() + campus = Campus.__new__(Campus) + campus._mode = "server" + campus.timeout = 5 + campus._auth = AuthRoot(json_client=client) + return campus, client + + def make_response(status_code: int, payload: dict) -> CampusResponse: """Build a CampusResponse over a real requests.Response so raise_for_status() runs the actual error-envelope mapping.""" @@ -46,40 +68,91 @@ def make_response(status_code: int, payload: dict) -> CampusResponse: return CampusResponse(raw) +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 TestGetTokenFromSession(unittest.TestCase): - """The refreshed token, not the stale credentials token, is returned.""" - - def test_returns_refreshed_token_after_refresh(self): - with patch.dict(os.environ, {"CLIENT_ID": "cid", "CLIENT_SECRET": "sec"}): - campus = Campus(timeout=5) - auth = campus.auth - - old_token = Mock() - old_token.is_expired.return_value = True - new_token = Mock() - - auth._logins = Mock() - auth._logins.from_session.return_value = Mock(user_id="user1") - - creds_resource = Mock() - creds_resource.get.return_value = Mock(token=old_token) - provider = MagicMock() - provider.__getitem__.return_value = creds_resource - creds_collection = MagicMock() - creds_collection.__getitem__.return_value = provider - auth._credentials = creds_collection - - with patch.object( - type(auth), "token", return_value=new_token - ) as token_mock: - result = campus._get_token_from_session(force_refresh=True) - - self.assertIs(result, new_token) - token_mock.assert_called_once_with( - grant_type="refresh_token", - refresh_token=old_token.refresh_token, - ) - creds_resource.update.assert_called_once_with(token=new_token) + """The session bearer comes from the login-session token route.""" + + def test_delegates_to_logins_token_once(self): + campus, client = make_campus() + auth = campus.auth + + logins = Mock() + live = Mock(access_token="tok-live") + logins.token.return_value = live + auth._logins = logins + + result = campus._get_token_from_session() + + self.assertIs(result, live) + logins.token.assert_called_once_with() + # One endpoint call, nothing else: no credentials GET, no + # refresh grant, no PATCH write-back (#854 lanes are gone). + client.get.assert_not_called() + client.post.assert_not_called() + client.patch.assert_not_called() + + +class TestLoginSessionTokenRoute(unittest.TestCase): + """logins.token() POSTs /auth/v1/logins/{id}/token (#117).""" + + def setUp(self): + self.auth, self.client = make_auth() + self.client.post.return_value = make_response( + 200, dict(LOGIN_TOKEN_PAYLOAD) + ) + + def test_posts_to_session_token_route(self): + with session_context(): + flask.session["logins_login_id"] = "login-1" + token = self.auth.logins.token() + + self.assertEqual(token.access_token, "tok-live") + self.assertEqual(token.scope, "campus.profile") + # The user_id echo survives as provider metadata (#648) + self.assertEqual(token.provider_fields.get("user_id"), "user1") + args, kwargs = self.client.post.call_args + # Canonical route has no trailing slash; the session id and + # the client's Basic credentials carry the request — no body. + self.assertEqual(args[0], "/auth/v1/logins/login-1/token") + self.assertIsNone(kwargs.get("json")) + + def test_get_token_uses_the_same_route(self): + with session_context(): + flask.session["logins_login_id"] = "login-1" + token = self.auth.get_token() + + self.assertEqual(token.access_token, "tok-live") + self.client.get.assert_not_called() + self.client.patch.assert_not_called() + + def test_no_login_session_raises_without_http(self): + with session_context(): + with self.assertRaises(errors.AuthenticationError): + self.auth.logins.token() + self.client.post.assert_not_called() + + def test_invalid_grant_surfaces_oauth_error(self): + """A gone session or campus grant is invalid_grant — the + caller's cue to require re-login (#87 convention).""" + self.client.post.return_value = make_response(400, { + "error": { + "code": "AUTH_INVALID_GRANT", + "message": "Invalid or expired login session", + "details": {"oauth_error": "invalid_grant"}, + "request_id": None, + } + }) + with session_context(): + flask.session["logins_login_id"] = "login-1" + with self.assertRaises(errors.APIError) as ctx: + self.auth.logins.token() + self.assertEqual(ctx.exception.oauth_error, "invalid_grant") class TestPublicClientRefresh(unittest.TestCase): diff --git a/tests/unit/test_unauthorized_refresh.py b/tests/unit/test_unauthorized_refresh.py index 12d0584..2995f54 100644 --- a/tests/unit/test_unauthorized_refresh.py +++ b/tests/unit/test_unauthorized_refresh.py @@ -7,8 +7,9 @@ original 401 to the caller — the hook is strictly a safety net. Campus.with_user_session(refresh_on_401=True) wires the hook so the -refresh round-trip force-refreshes the session token, authenticating as -the client (Basic) like session establishment does. +refresh round-trip re-acquires the session token via the login-session +token route (campus#964), authenticating as the client (Basic) — the +route denies user principals outright. """ import json @@ -147,8 +148,8 @@ def test_post_retries_like_get(self): class TestWithUserSessionWiring(unittest.TestCase): - """with_user_session() installs a force-refresh hook on both - service clients and removes it when the session closes.""" + """with_user_session() installs a token re-acquisition hook on + both service clients and removes it when the session closes.""" def setUp(self): self.campus = Campus.__new__(Campus) @@ -181,7 +182,7 @@ def setUp(self): token1 = Mock(access_token="tok1") self.get_token.return_value = token1 - def test_401_mid_session_force_refreshes_and_retries(self): + def test_401_mid_session_reacquires_and_retries(self): cast(MagicMock, self.api_client._session).request.side_effect = [ raw_response(401, {}), raw_response(200, {"ok": True}), ] @@ -201,9 +202,10 @@ def test_401_mid_session_force_refreshes_and_retries(self): ) self.assertEqual(resp.status_code, 200) - # Second call is the hook's force-refresh + # Second call is the hook's re-acquisition — same one-endpoint + # path as session establishment (#117): no refresh kwargs. self.assertEqual(self.get_token.call_count, 2) - self.assertTrue(self.get_token.call_args.kwargs["force_refresh"]) + self.assertEqual(self.get_token.call_args.kwargs, {}) def test_refresh_failure_surfaces_original_401(self): cast(MagicMock, self.api_client._session).request.side_effect = [raw_response(401, {})]