Skip to content

Parsing redirect uri wildcard #3896

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Viljen789 wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Parsing redirect uri wildcard #3896

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3d9754a
Add fixtures and OAUTH-API for AOC
Viljen789 Jan 5, 2026
be2fd53
Add parsing to Redirect URLs for applications, and wildcard support, …
Viljen789 Feb 7, 2026
b53ee43
Add correct models file, and update redirect URI
Viljen789 Feb 7, 2026
4aa9062
Remove local aoc scripts from PR
Viljen789 Feb 7, 2026
42ded7d
fixme
Viljen789 Feb 7, 2026
1434783
Add universal checks to avoid wildcard abuse
Viljen789 Feb 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 63 additions & 0 deletions lego/apps/oauth/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import fnmatch
from urllib.parse import urlparse

from django.db import models

from oauth2_provider.models import AbstractApplication
Expand All @@ -12,3 +15,63 @@ class APIApplication(AbstractApplication):

class Meta:
permission_handler = APIApplicationPermissionHandler()

def redirect_uri_allowed(self, uri):
"""
Check if a given URI matches one of the allowed redirect URIs.
Supports:
- Space-separated URIs
- Comma-separated URIs
- Wildcards (*) in host and path using fnmatch

Allowed patterns:
- https://example.com/callback
- https://*.example.com/callback
- https://example.com/*
- https://*.example.com/*

Not Allowed:
- *
- https://*
- https://*.com
"""
if not uri:
return False

# Support both space and comma separated URIs
raw_uris = self.redirect_uris.replace(",", " ")
allowed_uris = [u.strip() for u in raw_uris.split() if u.strip()]
Comment on lines +41 to +43
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self.redirect_uris.replace(",", " ") treats any comma as a separator, which breaks valid redirect URIs that legitimately contain commas (e.g., in the query string). Consider splitting on commas only when they are actual delimiters (e.g., CSV-style parsing or splitting on "," with optional surrounding whitespace), or documenting/enforcing that commas are not allowed inside individual URIs (requiring percent-encoding instead).

Copilot uses AI. Check for mistakes.

parsed_uri = urlparse(uri)
if not parsed_uri.scheme or not parsed_uri.netloc:
return False

for allowed_uri in allowed_uris:
parsed_allowed = urlparse(allowed_uri)

# Check to avoid universal links such as https://*, to avoid wildcard abuse
allowed_host = parsed_allowed.hostname or ""
if allowed_host == "*" or (
allowed_host.startswith("*.") and len(allowed_host.split(".")) < 3
):
continue

if parsed_allowed.scheme != parsed_uri.scheme:
continue

if not fnmatch.fnmatch(
parsed_uri.hostname or "", parsed_allowed.hostname or ""
):
continue
Comment on lines +52 to +65
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hostname wildcard checks only block fully-universal patterns (*, *.tld), but patterns like *example.com or ex*ample.com are still accepted and will match unrelated registrable domains (e.g., evil-example.com). If the intent is to support only subdomain wildcards (*.example.com), restrict allowed_host to either an exact hostname or a single leading *. label (and reject any other * usage in the hostname).

Copilot uses AI. Check for mistakes.

allowed_path = parsed_allowed.path or "/"
uri_path = parsed_uri.path or "/"
if not fnmatch.fnmatch(uri_path, allowed_path):
Comment on lines +62 to +69
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fnmatch.fnmatch applies OS-specific normalization (os.path.normcase), which can make URL matching behave differently across platforms (notably Windows). Since these are URLs (not filesystem paths), use fnmatch.fnmatchcase or a regex-based match to ensure consistent behavior across environments.

Suggested change
if not fnmatch.fnmatch(
parsed_uri.hostname or "", parsed_allowed.hostname or ""
):
continue
allowed_path = parsed_allowed.path or "/"
uri_path = parsed_uri.path or "/"
if not fnmatch.fnmatch(uri_path, allowed_path):
if not fnmatch.fnmatchcase(
parsed_uri.hostname or "", parsed_allowed.hostname or ""
):
continue
allowed_path = parsed_allowed.path or "/"
uri_path = parsed_uri.path or "/"
if not fnmatch.fnmatchcase(uri_path, allowed_path):

Copilot uses AI. Check for mistakes.
continue

