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

docs/client/oauth-clients.md

@@ -81,7 +81,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/client/auth/oauth2.py

@@ -374,7 +374,7 @@
         if self.context.client_metadata.redirect_uris is None:
             raise OAuthFlowError("No redirect URIs provided for authorization code grant")  # pragma: no cover
         if not self.context.redirect_handler:
-            raise OAuthFlowError("No redirect handler provided for authorization code grant")  # pragma: no cover
+            raise OAuthFlowError("No redirect handler provided for authorization code grant")
         if not self.context.callback_handler:
             raise OAuthFlowError("No callback handler provided for authorization code grant")  # pragma: no cover
 
@@ -521,6 +521,8 @@
         if response.status_code != 200:
             logger.warning(f"Token refresh failed: {response.status_code}")
             self.context.clear_tokens()
+            # Re-read storage on the next request: the failure may have been transient.
+            self._initialized = False
             return False
 
         try:
@@ -545,6 +547,7 @@
         except ValidationError:  # pragma: no cover
             logger.exception("Invalid refresh response")
             self.context.clear_tokens()
+            self._initialized = False
             return False
 
     async def _initialize(self) -> None:
@@ -586,14 +589,15 @@
             # Capture protocol version from request headers
             self.context.protocol_version = request.headers.get(MCP_PROTOCOL_VERSION_HEADER)
 
-            if not self.context.is_token_valid() and self.context.can_refresh_token():
-                # Try to refresh token
-                refresh_request = await self._refresh_token()
-                refresh_response = yield refresh_request
-
-                if not await self._handle_refresh_response(refresh_response):
-                    # Refresh failed, need full re-authentication
-                    self._initialized = False
+            # Refresh ahead of the request only when the token endpoint is already known; on a cold
+            # start the request goes out and the 401 branch discovers, then refreshes.
+            if (
+                not self.context.is_token_valid()
+                and self.context.can_refresh_token()
+                and self.context.oauth_metadata is not None
+            ):
+                refresh_response = yield await self._refresh_token()
+                await self._handle_refresh_response(refresh_response)
 
             if self.context.is_token_valid():
                 self._add_auth_header(request)
@@ -648,6 +652,18 @@
                         # Any cached AS metadata is for the old server; drop it so a failed
                         # rediscovery cannot leak the old registration/token endpoints into Step 4.
                         self.context.oauth_metadata = None
