fix(client/auth): make hint-less eager discovery best-effort, never destructive

Address review findings: without a WWW-Authenticate resource_metadata
hint the eager probes are unanchored, so a co-hosted origin can serve
another resource's documents. Treat a resource-mismatched PRM as failed
discovery instead of raising out of the auth flow; on a SEP-2352
binding mismatch skip the refresh and discard the unanchored discovery
results (including rejected ASM metadata) but keep the credentials for
the anchored 401 path to judge. Run the probes only once per context so
servers publishing no metadata are not re-probed on every in-process
refresh, and finalize the inner refresh generator with aclosing().

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CjbXueCDdFNJK6imejCXgM

Author: claude

src/mcp/client/auth/oauth2.py

@@ -10,6 +10,7 @@
 import string
 import time
 from collections.abc import AsyncGenerator, Awaitable, Callable
+from contextlib import aclosing
 from dataclasses import dataclass, field
 from typing import Any, Protocol, get_args
 from urllib.parse import quote, urlencode, urljoin, urlparse
@@ -160,6 +161,9 @@ class OAuthContext:
     oauth_metadata: OAuthMetadata | None = None
     auth_server_url: str | None = None
     protocol_version: str | None = None
+    # Whether the eager (pre-401) refresh already ran its blind discovery probes, so a
+    # server that publishes no metadata is not re-probed on every in-process refresh.
+    eager_discovery_attempted: bool = False
 
     # Client registration
     client_info: OAuthClientInformationFull | None = None
@@ -584,43 +588,63 @@ async def _refresh_with_discovery(self) -> AsyncGenerator[httpx2.Request, httpx2
         token reused before any 401) that metadata has not been discovered yet, so
         ``_refresh_token`` would fall back to ``{origin}/token`` — dropping any issuer
         path and 404ing on servers whose token endpoint lives elsewhere. Discovery runs
