From 8c9748e4a04e941cacbe35c29a22e0dab836ecc9 Mon Sep 17 00:00:00 2001
From: Jens Langhammer <jens.langhammer@beryju.org>
Date: Mon, 23 May 2022 20:47:36 +0200
Subject: [PATCH] providers/oauth2: improve error handling for invalid regular
 expressions

Signed-off-by: Jens Langhammer <jens.langhammer@beryju.org>
---
 .../providers/oauth2/tests/test_authorize.py  | 22 +++++++++++++++++++
 authentik/providers/oauth2/views/authorize.py | 17 +++++++++-----
 authentik/providers/oauth2/views/token.py     | 17 +++++++++-----
 3 files changed, 44 insertions(+), 12 deletions(-)

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: