Skip to content

fix(server-legacy): serve the RFC 8414 issuer without a trailing slash - #3184

Open
Alter-Igor wants to merge 2 commits into
modelcontextprotocol:mainfrom
Alter-Igor:fix/oauth-metadata-issuer-trailing-slash
Open

Alter-Igor wants to merge 2 commits into
modelcontextprotocol:mainfrom
Alter-Igor:fix/oauth-metadata-issuer-trailing-slash

Conversation

@Alter-Igor

Copy link
Copy Markdown

Summary

createOAuthMetadata wrote issuer: issuerUrl.href, and new URL('https://host').href normalizes to https://host/ — so an authorization server constructed from a slash-less origin string advertised a slashed issuer.

RFC 8414 §2 forbids a trailing slash on the issuer identifier, and §3.3 requires the metadata issuer to be byte-identical to the URL the well-known document was fetched from. The practical break: an SDK server and an SDK client could not complete discovery against each other for root issuers — the client's own IssuerMismatchError validation (metadata echo check) throws because the discovery URL has no slash while the metadata does. Other strict RFC 8414 clients fail the same way; we hit this in production (Central Agent Service at agent.alterspective.com.au: strict clients could not connect any OAuth-protected MCP behind it).

The fix

Serve the issuer identifier slash-less: a small issuerIdentifierOf() strips the single trailing slash new URL() added, in packages/server-legacy/src/auth/router.ts. Both spellings of issuerUrl (with or without /) now produce the same compliant metadata. Endpoint URLs are unaffected — they resolve against the full issuerUrl as before. mcpAuthMetadataRouter's PRM authorization_servers entry inherits the corrected issuer automatically.

Validation

  • 2 new regression tests in packages/server-legacy/test/auth/router.test.ts: issuer served slash-less for both new URL('https://host') and new URL('https://host/')
  • Full server-legacy suite: 183 passed (11 files) — includes the 3 pre-existing issuer assertions updated from the slashed to the compliant expectation (they were pinning the bug)
  • client package: 1076 passed; 2 pre-existing Windows-local failures in test/client/stdio.test.ts (timeouts, unrelated to OAuth — fail identically on a clean tree)
  • eslint + prettier clean on both changed files; tsc --noEmit clean
  • Deployed instance of this exact fix verified live in production (shadow route implementing the same correction): issuer served slash-less, and previously-failing strict clients now complete discovery

Notes

  • The client side needs no change: discoverAuthorizationServerMetadata already validates the echo and (on main) tolerates a slashed discovery URL against a slash-less metadata issuer.
  • I read this repo's CLAUDE.md/CONTRIBUTING guidance: this is a straightforward bug fix (a few lines + tests demonstrating it), so I've skipped the pre-discussion issue step. Happy to open one if maintainers prefer.
  • A related but distinct trailing-slash issue (OAuth resource indicator from protected resource metadata is normalized with a trailing slash #1968) covered the client-side resource indicator; this is the server-side issuer field, still unfixed on main.

@Alter-Igor
Alter-Igor requested a review from a team as a code owner November 14, 2026 20:07
@changeset-bot

changeset-bot Bot commented Nov 14, 2026 •

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: a85d202

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@pkg-pr-new

pkg-pr-new Bot commented Nov 14, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

@modelcontextprotocol/client

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

@modelcontextprotocol/codemod

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

@modelcontextprotocol/core

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

@modelcontextprotocol/server

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

@modelcontextprotocol/server-legacy

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

@modelcontextprotocol/express

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

@modelcontextprotocol/fastify

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

@modelcontextprotocol/hono

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

@modelcontextprotocol/node

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

commit: a85d202

@Alter-Igor

Copy link
Copy Markdown
Author

Filed the companion issue per CONTRIBUTING preference: #3186 (contains the runnable reproduction and the iss-parameter second half of the fix that this PR's first CI run surfaced).

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant