diff --git a/authentik/providers/oauth2/tests/test_authorize.py b/authentik/providers/oauth2/tests/test_authorize.py index 828581f36..096970cbe 100644 --- a/authentik/providers/oauth2/tests/test_authorize.py +++ b/authentik/providers/oauth2/tests/test_authorize.py @@ -78,6 +78,28 @@ class TestAuthorize(OAuthTestCase): ) OAuthAuthorizationParams.from_request(request) + def test_invalid_redirect_uri_regex(self): + """test missing/invalid redirect URI""" + OAuth2Provider.objects.create( + name="test", + client_id="test", + authorization_flow=create_test_flow(), + redirect_uris="*", + ) + with self.assertRaises(RedirectUriError): + request = self.factory.get("/", data={"response_type": "code", "client_id": "test"}) + OAuthAuthorizationParams.from_request(request) + with self.assertRaises(RedirectUriError): + request = self.factory.get( + "/", + data={ + "response_type": "code", + "client_id": "test", + "redirect_uri": "http://localhost", + }, + ) + OAuthAuthorizationParams.from_request(request) + def test_empty_redirect_uri(self): """test empty redirect URI (configure in provider)""" OAuth2Provider.objects.create( diff --git a/authentik/providers/oauth2/views/authorize.py b/authentik/providers/oauth2/views/authorize.py index e55349663..8116a593f 100644 --- a/authentik/providers/oauth2/views/authorize.py +++ b/authentik/providers/oauth2/views/authorize.py @@ -1,6 +1,7 @@ """authentik OAuth2 Authorization views""" from dataclasses import dataclass, field from datetime import timedelta +from re import error as RegexError from re import fullmatch from typing import Optional from urllib.parse import parse_qs, urlencode, urlparse, urlsplit, urlunsplit @@ -184,12 +185,16 @@ class OAuthAuthorizationParams: self.provider.save() allowed_redirect_urls = self.provider.redirect_uris.split() - if not any(fullmatch(x, self.redirect_uri) for x in allowed_redirect_urls): - LOGGER.warning( - "Invalid redirect uri", - redirect_uri=self.redirect_uri, - excepted=allowed_redirect_urls, - ) + try: + if not any(fullmatch(x, self.redirect_uri) for x in allowed_redirect_urls): + LOGGER.warning( + "Invalid redirect uri", + redirect_uri=self.redirect_uri, + excepted=allowed_redirect_urls, + ) + raise RedirectUriError(self.redirect_uri, allowed_redirect_urls) + except RegexError as exc: + LOGGER.warning("Invalid regular expression configured", exc=exc) raise RedirectUriError(self.redirect_uri, allowed_redirect_urls) if self.request: raise AuthorizeError( diff --git a/authentik/providers/oauth2/views/token.py b/authentik/providers/oauth2/views/token.py index 689f0e32d..baed2ef75 100644 --- a/authentik/providers/oauth2/views/token.py +++ b/authentik/providers/oauth2/views/token.py @@ -2,6 +2,7 @@ from base64 import urlsafe_b64encode from dataclasses import InitVar, dataclass from hashlib import sha256 +from re import error as RegexError from re import fullmatch from typing import Any, Optional @@ -149,12 +150,16 @@ class TokenParams: allowed_redirect_urls = self.provider.redirect_uris.split() # At this point, no provider should have a blank redirect_uri, in case they do # this will check an empty array and raise an error - if not any(fullmatch(x, self.redirect_uri) for x in allowed_redirect_urls): - LOGGER.warning( - "Invalid redirect uri", - redirect_uri=self.redirect_uri, - excepted=allowed_redirect_urls, - ) + try: + if not any(fullmatch(x, self.redirect_uri) for x in allowed_redirect_urls): + LOGGER.warning( + "Invalid redirect uri", + redirect_uri=self.redirect_uri, + excepted=allowed_redirect_urls, + ) + raise TokenError("invalid_client") + except RegexError as exc: + LOGGER.warning("Invalid regular expression configured", exc=exc) raise TokenError("invalid_client") try: