Skip to content

Commit 53c04e3

Browse files
committed
Carry an omitted refresh_token forward on refresh (RFC 6749 §6)
A refresh response may omit `refresh_token` when the AS does not rotate refresh tokens; the client is expected to keep using the previously issued one. The handler was overwriting the stored token wholesale, dropping the prior `refresh_token` and forcing a full re-authorization at the next expiry. Carry it forward alongside the existing scope carry-forward. Also stamp the SEP-2352 issuer on a same-origin fallback registration #2936 stopped recording the issuer when DCR fell back to the resource origin's `/register`, to avoid binding a registration to a PRM-advertised AS we never reached. That over-suppressed the legacy same-origin embedded-AS case (no PRM, root ASM at the resource origin without `registration_endpoint`), where the fallback `/register` is on the discovered issuer's own host and the binding was genuinely established. Restore the stamp when the fallback origin matches the discovered issuer's origin so a later AS migration still triggers discard and re-registration.
1 parent 5e013d9 commit 53c04e3

2 files changed

Lines changed: 132 additions & 16 deletions

File tree

src/mcp/client/auth/oauth2.py

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -477,11 +477,15 @@ async def _handle_refresh_response(self, response: httpx.Response) -> bool:
477477
content = await response.aread()
478478
token_response = OAuthToken.model_validate_json(content)
479479

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
480+
# RFC 6749 §6: a refresh response may omit scope (unchanged) and refresh_token
481+
# (the AS does not rotate). Carry both forward so the persisted token stays
482+
# self-describing for the SEP-2350 step-up union and the next expiry can
483+
# still refresh instead of forcing a full re-authorization.
484+
prior = self.context.current_tokens
485+
if token_response.scope is None and prior is not None:
486+
token_response.scope = prior.scope
487+
if token_response.refresh_token is None and prior is not None:
488+
token_response.refresh_token = prior.refresh_token
485489

