Repository navigation
Split the registration request model from the registered-client record - #3181
maxisbey merged 4 commits into
Conversation
Sorry, something went wrong.
📚 Documentation preview
|
Sorry, something went wrong.
There was a problem hiding this comment.
All reported issues were addressed across 11 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Sorry, something went wrong.
There was a problem hiding this comment.
Beyond the inline nits, two candidate concerns were examined and ruled out: check_registration_usable accepting private_key_jwt (correct — PrivateKeyJWTOAuthProvider supplies the assertion, and the recognized set is derived from the same TokenEndpointAuthMethod literal the SDK sends, so the refresh path a private-key-JWT client inherits keeps working), and an empty-string client_id bypassing the new client_id-REQUIRED invariant on the record (not a practical issue — "" still parses but downstream guards treat a falsy client_id as absent, and no observed server mints an empty id).
Extended reasoning...Bugs were found (three nits, posted inline), so the review body is limited to recording what else was examined this run. The two refuted candidates both target the new tolerance/usability mechanism in src/mcp/client/auth/oauth2.py and src/mcp/shared/auth.py; verifiers concluded neither is a real defect. This note is informational only — it is not an approval, and the PR (a breaking change across OAuth client/server auth code) should still get human review.
Sorry, something went wrong.
There was a problem hiding this comment.
All reported issues were addressed across 10 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Sorry, something went wrong.
|
Follow-up in 29a3942 addressing the review round; the six inline threads are replied to and resolved individually. Two further items came in without a thread, so recording them here: Reported: required client_id makes pyright fail with ~20 always-true is not None errors. This does not reproduce — uv run --frozen pyright over the whole repo on this branch reports 0 errors, 0 warnings, and the pre-commit pyright hook passes on both commits. The 22 assert … client_id is not None sites do exist and are now dead narrowing, but strict pyright here does not flag them, so no check fails. Cleaning them would touch roughly eight unrelated test/example files for no behaviour change, so they are left as-is; happy to trim them in a follow-up if wanted. Reported: the SDK-internal issuer binding (SEP-2352) was populatable from the untrusted response body. Confirmed — handle_registration_response parsed the wire body straight into the record, so a server-echoed "issuer" member could populate the field, and on the fallback branch where the flow does not overwrite it, that value would be persisted as the trust binding (after which every 401 mismatches and re-registers). Pre-existing rather than introduced here, but this PR is the redesign of exactly that parse boundary, so it is now sealed: issuer is cleared right after the parse — the binding is stamped by the flow and never taken from the wire — with a test that an echoed "issuer" does not seed it. Verified against a server echoing a bogus issuer: the stored binding is the flow’s own. Also folded in from the inline threads: private_key_jwt is now rejected at registration by the authorization-code flow (it holds no key to sign the assertion) while stored records still tolerate it; an explicit null in a registration response is read as an omitted key across the record, closing the grant_types/response_types gap; the bundled server emits client_secret_expires_at: 0 per RFC 7591 §3.2.1 instead of omitting it; and the migration entry now names both exception surfaces. One item was declined as out of scope (defaulting an omitted auth method to client_secret_basic — pre-existing and unchanged here, and a semantic change that wants its own discussion). |
Sorry, something went wrong.
There was a problem hiding this comment.
Beyond this run's inline notes, two other candidates were examined and ruled out: (1) widening the record's token_endpoint_auth_method to str does not break client_auth.py — its unsupported-method branch raises a handled AuthenticationError (client_auth.py:94-97), not an asserted-unreachable, and SDK-registered records are still built from the strict request model; (2) the record's now-required client_id accepting "" is not a regression — pre-PR the field was optional and an absent client_id parsed as success, so requiring it strictly narrows what passes.
Extended reasoning...Bugs were found and posted inline, so no approval or full deferral. This note records two candidate issues that were examined and refuted this run, so later review passes don't re-explore them from scratch: the widened auth-method type on the record versus the bundled AS middleware's unsupported-method branch (verified to be an explicit, handled error path), and the empty-string client_id parse (verified to be a strict improvement over the previous optional field). Informational only — not a guarantee of correctness.
Sorry, something went wrong.
There was a problem hiding this comment.
No new issues found in this pass over 884bf91 — all items from the previous review rounds are addressed — but this is a breaking change to the public auth models and a rewrite of the parse boundary for untrusted registration responses, so it warrants a maintainer's review rather than an auto-approval.
What was reviewed this pass:
- the pre-validation issuer drop in handle_registration_response — the except ValueError covers both from_json and ValidationError layers without over-catching
- the widened placeholder stripper against non-string members — client_secret_expires_at: 0 and other falsy-but-meaningful values survive (0 != "")
- removal of the client_id truthiness guards in prepare_token_auth — safe now that the record requires client_id and the placeholder rule rejects an empty one
- the new secret-not-issued gate — a "" client_secret reads as absent via the placeholder rule, so it correctly trips the gate before the browser round-trip
Overview
PR #3181 splits OAuthClientInformationFull (the registered-client record parsed from an untrusted DCR response) from OAuthClientMetadata (the strict registration request), making them siblings over OAuthClientMetadataBase. It touches src/mcp/shared/auth.py, the client auth flow (oauth2.py, utils.py), the bundled server registration handler, docs (migration.md, oauth-clients.md), and adds substantial model/interaction test coverage. Head commit 884bf91 is the third revision, addressing two prior review rounds from me and one finding from cubic.
Security risks
This is squarely security-sensitive territory: it changes how the client parses an untrusted authorization-server response, how the SEP-2352 issuer trust binding is populated (now sealed — dropped from the wire body before validation rather than parsed-then-cleared), which token-endpoint auth methods the flow will act on, and the bundled AS's registration echo. The direction of each change is defensive — parse-tolerant on cosmetic metadata, but client_id is now required, an empty client_id is rejected, unusable credential assignments fail closed before persistence or any browser round-trip, and the server refuses private_key_jwt registrations it could never authenticate. I found no injection, bypass, or exposure vector in this pass; the earlier issuer-seeding concern is resolved at the correct layer.
Level of scrutiny
High — production client-auth code plus an intentional breaking API change (documented in docs/migration.md per repo policy). The design decisions here (sibling split, parse-tolerant record, registration-time usability gate, required client_id, the deliberately-deferred server-record/client-response separation named in the PR description) are exactly the kind of API-shape calls the guidelines say a human maintainer should own, regardless of correctness. That, not any outstanding defect, is why this defers rather than approves.
Other factors
All prior review threads are resolved with code changes rather than pushback (one item — defaulting an omitted auth method to client_secret_basic — was reasonably declined as an out-of-scope semantic change). Test coverage is strong: model-level parametrization over placeholder styles, real handle_registration_response paths including malformed/non-object/non-UTF-8 bodies, end-to-end interaction tests asserting no authorize/token traffic and nothing persisted on the unusable paths, and server-side 201-body assertions where the earlier omission actually lived. The repo requires 100% coverage in CI, which backstops the branch-level claims.
Sorry, something went wrong.
|
Second review round addressed in 884bf91; the four new inline threads are replied to and resolved individually. Three further reported items had no thread, so recording them here: Reported (again): required client_id makes pyright fail with ~20 always-true is not None errors. Still does not reproduce, and this time there is external evidence: on this branch pyright reports 0 errors, 0 warnings locally, and CI is fully green — checks / pre-commit (which runs pyright), all-green, and the whole test matrix all pass on the pushed commit. The 22 assert ... client_id is not None sites are real but this configuration does not flag them, so no check fails and there is nothing to fix here. Reported: stale prose about auth-method tolerance. Both bits valid and fixed. The comment, test docstring, and migration line no longer claim the private_key_jwt refresh path "keeps working" as such: PrivateKeyJWTOAuthProvider signs its assertion only in the client-credentials exchange, so the tolerance's real job is to keep prepare_token_auth from raising out of the refresh step — a refresh the server rejects then falls back to a fresh, signed exchange. The stale Raises: text in prepare_token_auth now says it fires only for stored/pre-registered records, the registration-time judgment being check_registration_usable. Reported: bundled AS confirms private_key_jwt registrations it can never authenticate. Valid and fixed rather than deferred: the registration endpoint now answers token_endpoint_auth_method: private_key_jwt with 400 invalid_client_metadata. This server authenticates token requests with the secret it mints and holds no client key to verify an assertion, so confirming that method would register a client whose every request it then rejects — one this SDK's own client would immediately declare unusable, orphaning the record. Interaction test covers the refusal; a client_secret_basic control still registers. Two more from the inline threads worth naming in one place: an SDK-internal issuer is now dropped from the untrusted body before validation (never sourced from the wire, and a malformed value can no longer fail the parse), and both server placeholder styles (null and "") now read as omitted keys across the record, which also makes an empty client_id an ordinary missing-client_id rejection. One item remains deliberately out of scope — treating an omitted token_endpoint_auth_method as RFC 7591's client_secret_basic default. Two reviewers have now raised it; it is pre-existing and byte-identical to main, and inferring a method the server did not state is a real behavioural change with its own breakage surface, so it will get its own issue and discussion rather than riding on this change. |
Sorry, something went wrong.
OAuthClientInformationFull, the client's parse of the authorization server's Dynamic Client Registration response, inherited from OAuthClientMetadata, the request the client sends. That typed the response as though it had to be a request this SDK would send. RFC 7591 3.2.1 says otherwise: the server may reject or replace any requested metadata value, and real servers echo an application_type outside OIDC Registration's web/native, an explicit null, an auth method the SDK does not implement, or an empty redirect_uris. Each raised ValidationError on a 2xx response, after the server had already provisioned the client, so the registration was discarded and orphaned. Make the two models siblings over a shared OAuthClientMetadataBase. The request keeps its strict types, so the SDK still refuses to send an unregistered application_type. The record accepts what a server may echo: application_type and token_endpoint_auth_method are str | None (with an echoed "" read as absent, as the optional URL fields already were), grant_types is list[str], and redirect_uris may be absent or empty. client_id is now required, as RFC 7591 3.2.1 makes it in the response. Whether a substituted value is usable is judged where it matters, not at parse: an auth method the client cannot apply is reported as an OAuthRegistrationError when the registration completes, before the record is stored or any interactive authorization begins, and prepare_token_auth reports the same for a stored record. The recognized set is derived from the one TokenEndpointAuthMethod type so the two cannot drift. The bundled registration endpoint now returns all registered metadata in its 201 response, building the record from the validated request's dump so a field can no longer be silently dropped from the echo; it previously omitted application_type, reporting the default in place of a client's "web".
…boundary Two conflated questions shared one recognized-method set: whether the authorization-code registration flow can act on an assigned method, and whether a stored record may carry a method without the token step erroring. The flow authenticates with the minted secret and holds no key to sign a private_key_jwt assertion, so it now rejects that method at registration too, while prepare_token_auth still tolerates it for PrivateKeyJWTOAuthProvider's inherited refresh path. An explicit JSON null in the registration response is now read as an omitted key across the whole record, so grant_types and response_types (which pydantic rejects null for) parse like the other fields. The SDK-internal issuer binding is cleared after parsing, so a body echoing an "issuer" member cannot seed it. The bundled registration endpoint sets client_secret_expires_at to 0 rather than omitting it whenever a client_secret is issued with no expiry configured, as RFC 7591 3.2.1 requires. The migration entry now names both exception surfaces.
An SDK-internal field must never be sourced from the wire, not sourced then cleared: the registration response body is now parsed with issuer dropped before validation, so an echoed member neither seeds the SEP-2352 binding nor fails the parse when malformed. The parse goes through pydantic's JSON layer so malformed bytes still surface as OAuthRegistrationError. Both placeholder styles servers use for unset members - null, and "" - now read as omitted keys across the whole record, closing the gap for list fields and retiring the field-by-field empty-string coercions. An empty client_id is accordingly no client_id, so the unrepresentable empty-identifier record and its dead guards in prepare_token_auth go. check_registration_usable now also rejects a secret-based method for which the server issued no client_secret, before persistence and before any interactive authorization, instead of letting the token request go out unauthenticated after the browser round-trip. The bundled registration endpoint refuses private_key_jwt with 400 invalid_client_metadata: it authenticates token requests with the secret it mints and holds no client key to verify an assertion, so confirming the method would register a client it could never authenticate - which this SDK's own client would then immediately declare unusable, orphaning the record. Correct prose that overstated the private_key_jwt refresh path (the tolerance keeps prepare_token_auth from raising so a rejected refresh can fall back to a fresh, signed client-credentials exchange), the stale Raises text in prepare_token_auth, and the client failure-model page, which described OAuthRegistrationError as only a refusal.
There was a problem hiding this comment.
No new findings this pass — the latest revision addresses everything raised in the two earlier inline-review rounds — but this is a breaking change to the OAuth model hierarchy and client auth flow, so it should get a human sign-off rather than a bot approval.
What was reviewed this pass:
- The placeholder-as-omitted model validator now covers both null and "" uniformly, including list-typed fields, and an empty client_id correctly fails as a missing client_id.
- check_registration_usable now rejects all three unusable-registration shapes (unimplemented method, private_key_jwt, secret-based method with no secret issued) before the record is persisted, matching its docstring.
- issuer is dropped from the untrusted body before validation, so an echoed value can neither seed the SEP-2352 binding nor fail the parse; the bundled AS's model_dump()-based 201 echo round-trips through the record without losing fields (pinned by the field-superset test).
Overview
This PR splits OAuthClientInformationFull (the client's parse of a Dynamic Client Registration response, and the SDK's own AS's stored-client type) from OAuthClientMetadata (the registration request) into siblings over a shared base in src/mcp/shared/auth.py. It adds a registration-usability gate (check_registration_usable) and an unknown-method check in prepare_token_auth in src/mcp/client/auth/oauth2.py, hardens handle_registration_response (issuer stripped pre-validation, ValueError chain), and rebuilds the bundled registration endpoint's 201 echo from the validated request's dump, refusing private_key_jwt registrations. Docs (docs/migration.md, docs/client/oauth-clients.md) and an extensive test surface (model-level, handler-level, end-to-end interaction) are updated in the same PR.
Security risks
The changed code is squarely security-sensitive: it defines how an untrusted authorization-server response is parsed, which server-assigned credentials the client will act on, and what the SDK's own AS echoes at registration. The main risks were examined across the two review rounds and this pass: the SEP-2352 issuer trust binding is now never sourced from the wire (dropped before validation, with tests for string and non-string echoes); the tolerant record cannot parse a success without a client_id (required per RFC 7591 §3.2.1, and an empty-string client_id reads as missing); the request model stays strict, so nothing this SDK sends is widened; and unusable server-assigned auth methods fail closed with OAuthRegistrationError before persistence or any authorize/token traffic (asserted end-to-end). The relaxations are confined to parse-side tolerance of fields the flow either does not use (application_type) or judges explicitly at use sites.
Level of scrutiny
This warrants the highest scrutiny tier: it is a deliberate breaking change to public auth types on the v2 branch, it alters the parse boundary for untrusted input, and the record type still serves double duty as the server-side stored-client type — a design seam the author explicitly names as deliberately unfinished. Those are exactly the kinds of API-design and trust-boundary decisions a maintainer should weigh in on, independent of correctness. Bot approval is inappropriate regardless of how clean this pass came back.
Other factors
Two earlier review rounds (mine and cubic's) surfaced eight distinct issues; every one was either fixed with a targeted commit and a pinned test (null/"" placeholder generalization, secret-not-issued gate, issuer-before-validation, RFC 7591 client_secret_expires_at: 0, private_key_jwt refusals on both client and server side, doc corrections) or explicitly declined with a defensible scope argument (defaulting an omitted auth method to client_secret_basic — pre-existing behavior, deferred to its own issue). All inline threads are resolved, this run's bug hunting found nothing new, and I re-verified the four claims in the message against the current code. Coverage is 100% per repo policy and the interaction tests exercise the new failure modes over real HTTP. The remaining call is design judgment, not bug risk — hence defer rather than approve.
Sorry, something went wrong.
OAuthTokenError is no longer only the token endpoint's rejection: a stored client record whose token_endpoint_auth_method this client cannot apply raises it while the token request is being built, before the endpoint is contacted. Say so, parallel to the OAuthRegistrationError half of the same sentence. No-Verification-Needed: doc-only change
|
Third pass. Two items: Reported (fourth time): required client_id makes pyright fail with ~20 always-true is not None errors. This still does not reproduce, and now on the exact commit named (40fe0463): checks / pre-commit, which runs pyright, is green on the rebased head, as is all-green. The reason it never reproduces is that the premise is wrong about pyright, not about this repo's config: strict mode (typeCheckingMode = "strict", no relevant overrides here) does not report assert x is not None on an already-non-optional value, so those sites produce no diagnostic under strict at all. Nothing to fix. Given this is the fourth appearance of a claim contradicted by a passing check each time, please treat it as settled. Reported: the OAuthTokenError half of the failure-model sentence is stale. Valid — the OAuthRegistrationError half was fixed and its sibling left behind. Fixed in cc4e911: the client OAuth page now says OAuthTokenError means a token could not be obtained — the endpoint said no, or a stored client record carries an authentication method this client cannot apply, reported while building the token request rather than sent — so a reader is not pointed at a server-side rejection that never happened. |
Sorry, something went wrong.
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/client/oauth-clients.md">
<violation number="1" location="docs/client/oauth-clients.md:134">
P2: Stored `private_key_jwt` records and secret-based records missing `client_secret` are not rejected while building the token request; they fall through and yield an unauthenticated request. Narrow this to unrecognized methods, or document the other cases separately.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Sorry, something went wrong.
| ## When it fails | ||
|
|
||
| When the OAuth flow goes wrong, the provider raises an `OAuthFlowError` from `mcp.client.auth`. It has two subclasses. `OAuthRegistrationError` means the authorization server refused to register you. `OAuthTokenError` means the token endpoint said no. One `except OAuthFlowError:` covers discovery, registration, authorization, and exchange. | ||
| When the OAuth flow goes wrong, the provider raises an `OAuthFlowError` from `mcp.client.auth`. It has two subclasses. `OAuthRegistrationError` means registration did not yield a client you can use: the authorization server refused to register you, or it did register you but with credentials this flow cannot use (for instance an authentication method it does not implement). `OAuthTokenError` means a token could not be obtained: the token endpoint said no, or a stored client record carries an authentication method this client cannot apply, which is reported while building the token request rather than sent. One `except OAuthFlowError:` covers discovery, registration, authorization, and exchange. |
There was a problem hiding this comment.
P2: Stored private_key_jwt records and secret-based records missing client_secret are not rejected while building the token request; they fall through and yield an unauthenticated request. Narrow this to unrecognized methods, or document the other cases separately.
Prompt for AI agentsCheck if this issue is valid — if so, understand the root cause and fix it. At docs/client/oauth-clients.md, line 134:
<comment>Stored `private_key_jwt` records and secret-based records missing `client_secret` are not rejected while building the token request; they fall through and yield an unauthenticated request. Narrow this to unrecognized methods, or document the other cases separately.</comment>
<file context>
@@ -131,7 +131,7 @@ There is one more no-human situation: the client belongs to an enterprise whose
## When it fails
-When the OAuth flow goes wrong, the provider raises an `OAuthFlowError` from `mcp.client.auth`. It has two subclasses. `OAuthRegistrationError` means registration did not yield a client you can use: the authorization server refused to register you, or it did register you but with credentials this flow cannot use (for instance an authentication method it does not implement). `OAuthTokenError` means the token endpoint said no. One `except OAuthFlowError:` covers discovery, registration, authorization, and exchange.
+When the OAuth flow goes wrong, the provider raises an `OAuthFlowError` from `mcp.client.auth`. It has two subclasses. `OAuthRegistrationError` means registration did not yield a client you can use: the authorization server refused to register you, or it did register you but with credentials this flow cannot use (for instance an authentication method it does not implement). `OAuthTokenError` means a token could not be obtained: the token endpoint said no, or a stored client record carries an authentication method this client cannot apply, which is reported while building the token request rather than sent. One `except OAuthFlowError:` covers discovery, registration, authorization, and exchange.
Not everything is a flow error. The network can still fail; those are ordinary `httpx2` exceptions and pass through untouched.
</file context>
| When the OAuth flow goes wrong, the provider raises an `OAuthFlowError` from `mcp.client.auth`. It has two subclasses. `OAuthRegistrationError` means registration did not yield a client you can use: the authorization server refused to register you, or it did register you but with credentials this flow cannot use (for instance an authentication method it does not implement). `OAuthTokenError` means a token could not be obtained: the token endpoint said no, or a stored client record carries an authentication method this client cannot apply, which is reported while building the token request rather than sent. One `except OAuthFlowError:` covers discovery, registration, authorization, and exchange. | |
| When the OAuth flow goes wrong, the provider raises an `OAuthFlowError` from `mcp.client.auth`. It has two subclasses. `OAuthRegistrationError` means registration did not yield a client you can use: the authorization server refused to register you, or it did register you but with credentials this flow cannot use (for instance an authentication method it does not implement). `OAuthTokenError` means a token could not be obtained: the token endpoint said no, or a stored client record carries an unrecognized authentication method, which is reported while building the token request rather than sent. One `except OAuthFlowError:` covers discovery, registration, authorization, and exchange. |
Sorry, something went wrong.
There was a problem hiding this comment.
No new issues found in this pass over the revised code — the follow-up commits resolve all previously raised inline comments as described — but this is a large, breaking change to OAuth client/server auth models, so it still needs a human maintainer's sign-off.
What was reviewed this pass:
- The reworked check_registration_usable (all three unusable cases, including a secret-based method with no issued secret) and its interaction tests.
- The generalized placeholder rule in _placeholder_members_read_as_omitted — null and "" both read as omitted for scalar and list fields, and an empty client_id is now an ordinary missing-client_id rejection.
- The pre-validation issuer strip in handle_registration_response (never sourced from the wire; a malformed value cannot fail the parse) and the updated failure-model prose on the client OAuth page.
- Checked that every OAuthClientInformationFull construction site in src/ (CIMD helper, client-credentials, private-key-JWT, identity-assertion providers) supplies the now-required client_id — all do.
Overview
This PR splits OAuthClientInformationFull (the registered-client record parsed from a DCR response) from OAuthClientMetadata (the registration request) into siblings over a shared base, making the response parse tolerant of RFC 7591 §3.2.1 server substitutions while keeping the request strict. It also adds a registration-usability gate (check_registration_usable), seals the SDK-internal issuer binding against the untrusted response body, fixes the bundled AS's 201 echo (full metadata, client_secret_expires_at, private_key_jwt refusal), and documents the breaking changes in docs/migration.md. Twelve files across client auth, server auth handlers, shared models, docs, and tests.
Security risks
The changes are squarely in security-sensitive territory: OAuth client registration parsing, token-endpoint client authentication, and the SEP-2352 issuer trust binding. The direction of the changes is defensive — the issuer field is now dropped from the wire body before validation rather than parsed-then-cleared, unusable registrations fail before persistence or any browser round-trip, and the request side stays strict. Prior review rounds probed the loosened parse (placeholder handling, secret-not-issued, issuer seeding) and each gap found was closed with tests. Nothing new surfaced this pass. The residual risk is inherent to widening a parse boundary on untrusted input, which is exactly why a human should weigh the design.
Level of scrutiny
High. This is a breaking API change (isinstance relationships, required client_id, moved validators) in production auth code used by both the client flow and the SDK's bundled authorization server. Per the approval guidelines, security-sensitive auth code should not be shadow-approved regardless of how clean a single review pass is.
Other factors
Three review rounds (two from this reviewer, one from cubic) produced ten distinct findings; the author addressed or explicitly scoped out each one, with the one deferral (defaulting an omitted token_endpoint_auth_method to client_secret_basic) reasonably argued as pre-existing behavior deserving its own issue. Test coverage is strong: model-level parametrized tests, real handle_registration_response paths, end-to-end interaction tests asserting no authorize/token traffic and nothing persisted on unusable registrations, and a structural test pinning that every request field exists on the record. CI is green and coverage is at 100%. I verified in the working tree that all claimed fixes are present and that all in-tree OAuthClientInformationFull constructors supply client_id.
Sorry, something went wrong.
…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.
OAuthClientInformationFull — the client's parse of the authorization server's Dynamic Client Registration response — inherited from OAuthClientMetadata, the request the client sends. That typed the response as though it had to be a request this SDK would send, so any registered value the server substituted became a ValidationError on a 2xx. This splits the two into siblings so the record parses what a server may legitimately echo.
Motivation and Context
We received a report of an authorization server whose registration response returned an application_type value outside {"web", "native"}. The server answered 201 and provisioned the client; the SDK then raised Invalid registration response: … Input should be 'web' or 'native' [type=literal_error] out of the auth flow, discarding a registration whose client_id had already been minted (and orphaning it server-side on every retry). application_type is never read again anywhere in the SDK — it's a purely cosmetic field killing an otherwise-valid registration.
This isn't specific to that field. The response model is-a request model, so every tight type on the request re-arms the same failure on the response. The same closed-typing had already been widened piecemeal for *_supported metadata, grant_types, response_types, and empty-string URLs — this is the fifth instance of one class, and the class is the inheritance.
Spec. RFC 7591 §3.2.1: the server "MAY reject or replace any of the client's requested metadata values submitted during the registration and substitute them with suitable values", and "The client or developer can check the values in the response to determine if the registration is sufficient for use" — a substituted value is a judgment call for the client, not a parse failure. The MCP spec (SEP-837) constrains only what the client sends (MUST specify an appropriate application_type); it says nothing about the response. The server here is also off-spec — OIDC Registration §2 defines only native/web — but that doesn't oblige the client to abort a successful registration over a field it doesn't use. Both are true.
Cross-SDK. Every other MCP SDK constrains what it sends and parses application_type permissively — the TypeScript SDK types it z.string().optional() with a comment giving exactly this reasoning, go/rust/csharp/ruby likewise. This SDK was the only one that fails on it.
What changed
OAuthClientMetadata (request) and OAuthClientInformationFull (registered-client record) are now siblings over a shared OAuthClientMetadataBase holding only the fields that are genuinely identical between them:
How Has This Been Tested?
Breaking Changes
Yes — documented in docs/migration.md alongside the existing SEP-837 entry.
Conformance: no scenario puts these values in a 201 body, and SEP-837 defines no response-side requirement, so a send-strict / parse-tolerant client passes every existing check unchanged.
Types of changes
Checklist
Additional context
Two things I considered and want to name:
AI Disclaimer