SEP-2468: Recommend Issuer (iss) Parameter in MCP Auth Responses#2468
Conversation
Proposes requiring the inclusion and validation of an explicit issuer (iss) claim in MCP authorization responses to mitigate authorization mix-up attacks in multi-IdP environments.
This comment was marked as spam.
This comment was marked as spam.
|
I don't think we should bring in any new dependencies in this PR other than the |
This comment was marked as spam.
This comment was marked as spam.
This comment was marked as spam.
This comment was marked as spam.
SamMorrowDrums
left a comment
There was a problem hiding this comment.
Looks like a great addition. Added a couple of nits but should also add a link to the best practices documentation: https://modelcontextprotocol.io/docs/tutorials/security/security_best_practices
Specifically the confused deputy section. Then you can also update the guidance based on this being merged.
Adds 5 draft conformance scenarios testing RFC 9207 issuer parameter validation in OAuth authorization responses: - auth/iss-supported: server advertises support and sends correct iss - auth/iss-not-advertised: server omits iss parameter entirely - auth/iss-supported-missing: client must reject missing iss when required - auth/iss-wrong-issuer: client must reject mismatched iss value - auth/iss-unexpected: client must reject iss when not advertised Also adds auth-test-iss-validation.ts, a reference client that correctly validates iss per RFC 9207, and negative tests confirming the standard client fails all three rejection scenarios. TODO: Update RFC_9207_ISS_PARAMETER spec reference once SEP-2468 (modelcontextprotocol/modelcontextprotocol#2468) is merged.
State Transition: proposal → draftThis SEP has been transitioned from proposal to draft. @pcarleton has been assigned as the sponsor for this SEP. This is an automated message from the SEP lifecycle bot. |
Apply prettier, fix heading syntax, remove template note, and set SEP number, sponsor, and PR link.
| | ------------------------------------------------ | ----------------- | ------------------------------------------------------------------------------------------ | | ||
| | `true` | present | Compare to the recorded issuer using simple string comparison ([RFC3986 Section 6.2.1][1]) | | ||
| | `true` | absent | Reject the response | | ||
| | `false` or absent | present | Compare to the recorded issuer using simple string comparison ([RFC3986 Section 6.2.1][1]) | |
There was a problem hiding this comment.
This isn't the behavior recommended in 9207
Clients SHOULD discard authorization responses with the iss parameter from authorization servers that do not indicate their support for the parameter.
Do we have a strong reason for deviating from the SHOULD?
There was a problem hiding this comment.
I see it as filling in the noted "out of scope guidance" listed below that sentence:
Clients SHOULD discard authorization responses with the iss parameter from authorization servers that do not indicate their support for the parameter. However, there might be legitimate authorization servers that provide the iss parameter without indicating their support in their metadata. Local policy or configuration can determine whether to accept such responses, and specific guidance is out of scope for this specification.
In the experience working with folks AS metadata via PRM and OASM, changing those files can be finicky and slow where I think adding the iss parameter can be faster. I only see this "failing closed", i.e. causing more servers to be rejected than otherwise, so this seems to be only a tightening and means we can get the protection in more places more quickly by only requiring 1 change (the parameter) rather than 2 (parameter + metadata).
I haven't dug deeply into the history of that SHOULD though, so it's worth us seeing if we can find the reasoning there.
There was a problem hiding this comment.
dug into this and I'm now confident the deviation is net positive, but you're right we should be explicit about it. I've updated the PR and SEP to call it out:
- The truth table now has a note that the third row is invoking the local-policy provision §2.4 explicitly leaves open ("specific guidance is out of scope")
- The SEP's "alternatives considered" section now documents why we're not following the SHOULD literally
The short version of the digging: the SHOULD-discard sentence was a late, lightly-debated addition to RFC 9207. it doesn't appear in the original individual draft (which left this to local policy explicitly) and shows up in WG -01 as an unattributed "incorporated WG feedback" change. The one thing the security argument genuinely depends on is that the recorded issuer always comes from a trusted source, which we get for free since RFC 9728 PRM + RFC 8414 metadata validation is already required upstream in the spec.
Other notes:
- FAPI 2.0 (final, Feb 2025) drops the metadata-conditional logic entirely and makes iss validation an unconditional shall on both sides — same posture we're taking
- panva's oauth4webapi / openid-client (the library NextAuth and much of the JS ecosystem use) already implements this rule (i.e. process parameter regardless of metadata presence). This came up with Github's
issrollout: https://github.com/orgs/community/discussions/192143 -- specifically some configs of that library didn't fetch metadata and requiredissuerto be passed in to validate, which caused problems. That's the same issue noted in the backcompat section of the SEP (i.e. if you upgrade, but don't plumbissuerthrough from your metadata, you'll have an issue)
…2.4 carve-out - Adds the §3.3 issuer/well-known-URL match MUST to Authorization Server Metadata Discovery, with a normative rejection example. This is the trusted-baseline prerequisite that makes iss validation meaningful. - Response Validation now references the validated metadata and notes the validation provides no protection without it. - Replaces the row-3 leniency note with the actual justification: §2.4's explicit local-policy carve-out, the §3.3-validated baseline, and FAPI 2.0 §5.3.3.2 which makes client validation an unconditional shall. - Drops "2.0" from "OAuth mix-up attacks" in the SEP Motivation.
Spec note is now a single declarative sentence citing the §2.4 local-policy provision. The fuller reasoning (§3.3-validated baseline, fail-closed property, metadata-lag accommodation) lives in the SEP's Alternatives Considered where verbose justification is appropriate.
localden
left a comment
There was a problem hiding this comment.
Love the effort on this - thank you for putting it together!
| @@ -0,0 +1,86 @@ | |||
| # SEP-2468: Recommend Issuer (iss) Parameter in MCP Auth Responses | |||
There was a problem hiding this comment.
nit (non-blocking): the file slug still says claim after the parameter rename and becomes the published docs URL. If we want it consistent, rename to 2468-recommend-issuer-parameter-for-auth.md and re-run npm run generate:seps. Existing SEP slugs are not curated, so also fine to leave as-is.
Co-authored-by: Den Delimarsky <53200638+localden@users.noreply.github.com> Co-authored-by: Paul Carleton <paulcarletonjr@gmail.com>
Co-authored-by: Paul Carleton <paulcarletonjr@gmail.com>
…07 §2.3 Also normalizes CRLF→LF and trailing whitespace introduced by the GitHub-UI suggestion commits; the semantic diff is line 496 only.
|
This SEP was reviewed by the Core Maintainers and the decision was to Accept. |
Reference Implementation ReminderHi @EmLauber! This SEP was accepted 40 days ago. A reminder that accepted SEPs should have a reference implementation to move to final status.
Let us know the current status! This is an automated message from the SEP lifecycle bot. |
Regenerated docs/seps/index.mdx to resolve the conflict from new SEPs landing on main. :house: Remote-Dev: homespace
|
/lgtm |
The auto-merge left the SEP nav groups in the wrong order; render-seps emits In-Review before Draft. :house: Remote-Dev: homespace
|
/lgtm |
Proposes requiring the inclusion and validation of an explicit issuer (iss) claim in MCP authorization responses to mitigate authorization mix-up attacks in multi-IdP environments.
Motivation and Context
How Has This Been Tested?
Tested in OAuth scenarios. Will need additional testing specific to MCP environments before accepting and merging.
Breaking Changes
It is additive fro security. Clients will need to update code to validate the issuer parameter.
Types of changes
Checklist
Additional context
Some auth servers already use the issuer claim and can reference those examples once more details are added.