-        first, applying the same SEP-2352 issuer-binding checks as the 401 path so stored
-        credentials are never sent to an authorization server they are not bound to: on a
-        binding mismatch the credentials and tokens are dropped and the refresh is
-        skipped, letting the subsequent 401 flow re-register against the new server.
-        Yields the discovery and refresh requests so they run through the outer httpx
-        auth flow rather than a side-channel client.
+        first so the refresh targets the discovered token endpoint.
+
+        Unlike the 401 path, this discovery is unanchored: there is no WWW-Authenticate
+        ``resource_metadata`` hint, only blind well-known probes, so a co-hosted origin
+        can legitimately serve some *other* resource's documents. Results are therefore
+        treated as best-effort, never authoritative: a resource-mismatched PRM counts as
+        a failed discovery rather than an error, and a SEP-2352 issuer-binding mismatch
+        skips the eager refresh (so stored credentials are never presented to an
+        unvalidated authorization server) while leaving the credentials themselves for
+        the anchored 401 path to judge — that path re-discovers with the server's hint
+        and drops/re-registers only on a confirmed change. Servers publishing no
+        metadata at all keep the pre-existing ``{origin}/token`` fallback, and the
+        probes run only once per context (``eager_discovery_attempted``). Yields the
+        discovery and refresh requests so they run through the outer httpx auth flow
+        rather than a side-channel client.
         """
-        if self.context.oauth_metadata is None:
+        if self.context.oauth_metadata is None and not self.context.eager_discovery_attempted:
+            self.context.eager_discovery_attempted = True
+
             # Step 1: protected resource metadata -> authorization server URL (SEP-985).
-            # Best-effort: a legacy server without PRM falls through to the origin
-            # well-known fallback in the ASM step below. There is no 401 response at
-            # this point, so no WWW-Authenticate resource_metadata hint is available.
+            # Best-effort: a PRM that fails resource validation is some other co-hosted
+            # resource's document, not ours — skip it; a legacy server without PRM falls
+            # through to the origin well-known fallback in the ASM step below.
             for url in build_protected_resource_metadata_discovery_urls(None, self.context.server_url):
                 prm = await handle_protected_resource_response((yield create_oauth_metadata_request(url)))
                 if prm:
-                    # Validate PRM resource matches server URL (RFC 8707)
-                    await self._validate_resource_match(prm)
+                    try:
+                        # Validate PRM resource matches server URL (RFC 8707)
+                        await self._validate_resource_match(prm)
+                    except OAuthFlowError:
+                        logger.debug(f"Ignoring protected resource metadata for a different resource: {url}")
+                        continue
                     self.context.protected_resource_metadata = prm
                     self.context.auth_server_url = str(prm.authorization_servers[0])
                     break
                 else:
                     logger.debug(f"Protected resource metadata discovery failed: {url}")
 
             # SEP-2352: stored credentials are bound to the issuer that registered them.
-            # If the authorization server changed, drop them (and the old tokens) and skip
-            # the refresh so the 401 flow re-registers instead of presenting another
-            # server's credentials to the newly discovered one.
+            # A mismatch here may mean the AS changed — or merely that the blind probe
+            # found a different co-hosted resource's PRM. Skip the eager refresh so the
+            # credentials are never presented to an unvalidated AS, discard the
+            # unanchored discovery results, and let the 401 path decide with the
+            # server's own hint whether to drop the credentials and re-register.
             if (
                 self.context.client_info is not None
                 and self.context.auth_server_url is not None
                 and not credentials_match_issuer(
                     self.context.client_info, self.context.auth_server_url, self.context.client_metadata_url
                 )
             ):
-                logger.debug("Authorization server changed; discarding bound credentials and skipping refresh")
-                self.context.client_info = None
-                self.context.clear_tokens()
+                logger.debug(
+                    "Eagerly discovered authorization server does not match stored credential binding; "
+                    "skipping refresh and deferring to 401 discovery"
+                )
+                self.context.protected_resource_metadata = None
+                self.context.auth_server_url = None
                 return
 
             # Step 2: authorization server metadata -> the token endpoint (with fallback
@@ -641,8 +665,10 @@ async def _refresh_with_discovery(self) -> AsyncGenerator[httpx2.Request, httpx2
                     logger.debug(f"OAuth metadata discovery failed: {url}")
 
             # SEP-2352: on the legacy no-PRM path the issuer is only known after ASM
-            # discovery, so re-evaluate the binding here using the discovered metadata
-            # issuer (mirroring the 401 path's post-ASM check).
+            # discovery, so re-evaluate the binding here (mirroring the 401 path's
+            # post-ASM check). As above, skip the refresh and discard the unanchored
+            # metadata rather than acting on it — keeping it would let a rejected
+            # issuer's endpoints leak into a later 401 flow's registration step.
             if (
                 self.context.client_info is not None
                 and self.context.auth_server_url is None
@@ -653,9 +679,11 @@ async def _refresh_with_discovery(self) -> AsyncGenerator[httpx2.Request, httpx2
                     self.context.client_metadata_url,
                 )
             ):
-                logger.debug("Authorization server changed; discarding bound credentials and skipping refresh")
-                self.context.client_info = None
-                self.context.clear_tokens()
+                logger.debug(
+                    "Eagerly discovered authorization server does not match stored credential binding; "
+                    "skipping refresh and deferring to 401 discovery"
+                )
+                self.context.oauth_metadata = None
                 return
 
         refresh_response = yield await self._refresh_token()
@@ -675,15 +703,16 @@ async def async_auth_flow(self, request: httpx2.Request) -> AsyncGenerator[httpx
             if not self.context.is_token_valid() and self.context.can_refresh_token():
                 # Refresh the token, discovering authorization-server metadata first on a
                 # cold start (see _refresh_with_discovery). Driven inline so its requests
-                # run through this httpx auth flow, not a side-channel client.
-                refresh_flow = self._refresh_with_discovery()
-                refresh_request = await refresh_flow.__anext__()
-                while True:
-                    refresh_response = yield refresh_request
-                    try:
-                        refresh_request = await refresh_flow.asend(refresh_response)
-                    except StopAsyncIteration:
-                        break
+                # run through this httpx auth flow, not a side-channel client; aclosing
+                # finalizes the inner generator when httpx closes this flow mid-refresh.
+                async with aclosing(self._refresh_with_discovery()) as refresh_flow:
+                    refresh_request = await refresh_flow.__anext__()
+                    while True:
+                        refresh_response = yield refresh_request
+                        try:
+                            refresh_request = await refresh_flow.asend(refresh_response)
+                        except StopAsyncIteration:
+                            break
 
             if self.context.is_token_valid():
                 self._add_auth_header(request)

tests/client/test_auth.py

@@ -3417,10 +3417,11 @@ async def test_eager_refresh_skips_refresh_when_credentials_bound_to_different_i
 ):
     """SEP-2352: a cold-start refresh never sends credentials bound to another issuer.
 
