From 2bc618ab6de57dcb6ae08181d2e40ddd77cdd49a Mon Sep 17 00:00:00 2001 From: Max Isbey <224885523+maxisbey@users.noreply.github.com> Date: Tue, 18 Aug 2026 12:40:31 +0000 Subject: [PATCH 1/5] OAuth client: refresh before re-authorizing, and discover before refreshing The 401 branch of OAuthClientProvider discovered metadata but never tried the stored refresh token, and the pre-request refresh tried the refresh token but never discovered metadata. After a restart nothing restores an expiry, so the pre-request path is skipped, the stale bearer draws a 401, and the flow went straight to interactive authorization with a usable refresh token in hand; headless clients failed outright. When an application forced the pre-request path, the refresh was posted to a path guessed from the server origin, which 404s against an authorization server under a path, and the refresh token was dropped. The 401 branch now tries the refresh_token grant after discovery and registration and runs the full authorization only when there is no refresh token or the server rejects it. The pre-request refresh runs only when authorization server metadata is already known, so it never guesses an endpoint; a cold start takes the 401 and refreshes there. A fresh dynamic registration clears any held tokens, which belonged to a previous client. Closes #3240, #3250, #1318. --- docs/client/oauth-clients.md | 2 +- src/mcp/client/auth/oauth2.py | 25 +++-- tests/client/test_auth.py | 26 ++++++ tests/interaction/_requirements.py | 20 ++++ tests/interaction/auth/_harness.py | 56 +++++++++++- tests/interaction/auth/_provider.py | 4 + tests/interaction/auth/test_lifecycle.py | 112 +++++++++++++++++++++++ 7 files changed, 237 insertions(+), 8 deletions(-) diff --git a/docs/client/oauth-clients.md b/docs/client/oauth-clients.md index cd7de35626..9c81ce020b 100644 --- a/docs/client/oauth-clients.md +++ b/docs/client/oauth-clients.md @@ -81,7 +81,7 @@ The first time `Client` sends a request, the server answers `401`. The provider 3. **Authorization.** It generates the PKCE pair and a `state`, builds the authorization URL, awaits your `redirect_handler`, then awaits your `callback_handler` for the code. 4. **Exchange.** It trades the code for an `OAuthToken`, stores it, and replays your original request with `Authorization: Bearer ...`. -After that it is quiet. Tokens come out of storage, an expired access token is refreshed with the refresh token, and only when none of that works does it run the flow again. +After that it is quiet. Tokens come out of storage, an expired access token is refreshed with the refresh token, and only when none of that works does it run the flow again. That holds across restarts: a new process that finds a refresh token in storage answers the first `401` by rediscovering the authorization server and refreshing, not by sending anyone back to the browser. You wrote none of it. Two keyword arguments remain (`client_metadata_url` and `validate_resource_url`), and this file needs neither. `client_metadata_url` is the one worth knowing about; it gets its own section below. diff --git a/src/mcp/client/auth/oauth2.py b/src/mcp/client/auth/oauth2.py index 7dc62b52b9..17f4c97610 100644 --- a/src/mcp/client/auth/oauth2.py +++ b/src/mcp/client/auth/oauth2.py @@ -586,8 +586,13 @@ async def async_auth_flow(self, request: httpx2.Request) -> AsyncGenerator[httpx # Capture protocol version from request headers self.context.protocol_version = request.headers.get(MCP_PROTOCOL_VERSION_HEADER) - if not self.context.is_token_valid() and self.context.can_refresh_token(): - # Try to refresh token + # Refresh ahead of the request only when the token endpoint is already known; on a cold + # start the request goes out and the 401 branch discovers, then refreshes. + if ( + not self.context.is_token_valid() + and self.context.can_refresh_token() + and self.context.oauth_metadata is not None + ): refresh_request = await self._refresh_token() refresh_response = yield refresh_request @@ -741,10 +746,18 @@ async def async_auth_flow(self, request: httpx2.Request) -> AsyncGenerator[httpx client_information.issuer = discovered_issuer self.context.client_info = client_information await self.context.storage.set_client_info(client_information) - - # Step 5: Perform authorization and complete token exchange - token_response = yield await self._perform_authorization() - await self._handle_token_response(token_response) + # Held tokens belong to a previous client and cannot be refreshed by this one. + self.context.clear_tokens() + + # Step 5: Refresh with the stored refresh token first (RFC 6749 §6); run the full + # authorization only when there is none or the server rejects it. + refreshed = False + if self.context.can_refresh_token(): + refresh_response = yield await self._refresh_token() + refreshed = await self._handle_refresh_response(refresh_response) + if not refreshed: + token_response = yield await self._perform_authorization() + await self._handle_token_response(token_response) except Exception: logger.exception("OAuth flow error") raise diff --git a/tests/client/test_auth.py b/tests/client/test_auth.py index be96cc8eec..53fa105a0e 100644 --- a/tests/client/test_auth.py +++ b/tests/client/test_auth.py @@ -3253,3 +3253,29 @@ async def echo_callback() -> AuthorizationCodeResult: await auth_flow.asend(httpx2.Response(200, request=final_req)) except StopAsyncIteration: pass + + +@pytest.mark.anyio +async def test_expired_token_is_not_refreshed_ahead_of_the_request_before_metadata_is_discovered( + oauth_provider: OAuthClientProvider, valid_tokens: OAuthToken +) -> None: + """With no authorization-server metadata yet, an expired token is not refreshed at a guessed endpoint. + + The request goes out unauthenticated instead, so the 401 branch discovers the real token + endpoint before the refresh token is presented anywhere (#3240). + """ + oauth_provider.context.current_tokens = valid_tokens + oauth_provider.context.token_expiry_time = time.time() - 60 + oauth_provider.context.client_info = OAuthClientInformationFull(client_id="c", redirect_uris=None) + oauth_provider.context.oauth_metadata = None + oauth_provider._initialized = True + + request = httpx2.Request("POST", "https://api.example.com/v1/mcp") + auth_flow = oauth_provider.async_auth_flow(request) + first = await auth_flow.__anext__() + + assert first is request + assert "Authorization" not in first.headers + + with pytest.raises(StopAsyncIteration): + await auth_flow.asend(httpx2.Response(200, request=request)) diff --git a/tests/interaction/_requirements.py b/tests/interaction/_requirements.py index 86725bcb4f..0d3f2015d7 100644 --- a/tests/interaction/_requirements.py +++ b/tests/interaction/_requirements.py @@ -3883,6 +3883,26 @@ def __post_init__(self) -> None: transports=("streamable-http",), note="OAuth is HTTP-only.", ), + "client-auth:refresh:on-401": Requirement( + source="issue:#3250", + behavior=( + "A 401 received while a refresh token is held is answered, after rediscovery, with a " + "refresh_token grant before any interactive authorization, so a client constructed over " + "persisted tokens and client registration recovers from an expired access token headlessly." + ), + transports=("streamable-http",), + note="OAuth is HTTP-only. RFC 6749 §1.5 (E)-(H); matches the TypeScript, C# and Rust SDKs.", + ), + "client-auth:refresh:discovered-endpoint": Requirement( + source="issue:#3240", + behavior=( + "A refresh in a process that has not yet discovered the authorization server happens only after " + "protected-resource and authorization-server metadata discovery and posts to the advertised " + "token endpoint, never to a path guessed from the server origin." + ), + transports=("streamable-http",), + note="OAuth is HTTP-only.", + ), "client-auth:resource-parameter": Requirement( source=f"{SPEC_BASE_URL}/basic/authorization#resource-parameter-implementation", behavior=( diff --git a/tests/interaction/auth/_harness.py b/tests/interaction/auth/_harness.py index 856a1fe9a8..95cef172fe 100644 --- a/tests/interaction/auth/_harness.py +++ b/tests/interaction/auth/_harness.py @@ -26,7 +26,14 @@ from mcp.server import Server from mcp.server.auth.provider import AccessToken, ProviderTokenVerifier from mcp.server.auth.settings import AuthSettings, ClientRegistrationOptions, RevocationOptions -from mcp.shared.auth import AuthorizationCodeResult, OAuthClientInformationFull, OAuthClientMetadata, OAuthToken +from mcp.shared.auth import ( + AuthorizationCodeResult, + OAuthClientInformationFull, + OAuthClientMetadata, + OAuthMetadata, + OAuthToken, + ProtectedResourceMetadata, +) from tests.interaction._connect import BASE_URL, NO_DNS_REBINDING_PROTECTION from tests.interaction.auth._provider import InMemoryAuthorizationServerProvider from tests.interaction.transports._bridge import StreamingASGITransport @@ -273,6 +280,53 @@ def shim( return lambda app: shimmed_app(app, not_found=not_found, serve=serve) +def path_prefixed_as_shim(prefix: str) -> AppShim: + """Build an `app_shim` that presents the co-hosted authorization server as living under `prefix`. + + The SDK server mounts `/authorize`, `/token` and `/register` at the origin root whatever the + issuer, so an AS whose endpoints sit under a path cannot be configured natively. This serves + PRM naming `{BASE_URL}{prefix}` as the AS, serves that issuer's metadata at the RFC 8414 + path-inserted well-known URL with every endpoint under the prefix, forwards `{prefix}/x` to the + real `/x`, and 404s the bare root endpoints and root metadata so a client guessing origin-root + paths fails as it would against such a server. Pair with + `InMemoryAuthorizationServerProvider(issuer=f"{BASE_URL}{prefix}")` so the redirect `iss` matches. + """ + issuer = f"{BASE_URL}{prefix}" + prm = ProtectedResourceMetadata(resource=AnyHttpUrl(f"{BASE_URL}/mcp"), authorization_servers=[AnyHttpUrl(issuer)]) + asm = OAuthMetadata( + issuer=AnyHttpUrl(issuer), + authorization_endpoint=AnyHttpUrl(f"{issuer}/authorize"), + token_endpoint=AnyHttpUrl(f"{issuer}/token"), + registration_endpoint=AnyHttpUrl(f"{issuer}/register"), + scopes_supported=["mcp"], + response_types_supported=["code"], + grant_types_supported=["authorization_code", "refresh_token"], + token_endpoint_auth_methods_supported=["client_secret_post", "client_secret_basic", "none"], + code_challenge_methods_supported=["S256"], + ) + + def factory(app: ASGIApp) -> ASGIApp: + inner = shimmed_app( + app, + not_found=frozenset({"/token", "/authorize", "/register", "/.well-known/oauth-authorization-server"}), + serve={ + "/.well-known/oauth-protected-resource/mcp": metadata_body(prm), + f"/.well-known/oauth-authorization-server{prefix}": metadata_body(asm), + }, + ) + + async def wrapped(scope: Scope, receive: Receive, send: Send) -> None: + if scope["type"] == "http" and scope["path"].startswith(f"{prefix}/"): + path = scope["path"][len(prefix) :] + await app({**scope, "path": path, "raw_path": path.encode()}, receive, send) + return + await inner(scope, receive, send) + + return wrapped + + return factory + + @dataclass class _FirstChallenge: """ASGI shim that answers the first request to a path with 401 + a given WWW-Authenticate. diff --git a/tests/interaction/auth/_provider.py b/tests/interaction/auth/_provider.py index 0c54d4fd37..fd59b2ad57 100644 --- a/tests/interaction/auth/_provider.py +++ b/tests/interaction/auth/_provider.py @@ -103,6 +103,10 @@ def mint_access_token(self, *, client_id: str, scopes: list[str], resource: str ) return access + def expire_access_token(self, token: str) -> None: + """Move an issued access token's server-side expiry into the past so the bearer middleware 401s it.""" + self.access_tokens[token] = self.access_tokens[token].model_copy(update={"expires_at": int(time.time()) - 1}) + async def get_client(self, client_id: str) -> OAuthClientInformationFull | None: return self.clients.get(client_id) diff --git a/tests/interaction/auth/test_lifecycle.py b/tests/interaction/auth/test_lifecycle.py index 8f45a01510..43a6bb96d1 100644 --- a/tests/interaction/auth/test_lifecycle.py +++ b/tests/interaction/auth/test_lifecycle.py @@ -18,6 +18,7 @@ from pydantic import AnyHttpUrl, AnyUrl from mcp import MCPError +from mcp.client.auth import OAuthClientProvider from mcp.client.auth.extensions.client_credentials import ClientCredentialsOAuthProvider, PrivateKeyJWTOAuthProvider from mcp.server import Server, ServerRequestContext from mcp.shared.auth import OAuthClientInformationFull, OAuthMetadata @@ -25,12 +26,15 @@ from tests.interaction._requirements import requirement from tests.interaction.auth._harness import ( REDIRECT_URI, + AppShim, InMemoryTokenStorage, RecordedRequest, auth_settings, connect_with_oauth, m2m_token_shim, metadata_body, + oauth_client_metadata, + path_prefixed_as_shim, record_requests, shim, step_up_shim, @@ -98,6 +102,29 @@ def seeded_client(provider: InMemoryAuthorizationServerProvider, **kwargs: objec return info +async def first_process_login( + provider: InMemoryAuthorizationServerProvider, storage: InMemoryTokenStorage, *, app_shim: AppShim | None = None +) -> str: + """Run one interactive connect so `storage` holds what a first process leaves behind; return its access token. + + The restart tests then build a fresh `OAuthClientProvider` over the same storage, as a second + process would, so the registration and tokens carry exactly what the SDK persists. + """ + server = Server("guarded", on_list_tools=list_tools) + async with connect_with_oauth(server, provider=provider, storage=storage, app_shim=app_shim) as (client, _): + await client.list_tools() + assert storage.tokens is not None and storage.tokens.refresh_token is not None + return storage.tokens.access_token + + +def restarted_headless_provider(storage: InMemoryTokenStorage) -> OAuthClientProvider: + """A provider as a second, headless process constructs it: same storage, fresh state, no handlers. + + Reaching the interactive step raises rather than opening a browser the scenario says is absent. + """ + return OAuthClientProvider(server_url=f"{BASE_URL}/mcp", client_metadata=oauth_client_metadata(), storage=storage) + + @requirement("client-auth:refresh:transparent") async def test_an_expired_access_token_is_transparently_refreshed_before_the_next_request() -> None: """An access token the client considers expired is refreshed and the new bearer is used. @@ -354,6 +381,91 @@ async def test_a_failed_refresh_clears_stored_tokens_and_restarts_the_full_flow( assert storage.tokens.access_token in provider.access_tokens +@requirement("client-auth:refresh:on-401") +async def test_a_restarted_client_answers_a_401_with_its_stored_refresh_token() -> None: + """A second process holding only persisted tokens and registration refreshes on 401 instead of re-authorizing. + + Steps: (1) a first process logs in and its storage keeps the registration and a refresh token; + (2) the server-side access token lapses; (3) a fresh provider over the same storage, with no + browser, connects. The recording proves the stale bearer drew a 401, discovery ran, one + `refresh_token` grant followed, and neither `/authorize` nor `/register` was touched. + SDK behaviour per RFC 6749 §1.5; regression bar for #3250 / #1318. + """ + provider = InMemoryAuthorizationServerProvider() + storage = InMemoryTokenStorage() + with anyio.fail_after(5): + stale_access_token = await first_process_login(provider, storage) + provider.expire_access_token(stale_access_token) + + recorded, on_request = record_requests() + server = Server("guarded", on_list_tools=list_tools) + with anyio.fail_after(5): + async with connect_with_oauth( + server, provider=provider, auth=restarted_headless_provider(storage), on_request=on_request + ) as (client, _): + result = await client.list_tools() + + assert result.tools[0].name == "echo" + assert [(r.method, r.path) for r in recorded[:5]] == snapshot( + [ + ("POST", "/mcp"), + ("GET", "/.well-known/oauth-protected-resource/mcp"), + ("GET", "/.well-known/oauth-authorization-server"), + ("POST", "/token"), + ("POST", "/mcp"), + ] + ) + assert recorded[0].headers["authorization"] == f"Bearer {stale_access_token}" + assert [form_body(r)["grant_type"] for r in find(recorded, "POST", "/token")] == ["refresh_token"] + assert find(recorded, "GET", "/authorize") == [] and find(recorded, "POST", "/register") == [] + assert storage.tokens is not None and storage.tokens.access_token != stale_access_token + assert storage.tokens.access_token in provider.access_tokens + + +@requirement("client-auth:refresh:discovered-endpoint") +async def test_a_restarted_client_refreshes_at_the_token_endpoint_advertised_under_a_path() -> None: + """Against an authorization server under `/oauth2/v1`, a second process refreshes at `/oauth2/v1/token`. + + The bare `/token` 404s here. Nothing is discovered yet in the second process, so no refresh is + attempted before the request; the 401 drives discovery and the single refresh POST goes to the + advertised endpoint. Regression bar for #3240, where the guessed `{origin}/token` 404ed and the + refresh token was discarded. + """ + prefix = "/oauth2/v1" + provider = InMemoryAuthorizationServerProvider(issuer=f"{BASE_URL}{prefix}") + storage = InMemoryTokenStorage() + app_shim = path_prefixed_as_shim(prefix) + with anyio.fail_after(5): + stale_access_token = await first_process_login(provider, storage, app_shim=app_shim) + provider.expire_access_token(stale_access_token) + + recorded, on_request = record_requests() + server = Server("guarded", on_list_tools=list_tools) + with anyio.fail_after(5): + async with connect_with_oauth( + server, + provider=provider, + auth=restarted_headless_provider(storage), + app_shim=app_shim, + on_request=on_request, + ) as (client, _): + result = await client.list_tools() + + assert result.tools[0].name == "echo" + assert [(r.method, r.path) for r in recorded[:5]] == snapshot( + [ + ("POST", "/mcp"), + ("GET", "/.well-known/oauth-protected-resource/mcp"), + ("GET", "/.well-known/oauth-authorization-server/oauth2/v1"), + ("POST", "/oauth2/v1/token"), + ("POST", "/mcp"), + ] + ) + token_posts = [r for r in recorded if r.method == "POST" and r.path.endswith("/token")] + assert [(r.path, form_body(r)["grant_type"]) for r in token_posts] == [("/oauth2/v1/token", "refresh_token")] + assert not any(r.path.endswith("/authorize") for r in recorded) + + @requirement("client-auth:client-credentials") async def test_client_credentials_provider_obtains_a_token_without_an_authorize_step() -> None: """The client-credentials provider connects with no authorize step and a `client_credentials` grant. From ab4032413f75e98f6f3609a64c376fa12cb61da1 Mon Sep 17 00:00:00 2001 From: Max Isbey <224885523+maxisbey@users.noreply.github.com> Date: Tue, 18 Aug 2026 13:17:43 +0000 Subject: [PATCH 2/5] Address review: refresh-failure parity, CIMD token binding, pinning tests - A failed refresh resets _initialized inside _handle_refresh_response, so the 401-path refresh site behaves like the pre-request one: the next request re-reads storage and can retry a refresh token that is still good instead of leaving a long-lived headless provider with no tokens. - A CIMD client_id is portable across authorization servers but its tokens are not: when the discovered issuer differs from the record's stamp, keep the record, drop the tokens, and re-stamp before deciding to refresh. - Tests pinning: rejected 401-path refresh falls back to authorization; a headless provider retries the refresh on its next connection; a fresh registration does not present tokens left from a previous client; the CIMD issuer-change case. Manifest wording for the discovered-endpoint entry no longer overclaims about the no-metadata fallback. --- src/mcp/client/auth/oauth2.py | 28 +++- tests/interaction/_requirements.py | 9 +- tests/interaction/auth/test_lifecycle.py | 164 ++++++++++++++++++++++- 3 files changed, 189 insertions(+), 12 deletions(-) diff --git a/src/mcp/client/auth/oauth2.py b/src/mcp/client/auth/oauth2.py index 17f4c97610..02c9338fed 100644 --- a/src/mcp/client/auth/oauth2.py +++ b/src/mcp/client/auth/oauth2.py @@ -374,7 +374,7 @@ async def _perform_authorization_code_grant(self) -> tuple[str, str]: if self.context.client_metadata.redirect_uris is None: raise OAuthFlowError("No redirect URIs provided for authorization code grant") # pragma: no cover if not self.context.redirect_handler: - raise OAuthFlowError("No redirect handler provided for authorization code grant") # pragma: no cover + raise OAuthFlowError("No redirect handler provided for authorization code grant") if not self.context.callback_handler: raise OAuthFlowError("No callback handler provided for authorization code grant") # pragma: no cover @@ -521,6 +521,8 @@ async def _handle_refresh_response(self, response: httpx2.Response) -> bool: if response.status_code != 200: logger.warning(f"Token refresh failed: {response.status_code}") self.context.clear_tokens() + # Re-read storage on the next request: the failure may have been transient. + self._initialized = False return False try: @@ -545,6 +547,7 @@ async def _handle_refresh_response(self, response: httpx2.Response) -> bool: except ValidationError: # pragma: no cover logger.exception("Invalid refresh response") self.context.clear_tokens() + self._initialized = False return False async def _initialize(self) -> None: @@ -593,12 +596,8 @@ async def async_auth_flow(self, request: httpx2.Request) -> AsyncGenerator[httpx and self.context.can_refresh_token() and self.context.oauth_metadata is not None ): - refresh_request = await self._refresh_token() - refresh_response = yield refresh_request - - if not await self._handle_refresh_response(refresh_response): - # Refresh failed, need full re-authentication - self._initialized = False + refresh_response = yield await self._refresh_token() + await self._handle_refresh_response(refresh_response) if self.context.is_token_valid(): self._add_auth_header(request) @@ -749,6 +748,21 @@ async def async_auth_flow(self, request: httpx2.Request) -> AsyncGenerator[httpx # Held tokens belong to a previous client and cannot be refreshed by this one. self.context.clear_tokens() + # A CIMD client_id is portable across authorization servers (SEP-2352) but tokens + # issued under it are not: on an issuer change keep the record, drop the tokens. + client_info = self.context.client_info + current_issuer = self.context.auth_server_url or ( + str(self.context.oauth_metadata.issuer) if self.context.oauth_metadata else None + ) + if ( + client_info.client_id == self.context.client_metadata_url + and current_issuer is not None + and client_info.issuer not in (None, current_issuer) + ): + self.context.clear_tokens() + client_info.issuer = current_issuer + await self.context.storage.set_client_info(client_info) + # Step 5: Refresh with the stored refresh token first (RFC 6749 §6); run the full # authorization only when there is none or the server rejects it. refreshed = False diff --git a/tests/interaction/_requirements.py b/tests/interaction/_requirements.py index 0d3f2015d7..cb66e07165 100644 --- a/tests/interaction/_requirements.py +++ b/tests/interaction/_requirements.py @@ -3897,11 +3897,14 @@ def __post_init__(self) -> None: source="issue:#3240", behavior=( "A refresh in a process that has not yet discovered the authorization server happens only after " - "protected-resource and authorization-server metadata discovery and posts to the advertised " - "token endpoint, never to a path guessed from the server origin." + "protected-resource and authorization-server metadata discovery and posts to the token endpoint " + "that metadata advertises." ), transports=("streamable-http",), - note="OAuth is HTTP-only.", + note=( + "OAuth is HTTP-only. When discovery yields no AS metadata at all, the 2025-03-26 origin-derived " + "fallback endpoint is still used, as it is for the authorization itself." + ), ), "client-auth:resource-parameter": Requirement( source=f"{SPEC_BASE_URL}/basic/authorization#resource-parameter-implementation", diff --git a/tests/interaction/auth/test_lifecycle.py b/tests/interaction/auth/test_lifecycle.py index 43a6bb96d1..5bbb573051 100644 --- a/tests/interaction/auth/test_lifecycle.py +++ b/tests/interaction/auth/test_lifecycle.py @@ -18,15 +18,16 @@ from pydantic import AnyHttpUrl, AnyUrl from mcp import MCPError -from mcp.client.auth import OAuthClientProvider +from mcp.client.auth import OAuthClientProvider, OAuthFlowError from mcp.client.auth.extensions.client_credentials import ClientCredentialsOAuthProvider, PrivateKeyJWTOAuthProvider from mcp.server import Server, ServerRequestContext -from mcp.shared.auth import OAuthClientInformationFull, OAuthMetadata +from mcp.shared.auth import OAuthClientInformationFull, OAuthMetadata, OAuthToken from tests.interaction._connect import BASE_URL from tests.interaction._requirements import requirement from tests.interaction.auth._harness import ( REDIRECT_URI, AppShim, + HeadlessOAuth, InMemoryTokenStorage, RecordedRequest, auth_settings, @@ -125,6 +126,17 @@ def restarted_headless_provider(storage: InMemoryTokenStorage) -> OAuthClientPro return OAuthClientProvider(server_url=f"{BASE_URL}/mcp", client_metadata=oauth_client_metadata(), storage=storage) +def restarted_interactive_provider(storage: InMemoryTokenStorage, headless: HeadlessOAuth) -> OAuthClientProvider: + """A provider as a second process with a browser available constructs it: same storage, fresh state.""" + return OAuthClientProvider( + server_url=f"{BASE_URL}/mcp", + client_metadata=oauth_client_metadata(), + storage=storage, + redirect_handler=headless.redirect_handler, + callback_handler=headless.callback_handler, + ) + + @requirement("client-auth:refresh:transparent") async def test_an_expired_access_token_is_transparently_refreshed_before_the_next_request() -> None: """An access token the client considers expired is refreshed and the new bearer is used. @@ -466,6 +478,154 @@ async def test_a_restarted_client_refreshes_at_the_token_endpoint_advertised_und assert not any(r.path.endswith("/authorize") for r in recorded) +@requirement("client-auth:invalid-grant-clears-tokens") +async def test_a_refresh_the_server_rejects_on_the_401_path_falls_back_to_authorization() -> None: + """When the 401 path's refresh is rejected, the flow runs the full authorization instead of giving up. + + Second process with a browser; the harness denies the one refresh with `invalid_grant`. The + recording proves the refresh was tried first, then exactly one authorize and code exchange, + with no re-registration. + """ + provider = InMemoryAuthorizationServerProvider(fail_next_refresh=True) + storage = InMemoryTokenStorage() + with anyio.fail_after(5): + provider.expire_access_token(await first_process_login(provider, storage)) + + recorded, on_request = record_requests() + headless = HeadlessOAuth() + server = Server("guarded", on_list_tools=list_tools) + with anyio.fail_after(5): + async with connect_with_oauth( + server, + provider=provider, + auth=restarted_interactive_provider(storage, headless), + headless=headless, + on_request=on_request, + ) as (client, _): + result = await client.list_tools() + + assert result.tools[0].name == "echo" + assert [form_body(r)["grant_type"] for r in find(recorded, "POST", "/token")] == snapshot( + ["refresh_token", "authorization_code"] + ) + counts = path_counts(recorded) + assert counts[("GET", "/authorize")] == 1 + assert counts[("POST", "/register")] == 0 + + +@requirement("client-auth:refresh:on-401") +async def test_a_headless_client_whose_refresh_failed_retries_it_from_storage_on_the_next_connection() -> None: + """A failed refresh does not wedge a long-lived headless provider: the next connection reloads and refreshes. + + The harness denies the first refresh with `invalid_grant` but leaves the refresh token valid, + standing in for a transient token-endpoint failure. The first connect raises (no browser to + fall back to); the second, through the same provider instance, refreshes and succeeds. + """ + provider = InMemoryAuthorizationServerProvider(fail_next_refresh=True) + storage = InMemoryTokenStorage() + with anyio.fail_after(5): + provider.expire_access_token(await first_process_login(provider, storage)) + daemon = restarted_headless_provider(storage) + + with anyio.fail_after(5): + with pytest.RaisesGroup(pytest.RaisesExc(OAuthFlowError), flatten_subgroups=True): + await connect_with_oauth( + Server("guarded", on_list_tools=list_tools), provider=provider, auth=daemon + ).__aenter__() + + recorded, on_request = record_requests() + with anyio.fail_after(5): + async with connect_with_oauth( + Server("guarded", on_list_tools=list_tools), provider=provider, auth=daemon, on_request=on_request + ) as (client, _): + result = await client.list_tools() + + assert result.tools[0].name == "echo" + assert [form_body(r)["grant_type"] for r in find(recorded, "POST", "/token")] == ["refresh_token"] + assert find(recorded, "GET", "/authorize") == [] + + +@requirement("client-auth:refresh:on-401") +async def test_a_fresh_registration_does_not_present_tokens_left_from_a_previous_client() -> None: + """Tokens found in storage without their registration are not refreshed under a newly registered client. + + The second process finds tokens but no `client_info` (lost, or never persisted), so the 401 + path registers a new client; the refresh token belonged to the old one and is dropped rather + than presented, and the flow authorizes once. RFC 6749 §6 binds refresh tokens to their client. + """ + provider = InMemoryAuthorizationServerProvider() + storage = InMemoryTokenStorage() + with anyio.fail_after(5): + provider.expire_access_token(await first_process_login(provider, storage)) + storage.client_info = None + + recorded, on_request = record_requests() + headless = HeadlessOAuth() + server = Server("guarded", on_list_tools=list_tools) + with anyio.fail_after(5): + async with connect_with_oauth( + server, + provider=provider, + auth=restarted_interactive_provider(storage, headless), + headless=headless, + on_request=on_request, + ) as (client, _): + result = await client.list_tools() + + assert result.tools[0].name == "echo" + assert [(r.method, r.path) for r in recorded[:6]] == snapshot( + [ + ("POST", "/mcp"), + ("GET", "/.well-known/oauth-protected-resource/mcp"), + ("GET", "/.well-known/oauth-authorization-server"), + ("POST", "/register"), + ("GET", "/authorize"), + ("POST", "/token"), + ] + ) + assert [form_body(r)["grant_type"] for r in find(recorded, "POST", "/token")] == ["authorization_code"] + + +@requirement("client-auth:as-binding") +async def test_a_cimd_client_keeps_its_id_but_drops_its_tokens_when_the_authorization_server_changes() -> None: + """A CIMD registration is portable across authorization servers; the tokens issued under it are not. + + Storage holds a CIMD record stamped with a previous issuer and a refresh token minted there. + On the 401 the discovered issuer differs, so the flow keeps the URL client_id, re-stamps it, + and authorizes afresh instead of presenting the old refresh token to the new server (SEP-2352; + the TypeScript SDK discards tokens on the same mismatch). + """ + recorded, on_request = record_requests() + provider = InMemoryAuthorizationServerProvider() + seeded_client(provider, client_id=CIMD_URL) + stale = OAuthClientInformationFull( + client_id=CIMD_URL, + token_endpoint_auth_method="none", + redirect_uris=[AnyUrl(REDIRECT_URI)], + issuer="https://old-as.example.com", + ) + storage = InMemoryTokenStorage(client_info=stale) + storage.tokens = OAuthToken(access_token="issued-by-old-as", refresh_token="refresh-from-old-as", expires_in=3600) + server = Server("guarded", on_list_tools=list_tools) + + with anyio.fail_after(5): + async with connect_with_oauth( + server, + provider=provider, + storage=storage, + client_metadata_url=CIMD_URL, + app_shim=shim(serve={ASM_PATH: cimd_supported_metadata()}), + on_request=on_request, + ) as (client, _): + await client.list_tools() + + assert [form_body(r)["grant_type"] for r in find(recorded, "POST", "/token")] == ["authorization_code"] + assert all(b"refresh-from-old-as" not in r.content for r in recorded) + assert path_counts(recorded)[("POST", "/register")] == 0 + assert storage.client_info is not None + assert (storage.client_info.client_id, storage.client_info.issuer) == (CIMD_URL, f"{BASE_URL}/") + + @requirement("client-auth:client-credentials") async def test_client_credentials_provider_obtains_a_token_without_an_authorize_step() -> None: """The client-credentials provider connects with no authorize step and a `client_credentials` grant. From 76542f8cffb0b2109c754cb254f2f75bdd71469a Mon Sep 17 00:00:00 2001 From: Max Isbey <224885523+maxisbey@users.noreply.github.com> Date: Tue, 18 Aug 2026 13:43:42 +0000 Subject: [PATCH 3/5] Handle the CIMD issuer change beside the SEP-2352 guard so stale metadata is dropped too The CIMD token drop ran after authorization-server metadata rediscovery, so when that rediscovery failed the previous server's cached metadata survived under a record already re-stamped with the new issuer. Doing it at the same point as the existing bound-credentials guard, right after PRM names the issuer, drops the cached metadata as that guard does. A unit test pins all three effects. --- src/mcp/client/auth/oauth2.py | 27 ++++++++---------- tests/client/test_auth.py | 52 +++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 15 deletions(-) diff --git a/src/mcp/client/auth/oauth2.py b/src/mcp/client/auth/oauth2.py index 02c9338fed..5f357b6eb7 100644 --- a/src/mcp/client/auth/oauth2.py +++ b/src/mcp/client/auth/oauth2.py @@ -652,6 +652,18 @@ async def async_auth_flow(self, request: httpx2.Request) -> AsyncGenerator[httpx # Any cached AS metadata is for the old server; drop it so a failed # rediscovery cannot leak the old registration/token endpoints into Step 4. self.context.oauth_metadata = None + elif ( + self.context.client_info is not None + and self.context.client_info.client_id == self.context.client_metadata_url + and self.context.auth_server_url is not None + and self.context.client_info.issuer not in (None, self.context.auth_server_url) + ): + # A CIMD client_id is portable across authorization servers; the tokens issued + # under it and the cached metadata are not. Keep the record, re-stamped. + self.context.clear_tokens() + self.context.oauth_metadata = None + self.context.client_info.issuer = self.context.auth_server_url + await self.context.storage.set_client_info(self.context.client_info) asm_discovery_urls = build_oauth_authorization_server_metadata_discovery_urls( self.context.auth_server_url, self.context.server_url @@ -748,21 +760,6 @@ async def async_auth_flow(self, request: httpx2.Request) -> AsyncGenerator[httpx # Held tokens belong to a previous client and cannot be refreshed by this one. self.context.clear_tokens() - # A CIMD client_id is portable across authorization servers (SEP-2352) but tokens - # issued under it are not: on an issuer change keep the record, drop the tokens. - client_info = self.context.client_info - current_issuer = self.context.auth_server_url or ( - str(self.context.oauth_metadata.issuer) if self.context.oauth_metadata else None - ) - if ( - client_info.client_id == self.context.client_metadata_url - and current_issuer is not None - and client_info.issuer not in (None, current_issuer) - ): - self.context.clear_tokens() - client_info.issuer = current_issuer - await self.context.storage.set_client_info(client_info) - # Step 5: Refresh with the stored refresh token first (RFC 6749 §6); run the full # authorization only when there is none or the server rejects it. refreshed = False diff --git a/tests/client/test_auth.py b/tests/client/test_auth.py index 53fa105a0e..14d31d9abc 100644 --- a/tests/client/test_auth.py +++ b/tests/client/test_auth.py @@ -3279,3 +3279,55 @@ async def test_expired_token_is_not_refreshed_ahead_of_the_request_before_metada with pytest.raises(StopAsyncIteration): await auth_flow.asend(httpx2.Response(200, request=request)) + + +@pytest.mark.anyio +async def test_cimd_record_is_restamped_and_its_tokens_and_cached_metadata_dropped_when_prm_names_a_new_issuer( + client_metadata: OAuthClientMetadata, mock_storage: MockTokenStorage, valid_tokens: OAuthToken +) -> None: + """SEP-2352 for CIMD: the URL client_id survives an authorization-server change, nothing else does. + + A long-lived provider holds a CIMD record stamped with the old issuer, tokens minted there, and + the old server's cached metadata. As soon as PRM names a different issuer, the tokens and the + cached metadata are dropped and the record is re-stamped and persisted, so a failed + rediscovery cannot leave the old endpoints in play and no refresh reaches the new server. + """ + cimd_url = "https://client.example.com/.well-known/mcp-client" + provider = OAuthClientProvider( + server_url="https://api.example.com/v1/mcp", + client_metadata=client_metadata, + storage=mock_storage, + client_metadata_url=cimd_url, + ) + provider.context.client_info = OAuthClientInformationFull( + client_id=cimd_url, token_endpoint_auth_method="none", issuer="https://old-as.example.com" + ) + provider.context.current_tokens = valid_tokens + provider.context.token_expiry_time = time.time() + 1800 + provider.context.oauth_metadata = OAuthMetadata( + issuer=AnyHttpUrl("https://old-as.example.com"), + authorization_endpoint=AnyHttpUrl("https://old-as.example.com/authorize"), + token_endpoint=AnyHttpUrl("https://old-as.example.com/token"), + ) + provider._initialized = True + + auth_flow = provider.async_auth_flow(httpx2.Request("GET", "https://api.example.com/v1/mcp")) + request = await auth_flow.__anext__() + prm_req = await auth_flow.asend(httpx2.Response(401, request=request)) + prm_response = httpx2.Response( + 200, + content=b'{"resource": "https://api.example.com/v1/mcp", "authorization_servers": ["https://new-as.example.com"]}', + request=prm_req, + ) + asm_req = await auth_flow.asend(prm_response) + + assert str(asm_req.url) == "https://new-as.example.com/.well-known/oauth-authorization-server" + assert provider.context.current_tokens is None + assert provider.context.oauth_metadata is None + assert provider.context.client_info is not None + assert (provider.context.client_info.client_id, provider.context.client_info.issuer) == ( + cimd_url, + "https://new-as.example.com", + ) + assert mock_storage._client_info is provider.context.client_info + await auth_flow.aclose() From 7f01cd0cab0e93d162d776abf6bbe8ed24f09ec6 Mon Sep 17 00:00:00 2001 From: Max Isbey <224885523+maxisbey@users.noreply.github.com> Date: Tue, 18 Aug 2026 13:48:13 +0000 Subject: [PATCH 4/5] Treat an unstamped CIMD record like one stamped elsewhere: drop its tokens, then stamp it Records this SDK creates for a client ID metadata document always carry the issuer stamp, so an unstamped one comes from an older store and its tokens have no confirmed origin. Dropping them once and stamping the record converges after a single re-authorization instead of presenting a refresh token of unknown provenance to whichever server PRM names. --- src/mcp/client/auth/oauth2.py | 5 +++-- tests/client/test_auth.py | 17 +++++++++++------ 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/mcp/client/auth/oauth2.py b/src/mcp/client/auth/oauth2.py index 5f357b6eb7..6ce072525a 100644 --- a/src/mcp/client/auth/oauth2.py +++ b/src/mcp/client/auth/oauth2.py @@ -656,10 +656,11 @@ async def async_auth_flow(self, request: httpx2.Request) -> AsyncGenerator[httpx self.context.client_info is not None and self.context.client_info.client_id == self.context.client_metadata_url and self.context.auth_server_url is not None - and self.context.client_info.issuer not in (None, self.context.auth_server_url) + and self.context.client_info.issuer != self.context.auth_server_url ): # A CIMD client_id is portable across authorization servers; the tokens issued - # under it and the cached metadata are not. Keep the record, re-stamped. + # under it and the cached metadata are not. Keep the record, re-stamped. An + # unstamped record's tokens have unknown provenance and are dropped the same way. self.context.clear_tokens() self.context.oauth_metadata = None self.context.client_info.issuer = self.context.auth_server_url diff --git a/tests/client/test_auth.py b/tests/client/test_auth.py index 14d31d9abc..b6c7bf4b68 100644 --- a/tests/client/test_auth.py +++ b/tests/client/test_auth.py @@ -3282,15 +3282,20 @@ async def test_expired_token_is_not_refreshed_ahead_of_the_request_before_metada @pytest.mark.anyio +@pytest.mark.parametrize("stamped_issuer", ["https://old-as.example.com", None], ids=["stamped-elsewhere", "unstamped"]) async def test_cimd_record_is_restamped_and_its_tokens_and_cached_metadata_dropped_when_prm_names_a_new_issuer( - client_metadata: OAuthClientMetadata, mock_storage: MockTokenStorage, valid_tokens: OAuthToken + client_metadata: OAuthClientMetadata, + mock_storage: MockTokenStorage, + valid_tokens: OAuthToken, + stamped_issuer: str | None, ) -> None: """SEP-2352 for CIMD: the URL client_id survives an authorization-server change, nothing else does. - A long-lived provider holds a CIMD record stamped with the old issuer, tokens minted there, and - the old server's cached metadata. As soon as PRM names a different issuer, the tokens and the - cached metadata are dropped and the record is re-stamped and persisted, so a failed - rediscovery cannot leave the old endpoints in play and no refresh reaches the new server. + A long-lived provider holds a CIMD record stamped with another issuer (or, from an older store, + not stamped at all), tokens of matching provenance, and cached metadata. As soon as PRM names + the issuer in use, the tokens and the cached metadata are dropped and the record is re-stamped + and persisted, so a failed rediscovery cannot leave old endpoints in play and no refresh token + of unconfirmed origin reaches the named server. """ cimd_url = "https://client.example.com/.well-known/mcp-client" provider = OAuthClientProvider( @@ -3300,7 +3305,7 @@ async def test_cimd_record_is_restamped_and_its_tokens_and_cached_metadata_dropp client_metadata_url=cimd_url, ) provider.context.client_info = OAuthClientInformationFull( - client_id=cimd_url, token_endpoint_auth_method="none", issuer="https://old-as.example.com" + client_id=cimd_url, token_endpoint_auth_method="none", issuer=stamped_issuer ) provider.context.current_tokens = valid_tokens provider.context.token_expiry_time = time.time() + 1800 From caa022fabafa1b36edb5dbecf9aa2075c1a32200 Mon Sep 17 00:00:00 2001 From: Max Isbey <224885523+maxisbey@users.noreply.github.com> Date: Tue, 18 Aug 2026 13:52:28 +0000 Subject: [PATCH 5/5] Fold the SEP-2352 issuer-binding rules into one method used at both discovery points The bound-credential discard and the CIMD keep-but-rebind case were spelled out inline where PRM names the issuer, and only the former where the issuer is first learned from AS metadata on the legacy no-PRM path. One method now applies both rules wherever the issuer becomes known, so a CIMD record's carried-over tokens are dropped on that path too. A unit test covers it. --- src/mcp/client/auth/oauth2.py | 71 +++++++++++++++-------------------- tests/client/test_auth.py | 55 ++++++++++++++++++++++++++- 2 files changed, 84 insertions(+), 42 deletions(-) diff --git a/src/mcp/client/auth/oauth2.py b/src/mcp/client/auth/oauth2.py index 6ce072525a..12797f8319 100644 --- a/src/mcp/client/auth/oauth2.py +++ b/src/mcp/client/auth/oauth2.py @@ -550,6 +550,29 @@ async def _handle_refresh_response(self, response: httpx2.Response) -> bool: self._initialized = False return False + async def _apply_issuer_binding(self, issuer: str) -> bool: + """Apply SEP-2352 to the held registration now that the authorization server's issuer is known. + + Credentials bound to another issuer are discarded with their tokens so the flow re-registers. + A CIMD record is portable, so it is kept and re-stamped, but tokens it carried over from + another issuer (or of unknown origin, when the record is unstamped) are dropped. Returns + True when the held state was for a different issuer. + """ + client_info = self.context.client_info + if client_info is None: + return False + if not credentials_match_issuer(client_info, issuer, self.context.client_metadata_url): + logger.debug("Authorization server changed; discarding bound credentials and re-registering") + self.context.client_info = None + self.context.clear_tokens() + return True + if client_info.client_id == self.context.client_metadata_url and client_info.issuer != issuer: + self.context.clear_tokens() + client_info.issuer = issuer + await self.context.storage.set_client_info(client_info) + return True + return False + async def _initialize(self) -> None: """Load stored tokens and client info.""" self.context.current_tokens = await self.context.storage.get_tokens() @@ -636,35 +659,13 @@ async def async_auth_flow(self, request: httpx2.Request) -> AsyncGenerator[httpx else: logger.debug(f"Protected resource metadata discovery failed: {url}") - # SEP-2352: stored credentials are bound to the issuer that registered them. - # If the authorization server changed, drop them (and the old tokens) so the - # flow re-registers instead of presenting another server's credentials. - if ( - self.context.client_info is not None - and self.context.auth_server_url is not None - and not credentials_match_issuer( - self.context.client_info, self.context.auth_server_url, self.context.client_metadata_url - ) + # SEP-2352: stored credentials and tokens belong to the issuer they came from. + if self.context.auth_server_url is not None and await self._apply_issuer_binding( + self.context.auth_server_url ): - logger.debug("Authorization server changed; discarding bound credentials and re-registering") - self.context.client_info = None - self.context.clear_tokens() # Any cached AS metadata is for the old server; drop it so a failed - # rediscovery cannot leak the old registration/token endpoints into Step 4. - self.context.oauth_metadata = None - elif ( - self.context.client_info is not None - and self.context.client_info.client_id == self.context.client_metadata_url - and self.context.auth_server_url is not None - and self.context.client_info.issuer != self.context.auth_server_url - ): - # A CIMD client_id is portable across authorization servers; the tokens issued - # under it and the cached metadata are not. Keep the record, re-stamped. An - # unstamped record's tokens have unknown provenance and are dropped the same way. - self.context.clear_tokens() + # rediscovery cannot leak the old endpoints into Steps 4-5. self.context.oauth_metadata = None - self.context.client_info.issuer = self.context.auth_server_url - await self.context.storage.set_client_info(self.context.client_info) asm_discovery_urls = build_oauth_authorization_server_metadata_discovery_urls( self.context.auth_server_url, self.context.server_url @@ -688,21 +689,9 @@ async def async_auth_flow(self, request: httpx2.Request) -> AsyncGenerator[httpx logger.debug(f"OAuth metadata discovery failed: {url}") # SEP-2352: on the legacy no-PRM path the issuer is only known after ASM - # discovery, so re-evaluate the binding here using the discovered metadata - # issuer (mirroring the bound_issuer fallback in Step 4). - if ( - self.context.client_info is not None - and self.context.auth_server_url is None - and self.context.oauth_metadata is not None - and not credentials_match_issuer( - self.context.client_info, - str(self.context.oauth_metadata.issuer), - self.context.client_metadata_url, - ) - ): - logger.debug("Authorization server changed; discarding bound credentials and re-registering") - self.context.client_info = None - self.context.clear_tokens() + # discovery (mirroring the bound_issuer fallback in Step 4). + if self.context.auth_server_url is None and self.context.oauth_metadata is not None: + await self._apply_issuer_binding(str(self.context.oauth_metadata.issuer)) # Step 3: Apply scope selection strategy self.context.client_metadata.scope = get_client_metadata_scopes( diff --git a/tests/client/test_auth.py b/tests/client/test_auth.py index b6c7bf4b68..22870b6a03 100644 --- a/tests/client/test_auth.py +++ b/tests/client/test_auth.py @@ -4,7 +4,7 @@ import json import time from unittest import mock -from urllib.parse import parse_qs, quote, unquote, urlparse +from urllib.parse import parse_qs, parse_qsl, quote, unquote, urlparse import httpx2 import pytest @@ -3336,3 +3336,56 @@ async def test_cimd_record_is_restamped_and_its_tokens_and_cached_metadata_dropp ) assert mock_storage._client_info is provider.context.client_info await auth_flow.aclose() + + +@pytest.mark.anyio +async def test_cimd_record_is_restamped_and_its_tokens_dropped_when_only_asm_reveals_a_new_issuer( + client_metadata: OAuthClientMetadata, mock_storage: MockTokenStorage, valid_tokens: OAuthToken +) -> None: + """The CIMD rebinding also applies on the legacy no-PRM path, where the issuer is learned from AS metadata. + + PRM discovery 404s, so the issuer only becomes known from the root well-known metadata; it + differs from the record's stamp, so the tokens are dropped and the record re-stamped before + any refresh could be attempted, and the flow proceeds to authorize rather than refresh. + """ + cimd_url = "https://client.example.com/.well-known/mcp-client" + provider = OAuthClientProvider( + server_url="https://api.example.com/v1/mcp", + client_metadata=client_metadata, + storage=mock_storage, + client_metadata_url=cimd_url, + ) + provider.context.client_info = OAuthClientInformationFull( + client_id=cimd_url, token_endpoint_auth_method="none", issuer="https://old-as.example.com" + ) + provider.context.current_tokens = valid_tokens + provider.context.token_expiry_time = time.time() + 1800 + provider._initialized = True + provider._perform_authorization_code_grant = mock.AsyncMock(return_value=("auth-code", "verifier")) + + auth_flow = provider.async_auth_flow(httpx2.Request("GET", "https://api.example.com/v1/mcp")) + request = await auth_flow.__anext__() + prm_req = await auth_flow.asend(httpx2.Response(401, request=request)) + prm_req = await auth_flow.asend(httpx2.Response(404, request=prm_req)) + asm_req = await auth_flow.asend(httpx2.Response(404, request=prm_req)) + assert str(asm_req.url) == "https://api.example.com/.well-known/oauth-authorization-server" + asm_response = httpx2.Response( + 200, + content=( + b'{"issuer": "https://api.example.com", ' + b'"authorization_endpoint": "https://api.example.com/authorize", ' + b'"token_endpoint": "https://api.example.com/token", ' + b'"client_id_metadata_document_supported": true}' + ), + request=asm_req, + ) + next_req = await auth_flow.asend(asm_response) + + assert dict(parse_qsl(next_req.content.decode()))["grant_type"] == "authorization_code" + assert provider.context.client_info is not None + assert (provider.context.client_info.client_id, provider.context.client_info.issuer) == ( + cimd_url, + "https://api.example.com", + ) + assert mock_storage._client_info is provider.context.client_info + await auth_flow.aclose()