Skip to content

Commit f896b40

Browse files
Validate DCR redirect_uris with HTTPS-or-loopback policy
Reject dangerous redirect URI schemes and fragments during dynamic client registration, matching the existing issuer URL validation rules. Fixes #2629
1 parent cf110e3 commit f896b40

4 files changed

Lines changed: 85 additions & 22 deletions

File tree

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from mcp.server.auth.errors import stringify_pydantic_error
1212
from mcp.server.auth.json_response import PydanticJSONResponse
1313
from mcp.server.auth.provider import OAuthAuthorizationServerProvider, RegistrationError, RegistrationErrorCode
14+
from mcp.server.auth.validation import validate_registered_redirect_uri
1415
from mcp.server.auth.settings import ClientRegistrationOptions
1516
from mcp.shared.auth import OAuthClientInformationFull, OAuthClientMetadata
1617

@@ -90,6 +91,19 @@ async def handle(self, request: Request) -> Response:
9091
status_code=400,
9192
)
9293

94+
if client_metadata.redirect_uris is not None:
95+
for redirect_uri in client_metadata.redirect_uris:
96+
try:
97+
validate_registered_redirect_uri(redirect_uri)
98+
except ValueError as error:
99+
return PydanticJSONResponse(
100+
content=RegistrationErrorResponse(
101+
error="invalid_client_metadata",
102+
error_description=str(error),
103+
),
104+
status_code=400,
105+
)
106+
93107
client_id_issued_at = int(time.time())
94108
client_secret_expires_at = (
95109
client_id_issued_at + self.options.client_secret_expiry_seconds

src/mcp/server/auth/routes.py

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -21,26 +21,7 @@
2121
from mcp.shared.auth import OAuthMetadata, ProtectedResourceMetadata
2222

2323

24-
def validate_issuer_url(url: AnyHttpUrl):
25-
"""Validate that the issuer URL meets OAuth 2.0 requirements.
26-
27-
Args:
28-
url: The issuer URL to validate.
29-
30-
Raises:
31-
ValueError: If the issuer URL is invalid.
32-
"""
33-
34-
# RFC 8414 requires HTTPS, but we allow loopback/localhost HTTP for testing
35-
if url.scheme != "https" and url.host not in ("localhost", "127.0.0.1", "[::1]"):
36-
raise ValueError("Issuer URL must be HTTPS")
37-
38-
# No fragments or query parameters allowed
39-
if url.fragment:
40-
raise ValueError("Issuer URL must not have a fragment")
41-
if url.query:
42-
raise ValueError("Issuer URL must not have a query string")
43-
24+
from mcp.server.auth.validation import validate_issuer_url
4425

4526
AUTHORIZATION_PATH = "/authorize"
4627
TOKEN_PATH = "/token"

src/mcp/server/auth/validation.py

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
from pydantic import AnyHttpUrl, AnyUrl
2+
3+
4+
def validate_issuer_url(url: AnyHttpUrl):
5+
"""Validate that the issuer URL meets OAuth 2.0 requirements.
6+
7+
Args:
8+
url: The issuer URL to validate.
9+
10+
Raises:
11+
ValueError: If the issuer URL is invalid.
12+
"""
13+
14+
# RFC 8414 requires HTTPS, but we allow loopback/localhost HTTP for testing
15+
if url.scheme != "https" and url.host not in ("localhost", "127.0.0.1", "[::1]"):
16+
raise ValueError("Issuer URL must be HTTPS")
17+
18+
# No fragments or query parameters allowed
19+
if url.fragment:
20+
raise ValueError("Issuer URL must not have a fragment")
21+
if url.query:
22+
raise ValueError("Issuer URL must not have a query string")
23+
24+
25+
def validate_registered_redirect_uri(url: AnyUrl):
26+
"""Validate that a dynamically registered redirect URI is safe to use.
27+
28+
Mirrors the HTTPS-or-loopback policy used for issuer URLs and rejects
29+
dangerous schemes such as javascript:, data:, and file:.
30+
"""
31+
if url.scheme not in ("https", "http"):
32+
raise ValueError("Redirect URI must use HTTPS or HTTP")
33+
34+
if url.scheme != "https" and url.host not in ("localhost", "127.0.0.1", "[::1]"):
35+
raise ValueError("Redirect URI must be HTTPS unless loopback")
36+
37+
if url.fragment:
38+
raise ValueError("Redirect URI must not have a fragment")

tests/server/auth/test_routes.py

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

4-
from mcp.server.auth.routes import validate_issuer_url
4+
from mcp.server.auth.validation import validate_issuer_url, validate_registered_redirect_uri
55

66

77
def test_validate_issuer_url_https_allowed():
@@ -45,3 +45,33 @@ def test_validate_issuer_url_fragment_rejected():
4545
def test_validate_issuer_url_query_rejected():
4646
with pytest.raises(ValueError, match="query"):
4747
validate_issuer_url(AnyHttpUrl("https://example.com/path?q=1"))
48+
49+
50+
@pytest.mark.parametrize(
51+
"redirect_uri",
52+
[
53+
"https://example.com/callback",
54+
"http://localhost:8080/callback",
55+
"http://127.0.0.1:8080/callback",
56+
"http://[::1]:8080/callback",
57+
],
58+
)
59+
def test_validate_registered_redirect_uri_allowed(redirect_uri: str):
60+
validate_registered_redirect_uri(AnyUrl(redirect_uri))
61+
62+
63+
@pytest.mark.parametrize(
64+
"redirect_uri,message",
65+
[
66+
("javascript:alert(1)", "HTTPS or HTTP"),
67+
("data:text/html,<script>alert(1)</script>", "HTTPS or HTTP"),
68+
("file:///etc/passwd", "HTTPS or HTTP"),
69+
("vbscript:msgbox(1)", "HTTPS or HTTP"),
70+
("ftp://attacker.example/cb", "HTTPS or HTTP"),
71+
("http://attacker.example/cb", "unless loopback"),
72+
("https://example.com/cb#frag", "fragment"),
73+
],
74+
)
75+
def test_validate_registered_redirect_uri_rejected(redirect_uri: str, message: str):
76+
with pytest.raises(ValueError, match=message):
77+
validate_registered_redirect_uri(AnyUrl(redirect_uri))

0 commit comments

Comments
 (0)