feat(client,core): RFC 9207 iss parameter validation on authorization responses (SEP-2468)

[mattzcarey]

What

Implements RFC 9207 (OAuth 2.0 Authorization Server Issuer Identification) iss parameter validation for authorization responses, per the draft MCP spec (basic/authorization, "Authorization Response Validation").

Before redirecting the user, the client records the issuer from validated authorization server metadata alongside the PKCE verifier/state. Before sending an authorization code to any token endpoint, callers that provide authorization-response parameters can apply RFC 9207 Section 2.4 validation.

Decision table implemented by validateAuthorizationResponseIssuer():

AS metadata advertises authorization_response_iss_parameter_supported: true	Response iss	Outcome
yes	absent (caller passed iss: null)	Reject
yes or no	present	Exact string compare against recorded issuer (no normalization); mismatch rejects
no	absent	Proceed
-	on rejection	The client MUST NOT act on error/error_description from that response either

One extra fail-closed row: if an iss is received but no AS metadata was ever recorded, we reject because there is nothing trustworthy to compare against.

The iss option is tri-state (string | null | undefined)

The SDK never sees the authorization response itself; the caller does. So the strict advertised-but-missing rejection is gated behind an explicit caller signal:

• string - the iss from the authorization response; validated by exact comparison.
• null - the caller asserts it inspected the authorization response and it contained no iss. This enables the RFC 9207 fail-closed rejection when the AS advertises authorization_response_iss_parameter_supported: true.
• undefined (omitted) - the caller did not have access to the response parameters, matching existing finishAuth(code) callers. Validation is skipped because the SDK cannot distinguish "the response had no iss" from "the caller did not plumb it."

This keeps existing callers compiling and behaving identically while giving hosts that have the callback URL enough API surface to enforce RFC 9207.

API additions

• @modelcontextprotocol/core: OAuthMetadataSchema and OpenIdProviderMetadataSchema recognize authorization_response_iss_parameter_supported.
• @modelcontextprotocol/client:
  • New export validateAuthorizationResponseIssuer(metadata, iss).
  • auth(provider, { ..., iss? }) validates iss before fetchToken when authorizationCode is present and iss is provided (string or null).
  • StreamableHTTPClientTransport.finishAuth(authorizationCode, options?: { iss?: string | null }) and SSEClientTransport.finishAuth(...) plumb the optional value through to auth().

Metadata provenance

Validation strength depends on where the recorded issuer comes from. With a provider that implements discoveryState/saveDiscoveryState, the recorded issuer is the one from the provider's validated AS metadata captured before the redirect, which is the intended RFC 9207 anchor. Without cached discovery state, authInternal re-discovers metadata at code-exchange time, so the comparison anchor is the freshly-discovered issuer rather than the one recorded pre-redirect. That still ensures the iss must match the AS the client is about to use, but recording via discoveryState is stronger.

Land order / conformance note

This branch is rebased on origin/main and is not stacked. Because current conformance includes SEP-837 checks, the full client conformance baseline on this branch still depends on #2266 landing first; the observed local failure is DCR application_type specified, not an iss regression. Recommended order: #2265 -> #2266 -> #2271 -> this PR.

The auth/iss-* conformance scenarios remain in the expected-failures baseline because the conformance everythingClient does not yet plumb the redirect's iss through finishAuth.

Validation

After rebasing on current origin/main:

• pnpm --filter @modelcontextprotocol/client test -- auth.test.ts streamableHttp.test.ts sse.test.ts tokenProvider.test.ts - 388 passed
• pnpm --filter @modelcontextprotocol/core test - 477 passed
• pnpm run typecheck:all - clean
• pnpm run lint:all - clean
• pnpm run build:all - clean, with the existing AJV/fast-uri missing-export warnings
• pnpm --filter @modelcontextprotocol/test-conformance exec conformance client --command 'node --import tsx ./src/everythingClient.ts' --scenario auth/metadata-default --expected-failures ./expected-failures.yaml --verbose - fails only on SEP-837 application_type omission, fixed by feat(client,core): application_type client metadata with native/web inference (SEP-837) #2266

