Skip to content

Commit 890fd15

Browse files
fix(auth): recover from expired client registrations
1 parent a4f4ccd commit 890fd15

5 files changed

Lines changed: 121 additions & 13 deletions

File tree

‎src/mcp/client/auth/oauth2.py‎

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import base64
77
import hashlib
8+
import json
89
import logging
910
import secrets
1011
import string
@@ -109,6 +110,21 @@ def check_registration_usable(client_info: OAuthClientInformationFull) -> None:
109110
)
110111

111112

113+
def _is_expired_client_secret(client_info: OAuthClientInformationFull) -> bool:
114+
"""Return whether a stored registration reports an expired client secret."""
115+
expires_at = client_info.client_secret_expires_at
116+
return expires_at is not None and expires_at != 0 and expires_at <= int(time.time())
117+
118+
119+
def _is_invalid_client_response(body: bytes) -> bool:
120+
"""Identify RFC 6749 ``invalid_client`` responses independent of HTTP status."""
121+
try:
122+
payload = json.loads(body)
123+
except (json.JSONDecodeError, UnicodeDecodeError):
124+
return False
125+
return isinstance(payload, dict) and payload.get("error") == "invalid_client"
126+
127+
112128
class PKCEParameters(BaseModel):
113129
"""PKCE (Proof Key for Code Exchange) parameters."""
114130

@@ -131,16 +147,16 @@ async def get_tokens(self) -> OAuthToken | None:
131147
"""Get stored tokens."""
132148
...
133149

134-
async def set_tokens(self, tokens: OAuthToken) -> None:
135-
"""Store tokens."""
150+
async def set_tokens(self, tokens: OAuthToken | None) -> None:
151+
"""Store tokens, or clear them when ``tokens`` is ``None``."""
136152
...
137153

138154
async def get_client_info(self) -> OAuthClientInformationFull | None:
139155
"""Get stored client information."""
140156
...
141157

142-
async def set_client_info(self, client_info: OAuthClientInformationFull) -> None:
143-
"""Store client information."""
158+
async def set_client_info(self, client_info: OAuthClientInformationFull | None) -> None:
159+
"""Store client information, or clear it when ``client_info`` is ``None``."""
144160
...
145161

146162

@@ -468,6 +484,8 @@ async def _handle_token_response(self, response: httpx2.Response) -> None:
468484
"""Handle token exchange response."""
469485
if response.status_code not in {200, 201}:
470486
body = await response.aread()
487+
if _is_invalid_client_response(body):
488+
await self._clear_stored_credentials()
471489
body_text = body.decode("utf-8")
472490
raise OAuthTokenError(f"Token exchange failed ({response.status_code}): {body_text}")
473491

@@ -519,8 +537,12 @@ async def _refresh_token(self) -> httpx2.Request:
519537
async def _handle_refresh_response(self, response: httpx2.Response) -> bool:
520538
"""Handle token refresh response. Returns True if successful."""
521539
if response.status_code != 200:
540+
body = await response.aread()
522541
logger.warning(f"Token refresh failed: {response.status_code}")
523-
self.context.clear_tokens()
542+
if _is_invalid_client_response(body):
543+
await self._clear_stored_credentials()
544+
else:
545+
self.context.clear_tokens()
524546
return False
525547

526548
try:
@@ -551,8 +573,18 @@ async def _initialize(self) -> None:
551573
"""Load stored tokens and client info."""
552574
self.context.current_tokens = await self.context.storage.get_tokens()
553575
self.context.client_info = await self.context.storage.get_client_info()
576+
if self.context.client_info and _is_expired_client_secret(self.context.client_info):
577+
logger.info("Stored client registration has expired; clearing credentials and re-registering")
578+
await self._clear_stored_credentials()
554579
self._initialized = True
555580

581+
async def _clear_stored_credentials(self) -> None:
582+
"""Clear the in-memory and persisted credentials bound to a client registration."""
583+
self.context.client_info = None
584+
self.context.clear_tokens()
585+
await self.context.storage.set_client_info(None)
586+
await self.context.storage.set_tokens(None)
587+
556588
def _add_auth_header(self, request: httpx2.Request) -> None:
557589
"""Add authorization header to request if we have valid tokens."""
558590
if self.context.current_tokens and self.context.current_tokens.access_token: # pragma: no branch

