Skip to content

Commit 4fac3c7

Browse files
committed
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CjbXueCDdFNJK6imejCXgM
1 parent a4f4ccd commit 4fac3c7

2 files changed

Lines changed: 133 additions & 2 deletions

File tree

src/mcp/client/auth/oauth2.py

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,22 @@ def check_registration_usable(client_info: OAuthClientInformationFull) -> None:
109109
)
110110

111111

112+
def stored_registration_expired(client_info: OAuthClientInformationFull) -> bool:
113+
"""Whether a stored registration's minted secret has lapsed and can no longer authenticate.
114+
115+
RFC 7591 requires `client_secret_expires_at` whenever a secret is issued, with ``0``
116+
meaning the secret never expires. Once a non-zero expiry passes, every token-endpoint
117+
interaction authenticating with that secret fails with ``invalid_client`` — and with no
118+
RFC 7592 rotation endpoint, re-registration is the only standard recovery. The lapse
119+
only matters for registrations that authenticate with the minted secret: ``none`` (or
120+
an absent method) sends no secret, and `private_key_jwt` signs an assertion instead.
121+
"""
122+
if client_info.token_endpoint_auth_method not in _SECRET_TOKEN_ENDPOINT_AUTH_METHODS:
123+
return False
124+
expires_at = client_info.client_secret_expires_at
125+
return expires_at is not None and expires_at != 0 and expires_at < int(time.time())
126+
127+
112128
class PKCEParameters(BaseModel):
113129
"""PKCE (Proof Key for Code Exchange) parameters."""
114130

@@ -548,9 +564,23 @@ async def _handle_refresh_response(self, response: httpx2.Response) -> bool:
548564
return False
549565

550566
async def _initialize(self) -> None:
551-
"""Load stored tokens and client info."""
567+
"""Load stored tokens and client info.
568+
569+
Stored client information whose minted secret has expired (RFC 7591
570+
`client_secret_expires_at`) is treated as absent: reusing it can only produce
571+
`invalid_client` at the token endpoint — even interactive re-authorization ends in
572+
the same failure, permanently — so it is discarded here and the next 401 flow
573+
re-registers (or resolves CIMD), overwriting the dead record in storage. Any still
574+
stored tokens are kept: a live access token keeps working without client
575+
authentication, and with no client info the refresh path (which would present the
576+
lapsed secret) is skipped.
577+
"""
552578
self.context.current_tokens = await self.context.storage.get_tokens()
553-
self.context.client_info = await self.context.storage.get_client_info()
579+
client_info = await self.context.storage.get_client_info()
580+
if client_info is not None and stored_registration_expired(client_info):
581+
logger.debug("Stored client registration secret has expired; discarding so the next flow re-registers")
582+
client_info = None
583+
self.context.client_info = client_info
554584
self._initialized = True
555585

556586
def _add_auth_header(self, request: httpx2.Request) -> None:

tests/client/test_auth.py

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
from mcp.client.auth import OAuthClientProvider, PKCEParameters
1515
from mcp.client.auth.exceptions import OAuthFlowError, OAuthRegistrationError, OAuthTokenError
16+
from mcp.client.auth.oauth2 import stored_registration_expired
1617
from mcp.client.auth.utils import (
1718
build_oauth_authorization_server_metadata_discovery_urls,
1819
build_protected_resource_metadata_discovery_urls,
@@ -3253,3 +3254,103 @@ async def echo_callback() -> AuthorizationCodeResult:
32533254
await auth_flow.asend(httpx2.Response(200, request=final_req))
32543255
except StopAsyncIteration:
32553256
pass
3257+
3258+
3259+
def test_stored_registration_expired_only_for_lapsed_secret_backed_registrations():
3260+
"""RFC 7591: only a non-zero, past `client_secret_expires_at` on a secret-authenticating
3261+
registration marks the stored record as expired; `0` means the secret never expires, and
3262+
methods that send no secret (`none`) are unaffected by the lapse.
3263+
"""
3264+
base: dict[str, object] = {
3265+
"client_id": "c",
3266+
"client_secret": "s",
3267+
"redirect_uris": [AnyUrl("http://localhost:3030/callback")],
3268+
}
3269+
lapsed = int(time.time()) - 3600
3270+
live = int(time.time()) + 3600
3271+
3272+
expired = OAuthClientInformationFull.model_validate(
3273+
{**base, "token_endpoint_auth_method": "client_secret_post", "client_secret_expires_at": lapsed}
3274+
)
3275+
assert stored_registration_expired(expired)
3276+
assert stored_registration_expired(
3277+
OAuthClientInformationFull.model_validate(
3278+
{**base, "token_endpoint_auth_method": "client_secret_basic", "client_secret_expires_at": lapsed}
3279+
)
3280+
)
3281+
3282+
# 0 means "never expires" (RFC 7591); absent means no expiry was declared.
3283+
assert not stored_registration_expired(
3284+
OAuthClientInformationFull.model_validate(
3285+
{**base, "token_endpoint_auth_method": "client_secret_post", "client_secret_expires_at": 0}
3286+
)
3287+
)
3288+
assert not stored_registration_expired(
3289+
OAuthClientInformationFull.model_validate({**base, "token_endpoint_auth_method": "client_secret_post"})
3290+
)
3291+
3292+
# Still-live secret, and methods that never present the secret.
3293+
assert not stored_registration_expired(
3294+
OAuthClientInformationFull.model_validate(
3295+
{**base, "token_endpoint_auth_method": "client_secret_post", "client_secret_expires_at": live}
3296+
)
3297+
)
3298+
assert not stored_registration_expired(
3299+
OAuthClientInformationFull.model_validate(
3300+
{**base, "token_endpoint_auth_method": "none", "client_secret_expires_at": lapsed}
3301+
)
3302+
)
3303+
3304+
3305+
@pytest.mark.anyio
3306+
async def test_expired_stored_registration_is_discarded_and_the_flow_re_registers(
3307+
oauth_provider: OAuthClientProvider, mock_storage: MockTokenStorage, valid_tokens: OAuthToken
3308+
):
3309+
"""Regression for #3256: a stored DCR registration whose secret has lapsed is not reused.
3310+
3311+
Reusing it makes every token-endpoint interaction fail with ``invalid_client`` — even a
3312+
fresh interactive authorization ends in the same failure, so the client is permanently
3313+
stuck (\"I re-authenticated and nothing changed\"). The lapsed record must be treated as
3314+
absent on load, so the next 401 flow re-registers instead of presenting the dead secret;
3315+
stored tokens are kept (a live access token still works without client authentication).
3316+
"""
3317+
await mock_storage.set_client_info(
3318+
OAuthClientInformationFull(
3319+
client_id="dead-client",
3320+
client_secret="expired-secret",
3321+
client_secret_expires_at=int(time.time()) - 3600,
3322+
redirect_uris=[AnyUrl("http://localhost:3030/callback")],
3323+
token_endpoint_auth_method="client_secret_post",
3324+
)
3325+
)
3326+
await mock_storage.set_tokens(valid_tokens)
3327+
3328+
auth_flow = oauth_provider.async_auth_flow(httpx2.Request("GET", "https://api.example.com/v1/mcp"))
3329+
3330+
# The lapsed registration is treated as absent; the stored access token is kept and used.
3331+
request = await auth_flow.__anext__()
3332+
assert oauth_provider.context.client_info is None
3333+
assert oauth_provider.context.current_tokens is not None
3334+
assert request.headers["Authorization"] == f"Bearer {valid_tokens.access_token}"
3335+
3336+
# Server rejects the stale token: the 401 flow re-registers instead of reusing the record.
3337+
response_401 = httpx2.Response(401, request=request)
3338+
prm_req = await auth_flow.asend(response_401)
3339+
prm_req = await auth_flow.asend(httpx2.Response(404, request=prm_req))
3340+
asm_req = await auth_flow.asend(httpx2.Response(404, request=prm_req))
3341+
assert str(asm_req.url) == "https://api.example.com/.well-known/oauth-authorization-server"
3342+
asm_response = httpx2.Response(
3343+
200,
3344+
content=(
3345+
b'{"issuer": "https://api.example.com", '
3346+
b'"authorization_endpoint": "https://api.example.com/authorize", '
3347+
b'"token_endpoint": "https://api.example.com/token", '
3348+
b'"registration_endpoint": "https://api.example.com/register"}'
3349+
),
3350+
request=asm_req,
3351+
)
3352+
3353+
register_req = await auth_flow.asend(asm_response)
3354+
assert register_req.method == "POST"
3355+
assert str(register_req.url) == "https://api.example.com/register"
3356+
await auth_flow.aclose()

0 commit comments

Comments
 (0)