Skip to content

Commit 676cc1f

Browse files
committed
Harden OAuth issuer-binding and step-up scope on edge-case paths
Three follow-up fixes to the SEP-2352 / SEP-2350 work: - Backfill an omitted token-response `scope` with the requested scope (RFC 6749 §5.1/§6) before persisting, so the SEP-2350 step-up union still recovers prior grants after a process restart when the authorization server omits `scope` because granted == requested. Applied to both the authorization-code and refresh response handlers. - Clear cached `oauth_metadata` when the SEP-2352 issuer-mismatch block discards bound credentials, so a subsequent ASM-discovery failure for the new authorization server cannot leave the old server's registration/token endpoints in place for Step 4. - Re-evaluate the SEP-2352 issuer-binding check against `oauth_metadata.issuer` after ASM discovery on the legacy no-PRM path (PRM discovery failed, AS found via root well-known fallback), mirroring the existing stamping-side fallback so stale credentials are still discarded when the binding can only be learned post-ASM.
1 parent 984ad41 commit 676cc1f

2 files changed

Lines changed: 134 additions & 1 deletion

File tree

src/mcp/client/auth/oauth2.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,13 @@ async def _handle_token_response(self, response: httpx.Response) -> None:
424424
# Parse and validate response with scope validation
425425
token_response = await handle_token_response_scopes(response)
426426

427+
# RFC 6749 §5.1: an omitted scope means the granted scope equals the requested
428+
# scope. Record it explicitly so the persisted token is self-describing — the
429+
# SEP-2350 step-up union reads it after a restart, when client_metadata.scope
430+
# has reverted to its constructor value.
431+
if token_response.scope is None:
432+
token_response.scope = self.context.client_metadata.scope
433+
427434
# Store tokens in context
428435
self.context.current_tokens = token_response
429436
self.context.update_token_expiry(token_response)
@@ -470,6 +477,12 @@ async def _handle_refresh_response(self, response: httpx.Response) -> bool:
470477
content = await response.aread()
471478
token_response = OAuthToken.model_validate_json(content)
472479

