From 5577323240e0f9038b05addc16ccaba3aeeafe06 Mon Sep 17 00:00:00 2001 From: Pedro Rodrigues Date: Wed, 21 May 2025 16:58:46 -0700 Subject: [PATCH 1/7] fix auth metadata paths --- src/mcp/server/auth/routes.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/mcp/server/auth/routes.py b/src/mcp/server/auth/routes.py index 4c56ca247..c3424ba49 100644 --- a/src/mcp/server/auth/routes.py +++ b/src/mcp/server/auth/routes.py @@ -1,3 +1,5 @@ +import posixpath + from collections.abc import Awaitable, Callable from typing import Any @@ -166,11 +168,14 @@ def build_metadata( client_registration_options: ClientRegistrationOptions, revocation_options: RevocationOptions, ) -> OAuthMetadata: + def append_path(base: str, suffix: str) -> str: + return posixpath.join(base.rstrip("/"), suffix.lstrip("/")) + authorization_url = modify_url_path( - issuer_url, lambda path: path.rstrip("/") + AUTHORIZATION_PATH.lstrip("/") + issuer_url, lambda path: append_path(path, AUTHORIZATION_PATH) ) token_url = modify_url_path( - issuer_url, lambda path: path.rstrip("/") + TOKEN_PATH.lstrip("/") + issuer_url, lambda path: append_path(path, TOKEN_PATH) ) # Create metadata metadata = OAuthMetadata( @@ -194,13 +199,13 @@ def build_metadata( # Add registration endpoint if supported if client_registration_options.enabled: metadata.registration_endpoint = modify_url_path( - issuer_url, lambda path: path.rstrip("/") + REGISTRATION_PATH.lstrip("/") + issuer_url, lambda path: append_path(path, REGISTRATION_PATH) ) # Add revocation endpoint if supported if revocation_options.enabled: metadata.revocation_endpoint = modify_url_path( - issuer_url, lambda path: path.rstrip("/") + REVOCATION_PATH.lstrip("/") + issuer_url, lambda path: append_path(path, REVOCATION_PATH) ) metadata.revocation_endpoint_auth_methods_supported = ["client_secret_post"] From 521d3bcd56bd23f9dc4122c965903215f922527d Mon Sep 17 00:00:00 2001 From: Pedro Rodrigues Date: Wed, 21 May 2025 17:36:55 -0700 Subject: [PATCH 2/7] fix lint --- src/mcp/server/auth/routes.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/mcp/server/auth/routes.py b/src/mcp/server/auth/routes.py index c3424ba49..201e0c825 100644 --- a/src/mcp/server/auth/routes.py +++ b/src/mcp/server/auth/routes.py @@ -1,5 +1,4 @@ import posixpath - from collections.abc import Awaitable, Callable from typing import Any @@ -174,9 +173,7 @@ def append_path(base: str, suffix: str) -> str: authorization_url = modify_url_path( issuer_url, lambda path: append_path(path, AUTHORIZATION_PATH) ) - token_url = modify_url_path( - issuer_url, lambda path: append_path(path, TOKEN_PATH) - ) + token_url = modify_url_path(issuer_url, lambda path: append_path(path, TOKEN_PATH)) # Create metadata metadata = OAuthMetadata( issuer=issuer_url, From 18f88ffdbeaa72895b6079b3caca08158a1f2243 Mon Sep 17 00:00:00 2001 From: Pedro Rodrigues Date: Wed, 21 May 2025 19:01:05 -0700 Subject: [PATCH 3/7] more robust solution --- src/mcp/server/auth/routes.py | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/src/mcp/server/auth/routes.py b/src/mcp/server/auth/routes.py index 201e0c825..212141fd9 100644 --- a/src/mcp/server/auth/routes.py +++ b/src/mcp/server/auth/routes.py @@ -1,4 +1,5 @@ import posixpath +import urllib.parse from collections.abc import Awaitable, Callable from typing import Any @@ -167,13 +168,28 @@ def build_metadata( client_registration_options: ClientRegistrationOptions, revocation_options: RevocationOptions, ) -> OAuthMetadata: - def append_path(base: str, suffix: str) -> str: - return posixpath.join(base.rstrip("/"), suffix.lstrip("/")) + + def append_path(issuer_url: str, endpoint_path: str) -> str: + parsed = urllib.parse.urlparse(issuer_url) + + base_path = parsed.path.rstrip("/") + endpoint_path = endpoint_path.lstrip("/") + new_path = posixpath.join(base_path, endpoint_path) + + if not new_path.startswith("/"): + new_path = "/" + new_path + + new_url = urllib.parse.urlunparse(parsed._replace(path=new_path)) + + if new_url.startswith("/"): + new_url = new_url[1:] + return new_url authorization_url = modify_url_path( issuer_url, lambda path: append_path(path, AUTHORIZATION_PATH) ) token_url = modify_url_path(issuer_url, lambda path: append_path(path, TOKEN_PATH)) + # Create metadata metadata = OAuthMetadata( issuer=issuer_url, @@ -206,4 +222,4 @@ def append_path(base: str, suffix: str) -> str: ) metadata.revocation_endpoint_auth_methods_supported = ["client_secret_post"] - return metadata + return metadata \ No newline at end of file From c7068b3953861fd0f45386b827175639701cb203 Mon Sep 17 00:00:00 2001 From: Pedro Rodrigues Date: Fri, 23 May 2025 15:39:05 -0700 Subject: [PATCH 4/7] refactor append_path --- src/mcp/server/auth/routes.py | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/src/mcp/server/auth/routes.py b/src/mcp/server/auth/routes.py index 212141fd9..2324b0706 100644 --- a/src/mcp/server/auth/routes.py +++ b/src/mcp/server/auth/routes.py @@ -1,4 +1,3 @@ -import posixpath import urllib.parse from collections.abc import Awaitable, Callable from typing import Any @@ -167,23 +166,18 @@ def build_metadata( service_documentation_url: AnyHttpUrl | None, client_registration_options: ClientRegistrationOptions, revocation_options: RevocationOptions, -) -> OAuthMetadata: +) -> OAuthMetadata: + def append_path(path: str, endpoint_path: str) -> str: + # Ensures the path ends with a slash + path = f"{path}/" - def append_path(issuer_url: str, endpoint_path: str) -> str: - parsed = urllib.parse.urlparse(issuer_url) + # Ensures the endpoint path does not start with a slash + endpoint_path_lstrip = endpoint_path.lstrip("/") - base_path = parsed.path.rstrip("/") - endpoint_path = endpoint_path.lstrip("/") - new_path = posixpath.join(base_path, endpoint_path) - - if not new_path.startswith("/"): - new_path = "/" + new_path + # Join the two paths and remove leading slashes This ensures that the final + # path doesn't have double slashes between the host and the endpoint + return urllib.parse.urljoin(path, endpoint_path_lstrip).lstrip("/") - new_url = urllib.parse.urlunparse(parsed._replace(path=new_path)) - - if new_url.startswith("/"): - new_url = new_url[1:] - return new_url authorization_url = modify_url_path( issuer_url, lambda path: append_path(path, AUTHORIZATION_PATH) From 99ef0d43a19e48a2f5573d1adc848d4c2d31e4ee Mon Sep 17 00:00:00 2001 From: Pedro Rodrigues Date: Sat, 24 May 2025 20:05:25 -0700 Subject: [PATCH 5/7] delete modify_url_path function --- src/mcp/server/auth/routes.py | 44 ++++++++--------------------------- 1 file changed, 10 insertions(+), 34 deletions(-) diff --git a/src/mcp/server/auth/routes.py b/src/mcp/server/auth/routes.py index 2324b0706..b058c66ae 100644 --- a/src/mcp/server/auth/routes.py +++ b/src/mcp/server/auth/routes.py @@ -1,4 +1,3 @@ -import urllib.parse from collections.abc import Awaitable, Callable from typing import Any @@ -148,41 +147,18 @@ def create_auth_routes( return routes -def modify_url_path(url: AnyHttpUrl, path_mapper: Callable[[str], str]) -> AnyHttpUrl: - return AnyHttpUrl.build( - scheme=url.scheme, - username=url.username, - password=url.password, - host=url.host, - port=url.port, - path=path_mapper(url.path or ""), - query=url.query, - fragment=url.fragment, - ) - - def build_metadata( issuer_url: AnyHttpUrl, service_documentation_url: AnyHttpUrl | None, client_registration_options: ClientRegistrationOptions, revocation_options: RevocationOptions, -) -> OAuthMetadata: - def append_path(path: str, endpoint_path: str) -> str: - # Ensures the path ends with a slash - path = f"{path}/" - - # Ensures the endpoint path does not start with a slash - endpoint_path_lstrip = endpoint_path.lstrip("/") - - # Join the two paths and remove leading slashes This ensures that the final - # path doesn't have double slashes between the host and the endpoint - return urllib.parse.urljoin(path, endpoint_path_lstrip).lstrip("/") - - - authorization_url = modify_url_path( - issuer_url, lambda path: append_path(path, AUTHORIZATION_PATH) +) -> OAuthMetadata: + authorization_url = AnyHttpUrl( + str(issuer_url).rstrip("/") + AUTHORIZATION_PATH + ) + token_url = AnyHttpUrl( + str(issuer_url).rstrip("/") + TOKEN_PATH ) - token_url = modify_url_path(issuer_url, lambda path: append_path(path, TOKEN_PATH)) # Create metadata metadata = OAuthMetadata( @@ -205,14 +181,14 @@ def append_path(path: str, endpoint_path: str) -> str: # Add registration endpoint if supported if client_registration_options.enabled: - metadata.registration_endpoint = modify_url_path( - issuer_url, lambda path: append_path(path, REGISTRATION_PATH) + metadata.registration_endpoint = AnyHttpUrl( + str(issuer_url).rstrip("/") + REGISTRATION_PATH ) # Add revocation endpoint if supported if revocation_options.enabled: - metadata.revocation_endpoint = modify_url_path( - issuer_url, lambda path: append_path(path, REVOCATION_PATH) + metadata.revocation_endpoint = AnyHttpUrl( + str(issuer_url).rstrip("/") + REVOCATION_PATH ) metadata.revocation_endpoint_auth_methods_supported = ["client_secret_post"] From edd2e09b51f0621364230dcf54a99df781def6ce Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Mon, 26 May 2025 11:12:46 +0200 Subject: [PATCH 6/7] Update src/mcp/server/auth/routes.py --- src/mcp/server/auth/routes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mcp/server/auth/routes.py b/src/mcp/server/auth/routes.py index b058c66ae..b12199f21 100644 --- a/src/mcp/server/auth/routes.py +++ b/src/mcp/server/auth/routes.py @@ -192,4 +192,4 @@ def build_metadata( ) metadata.revocation_endpoint_auth_methods_supported = ["client_secret_post"] - return metadata \ No newline at end of file + return metadata From 045da79e73097c6fe0e93adec28ed62a399025a5 Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Mon, 26 May 2025 11:59:26 +0200 Subject: [PATCH 7/7] Add tests --- tests/client/test_auth.py | 76 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/tests/client/test_auth.py b/tests/client/test_auth.py index 996534e9c..781ec2129 100644 --- a/tests/client/test_auth.py +++ b/tests/client/test_auth.py @@ -10,9 +10,12 @@ import httpx import pytest +from inline_snapshot import snapshot from pydantic import AnyHttpUrl from mcp.client.auth import OAuthClientProvider +from mcp.server.auth.routes import build_metadata +from mcp.server.auth.settings import ClientRegistrationOptions, RevocationOptions from mcp.shared.auth import ( OAuthClientInformationFull, OAuthClientMetadata, @@ -905,3 +908,76 @@ async def test_token_exchange_error_basic(self, oauth_provider, oauth_client_inf await oauth_provider._exchange_code_for_token( "invalid_auth_code", oauth_client_info ) + + +@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, +): + 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 == snapshot( + OAuthMetadata( + 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=AnyHttpUrl(service_documentation_url), + revocation_endpoint=AnyHttpUrl(revocation_endpoint), + revocation_endpoint_auth_methods_supported=["client_secret_post"], + code_challenge_methods_supported=["S256"], + ) + )