fix: enforce redirect URI validation when allowed_client_redirect_uris is supplied (#3066)

* fix: enforce redirect URI validation when patterns are explicitly configured

Security fix: When allowed_redirect_uri_patterns is explicitly set, reject redirect URIs that don't match the patterns instead of falling back to parent validation. This prevents unauthorized OAuth clients from bypassing the allowlist and accessing protected resources.

* Update models.py

no need to return twice

* fix redirect uri access issue

* update style

* feat: add unit test to enforce fallback not applied when redirect uri's supplied

* fix: improve test case

* apply linter

* refactor: simplify logic and do not exposed allowed redirect patterns

---------

Co-authored-by: Nathan <2381793w@student.gla.ac.uk>
This commit is contained in:
Nathan 2026-02-04 22:33:33 +00:00 committed by GitHub
commit 76f054e957
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 43 additions and 4 deletions

View file

@ -8,7 +8,7 @@ from __future__ import annotations
import hashlib
from typing import Any, Final
from mcp.shared.auth import OAuthClientInformationFull
from mcp.shared.auth import InvalidRedirectUriError, OAuthClientInformationFull
from pydantic import AnyUrl, BaseModel, Field
from fastmcp.server.auth.redirect_validation import validate_redirect_uri
@ -172,7 +172,12 @@ class ProxyDCRClient(OAuthClientInformationFull):
allowed_patterns=self.allowed_redirect_uri_patterns,
):
return redirect_uri
# Fall back to normal validation if not in allowed patterns
return super().validate_redirect_uri(redirect_uri)
# If patterns are explicitly configured then reject non-matching URIs
if self.allowed_redirect_uri_patterns:
raise InvalidRedirectUriError(
f"Redirect URI '{redirect_uri}' does not match allowed patterns."
)
# If no redirect_uri provided, use default behavior
return super().validate_redirect_uri(redirect_uri)

View file

@ -67,6 +67,38 @@ class TestProxyDCRClient:
# Not allowed by patterns - will fallback to base validation
with pytest.raises(InvalidRedirectUriError):
client.validate_redirect_uri(AnyUrl("http://127.0.0.1:3000"))
with pytest.raises(InvalidRedirectUriError):
client.validate_redirect_uri(
AnyUrl("cursor://anysphere.cursor-mcp/oauth/callback")
)
def test_default_not_applied_when_custom_patterns_supplied(self):
"""Test that default validation is not applied when custom patterns are supplied."""
allowed_patterns = [
"cursor://anysphere.cursor-mcp/oauth/callback",
"https://app.example.com/*",
]
client = ProxyDCRClient(
client_id="test",
client_secret="secret",
redirect_uris=[AnyUrl("http://localhost:3000")],
allowed_redirect_uri_patterns=allowed_patterns,
)
assert client.validate_redirect_uri(
AnyUrl("https://app.example.com/oauth/callback")
)
assert client.validate_redirect_uri(
AnyUrl("cursor://anysphere.cursor-mcp/oauth/callback")
)
with pytest.raises(InvalidRedirectUriError):
client.validate_redirect_uri(AnyUrl("http://localhost:3000"))
with pytest.raises(InvalidRedirectUriError):
client.validate_redirect_uri(AnyUrl("http://127.0.0.1:3000"))
with pytest.raises(InvalidRedirectUriError):
client.validate_redirect_uri(AnyUrl("https://example.com"))
def test_empty_list_allows_none(self):
"""Test that empty pattern list allows no URIs."""
@ -77,7 +109,7 @@ class TestProxyDCRClient:
allowed_redirect_uri_patterns=[],
)
# Nothing should be allowed (except the pre-registered one via fallback)
# Nothing should be allowed (except the pre-registered redirect_uris via fallback)
# Pre-registered URI should work via fallback to base validation
assert client.validate_redirect_uri(AnyUrl("http://localhost:3000"))
@ -86,6 +118,8 @@ class TestProxyDCRClient:
client.validate_redirect_uri(AnyUrl("http://example.com"))
with pytest.raises(InvalidRedirectUriError):
client.validate_redirect_uri(AnyUrl("https://anywhere.com:9999/path"))
with pytest.raises(InvalidRedirectUriError):
client.validate_redirect_uri(AnyUrl("http://localhost:5000"))
def test_none_redirect_uri(self):
"""Test that None redirect URI uses default behavior."""