From fdcaa5723ee696e5a51aa0277987c8e0ae44ec11 Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Thu, 12 Jun 2025 12:35:35 +0200 Subject: [PATCH 1/5] tests: use `inline_snapshot.Is` on parametrized test --- tests/client/test_auth.py | 74 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/tests/client/test_auth.py b/tests/client/test_auth.py index affcaa276..f8491a20b 100644 --- a/tests/client/test_auth.py +++ b/tests/client/test_auth.py @@ -6,6 +6,7 @@ import httpx import pytest +from inline_snapshot import Is, snapshot from pydantic import AnyHttpUrl, AnyUrl from mcp.client.auth import OAuthClientProvider, PKCEParameters @@ -580,3 +581,76 @@ async def test_auth_flow_with_valid_tokens(self, oauth_provider, mock_storage, v await auth_flow.asend(response) except StopAsyncIteration: pass # Expected + + +@pytest.mark.parametrize( + ( + "issuer_url", + "service_documentation_url", + "authorization_endpoint", + "token_endpoint", + "registration_endpoint", + "revocation_endpoint", + ), + ( + pytest.param( + "https://auth.example.com", + "https://auth.example.com/docs", + "https://auth.example.com/authorize", + "https://auth.example.com/token", + "https://auth.example.com/register", + "https://auth.example.com/revoke", + id="simple-url", + ), + pytest.param( + "https://auth.example.com/", + "https://auth.example.com/docs", + "https://auth.example.com/authorize", + "https://auth.example.com/token", + "https://auth.example.com/register", + "https://auth.example.com/revoke", + id="with-trailing-slash", + ), + pytest.param( + "https://auth.example.com/v1/mcp", + "https://auth.example.com/v1/mcp/docs", + "https://auth.example.com/v1/mcp/authorize", + "https://auth.example.com/v1/mcp/token", + "https://auth.example.com/v1/mcp/register", + "https://auth.example.com/v1/mcp/revoke", + id="with-path-param", + ), + ), +) +def test_build_metadata( + issuer_url: str, + service_documentation_url: str, + authorization_endpoint: str, + token_endpoint: str, + registration_endpoint: str, + revocation_endpoint: str, +): + from mcp.server.auth import ClientRegistrationOptions, RevocationOptions, build_metadata + + metadata = build_metadata( + issuer_url=AnyHttpUrl(issuer_url), + service_documentation_url=AnyHttpUrl(service_documentation_url), + client_registration_options=ClientRegistrationOptions(enabled=True, valid_scopes=["read", "write", "admin"]), + revocation_options=RevocationOptions(enabled=True), + ) + + assert metadata.model_dump(exclude_defaults=True) == snapshot( + { + "issuer": Is(AnyHttpUrl(issuer_url)), + "authorization_endpoint": Is(AnyHttpUrl(authorization_endpoint)), + "token_endpoint": Is(AnyHttpUrl(token_endpoint)), + "registration_endpoint": Is(AnyHttpUrl(registration_endpoint)), + "scopes_supported": ["read", "write", "admin"], + "grant_types_supported": ["authorization_code", "refresh_token"], + "token_endpoint_auth_methods_supported": ["client_secret_post"], + "service_documentation": Is(AnyHttpUrl(service_documentation_url)), + "revocation_endpoint": Is(AnyHttpUrl(revocation_endpoint)), + "revocation_endpoint_auth_methods_supported": ["client_secret_post"], + "code_challenge_methods_supported": ["S256"], + } + ) From a2a0d571d6dd42ef007868f88245917959ac7022 Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Thu, 12 Jun 2025 12:45:35 +0200 Subject: [PATCH 2/5] Drop Is --- tests/client/test_auth.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/client/test_auth.py b/tests/client/test_auth.py index f8491a20b..2cde6a3b5 100644 --- a/tests/client/test_auth.py +++ b/tests/client/test_auth.py @@ -6,7 +6,7 @@ import httpx import pytest -from inline_snapshot import Is, snapshot +from inline_snapshot import snapshot from pydantic import AnyHttpUrl, AnyUrl from mcp.client.auth import OAuthClientProvider, PKCEParameters @@ -641,15 +641,15 @@ def test_build_metadata( assert metadata.model_dump(exclude_defaults=True) == snapshot( { - "issuer": Is(AnyHttpUrl(issuer_url)), - "authorization_endpoint": Is(AnyHttpUrl(authorization_endpoint)), - "token_endpoint": Is(AnyHttpUrl(token_endpoint)), - "registration_endpoint": Is(AnyHttpUrl(registration_endpoint)), + "issuer": AnyHttpUrl(issuer_url), + "authorization_endpoint": AnyHttpUrl(authorization_endpoint), + "token_endpoint": AnyHttpUrl(token_endpoint), + "registration_endpoint": AnyHttpUrl(registration_endpoint), "scopes_supported": ["read", "write", "admin"], "grant_types_supported": ["authorization_code", "refresh_token"], "token_endpoint_auth_methods_supported": ["client_secret_post"], - "service_documentation": Is(AnyHttpUrl(service_documentation_url)), - "revocation_endpoint": Is(AnyHttpUrl(revocation_endpoint)), + "service_documentation": AnyHttpUrl(service_documentation_url), + "revocation_endpoint": AnyHttpUrl(revocation_endpoint), "revocation_endpoint_auth_methods_supported": ["client_secret_post"], "code_challenge_methods_supported": ["S256"], } From a1a4b3a1f90afafe25e731cdb077a9960f2c5612 Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Thu, 12 Jun 2025 13:00:01 +0200 Subject: [PATCH 3/5] update again --- tests/client/test_auth.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/client/test_auth.py b/tests/client/test_auth.py index 2cde6a3b5..be389b8db 100644 --- a/tests/client/test_auth.py +++ b/tests/client/test_auth.py @@ -6,7 +6,7 @@ import httpx import pytest -from inline_snapshot import snapshot +from inline_snapshot import Is, snapshot from pydantic import AnyHttpUrl, AnyUrl from mcp.client.auth import OAuthClientProvider, PKCEParameters @@ -639,17 +639,17 @@ def test_build_metadata( revocation_options=RevocationOptions(enabled=True), ) - assert metadata.model_dump(exclude_defaults=True) == snapshot( + assert metadata.model_dump(exclude_defaults=True, mode="json") == snapshot( { - "issuer": AnyHttpUrl(issuer_url), - "authorization_endpoint": AnyHttpUrl(authorization_endpoint), - "token_endpoint": AnyHttpUrl(token_endpoint), - "registration_endpoint": AnyHttpUrl(registration_endpoint), + "issuer": Is(issuer_url), + "authorization_endpoint": Is(authorization_endpoint), + "token_endpoint": Is(token_endpoint), + "registration_endpoint": Is(registration_endpoint), "scopes_supported": ["read", "write", "admin"], "grant_types_supported": ["authorization_code", "refresh_token"], "token_endpoint_auth_methods_supported": ["client_secret_post"], - "service_documentation": AnyHttpUrl(service_documentation_url), - "revocation_endpoint": AnyHttpUrl(revocation_endpoint), + "service_documentation": Is(service_documentation_url), + "revocation_endpoint": Is(revocation_endpoint), "revocation_endpoint_auth_methods_supported": ["client_secret_post"], "code_challenge_methods_supported": ["S256"], } From c40ada6b16d07dc33c2c00bef32f87b0802b293d Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Fri, 13 Jun 2025 10:34:18 +0200 Subject: [PATCH 4/5] Add comment and ignore test --- tests/client/test_auth.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/tests/client/test_auth.py b/tests/client/test_auth.py index be389b8db..d3d5107a1 100644 --- a/tests/client/test_auth.py +++ b/tests/client/test_auth.py @@ -593,15 +593,16 @@ async def test_auth_flow_with_valid_tokens(self, oauth_provider, mock_storage, v "revocation_endpoint", ), ( - pytest.param( - "https://auth.example.com", - "https://auth.example.com/docs", - "https://auth.example.com/authorize", - "https://auth.example.com/token", - "https://auth.example.com/register", - "https://auth.example.com/revoke", - id="simple-url", - ), + # TODO(Marcelo): Since we are using `AnyUrl`, the trailing slash is always added. + # pytest.param( + # "https://auth.example.com", + # "https://auth.example.com/docs", + # "https://auth.example.com/authorize", + # "https://auth.example.com/token", + # "https://auth.example.com/register", + # "https://auth.example.com/revoke", + # id="simple-url", + # ), pytest.param( "https://auth.example.com/", "https://auth.example.com/docs", From 84937d1f85b5c991cdbe060ba00e6e8e5b446c60 Mon Sep 17 00:00:00 2001 From: Felix Weinberger <3823880+felixweinberger@users.noreply.github.com> Date: Mon, 14 Jul 2025 15:26:28 +0100 Subject: [PATCH 5/5] tests: mark trailing slash test as xfail (#1138) --- tests/client/test_auth.py | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/tests/client/test_auth.py b/tests/client/test_auth.py index d3d5107a1..d64687ff8 100644 --- a/tests/client/test_auth.py +++ b/tests/client/test_auth.py @@ -593,16 +593,20 @@ async def test_auth_flow_with_valid_tokens(self, oauth_provider, mock_storage, v "revocation_endpoint", ), ( - # TODO(Marcelo): Since we are using `AnyUrl`, the trailing slash is always added. - # pytest.param( - # "https://auth.example.com", - # "https://auth.example.com/docs", - # "https://auth.example.com/authorize", - # "https://auth.example.com/token", - # "https://auth.example.com/register", - # "https://auth.example.com/revoke", - # id="simple-url", - # ), + # Pydantic's AnyUrl incorrectly adds trailing slash to base URLs + # This is being fixed in https://github.com/pydantic/pydantic-core/pull/1719 (Pydantic 2.12+) + pytest.param( + "https://auth.example.com", + "https://auth.example.com/docs", + "https://auth.example.com/authorize", + "https://auth.example.com/token", + "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/", "https://auth.example.com/docs", @@ -631,7 +635,8 @@ def test_build_metadata( registration_endpoint: str, revocation_endpoint: str, ): - from mcp.server.auth import ClientRegistrationOptions, RevocationOptions, build_metadata + from mcp.server.auth.routes import build_metadata + from mcp.server.auth.settings import ClientRegistrationOptions, RevocationOptions metadata = build_metadata( issuer_url=AnyHttpUrl(issuer_url),