Downstream impact

cloudflare/agents calls transport.finishAuth(code) in packages/agents/src/mcp/client-connection.ts, so it keeps compiling and behaving identically through the undefined path. Agents will not get RFC 9207 protection until it extracts iss from the OAuth callback URL in packages/agents/src/mcp/client.ts and forwards it through completeAuthorization() / finishAuth().

Closes #2197

[changeset-bot]

🦋 Changeset detected

Latest commit: 6567674

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@modelcontextprotocol/core	Patch
@modelcontextprotocol/client	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[pkg-pr-new]

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/@modelcontextprotocol/client@2272

@modelcontextprotocol/codemod

npm i https://pkg.pr.new/@modelcontextprotocol/codemod@2272

@modelcontextprotocol/core

npm i https://pkg.pr.new/@modelcontextprotocol/core@2272

@modelcontextprotocol/server

npm i https://pkg.pr.new/@modelcontextprotocol/server@2272

@modelcontextprotocol/server-legacy

npm i https://pkg.pr.new/@modelcontextprotocol/server-legacy@2272

@modelcontextprotocol/express

npm i https://pkg.pr.new/@modelcontextprotocol/express@2272

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/@modelcontextprotocol/fastify@2272

@modelcontextprotocol/hono

npm i https://pkg.pr.new/@modelcontextprotocol/hono@2272

@modelcontextprotocol/node

npm i https://pkg.pr.new/@modelcontextprotocol/node@2272

commit: 6567674

[mattzcarey]

Rebased on current origin/main, pushed a625f46b, updated the PR body out of draft wording, and marked ready for review.

I checked the implementation against the current upstream spec text for Authorization Response Validation: present iss is compared exactly, advertised-but-missing iss rejects when the caller explicitly passes null, and mismatched issuer errors do not surface forged authorization-response error content.

Verification:

• pnpm --filter @modelcontextprotocol/client test -- auth.test.ts streamableHttp.test.ts sse.test.ts tokenProvider.test.ts (388 passed)
• pnpm --filter @modelcontextprotocol/core test (477 passed)
• pnpm run typecheck:all
• pnpm run lint:all
• pnpm run build:all
• pre-push hook (build, typecheck, lint)

Conformance note: full client conformance is currently blocked on sibling PR #2266 / SEP-837. A verbose auth/metadata-default run fails only on DCR application_type specified; the iss scenarios remain in the expected-failures baseline because everythingClient does not yet plumb callback iss into finishAuth.

[mattzcarey]

Resolved the latest review threads in ede900a.

Changes:

• Updated docs/client.md to show extracting callback iss and calling finishAuth(code, { iss }), including null vs undefined behavior.
• Updated simpleOAuthClient and elicitationUrlExample to forward the callback iss into finishAuth.
• Added SSE finishAuth coverage for mismatched iss, asserting rejection before token exchange.

Verification:

• pnpm --filter @modelcontextprotocol/client test -- sse.test.ts streamableHttp.test.ts auth.test.ts
• pnpm run typecheck:all
• pnpm run lint:all
• pnpm run build:all
• pre-push hook passed typecheck/build/lint

[claude]

Thanks — the docs/client.md prose, both runnable examples, and the SSE finishAuth test from my earlier comments are now in the diff, so I have nothing further to flag. Leaving final sign-off to a maintainer since this adds new public API (validateAuthorizationResponseIssuer, the iss option on auth()/finishAuth()) and changes security-relevant OAuth client behavior.

Extended reasoning...

Overview

This PR implements RFC 9207 (SEP-2468) iss-parameter validation for OAuth authorization responses. It adds authorization_response_iss_parameter_supported to the core metadata schemas, a new exported validateAuthorizationResponseIssuer() helper, an optional tri-state iss option on auth(), and an options?: { iss } second argument on both StreamableHTTPClientTransport.finishAuth and SSEClientTransport.finishAuth. Docs, both OAuth examples, and a changeset are updated, and there is substantial new test coverage in auth.test.ts, streamableHttp.test.ts, and sse.test.ts.