‎tests/client/auth/extensions/test_client_credentials.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,13 @@ def __init__(self):
2727
async def get_tokens(self) -> OAuthToken | None:
2828
return self._tokens
2929

30-
async def set_tokens(self, tokens: OAuthToken) -> None: # pragma: no cover
30+
async def set_tokens(self, tokens: OAuthToken | None) -> None: # pragma: no cover
3131
self._tokens = tokens
3232

3333
async def get_client_info(self) -> OAuthClientInformationFull | None: # pragma: no cover
3434
return self._client_info
3535

36-
async def set_client_info(self, client_info: OAuthClientInformationFull) -> None: # pragma: no cover
36+
async def set_client_info(self, client_info: OAuthClientInformationFull | None) -> None: # pragma: no cover
3737
self._client_info = client_info
3838

3939

‎tests/client/auth/extensions/test_identity_assertion.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,13 @@ def __init__(self, tokens: OAuthToken | None = None) -> None:
2929
async def get_tokens(self) -> OAuthToken | None:
3030
return self.tokens
3131

32-
async def set_tokens(self, tokens: OAuthToken) -> None:
32+
async def set_tokens(self, tokens: OAuthToken | None) -> None:
3333
self.tokens = tokens
3434

3535
async def get_client_info(self) -> OAuthClientInformationFull | None:
3636
raise NotImplementedError
3737

38-
async def set_client_info(self, client_info: OAuthClientInformationFull) -> None:
38+
async def set_client_info(self, client_info: OAuthClientInformationFull | None) -> None:
3939
raise NotImplementedError
4040

4141

‎tests/client/test_auth.py‎

Lines changed: 78 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,13 +53,13 @@ def __init__(self):
5353
async def get_tokens(self) -> OAuthToken | None:
5454
return self._tokens
5555

56-
async def set_tokens(self, tokens: OAuthToken) -> None:
56+
async def set_tokens(self, tokens: OAuthToken | None) -> None:
5757
self._tokens = tokens
5858

5959
async def get_client_info(self) -> OAuthClientInformationFull | None:
6060
return self._client_info
6161

62-
async def set_client_info(self, client_info: OAuthClientInformationFull) -> None:
62+
async def set_client_info(self, client_info: OAuthClientInformationFull | None) -> None:
6363
self._client_info = client_info
6464

6565

