Skip to content

Commit d4fbec4

Browse files
committed
Gate DCR issuer stamp on a discovered registration_endpoint
`registration_endpoint` is optional in RFC 8414 metadata, and DCR falls back to the resource-server origin's `/register` when it is absent. Recording that fallback registration as bound to the discovered issuer asserts a binding that was never established, so only stamp the issuer in the DCR branch when the discovered metadata actually carried a `registration_endpoint`. The CIMD branch is unaffected since CIMD records are portable across authorization servers.
1 parent a808d14 commit d4fbec4

2 files changed

Lines changed: 54 additions & 24 deletions

File tree

src/mcp/client/auth/oauth2.py

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -643,26 +643,22 @@ async def async_auth_flow(self, request: httpx.Request) -> AsyncGenerator[httpx.
643643

644644
# Step 4: Register client or use URL-based client ID (CIMD)
645645
if not self.context.client_info:
646-
# SEP-2352: bind the credentials to the issuing AS — but only when ASM
647-
# discovery succeeded, so the registration request below actually targets
648-
# that issuer's registration_endpoint. With no metadata (discovery failed),
649-
# DCR falls back to the resource-server origin's /register; recording that
650-
# as bound to a PRM-advertised AS we never reached would persist a
651-
# mis-bound record that the binding check then accepts indefinitely.
652-
bound_issuer: str | None = None
646+
# SEP-2352: the issuer to bind these credentials to, when known.
647+
discovered_issuer: str | None = None
653648
if self.context.oauth_metadata is not None:
654-
bound_issuer = self.context.auth_server_url or str(self.context.oauth_metadata.issuer)
649+
discovered_issuer = self.context.auth_server_url or str(self.context.oauth_metadata.issuer)
655650

656651
if should_use_client_metadata_url(
657652
self.context.oauth_metadata, self.context.client_metadata_url
658653
):
659-
# Use URL-based client ID (CIMD)
654+
# Use URL-based client ID (CIMD). CIMD records are portable across
655+
# authorization servers, so the issuer stamp is informational.
660656
logger.debug(f"Using URL-based client ID (CIMD): {self.context.client_metadata_url}")
661657
client_information = create_client_info_from_metadata_url(
662658
self.context.client_metadata_url, # type: ignore[arg-type]
663659
redirect_uris=self.context.client_metadata.redirect_uris,
664660
)
665-
client_information.issuer = bound_issuer
661+
client_information.issuer = discovered_issuer
666662
self.context.client_info = client_information
667663
await self.context.storage.set_client_info(client_information)
668664
else:
@@ -674,7 +670,16 @@ async def async_auth_flow(self, request: httpx.Request) -> AsyncGenerator[httpx.
674670
)
675671
registration_response = yield registration_request
676672
client_information = await handle_registration_response(registration_response)
677-
client_information.issuer = bound_issuer
673+
# Only record the issuer when the registration above actually targeted
674+
# the discovered AS's registration_endpoint. With no metadata, or
675+
# metadata that omits registration_endpoint, DCR fell back to the
676+
# resource-server origin's /register — recording that as bound to a
677+
# PRM-advertised AS would persist a binding that was never established.
678+
if (
679+
self.context.oauth_metadata is not None
680+
and self.context.oauth_metadata.registration_endpoint is not None
681+
):
682+
client_information.issuer = discovered_issuer
678683
self.context.client_info = client_information
679684
await self.context.storage.set_client_info(client_information)
680685

tests/client/test_auth.py

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2936,16 +2936,39 @@ async def test_issuer_binding_re_evaluated_after_asm_when_prm_discovery_failed(
29362936

29372937

29382938
@pytest.mark.anyio
2939-
async def test_issuer_is_not_stamped_when_registration_falls_back_after_asm_discovery_fails(
2940-
oauth_provider: OAuthClientProvider, mock_storage: MockTokenStorage
2939+
@pytest.mark.parametrize(
2940+
"asm_responses",
2941+
[
2942+
pytest.param(
2943+
[httpx.Response(404), httpx.Response(404)],
2944+
id="asm-discovery-failed",
2945+
),
2946+
pytest.param(
2947+
[
2948+
httpx.Response(
2949+
200,
2950+
content=(
2951+
b'{"issuer": "https://new-as.example.com", '
2952+
b'"authorization_endpoint": "https://new-as.example.com/authorize", '
2953+
b'"token_endpoint": "https://new-as.example.com/token"}'
2954+
),
2955+
)
2956+
],
2957+
id="asm-metadata-without-registration-endpoint",
2958+
),
2959+
],
2960+
)
2961+
async def test_issuer_is_not_stamped_when_registration_falls_back_to_the_resource_origin(
2962+
oauth_provider: OAuthClientProvider, mock_storage: MockTokenStorage, asm_responses: list[httpx.Response]
29412963
):
2942-
"""SEP-2352: a fallback registration is not recorded as bound to an undiscovered AS.
2964+
"""SEP-2352: a fallback registration is not recorded as bound to the PRM-advertised AS.
29432965
29442966
PRM advertises a new authorization server, so the stored credentials (bound to the old
2945-
issuer) are discarded. ASM discovery for the new server then fails, so DCR falls back to
2946-
the resource-server origin's ``/register``. That registration was not derived from the new
2947-
AS's metadata, so persisting it as bound to the new AS would wedge the binding check on
2948-
later flows; instead the issuer is left unset.
2967+
issuer) are discarded. DCR then falls back to the resource-server origin's ``/register``
2968+
because the new AS's metadata either could not be discovered or omits
2969+
``registration_endpoint``. That registration was not derived from the new AS's metadata,
2970+
so persisting it as bound to the new AS would wedge the binding check on later flows;
2971+
instead the issuer is left unset.
29492972
"""
29502973
oauth_provider.context.current_tokens = None
29512974
oauth_provider.context.token_expiry_time = None
@@ -2991,16 +3014,18 @@ async def echo_callback() -> AuthorizationCodeResult:
29913014
request=prm_req,
29923015
)
29933016

2994-
# ASM discovery for the new AS fails on every well-known URL.
2995-
asm_req = await auth_flow.asend(prm_response)
3017+
# ASM discovery for the new AS yields no usable registration_endpoint — either every
3018+
# well-known URL 404s, or metadata is returned without one.
3019+
next_req = await auth_flow.asend(prm_response)
29963020
assert oauth_provider.context.client_info is None
29973021
assert oauth_provider.context.oauth_metadata is None
2998-
assert str(asm_req.url) == "https://new-as.example.com/.well-known/oauth-authorization-server"
2999-
asm_req = await auth_flow.asend(httpx.Response(404, request=asm_req))
3000-
assert str(asm_req.url) == "https://new-as.example.com/.well-known/openid-configuration"
3022+
assert str(next_req.url) == "https://new-as.example.com/.well-known/oauth-authorization-server"
3023+
for asm_response in asm_responses:
3024+
asm_response.request = next_req
3025+
next_req = await auth_flow.asend(asm_response)
30013026

30023027
# Step 4 falls back to the resource-server origin's /register.
3003-
dcr_req = await auth_flow.asend(httpx.Response(404, request=asm_req))
3028+
dcr_req = next_req
30043029
assert dcr_req.method == "POST"
30053030
assert str(dcr_req.url) == "https://api.example.com/register"
30063031
dcr_response = httpx.Response(

0 commit comments

Comments
 (0)