Skip to content

Validate the iss authorization-response parameter (RFC 9207 / SEP-2468) - #2921

Merged
Kludex merged 4 commits into
mainfrom
sep-2468-validate-iss
Jun 20, 2026
Merged

Validate the iss authorization-response parameter (RFC 9207 / SEP-2468)#2921
Kludex merged 4 commits into
mainfrom
sep-2468-validate-iss

Conversation

@Kludex

@KludexKludex commented Jun 20, 2026

Copy link
Copy Markdown
Member

Implements the first item of #2902: validate the RFC 9207 iss authorization-response parameter (SEP-2468).

What changed

  • The OAuth client validates the iss query parameter returned on the authorization-response redirect, and the issuer of discovered authorization server metadata. Comparison is simple string comparison per RFC 3986 §6.2.1 (no normalization): a mismatched iss is rejected, and a missing iss is rejected when the AS advertised authorization_response_iss_parameter_supported.
  • callback_handler now returns AuthorizationCodeResult (code/state/iss) instead of tuple[str, str | None], so the redirect's iss reaches the validation. Breaking change — documented in docs/migration.md.
  • Added authorization_response_iss_parameter_supported to OAuthMetadata.

AnyHttpUrl appends a trailing slash to a bare authority (https://as → https://as/), which would defeat the byte-exact comparison the spec requires (iss-supported must accept https://host while iss-normalized must reject https://host/), so the recorded issuer is compared with that lone authority slash stripped.

Conformance

Flips auth/metadata-issuer-mismatch green (removed from both expected-failures baselines). The auth/iss-* scenarios validate iss correctly but stay baselined because they reach Dynamic Client Registration and trip the unimplemented SEP-837 application_type check — re-grouped under SEP-837 with accurate comments. Both conformance legs pass with no stale entries.

Out of scope (separate PRs, tracked by #2902)

SEP-837 (application_type), SEP-2350 (scope-union step-up), SEP-2352 (AS migration / credential binding). SEP-2207 (offline_access) is already done.

AI Disclaimer

This PR was developed with the assistance of either Claude or Codex. I've reviewed and verified the changes.

@Kludex
Validate the iss authorization-response parameter (RFC 9207 / SEP-2468)
9373eab
The OAuth client now validates the RFC 9207 iss parameter returned on the
authorization-response redirect and the issuer of discovered authorization
server metadata. iss is compared against the authorization server issuer with
simple string comparison (RFC 3986 6.2.1): a mismatch is rejected, and a
missing iss is rejected when the server advertised
authorization_response_iss_parameter_supported.
The callback_handler passed to OAuthClientProvider now returns an
AuthorizationCodeResult (code/state/iss) instead of a tuple[str, str | None]
so the redirect's iss reaches the validation. AnyHttpUrl appends a trailing
slash to a bare authority, which would defeat the byte-exact comparison the
spec requires, so the recorded issuer is compared with that lone slash stripped.
Flips the auth/metadata-issuer-mismatch conformance scenario green; the
auth/iss-* scenarios validate iss correctly but remain baselined because they
reach DCR and trip the unimplemented SEP-837 application_type check.
Kludex
Kludex commented Jun 20, 2026
View reviewed changes

@KludexKludex left a comment

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pointers to the substantive logic — the rest of the diff is mostly the mechanical tuple[str, str | None] → AuthorizationCodeResult rename across tests/examples.

Comment threadsrc/mcp/client/auth/utils.py
Comment threadsrc/mcp/client/auth/utils.py Outdated
Comment threadsrc/mcp/client/auth/utils.py
Comment threadsrc/mcp/client/auth/oauth2.py
Comment threadsrc/mcp/client/auth/oauth2.py
Comment threadsrc/mcp/client/auth/oauth2.py
Comment threadtests/interaction/auth/_provider.py Outdated
Comment threadtests/interaction/auth/test_discovery.py
Kludex
Kludex commented Jun 20, 2026
View reviewed changes
Comment thread.github/actions/conformance/client.py
claude[bot]
claudeBot reviewed Jun 20, 2026
View reviewed changes
Comment threaddocs/migration.md
Kludex
Kludex commented Jun 20, 2026
View reviewed changes
Comment threadexamples/clients/simple-auth-client/mcp_simple_auth_client/main.py Outdated
@Kludex
Address review: clarify iss migration note, make callback getters pro…
7eeee6c
…perties
- migration.md: spell out that omitting iss raises OAuthFlowError against
servers advertising authorization_response_iss_parameter_supported, rather
than merely disabling the check.
- simple-auth-client example: convert get_state/get_iss to @Property.
@KludexKludex mentioned this pull request Jun 20, 2026
Merged
@Kludex

Kludex commented Jun 20, 2026

Copy link
Copy Markdown
MemberAuthor

Prerequisite #2925 opened: it sets url_preserve_empty_path=True on the OAuth metadata models so issuers parsed from the wire keep their empty path. Once #2925 merges, I'll rebase this PR and drop the _strip_authority_trailing_slash workaround in favor of a direct comparison.

claude[bot]
claudeBot reviewed Jun 20, 2026
View reviewed changes
Comment threadsrc/mcp/client/auth/utils.py Outdated
Kludex added 2 commits June 20, 2026 17:35
@Kludex
Merge remote-tracking branch 'origin/main' into sep-2468-validate-iss
109c391
@Kludex
Drop iss slash-stripping workaround now that empty paths are preserved
15f11a5
With url_preserve_empty_path on the OAuth metadata models (#2925), the issuer
parsed from the wire keeps its empty path, so str(oauth_metadata.issuer) is
already the byte-exact value the authorization server transmitted. Remove
_strip_authority_trailing_slash / raw_issuer and compare directly.
This also fixes the false rejection the heuristic introduced for an issuer that
genuinely ends in a trailing slash (e.g. Auth0's https://tenant.auth0.com/):
its redirect iss now matches its advertised issuer instead of being stripped.
@Kludex

Kludex commented Jun 20, 2026

Copy link
Copy Markdown
MemberAuthor

Conformance scenarios this PR addresses (SEP-2468)

Run against @modelcontextprotocol/conformance@0.2.0-alpha.4. Every SEP-2468 check now passes:

ScenarioSEP-2468 checkResultScenario overall
auth/metadata-issuer-mismatchsep-2468-client-validate-metadata-issuer✅ SUCCESS✅ PASSES (removed from baseline)
auth/iss-supportedsep-2468-client-compare-iss-supported✅ SUCCESS⛔ blocked by SEP-837
auth/iss-not-advertisedsep-2468-client-proceed-no-iss✅ SUCCESS⛔ blocked by SEP-837
auth/iss-supported-missingsep-2468-client-reject-missing-iss✅ SUCCESS⛔ blocked by SEP-837
auth/iss-wrong-issuersep-2468-client-compare-iss-supported✅ SUCCESS⛔ blocked by SEP-837
auth/iss-unexpectedsep-2468-client-compare-iss-unadvertised✅ SUCCESS⛔ blocked by SEP-837
auth/iss-normalizedsep-2468-client-no-normalization✅ SUCCESS⛔ blocked by SEP-837

auth/metadata-issuer-mismatch is the only one that flips fully green here (it rejects before DCR), so it's removed from both expected-failures.yml and expected-failures.2026-07-28.yml.

The six auth/iss-* scenarios all run at spec version 2026-07-28 and reach Dynamic Client Registration, where the harness's sep-837-application-type-present check fires. Since the client doesn't yet send application_type (SEP-837, a separate item of #2902), each scenario fails overall on that check despite its SEP-2468 check passing. They stay baselined under SEP-837 with a comment noting iss validation is done; they'll flip green when SEP-837 lands.

Both conformance legs pass with no unexpected or stale baseline entries.

@Kludex
Kludex merged commit 48cf495 into mainJun 20, 2026
35 checks passed
@Kludex
Kludex deleted the sep-2468-validate-iss branch June 20, 2026 15:54
@KludexKludex mentioned this pull request Jun 20, 2026
Merged
@github-actions

github-actionsBot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

This pull request is included in pre-release v2.0.0a3

This was referenced Jun 26, 2026
Closed
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@claudeclaude[bot]claude[bot] left review comments
@maxisbeymaxisbeymaxisbey approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Kludex@maxisbey