if parsed_allowed.port and parsed_allowed.port != parsed_uri.port:
continue
Comment on lines +72 to +73
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Port comparison is currently one-directional: if the allowed URI omits a port, any port in the incoming redirect URI will be accepted (e.g., allowed https://example.com/callback would allow https://example.com:444/callback). For OAuth redirect URI verification, the port is part of the authority and should match; consider comparing effective ports (explicit or scheme default) for both URIs and requiring equality.

Copilot uses AI. Check for mistakes.

return True

return False
92 changes: 92 additions & 0 deletions lego/apps/oauth/tests/test_models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,95 @@ def test_initial_application(self):
api_app = APIApplication.objects.get(pk=1)
self.assertTrue(len(api_app.description))
self.assertEqual(api_app.authorization_grant_type, Application.GRANT_PASSWORD)


class RedirectURIAllowedTestCase(BaseTestCase):

def setUp(self):
self.app = APIApplication(
name="Test App",
client_type=APIApplication.CLIENT_PUBLIC,
authorization_grant_type=APIApplication.GRANT_AUTHORIZATION_CODE,
)

def test_exact_match(self):
self.app.redirect_uris = "https://example.com/callback"
self.assertTrue(self.app.redirect_uri_allowed("https://example.com/callback"))
self.assertFalse(self.app.redirect_uri_allowed("https://example.com/other"))

def test_space_separated_uris(self):
self.app.redirect_uris = "https://a.com/cb https://b.com/cb"
self.assertTrue(self.app.redirect_uri_allowed("https://a.com/cb"))
self.assertTrue(self.app.redirect_uri_allowed("https://b.com/cb"))
self.assertFalse(self.app.redirect_uri_allowed("https://c.com/cb"))

def test_comma_separated_uris(self):
self.app.redirect_uris = "https://a.com/cb, https://b.com/cb"
self.assertTrue(self.app.redirect_uri_allowed("https://a.com/cb"))
self.assertTrue(self.app.redirect_uri_allowed("https://b.com/cb"))
self.assertFalse(self.app.redirect_uri_allowed("https://c.com/cb"))

def test_mixed_comma_space_separated(self):
self.app.redirect_uris = "https://a.com/cb, https://b.com/cb https://c.com/cb"
self.assertTrue(self.app.redirect_uri_allowed("https://a.com/cb"))
self.assertTrue(self.app.redirect_uri_allowed("https://b.com/cb"))
self.assertTrue(self.app.redirect_uri_allowed("https://c.com/cb"))

def test_path_wildcard(self):
self.app.redirect_uris = "https://example.com/*"
self.assertTrue(self.app.redirect_uri_allowed("https://example.com/callback"))
self.assertTrue(self.app.redirect_uri_allowed("https://example.com/any/path"))
self.assertFalse(self.app.redirect_uri_allowed("https://other.com/callback"))

def test_subdomain_wildcard(self):
self.app.redirect_uris = "https://*.example.com/callback"
self.assertTrue(
self.app.redirect_uri_allowed("https://app.example.com/callback")
)
self.assertTrue(
self.app.redirect_uri_allowed("https://api.example.com/callback")
)
self.assertFalse(self.app.redirect_uri_allowed("https://example.com/callback"))
self.assertFalse(
self.app.redirect_uri_allowed("https://app.other.com/callback")
)

def test_combined_wildcards(self):
self.app.redirect_uris = "https://*.example.com/*"
self.assertTrue(
self.app.redirect_uri_allowed("https://app.example.com/callback")
)
self.assertTrue(
self.app.redirect_uri_allowed("https://api.example.com/any/path")
)
self.assertFalse(self.app.redirect_uri_allowed("https://example.com/callback"))

def test_scheme_must_match(self):
self.app.redirect_uris = "https://example.com/callback"
self.assertFalse(self.app.redirect_uri_allowed("http://example.com/callback"))

def test_port_matching(self):
self.app.redirect_uris = "https://example.com:8080/callback"
self.assertTrue(
self.app.redirect_uri_allowed("https://example.com:8080/callback")
)
self.assertFalse(
self.app.redirect_uri_allowed("https://example.com:9090/callback")
)
self.assertFalse(self.app.redirect_uri_allowed("https://example.com/callback"))

Comment on lines +81 to +90
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test coverage gap for the port-matching rules: there’s no test asserting that an allowed URI without an explicit port (e.g., https://example.com/callback) does not allow redirects to the same host/path on a different explicit port (e.g., https://example.com:444/callback). Adding this test will lock in the intended security behavior once the implementation is corrected.

Copilot uses AI. Check for mistakes.
def test_global_wildcard(self):
self.app.redirect_uris = "https://*"
self.assertFalse(self.app.redirect_uri_allowed("https://example.com/callback"))
self.assertFalse(self.app.redirect_uri_allowed("/"))
self.assertFalse(self.app.redirect_uri_allowed("https://*.com"))

def test_empty_uri(self):
self.app.redirect_uris = "https://example.com/callback"
self.assertFalse(self.app.redirect_uri_allowed(""))
self.assertFalse(self.app.redirect_uri_allowed(None))

def test_invalid_uri(self):
self.app.redirect_uris = "https://example.com/callback"
self.assertFalse(self.app.redirect_uri_allowed("not-a-valid-uri"))
self.assertFalse(self.app.redirect_uri_allowed("/relative/path"))
Loading