Status of prior feedback

My earlier review comments (missing prose docs/example updates, and missing SSE-side finishAuth iss test) are addressed in the current revision: docs/client.md now describes extracting and forwarding iss (including the null vs undefined semantics), simpleOAuthClient.ts and elicitationUrlExample.ts pass { iss } through, and sse.test.ts gained a mismatched-iss rejection test. The validation logic itself matches the RFC 9207 decision table, runs before the code reaches any token endpoint, and uses exact string comparison with no normalization; the mismatch error message does not echo attacker-controlled error_description content, and a dedicated test asserts that.

Security risks

This is security-positive code (mix-up attack mitigation), but it sits squarely in the OAuth client flow. The main design judgment calls a maintainer should own are: (1) the tri-state iss semantics where undefined silently skips validation — sound for backwards compatibility, but it means existing callers get no protection until they opt in; (2) the metadata-provenance caveat the author documents, where without cached discoveryState the comparison anchor is freshly re-discovered metadata rather than the pre-redirect issuer, which weakens the guarantee; (3) new public API surface that is hard to remove later.

Level of scrutiny

High — auth code, new exports across two packages, and a minor version bump for the client. The implementation looks correct and well-tested, but per the repo's own conventions (burden of proof on addition, security-sensitive paths) this should get a human maintainer's review rather than a shadow approval.

Other factors

The bug-hunting system found no bugs in this revision, the changeset text matches the implemented behavior, and the conformance/land-order caveats are clearly documented in the PR description. The cloudflare/agents downstream-compatibility analysis in the description is consistent with the undefined-skips-validation path.

[claude]

Thanks — the docs/examples and SSE test gaps from my earlier review are addressed in ede900a, and I have no further findings. Since this adds new public auth API surface (the tri-state iss option, a new export, and finishAuth signature changes) on a security-sensitive path, it should still get a human maintainer's review of the API design rather than an automated approval.

Extended reasoning...

Overview

This PR implements RFC 9207 (SEP-2468) iss parameter validation: a new validateAuthorizationResponseIssuer() export, an optional tri-state iss option on auth(), and an options?: { iss } second parameter on both StreamableHTTPClientTransport.finishAuth and SSEClientTransport.finishAuth. Core's OAuthMetadataSchema/OpenIdProviderMetadataSchema gain authorization_response_iss_parameter_supported. Docs (docs/client.md), both OAuth examples, a changeset, and substantial test coverage (auth.test.ts decision table, StreamableHTTP and SSE transport plumbing tests) are included.

Security risks

This is security-positive in intent (mix-up attack mitigation), but it sits squarely on the OAuth authorization-code exchange path. The validation logic itself looks correct against RFC 9207 Section 2.4 (exact string compare, fail-closed on advertised-but-missing only when the caller asserts null, rejection before any token-endpoint call, no surfacing of forged error params), and the undefined skip path preserves existing behavior for callers that never see the callback URL. The main residual risk is design-level rather than a bug: the protection is opt-in, and the metadata anchor is the freshly re-discovered issuer when no cached discovery state exists, which the PR description itself acknowledges as weaker than the pre-redirect anchor.

Level of scrutiny

High. This touches client auth code, adds three new public API surfaces (export, auth() option, transport method signature), and encodes an API-design decision (tri-state string | null | undefined) that a maintainer should explicitly endorse per the repo's minimalism / burden-of-proof principles. Per my approval criteria, security-sensitive auth changes and new public API design decisions should not be auto-approved regardless of implementation quality.

Other factors

The bug-hunting pass found no defects in the latest revision. My previous review's items (missing prose docs, examples not forwarding iss, missing SSE-side test) were all addressed in ede900a and the threads are resolved. Test coverage is thorough, including the negative paths and the no-normalization requirement. The changeset accurately describes the shipped behavior. Conformance baseline caveats are documented and depend on sibling PRs, which is a sequencing question for maintainers.

