From b288e05f0c59ee34b62a5828b20a6ac497cdf25a Mon Sep 17 00:00:00 2001 From: BabyChrist666 Date: Sun, 8 Feb 2026 15:53:08 -0500 Subject: [PATCH] fix: strip trailing slash from issuer URL in OAuth metadata serialization Fixes #1919 RFC 8414 examples show issuer URLs without trailing slashes (e.g., `https://example.com` rather than `https://example.com/`). Some OAuth clients (Google ADK, IBM MCP Context Forge) require exact match between the discovery URL and the returned issuer per RFC 8414 Section 3.3. Pydantic's AnyHttpUrl automatically adds a trailing slash when serializing URLs without a path component. This breaks clients that perform exact URL matching during the OAuth discovery flow. This fix adds field_serializer decorators to: - OAuthMetadata.issuer - ProtectedResourceMetadata.resource - ProtectedResourceMetadata.authorization_servers These serializers strip the trailing slash only during JSON serialization, preserving the internal AnyHttpUrl type while ensuring spec-compliant output. Co-Authored-By: Claude Opus 4.5 --- src/mcp/shared/auth.py | 28 +++++++- tests/server/auth/test_protected_resource.py | 5 +- tests/shared/test_auth.py | 73 +++++++++++++++++++- 3 files changed, 102 insertions(+), 4 deletions(-) diff --git a/src/mcp/shared/auth.py b/src/mcp/shared/auth.py index bf03a8b8d..4b61aee28 100644 --- a/src/mcp/shared/auth.py +++ b/src/mcp/shared/auth.py @@ -1,6 +1,6 @@ from typing import Any, Literal -from pydantic import AnyHttpUrl, AnyUrl, BaseModel, Field, field_validator +from pydantic import AnyHttpUrl, AnyUrl, BaseModel, Field, field_serializer, field_validator class OAuthToken(BaseModel): @@ -129,6 +129,18 @@ class OAuthMetadata(BaseModel): code_challenge_methods_supported: list[str] | None = None client_id_metadata_document_supported: bool | None = None + @field_serializer("issuer") + def serialize_issuer_without_trailing_slash(self, v: AnyHttpUrl) -> str: + """Strip trailing slash from issuer URL during serialization. + + RFC 8414 examples show issuer URLs without trailing slashes, and some + OAuth clients (Google ADK, IBM MCP Context Forge) require exact match + between discovery URL and returned issuer per RFC 8414 Section 3.3. + Pydantic's AnyHttpUrl automatically adds a trailing slash, which breaks + these clients. See: https://github.com/modelcontextprotocol/python-sdk/issues/1919 + """ + return str(v).rstrip("/") + class ProtectedResourceMetadata(BaseModel): """RFC 9728 OAuth 2.0 Protected Resource Metadata. @@ -151,3 +163,17 @@ class ProtectedResourceMetadata(BaseModel): dpop_signing_alg_values_supported: list[str] | None = None # dpop_bound_access_tokens_required default is False, but ommited here for clarity dpop_bound_access_tokens_required: bool | None = None + + @field_serializer("resource") + def serialize_resource_without_trailing_slash(self, v: AnyHttpUrl) -> str: + """Strip trailing slash from resource URL during serialization. + + Same rationale as OAuthMetadata.issuer - RFC specs show URLs without + trailing slashes, and clients may require exact URL matching. + """ + return str(v).rstrip("/") + + @field_serializer("authorization_servers") + def serialize_auth_servers_without_trailing_slash(self, v: list[AnyHttpUrl]) -> list[str]: + """Strip trailing slashes from authorization server URLs during serialization.""" + return [str(url).rstrip("/") for url in v] diff --git a/tests/server/auth/test_protected_resource.py b/tests/server/auth/test_protected_resource.py index 413a80276..a04f89efa 100644 --- a/tests/server/auth/test_protected_resource.py +++ b/tests/server/auth/test_protected_resource.py @@ -94,10 +94,11 @@ async def test_metadata_endpoint_without_path(root_resource_client: httpx.AsyncC # For root resource, metadata should be at standard location response = await root_resource_client.get("/.well-known/oauth-protected-resource") assert response.status_code == 200 + # Note: URLs should NOT have trailing slashes per RFC 8414/9728 (see issue #1919) assert response.json() == snapshot( { - "resource": "https://example.com/", - "authorization_servers": ["https://auth.example.com/"], + "resource": "https://example.com", + "authorization_servers": ["https://auth.example.com"], "scopes_supported": ["read"], "resource_name": "Root Resource", "bearer_methods_supported": ["header"], diff --git a/tests/shared/test_auth.py b/tests/shared/test_auth.py index cd3c35332..b28a99ac6 100644 --- a/tests/shared/test_auth.py +++ b/tests/shared/test_auth.py @@ -1,6 +1,8 @@ """Tests for OAuth 2.0 shared code.""" -from mcp.shared.auth import OAuthMetadata +import json + +from mcp.shared.auth import OAuthMetadata, ProtectedResourceMetadata def test_oauth(): @@ -58,3 +60,72 @@ def test_oauth_with_jarm(): "token_endpoint_auth_methods_supported": ["client_secret_basic", "client_secret_post"], } ) + + +class TestIssuerTrailingSlash: + """Tests for issue #1919: trailing slash in issuer URL. + + RFC 8414 examples show issuer URLs without trailing slashes, and some + OAuth clients require exact match between discovery URL and returned issuer. + Pydantic's AnyHttpUrl automatically adds a trailing slash, so we strip it + during serialization. + """ + + def test_oauth_metadata_issuer_no_trailing_slash_in_json(self): + """Serialized issuer should not have trailing slash.""" + metadata = OAuthMetadata( + issuer="https://example.com", + authorization_endpoint="https://example.com/oauth2/authorize", + token_endpoint="https://example.com/oauth2/token", + ) + serialized = json.loads(metadata.model_dump_json()) + assert serialized["issuer"] == "https://example.com" + assert not serialized["issuer"].endswith("/") + + def test_oauth_metadata_issuer_with_path_preserves_path(self): + """Issuer with path should preserve the path, only strip trailing slash.""" + metadata = OAuthMetadata( + issuer="https://example.com/auth", + authorization_endpoint="https://example.com/oauth2/authorize", + token_endpoint="https://example.com/oauth2/token", + ) + serialized = json.loads(metadata.model_dump_json()) + assert serialized["issuer"] == "https://example.com/auth" + assert not serialized["issuer"].endswith("/") + + def test_oauth_metadata_issuer_with_path_and_trailing_slash(self): + """Issuer with path and trailing slash should only strip the trailing slash.""" + metadata = OAuthMetadata( + issuer="https://example.com/auth/", + authorization_endpoint="https://example.com/oauth2/authorize", + token_endpoint="https://example.com/oauth2/token", + ) + serialized = json.loads(metadata.model_dump_json()) + assert serialized["issuer"] == "https://example.com/auth" + + def test_protected_resource_metadata_no_trailing_slash(self): + """ProtectedResourceMetadata.resource should not have trailing slash.""" + metadata = ProtectedResourceMetadata( + resource="https://example.com", + authorization_servers=["https://auth.example.com"], + ) + serialized = json.loads(metadata.model_dump_json()) + assert serialized["resource"] == "https://example.com" + assert not serialized["resource"].endswith("/") + + def test_protected_resource_metadata_auth_servers_no_trailing_slash(self): + """ProtectedResourceMetadata.authorization_servers should not have trailing slashes.""" + metadata = ProtectedResourceMetadata( + resource="https://example.com", + authorization_servers=[ + "https://auth1.example.com", + "https://auth2.example.com/path", + ], + ) + serialized = json.loads(metadata.model_dump_json()) + assert serialized["authorization_servers"] == [ + "https://auth1.example.com", + "https://auth2.example.com/path", + ] + for url in serialized["authorization_servers"]: + assert not url.endswith("/")