-    When PRM discovery reveals an authorization server different from the one the stored
-    client credentials are bound to, the credentials and tokens are dropped and the
-    refresh is skipped, so the subsequent 401 flow re-registers against the new server
-    — mirroring the issuer-binding check on the 401 discovery path.
+    When blind PRM discovery reveals an authorization server different from the one the
+    stored client credentials are bound to, the eager refresh is skipped and the
+    unanchored discovery results are discarded — but the credentials themselves are
+    kept: without a WWW-Authenticate hint the probe may have found a different
+    co-hosted resource's PRM, so dropping is deferred to the anchored 401 path.
     """
     oauth_provider.context.current_tokens = valid_tokens
     oauth_provider.context.token_expiry_time = time.time() - 100  # expired
@@ -3447,8 +3448,12 @@ async def test_eager_refresh_skips_refresh_when_credentials_bound_to_different_i
     api_request = await auth_flow.asend(prm_response)
     assert str(api_request.url) == "https://api.example.com/v1/mcp"
     assert "Authorization" not in api_request.headers
-    assert oauth_provider.context.client_info is None
-    assert oauth_provider.context.current_tokens is None
+    # Credentials and tokens are kept for the anchored 401 path to judge; the
+    # unanchored discovery results are discarded.
+    assert oauth_provider.context.client_info is not None
+    assert oauth_provider.context.current_tokens is not None
+    assert oauth_provider.context.protected_resource_metadata is None
+    assert oauth_provider.context.auth_server_url is None
     await auth_flow.aclose()
 
 
@@ -3459,8 +3464,9 @@ async def test_eager_refresh_legacy_path_rechecks_issuer_binding_after_asm(
     """SEP-2352 on the legacy no-PRM path: the binding is checked against the ASM issuer.
 
     PRM discovery fails so the issuer is only known once origin-fallback ASM discovery
-    succeeds; credentials bound to a different issuer are then dropped and the refresh is
-    skipped, exactly as on the 401 path's post-ASM re-evaluation.
+    succeeds; on a mismatch the refresh is skipped and the rejected metadata is
+    discarded (keeping it could leak the rejected issuer's endpoints into a later 401
+    flow's registration step), while the credentials are left for the 401 path to judge.
     """
     oauth_provider.context.current_tokens = valid_tokens
     oauth_provider.context.token_expiry_time = time.time() - 100  # expired
@@ -3494,10 +3500,101 @@ async def test_eager_refresh_legacy_path_rechecks_issuer_binding_after_asm(
     api_request = await auth_flow.asend(asm_response)
     assert str(api_request.url) == "https://api.example.com/v1/mcp"
     assert "Authorization" not in api_request.headers
-    assert oauth_provider.context.client_info is None
-    assert oauth_provider.context.current_tokens is None
-    # The just-discovered metadata is for the current server and is kept for the 401 flow.
-    assert oauth_provider.context.oauth_metadata is not None
+    # Credentials and tokens are kept for the anchored 401 path to judge; the metadata
+    # whose issuer failed the binding check is discarded, mirroring the 401 path's
+    # defensive clear.
+    assert oauth_provider.context.client_info is not None
+    assert oauth_provider.context.current_tokens is not None
+    assert oauth_provider.context.oauth_metadata is None
+    await auth_flow.aclose()
+
+
+@pytest.mark.anyio
+async def test_eager_refresh_treats_foreign_prm_as_failed_discovery(
+    oauth_provider: OAuthClientProvider, valid_tokens: OAuthToken
+):
+    """A blind well-known probe returning some other co-hosted resource's PRM is skipped.
+
+    Without a WWW-Authenticate hint, a resource-mismatched PRM is not an error (the 401
+    path's authoritative semantics) but simply not our document: discovery falls through
+    to the next URL and ultimately to the legacy ``{origin}/token`` refresh, instead of
+    raising out of the auth flow before the original request is ever sent.
+    """
+    oauth_provider.context.current_tokens = valid_tokens
+    oauth_provider.context.token_expiry_time = time.time() - 100  # expired
+    oauth_provider.context.client_info = OAuthClientInformationFull(
+        client_id="test_client",
+        redirect_uris=[AnyUrl("http://localhost:3030/callback")],
+        token_endpoint_auth_method="none",
+    )
+    oauth_provider._initialized = True
+
+    auth_flow = oauth_provider.async_auth_flow(httpx2.Request("GET", "https://api.example.com/v1/mcp"))
+
+    # Path-based well-known serves a *different* co-hosted resource's PRM: skipped.
+    prm_request = await auth_flow.__anext__()
+    foreign_prm = httpx2.Response(
+        200,
+        content=(
+            b'{"resource": "https://api.example.com/other-api", '
+            b'"authorization_servers": ["https://elsewhere.example.com"]}'
+        ),
+        request=prm_request,
+    )
+    prm_request = await auth_flow.asend(foreign_prm)
+    assert str(prm_request.url) == "https://api.example.com/.well-known/oauth-protected-resource"
+
+    # Root well-known 404s; legacy origin ASM fallback 404s; refresh uses {origin}/token.
+    asm_request = await auth_flow.asend(httpx2.Response(404, request=prm_request))
+    assert str(asm_request.url) == "https://api.example.com/.well-known/oauth-authorization-server"
+    refresh_request = await auth_flow.asend(httpx2.Response(404, request=asm_request))
+    assert refresh_request.method == "POST"
+    assert str(refresh_request.url) == "https://api.example.com/token"
+    assert oauth_provider.context.protected_resource_metadata is None
+    await auth_flow.aclose()
+
+
+@pytest.mark.anyio
+async def test_eager_refresh_probes_discovery_only_once_per_context(
+    oauth_provider: OAuthClientProvider, valid_tokens: OAuthToken
+):
+    """Against a server publishing no metadata, only the first refresh runs the probes.
+
+    Subsequent in-process refreshes skip straight to the ``{origin}/token`` fallback
+    (the pre-discovery behavior) instead of re-issuing the failed discovery requests on
+    every token expiry.
+    """
+    oauth_provider.context.current_tokens = valid_tokens
+    oauth_provider.context.token_expiry_time = time.time() - 100  # expired
+    oauth_provider.context.client_info = OAuthClientInformationFull(
+        client_id="test_client",
+        redirect_uris=[AnyUrl("http://localhost:3030/callback")],
+        token_endpoint_auth_method="none",
+    )
+    oauth_provider._initialized = True
+
+    # First refresh: probes (2x PRM, 1x legacy ASM) then the fallback refresh, succeeding.
+    auth_flow = oauth_provider.async_auth_flow(httpx2.Request("GET", "https://api.example.com/v1/mcp"))
+    request = await auth_flow.__anext__()
+    request = await auth_flow.asend(httpx2.Response(404, request=request))
+    request = await auth_flow.asend(httpx2.Response(404, request=request))
+    refresh_request = await auth_flow.asend(httpx2.Response(404, request=request))
+    assert str(refresh_request.url) == "https://api.example.com/token"
+    refresh_response = httpx2.Response(
+        200,
+        json={"access_token": "refreshed_token", "token_type": "Bearer", "expires_in": 3600},
+        request=refresh_request,
+    )
+    api_request = await auth_flow.asend(refresh_response)
+    assert api_request.headers["Authorization"] == "Bearer refreshed_token"
+    await auth_flow.aclose()
+
+    # Second refresh (token expired again): no probes, straight to the fallback.
+    oauth_provider.context.token_expiry_time = time.time() - 100
+    auth_flow = oauth_provider.async_auth_flow(httpx2.Request("GET", "https://api.example.com/v1/mcp"))
+    refresh_request = await auth_flow.__anext__()
+    assert refresh_request.method == "POST"
+    assert str(refresh_request.url) == "https://api.example.com/token"
     await auth_flow.aclose()