Skip to content

Commit 5717fa2

Browse files
committed
[v1.x] Derive the expected issuer before fetching authorization server metadata
Backport of the discovery half of #3398, together with the SEP-2352 credential binding that main has had since 2.0 and that change relies on (the parts of #2921, #2933, #2936, #2946 and #3181 it needs). The discovery step now knows which issuer the authorization server metadata must carry before fetching it: the PRM-advertised server, or on the 2025-03-26 no-PRM fallback the resource server's origin, which is what that well-known URL is built from (RFC 8414 section 3.3). On this line there was no issuer check at all; it now runs on both paths (`validate_metadata_issuer`, with `issuers_match`: exact, except that a root issuer with and without its trailing slash is the same server, which matters here because every parsed issuer renders with the slash). Stored client credentials gain the binding main has: the SDK records the issuer a registration was made with (`OAuthClientInformationFull.issuer`, never read from the registration response), and before any metadata is fetched a registration bound to a different issuer is discarded together with its tokens so the flow re-registers instead of presenting another server's credentials (`credentials_match_issuer`); newly registered clients are bound to the expected issuer when metadata for it was found and the registration actually went to that server. When no protected resource metadata location answers and one of them failed with a server error (or 429), the flow stops rather than reading that as "no such metadata". A 403 insufficient_scope step-up takes the same path as a 401: it discovers first when no metadata is held (extract_resource_metadata_from_www_auth also reads the `resource_metadata` hint from a 403 challenge) and then re-authorizes with the challenged scope; metadata already discovered in this process is reused as before. A 403 that is not a scope challenge is handed back to the caller instead of being retried unchanged. The conformance harness pinned on this line (0.1.13) still serves, in its two path-based authorization server scenarios, metadata whose `issuer` omits the advertised tenant path; those two scenarios are listed as expected failures until the pin moves past conformance 0.1.15, where the mock was corrected. Differences from the main change: - RFC 9207 `iss` validation is not included; the callback contract on 1.x stays `tuple[str, str | None]`. - 1.x has no SEP-2350 scope union, so a step-up asks for the challenged scope as it did before. - The root-slash allowance lives in `validate_metadata_issuer` itself (new on this line) rather than only on the legacy path.
1 parent 92120b4 commit 5717fa2

5 files changed

Lines changed: 965 additions & 87 deletions

File tree

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,11 @@
11
# Known conformance test failures for v1.x
22
# These are tracked and should be removed as they're fixed.
33
server: []
4-
client: []
4+
client:
5+
# The pinned harness (0.1.13) serves authorization server metadata whose `issuer`
6+
# omits the tenant path its resource metadata advertises (`/tenant1`), so a client
7+
# that checks RFC 8414 section 3.3 refuses it. The mock includes the path from
8+
# conformance 0.1.15 (modelcontextprotocol/conformance#152); drop these two entries
9+
# when the pin moves past it.
10+
- auth/metadata-var2
11+
- auth/metadata-var3

src/mcp/client/auth/oauth2.py

Lines changed: 124 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717

1818
import anyio
1919
import httpx
20-
from pydantic import BaseModel, Field, ValidationError
20+
from pydantic import AnyHttpUrl, BaseModel, Field, ValidationError
2121

