Skip to content

Commit 6d90887

Browse files
authored
Fix OAuth redirect URI validation for DCR compatibility (#1661)
1 parent 5b433f5 commit 6d90887

File tree

5 files changed

+92
-49
lines changed

5 files changed

+92
-49
lines changed

docs/servers/auth/oauth-proxy.mdx

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -164,8 +164,8 @@ The `OAuthProxy` class provides the complete proxy implementation:
164164

165165
<ParamField body="allowed_client_redirect_uris" type="list[str] | None">
166166
List of allowed redirect URI patterns for MCP clients. Patterns support wildcards (e.g., `"http://localhost:*"`, `"https://*.example.com/*"`).
167-
- `None` (default): Only localhost redirect URIs allowed (`http://localhost:*`, `http://127.0.0.1:*`)
168-
- Empty list `[]`: All redirect URIs allowed (not recommended for production)
167+
- `None` (default): All redirect URIs allowed (for MCP/DCR compatibility)
168+
- Empty list `[]`: No redirect URIs allowed
169169
- Custom list: Only matching patterns allowed
170170

171171
These patterns apply to MCP client loopback redirects, NOT the upstream OAuth app redirect URI.
@@ -230,29 +230,45 @@ The proxy automatically:
230230

231231
## Client Redirect URI Security
232232

233-
<Warning>
234-
By default, OAuth Proxy only accepts localhost redirect URIs from MCP clients for security. You can customize this with the `allowed_client_redirect_uris` parameter:
233+
<Note>
234+
OAuth Proxy accepts all redirect URIs by default to maintain compatibility with MCP's Dynamic Client Registration (DCR) pattern, where clients register with unpredictable redirect URIs.
235+
236+
If you know which clients will connect, you can restrict redirect URIs using the `allowed_client_redirect_uris` parameter:
235237

236238
```python
237-
# Default: localhost only (secure)
239+
# Default: allow all (for DCR compatibility)
238240
auth = OAuthProxy(...)
239241

240-
# Custom patterns with wildcards
242+
# Restrict to localhost only
243+
auth = OAuthProxy(
244+
...,
245+
allowed_client_redirect_uris=[
246+
"http://localhost:*",
247+
"http://127.0.0.1:*"
248+
]
249+
)
250+
251+
# Allow specific known clients (e.g., Claude.ai)
241252
auth = OAuthProxy(
242253
...,
243254
allowed_client_redirect_uris=[
244255
"http://localhost:*",
245-
"https://app.example.com/auth/*"
256+
"https://claude.ai/api/mcp/auth_callback"
246257
]
247258
)
248259

249-
# Allow all (NOT recommended for production)
260+
# Custom patterns with wildcards
250261
auth = OAuthProxy(
251262
...,
252-
allowed_client_redirect_uris=[]
263+
allowed_client_redirect_uris=[
264+
"http://localhost:*",
265+
"https://*.example.com/auth/*"
266+
]
253267
)
254268
```
255-
</Warning>
269+
270+
**Tip:** Check your server logs for debug messages that say "Client registered with redirect_uri" messages to see what redirect URIs your clients are using.
271+
</Note>
256272

257273
## Client Compatibility
258274

src/fastmcp/server/auth/oauth_proxy.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,15 @@ async def register_client(self, client_info: OAuthClientInformationFull) -> None
422422
# Store the ProxyDCRClient using the upstream ID
423423
self._clients[upstream_id] = proxy_client
424424

425+
# Log redirect URIs to help users discover what patterns they might need
426+
if client_info.redirect_uris:
427+
for uri in client_info.redirect_uris:
428+
logger.debug(
429+
"Client registered with redirect_uri: %s - if restricting redirect URIs, "
430+
"ensure this pattern is allowed in allowed_client_redirect_uris",
431+
uri,
432+
)
433+
425434
logger.debug(
426435
"Registered client %s with %d redirect URIs",
427436
upstream_id,

src/fastmcp/server/auth/redirect_validation.py

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,9 @@ def validate_redirect_uri(
3333
3434
Args:
3535
redirect_uri: The redirect URI to validate
36-
allowed_patterns: List of allowed patterns. If None, defaults to localhost.
37-
If empty list, all URIs are allowed.
36+
allowed_patterns: List of allowed patterns. If None, all URIs are allowed (for DCR compatibility).
37+
If empty list, no URIs are allowed.
38+
To restrict to localhost only, explicitly pass DEFAULT_LOCALHOST_PATTERNS.
3839
3940
Returns:
4041
True if the redirect URI is allowed
@@ -44,15 +45,9 @@ def validate_redirect_uri(
4445

4546
uri_str = str(redirect_uri)
4647

47-
# If no patterns specified, default to localhost only
48+
# If no patterns specified, allow all for DCR compatibility
49+
# (clients need to dynamically register with their own redirect URIs)
4850
if allowed_patterns is None:
49-
allowed_patterns = [
50-
"http://localhost:*",
51-
"http://127.0.0.1:*",
52-
]
53-
54-
# Empty list means allow all
55-
if len(allowed_patterns) == 0:
5651
return True
5752

5853
# Check if URI matches any allowed pattern

tests/server/auth/test_oauth_proxy_redirect_validation.py

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,15 @@ async def verify_token(self, token: str) -> dict | None:
2121
class TestProxyDCRClient:
2222
"""Test ProxyDCRClient redirect URI validation."""
2323

24-
def test_default_localhost_only(self):
25-
"""Test that default configuration only allows localhost."""
24+
def test_default_allows_all(self):
25+
"""Test that default configuration allows all URIs for DCR compatibility."""
2626
client = ProxyDCRClient(
2727
client_id="test",
2828
client_secret="secret",
2929
redirect_uris=[AnyUrl("http://localhost:3000")],
3030
)
3131

32-
# Localhost should be allowed
32+
# All URIs should be allowed by default for DCR compatibility
3333
assert client.validate_redirect_uri(AnyUrl("http://localhost:3000")) == AnyUrl(
3434
"http://localhost:3000"
3535
)
@@ -39,11 +39,12 @@ def test_default_localhost_only(self):
3939
assert client.validate_redirect_uri(AnyUrl("http://127.0.0.1:3000")) == AnyUrl(
4040
"http://127.0.0.1:3000"
4141
)
42-
43-
# Non-localhost should fallback to base validation
44-
# This will check against registered redirect_uris
45-
with pytest.raises(InvalidRedirectUriError):
46-
client.validate_redirect_uri(AnyUrl("http://example.com"))
42+
assert client.validate_redirect_uri(AnyUrl("http://example.com")) == AnyUrl(
43+
"http://example.com"
44+
)
45+
assert client.validate_redirect_uri(
46+
AnyUrl("https://claude.ai/api/mcp/auth_callback")
47+
) == AnyUrl("https://claude.ai/api/mcp/auth_callback")
4748

4849
def test_custom_patterns(self):
4950
"""Test custom redirect URI patterns."""
@@ -65,19 +66,24 @@ def test_custom_patterns(self):
6566
with pytest.raises(InvalidRedirectUriError):
6667
client.validate_redirect_uri(AnyUrl("http://127.0.0.1:3000"))
6768

68-
def test_empty_list_allows_all(self):
69-
"""Test that empty pattern list allows all URIs."""
69+
def test_empty_list_allows_none(self):
70+
"""Test that empty pattern list allows no URIs."""
7071
client = ProxyDCRClient(
7172
client_id="test",
7273
client_secret="secret",
7374
redirect_uris=[AnyUrl("http://localhost:3000")],
7475
allowed_redirect_uri_patterns=[],
7576
)
7677

77-
# Everything should be allowed
78+
# Nothing should be allowed (except the pre-registered one via fallback)
79+
# Pre-registered URI should work via fallback to base validation
7880
assert client.validate_redirect_uri(AnyUrl("http://localhost:3000"))
79-
assert client.validate_redirect_uri(AnyUrl("http://example.com"))
80-
assert client.validate_redirect_uri(AnyUrl("https://anywhere.com:9999/path"))
81+
82+
# Non-registered URIs should be rejected
83+
with pytest.raises(InvalidRedirectUriError):
84+
client.validate_redirect_uri(AnyUrl("http://example.com"))
85+
with pytest.raises(InvalidRedirectUriError):
86+
client.validate_redirect_uri(AnyUrl("https://anywhere.com:9999/path"))
8187

8288
def test_none_redirect_uri(self):
8389
"""Test that None redirect URI uses default behavior."""
@@ -95,8 +101,8 @@ def test_none_redirect_uri(self):
95101
class TestOAuthProxyRedirectValidation:
96102
"""Test OAuth proxy with redirect URI validation."""
97103

98-
def test_proxy_default_localhost_validation(self):
99-
"""Test that OAuth proxy defaults to localhost-only validation."""
104+
def test_proxy_default_allows_all(self):
105+
"""Test that OAuth proxy defaults to allowing all URIs for DCR compatibility."""
100106
proxy = OAuthProxy(
101107
upstream_authorization_endpoint="https://auth.example.com/authorize",
102108
upstream_token_endpoint="https://auth.example.com/token",
@@ -106,7 +112,7 @@ def test_proxy_default_localhost_validation(self):
106112
base_url="http://localhost:8000",
107113
)
108114

109-
# The proxy should store None for default localhost patterns
115+
# The proxy should store None for default (allow all)
110116
assert proxy._allowed_client_redirect_uris is None
111117

112118
def test_proxy_custom_patterns(self):
@@ -126,7 +132,7 @@ def test_proxy_custom_patterns(self):
126132
assert proxy._allowed_client_redirect_uris == custom_patterns
127133

128134
def test_proxy_empty_list_validation(self):
129-
"""Test OAuth proxy with empty list (allow all)."""
135+
"""Test OAuth proxy with empty list (allow none)."""
130136
proxy = OAuthProxy(
131137
upstream_authorization_endpoint="https://auth.example.com/authorize",
132138
upstream_token_endpoint="https://auth.example.com/token",

tests/server/auth/test_redirect_validation.py

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -66,21 +66,20 @@ def test_none_redirect_uri_allowed(self):
6666
assert validate_redirect_uri(None, [])
6767
assert validate_redirect_uri(None, ["http://localhost:*"])
6868

69-
def test_default_localhost_patterns(self):
70-
"""Test default localhost-only patterns when None is provided."""
71-
# Localhost patterns should be allowed by default
69+
def test_default_allows_all(self):
70+
"""Test that None (default) allows all URIs for DCR compatibility."""
71+
# All URIs should be allowed when None is provided (DCR compatibility)
7272
assert validate_redirect_uri("http://localhost:3000", None)
7373
assert validate_redirect_uri("http://127.0.0.1:8080", None)
74+
assert validate_redirect_uri("http://example.com", None)
75+
assert validate_redirect_uri("https://app.example.com", None)
76+
assert validate_redirect_uri("https://claude.ai/api/mcp/auth_callback", None)
7477

75-
# Non-localhost should be rejected by default
76-
assert not validate_redirect_uri("http://example.com", None)
77-
assert not validate_redirect_uri("https://app.example.com", None)
78-
79-
def test_empty_list_allows_all(self):
80-
"""Test that empty list allows all redirect URIs."""
81-
assert validate_redirect_uri("http://localhost:3000", [])
82-
assert validate_redirect_uri("http://example.com", [])
83-
assert validate_redirect_uri("https://anywhere.com:9999/path", [])
78+
def test_empty_list_allows_none(self):
79+
"""Test that empty list allows no redirect URIs."""
80+
assert not validate_redirect_uri("http://localhost:3000", [])
81+
assert not validate_redirect_uri("http://example.com", [])
82+
assert not validate_redirect_uri("https://anywhere.com:9999/path", [])
8483

8584
def test_custom_patterns(self):
8685
"""Test validation with custom pattern list."""
@@ -122,3 +121,21 @@ def test_default_patterns_include_localhost(self):
122121
"""Test that default patterns include localhost variations."""
123122
assert "http://localhost:*" in DEFAULT_LOCALHOST_PATTERNS
124123
assert "http://127.0.0.1:*" in DEFAULT_LOCALHOST_PATTERNS
124+
125+
def test_explicit_localhost_patterns(self):
126+
"""Test that explicitly passing DEFAULT_LOCALHOST_PATTERNS restricts to localhost."""
127+
# Localhost patterns should be allowed
128+
assert validate_redirect_uri(
129+
"http://localhost:3000", DEFAULT_LOCALHOST_PATTERNS
130+
)
131+
assert validate_redirect_uri(
132+
"http://127.0.0.1:8080", DEFAULT_LOCALHOST_PATTERNS
133+
)
134+
135+
# Non-localhost should be rejected
136+
assert not validate_redirect_uri(
137+
"http://example.com", DEFAULT_LOCALHOST_PATTERNS
138+
)
139+
assert not validate_redirect_uri(
140+
"https://claude.ai/api/mcp/auth_callback", DEFAULT_LOCALHOST_PATTERNS
141+
)

0 commit comments

Comments
 (0)