Skip to content

Commit d0440b9

Browse files
fix: strip trailing slashes at serialization layer for OAuth URLs
The previous fix attempted to strip trailing slashes at construction time, but Pydantic's AnyHttpUrl re-adds them to bare hostnames. This commit moves the fix to the serialization layer using @field_serializer decorators. Changes: - Add @field_serializer to OAuthMetadata for issuer field - Add @field_serializer to ProtectedResourceMetadata for resource and authorization_servers fields - Revert ineffective .rstrip("/") calls in routes.py - Update test assertions to expect slash-free URLs in JSON responses - Update unit tests to verify serialized JSON output This ensures RFC 8414 §3.3 and RFC 9728 §3 compliance by guaranteeing that issuer/resource URLs in metadata responses match discovery URLs exactly, without trailing slashes. Co-authored-by: Max Isbey <maxisbey@users.noreply.github.com>
1 parent 4e3b959 commit d0440b9

File tree

5 files changed

+45
-26
lines changed

5 files changed

+45
-26
lines changed

src/mcp/server/auth/routes.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ def build_metadata(
157157

158158
# Create metadata
159159
metadata = OAuthMetadata(
160-
issuer=AnyHttpUrl(str(issuer_url).rstrip("/")),
160+
issuer=issuer_url,
161161
authorization_endpoint=authorization_url,
162162
token_endpoint=token_url,
163163
scopes_supported=client_registration_options.valid_scopes,
@@ -222,8 +222,8 @@ def create_protected_resource_routes(
222222
List of Starlette routes for protected resource metadata
223223
"""
224224
metadata = ProtectedResourceMetadata(
225-
resource=AnyHttpUrl(str(resource_url).rstrip("/")),
226-
authorization_servers=[AnyHttpUrl(str(server).rstrip("/")) for server in authorization_servers],
225+
resource=resource_url,
226+
authorization_servers=authorization_servers,
227227
scopes_supported=scopes_supported,
228228
resource_name=resource_name,
229229
resource_documentation=resource_documentation,

src/mcp/shared/auth.py

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
from typing import Any, Literal
22

3-
from pydantic import AnyHttpUrl, AnyUrl, BaseModel, Field, field_validator
3+
from pydantic import AnyHttpUrl, AnyUrl, BaseModel, Field, field_serializer, field_validator
44

55

66
class OAuthToken(BaseModel):
@@ -129,6 +129,11 @@ class OAuthMetadata(BaseModel):
129129
code_challenge_methods_supported: list[str] | None = None
130130
client_id_metadata_document_supported: bool | None = None
131131

132+
@field_serializer("issuer")
133+
def serialize_issuer(self, v: AnyHttpUrl) -> str:
134+
"""Strip trailing slash from issuer URL for RFC 8414 §3.3 compliance."""
135+
return str(v).rstrip("/")
136+
132137

133138
class ProtectedResourceMetadata(BaseModel):
134139
"""RFC 9728 OAuth 2.0 Protected Resource Metadata.
@@ -151,3 +156,13 @@ class ProtectedResourceMetadata(BaseModel):
151156
dpop_signing_alg_values_supported: list[str] | None = None
152157
# dpop_bound_access_tokens_required default is False, but ommited here for clarity
153158
dpop_bound_access_tokens_required: bool | None = None
159+
160+
@field_serializer("resource")
161+
def serialize_resource(self, v: AnyHttpUrl) -> str:
162+
"""Strip trailing slash from resource URL for RFC 9728 §3 compliance."""
163+
return str(v).rstrip("/")
164+
165+
@field_serializer("authorization_servers")
166+
def serialize_authorization_servers(self, v: list[AnyHttpUrl]) -> list[str]:
167+
"""Strip trailing slashes from authorization server URLs for RFC 9728 §3 compliance."""
168+
return [str(s).rstrip("/") for s in v]

tests/server/auth/test_protected_resource.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,8 @@ async def test_metadata_endpoint_without_path(root_resource_client: httpx.AsyncC
9696
assert response.status_code == 200
9797
assert response.json() == snapshot(
9898
{
99-
"resource": "https://example.com/",
100-
"authorization_servers": ["https://auth.example.com/"],
99+
"resource": "https://example.com",
100+
"authorization_servers": ["https://auth.example.com"],
101101
"scopes_supported": ["read"],
102102
"resource_name": "Root Resource",
103103
"bearer_methods_supported": ["header"],

tests/server/auth/test_trailing_slash_fix.py

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,10 @@
1818

1919

2020
def test_build_metadata_strips_trailing_slash_from_issuer():
21-
"""Test that build_metadata strips trailing slash from issuer URL.
21+
"""Test that build_metadata strips trailing slash from issuer URL when serialized.
2222
2323
Pydantic's AnyHttpUrl automatically adds trailing slashes to bare hostnames.
24-
This test verifies that we strip them to comply with RFC 8414 §3.3.
24+
This test verifies that we strip them during serialization to comply with RFC 8414 §3.3.
2525
"""
2626
# Use a bare hostname URL which Pydantic will add a trailing slash to
2727
issuer_url = AnyHttpUrl("http://localhost:8000")
@@ -33,13 +33,14 @@ def test_build_metadata_strips_trailing_slash_from_issuer():
3333
revocation_options=RevocationOptions(enabled=False),
3434
)
3535

36-
# The issuer should NOT have a trailing slash
37-
assert str(metadata.issuer) == "http://localhost:8000"
38-
assert not str(metadata.issuer).endswith("/")
36+
# The serialized issuer should NOT have a trailing slash
37+
serialized = metadata.model_dump(mode="json")
38+
assert serialized["issuer"] == "http://localhost:8000"
39+
assert not serialized["issuer"].endswith("/")
3940

4041

4142
def test_build_metadata_strips_trailing_slash_from_issuer_with_path():
42-
"""Test that build_metadata strips trailing slash from issuer URL with path."""
43+
"""Test that build_metadata strips trailing slash from issuer URL with path when serialized."""
4344
# URL with path that has trailing slash
4445
issuer_url = AnyHttpUrl("http://localhost:8000/auth/")
4546

@@ -50,9 +51,10 @@ def test_build_metadata_strips_trailing_slash_from_issuer_with_path():
5051
revocation_options=RevocationOptions(enabled=False),
5152
)
5253

53-
# The issuer should NOT have a trailing slash
54-
assert str(metadata.issuer) == "http://localhost:8000/auth"
55-
assert not str(metadata.issuer).endswith("/")
54+
# The serialized issuer should NOT have a trailing slash
55+
serialized = metadata.model_dump(mode="json")
56+
assert serialized["issuer"] == "http://localhost:8000/auth"
57+
assert not serialized["issuer"].endswith("/")
5658

5759

5860
def test_build_metadata_endpoints_have_no_double_slashes():
@@ -81,7 +83,7 @@ def test_build_metadata_endpoints_have_no_double_slashes():
8183

8284

8385
def test_protected_resource_metadata_strips_trailing_slash_from_resource():
84-
"""Test that protected resource metadata strips trailing slash from resource URL.
86+
"""Test that protected resource metadata strips trailing slash from resource URL when serialized.
8587
8688
RFC 9728 §3 requires that the resource URL in the metadata response must be
8789
identical to the URL used for discovery.
@@ -104,13 +106,14 @@ def test_protected_resource_metadata_strips_trailing_slash_from_resource():
104106

105107
metadata = handler.__self__.metadata # type: ignore
106108

107-
# The resource URL should NOT have a trailing slash
108-
assert str(metadata.resource) == "http://localhost:8000"
109-
assert not str(metadata.resource).endswith("/")
109+
# The serialized resource URL should NOT have a trailing slash
110+
serialized = metadata.model_dump(mode="json")
111+
assert serialized["resource"] == "http://localhost:8000"
112+
assert not serialized["resource"].endswith("/")
110113

111114

112115
def test_protected_resource_metadata_strips_trailing_slash_from_authorization_servers():
113-
"""Test that protected resource metadata strips trailing slashes from authorization server URLs."""
116+
"""Test that protected resource metadata strips trailing slashes from auth server URLs when serialized."""
114117
resource_url = AnyHttpUrl("http://localhost:8000/resource")
115118
# Use bare hostname URLs which Pydantic will add trailing slashes to
116119
auth_servers = [
@@ -129,11 +132,12 @@ def test_protected_resource_metadata_strips_trailing_slash_from_authorization_se
129132
handler = cors_app.app.func # type: ignore
130133
metadata = handler.__self__.metadata # type: ignore
131134

132-
# All authorization server URLs should NOT have trailing slashes
133-
assert str(metadata.authorization_servers[0]) == "http://auth1.example.com"
134-
assert str(metadata.authorization_servers[1]) == "http://auth2.example.com"
135-
assert not str(metadata.authorization_servers[0]).endswith("/")
136-
assert not str(metadata.authorization_servers[1]).endswith("/")
135+
# All serialized authorization server URLs should NOT have trailing slashes
136+
serialized = metadata.model_dump(mode="json")
137+
assert serialized["authorization_servers"][0] == "http://auth1.example.com"
138+
assert serialized["authorization_servers"][1] == "http://auth2.example.com"
139+
assert not serialized["authorization_servers"][0].endswith("/")
140+
assert not serialized["authorization_servers"][1].endswith("/")
137141

138142

139143
@pytest.fixture

tests/server/fastmcp/auth/test_auth_integration.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ async def test_metadata_endpoint(self, test_client: httpx.AsyncClient):
311311
assert response.status_code == 200
312312

313313
metadata = response.json()
314-
assert metadata["issuer"] == "https://auth.example.com/"
314+
assert metadata["issuer"] == "https://auth.example.com"
315315
assert metadata["authorization_endpoint"] == "https://auth.example.com/authorize"
316316
assert metadata["token_endpoint"] == "https://auth.example.com/token"
317317
assert metadata["registration_endpoint"] == "https://auth.example.com/register"

0 commit comments

Comments
 (0)