486490
self.context.current_tokens = token_response
487491
self.context.update_token_expiry(token_response)
@@ -663,21 +667,25 @@ async def async_auth_flow(self, request: httpx.Request) -> AsyncGenerator[httpx.
663667
await self.context.storage.set_client_info(client_information)
664668
else:
665669
# Fallback to Dynamic Client Registration
670+
fallback_base = self.context.get_authorization_base_url(self.context.server_url)
666671
registration_request = create_client_registration_request(
667-
self.context.oauth_metadata,
668-
self.context.client_metadata,
669-
self.context.get_authorization_base_url(self.context.server_url),
672+
self.context.oauth_metadata, self.context.client_metadata, fallback_base
670673
)
671674
registration_response = yield registration_request
672675
client_information = await handle_registration_response(registration_response)
673676
# Only record the issuer when the registration above actually targeted
674-
# the discovered AS's registration_endpoint. With no metadata, or
675-
# metadata that omits registration_endpoint, DCR fell back to the
676-
# resource-server origin's /register — recording that as bound to a
677-
# PRM-advertised AS would persist a binding that was never established.
677+
# the discovered AS — either via its published registration_endpoint,
678+
# or because the resource-origin /register fallback is on the issuer's
679+
# own host (legacy same-origin embedded AS). Otherwise the fallback hit
680+
# a different server and recording a binding to the PRM-advertised AS
681+
# would persist a binding that was never established.
678682
if (
679683
self.context.oauth_metadata is not None
680-
and self.context.oauth_metadata.registration_endpoint is not None
684+
and discovered_issuer is not None
685+
and (
686+
self.context.oauth_metadata.registration_endpoint is not None
687+
or self.context.get_authorization_base_url(discovered_issuer) == fallback_base
688+
)
681689
):
682690
client_information.issuer = discovered_issuer
683691
self.context.client_info = client_information

tests/client/test_auth.py

Lines changed: 111 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2861,11 +2861,18 @@ async def test_handle_token_response_backfills_omitted_scope_from_request(
28612861

28622862

28632863
@pytest.mark.anyio
2864-
async def test_handle_refresh_response_carries_prior_scope_when_response_omits_it(
2864+
async def test_handle_refresh_response_carries_prior_scope_and_refresh_token_when_omitted(
28652865
oauth_provider: OAuthClientProvider, mock_storage: MockTokenStorage
28662866
):
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")
2867+
"""RFC 6749 §6: omitted refresh-response scope and refresh_token are carried forward.
2868+
2869+
Omitted scope means it is unchanged from the prior access token. Omitted refresh_token
2870+
means the AS does not rotate refresh tokens; the client keeps using the previously
2871+
issued one so the next expiry can refresh instead of forcing a full re-authorization.
2872+
"""
2873+
oauth_provider.context.current_tokens = OAuthToken(
2874+
access_token="old", scope="read write", refresh_token="prior-refresh"
2875+
)
28692876
response = httpx.Response(
28702877
200,
28712878
json={"access_token": "new", "token_type": "Bearer", "expires_in": 3600},
@@ -2877,9 +2884,32 @@ async def test_handle_refresh_response_carries_prior_scope_when_response_omits_i
28772884
assert oauth_provider.context.current_tokens is not None
28782885
assert oauth_provider.context.current_tokens.access_token == "new"
28792886
assert oauth_provider.context.current_tokens.scope == "read write"
2887+
assert oauth_provider.context.current_tokens.refresh_token == "prior-refresh"
28802888
stored = await mock_storage.get_tokens()
28812889
assert stored is not None
28822890
assert stored.scope == "read write"
2891+
assert stored.refresh_token == "prior-refresh"
2892+
2893+
2894+
@pytest.mark.anyio
2895+
async def test_handle_refresh_response_adopts_rotated_refresh_token_when_returned(
2896+
oauth_provider: OAuthClientProvider, mock_storage: MockTokenStorage
2897+
):
2898+
"""A refresh response that includes ``refresh_token`` replaces the prior one (rotation)."""
2899+
oauth_provider.context.current_tokens = OAuthToken(
2900+
access_token="old", scope="read write", refresh_token="prior-refresh"
2901+
)
2902+
response = httpx.Response(
2903+
200,
2904+
json={"access_token": "new", "token_type": "Bearer", "expires_in": 3600, "refresh_token": "rotated"},
2905+
request=httpx.Request("POST", "https://auth.example.com/token"),
2906+
)
2907+
ok = await oauth_provider._handle_refresh_response(response)
2908+
2909+
assert ok is True
2910+
stored = await mock_storage.get_tokens()
2911+
assert stored is not None
2912+
assert stored.refresh_token == "rotated"
28832913

28842914

28852915
@pytest.mark.anyio
@@ -3050,3 +3080,81 @@ async def echo_callback() -> AuthorizationCodeResult:
30503080
await auth_flow.asend(httpx.Response(200, request=final_req))
30513081
except StopAsyncIteration:
30523082
pass
3083+
3084+
3085+
@pytest.mark.anyio
3086+
async def test_issuer_is_stamped_when_same_origin_fallback_register_is_on_the_discovered_issuer(
3087+
oauth_provider: OAuthClientProvider, mock_storage: MockTokenStorage
3088+
):
3089+
"""SEP-2352: a fallback registration on the discovered issuer's own host is still bound.
3090+
3091+
Legacy same-origin embedded AS: PRM is absent, root ASM discovery succeeds with
3092+
``issuer`` equal to the resource origin and no ``registration_endpoint``. DCR falls
3093+
back to ``<resource-origin>/register`` — the issuer's own host — so the binding was
3094+
established and is recorded, preserving auto-recovery on a later AS migration.
3095+
"""
3096+
oauth_provider.context.current_tokens = None
3097+
oauth_provider.context.token_expiry_time = None
3098+
oauth_provider._initialized = True
3099+
oauth_provider.context.client_info = None
3100+
3101+
captured_state: str | None = None
3102+
3103+
async def capture_redirect(url: str) -> None:
3104+
nonlocal captured_state
3105+
captured_state = parse_qs(urlparse(url).query).get("state", [None])[0]
3106+
3107+
async def echo_callback() -> AuthorizationCodeResult:
3108+
return AuthorizationCodeResult(code="auth_code", state=captured_state)
3109+
3110+
oauth_provider.context.redirect_handler = capture_redirect
3111+
oauth_provider.context.callback_handler = echo_callback
3112+
3113+
auth_flow = oauth_provider.async_auth_flow(httpx.Request("GET", "https://api.example.com/v1/mcp"))
3114+
request = await auth_flow.__anext__()
3115+
3116+
# PRM discovery 404s on both well-known URLs.
3117+
prm_req = await auth_flow.asend(httpx.Response(401, request=request))
3118+
assert str(prm_req.url) == "https://api.example.com/.well-known/oauth-protected-resource/v1/mcp"
3119+
prm_req = await auth_flow.asend(httpx.Response(404, request=prm_req))
3120+
assert str(prm_req.url) == "https://api.example.com/.well-known/oauth-protected-resource"
3121+
3122+
# Root ASM discovery succeeds with the resource origin as issuer and no registration_endpoint.
3123+
asm_req = await auth_flow.asend(httpx.Response(404, request=prm_req))
3124+
assert str(asm_req.url) == "https://api.example.com/.well-known/oauth-authorization-server"
3125+
asm_response = httpx.Response(
3126+
200,
3127+
content=(
3128+
b'{"issuer": "https://api.example.com", '
3129+
b'"authorization_endpoint": "https://api.example.com/authorize", '
3130+
b'"token_endpoint": "https://api.example.com/token"}'
3131+
),
3132+
request=asm_req,
3133+
)
3134+
3135+
# DCR falls back to the resource origin's /register — the issuer's own host.
3136+
dcr_req = await auth_flow.asend(asm_response)
3137+
assert dcr_req.method == "POST"
3138+
assert str(dcr_req.url) == "https://api.example.com/register"
3139+
dcr_response = httpx.Response(
3140+
201,
3141+
json={"client_id": "embedded-client", "redirect_uris": ["http://localhost:3030/callback"]},
3142+
request=dcr_req,
3143+
)
3144+
token_req = await auth_flow.asend(dcr_response)
3145+
3146+
stored = await mock_storage.get_client_info()
3147+
assert stored is not None
3148+
assert oauth_provider.context.oauth_metadata is not None
3149+
assert stored.client_id == "embedded-client"
3150+
assert stored.issuer == str(oauth_provider.context.oauth_metadata.issuer)
3151+
assert urlparse(stored.issuer).netloc == "api.example.com"
3152+
3153+
token_response = httpx.Response(
3154+
200, json={"access_token": "t", "token_type": "Bearer", "expires_in": 3600}, request=token_req
3155+
)
3156+
final_req = await auth_flow.asend(token_response)
3157+
try:
3158+
await auth_flow.asend(httpx.Response(200, request=final_req))
3159+
except StopAsyncIteration:
3160+
pass

0 commit comments

Comments
 (0)