[v1.x] Validate the authorization server metadata issuer on every discovery path - #3431
Merged
Conversation
maxisbey
marked this pull request as ready for review
September 2, 2026 16:32
…r 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.
Backport of the second half of #3398. ClientCredentialsOAuthProvider and PrivateKeyJWTOAuthProvider take an optional issuer keyword: the issuer identifier of the authorization server the fixed client_id (and secret) were issued by. When set, token requests are only built from discovered authorization server metadata whose issuer matches it; if discovery yields metadata for another server, or none at all, the flow stops with OAuthFlowError before the secret is attached or an assertion is minted, and the metadata and tokens held are dropped so the next request starts discovery again. When the resource advertises several authorization servers the one matching the configured issuer is used; the comparison is issuers_match (exact, root slash aside); a value that is not an http(s) URL is a ValueError. Omitting it keeps the current behaviour. Differences from the main change: the keyword follows `scopes` (the 1.x name); there is no docs page for these providers on 1.x, so the docstrings carry the description.
maxisbey
force-pushed
the
oauth-issuer-binding-v1x
branch
from
September 2, 2026 16:36
b4c142c to
e5b6719
Compare
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review completed against the latest diff
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…ut an issuer Backport of #3435. Omitting `issuer=` on ClientCredentialsOAuthProvider and PrivateKeyJWTOAuthProvider keeps working and now warns at construction that it will be required in 3.0, saying why (without it the MCP server decides which authorization server receives the credentials) and what to pass. Difference from main: 1.x has no MCPDeprecationWarning, so the category is DeprecationWarning like the other deprecations on this branch, and there is no deprecations docs page to list it on; the docstrings carry the note and the module's examples pass `issuer=`.
11 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
v1.x backport of #3398 and #3435.
The OAuth client on 1.x now decides which issuer it expects before it fetches any authorization server metadata and checks what follows against that one value: the metadata's
issuer(RFC 8414 section 3.3), which 1.x did not check on any path; the binding of stored client credentials to the authorization server they were registered with, which 1.x did not have; and a 403 scope step-up that goes through the same discovery when no metadata is held. The two machine-to-machine providers gain the optionalissuer=keyword.Motivation and Context
Same as #3398. On 1.x the client accepted authorization server metadata without comparing its
issuerto the URL the well-known was built from, and storedclient_id/client_secretcarried no record of which server issued them, so they were presented to whichever authorization server the resource advertised next (or, on the 2025-03-26 fallback, to the resource server's own origin). This brings the 1.x client to the behaviour main has: the expected issuer is the PRM-advertised server, or on the no-PRM path the resource server's origin; metadata that names anything else is refused (a root issuer with and without its trailing slash is the same server); a registration bound to a different issuer is dropped, together with its tokens, before any metadata is fetched, and the client re-registers; newly registered clients are bound to the expected issuer; a 403insufficient_scopewith no metadata held discovers first; a 5xx/429 on the resource metadata request with no location answering stops the flow; andClientCredentialsOAuthProvider/PrivateKeyJWTOAuthProvideracceptissuer=so pre-provisioned credentials only go to that server.Because 1.x had no credential binding, this also carries the parts of #2921, #2933, #2936, #2946 and #3181 that the change relies on:
OAuthClientInformationFull.issuer(recorded by the SDK, never read from a registration response),validate_metadata_issuer,credentials_match_issuerandissuers_matchinmcp.client.auth.utils, and the registration step recording the issuer.Differences from #3398:
issvalidation is not included; the callback contract on 1.x staystuple[str, str | None].validate_metadata_issueritself (new here) rather than only on the legacy path, and recorded issuers carry the slash.issuer=keyword followsscopes(the 1.x name); there is no docs page for these providers on 1.x, so the docstrings carry the description.issuer=is included as a third commit; 1.x has noMCPDeprecationWarning, so it warns withDeprecationWarninglike the other deprecations on this branch, and the note lives in the docstrings (1.x has no deprecations docs page).auth/metadata-var2andauth/metadata-var3, authorization server metadata whoseissueromits the tenant path its resource metadata advertises, so a client that checks RFC 8414 section 3.3 refuses it. Those two scenarios are listed inexpected-failures.ymlwith that note; the mock includes the path from conformance 0.1.15 (fix: include tenant path in auth serverissuerclaim conformance#152), so the entries go when the pin moves past it. (Bumping the pin here instead would trade them for two unrelated baseline entries, so that is left for its own change.)How Has This Been Tested?
New tests in
tests/client/test_auth.pyandtests/client/auth/extensions/test_client_credentials.py, ported from #3398 and from main's credential-binding suite: issuer check on the PRM and legacy paths, root-slash tolerance and its limits, binding kept/discarded/stamped/not-stamped cases, registration responses cannot seed the binding, legacy path with no metadata (from a 401 and from a 403), step-up with and without metadata held, non-challenge 403, PRM 5xx/429,issuer=on both providers. The flow tests were checked to fail againstv1.x'soauth2.pywith the new helpers in place (they import functions 1.x did not have, so the plain "restore src/" recipe cannot collect them). I also drove the 1.x provider throughhttpx.AsyncClient(auth=...)andstreamablehttp_clientagainst local test servers for the PRM and legacy paths, 401-first and 403-first, and client-credentials with and withoutissuer=. Full suite, coverage, pyright and ruff pass locally.Breaking Changes
No API removals or signature changes; additions are the optional
issuer=keyword, the optionalOAuthClientInformationFull.issuerfield (round-trips through the unchangedTokenStoragemethods; the server side does not emit it), and three functions inmcp.client.auth.utils. Observable differences:issueris not the advertised server (or, without protected resource metadata, the resource server's origin) now getsOAuthFlowError: Authorization server metadata issuer mismatch. No client-side override, as on main.POST /register). Records stored by earlier 1.x releases carry no issuer and are left as they are.insufficient_scopeis no longer retried.issuermember in a dynamic client registration response is ignored.Types of changes
Checklist
help wanted, or I'm a maintainer)Additional context
Three commits: the discovery change (with the binding layer it needs on 1.x), the
issuer=keyword, then its deprecation-when-omitted from #3435. The discovery block inasync_auth_flowis re-indented under one new condition; the whitespace-insensitive view is the easier read.AI Disclaimer