[claude]

Additional findings (outside current diff — PR may have been updated during review):

• 🔴 packages/client/src/client/auth.ts:736-739 — Discovery state persisted by an older SDK release via the legacy no-PRM fallback (where authorizationServerUrl = MCP origin but the metadata's issuer = the real IdP) now fails the new validateAuthorizationServerMetadataIssuer() check on the cached-state path of authInternal() with a plain Error, which the auth() recovery wrapper never handles — so every auth() call (including silent token refresh) fails permanently until the application manually calls invalidateCredentials('discovery'). Consider invalidating/ignoring the cached discovery state on this mismatch and falling through to full re-discovery (which would now succeed thanks to this PR's own validateIssuer: false + issuer-swap handling for the fallback path), or at minimum throwing a typed, recoverable error.

  Extended reasoning...

  What the bug is

  The cached-discovery-state branch of authInternal() (packages/client/src/client/auth.ts:736-739) now unconditionally calls validateAuthorizationServerMetadataIssuer(metadata, cachedState.authorizationServerUrl). That helper throws a plain Error when the cached authorizationServerUrl and the metadata's issuer differ (modulo trailing slash). The problem is that there is a legitimate, previously-valid configuration that produces exactly this mismatch in already-persisted state: discovery state written by an earlier SDK release via the legacy no-PRM fallback, where authorizationServerUrl was set to the MCP server origin while authorizationServerMetadata.issuer was the real IdP. The old SDK never validated or swapped these, so providers that implement saveDiscoveryState()/discoveryState() (cross-session persistence is the documented purpose of OAuthDiscoveryState) can have this state on disk today.

  Why this PR makes it inconsistent

  The PR itself acknowledges this configuration is legitimate on the fresh-discovery path: commit df9db5b makes discoverOAuthServerInfo() skip issuer validation when the AS URL came from the legacy fallback (validateIssuer: authorizationServerUrlFromResourceMetadata) and swaps authorizationServerUrl to metadata.issuer when they differ, with the test 'uses legacy fallback metadata issuer when it differs from the MCP origin' covering it. So a brand-new client against the same server works fine — but a client upgrading with state persisted by the previous release hits the cached-state check and fails.

  Why there is no recovery path

  The thrown value is a plain Error, not an OAuthError. The recovery wrapper in auth() only catches OAuthError with codes InvalidClient/UnauthorizedClient/InvalidGrant, in which case it calls provider.invalidateCredentials() and retries. For this error nothing is invalidated and there is no fall-through to fresh discovery. Since the cached state is consulted on every authInternal() call — 401-triggered re-auth, silent token refresh, finishAuth() — the failure repeats forever. The error message ('Authorization server metadata issuer does not match the expected issuer ... RFC 8414 Section 3.3') gives no hint that the remedy is for the host application to call invalidateCredentials('discovery').

  Step-by-step proof

  1. Under the previous SDK release, a provider connects to https://mcp.example.com which has no protected-resource metadata. The legacy fallback sets authorizationServerUrl = 'https://mcp.example.com/'; /.well-known/oauth-authorization-server there returns metadata with issuer: 'https://idp.example.com' (a proxied IdP). saveDiscoveryState() persists { authorizationServerUrl: 'https://mcp.example.com/', authorizationServerMetadata: { issuer: 'https://idp.example.com', ... } }. Auth completes; tokens with a refresh token are saved. This was fully supported behavior.
  2. The application upgrades to the SDK containing this PR.
  3. The access token expires; the transport gets a 401 and calls auth(). provider.discoveryState() returns the persisted state, so authInternal() takes the cached branch and calls validateAuthorizationServerMetadataIssuer({ issuer: 'https://idp.example.com', ... }, 'https://mcp.example.com/') → 'https://idp.example.com' !== 'https://mcp.example.com' → throws Error.
  4. auth()'s catch sees a non-OAuthError and rethrows. No credentials are invalidated, no re-discovery happens. Every subsequent auth() call replays steps 3-4. The connection is permanently broken even though the server configuration is one this PR explicitly continues to support, and even though dropping the cache and re-running discoverOAuthServerInfo() would succeed and persist corrected state (authorizationServerUrl = 'https://idp.example.com').

  Why existing handling doesn't cover it

  The new test 'rejects cached AS metadata with a mismatched issuer' shows the strict check is intentional for adversarial cached metadata, but it doesn't distinguish the stale-but-previously-valid legacy state, and there is no corresponding self-healing path. The earlier review comment on the changeset asked for documentation/gating of the unconditional RFC 8414 check; the fresh-discovery part of that concern was addressed by df9db5b, but this cached-state recovery gap remains.

  How to fix

  In the cached-state branch, when validateAuthorizationServerMetadataIssuer() would fail, treat the cached discovery state as stale: call provider.invalidateCredentials?.('discovery') (or simply ignore the cached state) and fall through to the full discoverOAuthServerInfo() path, which under this PR's own swap logic will resolve the correct issuer and re-persist consistent state. Alternatively (weaker), throw a typed/OAuthError-style error and document the migration step so applications can recover programmatically.

