Skip to content

Commit a808d14

Browse files
committed
Only bind issuer when registration targets the discovered AS
When ASM discovery fails, dynamic client registration falls back to the resource-server origin's `/register`. Recording that registration as bound to a PRM-advertised authorization server we never actually reached would persist a mis-bound record that the SEP-2352 binding check then accepts on every later flow, wedging the client on the wrong `client_id`. Leave the issuer unset in that case so a later 401 with working discovery re-evaluates cleanly.
1 parent 676cc1f commit a808d14

2 files changed

Lines changed: 102 additions & 7 deletions

File tree

src/mcp/client/auth/oauth2.py

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -643,12 +643,15 @@ 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. Prefer the PRM-advertised
647-
# authorization server; on the legacy no-PRM path fall back to the issuer from
648-
# the discovered metadata so the binding is still recorded.
649-
bound_issuer = self.context.auth_server_url
650-
if bound_issuer is None and self.context.oauth_metadata is not None:
651-
bound_issuer = str(self.context.oauth_metadata.issuer)
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
653+
if self.context.oauth_metadata is not None:
654+
bound_issuer = self.context.auth_server_url or str(self.context.oauth_metadata.issuer)
652655

653656
if should_use_client_metadata_url(
654657
self.context.oauth_metadata, self.context.client_metadata_url

tests/client/test_auth.py

Lines changed: 93 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ async def set_tokens(self, tokens: OAuthToken) -> None:
5757
self._tokens = tokens
5858

5959
async def get_client_info(self) -> OAuthClientInformationFull | None:
60-
return self._client_info # pragma: no cover
60+
return self._client_info
6161

6262
async def set_client_info(self, client_info: OAuthClientInformationFull) -> None:
6363
self._client_info = client_info
@@ -2933,3 +2933,95 @@ async def test_issuer_binding_re_evaluated_after_asm_when_prm_discovery_failed(
29332933
assert next_req.method == "POST"
29342934
assert str(next_req.url) == "https://api.example.com/register"
29352935
await auth_flow.aclose()
2936+
2937+
2938+
@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
2941+
):
2942+
"""SEP-2352: a fallback registration is not recorded as bound to an undiscovered AS.
2943+
2944+
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.
2949+
"""
2950+
oauth_provider.context.current_tokens = None
2951+
oauth_provider.context.token_expiry_time = None
2952+
oauth_provider._initialized = True
2953+
oauth_provider.context.client_info = OAuthClientInformationFull(
2954+
client_id="stale-client",
2955+
redirect_uris=[AnyUrl("http://localhost:3030/callback")],
2956+
issuer="https://api.example.com/",
2957+
)
2958+
2959+
captured_state: str | None = None
2960+
2961+
async def capture_redirect(url: str) -> None:
2962+
nonlocal captured_state
2963+
captured_state = parse_qs(urlparse(url).query).get("state", [None])[0]
2964+
2965+
async def echo_callback() -> AuthorizationCodeResult:
2966+
return AuthorizationCodeResult(code="auth_code", state=captured_state)
2967+
2968+
oauth_provider.context.redirect_handler = capture_redirect
2969+
oauth_provider.context.callback_handler = echo_callback
2970+
2971+
auth_flow = oauth_provider.async_auth_flow(httpx.Request("GET", "https://api.example.com/v1/mcp"))
2972+
request = await auth_flow.__anext__()
2973+
response_401 = httpx.Response(
2974+
401,
2975+
headers={
2976+
"WWW-Authenticate": (
2977+
'Bearer resource_metadata="https://api.example.com/.well-known/oauth-protected-resource"'
2978+
)
2979+
},
2980+
request=request,
2981+
)
2982+
2983+
# PRM succeeds and advertises a new AS — the discard block fires.
2984+
prm_req = await auth_flow.asend(response_401)
2985+
assert str(prm_req.url) == "https://api.example.com/.well-known/oauth-protected-resource"
2986+
prm_response = httpx.Response(
2987+
200,
2988+
content=(
2989+
b'{"resource": "https://api.example.com/v1/mcp", "authorization_servers": ["https://new-as.example.com"]}'
2990+
),
2991+
request=prm_req,
2992+
)
2993+
2994+
# ASM discovery for the new AS fails on every well-known URL.
2995+
asm_req = await auth_flow.asend(prm_response)
2996+
assert oauth_provider.context.client_info is None
2997+
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"
3001+
3002+
# Step 4 falls back to the resource-server origin's /register.
3003+
dcr_req = await auth_flow.asend(httpx.Response(404, request=asm_req))
3004+
assert dcr_req.method == "POST"
3005+
assert str(dcr_req.url) == "https://api.example.com/register"
3006+
dcr_response = httpx.Response(
3007+
201,
3008+
json={"client_id": "fallback-client", "redirect_uris": ["http://localhost:3030/callback"]},
3009+
request=dcr_req,
3010+
)
3011+
token_req = await auth_flow.asend(dcr_response)
3012+
3013+
# The persisted record carries no issuer binding — not the PRM-advertised AS we never reached.
3014+
stored = await mock_storage.get_client_info()
3015+
assert stored is not None
3016+
assert stored.client_id == "fallback-client"
3017+
assert stored.issuer is None
3018+
3019+
# Drive the flow to completion so the context lock is released cleanly.
3020+
token_response = httpx.Response(
3021+
200, json={"access_token": "t", "token_type": "Bearer", "expires_in": 3600}, request=token_req
3022+
)
3023+
final_req = await auth_flow.asend(token_response)
3024+
try:
3025+
await auth_flow.asend(httpx.Response(200, request=final_req))
3026+
except StopAsyncIteration:
3027+
pass

0 commit comments

Comments
 (0)