Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 23 additions & 3 deletions campus_python/auth/v1/logins.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
# /<session_id>/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())
5 changes: 5 additions & 0 deletions campus_python/json_client/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
5 changes: 5 additions & 0 deletions campus_python/json_client/interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
65 changes: 64 additions & 1 deletion tests/unit/test_token_refresh.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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)."""
Expand Down
Loading