@@ -264,6 +264,26 @@ def test_clear_tokens(self, oauth_provider: OAuthClientProvider, valid_tokens: O
264264
assert context.current_tokens is None
265265
assert context.token_expiry_time is None
266266

267+
@pytest.mark.anyio
268+
async def test_initialize_discards_expired_client_registration(
269+
self, oauth_provider: OAuthClientProvider, mock_storage: MockTokenStorage, valid_tokens: OAuthToken
270+
):
271+
expired_client = OAuthClientInformationFull(
272+
client_id="expired-client",
273+
client_secret="expired-secret",
274+
client_secret_expires_at=int(time.time()) - 1,
275+
redirect_uris=[AnyUrl("http://localhost:3030/callback")],
276+
)
277+
await mock_storage.set_client_info(expired_client)
278+
await mock_storage.set_tokens(valid_tokens)
279+
280+
await oauth_provider._initialize()
281+
282+
assert oauth_provider.context.client_info is None
283+
assert oauth_provider.context.current_tokens is None
284+
assert await mock_storage.get_client_info() is None
285+
assert await mock_storage.get_tokens() is None
286+
267287

268288
class TestOAuthFlow:
269289
"""Test OAuth flow methods."""
@@ -2955,6 +2975,34 @@ async def test_handle_token_response_raises_on_non_2xx_with_body(oauth_provider:
29552975
await oauth_provider._handle_token_response(response)
29562976

29572977

2978+
@pytest.mark.anyio
2979+
async def test_handle_token_response_invalid_client_clears_stored_credentials(
2980+
oauth_provider: OAuthClientProvider, mock_storage: MockTokenStorage, valid_tokens: OAuthToken
2981+
):
2982+
client_info = OAuthClientInformationFull(
2983+
client_id="stale-client",
2984+
client_secret="stale-secret",
2985+
redirect_uris=[AnyUrl("http://localhost:3030/callback")],
2986+
)
2987+
oauth_provider.context.client_info = client_info
2988+
oauth_provider.context.current_tokens = valid_tokens
2989+
await mock_storage.set_client_info(client_info)
2990+
await mock_storage.set_tokens(valid_tokens)
2991+
response = httpx2.Response(
2992+
401,
2993+
json={"error": "invalid_client"},
2994+
request=httpx2.Request("POST", "https://auth.example.com/token"),
2995+
)
2996+
2997+
with pytest.raises(OAuthTokenError, match=r"Token exchange failed \(401\).*invalid_client"):
2998+
await oauth_provider._handle_token_response(response)
2999+
3000+
assert oauth_provider.context.client_info is None
3001+
assert oauth_provider.context.current_tokens is None
3002+
assert await mock_storage.get_client_info() is None
3003+
assert await mock_storage.get_tokens() is None
3004+
3005+
29583006
@pytest.mark.anyio
29593007
async def test_handle_refresh_response_carries_prior_scope_and_refresh_token_when_omitted(
29603008
oauth_provider: OAuthClientProvider, mock_storage: MockTokenStorage
@@ -3007,6 +3055,34 @@ async def test_handle_refresh_response_adopts_rotated_refresh_token_when_returne
30073055
assert stored.refresh_token == "rotated"
30083056

30093057

3058+
@pytest.mark.anyio
3059+
async def test_handle_refresh_response_invalid_client_clears_stored_credentials(
3060+
oauth_provider: OAuthClientProvider, mock_storage: MockTokenStorage, valid_tokens: OAuthToken
3061+
):
3062+
client_info = OAuthClientInformationFull(
3063+
client_id="stale-client",
3064+
client_secret="stale-secret",
3065+
redirect_uris=[AnyUrl("http://localhost:3030/callback")],
3066+
)
3067+
oauth_provider.context.client_info = client_info
3068+
oauth_provider.context.current_tokens = valid_tokens
3069+
await mock_storage.set_client_info(client_info)
3070+
await mock_storage.set_tokens(valid_tokens)
3071+
response = httpx2.Response(
3072+
400,
3073+
json={"error": "invalid_client"},
3074+
request=httpx2.Request("POST", "https://auth.example.com/token"),
3075+
)
3076+
3077+
ok = await oauth_provider._handle_refresh_response(response)
3078+
3079+
assert ok is False
3080+
assert oauth_provider.context.client_info is None
3081+
assert oauth_provider.context.current_tokens is None
3082+
assert await mock_storage.get_client_info() is None
3083+
assert await mock_storage.get_tokens() is None
3084+
3085+
30103086
@pytest.mark.anyio
30113087
async def test_issuer_binding_re_evaluated_after_asm_when_prm_discovery_failed(
30123088
oauth_provider: OAuthClientProvider,

‎tests/client/test_scope_bug_1630.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,13 @@ def __init__(self) -> None:
2929
async def get_tokens(self) -> OAuthToken | None:
3030
return self._tokens # pragma: no cover
3131

32-
async def set_tokens(self, tokens: OAuthToken) -> None:
32+
async def set_tokens(self, tokens: OAuthToken | None) -> None:
3333
self._tokens = tokens
3434

3535
async def get_client_info(self) -> OAuthClientInformationFull | None:
3636
return self._client_info # pragma: no cover
3737

38-
async def set_client_info(self, client_info: OAuthClientInformationFull) -> None:
38+
async def set_client_info(self, client_info: OAuthClientInformationFull | None) -> None:
3939
self._client_info = client_info # pragma: no cover
4040

4141

0 commit comments

Comments
 (0)