From 4fac3c7b015ccc3ac6c806c528bc8c618448c1e3 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 05:32:40 +0000 Subject: [PATCH 1/3] fix(client/auth): discard stored client registrations with an expired secret The client persists client_secret_expires_at (RFC 7591) through TokenStorage but never reads it back, and registration only happens when stored client info is absent. Once a dynamically registered secret lapses, every token-endpoint interaction fails with invalid_client - including the exchange after a fresh interactive authorization - so the client is permanently stuck until the application manually deletes the persisted client info (#3256). Treat a stored registration whose secret-authenticating record carries a non-zero, past client_secret_expires_at as absent when loading from storage. The next 401 flow then re-registers (or resolves CIMD) and overwrites the dead record via the existing set_client_info call - no change to the TokenStorage contract. Stored tokens are kept: a live access token continues to work without client authentication, and with no client info the refresh path that would present the lapsed secret is skipped. Fixes #3256 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01CjbXueCDdFNJK6imejCXgM --- src/mcp/client/auth/oauth2.py | 34 +++++++++++- tests/client/test_auth.py | 101 ++++++++++++++++++++++++++++++++++ 2 files changed, 133 insertions(+), 2 deletions(-) diff --git a/src/mcp/client/auth/oauth2.py b/src/mcp/client/auth/oauth2.py index 7dc62b52b9..46cb33e9c4 100644 --- a/src/mcp/client/auth/oauth2.py +++ b/src/mcp/client/auth/oauth2.py @@ -109,6 +109,22 @@ def check_registration_usable(client_info: OAuthClientInformationFull) -> None: ) +def stored_registration_expired(client_info: OAuthClientInformationFull) -> bool: + """Whether a stored registration's minted secret has lapsed and can no longer authenticate. + + RFC 7591 requires `client_secret_expires_at` whenever a secret is issued, with ``0`` + meaning the secret never expires. Once a non-zero expiry passes, every token-endpoint + interaction authenticating with that secret fails with ``invalid_client`` — and with no + RFC 7592 rotation endpoint, re-registration is the only standard recovery. The lapse + only matters for registrations that authenticate with the minted secret: ``none`` (or + an absent method) sends no secret, and `private_key_jwt` signs an assertion instead. + """ + if client_info.token_endpoint_auth_method not in _SECRET_TOKEN_ENDPOINT_AUTH_METHODS: + return False + expires_at = client_info.client_secret_expires_at + return expires_at is not None and expires_at != 0 and expires_at < int(time.time()) + + class PKCEParameters(BaseModel): """PKCE (Proof Key for Code Exchange) parameters.""" @@ -548,9 +564,23 @@ async def _handle_refresh_response(self, response: httpx2.Response) -> bool: return False async def _initialize(self) -> None: - """Load stored tokens and client info.""" + """Load stored tokens and client info. + + Stored client information whose minted secret has expired (RFC 7591 + `client_secret_expires_at`) is treated as absent: reusing it can only produce + `invalid_client` at the token endpoint — even interactive re-authorization ends in + the same failure, permanently — so it is discarded here and the next 401 flow + re-registers (or resolves CIMD), overwriting the dead record in storage. Any still + stored tokens are kept: a live access token keeps working without client + authentication, and with no client info the refresh path (which would present the + lapsed secret) is skipped. + """ self.context.current_tokens = await self.context.storage.get_tokens() - self.context.client_info = await self.context.storage.get_client_info() + client_info = await self.context.storage.get_client_info() + if client_info is not None and stored_registration_expired(client_info): + logger.debug("Stored client registration secret has expired; discarding so the next flow re-registers") + client_info = None + self.context.client_info = client_info self._initialized = True def _add_auth_header(self, request: httpx2.Request) -> None: diff --git a/tests/client/test_auth.py b/tests/client/test_auth.py index be96cc8eec..d9056dd42a 100644 --- a/tests/client/test_auth.py +++ b/tests/client/test_auth.py @@ -13,6 +13,7 @@ from mcp.client.auth import OAuthClientProvider, PKCEParameters from mcp.client.auth.exceptions import OAuthFlowError, OAuthRegistrationError, OAuthTokenError +from mcp.client.auth.oauth2 import stored_registration_expired from mcp.client.auth.utils import ( build_oauth_authorization_server_metadata_discovery_urls, build_protected_resource_metadata_discovery_urls, @@ -3253,3 +3254,103 @@ async def echo_callback() -> AuthorizationCodeResult: await auth_flow.asend(httpx2.Response(200, request=final_req)) except StopAsyncIteration: pass + + +def test_stored_registration_expired_only_for_lapsed_secret_backed_registrations(): + """RFC 7591: only a non-zero, past `client_secret_expires_at` on a secret-authenticating + registration marks the stored record as expired; `0` means the secret never expires, and + methods that send no secret (`none`) are unaffected by the lapse. + """ + base: dict[str, object] = { + "client_id": "c", + "client_secret": "s", + "redirect_uris": [AnyUrl("http://localhost:3030/callback")], + } + lapsed = int(time.time()) - 3600 + live = int(time.time()) + 3600 + + expired = OAuthClientInformationFull.model_validate( + {**base, "token_endpoint_auth_method": "client_secret_post", "client_secret_expires_at": lapsed} + ) + assert stored_registration_expired(expired) + assert stored_registration_expired( + OAuthClientInformationFull.model_validate( + {**base, "token_endpoint_auth_method": "client_secret_basic", "client_secret_expires_at": lapsed} + ) + ) + + # 0 means "never expires" (RFC 7591); absent means no expiry was declared. + assert not stored_registration_expired( + OAuthClientInformationFull.model_validate( + {**base, "token_endpoint_auth_method": "client_secret_post", "client_secret_expires_at": 0} + ) + ) + assert not stored_registration_expired( + OAuthClientInformationFull.model_validate({**base, "token_endpoint_auth_method": "client_secret_post"}) + ) + + # Still-live secret, and methods that never present the secret. + assert not stored_registration_expired( + OAuthClientInformationFull.model_validate( + {**base, "token_endpoint_auth_method": "client_secret_post", "client_secret_expires_at": live} + ) + ) + assert not stored_registration_expired( + OAuthClientInformationFull.model_validate( + {**base, "token_endpoint_auth_method": "none", "client_secret_expires_at": lapsed} + ) + ) + + +@pytest.mark.anyio +async def test_expired_stored_registration_is_discarded_and_the_flow_re_registers( + oauth_provider: OAuthClientProvider, mock_storage: MockTokenStorage, valid_tokens: OAuthToken +): + """Regression for #3256: a stored DCR registration whose secret has lapsed is not reused. + + Reusing it makes every token-endpoint interaction fail with ``invalid_client`` — even a + fresh interactive authorization ends in the same failure, so the client is permanently + stuck (\"I re-authenticated and nothing changed\"). The lapsed record must be treated as + absent on load, so the next 401 flow re-registers instead of presenting the dead secret; + stored tokens are kept (a live access token still works without client authentication). + """ + await mock_storage.set_client_info( + OAuthClientInformationFull( + client_id="dead-client", + client_secret="expired-secret", + client_secret_expires_at=int(time.time()) - 3600, + redirect_uris=[AnyUrl("http://localhost:3030/callback")], + token_endpoint_auth_method="client_secret_post", + ) + ) + await mock_storage.set_tokens(valid_tokens) + + auth_flow = oauth_provider.async_auth_flow(httpx2.Request("GET", "https://api.example.com/v1/mcp")) + + # The lapsed registration is treated as absent; the stored access token is kept and used. + request = await auth_flow.__anext__() + assert oauth_provider.context.client_info is None + assert oauth_provider.context.current_tokens is not None + assert request.headers["Authorization"] == f"Bearer {valid_tokens.access_token}" + + # Server rejects the stale token: the 401 flow re-registers instead of reusing the record. + response_401 = httpx2.Response(401, request=request) + prm_req = await auth_flow.asend(response_401) + 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'"registration_endpoint": "https://api.example.com/register"}' + ), + request=asm_req, + ) + + register_req = await auth_flow.asend(asm_response) + assert register_req.method == "POST" + assert str(register_req.url) == "https://api.example.com/register" + await auth_flow.aclose() From 50d6b0b2ff10fb44bd2dad2ef35ee8aa766a5a4e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 05:56:10 +0000 Subject: [PATCH 2/3] fix(client/auth): re-check secret expiry in the 401 flow; document the discard Address review findings: _initialize runs once per provider instance, so a registration whose secret lapses mid-session (long-lived process) was still reused - Step 4 skipped re-registration and the interactive authorization burned a user consent only to fail invalid_client at the token exchange. Re-check stored_registration_expired at the start of the 401 handler and discard the dead record so Step 4 re-registers. Also update docs/client/oauth-clients.md: the storage tip and the 'stored client_info still wins' statement now mention the expired-secret discard. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01CjbXueCDdFNJK6imejCXgM --- docs/client/oauth-clients.md | 7 +++++- src/mcp/client/auth/oauth2.py | 12 ++++++++++ tests/client/test_auth.py | 45 +++++++++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 1 deletion(-) diff --git a/docs/client/oauth-clients.md b/docs/client/oauth-clients.md index cd7de35626..131ebe3e85 100644 --- a/docs/client/oauth-clients.md +++ b/docs/client/oauth-clients.md @@ -52,6 +52,11 @@ The in-memory version above works. It also forgets everything when the process e Store `client_info`, not only the tokens. The provider registers dynamically the first time it finds no stored `client_info`. Throw it away and you mint a fresh registration on every run. + One exception: a stored registration whose dynamically issued secret has expired (a non-zero + `client_secret_expires_at` in the past) is treated as absent — the expired secret could never + authenticate again, so the provider discards the record and re-registers on the next flow, + overwriting it in your storage with the fresh registration. + ### The two handlers The authorization code flow needs a human exactly once: someone has to sign in and click "allow". @@ -95,7 +100,7 @@ The repository ships the live version. `examples/servers/simple-auth/` runs a st The 2026-07-28 revision of the spec deprecates dynamic client registration in favor of **Client ID Metadata Documents** (CIMD). Instead of POSTing a fresh registration to every authorization server it meets, your client publishes one JSON document about itself at a stable HTTPS URL, and that URL *is* its `client_id`. The authorization server fetches the document; the provider never touches it. -The SDK already speaks it: pass the URL as `client_metadata_url=` when you construct the provider. When the authorization server's metadata advertises `client_id_metadata_document_supported: true`, the provider skips the `/register` request entirely: the URL goes into the flow as the `client_id`, and there is no `client_secret`. When the server doesn't advertise it (most don't yet), or you never pass a URL, the provider falls back to dynamic registration **silently**, and everything above works exactly as described. Stored `client_info` still wins over both. +The SDK already speaks it: pass the URL as `client_metadata_url=` when you construct the provider. When the authorization server's metadata advertises `client_id_metadata_document_supported: true`, the provider skips the `/register` request entirely: the URL goes into the flow as the `client_id`, and there is no `client_secret`. When the server doesn't advertise it (most don't yet), or you never pass a URL, the provider falls back to dynamic registration **silently**, and everything above works exactly as described. Stored `client_info` still wins over both, as long as its registration is usable — a record whose dynamically issued secret has expired is discarded and the provider registers (or resolves the CIMD URL) afresh. The URL must be HTTPS with a non-root path; anything else is a `ValueError` at construction, before any network happens. The shipped `examples/clients/simple-auth-client/` takes it as the `MCP_CLIENT_METADATA_URL` environment variable. diff --git a/src/mcp/client/auth/oauth2.py b/src/mcp/client/auth/oauth2.py index 46cb33e9c4..a35c8a10ac 100644 --- a/src/mcp/client/auth/oauth2.py +++ b/src/mcp/client/auth/oauth2.py @@ -634,6 +634,18 @@ async def async_auth_flow(self, request: httpx2.Request) -> AsyncGenerator[httpx # Perform full OAuth flow try: # OAuth flow must be inline due to generator constraints + + # A registration whose minted secret lapsed mid-session (after + # _initialize already loaded it) can no longer authenticate either — + # discard it here too, so Step 4 re-registers instead of running an + # interactive authorization doomed to fail `invalid_client` at the + # token endpoint. + if self.context.client_info is not None and stored_registration_expired(self.context.client_info): + logger.debug( + "Stored client registration secret has expired; discarding so this flow re-registers" + ) + self.context.client_info = None + www_auth_resource_metadata_url = extract_resource_metadata_from_www_auth(response) # Step 1: Discover protected resource metadata (SEP-985 with fallback support) diff --git a/tests/client/test_auth.py b/tests/client/test_auth.py index d9056dd42a..64f7a40323 100644 --- a/tests/client/test_auth.py +++ b/tests/client/test_auth.py @@ -3354,3 +3354,48 @@ async def test_expired_stored_registration_is_discarded_and_the_flow_re_register assert register_req.method == "POST" assert str(register_req.url) == "https://api.example.com/register" await auth_flow.aclose() + + +@pytest.mark.anyio +async def test_registration_that_expires_mid_session_is_discarded_by_the_401_flow( + oauth_provider: OAuthClientProvider, +): + """A secret that lapses after load (long-lived process) is also discarded, in-flow. + + ``_initialize`` runs once per provider instance, so a registration that expires while + the process is running would otherwise be reused by the 401 flow: Step 4 would skip + re-registration and the interactive authorization would burn a user consent only to + fail ``invalid_client`` at the token exchange. The 401 handler re-checks the expiry + and discards the dead record so Step 4 re-registers instead. + """ + oauth_provider._initialized = True # already initialized while the secret was live + oauth_provider.context.client_info = OAuthClientInformationFull( + client_id="dead-client", + client_secret="expired-secret", + client_secret_expires_at=int(time.time()) - 3600, # lapsed after load + redirect_uris=[AnyUrl("http://localhost:3030/callback")], + token_endpoint_auth_method="client_secret_post", + ) + + auth_flow = oauth_provider.async_auth_flow(httpx2.Request("GET", "https://api.example.com/v1/mcp")) + request = await auth_flow.__anext__() + + # 401 → discovery; the lapsed registration is discarded, so the flow re-registers. + 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)) + 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'"registration_endpoint": "https://api.example.com/register"}' + ), + request=asm_req, + ) + + register_req = await auth_flow.asend(asm_response) + assert register_req.method == "POST" + assert str(register_req.url) == "https://api.example.com/register" + await auth_flow.aclose() From 6b7ab0d87fdcc00b4ce8191cf4bbe1f020f6b356 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 05:59:44 +0000 Subject: [PATCH 3/3] chore: retrigger CI after PyPI download timeout The 3.13/locked/ubuntu job failed fetching opentelemetry-api from files.pythonhosted.org (operation timed out) during uv sync - an infrastructure flake unrelated to the change. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01CjbXueCDdFNJK6imejCXgM