Skip to content

Commit 07d639a

Browse files
committed
OAuth client: refresh before re-authorizing, and discover before refreshing
OAuthClientProvider had two disconnected ways to obtain a token: a pre-request branch that could refresh but never discovered metadata, and a 401 branch that discovered but never tried the refresh token. After a process restart the first is unreachable (no expiry is restored, so the loaded token always reads as valid) and the second goes straight to interactive authorization, so every restarted client re-opened the browser once its access token lapsed and headless clients failed outright. When an application forced the pre-request path, the refresh was posted to a path guessed from the server origin and 404ed against any authorization server mounted under a path. Separately, a dynamically registered client whose secret had expired was reused forever: the stored client_secret_expires_at was never read and invalid_client from the token endpoint was not recognised, so each attempt spent a consent and failed at the code exchange. Both entry points now drive one sequence, _reacquire_tokens: discover (always on a 401, and before a cold-start refresh when nothing is cached), drop a registration the SDK minted whose secret has lapsed, try the refresh_token grant when one is held, and only then run the provider's full grant. invalid_client from the token endpoint discards an SDK-minted registration and its tokens and runs registration once more; pre-registered credentials surface the error instead. _initialize derives an expiry from the loaded token unless the application already set one. The 2025-03-26 origin-path fallbacks remain for servers with no metadata but are no longer reached merely because state was not loaded. Closes #3240, #3250, #3256, #1318.
1 parent b2025ab commit 07d639a

8 files changed

Lines changed: 813 additions & 169 deletions

File tree

docs/client/oauth-clients.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,11 @@ The in-memory version above works. It also forgets everything when the process e
5151
!!! tip
5252
Store `client_info`, not only the tokens. The provider registers dynamically the first time it
5353
finds no stored `client_info`. Throw it away and you mint a fresh registration on every run.
54+
The one case where the provider throws it away for you is a registration it made that has
55+
stopped working: its `client_secret_expires_at` has passed, or the token endpoint answered
56+
`invalid_client`. Then it registers again and overwrites the stored record. Credentials you
57+
seeded into storage yourself are never replaced this way; an `invalid_client` for those
58+
surfaces as an `OAuthTokenError`.
5459

5560
### The two handlers
5661

@@ -81,7 +86,7 @@ The first time `Client` sends a request, the server answers `401`. The provider
8186
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.
8287
4. **Exchange.** It trades the code for an `OAuthToken`, stores it, and replays your original request with `Authorization: Bearer ...`.
8388

84-
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.
89+
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.
8590

8691
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.
8792

src/mcp/client/auth/oauth2.py

Lines changed: 265 additions & 151 deletions
Large diffs are not rendered by default.

src/mcp/server/auth/middleware/client_auth.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,6 @@ async def authenticate_request(self, request: Request) -> OAuthClientInformation
114114
raise AuthenticationError("Invalid client_secret")
115115

116116
if client.client_secret_expires_at and client.client_secret_expires_at < int(time.time()):
117-
raise AuthenticationError("Client secret has expired") # pragma: no cover
117+
raise AuthenticationError("Client secret has expired")
118118

119119
return client

tests/client/test_auth.py