+                    elif (
+                        self.context.client_info is not None
+                        and self.context.client_info.client_id == self.context.client_metadata_url
+                        and self.context.auth_server_url is not None
+                        and self.context.client_info.issuer not in (None, self.context.auth_server_url)
+                    ):
+                        # A CIMD client_id is portable across authorization servers; the tokens issued
+                        # under it and the cached metadata are not. Keep the record, re-stamped.
+                        self.context.clear_tokens()
+                        self.context.oauth_metadata = None
+                        self.context.client_info.issuer = self.context.auth_server_url
+                        await self.context.storage.set_client_info(self.context.client_info)
 
                     asm_discovery_urls = build_oauth_authorization_server_metadata_discovery_urls(
                         self.context.auth_server_url, self.context.server_url
@@ -741,10 +757,18 @@
                                 client_information.issuer = discovered_issuer
                             self.context.client_info = client_information
                             await self.context.storage.set_client_info(client_information)
-
-                    # Step 5: Perform authorization and complete token exchange
-                    token_response = yield await self._perform_authorization()
-                    await self._handle_token_response(token_response)
+                            # Held tokens belong to a previous client and cannot be refreshed by this one.
+                            self.context.clear_tokens()
+
+                    # Step 5: Refresh with the stored refresh token first (RFC 6749 §6); run the full
+                    # authorization only when there is none or the server rejects it.
+                    refreshed = False
+                    if self.context.can_refresh_token():
+                        refresh_response = yield await self._refresh_token()
+                        refreshed = await self._handle_refresh_response(refresh_response)
+                    if not refreshed:
+                        token_response = yield await self._perform_authorization()
+                        await self._handle_token_response(token_response)
                 except Exception:
                     logger.exception("OAuth flow error")
                     raise

tests/client/test_auth.py

@@ -3253,3 +3253,81 @@ async def echo_callback() -> AuthorizationCodeResult:
         await auth_flow.asend(httpx2.Response(200, request=final_req))
     except StopAsyncIteration:
         pass
+
+
+@pytest.mark.anyio
+async def test_expired_token_is_not_refreshed_ahead_of_the_request_before_metadata_is_discovered(
+    oauth_provider: OAuthClientProvider, valid_tokens: OAuthToken
+) -> None:
+    """With no authorization-server metadata yet, an expired token is not refreshed at a guessed endpoint.
+
+    The request goes out unauthenticated instead, so the 401 branch discovers the real token
+    endpoint before the refresh token is presented anywhere (#3240).
+    """
+    oauth_provider.context.current_tokens = valid_tokens
+    oauth_provider.context.token_expiry_time = time.time() - 60
+    oauth_provider.context.client_info = OAuthClientInformationFull(client_id="c", redirect_uris=None)
+    oauth_provider.context.oauth_metadata = None
+    oauth_provider._initialized = True
+
+    request = httpx2.Request("POST", "https://api.example.com/v1/mcp")
+    auth_flow = oauth_provider.async_auth_flow(request)
+    first = await auth_flow.__anext__()
+
+    assert first is request
+    assert "Authorization" not in first.headers
+
+    with pytest.raises(StopAsyncIteration):
+        await auth_flow.asend(httpx2.Response(200, request=request))
+
+
+@pytest.mark.anyio
+async def test_cimd_record_is_restamped_and_its_tokens_and_cached_metadata_dropped_when_prm_names_a_new_issuer(
+    client_metadata: OAuthClientMetadata, mock_storage: MockTokenStorage, valid_tokens: OAuthToken
+) -> None:
+    """SEP-2352 for CIMD: the URL client_id survives an authorization-server change, nothing else does.
+
+    A long-lived provider holds a CIMD record stamped with the old issuer, tokens minted there, and
+    the old server's cached metadata. As soon as PRM names a different issuer, the tokens and the
+    cached metadata are dropped and the record is re-stamped and persisted, so a failed
+    rediscovery cannot leave the old endpoints in play and no refresh reaches the new server.
+    """
+    cimd_url = "https://client.example.com/.well-known/mcp-client"
+    provider = OAuthClientProvider(
+        server_url="https://api.example.com/v1/mcp",
+        client_metadata=client_metadata,
+        storage=mock_storage,
+        client_metadata_url=cimd_url,
+    )
+    provider.context.client_info = OAuthClientInformationFull(
+        client_id=cimd_url, token_endpoint_auth_method="none", issuer="https://old-as.example.com"
+    )
+    provider.context.current_tokens = valid_tokens
+    provider.context.token_expiry_time = time.time() + 1800
+    provider.context.oauth_metadata = OAuthMetadata(
+        issuer=AnyHttpUrl("https://old-as.example.com"),
+        authorization_endpoint=AnyHttpUrl("https://old-as.example.com/authorize"),
+        token_endpoint=AnyHttpUrl("https://old-as.example.com/token"),
+    )
+    provider._initialized = True
+
+    auth_flow = provider.async_auth_flow(httpx2.Request("GET", "https://api.example.com/v1/mcp"))
+    request = await auth_flow.__anext__()
+    prm_req = await auth_flow.asend(httpx2.Response(401, request=request))
+    prm_response = httpx2.Response(
+        200,
+        content=b'{"resource": "https://api.example.com/v1/mcp", "authorization_servers": ["https://new-as.example.com"]}',
+        request=prm_req,
+    )
+    asm_req = await auth_flow.asend(prm_response)
+
+    assert str(asm_req.url) == "https://new-as.example.com/.well-known/oauth-authorization-server"
+    assert provider.context.current_tokens is None
+    assert provider.context.oauth_metadata is None
+    assert provider.context.client_info is not None
+    assert (provider.context.client_info.client_id, provider.context.client_info.issuer) == (
+        cimd_url,
+        "https://new-as.example.com",
+    )
+    assert mock_storage._client_info is provider.context.client_info
+    await auth_flow.aclose()

tests/interaction/_requirements.py

@@ -3883,6 +3883,29 @@ 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 in a process that has not yet discovered the authorization server happens only after "
+            "protected-resource and authorization-server metadata discovery and posts to the token endpoint "
+            "that metadata advertises."
+        ),
+        transports=("streamable-http",),
+        note=(
+            "OAuth is HTTP-only. When discovery yields no AS metadata at all, the 2025-03-26 origin-derived "
+            "fallback endpoint is still used, as it is for the authorization itself."
+        ),
+    ),
     "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
@@ -273,6 +280,53 @@ def shim(
     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 AS whose endpoints sit under a path cannot be configured natively. This serves
+    PRM naming `{BASE_URL}{prefix}` as the AS, 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`, and 404s the bare root endpoints and root metadata so a client guessing origin-root
+    paths fails as it would against such a server. Pair with
+    `InMemoryAuthorizationServerProvider(issuer=f"{BASE_URL}{prefix}")` so the redirect `iss` 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,10 @@ 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."""
+        self.access_tokens[token] = self.access_tokens[token].model_copy(update={"expires_at": int(time.time()) - 1})
+
     async def get_client(self, client_id: str) -> OAuthClientInformationFull | None:
         return self.clients.get(client_id)