Skip to content

Commit 43d9599

Browse files
committed
fix(client/auth): run discovery before the 403 step-up re-registers when no AS metadata is cached
After a restart _initialize() restores tokens and client info but never AS metadata, so a 403 insufficient_scope step-up that needs to re-register (lapsed secret, or no stored record) previously registered blind: it bypassed a configured CIMD URL (should_use_client_metadata_url is False without metadata) and POSTed the registration document to the resource origin's /register fallback — the wrong server in the standard separate-AS topology, failing every step-up until the access token expired. The 401 flow's discovery sequence (PRM + SEP-2352 issuer checks + ASM) now lives in a _discover_authorization_server_metadata sub-generator shared by both branches; the 403 step-up runs it before the expiry discard whenever oauth_metadata is None and re-registration is coming, so registration targets the discovered endpoint (or resolves CIMD). extract_resource_metadata_from_ www_auth now honors the resource_metadata parameter on 403 challenges too (RFC 9728 attaches it to any WWW-Authenticate challenge), so the step-up's discovery is seeded from the challenge header exactly like the 401 flow's. Covered by two restart-shaped step-up tests: DCR against the discovered registration endpoint, and CIMD resolution once discovery restores the advertised capability.
1 parent 32a056d commit 43d9599

3 files changed

Lines changed: 353 additions & 83 deletions

File tree

src/mcp/client/auth/oauth2.py

Lines changed: 131 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -678,6 +678,101 @@ async def _complete_client_registration(self, response: httpx2.Response) -> None
678678
self.context.client_info = client_information
679679
await self.context.storage.set_client_info(client_information)
680680