Lines changed: 88 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 client_secret_lapsed, token_error_code
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,90 @@ async def echo_callback() -> AuthorizationCodeResult:
32533254
await auth_flow.asend(httpx2.Response(200, request=final_req))
32543255
except StopAsyncIteration:
32553256
pass
3257+
3258+
3259+
@pytest.mark.parametrize(
3260+
("method", "expires_at", "expected"),
3261+
[
3262+
pytest.param("client_secret_post", 999, True, id="post-secret-past"),
3263+
pytest.param("client_secret_basic", 999, True, id="basic-secret-past"),
3264+
pytest.param("client_secret_post", 1001, False, id="secret-still-live"),
3265+
pytest.param("client_secret_post", 0, False, id="zero-never-expires"),
3266+
pytest.param("client_secret_post", None, False, id="expiry-undeclared"),
3267+
pytest.param("none", 999, False, id="public-client-ignores-secret-expiry"),
3268+
],
3269+
)
3270+
def test_client_secret_lapsed_only_for_secret_auth_with_a_past_nonzero_expiry(
3271+
method: str, expires_at: int | None, expected: bool
3272+
) -> None:
3273+
"""RFC 7591 §3.2.1: a stored secret is dead once `client_secret_expires_at` (non-zero) is in the past.
3274+
3275+
Only registrations that authenticate with the secret are affected; `0` and an absent field
3276+
both mean no expiry is known.
3277+
"""
3278+
info = OAuthClientInformationFull(
3279+
client_id="c", client_secret="s", client_secret_expires_at=expires_at, token_endpoint_auth_method=method
3280+
)
3281+
assert client_secret_lapsed(info, now=1000) is expected
3282+
3283+
3284+
def test_client_secret_lapsed_defaults_to_the_current_time() -> None:
3285+
"""Without an explicit `now`, the wall clock decides."""
3286+
info = OAuthClientInformationFull(
3287+
client_id="c",
3288+
client_secret="s",
3289+
client_secret_expires_at=int(time.time()) - 60,
3290+
token_endpoint_auth_method="client_secret_post",
3291+
)
3292+
assert client_secret_lapsed(info) is True
3293+
3294+
3295+
@pytest.mark.anyio
3296+
@pytest.mark.parametrize(
3297+
("status", "body", "expected"),
3298+
[
3299+
pytest.param(400, b'{"error":"invalid_grant"}', "invalid_grant", id="400-json-error"),
3300+
pytest.param(401, b'{"error":"invalid_client","error_description":"x"}', "invalid_client", id="401-json-error"),
3301+
pytest.param(404, b'{"error":"invalid_client"}', None, id="non-token-error-status"),
3302+
pytest.param(400, b"<html>bad gateway</html>", None, id="non-json-body"),
3303+
pytest.param(400, b'["invalid_client"]', None, id="json-but-not-an-object"),
3304+
pytest.param(400, b'{"error": 7}', None, id="error-member-not-a-string"),
3305+
],
3306+
)
3307+
async def test_token_error_code_reads_the_rfc6749_error_member_from_400_and_401_bodies(
3308+
status: int, body: bytes, expected: str | None
3309+
) -> None:
3310+
"""RFC 6749 §5.2 puts token-endpoint errors on 400, or 401 for `invalid_client`; anything else carries no code."""
3311+
response = httpx2.Response(status, content=body, request=httpx2.Request("POST", "https://as.example/token"))
3312+
assert await token_error_code(response) == expected
3313+
3314+
3315+
@pytest.mark.anyio
3316+
async def test_initialize_derives_expiry_from_the_loaded_token(
3317+
oauth_provider: OAuthClientProvider, mock_storage: MockTokenStorage, valid_tokens: OAuthToken
3318+
) -> None:
3319+
"""A token loaded from storage gets an expiry derived from its `expires_in` instead of counting as valid forever."""
3320+
await mock_storage.set_tokens(valid_tokens)
3321+
before = time.time()
3322+
3323+
await oauth_provider._initialize()
3324+
3325+
assert oauth_provider.context.token_expiry_time is not None
3326+
assert oauth_provider.context.token_expiry_time >= before + 3600
3327+
3328+
3329+
@pytest.mark.anyio
3330+
async def test_initialize_keeps_an_expiry_the_application_already_set(
3331+
oauth_provider: OAuthClientProvider, mock_storage: MockTokenStorage, valid_tokens: OAuthToken
3332+
) -> None:
3333+
"""An application that records the real expiry and assigns `context.token_expiry_time` itself is not overridden.
3334+
3335+
Storages in the wild set it before the first request or from inside `get_tokens`; the value
3336+
derived from a persisted relative `expires_in` would be staler than theirs.
3337+
"""
3338+
await mock_storage.set_tokens(valid_tokens)
3339+
oauth_provider.context.token_expiry_time = 12345.0
3340+
3341+
await oauth_provider._initialize()
3342+
3343+
assert oauth_provider.context.token_expiry_time == 12345.0