[claude]

Thanks for working through the earlier feedback — the latest commits (issuer-validation carve-outs for legacy/cached state, the Cross-App opt-out, the exported options type, and the doc/changeset/SKILL updates) address all of the issues I'd previously flagged, and I found no new bugs in this revision. I'm leaving final sign-off to a maintainer since this changes default OAuth discovery behavior (RFC 8414 issuer validation now on by default) in security-sensitive client auth code.

Extended reasoning...

Overview

This PR adds RFC 9207 iss parameter validation (SEP-2468) via a new validateAuthorizationResponseIssuer() export, an optional tri-state iss option on auth() and both transports' finishAuth(), plus schema recognition of authorization_response_iss_parameter_supported. Along the way it also introduces RFC 8414 Section 3.3 issuer validation of discovered authorization-server metadata (default-on, with a validateIssuer: false opt-out), legacy no-PRM fallback handling, cached-discovery-state revalidation, docs/migration/changeset updates, and example updates that forward the callback iss.

Status of prior feedback

All issues raised in earlier review rounds appear addressed in the current revision: the legacy no-PRM and cached-state paths now tolerate malformed/stale legacy issuers (with self-healing rediscovery), Cross-App Access IdP discovery opts out of issuer validation, DiscoverAuthorizationServerMetadataOptions is exported from the package index, the public helper's JSDoc documents the throw, and migration.md / migration-SKILL.md / the changeset describe the behavior change. The bug-hunting pass on this revision found no new issues, and the new behavior is well covered by tests (RFC 9207 decision table, transport plumbing for both StreamableHTTP and SSE, legacy fallback and stale-cache scenarios).

Security risks

The change is security-positive in intent (mix-up attack mitigation), but it sits squarely in client OAuth code: issuer comparison logic, discovery validation, and cached-state handling. The main residual risks are backwards-compat (deployments whose PRM-derived AS metadata issuer doesn't match the discovery URL will now hard-fail, which is documented but is a real behavior change) and the complexity of the layered legacy carve-outs, which a maintainer should sanity-check for both correctness and maintainability.

Level of scrutiny

High. This is a minor-version feature touching production auth paths with a default-on validation change affecting every auth() call, not just the new opt-in iss flow. That design trade-off (strict RFC 8414 validation by default vs. the carve-out complexity it required) is a maintainer decision, so this should not be auto-approved despite the clean bug-hunt result.

Other factors

Test coverage is thorough and the examples/docs were updated to demonstrate the secure path. Conformance for the auth/iss-* scenarios remains in the expected-failures baseline pending the conformance client plumbing, and the PR notes a land-order dependency on sibling PRs — both reasonable, but worth a maintainer's confirmation.