OAuth client: refresh before re-authorizing, and discover before refreshing

docs/client/oauth-clients.md

@@ -51,6 +51,11 @@ The in-memory version above works. It also forgets everything when the process e
 !!! tip
     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.
+    The one case where the provider throws it away for you is a registration it made that has
+    stopped working: its `client_secret_expires_at` has passed, or the token endpoint answered
+    `invalid_client`. Then it registers again and overwrites the stored record. Credentials you
+    seeded into storage yourself are never replaced this way; an `invalid_client` for those
+    surfaces as an `OAuthTokenError`.
 
 ### The two handlers
 
@@ -81,7 +86,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.
 

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

@@ -114,6 +114,6 @@ async def authenticate_request(self, request: Request) -> OAuthClientInformation
                 raise AuthenticationError("Invalid client_secret")
 
             if client.client_secret_expires_at and client.client_secret_expires_at < int(time.time()):
-                raise AuthenticationError("Client secret has expired")  # pragma: no cover
+                raise AuthenticationError("Client secret has expired")
 
         return client

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 client_secret_lapsed, token_error_code
 from mcp.client.auth.utils import (
     build_oauth_authorization_server_metadata_discovery_urls,
     build_protected_resource_metadata_discovery_urls,
@@ -3253,3 +3254,90 @@ async def echo_callback() -> AuthorizationCodeResult:
         await auth_flow.asend(httpx2.Response(200, request=final_req))
     except StopAsyncIteration:
         pass
+
+
+@pytest.mark.parametrize(
+    ("method", "expires_at", "expected"),
+    [
+        pytest.param("client_secret_post", 999, True, id="post-secret-past"),
+        pytest.param("client_secret_basic", 999, True, id="basic-secret-past"),
+        pytest.param("client_secret_post", 1001, False, id="secret-still-live"),
+        pytest.param("client_secret_post", 0, False, id="zero-never-expires"),
+        pytest.param("client_secret_post", None, False, id="expiry-undeclared"),
+        pytest.param("none", 999, False, id="public-client-ignores-secret-expiry"),
+    ],
+)
+def test_client_secret_lapsed_only_for_secret_auth_with_a_past_nonzero_expiry(
+    method: str, expires_at: int | None, expected: bool
+) -> None:
+    """RFC 7591 §3.2.1: a stored secret is dead once `client_secret_expires_at` (non-zero) is in the past.
+
+    Only registrations that authenticate with the secret are affected; `0` and an absent field
+    both mean no expiry is known.
+    """
+    info = OAuthClientInformationFull(
+        client_id="c", client_secret="s", client_secret_expires_at=expires_at, token_endpoint_auth_method=method
+    )
+    assert client_secret_lapsed(info, now=1000) is expected
+
+
+def test_client_secret_lapsed_defaults_to_the_current_time() -> None:
+    """Without an explicit `now`, the wall clock decides."""
+    info = OAuthClientInformationFull(
+        client_id="c",
+        client_secret="s",
+        client_secret_expires_at=int(time.time()) - 60,
+        token_endpoint_auth_method="client_secret_post",
+    )
+    assert client_secret_lapsed(info) is True
+
+
+@pytest.mark.anyio
+@pytest.mark.parametrize(
+    ("status", "body", "expected"),
+    [
+        pytest.param(400, b'{"error":"invalid_grant"}', "invalid_grant", id="400-json-error"),
+        pytest.param(401, b'{"error":"invalid_client","error_description":"x"}', "invalid_client", id="401-json-error"),
+        pytest.param(404, b'{"error":"invalid_client"}', None, id="non-token-error-status"),
+        pytest.param(400, b"<html>bad gateway</html>", None, id="non-json-body"),
+        pytest.param(400, b'["invalid_client"]', None, id="json-but-not-an-object"),
+        pytest.param(400, b'{"error": 7}', None, id="error-member-not-a-string"),
+    ],
+)
+async def test_token_error_code_reads_the_rfc6749_error_member_from_400_and_401_bodies(
+    status: int, body: bytes, expected: str | None
+) -> None:
+    """RFC 6749 §5.2 puts token-endpoint errors on 400, or 401 for `invalid_client`; anything else carries no code."""
+    response = httpx2.Response(status, content=body, request=httpx2.Request("POST", "https://as.example/token"))
+    assert await token_error_code(response) == expected
+
+
+@pytest.mark.anyio
+async def test_initialize_derives_expiry_from_the_loaded_token(
+    oauth_provider: OAuthClientProvider, mock_storage: MockTokenStorage, valid_tokens: OAuthToken
+) -> None:
+    """A token loaded from storage gets an expiry derived from its `expires_in` instead of counting as valid forever."""
+    await mock_storage.set_tokens(valid_tokens)
+    before = time.time()
+
+    await oauth_provider._initialize()
+
+    assert oauth_provider.context.token_expiry_time is not None
+    assert oauth_provider.context.token_expiry_time >= before + 3600
+
+
+@pytest.mark.anyio
+async def test_initialize_keeps_an_expiry_the_application_already_set(
+    oauth_provider: OAuthClientProvider, mock_storage: MockTokenStorage, valid_tokens: OAuthToken
+) -> None:
+    """An application that records the real expiry and assigns `context.token_expiry_time` itself is not overridden.
+
+    Storages in the wild set it before the first request or from inside `get_tokens`; the value
+    derived from a persisted relative `expires_in` would be staler than theirs.
+    """
+    await mock_storage.set_tokens(valid_tokens)
+    oauth_provider.context.token_expiry_time = 12345.0
+
+    await oauth_provider._initialize()
+
+    assert oauth_provider.context.token_expiry_time == 12345.0

tests/interaction/_requirements.py

@@ -3783,22 +3783,16 @@ def __post_init__(self) -> None:
         note="OAuth is HTTP-only.",
     ),
     "client-auth:invalid-client-clears-all": Requirement(
-        source="sdk",
+        source="issue:#3256",
         behavior=(
-            "An invalid-client or unauthorized-client error during authorization invalidates all stored credentials."
+            "An invalid_client error from the token endpoint (refresh or code exchange) for a registration "
+            "the SDK obtained itself discards that registration and the tokens bound to it, and the flow "
+            "re-registers and continues once; for pre-registered credentials the error surfaces instead."
         ),
         transports=("streamable-http",),
-        note="OAuth is HTTP-only.",
-        divergence=Divergence(
-            note=(
-                "The token-response handlers do not parse the error body; an invalid_client or "
-                "unauthorized_client response leaves stored client_info untouched. The TypeScript SDK "
-                "clears it."
-            ),
-        ),
-        deferred=(
-            "Not implemented in the SDK: no token-response path inspects the error code to decide "
-            "whether to clear client_info."
+        note=(
+            "OAuth is HTTP-only. Registrations the SDK minted carry the SEP-2352 issuer stamp; that is the "
+            "provenance test. unauthorized_client is not treated the same way (the TypeScript SDK does)."
         ),
     ),
     "client-auth:invalid-grant-clears-tokens": Requirement(
@@ -3883,6 +3877,36 @@ 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 attempted before the first request of a process (the loaded token is known to be "
+            "expired) performs protected-resource and authorization-server metadata discovery first 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:registration:secret-expiry": Requirement(
+        source="issue:#3256",
+        behavior=(
+            "A stored dynamically registered client whose client_secret_expires_at (RFC 7591) has passed is "
+            "treated as absent: the flow registers afresh before authorizing instead of presenting the dead "
+            "secret at the token endpoint."
+        ),
+        transports=("streamable-http",),
+        note="OAuth is HTTP-only. 0 means the secret never expires; only secret-based auth methods are affected.",
+    ),
     "client-auth:resource-parameter": Requirement(
         source=f"{SPEC_BASE_URL}/basic/authorization#resource-parameter-implementation",
         behavior=(

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
@@ -107,13 +114,23 @@
     Tests pre-seed `client_info` (via the constructor or by assignment) to drive the
     pre-registered path, and read both attributes after the flow to assert what the SDK
     persisted.
+
+    `report_expired_on_load`: `get_tokens` returns the held token with `expires_in=0`, standing
+    in for an application storage that records an absolute expiry and reports the remaining
+    lifetime on load, so a freshly constructed provider knows the access token is already dead
+    before it sends anything.
     """
 
-    def __init__(self, *, client_info: OAuthClientInformationFull | None = None) -> None:
+    def __init__(
+        self, *, client_info: OAuthClientInformationFull | None = None, report_expired_on_load: bool = False
+    ) -> None:
         self.tokens: OAuthToken | None = None
         self.client_info: OAuthClientInformationFull | None = client_info
+        self.report_expired_on_load = report_expired_on_load
 
     async def get_tokens(self) -> OAuthToken | None:
+        if self.tokens is not None and self.report_expired_on_load:
+            return self.tokens.model_copy(update={"expires_in": 0})
         return self.tokens
 
     async def set_tokens(self, tokens: OAuthToken) -> None:
@@ -182,6 +199,7 @@
     required_scopes: Sequence[str] = ("mcp",),
     valid_scopes: Sequence[str] | None = None,
     identity_assertion_enabled: bool = False,
+    client_secret_expiry_seconds: int | None = None,
 ) -> AuthSettings:
     """Build `AuthSettings` for the co-hosted authorization + resource server.
 
@@ -195,6 +213,9 @@
     `identity_assertion_enabled` advertises and accepts the SEP-990 ID-JAG grant (RFC 7523
     jwt-bearer); the provider must implement `exchange_identity_assertion` for the endpoint to
     issue tokens.
+
+    `client_secret_expiry_seconds` makes dynamic registration issue secrets that expire that many
+    seconds after issuance (`client_secret_expires_at`), which the token endpoint then enforces.
     """
     required = list(required_scopes)
     valid = list(valid_scopes) if valid_scopes is not None else required
@@ -203,7 +224,10 @@
         resource_server_url=AnyHttpUrl(f"{BASE_URL}/mcp"),
         required_scopes=required,
         client_registration_options=ClientRegistrationOptions(
-            enabled=True, valid_scopes=valid, default_scopes=required
+            enabled=True,
+            valid_scopes=valid,
+            default_scopes=required,
+            client_secret_expiry_seconds=client_secret_expiry_seconds,
         ),
         revocation_options=RevocationOptions(enabled=False),
         identity_assertion_enabled=identity_assertion_enabled,
@@ -273,6 +297,55 @@
     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 authorization server whose endpoints sit under a path (a common hosted shape,
+    e.g. `https://host/oauth2/v1/token`) cannot be configured natively. This shim serves
+    protected-resource metadata naming `{BASE_URL}{prefix}` as the authorization server, 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` route, and 404s the bare root endpoints and
+    root metadata so a client that guesses origin-root paths fails the way it would against such
+    a server. Pair with `InMemoryAuthorizationServerProvider(issuer=f"{BASE_URL}{prefix}")` so
+    the RFC 9207 `iss` on the redirect 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.

tests/interaction/auth/_provider.py

@@ -103,6 +103,25 @@ 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.
+
+        Models time passing between two client processes: the token the first process stored is
+        no longer accepted when the second presents it.
+        """
+        self.access_tokens[token] = self.access_tokens[token].model_copy(update={"expires_at": int(time.time()) - 1})
+
+    def lapse_client_secret(self, client_id: str) -> None:
+        """Move a registered client's `client_secret_expires_at` into the past so the token endpoint rejects it.
+
+        The SDK's client authenticator answers `invalid_client` ("Client secret has expired") for
+        every grant once this is set, which is how an authorization server that issues expiring
+        registration secrets behaves after the window passes.
+        """
+        self.clients[client_id] = self.clients[client_id].model_copy(
+            update={"client_secret_expires_at": int(time.time()) - 1}
+        )
+
     async def get_client(self, client_id: str) -> OAuthClientInformationFull | None:
         return self.clients.get(client_id)