480+
# RFC 6749 §6: an omitted scope on refresh means the scope is unchanged from
481+
# the prior access token. Carry it forward so the persisted token stays
482+
# self-describing for the SEP-2350 step-up union after a restart.
483+
if token_response.scope is None and self.context.current_tokens is not None:
484+
token_response.scope = self.context.current_tokens.scope
485+
473486
self.context.current_tokens = token_response
474487
self.context.update_token_expiry(token_response)
475488
await self.context.storage.set_tokens(token_response)
@@ -578,6 +591,9 @@ async def async_auth_flow(self, request: httpx.Request) -> AsyncGenerator[httpx.
578591
logger.debug("Authorization server changed; discarding bound credentials and re-registering")
579592
self.context.client_info = None
580593
self.context.clear_tokens()
594+
# Any cached AS metadata is for the old server; drop it so a failed
595+
# rediscovery cannot leak the old registration/token endpoints into Step 4.
596+
self.context.oauth_metadata = None
581597

582598
asm_discovery_urls = build_oauth_authorization_server_metadata_discovery_urls(
583599
self.context.auth_server_url, self.context.server_url
@@ -600,6 +616,23 @@ async def async_auth_flow(self, request: httpx.Request) -> AsyncGenerator[httpx.
600616
else:
601617
logger.debug(f"OAuth metadata discovery failed: {url}")
602618

619+
# SEP-2352: on the legacy no-PRM path the issuer is only known after ASM
620+
# discovery, so re-evaluate the binding here using the discovered metadata
621+
# issuer (mirroring the bound_issuer fallback in Step 4).
622+
if (
623+
self.context.client_info is not None
624+
and self.context.auth_server_url is None
625+
and self.context.oauth_metadata is not None
626+
and not credentials_match_issuer(
627+
self.context.client_info,
628+
str(self.context.oauth_metadata.issuer),
629+
self.context.client_metadata_url,
630+
)
631+
):
632+
logger.debug("Authorization server changed; discarding bound credentials and re-registering")
633+
self.context.client_info = None
634+
self.context.clear_tokens()
635+
603636
# Step 3: Apply scope selection strategy
604637
self.context.client_metadata.scope = get_client_metadata_scopes(
605638
extract_scope_from_www_auth(response),

tests/client/test_auth.py

Lines changed: 101 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ def __init__(self):
5151
self._client_info: OAuthClientInformationFull | None = None
5252

5353
async def get_tokens(self) -> OAuthToken | None:
54-
return self._tokens # pragma: no cover
54+
return self._tokens
5555

5656
async def set_tokens(self, tokens: OAuthToken) -> None:
5757
self._tokens = tokens
@@ -2833,3 +2833,103 @@ def test_credentials_match_issuer_url_shaped_dcr_id_is_not_portable():
28332833
issuer="https://as.example.com",
28342834
)
28352835
assert credentials_match_issuer(info, "https://other", "https://client.example/metadata.json") is False
2836+
2837+
2838+
@pytest.mark.anyio
2839+
async def test_handle_token_response_backfills_omitted_scope_from_request(
2840+
oauth_provider: OAuthClientProvider, mock_storage: MockTokenStorage
2841+
):
2842+
"""RFC 6749 §5.1: an omitted token-response scope means granted == requested.
2843+
2844+
The token is stored with the requested scope filled in so it remains self-describing
2845+
after a restart, when the SEP-2350 step-up union reads it but ``client_metadata.scope``
2846+
has reverted to its constructor value.
2847+
"""
2848+
oauth_provider.context.client_metadata.scope = "read admin"
2849+
response = httpx.Response(
2850+
200,
2851+
json={"access_token": "t", "token_type": "Bearer", "expires_in": 3600},
2852+
request=httpx.Request("POST", "https://auth.example.com/token"),
2853+
)
2854+
await oauth_provider._handle_token_response(response)
2855+
2856+
assert oauth_provider.context.current_tokens is not None
2857+
assert oauth_provider.context.current_tokens.scope == "read admin"
2858+
stored = await mock_storage.get_tokens()
2859+
assert stored is not None
2860+
assert stored.scope == "read admin"
2861+
2862+
2863+
@pytest.mark.anyio
2864+
async def test_handle_refresh_response_carries_prior_scope_when_response_omits_it(
2865+
oauth_provider: OAuthClientProvider, mock_storage: MockTokenStorage
2866+
):
2867+
"""RFC 6749 §6: an omitted refresh-response scope means scope is unchanged from the prior token."""
2868+
oauth_provider.context.current_tokens = OAuthToken(access_token="old", scope="read write")
2869+
response = httpx.Response(
2870+
200,
2871+
json={"access_token": "new", "token_type": "Bearer", "expires_in": 3600},
2872+
request=httpx.Request("POST", "https://auth.example.com/token"),
2873+
)
2874+
ok = await oauth_provider._handle_refresh_response(response)
2875+
2876+
assert ok is True
2877+
assert oauth_provider.context.current_tokens is not None
2878+
assert oauth_provider.context.current_tokens.access_token == "new"
2879+
assert oauth_provider.context.current_tokens.scope == "read write"
2880+
stored = await mock_storage.get_tokens()
2881+
assert stored is not None
2882+
assert stored.scope == "read write"
2883+
2884+
2885+
@pytest.mark.anyio
2886+
async def test_issuer_binding_re_evaluated_after_asm_when_prm_discovery_failed(
2887+
oauth_provider: OAuthClientProvider,
2888+
):
2889+
"""SEP-2352: on the legacy no-PRM path the binding check uses the ASM-discovered issuer.
2890+
2891+
PRM discovery fails (404) so ``auth_server_url`` stays ``None`` and the post-PRM check is
2892+
skipped; when ASM discovery then succeeds via the root well-known fallback, the discovered
2893+
metadata's issuer is compared against the stored credentials' bound issuer and a mismatch
2894+
triggers re-registration.
2895+
"""
2896+
oauth_provider.context.current_tokens = None
2897+
oauth_provider.context.token_expiry_time = None
2898+
oauth_provider._initialized = True
2899+
oauth_provider.context.client_info = OAuthClientInformationFull(
2900+
client_id="stale-client",
2901+
redirect_uris=[AnyUrl("http://localhost:3030/callback")],
2902+
issuer="https://old-as.example.com",
2903+
)
2904+
2905+
auth_flow = oauth_provider.async_auth_flow(httpx.Request("GET", "https://api.example.com/v1/mcp"))
2906+
request = await auth_flow.__anext__()
2907+
response_401 = httpx.Response(401, request=request)
2908+
2909+
# PRM discovery: path-based then root, both 404.
2910+
prm_req = await auth_flow.asend(response_401)
2911+
assert str(prm_req.url) == "https://api.example.com/.well-known/oauth-protected-resource/v1/mcp"
2912+
prm_req = await auth_flow.asend(httpx.Response(404, request=prm_req))
2913+
assert str(prm_req.url) == "https://api.example.com/.well-known/oauth-protected-resource"
2914+
2915+
# ASM discovery via root fallback (no auth_server_url) succeeds with a different issuer.
2916+
asm_req = await auth_flow.asend(httpx.Response(404, request=prm_req))
2917+
assert str(asm_req.url) == "https://api.example.com/.well-known/oauth-authorization-server"
2918+
asm_response = httpx.Response(
2919+
200,
2920+
content=(
2921+
b'{"issuer": "https://api.example.com", '
2922+
b'"authorization_endpoint": "https://api.example.com/authorize", '
2923+
b'"token_endpoint": "https://api.example.com/token", '
2924+
b'"registration_endpoint": "https://api.example.com/register"}'
2925+
),
2926+
request=asm_req,
2927+
)
2928+
2929+
# The stale bound credentials are discarded, so the next yield is a DCR request
2930+
# rather than the authorize redirect.
2931+
next_req = await auth_flow.asend(asm_response)
2932+
assert oauth_provider.context.auth_server_url is None
2933+
assert next_req.method == "POST"
2934+
assert str(next_req.url) == "https://api.example.com/register"
2935+
await auth_flow.aclose()

0 commit comments

Comments
 (0)