Skip to content

Commit 1096353

Browse files
author
William O'Meara
committed
Fix RFC 8252 Section 7.3 compliance for loopback redirect URIs
The MCP SDK's OAuthClientMetadata.validate_redirect_uri() was performing exact string matching, which violates RFC 8252 Section 7.3 for loopback addresses. RFC 8252 Section 7.3 states: 'The authorization server MUST allow any port to be specified at the time of the request for loopback IP redirect URIs, to accommodate clients that obtain an available ephemeral port from the operating system at the time of the request.' Changes: - Added RFC 8252-compliant validation for loopback addresses - For localhost/127.0.0.1/::1: port is ignored, scheme/hostname/path must match - For non-loopback URIs: exact matching as before (no behavior change) - Added comprehensive test coverage (13 tests) This fix enables native OAuth clients (like GitHub Copilot CLI) to work properly with MCP servers that use ephemeral loopback redirect ports.
1 parent 5301298 commit 1096353

File tree

2 files changed

+229
-4
lines changed

2 files changed

+229
-4
lines changed

src/mcp/shared/auth.py

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
from typing import Any, Literal
2+
from urllib.parse import urlparse
23

34
from pydantic import AnyHttpUrl, AnyUrl, BaseModel, Field, field_validator
45

@@ -79,11 +80,51 @@ def validate_scope(self, requested_scope: str | None) -> list[str] | None:
7980
return requested_scopes # pragma: no cover
8081

