Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion docs_src/identity_assertion/tutorial001.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ async def fetch_id_jag(audience: str, resource: str) -> str:
storage=InMemoryTokenStorage(),
client_id="finance-agent",
client_secret="finance-agent-secret",
issuer="https://auth.example.com/",
issuer="https://auth.example.com",
assertion_provider=fetch_id_jag,
scope="notes:read",
)
Expand Down
2 changes: 1 addition & 1 deletion docs_src/identity_assertion/tutorial002.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from mcp.server.auth.routes import create_auth_routes
from mcp.shared.auth import JWT_BEARER_GRANT_TYPE, OAuthClientInformationFull, OAuthToken

ISSUER = "https://auth.example.com/"
ISSUER = "https://auth.example.com"
MCP_SERVER = "http://localhost:8001/mcp"
IDP_ISSUER = "https://idp.example.com"
IDP_SIGNING_KEY = "the-enterprise-idp-signing-key"
Expand Down
5 changes: 2 additions & 3 deletions examples/stories/_shared/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
from urllib.parse import parse_qs, urlsplit

import httpx
from pydantic import AnyHttpUrl

from mcp.server.auth.provider import (
AccessToken,
Expand Down Expand Up @@ -164,8 +163,8 @@ def auth_settings(
"""
scopes = required_scopes or ["mcp"]
return AuthSettings(
issuer_url=AnyHttpUrl(BASE_URL),
resource_server_url=AnyHttpUrl(MCP_URL),
issuer_url=BASE_URL, # type: ignore[arg-type]
resource_server_url=MCP_URL, # type: ignore[arg-type]
required_scopes=scopes,
client_registration_options=ClientRegistrationOptions(enabled=True, valid_scopes=scopes, default_scopes=scopes),
identity_assertion_enabled=identity_assertion_enabled,
Expand Down
2 changes: 1 addition & 1 deletion examples/stories/identity_assertion/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def build_auth(_http: httpx.AsyncClient) -> httpx.Auth:

`issuer` is configuration, not discovery: the provider fetches metadata from this issuer's
well-known and never asks the MCP server which authorization server to use. The string must
equal the `issuer` its metadata serves byte for byte (note the trailing slash).
equal the `issuer` its metadata serves byte for byte.
`Client(url, auth=...)` doesn't exist yet, so the harness threads this onto the underlying
`httpx.AsyncClient` and hands `main` a target that is already routed through it.
"""
Expand Down
3 changes: 1 addition & 2 deletions examples/stories/identity_assertion/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@
DEMO_CLIENT_SECRET = "demo-finance-agent-secret"
DEMO_SCOPE = "mcp"
# The exact `issuer` string this authorization server's metadata serves. The client must configure
# the byte-identical string: RFC 8414 issuer comparison is character for character, and the
# settings' `AnyHttpUrl` renders the path-less loopback origin with a trailing slash.
# the byte-identical string: RFC 8414 issuer comparison is character for character.
ISSUER = str(auth_settings().issuer_url)


Expand Down
2 changes: 1 addition & 1 deletion examples/stories/oauth_client_credentials/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def whoami() -> Whoami:
@mcp.custom_route("/.well-known/oauth-authorization-server", methods=["GET"])
async def as_metadata(request: Request) -> JSONResponse:
meta = OAuthMetadata(
issuer=AnyHttpUrl(BASE_URL),
issuer=BASE_URL, # type: ignore[arg-type]
authorization_endpoint=AnyHttpUrl(f"{BASE_URL}/authorize"), # unused; required
token_endpoint=AnyHttpUrl(f"{BASE_URL}/token"),
grant_types_supported=["client_credentials"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ async def call_tool(ctx: ServerRequestContext[Any], params: types.CallToolReques

async def as_metadata(request: Request) -> JSONResponse:
meta = OAuthMetadata(
issuer=AnyHttpUrl(BASE_URL),
issuer=BASE_URL, # type: ignore[arg-type]
authorization_endpoint=AnyHttpUrl(f"{BASE_URL}/authorize"), # unused; required
token_endpoint=AnyHttpUrl(f"{BASE_URL}/token"),
grant_types_supported=["client_credentials"],
Expand Down
11 changes: 7 additions & 4 deletions src/mcp/server/auth/routes.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from collections.abc import Awaitable, Callable
from typing import Any
from typing import Any, cast
from urllib.parse import urlparse

from pydantic import AnyHttpUrl
Expand Down Expand Up @@ -169,7 +169,7 @@ def build_metadata(

# 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

authorization_endpoint=authorization_url,
token_endpoint=token_url,
scopes_supported=client_registration_options.valid_scopes,
Expand Down Expand Up @@ -237,8 +237,11 @@ def create_protected_resource_routes(
List of Starlette routes for protected resource metadata
"""
metadata = ProtectedResourceMetadata(
resource=resource_url,
authorization_servers=authorization_servers,
resource=cast(AnyHttpUrl, str(resource_url).rstrip("/")),
authorization_servers=cast(
list[AnyHttpUrl],
[str(server).rstrip("/") for server in authorization_servers],
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
),
scopes_supported=scopes_supported,
resource_name=resource_name,
resource_documentation=resource_documentation,
Expand Down
5 changes: 1 addition & 4 deletions tests/client/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -1529,9 +1529,6 @@ async def mock_callback() -> AuthorizationCodeResult:
"https://auth.example.com/register",
"https://auth.example.com/revoke",
id="simple-url",
marks=pytest.mark.xfail(
reason="Pydantic AnyUrl adds trailing slash to base URLs - fixed in Pydantic 2.12+"
),
),
pytest.param(
"https://auth.example.com/",
Expand Down Expand Up @@ -1570,7 +1567,7 @@ def test_build_metadata(

assert metadata.model_dump(exclude_defaults=True, mode="json") == snapshot(
{
"issuer": Is(issuer_url),
"issuer": Is(issuer_url.rstrip("/")),
"authorization_endpoint": Is(authorization_endpoint),
"token_endpoint": Is(token_endpoint),
"registration_endpoint": Is(registration_endpoint),
Expand Down
2 changes: 1 addition & 1 deletion tests/docs_src/test_authorization.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ async def test_the_metadata_document_is_built_from_auth_settings() -> None:
assert response.json() == snapshot(
{
"resource": "http://127.0.0.1:8000/mcp",
"authorization_servers": ["https://auth.example.com/"],
"authorization_servers": ["https://auth.example.com"],
"scopes_supported": ["notes:read"],
"bearer_methods_supported": ["header"],
}
Expand Down
2 changes: 1 addition & 1 deletion tests/docs_src/test_identity_assertion.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ async def test_the_metadata_advertises_the_grant_type_and_the_id_jag_profile() -
response = await http_client.get("/.well-known/oauth-authorization-server")
assert response.status_code == 200
metadata = response.json()
assert metadata["issuer"] == "https://auth.example.com/"
assert metadata["issuer"] == "https://auth.example.com"
assert "urn:ietf:params:oauth:grant-type:jwt-bearer" in metadata["grant_types_supported"]
assert metadata["authorization_grant_profiles_supported"] == ["urn:ietf:params:oauth:grant-profile:id-jag"]

Expand Down
8 changes: 4 additions & 4 deletions tests/interaction/auth/_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,10 @@ def __init__(
) -> None:
self._default_scopes = list(default_scopes) if default_scopes is not None else ["mcp"]
# The authorization-response iss must equal the AS metadata issuer the client recorded
# (RFC 9207 simple string comparison). `real_asm` builds the issuer from an AnyHttpUrl
# object, so it carries the trailing slash; the redirect iss matches it. Path-issuer
# tests pass the recorded issuer explicitly.
self._issuer = issuer if issuer is not None else f"{BASE_URL}/"
# (RFC 9207 simple string comparison). `build_metadata` serves path-less issuers without a
# trailing slash, so the redirect iss matches that canonical form. Path-issuer tests pass
# the recorded issuer explicitly.
self._issuer = issuer if issuer is not None else BASE_URL
self._deny_authorize = deny_authorize
self._issue_expired_first = issue_expired_first
self._fail_next_refresh = fail_next_refresh
Expand Down
5 changes: 3 additions & 2 deletions tests/interaction/auth/test_authorize_token.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import re
from collections.abc import AsyncIterator
from dataclasses import dataclass
from typing import cast
from urllib.parse import parse_qsl, quote, urlsplit

import anyio
Expand Down Expand Up @@ -297,7 +298,7 @@ async def test_the_registered_auth_method_is_used_regardless_of_as_metadata_adve
server = Server("guarded", on_list_tools=list_tools)

override = OAuthMetadata(
issuer=AnyHttpUrl(f"{BASE_URL}/"),
issuer=cast(AnyHttpUrl, BASE_URL),
authorization_endpoint=AnyHttpUrl(f"{BASE_URL}/authorize"),
token_endpoint=AnyHttpUrl(f"{BASE_URL}/token"),
registration_endpoint=AnyHttpUrl(f"{BASE_URL}/register"),
Expand Down Expand Up @@ -367,7 +368,7 @@ async def test_pkce_is_still_sent_when_as_metadata_omits_code_challenge_methods_
completes. See the divergence on the requirement.
"""
override = OAuthMetadata(
issuer=AnyHttpUrl(f"{BASE_URL}/"),
issuer=cast(AnyHttpUrl, BASE_URL),
authorization_endpoint=AnyHttpUrl(f"{BASE_URL}/authorize"),
token_endpoint=AnyHttpUrl(f"{BASE_URL}/token"),
registration_endpoint=AnyHttpUrl(f"{BASE_URL}/register"),
Expand Down
7 changes: 4 additions & 3 deletions tests/interaction/auth/test_discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
"""

import json
from typing import cast

import anyio
import mcp_types as types
Expand Down Expand Up @@ -54,7 +55,7 @@ def discovery_gets(recorded: list[RecordedRequest]) -> list[str]:
def real_asm() -> OAuthMetadata:
"""Build an authorization-server metadata document pointing at the real co-hosted endpoints."""
return OAuthMetadata(
issuer=AnyHttpUrl(BASE_URL),
issuer=cast(AnyHttpUrl, BASE_URL),
authorization_endpoint=AnyHttpUrl(f"{BASE_URL}/authorize"),
token_endpoint=AnyHttpUrl(f"{BASE_URL}/token"),
registration_endpoint=AnyHttpUrl(f"{BASE_URL}/register"),
Expand Down Expand Up @@ -100,7 +101,7 @@ async def test_prm_discovery_falls_back_from_path_well_known_to_root_on_404() ->
server = Server("guarded", on_list_tools=list_tools)

prm = ProtectedResourceMetadata(
resource=AnyHttpUrl(f"{BASE_URL}/mcp"), authorization_servers=[AnyHttpUrl(BASE_URL)]
resource=AnyHttpUrl(f"{BASE_URL}/mcp"), authorization_servers=[cast(AnyHttpUrl, BASE_URL)]
)
app_shim = shim(
not_found=frozenset({PRM_PATH_SUFFIXED}),
Expand Down Expand Up @@ -188,7 +189,7 @@ async def test_prm_with_a_mismatched_resource_aborts_the_flow_before_authorize()
server = Server("guarded", on_list_tools=list_tools)

prm = ProtectedResourceMetadata(
resource=AnyHttpUrl(f"{BASE_URL}/other"), authorization_servers=[AnyHttpUrl(BASE_URL)]
resource=AnyHttpUrl(f"{BASE_URL}/other"), authorization_servers=[cast(AnyHttpUrl, BASE_URL)]
)
app_shim = shim(serve={PRM_PATH_SUFFIXED: metadata_body(prm)})

Expand Down
2 changes: 1 addition & 1 deletion tests/interaction/auth/test_flow.py
Original file line number Diff line number Diff line change
Expand Up @@ -237,4 +237,4 @@ async def test_shimmed_app_serves_overrides_404s_and_otherwise_forwards_to_the_w

forwarded = await http.get("/.well-known/oauth-authorization-server")
assert forwarded.status_code == 200
assert forwarded.json()["issuer"] == "http://127.0.0.1:8000/"
assert forwarded.json()["issuer"] == "http://127.0.0.1:8000"
6 changes: 3 additions & 3 deletions tests/interaction/auth/test_identity_assertion.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@
ID_JAG_GRANT_PROFILE = "urn:ietf:params:oauth:grant-profile:id-jag"
CLIENT_ID = "enterprise-mcp-client"
CLIENT_SECRET = "enterprise-secret"
# The AS metadata issuer carries a trailing slash (built from an AnyHttpUrl object); the client
# The AS metadata issuer is path-less and slash-free (matches build_metadata); the client
# pins against exactly that.
EXPECTED_ISSUER = f"{BASE_URL}/"
EXPECTED_ISSUER = BASE_URL


async def list_tools(ctx: ServerRequestContext, params: types.PaginatedRequestParams | None) -> ListToolsResult:
Expand Down Expand Up @@ -215,7 +215,7 @@ async def test_unexpected_issuer_aborts_before_sending_credentials() -> None:
provider = InMemoryAuthorizationServerProvider()
preregister_confidential_client(provider)
server = Server("guarded", on_list_tools=list_tools)
# The served AS metadata has issuer BASE_URL/, but the client is configured for a different one.
# The served AS metadata has issuer BASE_URL, but the client is configured for a different one.
auth = identity_assertion_provider(InMemoryTokenStorage(), issuer="https://corp-as.example/", record=record)

with anyio.fail_after(5):
Expand Down
7 changes: 4 additions & 3 deletions tests/interaction/auth/test_lifecycle.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import base64
from collections import Counter
from typing import cast
from urllib.parse import parse_qsl, urlsplit

import anyio
Expand Down Expand Up @@ -69,7 +70,7 @@ def path_counts(recorded: list[RecordedRequest]) -> Counter[tuple[str, str]]:
def cimd_supported_metadata() -> bytes:
"""AS metadata advertising `client_id_metadata_document_supported: true` (the SDK server never sets it)."""
metadata = OAuthMetadata(
issuer=AnyHttpUrl(f"{BASE_URL}/"),
issuer=cast(AnyHttpUrl, BASE_URL),
authorization_endpoint=AnyHttpUrl(f"{BASE_URL}/authorize"),
token_endpoint=AnyHttpUrl(f"{BASE_URL}/token"),
registration_endpoint=AnyHttpUrl(f"{BASE_URL}/register"),
Expand Down Expand Up @@ -239,7 +240,7 @@ async def test_credentials_bound_to_a_different_issuer_are_discarded_and_the_cli
# The persisted client is now bound to the current AS.
assert storage.client_info is not None
assert storage.client_info.client_id != "stale-as-client"
assert storage.client_info.issuer == f"{BASE_URL}/"
assert storage.client_info.issuer == BASE_URL


@requirement("client-auth:401-after-auth-throws")
Expand Down Expand Up @@ -437,7 +438,7 @@ async def assertion_provider(audience: str) -> str:
result = await client.list_tools()

assert result.tools[0].name == "echo"
assert audiences == [f"{BASE_URL}/"]
assert audiences == [BASE_URL]

[token_req] = find(recorded, "POST", "/token")
body = form_body(token_req)
Expand Down
4 changes: 2 additions & 2 deletions tests/server/auth/test_protected_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ async def test_metadata_endpoint_without_path(root_resource_client: httpx.AsyncC
assert response.status_code == 200
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"],
Expand Down
11 changes: 11 additions & 0 deletions tests/server/auth/test_routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,14 @@ def test_build_metadata_serves_issuer_without_trailing_slash():
assert served["issuer"] == "https://as.example.com"
assert served["authorization_endpoint"] == "https://as.example.com/authorize"
assert served["token_endpoint"] == "https://as.example.com/token"


def test_build_metadata_strips_trailing_slash_from_anyhttpurl_issuer():
"""AnyHttpUrl adds a trailing slash to bare hostnames; served metadata must not."""
issuer_url = AnyHttpUrl("http://localhost:8000")
assert str(issuer_url).endswith("/")

metadata = build_metadata(issuer_url, None, ClientRegistrationOptions(), RevocationOptions())

served = metadata.model_dump(mode="json", exclude_none=True)
assert served["issuer"] == "http://localhost:8000"
2 changes: 1 addition & 1 deletion tests/server/mcpserver/auth/test_auth_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ async def test_metadata_endpoint(self, test_client: httpx.AsyncClient):
assert response.status_code == 200

metadata = response.json()
assert metadata["issuer"] == "https://auth.example.com/"
assert metadata["issuer"] == "https://auth.example.com"
assert metadata["authorization_endpoint"] == "https://auth.example.com/authorize"
assert metadata["token_endpoint"] == "https://auth.example.com/token"
assert metadata["registration_endpoint"] == "https://auth.example.com/register"
Expand Down
Loading