tests/interaction/_requirements.py

Lines changed: 37 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3783,22 +3783,16 @@ def __post_init__(self) -> None:
37833783
note="OAuth is HTTP-only.",
37843784
),
37853785
"client-auth:invalid-client-clears-all": Requirement(
3786-
source="sdk",
3786+
source="issue:#3256",
37873787
behavior=(
3788-
"An invalid-client or unauthorized-client error during authorization invalidates all stored credentials."
3788+
"An invalid_client error from the token endpoint (refresh or code exchange) for a registration "
3789+
"the SDK obtained itself discards that registration and the tokens bound to it, and the flow "
3790+
"re-registers and continues once; for pre-registered credentials the error surfaces instead."
37893791
),
37903792
transports=("streamable-http",),
3791-
note="OAuth is HTTP-only.",
3792-
divergence=Divergence(
3793-
note=(
3794-
"The token-response handlers do not parse the error body; an invalid_client or "
3795-
"unauthorized_client response leaves stored client_info untouched. The TypeScript SDK "
3796-
"clears it."
3797-
),
3798-
),
3799-
deferred=(
3800-
"Not implemented in the SDK: no token-response path inspects the error code to decide "
3801-
"whether to clear client_info."
3793+
note=(
3794+
"OAuth is HTTP-only. Registrations the SDK minted carry the SEP-2352 issuer stamp; that is the "
3795+
"provenance test. unauthorized_client is not treated the same way (the TypeScript SDK does)."
38023796
),
38033797
),
38043798
"client-auth:invalid-grant-clears-tokens": Requirement(
@@ -3883,6 +3877,36 @@ def __post_init__(self) -> None:
38833877
transports=("streamable-http",),
38843878
note="OAuth is HTTP-only.",
38853879
),
3880+
"client-auth:refresh:on-401": Requirement(
3881+
source="issue:#3250",
3882+
behavior=(
3883+
"A 401 received while a refresh token is held is answered, after rediscovery, with a "
3884+
"refresh_token grant before any interactive authorization, so a client constructed over "
3885+
"persisted tokens and client registration recovers from an expired access token headlessly."
3886+
),
3887+
transports=("streamable-http",),
3888+
note="OAuth is HTTP-only. RFC 6749 §1.5 (E)-(H); matches the TypeScript, C# and Rust SDKs.",
3889+
),
3890+
"client-auth:refresh:discovered-endpoint": Requirement(
3891+
source="issue:#3240",
3892+
behavior=(
3893+
"A refresh attempted before the first request of a process (the loaded token is known to be "
3894+
"expired) performs protected-resource and authorization-server metadata discovery first and "
3895+
"posts to the advertised token endpoint, never to a path guessed from the server origin."
3896+
),
3897+
transports=("streamable-http",),
3898+
note="OAuth is HTTP-only.",
3899+
),
3900+
"client-auth:registration:secret-expiry": Requirement(
3901+
source="issue:#3256",
3902+
behavior=(
3903+
"A stored dynamically registered client whose client_secret_expires_at (RFC 7591) has passed is "
3904+
"treated as absent: the flow registers afresh before authorizing instead of presenting the dead "
3905+
"secret at the token endpoint."
3906+
),
3907+
transports=("streamable-http",),
3908+
note="OAuth is HTTP-only. 0 means the secret never expires; only secret-based auth methods are affected.",
3909+
),
38863910
"client-auth:resource-parameter": Requirement(
38873911
source=f"{SPEC_BASE_URL}/basic/authorization#resource-parameter-implementation",
38883912
behavior=(

tests/interaction/auth/_harness.py

Lines changed: 76 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,14 @@
2626
from mcp.server import Server
2727
from mcp.server.auth.provider import AccessToken, ProviderTokenVerifier
2828
from mcp.server.auth.settings import AuthSettings, ClientRegistrationOptions, RevocationOptions
29-
from mcp.shared.auth import AuthorizationCodeResult, OAuthClientInformationFull, OAuthClientMetadata, OAuthToken
29+
from mcp.shared.auth import (
30+
AuthorizationCodeResult,
31+
OAuthClientInformationFull,
32+
OAuthClientMetadata,
33+
OAuthMetadata,
34+
OAuthToken,
35+
ProtectedResourceMetadata,
36+
)
3037
from tests.interaction._connect import BASE_URL, NO_DNS_REBINDING_PROTECTION
3138
from tests.interaction.auth._provider import InMemoryAuthorizationServerProvider
3239
from tests.interaction.transports._bridge import StreamingASGITransport
@@ -107,13 +114,23 @@ class InMemoryTokenStorage:
107114
Tests pre-seed `client_info` (via the constructor or by assignment) to drive the
108115
pre-registered path, and read both attributes after the flow to assert what the SDK
109116
persisted.
117+
118+
`report_expired_on_load`: `get_tokens` returns the held token with `expires_in=0`, standing
119+
in for an application storage that records an absolute expiry and reports the remaining
120+
lifetime on load, so a freshly constructed provider knows the access token is already dead
121+
before it sends anything.
110122
"""
111123

112-
def __init__(self, *, client_info: OAuthClientInformationFull | None = None) -> None:
124+
def __init__(
125+
self, *, client_info: OAuthClientInformationFull | None = None, report_expired_on_load: bool = False
126+
) -> None:
113127
self.tokens: OAuthToken | None = None
114128
self.client_info: OAuthClientInformationFull | None = client_info
129+
self.report_expired_on_load = report_expired_on_load
115130

116131
async def get_tokens(self) -> OAuthToken | None:
132+
if self.tokens is not None and self.report_expired_on_load:
133+
return self.tokens.model_copy(update={"expires_in": 0})
117134
return self.tokens
118135

119136
async def set_tokens(self, tokens: OAuthToken) -> None:
@@ -182,6 +199,7 @@ def auth_settings(
182199
required_scopes: Sequence[str] = ("mcp",),
183200
valid_scopes: Sequence[str] | None = None,
184201
identity_assertion_enabled: bool = False,
202+
client_secret_expiry_seconds: int | None = None,
185203
) -> AuthSettings:
186204
"""Build `AuthSettings` for the co-hosted authorization + resource server.
187205
@@ -195,6 +213,9 @@ def auth_settings(
195213
`identity_assertion_enabled` advertises and accepts the SEP-990 ID-JAG grant (RFC 7523
196214
jwt-bearer); the provider must implement `exchange_identity_assertion` for the endpoint to
197215
issue tokens.
216+
217+
`client_secret_expiry_seconds` makes dynamic registration issue secrets that expire that many
218+
seconds after issuance (`client_secret_expires_at`), which the token endpoint then enforces.
198219
"""
199220
required = list(required_scopes)
200221
valid = list(valid_scopes) if valid_scopes is not None else required
@@ -203,7 +224,10 @@ def auth_settings(
203224
resource_server_url=AnyHttpUrl(f"{BASE_URL}/mcp"),
204225
required_scopes=required,
205226
client_registration_options=ClientRegistrationOptions(
206-
enabled=True, valid_scopes=valid, default_scopes=required
227+
enabled=True,
228+
valid_scopes=valid,
229+
default_scopes=required,
230+
client_secret_expiry_seconds=client_secret_expiry_seconds,
207231
),
208232
revocation_options=RevocationOptions(enabled=False),
209233
identity_assertion_enabled=identity_assertion_enabled,
@@ -273,6 +297,55 @@ def shim(
273297
return lambda app: shimmed_app(app, not_found=not_found, serve=serve)
274298

275299

300+
def path_prefixed_as_shim(prefix: str) -> AppShim:
301+
"""Build an `app_shim` that presents the co-hosted authorization server as living under `prefix`.
302+
303+
The SDK server mounts `/authorize`, `/token` and `/register` at the origin root whatever the
304+
issuer, so an authorization server whose endpoints sit under a path (a common hosted shape,
305+
e.g. `https://host/oauth2/v1/token`) cannot be configured natively. This shim serves
306+
protected-resource metadata naming `{BASE_URL}{prefix}` as the authorization server, serves
307+
that issuer's metadata at the RFC 8414 path-inserted well-known URL with every endpoint under
308+
the prefix, forwards `{prefix}/x` to the real `/x` route, and 404s the bare root endpoints and
309+
root metadata so a client that guesses origin-root paths fails the way it would against such
310+
a server. Pair with `InMemoryAuthorizationServerProvider(issuer=f"{BASE_URL}{prefix}")` so
311+
the RFC 9207 `iss` on the redirect matches.
312+
"""
313+
issuer = f"{BASE_URL}{prefix}"
314+
prm = ProtectedResourceMetadata(resource=AnyHttpUrl(f"{BASE_URL}/mcp"), authorization_servers=[AnyHttpUrl(issuer)])
315+
asm = OAuthMetadata(
316+
issuer=AnyHttpUrl(issuer),
317+
authorization_endpoint=AnyHttpUrl(f"{issuer}/authorize"),
318+
token_endpoint=AnyHttpUrl(f"{issuer}/token"),
319+
registration_endpoint=AnyHttpUrl(f"{issuer}/register"),
320+
scopes_supported=["mcp"],
321+
response_types_supported=["code"],
322+
grant_types_supported=["authorization_code", "refresh_token"],
323+
token_endpoint_auth_methods_supported=["client_secret_post", "client_secret_basic", "none"],
324+
code_challenge_methods_supported=["S256"],
325+
)
326+
327+
def factory(app: ASGIApp) -> ASGIApp:
328+
inner = shimmed_app(
329+
app,
330+
not_found=frozenset({"/token", "/authorize", "/register", "/.well-known/oauth-authorization-server"}),
331+
serve={
332+
"/.well-known/oauth-protected-resource/mcp": metadata_body(prm),
333+
f"/.well-known/oauth-authorization-server{prefix}": metadata_body(asm),
334+
},
335+
)
336+
337+
async def wrapped(scope: Scope, receive: Receive, send: Send) -> None:
338+
if scope["type"] == "http" and scope["path"].startswith(f"{prefix}/"):
339+
path = scope["path"][len(prefix) :]
340+
await app({**scope, "path": path, "raw_path": path.encode()}, receive, send)
341+
return
342+
await inner(scope, receive, send)
343+
344+
return wrapped
345+
346+
return factory
347+
348+
276349
@dataclass
277350
class _FirstChallenge:
278351
"""ASGI shim that answers the first request to a path with 401 + a given WWW-Authenticate.

tests/interaction/auth/_provider.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,25 @@ def mint_access_token(self, *, client_id: str, scopes: list[str], resource: str
103103
)
104104
return access
105105

106+
def expire_access_token(self, token: str) -> None:
107+
"""Move an issued access token's server-side expiry into the past so the bearer middleware 401s it.
108+
109+
Models time passing between two client processes: the token the first process stored is
110+
no longer accepted when the second presents it.
111+
"""
112+
self.access_tokens[token] = self.access_tokens[token].model_copy(update={"expires_at": int(time.time()) - 1})
113+
114+
def lapse_client_secret(self, client_id: str) -> None:
115+
"""Move a registered client's `client_secret_expires_at` into the past so the token endpoint rejects it.
116+
117+
The SDK's client authenticator answers `invalid_client` ("Client secret has expired") for
118+
every grant once this is set, which is how an authorization server that issues expiring
119+
registration secrets behaves after the window passes.
120+
"""
121+
self.clients[client_id] = self.clients[client_id].model_copy(
122+
update={"client_secret_expires_at": int(time.time()) - 1}
123+
)
124+
106125
async def get_client(self, client_id: str) -> OAuthClientInformationFull | None:
107126
return self.clients.get(client_id)
108127

0 commit comments

Comments
 (0)