8182
def validate_redirect_uri(self, redirect_uri: AnyUrl | None) -> AnyUrl:
83+
"""Validate redirect_uri against client's registered URIs.
84+
85+
Implements RFC 8252 Section 7.3 for loopback addresses:
86+
"The authorization server MUST allow any port to be specified at the time
87+
of the request for loopback IP redirect URIs, to accommodate clients that
88+
obtain an available ephemeral port from the operating system at the time
89+
of the request."
90+
91+
For loopback addresses (localhost, 127.0.0.1, ::1), the port is ignored
92+
during validation. For all other URIs, exact matching is required.
93+
94+
Args:
95+
redirect_uri: The redirect_uri from the authorization request
96+
97+
Returns:
98+
The validated redirect_uri
99+
100+
Raises:
101+
InvalidRedirectUriError: If redirect_uri is invalid or not registered
102+
"""
82103
if redirect_uri is not None:
83-
# Validate redirect_uri against client's registered redirect URIs
84-
if self.redirect_uris is None or redirect_uri not in self.redirect_uris:
85-
raise InvalidRedirectUriError(f"Redirect URI '{redirect_uri}' not registered for client")
86-
return redirect_uri
104+
if self.redirect_uris is None:
105+
raise InvalidRedirectUriError("No redirect URIs registered for client")
106+
107+
# Try exact match first (fast path)
108+
if redirect_uri in self.redirect_uris:
109+
return redirect_uri
110+
111+
# RFC 8252 loopback matching: ignore port for localhost/127.0.0.1/::1
112+
requested_str = str(redirect_uri)
113+
parsed_requested = urlparse(requested_str)
114+
is_loopback = parsed_requested.hostname in ("localhost", "127.0.0.1", "::1", "[::1]")
115+
116+
if is_loopback:
117+
for registered in self.redirect_uris:
118+
parsed_registered = urlparse(str(registered))
119+
if parsed_registered.hostname not in ("localhost", "127.0.0.1", "::1", "[::1]"):
120+
continue
121+
# Match scheme, hostname, and path - port can differ per RFC 8252
122+
if (parsed_requested.scheme == parsed_registered.scheme and
123+
parsed_requested.hostname == parsed_registered.hostname and
124+
(parsed_requested.path or "/") == (parsed_registered.path or "/")):
125+
return redirect_uri
126+
127+
raise InvalidRedirectUriError(f"Redirect URI '{redirect_uri}' not registered for client")
87128
elif self.redirect_uris is not None and len(self.redirect_uris) == 1:
88129
return self.redirect_uris[0]
89130
else:
Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
"""Tests for RFC 8252 Section 7.3 compliant redirect_uri validation.
2+
3+
RFC 8252 Section 7.3 states:
4+
"The authorization server MUST allow any port to be specified at the time of
5+
the request for loopback IP redirect URIs, to accommodate clients that obtain
6+
an available ephemeral port from the operating system at the time of the request."
7+
"""
8+
9+
import pytest
10+
from pydantic import AnyUrl
11+
12+
from mcp.shared.auth import InvalidRedirectUriError, OAuthClientMetadata
13+
14+
15+
def test_exact_match_non_loopback():
16+
"""Non-loopback URIs must match exactly."""
17+
client = OAuthClientMetadata(
18+
redirect_uris=[AnyUrl("https://example.com:8080/callback")]
19+
)
20+
21+
# Exact match should work
22+
result = client.validate_redirect_uri(AnyUrl("https://example.com:8080/callback"))
23+
assert str(result) == "https://example.com:8080/callback"
24+
25+
# Different port should fail
26+
with pytest.raises(InvalidRedirectUriError):
27+
client.validate_redirect_uri(AnyUrl("https://example.com:9090/callback"))
28+
29+
30+
def test_loopback_localhost_port_ignored():
31+
"""Localhost loopback URIs should ignore port per RFC 8252."""
32+
client = OAuthClientMetadata(
33+
redirect_uris=[AnyUrl("http://localhost:8080/callback")]
34+
)
35+
36+
# Different port should work for loopback
37+
result = client.validate_redirect_uri(AnyUrl("http://localhost:9999/callback"))
38+
assert str(result) == "http://localhost:9999/callback"
39+
40+
# Same port should also work
41+
result = client.validate_redirect_uri(AnyUrl("http://localhost:8080/callback"))
42+
assert str(result) == "http://localhost:8080/callback"
43+
44+
45+
def test_loopback_ipv4_port_ignored():
46+
"""127.0.0.1 loopback URIs should ignore port per RFC 8252."""
47+
client = OAuthClientMetadata(
48+
redirect_uris=[AnyUrl("http://127.0.0.1:5000/")]
49+
)
50+
51+
# Different port should work for loopback
52+
result = client.validate_redirect_uri(AnyUrl("http://127.0.0.1:60847/"))
53+
assert str(result) == "http://127.0.0.1:60847/"
54+
55+
56+
def test_loopback_ipv6_port_ignored():
57+
"""[::1] loopback URIs should ignore port per RFC 8252."""
58+
client = OAuthClientMetadata(
59+
redirect_uris=[AnyUrl("http://[::1]:8080/")]
60+
)
61+
62+
# Different port should work for loopback
63+
result = client.validate_redirect_uri(AnyUrl("http://[::1]:9999/"))
64+
assert str(result) == "http://[::1]:9999/"
65+
66+
67+
def test_loopback_scheme_must_match():
68+
"""Loopback URIs must still match scheme."""
69+
client = OAuthClientMetadata(
70+
redirect_uris=[AnyUrl("http://localhost:8080/callback")]
71+
)
72+
73+
# HTTPS vs HTTP should fail
74+
with pytest.raises(InvalidRedirectUriError):
75+
client.validate_redirect_uri(AnyUrl("https://localhost:9999/callback"))
76+
77+
78+
def test_loopback_path_must_match():
79+
"""Loopback URIs must still match path."""
80+
client = OAuthClientMetadata(
81+
redirect_uris=[AnyUrl("http://localhost:8080/callback")]
82+
)
83+
84+
# Different path should fail
85+
with pytest.raises(InvalidRedirectUriError):
86+
client.validate_redirect_uri(AnyUrl("http://localhost:9999/other"))
87+
88+
89+
def test_loopback_hostname_must_match():
90+
"""Loopback hostname must match (can't mix localhost and 127.0.0.1)."""
91+
client = OAuthClientMetadata(
92+
redirect_uris=[AnyUrl("http://localhost:8080/callback")]
93+
)
94+
95+
# Different loopback hostname should fail
96+
with pytest.raises(InvalidRedirectUriError):
97+
client.validate_redirect_uri(AnyUrl("http://127.0.0.1:9999/callback"))
98+
99+
100+
def test_multiple_redirect_uris_loopback():
101+
"""Should match against any registered loopback URI."""
102+
client = OAuthClientMetadata(
103+
redirect_uris=[
104+
AnyUrl("http://localhost:8080/callback"),
105+
AnyUrl("http://127.0.0.1:5000/auth"),
106+
]
107+
)
108+
109+
# Should match first with different port
110+
result = client.validate_redirect_uri(AnyUrl("http://localhost:9999/callback"))
111+
assert str(result) == "http://localhost:9999/callback"
112+
113+
# Should match second with different port
114+
result = client.validate_redirect_uri(AnyUrl("http://127.0.0.1:6000/auth"))
115+
assert str(result) == "http://127.0.0.1:6000/auth"
116+
117+
118+
def test_mixed_loopback_and_non_loopback():
119+
"""Client can have both loopback and non-loopback URIs."""
120+
client = OAuthClientMetadata(
121+
redirect_uris=[
122+
AnyUrl("http://localhost:8080/callback"),
123+
AnyUrl("https://example.com:8080/callback"),
124+
]
125+
)
126+
127+
# Loopback with different port should work
128+
result = client.validate_redirect_uri(AnyUrl("http://localhost:9999/callback"))
129+
assert str(result) == "http://localhost:9999/callback"
130+
131+
# Non-loopback with different port should fail
132+
with pytest.raises(InvalidRedirectUriError):
133+
client.validate_redirect_uri(AnyUrl("https://example.com:9999/callback"))
134+
135+
# Non-loopback exact match should work
136+
result = client.validate_redirect_uri(AnyUrl("https://example.com:8080/callback"))
137+
assert str(result) == "https://example.com:8080/callback"
138+
139+
140+
def test_no_redirect_uris_registered():
141+
"""Should fail if no redirect URIs are registered."""
142+
client = OAuthClientMetadata(redirect_uris=None)
143+
144+
with pytest.raises(InvalidRedirectUriError, match="No redirect URIs registered"):
145+
client.validate_redirect_uri(AnyUrl("http://localhost:8080/"))
146+
147+
148+
def test_single_registered_uri_omit_request():
149+
"""If only one URI registered and none provided, use the registered one."""
150+
client = OAuthClientMetadata(
151+
redirect_uris=[AnyUrl("http://localhost:8080/callback")]
152+
)
153+
154+
result = client.validate_redirect_uri(None)
155+
assert str(result) == "http://localhost:8080/callback"
156+
157+
158+
def test_multiple_registered_uris_omit_request():
159+
"""Must specify redirect_uri when multiple are registered."""
160+
client = OAuthClientMetadata(
161+
redirect_uris=[
162+
AnyUrl("http://localhost:8080/callback"),
163+
AnyUrl("http://127.0.0.1:5000/auth"),
164+
]
165+
)
166+
167+
with pytest.raises(InvalidRedirectUriError, match="must be specified"):
168+
client.validate_redirect_uri(None)
169+
170+
171+
def test_root_path_normalization():
172+
"""Empty path should be treated as '/'."""
173+
client = OAuthClientMetadata(
174+
redirect_uris=[AnyUrl("http://localhost:8080/")]
175+
)
176+
177+
# Both should match
178+
result = client.validate_redirect_uri(AnyUrl("http://localhost:9999/"))
179+
assert str(result) == "http://localhost:9999/"
180+
181+
# Without trailing slash - Pydantic normalizes to include slash
182+
result = client.validate_redirect_uri(AnyUrl("http://localhost:9999"))
183+
# AnyUrl normalizes URLs, so both forms match
184+
assert "localhost:9999" in str(result)

0 commit comments

Comments
 (0)