fix: use exact match for loopback hosts in issuer URL validation (#2089)
This commit is contained in:
parent
b9431d483f
commit
0e96aecd1d
2 changed files with 52 additions and 9 deletions
|
|
@ -31,19 +31,15 @@ def validate_issuer_url(url: AnyHttpUrl):
|
||||||
ValueError: If the issuer URL is invalid
|
ValueError: If the issuer URL is invalid
|
||||||
"""
|
"""
|
||||||
|
|
||||||
# RFC 8414 requires HTTPS, but we allow localhost HTTP for testing
|
# RFC 8414 requires HTTPS, but we allow loopback/localhost HTTP for testing
|
||||||
if (
|
if url.scheme != "https" and url.host not in ("localhost", "127.0.0.1", "[::1]"):
|
||||||
url.scheme != "https"
|
raise ValueError("Issuer URL must be HTTPS")
|
||||||
and url.host != "localhost"
|
|
||||||
and (url.host is not None and not url.host.startswith("127.0.0.1"))
|
|
||||||
):
|
|
||||||
raise ValueError("Issuer URL must be HTTPS") # pragma: no cover
|
|
||||||
|
|
||||||
# No fragments or query parameters allowed
|
# No fragments or query parameters allowed
|
||||||
if url.fragment:
|
if url.fragment:
|
||||||
raise ValueError("Issuer URL must not have a fragment") # pragma: no cover
|
raise ValueError("Issuer URL must not have a fragment")
|
||||||
if url.query:
|
if url.query:
|
||||||
raise ValueError("Issuer URL must not have a query string") # pragma: no cover
|
raise ValueError("Issuer URL must not have a query string")
|
||||||
|
|
||||||
|
|
||||||
AUTHORIZATION_PATH = "/authorize"
|
AUTHORIZATION_PATH = "/authorize"
|
||||||
|
|
|
||||||
47
tests/server/auth/test_routes.py
Normal file
47
tests/server/auth/test_routes.py
Normal file
|
|
@ -0,0 +1,47 @@
|
||||||
|
import pytest
|
||||||
|
from pydantic import AnyHttpUrl
|
||||||
|
|
||||||
|
from mcp.server.auth.routes import validate_issuer_url
|
||||||
|
|
||||||
|
|
||||||
|
def test_validate_issuer_url_https_allowed():
|
||||||
|
validate_issuer_url(AnyHttpUrl("https://example.com/path"))
|
||||||
|
|
||||||
|
|
||||||
|
def test_validate_issuer_url_http_localhost_allowed():
|
||||||
|
validate_issuer_url(AnyHttpUrl("http://localhost:8080/path"))
|
||||||
|
|
||||||
|
|
||||||
|
def test_validate_issuer_url_http_127_0_0_1_allowed():
|
||||||
|
validate_issuer_url(AnyHttpUrl("http://127.0.0.1:8080/path"))
|
||||||
|
|
||||||
|
|
||||||
|
def test_validate_issuer_url_http_ipv6_loopback_allowed():
|
||||||
|
validate_issuer_url(AnyHttpUrl("http://[::1]:8080/path"))
|
||||||
|
|
||||||
|
|
||||||
|
def test_validate_issuer_url_http_non_loopback_rejected():
|
||||||
|
with pytest.raises(ValueError, match="Issuer URL must be HTTPS"):
|
||||||
|
validate_issuer_url(AnyHttpUrl("http://evil.com/path"))
|
||||||
|
|
||||||
|
|
||||||
|
def test_validate_issuer_url_http_127_prefix_domain_rejected():
|
||||||
|
"""A domain like 127.0.0.1.evil.com is not loopback."""
|
||||||
|
with pytest.raises(ValueError, match="Issuer URL must be HTTPS"):
|
||||||
|
validate_issuer_url(AnyHttpUrl("http://127.0.0.1.evil.com/path"))
|
||||||
|
|
||||||
|
|
||||||
|
def test_validate_issuer_url_http_127_prefix_subdomain_rejected():
|
||||||
|
"""A domain like 127.0.0.1something.example.com is not loopback."""
|
||||||
|
with pytest.raises(ValueError, match="Issuer URL must be HTTPS"):
|
||||||
|
validate_issuer_url(AnyHttpUrl("http://127.0.0.1something.example.com/path"))
|
||||||
|
|
||||||
|
|
||||||
|
def test_validate_issuer_url_fragment_rejected():
|
||||||
|
with pytest.raises(ValueError, match="fragment"):
|
||||||
|
validate_issuer_url(AnyHttpUrl("https://example.com/path#frag"))
|
||||||
|
|
||||||
|
|
||||||
|
def test_validate_issuer_url_query_rejected():
|
||||||
|
with pytest.raises(ValueError, match="query"):
|
||||||
|
validate_issuer_url(AnyHttpUrl("https://example.com/path?q=1"))
|
||||||
Loading…
Reference in a new issue