Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions docs/migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -1220,6 +1220,16 @@ Tasks are expected to return as a separate MCP extension in a future release.

## Bug Fixes

### OAuth metadata URLs no longer gain a trailing slash

`OAuthMetadata`, `ProtectedResourceMetadata`, and `OAuthClientMetadata` now set
`url_preserve_empty_path=True` (Pydantic 2.12+). A path-less URL parsed from the wire keeps its
empty path instead of acquiring a trailing slash, so e.g. an `issuer` of `https://as.example.com`
round-trips as `https://as.example.com` rather than `https://as.example.com/`. This matters for
RFC 9207 / RFC 8414 issuer comparisons, which require simple string comparison (RFC 3986 §6.2.1).
URLs constructed in Python from an already-built `AnyHttpUrl` object are unaffected (they were
normalized at construction); only values parsed from strings/JSON change.

### Lowlevel `Server`: `subscribe` capability now correctly reported

Previously, the lowlevel `Server` hardcoded `subscribe=False` in resource capabilities even when a `subscribe_resource()` handler was registered. The `subscribe` capability is now dynamically set to `True` when an `on_subscribe_resource` handler is provided. Clients that previously didn't see `subscribe: true` in capabilities will now see it when a handler is registered, which may change client behavior.
Expand Down
14 changes: 13 additions & 1 deletion src/mcp/shared/auth.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from typing import Any, Literal

from pydantic import AnyHttpUrl, AnyUrl, BaseModel, Field, field_validator
from pydantic import AnyHttpUrl, AnyUrl, BaseModel, ConfigDict, Field, field_validator


class OAuthToken(BaseModel):
Expand Down Expand Up @@ -37,6 +37,10 @@
See https://datatracker.ietf.org/doc/html/rfc7591#section-2
"""

# Preserve empty URL paths so identifiers are compared as transmitted (RFC 3986 6.2.1)
# instead of acquiring a trailing slash; defaults in Pydantic v3.
model_config = ConfigDict(url_preserve_empty_path=True)

Check failure on line 42 in src/mcp/shared/auth.py

View check run for this annotation

Claude / Claude Code Review

redirect_uri serialization change can break previously-registered path-less redirect URIs

Adding `url_preserve_empty_path=True` to `OAuthClientMetadata` changes the wire serialization of `redirect_uris`, not just metadata parsing: a path-less redirect URI passed as a string (e.g. `redirect_uris=['http://localhost:8080']`) previously serialized as `http://localhost:8080/` but now keeps the empty path, and the client sends this value verbatim in the /authorize and token-exchange requests (oauth2.py:337, :393). A client that registered with the old SDK (registration persisted in TokenSt
Comment on lines +40 to +42

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 Adding url_preserve_empty_path=True to OAuthClientMetadata changes the wire serialization of redirect_uris, not just metadata parsing: a path-less redirect URI passed as a string (e.g. redirect_uris=['http://localhost:8080']) previously serialized as http://localhost:8080/ but now keeps the empty path, and the client sends this value verbatim in the /authorize and token-exchange requests (oauth2.py:337, :393). A client that registered with the old SDK (registration persisted in TokenStorage, so no re-registration) will now send a redirect_uri that no longer exact-string-matches the one recorded at an RFC 6749-compliant authorization server, breaking a previously working flow. Consider scoping the flag to OAuthMetadata/ProtectedResourceMetadata (which is all the issuer-comparison goal needs) or calling out the redirect_uri implication in the migration note.

Extended reasoning...

What the bug is. The PR's stated goal is to make issuer/resource/authorization_servers round-trip as transmitted for RFC 9207 / RFC 8414 issuer comparison. But the flag is also applied to OAuthClientMetadata, where the comparison-sensitive field on the wire is redirect_uris — and that field's serialized form is exact-string-matched by an external party (the authorization server), persistently, across SDK upgrades.

The PR's scope claim doesn't fully hold. The description says "URLs constructed in Python from an already-built AnyHttpUrl object are unchanged ... only values parsed from strings/JSON change." That is true for pre-built AnyHttpUrl objects, but the common way client metadata is constructed is with plain strings: OAuthClientMetadata(redirect_uris=['http://localhost:8080']). Verified on pydantic 2.12.5 (the repo's installed floor): with url_preserve_empty_path=True, python-mode validation of that string now yields 'http://localhost:8080' (and model_dump_json emits it without the slash), whereas before this PR it normalized to 'http://localhost:8080/'. So user code that hasn't changed at all produces a different wire value after upgrading.

The code path that puts it on the wire. src/mcp/client/auth/oauth2.py sends str(self.context.client_metadata.redirect_uris[0]) directly in both the authorization request (oauth2.py:337) and the token-exchange request (oauth2.py:393), and the DCR registration body is serialized from this same model. Nothing re-normalizes the URI before transmission.

Step-by-step regression scenario.

  1. With the previous SDK release, an app constructs OAuthClientMetadata(redirect_uris=['http://localhost:8080']) and performs dynamic client registration. The registration request carries "redirect_uris": ["http://localhost:8080/"] (old normalization), so the AS records exactly that string. The resulting client_id/client info is persisted in TokenStorage — the SDK's normal mode of operation — so no re-registration happens later.
  2. The app upgrades to an SDK containing this PR. Same user code, but redirect_uris[0] now serializes as http://localhost:8080 (no trailing slash).
  3. The next OAuth flow sends redirect_uri=http://localhost:8080 in the /authorize request (oauth2.py:337). RFC 6749 §3.1.2.3 requires the AS to compare redirect URIs using simple string comparison, so a spec-compliant external AS rejects the request with invalid_redirect_uri — a hard, confusing auth failure for a flow that worked before the upgrade, until the user clears stored client info and re-registers.

Why nothing else prevents it. The SDK's own server-side AuthorizationHandler is not affected, because pydantic AnyUrl equality treats http://localhost:8080 and http://localhost:8080/ as equal (verified), so validate_redirect_uri still matches. The breakage is specifically against third-party authorization servers doing the RFC-mandated literal string match on the transmitted value, where pydantic equality is irrelevant. Neither the migration note nor the PR description mentions that OAuthClientMetadata.redirect_uris output changes on the wire.

Mitigating factors and how to fix. The trigger is fairly narrow: it requires a path-less redirect URI (uncommon — the SDK examples use /callback), a DCR registration persisted across the upgrade, and an AS that does strict string matching; recovery is re-registering. Still, the fix is cheap: scope url_preserve_empty_path=True to OAuthMetadata and ProtectedResourceMetadata only (which is everything the issuer-comparison goal and #2921 need), or — if the new behavior on OAuthClientMetadata is intentional — explicitly document the redirect_uri wire-format change in the migration note so users with persisted registrations know to re-register.


redirect_uris: list[AnyUrl] | None = Field(..., min_length=1)
# supported auth methods for the token endpoint
token_endpoint_auth_method: (
Expand Down Expand Up @@ -123,6 +127,10 @@
See https://datatracker.ietf.org/doc/html/rfc8414#section-2
"""