681+
async def _discover_authorization_server_metadata(
682+
self, response: httpx2.Response
683+
) -> AsyncGenerator[httpx2.Request, httpx2.Response]:
684+
"""Discover protected resource and authorization server metadata.
685+
686+
Runs the discovery sequence shared by the 401 flow and the 403 step-up:
687+
protected resource metadata (SEP-985, seeded from the challenge's
688+
`resource_metadata` parameter when present), the SEP-2352 issuer checks on
689+
stored credentials, and OAuth authorization server metadata. Yields each
690+
discovery request; the caller must send the response back into the generator
691+
(the httpx auth-flow protocol).
692+
"""
693+
www_auth_resource_metadata_url = extract_resource_metadata_from_www_auth(response)
694+
695+
# Step 1: Discover protected resource metadata (SEP-985 with fallback support)
696+
prm_discovery_urls = build_protected_resource_metadata_discovery_urls(
697+
www_auth_resource_metadata_url, self.context.server_url
698+
)
699+
700+
for url in prm_discovery_urls: # pragma: no branch
701+
discovery_request = create_oauth_metadata_request(url)
702+
703+
discovery_response = yield discovery_request # sending request
704+
705+
prm = await handle_protected_resource_response(discovery_response)
706+
if prm:
707+
# Validate PRM resource matches server URL (RFC 8707)
708+
await self._validate_resource_match(prm)
709+
self.context.protected_resource_metadata = prm
710+
711+
# todo: try all authorization_servers to find the OASM
712+
assert (
713+
len(prm.authorization_servers) > 0
714+
) # this is always true as authorization_servers has a min length of 1
715+
716+
self.context.auth_server_url = str(prm.authorization_servers[0])
717+
break
718+
else:
719+
logger.debug(f"Protected resource metadata discovery failed: {url}")
720+
721+
# SEP-2352: stored credentials are bound to the issuer that registered them.
722+
# If the authorization server changed, drop them (and the old tokens) so the
723+
# flow re-registers instead of presenting another server's credentials.
724+
if (
725+
self.context.client_info is not None
726+
and self.context.auth_server_url is not None
727+
and not credentials_match_issuer(
728+
self.context.client_info, self.context.auth_server_url, self.context.client_metadata_url
729+
)
730+
):
731+
logger.debug("Authorization server changed; discarding bound credentials and re-registering")
732+
self.context.client_info = None
733+
self.context.clear_tokens()
734+
# Any cached AS metadata is for the old server; drop it so a failed
735+
# rediscovery cannot leak the old registration/token endpoints into Step 4.
736+
self.context.oauth_metadata = None
737+
738+
asm_discovery_urls = build_oauth_authorization_server_metadata_discovery_urls(
739+
self.context.auth_server_url, self.context.server_url
740+
)
741+
742+
# Step 2: Discover OAuth Authorization Server Metadata (OASM) (with fallback for legacy servers)
743+
for url in asm_discovery_urls: # pragma: no branch
744+
oauth_metadata_request = create_oauth_metadata_request(url)
745+
oauth_metadata_response = yield oauth_metadata_request
746+
747+
ok, asm = await handle_auth_metadata_response(oauth_metadata_response)
748+
if not ok:
749+
break
750+
if ok and asm:
751+
# SEP-2468: metadata issuer must match the discovery issuer
752+
if self.context.auth_server_url is not None:
753+
validate_metadata_issuer(asm, self.context.auth_server_url)
754+
self.context.oauth_metadata = asm
755+
break
756+
else:
757+
logger.debug(f"OAuth metadata discovery failed: {url}")
758+
759+
# SEP-2352: on the legacy no-PRM path the issuer is only known after ASM
760+
# discovery, so re-evaluate the binding here using the discovered metadata
761+
# issuer (mirroring the bound_issuer fallback in Step 4).
762+
if (
763+
self.context.client_info is not None
764+
and self.context.auth_server_url is None
765+
and self.context.oauth_metadata is not None
766+
and not credentials_match_issuer(
767+
self.context.client_info,
768+
str(self.context.oauth_metadata.issuer),
769+
self.context.client_metadata_url,
770+
)
771+
):
772+
logger.debug("Authorization server changed; discarding bound credentials and re-registering")
773+
self.context.client_info = None
774+
self.context.clear_tokens()
775+
681776
async def async_auth_flow(self, request: httpx2.Request) -> AsyncGenerator[httpx2.Request, httpx2.Response]:
682777
"""httpx2 auth flow integration."""
683778
async with self.context.lock:
@@ -713,88 +808,19 @@ async def async_auth_flow(self, request: httpx2.Request) -> AsyncGenerator[httpx
713808
try:
714809
# OAuth flow must be inline due to generator constraints
715810

716-
www_auth_resource_metadata_url = extract_resource_metadata_from_www_auth(response)
717-
718-
# Step 1: Discover protected resource metadata (SEP-985 with fallback support)
719-
prm_discovery_urls = build_protected_resource_metadata_discovery_urls(
720-
www_auth_resource_metadata_url, self.context.server_url
721-
)
722-
723-
for url in prm_discovery_urls: # pragma: no branch
724-
discovery_request = create_oauth_metadata_request(url)
725-
726-
discovery_response = yield discovery_request # sending request
727-
728-
prm = await handle_protected_resource_response(discovery_response)
729-
if prm:
730-
# Validate PRM resource matches server URL (RFC 8707)
731-
await self._validate_resource_match(prm)
732-
self.context.protected_resource_metadata = prm
733-
734-
# todo: try all authorization_servers to find the OASM
735-
assert (
736-
len(prm.authorization_servers) > 0
737-
) # this is always true as authorization_servers has a min length of 1
738-
739-
self.context.auth_server_url = str(prm.authorization_servers[0])
740-
break
741-
else:
742-
logger.debug(f"Protected resource metadata discovery failed: {url}")
743-
744-
# SEP-2352: stored credentials are bound to the issuer that registered them.
745-
# If the authorization server changed, drop them (and the old tokens) so the
746-
# flow re-registers instead of presenting another server's credentials.
747-
if (
748-
self.context.client_info is not None
749-
and self.context.auth_server_url is not None
750-
and not credentials_match_issuer(
751-
self.context.client_info, self.context.auth_server_url, self.context.client_metadata_url
752-
)
753-
):
754-
logger.debug("Authorization server changed; discarding bound credentials and re-registering")
755-
self.context.client_info = None
756-
self.context.clear_tokens()
757-
# Any cached AS metadata is for the old server; drop it so a failed
758-
# rediscovery cannot leak the old registration/token endpoints into Step 4.
759-
self.context.oauth_metadata = None
760-
761-
asm_discovery_urls = build_oauth_authorization_server_metadata_discovery_urls(
762-
self.context.auth_server_url, self.context.server_url
763-
)
764-
765-
# Step 2: Discover OAuth Authorization Server Metadata (OASM) (with fallback for legacy servers)
766-
for url in asm_discovery_urls: # pragma: no branch
767-
oauth_metadata_request = create_oauth_metadata_request(url)
768-
oauth_metadata_response = yield oauth_metadata_request
769-
770-
ok, asm = await handle_auth_metadata_response(oauth_metadata_response)
771-
if not ok:
772-
break
773-
if ok and asm:
774-
# SEP-2468: metadata issuer must match the discovery issuer
775-
if self.context.auth_server_url is not None:
776-
validate_metadata_issuer(asm, self.context.auth_server_url)
777-
self.context.oauth_metadata = asm
778-
break
779-
else:
780-
logger.debug(f"OAuth metadata discovery failed: {url}")
781-
782-
# SEP-2352: on the legacy no-PRM path the issuer is only known after ASM
783-
# discovery, so re-evaluate the binding here using the discovered metadata
784-
# issuer (mirroring the bound_issuer fallback in Step 4).
785-
if (
786-
self.context.client_info is not None
787-
and self.context.auth_server_url is None
788-
and self.context.oauth_metadata is not None
789-
and not credentials_match_issuer(
790-
self.context.client_info,
791-
str(self.context.oauth_metadata.issuer),
792-
self.context.client_metadata_url,
793-
)
794-
):
795-
logger.debug("Authorization server changed; discarding bound credentials and re-registering")
796-
self.context.client_info = None
797-
self.context.clear_tokens()
811+
# Steps 1-2: Discover protected resource and authorization server
812+
# metadata, applying the SEP-2352 issuer checks along the way. The
813+
# sequence lives in a sub-generator shared with the 403 step-up;
814+
# its requests are relayed by hand (`yield from` cannot cross an
815+
# async generator).
816+
discovery = self._discover_authorization_server_metadata(response)
817+
try:
818+
discovery_request = await anext(discovery)
819+
while True:
820+
discovery_response = yield discovery_request
821+
discovery_request = await discovery.asend(discovery_response)
822+
except StopAsyncIteration:
823+
pass
798824

799825
# Step 3: Apply scope selection strategy
800826
self.context.client_metadata.scope = get_client_metadata_scopes(
@@ -844,6 +870,29 @@ async def async_auth_flow(self, request: httpx2.Request) -> AsyncGenerator[httpx
844870
# Step 2: Check if we need to step-up authorization
845871
if error == "insufficient_scope": # pragma: no branch
846872
try:
873+
# After a restart the 403 step-up can be the first auth event:
874+
# `_initialize` restores tokens and client info but never AS
875+
# metadata, and the still-live access token keeps the 401 flow
876+
# (and its discovery) from running. When this step-up will need
877+
# to re-register — the stored secret lapsed, or no record is
878+
# stored at all — discover the metadata first instead of
879+
# registering blind: the blind fallback would bypass a
880+
# configured CIMD URL and POST the registration document to the
881+
# resource origin's `/register`, the wrong server in the
882+
# separate-AS topology. Running it before the expiry discard
883+
# lets the SEP-2352 issuer checks still see the record's stamp.
884+
if self.context.oauth_metadata is None and (
885+
self.context.registration_secret_expired() or self.context.client_info is None
886+
):
887+
discovery = self._discover_authorization_server_metadata(response)
888+
try:
889+
discovery_request = await anext(discovery)
890+
while True:
891+
discovery_response = yield discovery_request
892+
discovery_request = await discovery.asend(discovery_response)
893+
except StopAsyncIteration:
894+
pass
895+
847896
# Step 2a: Union previously requested scopes with the newly challenged
848897
# scopes (SEP-2350) so escalating one operation keeps the others' grants.
849898
# Fold in the stored token's scope too: on a restart the token is reloaded

src/mcp/client/auth/utils.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,13 @@ def extract_scope_from_www_auth(response: Response) -> str | None:
5151
def extract_resource_metadata_from_www_auth(response: Response) -> str | None:
5252
"""Extract protected resource metadata URL from WWW-Authenticate header as per RFC 9728.
5353
54+
RFC 9728 attaches the `resource_metadata` parameter to any WWW-Authenticate
55+
challenge, so both 401 and 403 (scope step-up) responses are honored.
56+
5457
Returns:
5558
Resource metadata URL if found in WWW-Authenticate header, None otherwise
5659
"""
57-
if not response or response.status_code != 401:
60+
if not response or response.status_code not in (401, 403):
5861
return None # pragma: no cover
5962

6063
return extract_field_from_www_auth(response, "resource_metadata")

0 commit comments

Comments
 (0)