2222
from mcp.client.auth.exceptions import OAuthFlowError, OAuthTokenError
2323
from mcp.client.auth.utils import (
@@ -26,6 +26,7 @@
2626
create_client_info_from_metadata_url,
2727
create_client_registration_request,
2828
create_oauth_metadata_request,
29+
credentials_match_issuer,
2930
extract_field_from_www_auth,
3031
extract_resource_metadata_from_www_auth,
3132
extract_scope_from_www_auth,
@@ -36,6 +37,7 @@
3637
handle_token_response_scopes,
3738
is_valid_client_metadata_url,
3839
should_use_client_metadata_url,
40+
validate_metadata_issuer,
3941
)
4042
from mcp.client.streamable_http import MCP_PROTOCOL_VERSION
4143
from mcp.shared.auth import (
@@ -214,6 +216,14 @@ def prepare_token_auth(
214216
return data, headers
215217

216218

219+
def _origin_issuer(server_url: str) -> str:
220+
"""The resource server's origin as an issuer identifier: `scheme://authority`, rendered the way
221+
`OAuthMetadata.issuer` renders URLs (host case, default ports, trailing slash) so the two compare
222+
as strings."""
223+
parsed = urlparse(server_url)
224+
return str(AnyHttpUrl(f"{parsed.scheme}://{parsed.netloc}"))
225+
226+
217227
class OAuthClientProvider(httpx.Auth):
218228
"""
219229
OAuth2 authentication for httpx.
@@ -488,6 +498,16 @@ async def _handle_oauth_metadata_response(self, response: httpx.Response) -> Non
488498
metadata = OAuthMetadata.model_validate_json(content)
489499
self.context.oauth_metadata = metadata
490500

501+
def _select_authorization_server(self, advertised: list[str]) -> str:
502+
"""Which of the servers listed in protected resource metadata to use: the first (the list is never empty)."""
503+
return advertised[0]
504+
505+
def _expected_issuer(self) -> str:
506+
"""The issuer that authorization server metadata and client credentials must belong to: the
507+
PRM-advertised server, or on the legacy no-PRM path the resource server's origin, which is what
508+
the 2025-03-26 well-known URL is built from (RFC 8414 §3.3)."""
509+
return self.context.auth_server_url or _origin_issuer(self.context.server_url)
510+
491511
async def async_auth_flow(self, request: httpx.Request) -> AsyncGenerator[httpx.Request, httpx.Response]:
492512
"""HTTPX auth flow integration."""
493513
async with self.context.lock:
@@ -511,55 +531,89 @@ async def async_auth_flow(self, request: httpx.Request) -> AsyncGenerator[httpx.
511531

512532
response = yield request
513533

514-
if response.status_code == 401:
534+
step_up = (
535+
response.status_code == 403 and extract_field_from_www_auth(response, "error") == "insufficient_scope"
536+
)
537+
538+
if response.status_code == 401 or step_up:
515539
# Perform full OAuth flow
516540
try:
517-
# OAuth flow must be inline due to generator constraints
518-
www_auth_resource_metadata_url = extract_resource_metadata_from_www_auth(response)
519-
520-
# Step 1: Discover protected resource metadata (SEP-985 with fallback support)
521-
prm_discovery_urls = build_protected_resource_metadata_discovery_urls(
522-
www_auth_resource_metadata_url, self.context.server_url
523-
)
524-
525-
for url in prm_discovery_urls: # pragma: no branch
526-
discovery_request = create_oauth_metadata_request(url)
527-
528-
discovery_response = yield discovery_request # sending request
529-
530-
prm = await handle_protected_resource_response(discovery_response)
531-
if prm:
532-
# Validate PRM resource matches server URL (RFC 8707)
533-
await self._validate_resource_match(prm)
534-
self.context.protected_resource_metadata = prm
535-
536-
# todo: try all authorization_servers to find the OASM
537-
assert (
538-
len(prm.authorization_servers) > 0
539-
) # this is always true as authorization_servers has a min length of 1
541+
# OAuth flow must be inline due to generator constraints.
542+
# Steps 1-2 run on every 401. A scope step-up reuses the metadata discovered earlier
543+
# in this process, and discovers it first when none is held yet (for example when
544+
# tokens were loaded from storage), so re-authorization targets the right server.
545+
if response.status_code == 401 or self.context.oauth_metadata is None:
546+
www_auth_resource_metadata_url = extract_resource_metadata_from_www_auth(response)
547+
548+
# Step 1: Discover protected resource metadata (SEP-985 with fallback support)
549+
prm_discovery_urls = build_protected_resource_metadata_discovery_urls(
550+
www_auth_resource_metadata_url, self.context.server_url
551+
)
540552

541-
self.context.auth_server_url = str(prm.authorization_servers[0])
542-
break
553+
prm_request_failed: int | None = None
554+
for url in prm_discovery_urls:
555+
discovery_request = create_oauth_metadata_request(url)
556+
557+
discovery_response = yield discovery_request # sending request
558+
559+
if discovery_response.status_code >= 500 or discovery_response.status_code == 429:
560+
prm_request_failed = discovery_response.status_code
561+
prm = await handle_protected_resource_response(discovery_response)
562+
if prm:
563+
# Validate PRM resource matches server URL (RFC 8707)
564+
await self._validate_resource_match(prm)
565+
self.context.protected_resource_metadata = prm
566+
self.context.auth_server_url = self._select_authorization_server(
567+
[str(url) for url in prm.authorization_servers]
568+
)
569+
break
570+
else:
571+
logger.debug(f"Protected resource metadata discovery failed: {url}")
543572
else:
544-
logger.debug(f"Protected resource metadata discovery failed: {url}")
545-
546-
asm_discovery_urls = build_oauth_authorization_server_metadata_discovery_urls(
547-
self.context.auth_server_url, self.context.server_url
548-
)
573+
if prm_request_failed is not None:
574+
# A server error says nothing about whether the resource publishes
575+
# metadata, so it must not send the flow down the legacy path.
576+
raise OAuthFlowError(
577+
f"Protected resource metadata request failed: HTTP {prm_request_failed}"
578+
)
579+
580+
expected_issuer = self._expected_issuer()
581+
582+
# SEP-2352: stored credentials are bound to the issuer that registered them.
583+
# Decided before any metadata is fetched: if the expected issuer is a different
584+
# server, drop them (and the old tokens) so the flow re-registers instead of
585+
# presenting another server's credentials.
586+
if self.context.client_info is not None and not credentials_match_issuer(
587+
self.context.client_info, expected_issuer, self.context.client_metadata_url
588+
):
589+
logger.debug(
590+
"Authorization server changed; discarding bound credentials and re-registering"
591+
)
592+
self.context.client_info = None
593+
self.context.clear_tokens()
594+
# Any cached AS metadata is for the old server; drop it so a failed
595+
# rediscovery cannot leak the old registration/token endpoints into Step 4.
596+
self.context.oauth_metadata = None
597+
598+
asm_discovery_urls = build_oauth_authorization_server_metadata_discovery_urls(
599+
self.context.auth_server_url, self.context.server_url
600+
)
549601

550-
# Step 2: Discover OAuth Authorization Server Metadata (OASM) (with fallback for legacy servers)
551-
for url in asm_discovery_urls: # pragma: no cover
552-
oauth_metadata_request = create_oauth_metadata_request(url)
553-
oauth_metadata_response = yield oauth_metadata_request
554-
555-
ok, asm = await handle_auth_metadata_response(oauth_metadata_response)
556-
if not ok:
557-
break
558-
if ok and asm:
559-
self.context.oauth_metadata = asm
560-
break
561-
else:
562-
logger.debug(f"OAuth metadata discovery failed: {url}")
602+
# Step 2: Discover OAuth Authorization Server Metadata (OASM) (with fallback for legacy servers)
603+
for url in asm_discovery_urls: # pragma: no branch
604+
oauth_metadata_request = create_oauth_metadata_request(url)
605+
oauth_metadata_response = yield oauth_metadata_request
606+
607+
ok, asm = await handle_auth_metadata_response(oauth_metadata_response)
608+
if not ok:
609+
break
610+
if ok and asm:
611+
# SEP-2468 / RFC 8414 section 3.3: the metadata must name the expected issuer
612+
validate_metadata_issuer(asm, expected_issuer)
613+
self.context.oauth_metadata = asm
614+
break
615+
else:
616+
logger.debug(f"OAuth metadata discovery failed: {url}")
563617

564618
# Step 3: Apply scope selection strategy
565619
self.context.client_metadata.scope = get_client_metadata_scopes(
@@ -570,58 +624,56 @@ async def async_auth_flow(self, request: httpx.Request) -> AsyncGenerator[httpx.
570624

571625
# Step 4: Register client or use URL-based client ID (CIMD)
572626
if not self.context.client_info:
627+
# SEP-2352: the issuer to bind these credentials to, once metadata for it
628+
# was actually found.
629+
discovered_issuer = self._expected_issuer() if self.context.oauth_metadata is not None else None
630+
573631
if should_use_client_metadata_url(
574632
self.context.oauth_metadata, self.context.client_metadata_url
575633
):
576-
# Use URL-based client ID (CIMD)
634+
# Use URL-based client ID (CIMD). CIMD records are portable across
635+
# authorization servers, so the issuer stamp is informational.
577636
logger.debug(f"Using URL-based client ID (CIMD): {self.context.client_metadata_url}")
578637
client_information = create_client_info_from_metadata_url(
579638
self.context.client_metadata_url, # type: ignore[arg-type]
580639
redirect_uris=self.context.client_metadata.redirect_uris,
581640
)
641+
client_information.issuer = discovered_issuer
582642
self.context.client_info = client_information
583643
await self.context.storage.set_client_info(client_information)
584644
else:
585645
# Fallback to Dynamic Client Registration
646+
fallback_base = self.context.get_authorization_base_url(self.context.server_url)
586647
registration_request = create_client_registration_request(
587-
self.context.oauth_metadata,
588-
self.context.client_metadata,
589-
self.context.get_authorization_base_url(self.context.server_url),
648+
self.context.oauth_metadata, self.context.client_metadata, fallback_base
590649
)
591650
registration_response = yield registration_request
592651
client_information = await handle_registration_response(registration_response)
652+
# Only record the issuer when the registration above actually targeted
653+
# the discovered AS - either via its published registration_endpoint,
654+
# or because the resource-origin /register fallback is on the issuer's
655+
# own host (legacy same-origin embedded AS). Otherwise the fallback hit
656+
# a different server and recording a binding to the PRM-advertised AS
657+
# would persist a binding that was never established.
658+
if (
659+
self.context.oauth_metadata is not None
660+
and discovered_issuer is not None
661+
and (
662+
self.context.oauth_metadata.registration_endpoint is not None
663+
or self.context.get_authorization_base_url(discovered_issuer) == fallback_base
664+
)
665+
):
666+
client_information.issuer = discovered_issuer
593667
self.context.client_info = client_information
594668
await self.context.storage.set_client_info(client_information)
595669

596670
# Step 5: Perform authorization and complete token exchange
597671
token_response = yield await self._perform_authorization()
598672
await self._handle_token_response(token_response)
599-
except Exception: # pragma: no cover
673+
except Exception:
600674
logger.exception("OAuth flow error")
601675
raise
602676

603677
# Retry with new tokens
604678
self._add_auth_header(request)
605679
yield request
606-
elif response.status_code == 403:
607-
# Step 1: Extract error field from WWW-Authenticate header
608-
error = extract_field_from_www_auth(response, "error")
609-
610-
# Step 2: Check if we need to step-up authorization
611-
if error == "insufficient_scope": # pragma: no branch
612-
try:
613-
# Step 2a: Update the required scopes
614-
self.context.client_metadata.scope = get_client_metadata_scopes(
615-
extract_scope_from_www_auth(response), self.context.protected_resource_metadata
616-
)
617-
618-
# Step 2b: Perform (re-)authorization and token exchange
619-
token_response = yield await self._perform_authorization()
620-
await self._handle_token_response(token_response)
621-
except Exception: # pragma: no cover
622-
logger.exception("OAuth flow error")
623-
raise
624-
625-
# Retry with new tokens
626-
self._add_auth_header(request)
627-
yield request

0 commit comments

Comments
 (0)