Skip to content

Commit 67527c1

Browse files
authored
Block unsafe OAuth redirect schemes (#4419)
1 parent 57a2799 commit 67527c1

14 files changed

Lines changed: 354 additions & 35 deletions

docs/servers/auth/oauth-proxy.mdx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,8 @@ mcp = FastMCP(name="My Server", auth=auth)
194194
List of allowed redirect URI patterns for MCP clients. Patterns support
195195
wildcards (e.g., `"http://localhost:*"`, `"https://*.example.com/*"`).
196196
- `None` (default): DCR clients use registered redirect URIs, with loopback
197-
ports allowed to vary for MCP compatibility
197+
ports allowed to vary for MCP compatibility. Unsafe browser schemes such as
198+
`javascript:`, `data:`, `file:`, and `vbscript:` are rejected.
198199
- Empty list `[]`: No redirect URIs allowed
199200
- Custom list: Only matching patterns allowed
200201

@@ -559,7 +560,7 @@ auth = OAuthProxy(
559560

560561
### Redirect URI Validation
561562

562-
By default, the OAuth proxy validates DCR clients against their registered redirect URIs while allowing loopback ports to vary for MCP compatibility. You can restrict which clients can connect at the server level by specifying allowed patterns:
563+
By default, the OAuth proxy validates DCR clients against their registered redirect URIs while allowing loopback ports to vary for MCP compatibility. Unsafe browser schemes such as `javascript:` are always rejected. You can restrict which clients can connect at the server level by specifying allowed patterns:
563564

564565
```python
565566
# Allow only localhost clients (common for development)

docs/servers/auth/oidc-proxy.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ mcp = FastMCP(name="My Server", auth=auth)
124124

125125
<ParamField body="allowed_client_redirect_uris" type="list[str] | None">
126126
List of allowed redirect URI patterns for MCP clients. Patterns support wildcards (e.g., `"http://localhost:*"`, `"https://*.example.com/*"`).
127-
- `None` (default): DCR clients use registered redirect URIs, with loopback ports allowed to vary for MCP compatibility
127+
- `None` (default): DCR clients use registered redirect URIs, with loopback ports allowed to vary for MCP compatibility. Unsafe browser schemes such as `javascript:`, `data:`, `file:`, and `vbscript:` are rejected.
128128
- Empty list `[]`: No redirect URIs allowed
129129
- Custom list: Only matching patterns allowed
130130

docs/servers/auth/remote-oauth.mdx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ auth = RemoteAuthProvider(
116116
token_verifier=token_verifier,
117117
authorization_servers=[AnyHttpUrl("https://auth.yourcompany.com")],
118118
base_url="https://api.yourcompany.com", # Your server base URL
119-
# Optional: restrict allowed client redirect URIs (defaults to all for DCR compatibility)
119+
# Optional: restrict allowed client redirect URIs
120120
allowed_client_redirect_uris=["http://localhost:*", "http://127.0.0.1:*"]
121121
)
122122

@@ -218,7 +218,7 @@ WorkOS's support for Dynamic Client Registration makes it particularly well-suit
218218
<Note>
219219
`RemoteAuthProvider` also supports the `allowed_client_redirect_uris` parameter for controlling which redirect URIs are accepted from MCP clients during DCR:
220220

221-
- `None` (default): All redirect URIs allowed (for DCR compatibility)
221+
- `None` (default): Broad DCR-compatible redirect support, while rejecting unsafe browser schemes such as `javascript:`, `data:`, `file:`, and `vbscript:`
222222
- Custom list: Specify allowed patterns with wildcard support
223223
- Empty list `[]`: No redirect URIs allowed
224224

@@ -237,4 +237,4 @@ Remote OAuth integration requires careful attention to several technical details
237237

238238
**Scope Management**: Map token scopes to your application's permission model consistently. Consider how scope changes affect existing tokens and plan for smooth permission updates.
239239

240-
The complexity of these considerations reinforces why external identity providers are recommended over custom OAuth implementations. Established providers handle these technical details with extensive testing and operational experience.
240+
The complexity of these considerations reinforces why external identity providers are recommended over custom OAuth implementations. Established providers handle these technical details with extensive testing and operational experience.

fastmcp_slim/fastmcp/server/auth/oauth_proxy/consent.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323

2424
from fastmcp.server.auth.oauth_proxy.models import ProxyDCRClient
2525
from fastmcp.server.auth.oauth_proxy.ui import create_consent_html
26+
from fastmcp.server.auth.redirect_validation import validate_redirect_uri
2627
from fastmcp.utilities.logging import get_logger
2728
from fastmcp.utilities.ui import create_secure_html_response
2829

@@ -60,6 +61,16 @@ def _make_client_key(self, client_id: str, redirect_uri: str | AnyUrl) -> str:
6061
normalized = self._normalize_uri(str(redirect_uri))
6162
return f"{client_id}:{normalized}"
6263

64+
def _validate_client_redirect_uri(
65+
self: OAuthProxy,
66+
redirect_uri: str,
67+
) -> bool:
68+
"""Validate a stored transaction redirect URI before sending a browser to it."""
69+
return validate_redirect_uri(
70+
redirect_uri=redirect_uri,
71+
allowed_patterns=self._allowed_client_redirect_uris,
72+
)
73+
6374
def _cookie_name(self: OAuthProxy, base_name: str) -> str:
6475
"""Return secure cookie name for HTTPS, fallback for HTTP development."""
6576
if self._is_https:
@@ -361,6 +372,18 @@ async def _show_consent_page(
361372
return response
362373

363374
if client_key in denied:
375+
if not self._validate_client_redirect_uri(
376+
txn["client_redirect_uri"]
377+
):
378+
logger.warning(
379+
"Blocked consent denial redirect to disallowed URI for transaction %s",
380+
txn_id,
381+
)
382+
return create_secure_html_response(
383+
"<h1>Error</h1><p>Invalid redirect URI</p>",
384+
status_code=400,
385+
)
386+
364387
callback_params = {
365388
"error": "access_denied",
366389
"state": txn.get("client_state") or "",
@@ -526,6 +549,16 @@ async def _submit_consent(
526549
return response
527550

528551
elif action == "deny":
552+
if not self._validate_client_redirect_uri(txn["client_redirect_uri"]):
553+
logger.warning(
554+
"Blocked consent denial redirect to disallowed URI for transaction %s",
555+
txn_id,
556+
)
557+
return create_secure_html_response(
558+
"<h1>Error</h1><p>Invalid redirect URI</p>",
559+
status_code=400,
560+
)
561+
529562
callback_params = {
530563
"error": "access_denied",
531564
"state": txn.get("client_state") or "",

fastmcp_slim/fastmcp/server/auth/oauth_proxy/models.py

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -243,14 +243,9 @@ def validate_redirect_uri(self, redirect_uri: AnyUrl | None) -> AnyUrl:
243243
f"Invalid CIMD redirect_uri: {e}"
244244
) from e
245245

246-
# Respect proxy-level redirect URI restrictions even when the
247-
# client omits redirect_uri and we fall back to CIMD defaults.
248-
if (
249-
self.allowed_redirect_uri_patterns is not None
250-
and not validate_redirect_uri(
251-
redirect_uri=resolved,
252-
allowed_patterns=self.allowed_redirect_uri_patterns,
253-
)
246+
if not validate_redirect_uri(
247+
redirect_uri=resolved,
248+
allowed_patterns=self.allowed_redirect_uri_patterns,
254249
):
255250
raise InvalidRedirectUriError(
256251
f"Redirect URI '{resolved}' does not match allowed patterns."
@@ -263,6 +258,11 @@ def validate_redirect_uri(self, redirect_uri: AnyUrl | None) -> AnyUrl:
263258
)
264259

265260
if redirect_uri is not None:
261+
if not validate_redirect_uri(redirect_uri, None):
262+
raise InvalidRedirectUriError(
263+
f"Redirect URI '{redirect_uri}' uses an unsafe scheme."
264+
)
265+
266266
cimd_redirect_uris = (
267267
self.cimd_document.redirect_uris if self.cimd_document else None
268268
)
@@ -312,9 +312,8 @@ def validate_redirect_uri(self, redirect_uri: AnyUrl | None) -> AnyUrl:
312312
# (handles the single-registered-URI shortcut for DCR clients), then validate
313313
# the resolved URI against patterns so [] and other restrictions are enforced.
314314
resolved = super().validate_redirect_uri(redirect_uri)
315-
if self.allowed_redirect_uri_patterns is not None:
316-
if not validate_redirect_uri(resolved, self.allowed_redirect_uri_patterns):
317-
raise InvalidRedirectUriError(
318-
f"Redirect URI '{resolved}' does not match allowed patterns."
319-
)
315+
if not validate_redirect_uri(resolved, self.allowed_redirect_uri_patterns):
316+
raise InvalidRedirectUriError(
317+
f"Redirect URI '{resolved}' does not match allowed patterns."
318+
)
320319
return resolved

fastmcp_slim/fastmcp/server/auth/oauth_proxy/proxy.py

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
AuthorizationParams,
4949
AuthorizeError,
5050
RefreshToken,
51+
RegistrationError,
5152
TokenError,
5253
)
5354
from mcp.server.auth.routes import build_metadata, cors_middleware
@@ -91,6 +92,7 @@
9192
_hash_token,
9293
)
9394
from fastmcp.server.auth.oauth_proxy.ui import create_error_html
95+
from fastmcp.server.auth.redirect_validation import validate_redirect_uri
9496
from fastmcp.utilities.auth import parse_scopes
9597
from fastmcp.utilities.logging import get_logger
9698

@@ -301,7 +303,7 @@ def __init__(
301303
allowed_client_redirect_uris: List of allowed redirect URI patterns for MCP clients.
302304
Patterns support wildcards (e.g., "http://localhost:*", "https://*.example.com/*").
303305
If None (default), DCR clients use registered redirect URIs, with loopback
304-
ports allowed to vary for MCP compatibility.
306+
ports allowed to vary for MCP compatibility. Unsafe browser schemes are rejected.
305307
If empty list, no redirect URIs are allowed.
306308
These are for MCP clients performing loopback redirects, NOT for the upstream OAuth app.
307309
valid_scopes: List of all the possible valid scopes for a client.
@@ -839,14 +841,27 @@ async def register_client(self, client_info: OAuthClientInformationFull) -> None
839841
# Create a ProxyDCRClient with configured redirect URI validation
840842
if client_info.client_id is None:
841843
raise ValueError("client_id is required for client registration")
844+
if client_info.redirect_uris:
845+
for redirect_uri in client_info.redirect_uris:
846+
if not validate_redirect_uri(
847+
redirect_uri=redirect_uri,
848+
allowed_patterns=self._allowed_client_redirect_uris,
849+
):
850+
raise RegistrationError(
851+
"invalid_redirect_uri",
852+
f"Redirect URI '{redirect_uri}' is not allowed.",
853+
)
854+
855+
redirect_uris = client_info.redirect_uris or [AnyUrl("http://localhost")]
856+
842857
# We use token_endpoint_auth_method="none" because the proxy handles
843858
# all upstream authentication. The client_secret must also be None
844859
# because the SDK requires secrets to be provided if they're set,
845860
# regardless of auth method.
846861
proxy_client: ProxyDCRClient = ProxyDCRClient(
847862
client_id=client_info.client_id,
848863
client_secret=None,
849-
redirect_uris=client_info.redirect_uris or [AnyUrl("http://localhost")],
864+
redirect_uris=redirect_uris,
850865
grant_types=client_info.grant_types
851866
or ["authorization_code", "refresh_token"],
852867
scope=client_info.scope or self._default_scope_str,
@@ -2160,13 +2175,25 @@ async def _handle_idp_callback(
21602175
)
21612176
if transaction_model:
21622177
# Forward the error to the client's redirect_uri (RFC 6749 §4.1.2.1)
2178+
client_redirect_uri = transaction_model.client_redirect_uri
2179+
if not self._validate_client_redirect_uri(client_redirect_uri):
2180+
logger.warning(
2181+
"Blocked IdP callback error redirect to disallowed URI "
2182+
"for transaction %s",
2183+
txn_id,
2184+
)
2185+
html_content = create_error_html(
2186+
error_title="OAuth Error",
2187+
error_message="Invalid redirect URI",
2188+
)
2189+
return HTMLResponse(content=html_content, status_code=400)
2190+
21632191
error_params: dict[str, str] = {
21642192
"error": error,
21652193
"state": transaction_model.client_state,
21662194
}
21672195
if error_description:
21682196
error_params["error_description"] = error_description
2169-
client_redirect_uri = transaction_model.client_redirect_uri
21702197
separator = "&" if "?" in client_redirect_uri else "?"
21712198
return RedirectResponse(
21722199
url=f"{client_redirect_uri}{separator}{urlencode(error_params)}",
@@ -2186,6 +2213,20 @@ async def _handle_idp_callback(
21862213
error_message="Invalid or expired authorization transaction. Please try authenticating again.",
21872214
)
21882215
return HTMLResponse(content=html_content, status_code=400)
2216+
2217+
if not self._validate_client_redirect_uri(
2218+
transaction_model.client_redirect_uri
2219+
):
2220+
logger.warning(
2221+
"Blocked IdP callback redirect to disallowed URI for transaction %s",
2222+
txn_id,
2223+
)
2224+
html_content = create_error_html(
2225+
error_title="OAuth Error",
2226+
error_message="Invalid redirect URI",
2227+
)
2228+
return HTMLResponse(content=html_content, status_code=400)
2229+
21892230
# Verify consent binding cookie to prevent confused deputy attacks.
21902231
# When consent is enabled, the browser that approved consent receives
21912232
# a signed cookie. A different browser (e.g., a victim lured to the

fastmcp_slim/fastmcp/server/auth/oidc_proxy.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ def __init__(
278278
allowed_client_redirect_uris: List of allowed redirect URI patterns for MCP clients.
279279
Patterns support wildcards (e.g., "http://localhost:*", "https://*.example.com/*").
280280
If None (default), DCR clients use registered redirect URIs, with loopback
281-
ports allowed to vary for MCP compatibility.
281+
ports allowed to vary for MCP compatibility. Unsafe browser schemes are rejected.
282282
If empty list, no redirect URIs are allowed.
283283
These are for MCP clients performing loopback redirects, NOT for the upstream OAuth app.
284284
client_storage: Storage backend for OAuth state (client registrations, encrypted tokens).

fastmcp_slim/fastmcp/server/auth/redirect_validation.py

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,15 @@
99

1010
from pydantic import AnyUrl
1111

12+
UNSAFE_REDIRECT_URI_SCHEMES = frozenset(
13+
{
14+
"javascript",
15+
"data",
16+
"file",
17+
"vbscript",
18+
}
19+
)
20+
1221

1322
def _parse_host_port(netloc: str) -> tuple[str | None, str | None]:
1423
"""Parse host and port from netloc, handling wildcards.
@@ -144,6 +153,15 @@ def _match_path(uri_path: str, pattern_path: str) -> bool:
144153
return fnmatch.fnmatch(uri_path, pattern_path)
145154

146155

156+
def _is_unsafe_redirect_uri(uri: str) -> bool:
157+
try:
158+
parsed = urlparse(uri)
159+
except ValueError:
160+
return True
161+
162+
return parsed.scheme.lower() in UNSAFE_REDIRECT_URI_SCHEMES
163+
164+
147165
def matches_allowed_pattern(uri: str, pattern: str) -> bool:
148166
"""Securely check if a URI matches an allowed pattern with wildcard support.
149167
@@ -172,6 +190,9 @@ def matches_allowed_pattern(uri: str, pattern: str) -> bool:
172190
except ValueError:
173191
return False
174192

193+
if uri_parsed.scheme.lower() in UNSAFE_REDIRECT_URI_SCHEMES:
194+
return False
195+
175196
# SECURITY: Reject URIs with userinfo (user:pass@host)
176197
# This prevents bypass attacks like http://localhost@evil.com/callback
177198
# which would match http://localhost:* with naive fnmatch
@@ -216,7 +237,8 @@ def validate_redirect_uri(
216237
217238
Args:
218239
redirect_uri: The redirect URI to validate
219-
allowed_patterns: List of allowed patterns. If None, all URIs are allowed (for DCR compatibility).
240+
allowed_patterns: List of allowed patterns. If None, ordinary URIs are allowed
241+
for DCR compatibility, while unsafe browser schemes are rejected.
220242
If empty list, no URIs are allowed.
221243
To restrict to localhost only, explicitly pass DEFAULT_LOCALHOST_PATTERNS.
222244
@@ -228,8 +250,11 @@ def validate_redirect_uri(
228250

229251
uri_str = str(redirect_uri)
230252

231-
# If no patterns specified, allow all for DCR compatibility
232-
# (clients need to dynamically register with their own redirect URIs)
253+
if _is_unsafe_redirect_uri(uri_str):
254+
return False
255+
256+
# If no patterns specified, preserve broad DCR compatibility after the
257+
# unsafe browser-scheme check above.
233258
if allowed_patterns is None:
234259
return True
235260

0 commit comments

Comments
 (0)