Skip to content

fix: strip trailing slashes from OAuth metadata URL fields - #1938

Draft
maxisbey wants to merge 2 commits into
mainfrom
fix/trailing-slash-oauth-metadata
Draft

maxisbey wants to merge 2 commits into
mainfrom
fix/trailing-slash-oauth-metadata

Conversation

@maxisbey

Copy link
Copy Markdown
Contributor

Summary

Pydantic's AnyHttpUrl automatically appends a trailing slash to bare hostnames (e.g., http://localhost:8000 becomes http://localhost:8000/). This causes OAuth metadata discovery to fail in clients that validate per RFC 8414 §3.3 and RFC 9728 §3, which require the returned issuer/resource URL to be identical to the URL used for discovery.

This broke interop with Google ADK and IBM's MCP Context Forge, which correctly perform this identity check.

Changes

Add field_serializer to strip trailing slashes during JSON serialization for:

  • OAuthMetadata.issuer (RFC 8414 §3.3)
  • ProtectedResourceMetadata.resource (RFC 9728 §3)
  • ProtectedResourceMetadata.authorization_servers (RFC 9728 §3)

The fix is at the serialization layer so the internal AnyHttpUrl representation is unchanged, but the JSON responses no longer include spurious trailing slashes.

Fixes #1919
Fixes #1265

Pydantic's AnyHttpUrl automatically appends a trailing slash to bare
hostnames (e.g., http://localhost:8000 becomes http://localhost:8000/).
This causes OAuth discovery to fail in clients that validate per
RFC 8414 §3.3 and RFC 9728 §3, which require the returned issuer/resource
URL to be identical to the URL used for discovery.

Add field_serializer to OAuthMetadata.issuer,
ProtectedResourceMetadata.resource, and
ProtectedResourceMetadata.authorization_servers to strip the trailing
slash during JSON serialization.

Fixes #1919
Fixes #1265

Reported-by: joar
Github-Issue: #1919
@classmethod is not the intended decorator for Pydantic's
field_serializer (unlike field_validator which requires it).
Using @staticmethod avoids IDE warnings about incorrect descriptor
protocol usage.
@claude

claude Bot commented Jan 23, 2026

Copy link
Copy Markdown
Contributor

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

Comment thread src/mcp/shared/auth.py
Comment on lines +132 to +136
@field_serializer("issuer")
@staticmethod
def _serialize_issuer(v: AnyHttpUrl) -> str:
"""Strip trailing slash added by AnyHttpUrl for RFC 8414 §3.3 compliance."""
return str(v).rstrip("/")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we just use str instead of AnyHttpUrl in the field?

@clsferguson

Copy link
Copy Markdown

Real-world breakage report in support of this PR — the strict issuer comparison is blocking several production MCP OAuth logins today.

Reproduction (SnapTrade MCP): the server advertises its authorization server as https://api.snaptrade.com while its /.well-known/openid-configuration issuer is https://api.snaptrade.com/. validate_metadata_issuer rejects the pair, so mcp client OAuth login fails on every release to date — I verified the comparison is still strict != in the current mcp 2.3.0 wheel. Our stopgap is a local one-line patch to mcp/client/auth/utils.py (rstrip('/') on both sides), applied in-place in our venv; it gets clobbered on every package upgrade until this lands.

It's a cluster, not one server. Hermes Agent (which uses the mcp client for MCP OAuth) is tracking the same class of failure against multiple providers in NousResearch/hermes-agent#95508:

  • Indeed: https://secure.indeed.com vs https://secure.indeed.com/
  • Gmail MCP (gmailmcp.googleapis.com)
  • Karbon (identity.karbonhq.com)
  • Semgrep (login.semgrep.io)

…and a sibling variant where the emitted iss differs from the advertised one (NousResearch/hermes-agent#96107, monday.com).

Normalization on the serializer side (this PR's approach, RFC 8414 §3.3 / RFC 9728 §3) would fix the advertised-vs-issuer mismatch across the board, and it matches the local patches we're running. Happy to test a release candidate if that helps. Thanks!

This branch has not been deployed

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

Labels

None yet

Projects

None yet

3 participants