# Preserve empty URL paths so the issuer is compared as transmitted (RFC 3986 6.2.1)
# instead of acquiring a trailing slash; defaults in Pydantic v3.
model_config = ConfigDict(url_preserve_empty_path=True)

issuer: AnyHttpUrl
authorization_endpoint: AnyHttpUrl
token_endpoint: AnyHttpUrl
Expand Down Expand Up @@ -152,6 +160,10 @@
See https://datatracker.ietf.org/doc/html/rfc9728#section-2
"""

# Preserve empty URL paths so the resource and authorization servers are compared as
# transmitted (RFC 3986 6.2.1) instead of acquiring a trailing slash; defaults in Pydantic v3.
model_config = ConfigDict(url_preserve_empty_path=True)

resource: AnyHttpUrl
authorization_servers: list[AnyHttpUrl] = Field(..., min_length=1)
jwks_uri: AnyHttpUrl | None = None
Expand Down
4 changes: 2 additions & 2 deletions tests/client/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -517,10 +517,10 @@ async def test_handle_metadata_response_success(self, oauth_provider: OAuthClien
}"""
response = httpx.Response(200, content=content)

# Should set metadata
# Should set metadata; the empty path is preserved (no trailing slash added)
await oauth_provider._handle_oauth_metadata_response(response)
assert oauth_provider.context.oauth_metadata is not None
assert str(oauth_provider.context.oauth_metadata.issuer) == "https://auth.example.com/"
assert str(oauth_provider.context.oauth_metadata.issuer) == "https://auth.example.com"

@pytest.mark.anyio
async def test_prioritize_www_auth_scope_over_prm(
Expand Down
Loading