diff --git a/campus_python/auth/v1/logins.py b/campus_python/auth/v1/logins.py index 9d9fee6..6ec46fa 100644 --- a/campus_python/auth/v1/logins.py +++ b/campus_python/auth/v1/logins.py @@ -143,12 +143,32 @@ def token(self) -> campus.model.OAuthToken: nothing (C5). The response omits the refresh token — campus refresh material never leaves the auth service in this lane. + + The call presents Basic client credentials, never the + ambient bearer: the route denies user principals outright + (#963/A10), and with_user_session() nested inside another + with_user_session() (e.g. an auth-broker release reached + from a route that already holds the user session) would + otherwise present the outer session's user token and be + refused with 403. Any prior Authorization header is + restored afterwards, so an enclosing session keeps its + authorization until it re-runs use_token() with the fresh + bearer. """ # Canonical path has no trailing slash (route rule is # //token, unlike the CRUD subresources). - resp = self.client.post( - self.make_path("token") - ) + prior_authorization = self.client.authorization + self.client.reset_authorization() + try: + resp = self.client.post( + self.make_path("token") + ) + finally: + if prior_authorization is not None and prior_authorization.startswith( + "Bearer "): + self.client.set_bearer_authorization( + prior_authorization[len("Bearer "):] + ) # 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/campus_python/json_client/__init__.py b/campus_python/json_client/__init__.py index c7904c8..2ae7cab 100644 --- a/campus_python/json_client/__init__.py +++ b/campus_python/json_client/__init__.py @@ -130,6 +130,11 @@ def reset_authorization(self) -> None: secret=env.CLIENT_SECRET ) + @property + def authorization(self) -> str | None: + """The current Authorization header value, or None when unset.""" + return self._session.headers.get("Authorization") + def set_basic_authorization(self, client_id: str, secret: str) -> None: """Set Basic Authorization header using a pre-encoded token. diff --git a/campus_python/json_client/interface.py b/campus_python/json_client/interface.py index e23b150..4a7e500 100644 --- a/campus_python/json_client/interface.py +++ b/campus_python/json_client/interface.py @@ -131,6 +131,11 @@ def __init__( def reset_authorization(self) -> None: """Reset authorization back to client credentials from env.""" + @property + @abstractmethod + def authorization(self) -> str | None: + """The current Authorization header value, or None when unset.""" + @abstractmethod def set_basic_authorization(self, client_id: str, secret: str) -> None: """Set Basic Authorization header using a pre-encoded token. diff --git a/tests/unit/test_token_refresh.py b/tests/unit/test_token_refresh.py index b00d0ea..cd0bc3f 100644 --- a/tests/unit/test_token_refresh.py +++ b/tests/unit/test_token_refresh.py @@ -43,8 +43,14 @@ def make_auth() -> tuple[AuthRoot, Mock]: - """Create an AuthRoot backed by a mock JSON client.""" + """Create an AuthRoot backed by a mock JSON client. + + headers.get defaults to None (no ambient Authorization header) — + token() restores whatever header it finds, so tests that exercise + the ambient-bearer path override this. + """ client = Mock() + client.authorization = None # property read in token() return AuthRoot(json_client=client), client @@ -155,6 +161,63 @@ def test_invalid_grant_surfaces_oauth_error(self): self.assertEqual(ctx.exception.oauth_error, "invalid_grant") +class TestLoginTokenPresentsBasicOnly(unittest.TestCase): + """The session-token route serves client principals only (#963/A10): + token() must present Basic credentials even when an ambient bearer + is set — a with_user_session() nested inside another one otherwise + presents the outer session's user token and is refused with 403. + The prior header is restored afterwards.""" + + def setUp(self): + self.auth, self.client = make_auth() + self.client.post.return_value = make_response( + 200, dict(LOGIN_TOKEN_PAYLOAD) + ) + + def _call(self): + with session_context(): + flask.session["logins_login_id"] = "login-1" + return self.auth.logins.token() + + def test_forces_basic_when_bearer_is_set(self): + self.client.authorization = "Bearer outer-user-token" + self._call() + calls = [name for name, _, _ in self.client.mock_calls + if name in ("reset_authorization", "post", + "set_bearer_authorization")] + # The exact ordering contract: drop the ambient bearer (Basic), + # make the call, then restore it. + self.assertEqual( + ["reset_authorization", "post", "set_bearer_authorization"], calls + ) + + def test_restores_ambient_bearer_after_the_call(self): + self.client.authorization = "Bearer outer-user-token" + self._call() + self.client.set_bearer_authorization.assert_called_once_with( + "outer-user-token" + ) + + def test_failure_still_restores_ambient_bearer(self): + self.client.authorization = "Bearer outer-user-token" + self.client.post.return_value = make_response(403, { + "error": {"code": "FORBIDDEN", "message": "denied"} + }) + with session_context(): + flask.session["logins_login_id"] = "login-1" + with self.assertRaises(errors.APIError): + self.auth.logins.token() + self.client.set_bearer_authorization.assert_called_once_with( + "outer-user-token" + ) + + def test_no_ambient_header_leaves_basic_in_place(self): + self.client.headers.get.return_value = None + self._call() + self.client.reset_authorization.assert_called_once() + self.client.set_bearer_authorization.assert_not_called() + + class TestPublicClientRefresh(unittest.TestCase): """auth.refresh(stored) presents the stored refresh token with the issuing client_id and returns the rotated pair (#87)."""