Skip to content

Commit 9eb6e29

Browse files
committed
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)
1 parent ef02cc7 commit 9eb6e29

5 files changed

Lines changed: 56 additions & 33 deletions

File tree

src/mcp/server/auth/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,3 @@
11
"""MCP OAuth server authorization components."""
2+
3+
from .url_validators import validate_issuer_url, validate_redirect_uri

src/mcp/server/auth/handlers/register.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
from starlette.responses import Response
1010

1111
from mcp.server.auth.errors import stringify_pydantic_error
12-
from mcp.server.auth.routes import validate_redirect_uri
12+
from mcp.server.auth.url_validators import validate_redirect_uri
1313
from mcp.server.auth.json_response import PydanticJSONResponse
1414
from mcp.server.auth.provider import OAuthAuthorizationServerProvider, RegistrationError, RegistrationErrorCode
1515
from mcp.server.auth.settings import ClientRegistrationOptions

src/mcp/server/auth/routes.py

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -21,32 +21,6 @@
2121
from mcp.shared.inbound import MCP_PROTOCOL_VERSION_HEADER
2222

2323

24-
def validate_redirect_uri(url: AnyHttpUrl):
25-
"""Validate a registered redirect_uri for DCR.
26-
27-
RFC 9700 section 4.1.1 and RFC 7591 section 2 require HTTPS for
28-
redirect_uris, with an HTTP loopback exception for local development.
29-
30-
Args:
31-
url: The redirect URI to validate.
32-
33-
Raises:
34-
ValueError: If the redirect URI uses an unsafe scheme or contains
35-
a fragment.
36-
"""
37-
if url.scheme != "https" and url.host not in (
38-
"localhost",
39-
"127.0.0.1",
40-
"[::1]",
41-
):
42-
raise ValueError(
43-
"Redirect URI must use HTTPS (or HTTP loopback for local development)"
44-
)
45-
46-
if url.fragment is not None:
47-
raise ValueError("Redirect URI must not contain a fragment")
48-
49-
5024
def validate_issuer_url(url: AnyHttpUrl):
5125
"""Validate that the issuer URL meets OAuth 2.0 requirements.
5226
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
\"\"\"OAuth 2.0 URL validation helpers for MCP authorization servers.
2+
3+
RFC 9700 4.1.1 and RFC 7591 2 require HTTPS for authorization endpoint URLs
4+
and registered redirect_uris, with an HTTP loopback exception for local
5+
development.
6+
\"\"\"
7+
8+
from pydantic import AnyHttpUrl
9+
10+
11+
def validate_issuer_url(url: AnyHttpUrl):
12+
\"\"\"Validate that the issuer URL meets OAuth 2.0 requirements.
13+
14+
Args:
15+
url: The issuer URL to validate.
16+
17+
Raises:
18+
ValueError: If the issuer URL is invalid.
19+
\"\"\"
20+
if url.scheme != "https" and url.host not in ("localhost", "127.0.0.1", "[::1]"):
21+
raise ValueError("Issuer URL must be HTTPS")
22+
23+
if url.fragment:
24+
raise ValueError("Issuer URL must not have a fragment")
25+
if url.query:
26+
raise ValueError("Issuer URL must not have a query string")
27+
28+
29+
def validate_redirect_uri(url: AnyHttpUrl):
30+
\"\"\"Validate a registered redirect_uri for DCR.
31+
32+
RFC 9700 section 4.1.1 and RFC 7591 section 2 require HTTPS for
33+
redirect_uris, with an HTTP loopback exception for local development.
34+
35+
Args:
36+
url: The redirect URI to validate.
37+
38+
Raises:
39+
ValueError: If the redirect URI uses an unsafe scheme or contains
40+
a fragment.
41+
\"\"\"
42+
if url.scheme != "https" and url.host not in ("localhost", "127.0.0.1", "[::1]"):
43+
raise ValueError("Redirect URI must use HTTPS (or HTTP loopback for local development)")
44+
45+
if url.fragment is not None:
46+
raise ValueError("Redirect URI must not contain a fragment")

tests/server/auth/test_routes.py

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import pytest
22
from pydantic import AnyHttpUrl
33

4-
from mcp.server.auth.routes import build_metadata, validate_issuer_url, validate_redirect_uri
4+
from mcp.server.auth.routes import build_metadata, validate_issuer_url
5+
from mcp.server.auth.url_validators import validate_redirect_uri
56
from mcp.server.auth.settings import AuthSettings, ClientRegistrationOptions, RevocationOptions
67

78

@@ -88,25 +89,25 @@ def test_validate_redirect_uri_http_ipv6_loopback_allowed():
8889

8990

9091
def test_validate_redirect_uri_javascript_scheme_rejected():
91-
with pytest.raises(ValueError, match='Redirect URI must use HTTPS'):
92+
with pytest.raises(ValueError, match="Redirect URI must use HTTPS"):
9293
validate_redirect_uri(AnyHttpUrl('javascript:alert(1)'))
9394

9495

9596
def test_validate_redirect_uri_file_scheme_rejected():
96-
with pytest.raises(ValueError, match='Redirect URI must use HTTPS'):
97+
with pytest.raises(ValueError, match="Redirect URI must use HTTPS"):
9798
validate_redirect_uri(AnyHttpUrl('file:///etc/passwd'))
9899

99100

100101
def test_validate_redirect_uri_http_non_loopback_rejected():
101-
with pytest.raises(ValueError, match='Redirect URI must use HTTPS'):
102+
with pytest.raises(ValueError, match="Redirect URI must use HTTPS"):
102103
validate_redirect_uri(AnyHttpUrl('http://evil.com/cb'))
103104

104105

105106
def test_validate_redirect_uri_fragment_rejected():
106-
with pytest.raises(ValueError, match='Redirect URI must not contain a fragment'):
107+
with pytest.raises(ValueError, match="Redirect URI must not contain a fragment"):
107108
validate_redirect_uri(AnyHttpUrl('https://example.com/cb#frag'))
108109

109110

110111
def test_validate_redirect_uri_empty_fragment_rejected():
111-
with pytest.raises(ValueError, match='Redirect URI must not contain a fragment'):
112+
with pytest.raises(ValueError, match="Redirect URI must not contain a fragment"):
112113
validate_redirect_uri(AnyHttpUrl('https://example.com/cb#'))

0 commit comments

Comments
 (0)