Skip to content

fix(auth): strip trailing slashes from OAuth metadata URLs#3013

Open
piyushbag wants to merge 7 commits into
modelcontextprotocol:mainfrom
piyushbag:fix-1919-oauth-metadata-trailing-slash
Open

fix(auth): strip trailing slashes from OAuth metadata URLs#3013
piyushbag wants to merge 7 commits into
modelcontextprotocol:mainfrom
piyushbag:fix-1919-oauth-metadata-trailing-slash

Conversation

@piyushbag

Copy link
Copy Markdown

Summary

  • strip trailing slashes from issuer in build_metadata() before constructing OAuthMetadata
  • strip trailing slashes from resource and each authorization_servers entry in create_protected_resource_routes()
  • pass canonical URL strings so metadata models with url_preserve_empty_path serialize without a synthetic root slash
  • add regression tests for AnyHttpUrl issuer inputs and root-level protected-resource metadata responses

Fixes #1919. Also fixes #1265.

Test plan

  • uv run pytest tests/server/auth/test_routes.py -q
  • uv run pytest tests/server/auth/test_protected_resource.py -q
  • uv run pytest tests/server/auth/ -q
  • uv run ruff check src/mcp/server/auth/routes.py tests/server/auth/test_routes.py tests/server/auth/test_protected_resource.py
  • uv run pyright src/mcp/server/auth/routes.py

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

All reported issues were addressed across 3 files

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread src/mcp/server/auth/routes.py Outdated
Comment thread src/mcp/server/auth/routes.py Outdated
@piyushbag
piyushbag force-pushed the fix-1919-oauth-metadata-trailing-slash branch from a591757 to c65b029 Compare July 11, 2026 11:48
@piyushbag

Copy link
Copy Markdown
Author

Rebased onto current main. Updated client test_build_metadata expectations now that issuers are served without a synthetic trailing slash, and removed the obsolete Pydantic AnyUrl xfail.

Local: targeted auth route and metadata tests pass.

@piyushbag

Copy link
Copy Markdown
Author

CI failures were issuer string mismatches after the metadata slash strip (iss / PRM authorization_servers still carried a trailing slash).

Aligned the interaction auth fixtures and assertions with the canonical slash-free issuer. Local: full tests/interaction/auth/ plus route/metadata unit tests pass (87).

Pydantic AnyHttpUrl adds a trailing slash to bare hostnames, which breaks
RFC 8414/9728 exact issuer and resource comparison during OAuth discovery.
Pass canonical URL strings into metadata models so served wire JSON omits
the synthetic root slash. Fixes modelcontextprotocol#1919 and modelcontextprotocol#1265.
Update docs, stories, and tests that asserted trailing slashes on root
issuer and authorization_server metadata fields after the routes.py fix.
Remove the Pydantic AnyUrl xfail now that routes.strip trailing slashes,
and assert issuer against the canonical form.
build_metadata now serves path-less issuers without a trailing slash.
Update authorize iss defaults, PRM/ASM overrides, and assertions so RFC
9207 string compares match the canonical metadata form.
Keep path-based issuer/resource/authorization_server identifiers intact
so exact-string matching still works. Root URLs still drop the synthetic
slash added by AnyHttpUrl.
@piyushbag
piyushbag force-pushed the fix-1919-oauth-metadata-trailing-slash branch from 46006a4 to 9801940 Compare July 17, 2026 13:56
@piyushbag

Copy link
Copy Markdown
Author

Rebased onto current main and addressed the cubic P2 notes on over-broad rstrip('/').

Identifier fields (issuer, resource, authorization_servers) now strip a trailing slash only when the URL path is empty or /. Path-based identifiers such as /tenant/ and /mcp/ keep their configured trailing slash for exact-string matching.

Endpoint path joining still uses rstrip("/") before appending /authorize etc.

Local: 28 auth route / protected-resource tests passed, including new path-based trailing-slash cases.

@piyushbag

Copy link
Copy Markdown
Author

CI is green and cubic threads are addressed. Ready for review when you have bandwidth — happy to rebase or adjust if anything should change before merge.

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

Labels

None yet

Projects

None yet

1 participant