fix(client/auth): non-fatal SEP-2468 check on eager path; flag discovery only on completion

Address second-round review findings: an issuer-mismatched ASM from a
blind eager probe is skipped as failed discovery (falling through to
the {origin}/token fallback) instead of raising out of the auth flow
before the original request is sent; and eager_discovery_attempted is
now set only when the probe sequence completes, so a probe interrupted
by a transport failure is retried on the next refresh rather than
permanently recorded as done.

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

@@ -593,8 +593,9 @@ async def _refresh_with_discovery(self) -> AsyncGenerator[httpx2.Request, httpx2
         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
+        treated as best-effort, never authoritative: a resource-mismatched PRM or an
+        issuer-mismatched ASM 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
@@ -605,8 +606,6 @@ async def _refresh_with_discovery(self) -> AsyncGenerator[httpx2.Request, httpx2
         rather than a side-channel client.
         """
         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 PRM that fails resource validation is some other co-hosted
             # resource's document, not ours — skip it; a legacy server without PRM falls
@@ -645,6 +644,7 @@ async def _refresh_with_discovery(self) -> AsyncGenerator[httpx2.Request, httpx2
                 )
                 self.context.protected_resource_metadata = None
                 self.context.auth_server_url = None
+                self.context.eager_discovery_attempted = True
                 return
 
             # Step 2: authorization server metadata -> the token endpoint (with fallback
@@ -656,9 +656,13 @@ async def _refresh_with_discovery(self) -> AsyncGenerator[httpx2.Request, httpx2
                 if not ok:
                     break
                 if asm:
-                    # SEP-2468: metadata issuer must match the discovery issuer
                     if self.context.auth_server_url is not None:
-                        validate_metadata_issuer(asm, self.context.auth_server_url)
+                        try:
+                            # SEP-2468: metadata issuer must match the discovery issuer
+                            validate_metadata_issuer(asm, self.context.auth_server_url)
+                        except OAuthFlowError:
+                            logger.debug(f"Ignoring authorization server metadata with mismatched issuer: {url}")
+                            continue
                     self.context.oauth_metadata = asm
                     break
                 else:
@@ -684,8 +688,14 @@ async def _refresh_with_discovery(self) -> AsyncGenerator[httpx2.Request, httpx2
                     "skipping refresh and deferring to 401 discovery"
                 )
                 self.context.oauth_metadata = None
+                self.context.eager_discovery_attempted = True
                 return
 
+            # Mark completion only now: an interrupted probe sequence (the transport
+            # failing mid-discovery closes this generator) is retried on the next
+            # refresh instead of being recorded as done.
+            self.context.eager_discovery_attempted = True
+
         refresh_response = yield await self._refresh_token()
         if not await self._handle_refresh_response(refresh_response):
             # Refresh failed, need full re-authentication

tests/client/test_auth.py

@@ -3625,3 +3625,91 @@ async def test_eager_refresh_skips_discovery_when_metadata_already_known(
     assert refresh_request.method == "POST"
     assert str(refresh_request.url) == "https://auth.example.com/oauth2/api/v1/token"
     await auth_flow.aclose()
+
+
+@pytest.mark.anyio
+async def test_eager_refresh_treats_issuer_mismatched_asm_as_failed_discovery(
+    oauth_provider: OAuthClientProvider, valid_tokens: OAuthToken
+):
+    """An eagerly probed ASM whose issuer fails SEP-2468 validation is skipped, not fatal.
+
+    On the hint-less path a mismatched issuer cannot brick the flow: the document is
+    ignored, the remaining fallback URLs are tried, and the refresh falls through to
+    ``{origin}/token`` — the anchored 401 path still applies the authoritative check.
+    """
+    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"))
+
+    # PRM discovery succeeds and points at auth.example.com.
+    prm_request = await auth_flow.__anext__()
+    prm_response = httpx2.Response(
+        200,
+        content=(
+            b'{"resource": "https://api.example.com/v1/mcp", "authorization_servers": ["https://auth.example.com"]}'
+        ),
+        request=prm_request,
+    )
+
+    # First ASM URL answers with a mismatched issuer (SEP-2468): skipped, next URL tried.
+    asm_request = await auth_flow.asend(prm_response)
+    assert str(asm_request.url) == "https://auth.example.com/.well-known/oauth-authorization-server"
+    mismatched_asm = httpx2.Response(
+        200,
+        content=(
+            b'{"issuer": "https://internal.example.com", '
+            b'"authorization_endpoint": "https://internal.example.com/authorize", '
+            b'"token_endpoint": "https://internal.example.com/token"}'
+        ),
+        request=asm_request,
+    )
+    asm_request = await auth_flow.asend(mismatched_asm)
+    assert str(asm_request.url) == "https://auth.example.com/.well-known/openid-configuration"
+
+    # The fallback URL 404s; the refresh falls through to {origin}/token, no raise.
+    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.oauth_metadata is None
+    await auth_flow.aclose()
+
+
+@pytest.mark.anyio
+async def test_eager_discovery_interrupted_mid_probe_is_retried_on_the_next_refresh(
+    oauth_provider: OAuthClientProvider, valid_tokens: OAuthToken
+):
+    """An aborted probe sequence is not recorded as a completed discovery attempt.
+
+    httpx acloses the auth flow when a probe fails at the transport level; the
+    completion flag must stay unset so the next refresh retries discovery instead of
+    permanently falling back to ``{origin}/token`` against a server whose token
+    endpoint lives elsewhere.
+    """
+    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 attempt: the transport dies during the first probe; httpx acloses the flow.
+    auth_flow = oauth_provider.async_auth_flow(httpx2.Request("GET", "https://api.example.com/v1/mcp"))
+    first_probe = await auth_flow.__anext__()
+    assert "oauth-protected-resource" in str(first_probe.url)
+    await auth_flow.aclose()
+    assert not oauth_provider.context.eager_discovery_attempted
+
+    # Next refresh retries discovery from the start rather than skipping to the fallback.
+    auth_flow = oauth_provider.async_auth_flow(httpx2.Request("GET", "https://api.example.com/v1/mcp"))
+    retried_probe = await auth_flow.__anext__()
+    assert str(retried_probe.url) == "https://api.example.com/.well-known/oauth-protected-resource/v1/mcp"
+    await auth_flow.aclose()