Bind client credentials to their authorization server (SEP-2352)#2933
Conversation
Persisted client credentials are now bound to the issuer that registered them: OAuthClientInformationFull records an issuer, set by the SDK after DCR/CIMD. When protected resource metadata points at a different authorization server, the client discards the bound credentials and old tokens and re-registers, instead of presenting one server's client_id to another. URL-based client IDs (CIMD) are portable and always match; credentials with no recorded issuer (pre-registered, or stored before this change) carry no binding to enforce and are left as-is. No TokenStorage protocol change - the issuer round-trips through the existing get_client_info/set_client_info. Follows the Go SDK's approach. The auth/authorization-server-migration conformance scenario's re-register check is satisfied in spirit (no-reuse and no-cross-AS checks pass) but the scenario stays baselined: it runs at 2026-07-28, where client.py's 2025 lifecycle is rejected before the migration 401 fires. It unblocks with the 2026 stateless client lifecycle.
- Stamp the bound issuer from the discovered oauth_metadata.issuer when PRM did not advertise an authorization server (legacy no-PRM path), instead of leaving it None — otherwise migrated resources could reuse the old DCR client_id. - Detect CIMD portability by the client_id equaling the configured client_metadata_url, not by URL shape, so a registration server that issues a URL-shaped client_id is still treated as bound to its issuer.
|
Ran a
Full suite + 100% coverage + both conformance legs still green. |
There was a problem hiding this comment.
I didn't find any bugs in this change, but it modifies the OAuth client credential-handling path (issuer binding, credential/token invalidation, and re-registration logic), so it warrants a human security-minded review rather than an automated approval.
Extended reasoning...
Overview
The PR implements SEP-2352: OAuthClientInformationFull gains an SDK-owned issuer field, the OAuth client provider stamps it after DCR/CIMD registration, and a new credentials_match_issuer helper drops stored credentials (and tokens) when the discovered authorization server no longer matches the recorded issuer. Supporting changes include conformance expected-failure annotations, migration-doc text, unit tests for the matching helper, and an end-to-end interaction test covering the migration scenario.
Security risks
This is security-relevant code by nature: it governs whether persisted client credentials and tokens are reused or discarded across authorization servers. The logic looks sound — CIMD portability is keyed on the client_id equaling the configured client_metadata_url (not URL shape), legacy credentials with no recorded issuer are left alone, and the no-PRM fallback binds to the discovered metadata issuer. The main residual considerations are policy ones (e.g. leaving unbound legacy credentials reusable across AS changes is an intentional compatibility choice, and the issuer comparison is a simple string compare per RFC 3986 §6.2.1), which are reasonable but worth a maintainer's eyes.
Level of scrutiny
OAuth/auth client code in the SDK warrants a higher bar than a mechanical change; per my approval criteria I do not auto-approve security-sensitive paths even when no bugs are found. The change is moderate in size and well-scoped, but it alters when credentials and tokens are invalidated mid-flow, which can affect existing users with persisted storage.
Other factors
Test coverage is good: targeted unit tests for credentials_match_issuer (including the URL-shaped DCR id regression case) and an interaction test asserting the stale client_id never reaches the authorize/token endpoints and that re-registration occurs. The author also already addressed two edge cases raised by a prior automated review pass (legacy no-PRM issuer fallback, CIMD misclassification) in 523b32b. Conformance baselines are updated with clear annotations rather than silently dropped.
Implements SEP-2352 (the last client-side item of #2902): bind OAuth client credentials to their authorization server and re-register when it changes.
What changed
Persisted client credentials are now bound to the issuer that registered them.
OAuthClientInformationFullgains an SDK-ownedissuerfield, set after DCR/CIMD. Before reusing stored credentials, the provider compares their recorded issuer to the authorization server discovered from protected resource metadata; on a mismatch it discards the credentials (and the old tokens) and re-registers, instead of presenting one server'sclient_idto another.TokenStorageprotocol change - theissuerround-trips through the existingget_client_info/set_client_info, so the 8 in-repo implementations and downstream users are unaffected.Follows the Go SDK's approach (issuer field on the client-info, proactive comparison). The TS SDK takes a reactive invalidate-on-error path that doesn't fit our httpx auth-generator cleanly.
Conformance
The
auth/authorization-server-migrationscenario'sno-reuse-on-as-changeandno-cross-as-credential-reusechecks pass (the client never presents the old AS'sclient_idto the new AS). Thereregister-on-as-changecheck is not yet reached: the scenario runs only at2026-07-28, whereclient.py's 2025 stateful lifecycle is rejected (400 oninitialize) before the migration 401 fires. The scenario therefore stays baselined in both expected-failures files, re-annotated to point at the stateless-lifecycle gap (the same gap blockingtools_call,scope-step-up, etc. on the 2026 leg) rather than SEP-2352. An in-process interaction test covers the migration end to end.Both conformance legs pass with no stale or unexpected entries.
#2902 status
This completes the implementation of all five client-side items (SEP-2207, SEP-2468, SEP-837, SEP-2350, SEP-2352). The AS-migration conformance scenario will go green once the 2026 stateless client lifecycle lands.
AI Disclaimer
This PR was developed with the assistance of either Claude or Codex. I've reviewed and verified the changes.