Skip to content

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

Open
piyushbag wants to merge 2 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 2 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

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.

@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.

2 issues found across 3 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/mcp/server/auth/routes.py">

<violation number="1" location="src/mcp/server/auth/routes.py:172">
P2: Issuer canonicalization is over-broad: `rstrip('/')` also rewrites non-root path issuers (e.g. `/tenant/` -> `/tenant`), which can break exact issuer identifier matching.</violation>

<violation number="2" location="src/mcp/server/auth/routes.py:243">
P2: Protected-resource metadata now removes trailing slashes from all configured URLs, which can silently change path-based resource or authorization-server identifiers.</violation>
</file>

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

Fix all with cubic | Re-trigger cubic

resource=cast(AnyHttpUrl, str(resource_url).rstrip("/")),
authorization_servers=cast(
list[AnyHttpUrl],
[str(server).rstrip("/") for server in authorization_servers],

@cubic-dev-ai cubic-dev-ai Bot Jun 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: Protected-resource metadata now removes trailing slashes from all configured URLs, which can silently change path-based resource or authorization-server identifiers.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/mcp/server/auth/routes.py, line 243:

<comment>Protected-resource metadata now removes trailing slashes from all configured URLs, which can silently change path-based resource or authorization-server identifiers.</comment>

<file context>
@@ -237,8 +237,11 @@ def create_protected_resource_routes(
+        resource=cast(AnyHttpUrl, str(resource_url).rstrip("/")),
+        authorization_servers=cast(
+            list[AnyHttpUrl],
+            [str(server).rstrip("/") for server in authorization_servers],
+        ),
         scopes_supported=scopes_supported,
</file context>
Fix with cubic

# Create metadata
metadata = OAuthMetadata(
issuer=issuer_url,
issuer=cast(AnyHttpUrl, str(issuer_url).rstrip("/")),

@cubic-dev-ai cubic-dev-ai Bot Jun 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: Issuer canonicalization is over-broad: rstrip('/') also rewrites non-root path issuers (e.g. /tenant/ -> /tenant), which can break exact issuer identifier matching.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/mcp/server/auth/routes.py, line 172:

<comment>Issuer canonicalization is over-broad: `rstrip('/')` also rewrites non-root path issuers (e.g. `/tenant/` -> `/tenant`), which can break exact issuer identifier matching.</comment>

<file context>
@@ -169,7 +169,7 @@ def build_metadata(
     # Create metadata
     metadata = OAuthMetadata(
-        issuer=issuer_url,
+        issuer=cast(AnyHttpUrl, str(issuer_url).rstrip("/")),
         authorization_endpoint=authorization_url,
         token_endpoint=token_url,
</file context>
Fix with cubic

Update docs, stories, and tests that asserted trailing slashes on root
issuer and authorization_server metadata fields after the routes.py fix.
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