From 6c627572da57880de9297e63549beb7756908d16 Mon Sep 17 00:00:00 2001 From: Varshith Puli Date: Tue, 28 Jul 2026 22:09:17 +0530 Subject: [PATCH 1/5] Validates registered redirect_uris for DCR are a secure schema with no fragments --- src/mcp/server/auth/handlers/register.py | 15 +++++++++ src/mcp/server/auth/routes.py | 26 +++++++++++++++ tests/server/auth/test_routes.py | 42 +++++++++++++++++++++++- 3 files changed, 82 insertions(+), 1 deletion(-) diff --git a/src/mcp/server/auth/handlers/register.py b/src/mcp/server/auth/handlers/register.py index 7fb14b2c43..0f9378e458 100644 --- a/src/mcp/server/auth/handlers/register.py +++ b/src/mcp/server/auth/handlers/register.py @@ -9,6 +9,7 @@ from starlette.responses import Response from mcp.server.auth.errors import stringify_pydantic_error +from mcp.server.auth.routes import validate_redirect_uri from mcp.server.auth.json_response import PydanticJSONResponse from mcp.server.auth.provider import OAuthAuthorizationServerProvider, RegistrationError, RegistrationErrorCode from mcp.server.auth.settings import ClientRegistrationOptions @@ -35,6 +36,20 @@ async def handle(self, request: Request) -> Response: body = await request.body() client_metadata = OAuthClientMetadata.model_validate_json(body) + # Validate redirect_uris per RFC 7591 section 2 + if client_metadata.redirect_uris: + for uri in client_metadata.redirect_uris: + try: + validate_redirect_uri(uri) + except ValueError as e: + return PydanticJSONResponse( + content=RegistrationErrorResponse( + error="invalid_redirect_uri", + error_description=str(e), + ), + status_code=400, + ) + # Scope validation is handled below except ValidationError as validation_error: return PydanticJSONResponse( diff --git a/src/mcp/server/auth/routes.py b/src/mcp/server/auth/routes.py index fa88dddcf4..1764e95fed 100644 --- a/src/mcp/server/auth/routes.py +++ b/src/mcp/server/auth/routes.py @@ -21,6 +21,32 @@ from mcp.shared.inbound import MCP_PROTOCOL_VERSION_HEADER +def validate_redirect_uri(url: AnyHttpUrl): + """Validate a registered redirect_uri for DCR. + + RFC 9700 section 4.1.1 and RFC 7591 section 2 require HTTPS for + redirect_uris, with an HTTP loopback exception for local development. + + Args: + url: The redirect URI to validate. + + Raises: + ValueError: If the redirect URI uses an unsafe scheme or contains + a fragment. + """ + if url.scheme != "https" and url.host not in ( + "localhost", + "127.0.0.1", + "[::1]", + ): + raise ValueError( + "Redirect URI must use HTTPS (or HTTP loopback for local development)" + ) + + if url.fragment is not None: + raise ValueError("Redirect URI must not contain a fragment") + + def validate_issuer_url(url: AnyHttpUrl): """Validate that the issuer URL meets OAuth 2.0 requirements. diff --git a/tests/server/auth/test_routes.py b/tests/server/auth/test_routes.py index 58685c64c7..5cd3c8748b 100644 --- a/tests/server/auth/test_routes.py +++ b/tests/server/auth/test_routes.py @@ -1,7 +1,7 @@ import pytest from pydantic import AnyHttpUrl -from mcp.server.auth.routes import build_metadata, validate_issuer_url +from mcp.server.auth.routes import build_metadata, validate_issuer_url, validate_redirect_uri from mcp.server.auth.settings import AuthSettings, ClientRegistrationOptions, RevocationOptions @@ -70,3 +70,43 @@ 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_validate_redirect_uri_https_allowed(): + validate_redirect_uri(AnyHttpUrl('https://example.com/cb')) + + +def test_validate_redirect_uri_http_localhost_allowed(): + validate_redirect_uri(AnyHttpUrl('http://localhost:3000/cb')) + + +def test_validate_redirect_uri_http_127_0_0_1_allowed(): + validate_redirect_uri(AnyHttpUrl('http://127.0.0.1:8080/cb')) + + +def test_validate_redirect_uri_http_ipv6_loopback_allowed(): + validate_redirect_uri(AnyHttpUrl('http://[::1]:9090/cb')) + + +def test_validate_redirect_uri_javascript_scheme_rejected(): + with pytest.raises(ValueError, match='Redirect URI must use HTTPS'): + validate_redirect_uri(AnyHttpUrl('javascript:alert(1)')) + + +def test_validate_redirect_uri_file_scheme_rejected(): + with pytest.raises(ValueError, match='Redirect URI must use HTTPS'): + validate_redirect_uri(AnyHttpUrl('file:///etc/passwd')) + + +def test_validate_redirect_uri_http_non_loopback_rejected(): + with pytest.raises(ValueError, match='Redirect URI must use HTTPS'): + validate_redirect_uri(AnyHttpUrl('http://evil.com/cb')) + + +def test_validate_redirect_uri_fragment_rejected(): + with pytest.raises(ValueError, match='Redirect URI must not contain a fragment'): + validate_redirect_uri(AnyHttpUrl('https://example.com/cb#frag')) + + +def test_validate_redirect_uri_empty_fragment_rejected(): + with pytest.raises(ValueError, match='Redirect URI must not contain a fragment'): + validate_redirect_uri(AnyHttpUrl('https://example.com/cb#')) From fb893dd02deb7448961d87d10e087dd11113641c Mon Sep 17 00:00:00 2001 From: Varshith Puli Date: Wed, 29 Jul 2026 20:37:13 +0530 Subject: [PATCH 2/5] Fix circular import by moving validate_redirect_uri to url_validators module The original implementation put validate_redirect_uri in routes.py, which caused a circular import: register.py imports from routes, and routes imports from other modules that import back. Moving the validation functions to a dedicated url_validators.py module breaks the cycle. - validate_redirect_uri now lives in url_validators.py alongside validate_issuer_url (previously in routes.py) - register.py imports from url_validators instead of routes - test_routes.py updated to import from url_validators - Ruff format fixes applied (single-line raise, double-quote match strings) --- src/mcp/server/auth/__init__.py | 2 ++ src/mcp/server/auth/handlers/register.py | 2 +- src/mcp/server/auth/routes.py | 26 -------------- src/mcp/server/auth/url_validators.py | 46 ++++++++++++++++++++++++ tests/server/auth/test_routes.py | 13 +++---- 5 files changed, 56 insertions(+), 33 deletions(-) create mode 100644 src/mcp/server/auth/url_validators.py diff --git a/src/mcp/server/auth/__init__.py b/src/mcp/server/auth/__init__.py index 61b60e3487..34f2e3e1c9 100644 --- a/src/mcp/server/auth/__init__.py +++ b/src/mcp/server/auth/__init__.py @@ -1 +1,3 @@ """MCP OAuth server authorization components.""" + +from .url_validators import validate_issuer_url, validate_redirect_uri diff --git a/src/mcp/server/auth/handlers/register.py b/src/mcp/server/auth/handlers/register.py index 0f9378e458..ff41d95bd0 100644 --- a/src/mcp/server/auth/handlers/register.py +++ b/src/mcp/server/auth/handlers/register.py @@ -9,7 +9,7 @@ from starlette.responses import Response from mcp.server.auth.errors import stringify_pydantic_error -from mcp.server.auth.routes import validate_redirect_uri +from mcp.server.auth.url_validators import validate_redirect_uri from mcp.server.auth.json_response import PydanticJSONResponse from mcp.server.auth.provider import OAuthAuthorizationServerProvider, RegistrationError, RegistrationErrorCode from mcp.server.auth.settings import ClientRegistrationOptions diff --git a/src/mcp/server/auth/routes.py b/src/mcp/server/auth/routes.py index 1764e95fed..fa88dddcf4 100644 --- a/src/mcp/server/auth/routes.py +++ b/src/mcp/server/auth/routes.py @@ -21,32 +21,6 @@ from mcp.shared.inbound import MCP_PROTOCOL_VERSION_HEADER -def validate_redirect_uri(url: AnyHttpUrl): - """Validate a registered redirect_uri for DCR. - - RFC 9700 section 4.1.1 and RFC 7591 section 2 require HTTPS for - redirect_uris, with an HTTP loopback exception for local development. - - Args: - url: The redirect URI to validate. - - Raises: - ValueError: If the redirect URI uses an unsafe scheme or contains - a fragment. - """ - if url.scheme != "https" and url.host not in ( - "localhost", - "127.0.0.1", - "[::1]", - ): - raise ValueError( - "Redirect URI must use HTTPS (or HTTP loopback for local development)" - ) - - if url.fragment is not None: - raise ValueError("Redirect URI must not contain a fragment") - - def validate_issuer_url(url: AnyHttpUrl): """Validate that the issuer URL meets OAuth 2.0 requirements. diff --git a/src/mcp/server/auth/url_validators.py b/src/mcp/server/auth/url_validators.py new file mode 100644 index 0000000000..dbd2cf58a6 --- /dev/null +++ b/src/mcp/server/auth/url_validators.py @@ -0,0 +1,46 @@ +\"\"\"OAuth 2.0 URL validation helpers for MCP authorization servers. + +RFC 9700 4.1.1 and RFC 7591 2 require HTTPS for authorization endpoint URLs +and registered redirect_uris, with an HTTP loopback exception for local +development. +\"\"\" + +from pydantic import AnyHttpUrl + + +def validate_issuer_url(url: AnyHttpUrl): + \"\"\"Validate that the issuer URL meets OAuth 2.0 requirements. + + Args: + url: The issuer URL to validate. + + Raises: + ValueError: If the issuer URL is invalid. + \"\"\" + if url.scheme != "https" and url.host not in ("localhost", "127.0.0.1", "[::1]"): + raise ValueError("Issuer URL must be HTTPS") + + if url.fragment: + raise ValueError("Issuer URL must not have a fragment") + if url.query: + raise ValueError("Issuer URL must not have a query string") + + +def validate_redirect_uri(url: AnyHttpUrl): + \"\"\"Validate a registered redirect_uri for DCR. + + RFC 9700 section 4.1.1 and RFC 7591 section 2 require HTTPS for + redirect_uris, with an HTTP loopback exception for local development. + + Args: + url: The redirect URI to validate. + + Raises: + ValueError: If the redirect URI uses an unsafe scheme or contains + a fragment. + \"\"\" + if url.scheme != "https" and url.host not in ("localhost", "127.0.0.1", "[::1]"): + raise ValueError("Redirect URI must use HTTPS (or HTTP loopback for local development)") + + if url.fragment is not None: + raise ValueError("Redirect URI must not contain a fragment") diff --git a/tests/server/auth/test_routes.py b/tests/server/auth/test_routes.py index 5cd3c8748b..a2c627d393 100644 --- a/tests/server/auth/test_routes.py +++ b/tests/server/auth/test_routes.py @@ -1,7 +1,8 @@ import pytest from pydantic import AnyHttpUrl -from mcp.server.auth.routes import build_metadata, validate_issuer_url, validate_redirect_uri +from mcp.server.auth.routes import build_metadata, validate_issuer_url +from mcp.server.auth.url_validators import validate_redirect_uri from mcp.server.auth.settings import AuthSettings, ClientRegistrationOptions, RevocationOptions @@ -88,25 +89,25 @@ def test_validate_redirect_uri_http_ipv6_loopback_allowed(): def test_validate_redirect_uri_javascript_scheme_rejected(): - with pytest.raises(ValueError, match='Redirect URI must use HTTPS'): + with pytest.raises(ValueError, match="Redirect URI must use HTTPS"): validate_redirect_uri(AnyHttpUrl('javascript:alert(1)')) def test_validate_redirect_uri_file_scheme_rejected(): - with pytest.raises(ValueError, match='Redirect URI must use HTTPS'): + with pytest.raises(ValueError, match="Redirect URI must use HTTPS"): validate_redirect_uri(AnyHttpUrl('file:///etc/passwd')) def test_validate_redirect_uri_http_non_loopback_rejected(): - with pytest.raises(ValueError, match='Redirect URI must use HTTPS'): + with pytest.raises(ValueError, match="Redirect URI must use HTTPS"): validate_redirect_uri(AnyHttpUrl('http://evil.com/cb')) def test_validate_redirect_uri_fragment_rejected(): - with pytest.raises(ValueError, match='Redirect URI must not contain a fragment'): + with pytest.raises(ValueError, match="Redirect URI must not contain a fragment"): validate_redirect_uri(AnyHttpUrl('https://example.com/cb#frag')) def test_validate_redirect_uri_empty_fragment_rejected(): - with pytest.raises(ValueError, match='Redirect URI must not contain a fragment'): + with pytest.raises(ValueError, match="Redirect URI must not contain a fragment"): validate_redirect_uri(AnyHttpUrl('https://example.com/cb#')) From 9060ac0f21d7c42b5c249e45f984a96d3f1d0775 Mon Sep 17 00:00:00 2001 From: Varshith Puli Date: Wed, 29 Jul 2026 20:42:45 +0530 Subject: [PATCH 3/5] Fix BOM, CRLF line endings, and escaped quotes in url_validators --- src/mcp/server/auth/url_validators.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/mcp/server/auth/url_validators.py b/src/mcp/server/auth/url_validators.py index dbd2cf58a6..4975ddb208 100644 --- a/src/mcp/server/auth/url_validators.py +++ b/src/mcp/server/auth/url_validators.py @@ -1,22 +1,22 @@ -\"\"\"OAuth 2.0 URL validation helpers for MCP authorization servers. +"""OAuth 2.0 URL validation helpers for MCP authorization servers. RFC 9700 4.1.1 and RFC 7591 2 require HTTPS for authorization endpoint URLs and registered redirect_uris, with an HTTP loopback exception for local development. -\"\"\" +""" from pydantic import AnyHttpUrl def validate_issuer_url(url: AnyHttpUrl): - \"\"\"Validate that the issuer URL meets OAuth 2.0 requirements. + """Validate that the issuer URL meets OAuth 2.0 requirements. Args: url: The issuer URL to validate. Raises: ValueError: If the issuer URL is invalid. - \"\"\" + """ if url.scheme != "https" and url.host not in ("localhost", "127.0.0.1", "[::1]"): raise ValueError("Issuer URL must be HTTPS") @@ -27,7 +27,7 @@ def validate_issuer_url(url: AnyHttpUrl): def validate_redirect_uri(url: AnyHttpUrl): - \"\"\"Validate a registered redirect_uri for DCR. + """Validate a registered redirect_uri for DCR. RFC 9700 section 4.1.1 and RFC 7591 section 2 require HTTPS for redirect_uris, with an HTTP loopback exception for local development. @@ -38,7 +38,7 @@ def validate_redirect_uri(url: AnyHttpUrl): Raises: ValueError: If the redirect URI uses an unsafe scheme or contains a fragment. - \"\"\" + """ if url.scheme != "https" and url.host not in ("localhost", "127.0.0.1", "[::1]"): raise ValueError("Redirect URI must use HTTPS (or HTTP loopback for local development)") From 0561256c7e80a44b7cfa98b5dc596557d796dcf1 Mon Sep 17 00:00:00 2001 From: Varshith Puli Date: Wed, 29 Jul 2026 22:02:35 +0530 Subject: [PATCH 4/5] Apply ruff formatting to test_routes.py --- tests/server/auth/test_routes.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/tests/server/auth/test_routes.py b/tests/server/auth/test_routes.py index a2c627d393..ede29b3e78 100644 --- a/tests/server/auth/test_routes.py +++ b/tests/server/auth/test_routes.py @@ -2,8 +2,8 @@ from pydantic import AnyHttpUrl from mcp.server.auth.routes import build_metadata, validate_issuer_url -from mcp.server.auth.url_validators import validate_redirect_uri from mcp.server.auth.settings import AuthSettings, ClientRegistrationOptions, RevocationOptions +from mcp.server.auth.url_validators import validate_redirect_uri def test_validate_issuer_url_https_allowed(): @@ -72,42 +72,43 @@ def test_build_metadata_serves_issuer_without_trailing_slash(): assert served["authorization_endpoint"] == "https://as.example.com/authorize" assert served["token_endpoint"] == "https://as.example.com/token" + def test_validate_redirect_uri_https_allowed(): - validate_redirect_uri(AnyHttpUrl('https://example.com/cb')) + validate_redirect_uri(AnyHttpUrl("https://example.com/cb")) def test_validate_redirect_uri_http_localhost_allowed(): - validate_redirect_uri(AnyHttpUrl('http://localhost:3000/cb')) + validate_redirect_uri(AnyHttpUrl("http://localhost:3000/cb")) def test_validate_redirect_uri_http_127_0_0_1_allowed(): - validate_redirect_uri(AnyHttpUrl('http://127.0.0.1:8080/cb')) + validate_redirect_uri(AnyHttpUrl("http://127.0.0.1:8080/cb")) def test_validate_redirect_uri_http_ipv6_loopback_allowed(): - validate_redirect_uri(AnyHttpUrl('http://[::1]:9090/cb')) + validate_redirect_uri(AnyHttpUrl("http://[::1]:9090/cb")) def test_validate_redirect_uri_javascript_scheme_rejected(): with pytest.raises(ValueError, match="Redirect URI must use HTTPS"): - validate_redirect_uri(AnyHttpUrl('javascript:alert(1)')) + validate_redirect_uri(AnyHttpUrl("javascript:alert(1)")) def test_validate_redirect_uri_file_scheme_rejected(): with pytest.raises(ValueError, match="Redirect URI must use HTTPS"): - validate_redirect_uri(AnyHttpUrl('file:///etc/passwd')) + validate_redirect_uri(AnyHttpUrl("file:///etc/passwd")) def test_validate_redirect_uri_http_non_loopback_rejected(): with pytest.raises(ValueError, match="Redirect URI must use HTTPS"): - validate_redirect_uri(AnyHttpUrl('http://evil.com/cb')) + validate_redirect_uri(AnyHttpUrl("http://evil.com/cb")) def test_validate_redirect_uri_fragment_rejected(): with pytest.raises(ValueError, match="Redirect URI must not contain a fragment"): - validate_redirect_uri(AnyHttpUrl('https://example.com/cb#frag')) + validate_redirect_uri(AnyHttpUrl("https://example.com/cb#frag")) def test_validate_redirect_uri_empty_fragment_rejected(): with pytest.raises(ValueError, match="Redirect URI must not contain a fragment"): - validate_redirect_uri(AnyHttpUrl('https://example.com/cb#')) + validate_redirect_uri(AnyHttpUrl("https://example.com/cb#")) From 4e7749096cdc5b112057400dcea9e893233d2f6b Mon Sep 17 00:00:00 2001 From: Varshith Puli Date: Thu, 30 Jul 2026 00:00:27 +0530 Subject: [PATCH 5/5] Only reject non-HTTP(S) schemes and fragments for redirect URIs The SDK intentionally accepts non-loopback HTTP redirect URIs per existing tests (test_a_non_loopback_http_redirect_uri_is_accepted). Narrow scope to only reject dangerous schemes (javascript:, data:, file:, etc.) and fragments, matching the SDK's existing policy. --- src/mcp/server/auth/url_validators.py | 4 ++-- tests/server/auth/test_routes.py | 5 ++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/mcp/server/auth/url_validators.py b/src/mcp/server/auth/url_validators.py index 4975ddb208..60b6f5d55e 100644 --- a/src/mcp/server/auth/url_validators.py +++ b/src/mcp/server/auth/url_validators.py @@ -39,8 +39,8 @@ def validate_redirect_uri(url: AnyHttpUrl): ValueError: If the redirect URI uses an unsafe scheme or contains a fragment. """ - if url.scheme != "https" and url.host not in ("localhost", "127.0.0.1", "[::1]"): - raise ValueError("Redirect URI must use HTTPS (or HTTP loopback for local development)") + if url.scheme not in ("http", "https"): + raise ValueError("Redirect URI must use an HTTP(S) scheme") if url.fragment is not None: raise ValueError("Redirect URI must not contain a fragment") diff --git a/tests/server/auth/test_routes.py b/tests/server/auth/test_routes.py index ede29b3e78..ad2935fdaf 100644 --- a/tests/server/auth/test_routes.py +++ b/tests/server/auth/test_routes.py @@ -99,9 +99,8 @@ def test_validate_redirect_uri_file_scheme_rejected(): validate_redirect_uri(AnyHttpUrl("file:///etc/passwd")) -def test_validate_redirect_uri_http_non_loopback_rejected(): - with pytest.raises(ValueError, match="Redirect URI must use HTTPS"): - validate_redirect_uri(AnyHttpUrl("http://evil.com/cb")) +def test_validate_redirect_uri_http_non_loopback_allowed(): + validate_redirect_uri(AnyHttpUrl("http://evil.com/cb")) def test_validate_redirect_uri_fragment_rejected():