diff --git a/authentik/core/tests/test_applications_api.py b/authentik/core/tests/test_applications_api.py index 51adf4b878..ddf1710981 100644 --- a/authentik/core/tests/test_applications_api.py +++ b/authentik/core/tests/test_applications_api.py @@ -12,7 +12,7 @@ from authentik.core.tests.utils import create_test_admin_user, create_test_flow from authentik.lib.generators import generate_id from authentik.policies.dummy.models import DummyPolicy from authentik.policies.models import PolicyBinding -from authentik.providers.oauth2.models import OAuth2Provider +from authentik.providers.oauth2.models import OAuth2Provider, RedirectURI, RedirectURIMatchingMode from authentik.providers.proxy.models import ProxyProvider from authentik.providers.saml.models import SAMLProvider @@ -24,7 +24,7 @@ class TestApplicationsAPI(APITestCase): self.user = create_test_admin_user() self.provider = OAuth2Provider.objects.create( name="test", - redirect_uris="http://some-other-domain", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "http://some-other-domain")], authorization_flow=create_test_flow(), ) self.allowed: Application = Application.objects.create( diff --git a/authentik/core/tests/test_transactional_applications_api.py b/authentik/core/tests/test_transactional_applications_api.py index 3d18e8cd9b..7842b2ec0b 100644 --- a/authentik/core/tests/test_transactional_applications_api.py +++ b/authentik/core/tests/test_transactional_applications_api.py @@ -31,6 +31,7 @@ class TestTransactionalApplicationsAPI(APITestCase): "provider": { "name": uid, "authorization_flow": str(authorization_flow.pk), + "redirect_uris": [], }, }, ) @@ -56,6 +57,7 @@ class TestTransactionalApplicationsAPI(APITestCase): "provider": { "name": uid, "authorization_flow": "", + "redirect_uris": [], }, }, ) diff --git a/authentik/crypto/tests.py b/authentik/crypto/tests.py index e2dc755e7c..c97faf2bd7 100644 --- a/authentik/crypto/tests.py +++ b/authentik/crypto/tests.py @@ -18,7 +18,7 @@ from authentik.crypto.models import CertificateKeyPair from authentik.crypto.tasks import MANAGED_DISCOVERED, certificate_discovery from authentik.lib.config import CONFIG from authentik.lib.generators import generate_id, generate_key -from authentik.providers.oauth2.models import OAuth2Provider +from authentik.providers.oauth2.models import OAuth2Provider, RedirectURI, RedirectURIMatchingMode class TestCrypto(APITestCase): @@ -263,7 +263,7 @@ class TestCrypto(APITestCase): client_id="test", client_secret=generate_key(), authorization_flow=create_test_flow(), - redirect_uris="http://localhost", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "http://localhost")], signing_key=keypair, ) response = self.client.get( @@ -295,7 +295,7 @@ class TestCrypto(APITestCase): client_id="test", client_secret=generate_key(), authorization_flow=create_test_flow(), - redirect_uris="http://localhost", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "http://localhost")], signing_key=keypair, ) response = self.client.get( diff --git a/authentik/providers/oauth2/api/providers.py b/authentik/providers/oauth2/api/providers.py index 632fabca5b..a66b26db80 100644 --- a/authentik/providers/oauth2/api/providers.py +++ b/authentik/providers/oauth2/api/providers.py @@ -1,15 +1,18 @@ """OAuth2Provider API Views""" from copy import copy +from re import compile +from re import error as RegexError from django.urls import reverse from django.utils import timezone +from django.utils.translation import gettext_lazy as _ from drf_spectacular.types import OpenApiTypes from drf_spectacular.utils import OpenApiParameter, OpenApiResponse, extend_schema from guardian.shortcuts import get_objects_for_user from rest_framework.decorators import action from rest_framework.exceptions import ValidationError -from rest_framework.fields import CharField +from rest_framework.fields import CharField, ChoiceField from rest_framework.generics import get_object_or_404 from rest_framework.request import Request from rest_framework.response import Response @@ -20,13 +23,39 @@ from authentik.core.api.used_by import UsedByMixin from authentik.core.api.utils import PassiveSerializer, PropertyMappingPreviewSerializer from authentik.core.models import Provider from authentik.providers.oauth2.id_token import IDToken -from authentik.providers.oauth2.models import AccessToken, OAuth2Provider, ScopeMapping +from authentik.providers.oauth2.models import ( + AccessToken, + OAuth2Provider, + RedirectURIMatchingMode, + ScopeMapping, +) from authentik.rbac.decorators import permission_required +class RedirectURISerializer(PassiveSerializer): + """A single allowed redirect URI entry""" + + matching_mode = ChoiceField(choices=RedirectURIMatchingMode.choices) + url = CharField() + + class OAuth2ProviderSerializer(ProviderSerializer): """OAuth2Provider Serializer""" + redirect_uris = RedirectURISerializer(many=True, source="_redirect_uris") + + def validate_redirect_uris(self, data: list) -> list: + for entry in data: + if entry.get("matching_mode") == RedirectURIMatchingMode.REGEX: + url = entry.get("url") + try: + compile(url) + except RegexError: + raise ValidationError( + _("Invalid Regex Pattern: {url}".format(url=url)) + ) from None + return data + class Meta: model = OAuth2Provider fields = ProviderSerializer.Meta.fields + [ @@ -78,7 +107,6 @@ class OAuth2ProviderViewSet(UsedByMixin, ModelViewSet): "refresh_token_validity", "include_claims_in_id_token", "signing_key", - "redirect_uris", "sub_mode", "property_mappings", "issuer_mode", diff --git a/authentik/providers/oauth2/errors.py b/authentik/providers/oauth2/errors.py index e8c5fd9ed8..479eae16f2 100644 --- a/authentik/providers/oauth2/errors.py +++ b/authentik/providers/oauth2/errors.py @@ -7,7 +7,7 @@ from django.http import HttpRequest, HttpResponse, HttpResponseRedirect from authentik.events.models import Event, EventAction from authentik.lib.sentry import SentryIgnoredException from authentik.lib.views import bad_request_message -from authentik.providers.oauth2.models import GrantTypes +from authentik.providers.oauth2.models import GrantTypes, RedirectURI class OAuth2Error(SentryIgnoredException): @@ -46,9 +46,9 @@ class RedirectUriError(OAuth2Error): ) provided_uri: str - allowed_uris: list[str] + allowed_uris: list[RedirectURI] - def __init__(self, provided_uri: str, allowed_uris: list[str]) -> None: + def __init__(self, provided_uri: str, allowed_uris: list[RedirectURI]) -> None: super().__init__() self.provided_uri = provided_uri self.allowed_uris = allowed_uris diff --git a/authentik/providers/oauth2/migrations/0024_remove_oauth2provider_redirect_uris_and_more.py b/authentik/providers/oauth2/migrations/0024_remove_oauth2provider_redirect_uris_and_more.py new file mode 100644 index 0000000000..0e88ba91c5 --- /dev/null +++ b/authentik/providers/oauth2/migrations/0024_remove_oauth2provider_redirect_uris_and_more.py @@ -0,0 +1,48 @@ +# Generated by Django 5.0.9 on 2024-11-04 12:56 +from django.apps.registry import Apps + +from django.db.backends.base.schema import BaseDatabaseSchemaEditor + +from django.db import migrations, models + + +def migrate_redirect_uris(apps: Apps, schema_editor: BaseDatabaseSchemaEditor): + from authentik.providers.oauth2.models import RedirectURI, RedirectURIMatchingMode + + OAuth2Provider = apps.get_model("authentik_providers_oauth2", "oauth2provider") + + db_alias = schema_editor.connection.alias + for provider in OAuth2Provider.objects.using(db_alias).all(): + uris = [] + for old in provider.old_redirect_uris.split("\n"): + mode = RedirectURIMatchingMode.STRICT + if old == "*" or old == ".*": + mode = RedirectURIMatchingMode.REGEX + uris.append(RedirectURI(mode, url=old)) + provider.redirect_uris = uris + provider.save() + + +class Migration(migrations.Migration): + + dependencies = [ + ("authentik_providers_oauth2", "0023_alter_accesstoken_refreshtoken_use_hash_index"), + ] + + operations = [ + migrations.RenameField( + model_name="oauth2provider", + old_name="redirect_uris", + new_name="old_redirect_uris", + ), + migrations.AddField( + model_name="oauth2provider", + name="_redirect_uris", + field=models.JSONField(default=dict, verbose_name="Redirect URIs"), + ), + migrations.RunPython(migrate_redirect_uris, lambda *args: ...), + migrations.RemoveField( + model_name="oauth2provider", + name="old_redirect_uris", + ), + ] diff --git a/authentik/providers/oauth2/models.py b/authentik/providers/oauth2/models.py index 9484d98514..59ac273701 100644 --- a/authentik/providers/oauth2/models.py +++ b/authentik/providers/oauth2/models.py @@ -3,7 +3,7 @@ import base64 import binascii import json -from dataclasses import asdict +from dataclasses import asdict, dataclass from functools import cached_property from hashlib import sha256 from typing import Any @@ -12,6 +12,7 @@ from urllib.parse import urlparse, urlunparse from cryptography.hazmat.primitives.asymmetric.ec import EllipticCurvePrivateKey from cryptography.hazmat.primitives.asymmetric.rsa import RSAPrivateKey from cryptography.hazmat.primitives.asymmetric.types import PrivateKeyTypes +from dacite import Config from dacite.core import from_dict from django.db import models from django.http import HttpRequest @@ -67,11 +68,25 @@ class IssuerMode(models.TextChoices): """Configure how the `iss` field is created.""" GLOBAL = "global", _("Same identifier is used for all providers") - PER_PROVIDER = "per_provider", _( - "Each provider has a different issuer, based on the application slug." + PER_PROVIDER = ( + "per_provider", + _("Each provider has a different issuer, based on the application slug."), ) +class RedirectURIMatchingMode(models.TextChoices): + STRICT = "strict", _("Strict URL comparison") + REGEX = "regex", _("Regular Expression URL matching") + + +@dataclass +class RedirectURI: + """A single redirect URI entry""" + + matching_mode: RedirectURIMatchingMode + url: str + + class ResponseTypes(models.TextChoices): """Response Type required by the client.""" @@ -146,11 +161,9 @@ class OAuth2Provider(WebfingerProvider, Provider): verbose_name=_("Client Secret"), default=generate_client_secret, ) - redirect_uris = models.TextField( - default="", - blank=True, + _redirect_uris = models.JSONField( + default=dict, verbose_name=_("Redirect URIs"), - help_text=_("Enter each URI on a new line."), ) include_claims_in_id_token = models.BooleanField( @@ -251,12 +264,33 @@ class OAuth2Provider(WebfingerProvider, Provider): except Provider.application.RelatedObjectDoesNotExist: return None + @property + def redirect_uris(self) -> list[RedirectURI]: + uris = [] + for entry in self._redirect_uris: + uris.append( + from_dict( + RedirectURI, + entry, + config=Config(type_hooks={RedirectURIMatchingMode: RedirectURIMatchingMode}), + ) + ) + return uris + + @redirect_uris.setter + def redirect_uris(self, value: list[RedirectURI]): + cleansed = [] + for entry in value: + cleansed.append(asdict(entry)) + self._redirect_uris = cleansed + @property def launch_url(self) -> str | None: """Guess launch_url based on first redirect_uri""" - if self.redirect_uris == "": + redirects = self.redirect_uris + if len(redirects) < 1: return None - main_url = self.redirect_uris.split("\n", maxsplit=1)[0] + main_url = redirects[0].url try: launch_url = urlparse(main_url)._replace(path="") return urlunparse(launch_url) diff --git a/authentik/providers/oauth2/tests/test_api.py b/authentik/providers/oauth2/tests/test_api.py index 827b68b0c4..47ea8ac8df 100644 --- a/authentik/providers/oauth2/tests/test_api.py +++ b/authentik/providers/oauth2/tests/test_api.py @@ -10,7 +10,13 @@ from rest_framework.test import APITestCase from authentik.blueprints.tests import apply_blueprint from authentik.core.models import Application from authentik.core.tests.utils import create_test_admin_user, create_test_flow -from authentik.providers.oauth2.models import OAuth2Provider, ScopeMapping +from authentik.lib.generators import generate_id +from authentik.providers.oauth2.models import ( + OAuth2Provider, + RedirectURI, + RedirectURIMatchingMode, + ScopeMapping, +) class TestAPI(APITestCase): @@ -21,7 +27,7 @@ class TestAPI(APITestCase): self.provider: OAuth2Provider = OAuth2Provider.objects.create( name="test", authorization_flow=create_test_flow(), - redirect_uris="http://testserver", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "http://testserver")], ) self.provider.property_mappings.set(ScopeMapping.objects.all()) self.app = Application.objects.create(name="test", slug="test", provider=self.provider) @@ -50,9 +56,29 @@ class TestAPI(APITestCase): @skipUnless(version_info >= (3, 11, 4), "This behaviour is only Python 3.11.4 and up") def test_launch_url(self): """Test launch_url""" - self.provider.redirect_uris = ( - "https://[\\d\\w]+.pr.test.goauthentik.io/source/oauth/callback/authentik/\n" - ) + self.provider.redirect_uris = [ + RedirectURI( + RedirectURIMatchingMode.REGEX, + "https://[\\d\\w]+.pr.test.goauthentik.io/source/oauth/callback/authentik/", + ), + ] self.provider.save() self.provider.refresh_from_db() self.assertIsNone(self.provider.launch_url) + + def test_validate_redirect_uris(self): + """Test redirect_uris API""" + response = self.client.post( + reverse("authentik_api:oauth2provider-list"), + data={ + "name": generate_id(), + "authorization_flow": create_test_flow().pk, + "invalidation_flow": create_test_flow().pk, + "redirect_uris": [ + {"matching_mode": "strict", "url": "http://goauthentik.io"}, + {"matching_mode": "regex", "url": "**"}, + ], + }, + ) + self.assertJSONEqual(response.content, {"redirect_uris": ["Invalid Regex Pattern: **"]}) + self.assertEqual(response.status_code, 400) diff --git a/authentik/providers/oauth2/tests/test_authorize.py b/authentik/providers/oauth2/tests/test_authorize.py index c3d18d0e31..ecccd424e2 100644 --- a/authentik/providers/oauth2/tests/test_authorize.py +++ b/authentik/providers/oauth2/tests/test_authorize.py @@ -19,6 +19,8 @@ from authentik.providers.oauth2.models import ( AuthorizationCode, GrantTypes, OAuth2Provider, + RedirectURI, + RedirectURIMatchingMode, ScopeMapping, ) from authentik.providers.oauth2.tests.utils import OAuthTestCase @@ -39,7 +41,7 @@ class TestAuthorize(OAuthTestCase): name=generate_id(), client_id="test", authorization_flow=create_test_flow(), - redirect_uris="http://local.invalid/Foo", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "http://local.invalid/Foo")], ) with self.assertRaises(AuthorizeError): request = self.factory.get( @@ -64,7 +66,7 @@ class TestAuthorize(OAuthTestCase): name=generate_id(), client_id="test", authorization_flow=create_test_flow(), - redirect_uris="http://local.invalid/Foo", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "http://local.invalid/Foo")], ) with self.assertRaises(AuthorizeError): request = self.factory.get( @@ -84,7 +86,7 @@ class TestAuthorize(OAuthTestCase): name=generate_id(), client_id="test", authorization_flow=create_test_flow(), - redirect_uris="http://local.invalid", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "http://local.invalid")], ) with self.assertRaises(RedirectUriError): request = self.factory.get("/", data={"response_type": "code", "client_id": "test"}) @@ -106,7 +108,7 @@ class TestAuthorize(OAuthTestCase): name=generate_id(), client_id="test", authorization_flow=create_test_flow(), - redirect_uris="data:local.invalid", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "data:local.invalid")], ) with self.assertRaises(RedirectUriError): request = self.factory.get( @@ -125,7 +127,7 @@ class TestAuthorize(OAuthTestCase): name=generate_id(), client_id="test", authorization_flow=create_test_flow(), - redirect_uris="", + redirect_uris=[], ) with self.assertRaises(RedirectUriError): request = self.factory.get("/", data={"response_type": "code", "client_id": "test"}) @@ -140,7 +142,7 @@ class TestAuthorize(OAuthTestCase): ) OAuthAuthorizationParams.from_request(request) provider.refresh_from_db() - self.assertEqual(provider.redirect_uris, "+") + self.assertEqual(provider.redirect_uris, [RedirectURI(RedirectURIMatchingMode.STRICT, "+")]) def test_invalid_redirect_uri_regex(self): """test missing/invalid redirect URI""" @@ -148,7 +150,7 @@ class TestAuthorize(OAuthTestCase): name=generate_id(), client_id="test", authorization_flow=create_test_flow(), - redirect_uris="http://local.invalid?", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "http://local.invalid?")], ) with self.assertRaises(RedirectUriError): request = self.factory.get("/", data={"response_type": "code", "client_id": "test"}) @@ -170,7 +172,7 @@ class TestAuthorize(OAuthTestCase): name=generate_id(), client_id="test", authorization_flow=create_test_flow(), - redirect_uris="+", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "+")], ) with self.assertRaises(RedirectUriError): request = self.factory.get("/", data={"response_type": "code", "client_id": "test"}) @@ -213,7 +215,7 @@ class TestAuthorize(OAuthTestCase): name=generate_id(), client_id="test", authorization_flow=create_test_flow(), - redirect_uris="http://local.invalid/Foo", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "http://local.invalid/Foo")], ) provider.property_mappings.set( ScopeMapping.objects.filter( @@ -301,7 +303,7 @@ class TestAuthorize(OAuthTestCase): name=generate_id(), client_id="test", authorization_flow=flow, - redirect_uris="foo://localhost", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "foo://localhost")], access_code_validity="seconds=100", ) Application.objects.create(name="app", slug="app", provider=provider) @@ -343,7 +345,7 @@ class TestAuthorize(OAuthTestCase): name=generate_id(), client_id="test", authorization_flow=flow, - redirect_uris="http://localhost", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "http://localhost")], signing_key=self.keypair, ) provider.property_mappings.set( @@ -419,7 +421,7 @@ class TestAuthorize(OAuthTestCase): name=generate_id(), client_id="test", authorization_flow=flow, - redirect_uris="http://localhost", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "http://localhost")], signing_key=self.keypair, ) Application.objects.create(name="app", slug="app", provider=provider) @@ -474,7 +476,7 @@ class TestAuthorize(OAuthTestCase): name=generate_id(), client_id=generate_id(), authorization_flow=flow, - redirect_uris="http://localhost", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "http://localhost")], signing_key=self.keypair, ) provider.property_mappings.set( @@ -532,7 +534,7 @@ class TestAuthorize(OAuthTestCase): name=generate_id(), client_id=generate_id(), authorization_flow=flow, - redirect_uris="http://localhost", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "http://localhost")], signing_key=self.keypair, ) app = Application.objects.create(name=generate_id(), slug=generate_id(), provider=provider) diff --git a/authentik/providers/oauth2/tests/test_introspect.py b/authentik/providers/oauth2/tests/test_introspect.py index 374260a527..f3f2a03243 100644 --- a/authentik/providers/oauth2/tests/test_introspect.py +++ b/authentik/providers/oauth2/tests/test_introspect.py @@ -11,7 +11,14 @@ from authentik.core.models import Application from authentik.core.tests.utils import create_test_admin_user, create_test_cert, create_test_flow from authentik.lib.generators import generate_id from authentik.providers.oauth2.constants import ACR_AUTHENTIK_DEFAULT -from authentik.providers.oauth2.models import AccessToken, IDToken, OAuth2Provider, RefreshToken +from authentik.providers.oauth2.models import ( + AccessToken, + IDToken, + OAuth2Provider, + RedirectURI, + RedirectURIMatchingMode, + RefreshToken, +) from authentik.providers.oauth2.tests.utils import OAuthTestCase @@ -23,7 +30,7 @@ class TesOAuth2Introspection(OAuthTestCase): self.provider: OAuth2Provider = OAuth2Provider.objects.create( name=generate_id(), authorization_flow=create_test_flow(), - redirect_uris="", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "")], signing_key=create_test_cert(), ) self.app = Application.objects.create( @@ -118,7 +125,7 @@ class TesOAuth2Introspection(OAuthTestCase): provider: OAuth2Provider = OAuth2Provider.objects.create( name=generate_id(), authorization_flow=create_test_flow(), - redirect_uris="", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "")], signing_key=create_test_cert(), ) auth = b64encode(f"{provider.client_id}:{provider.client_secret}".encode()).decode() diff --git a/authentik/providers/oauth2/tests/test_jwks.py b/authentik/providers/oauth2/tests/test_jwks.py index 0858df08e4..e8ea0caac0 100644 --- a/authentik/providers/oauth2/tests/test_jwks.py +++ b/authentik/providers/oauth2/tests/test_jwks.py @@ -13,7 +13,7 @@ from authentik.core.tests.utils import create_test_cert, create_test_flow from authentik.crypto.builder import PrivateKeyAlg from authentik.crypto.models import CertificateKeyPair from authentik.lib.generators import generate_id -from authentik.providers.oauth2.models import OAuth2Provider +from authentik.providers.oauth2.models import OAuth2Provider, RedirectURI, RedirectURIMatchingMode from authentik.providers.oauth2.tests.utils import OAuthTestCase TEST_CORDS_CERT = """ @@ -49,7 +49,7 @@ class TestJWKS(OAuthTestCase): name="test", client_id="test", authorization_flow=create_test_flow(), - redirect_uris="http://local.invalid", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "http://local.invalid")], signing_key=create_test_cert(), ) app = Application.objects.create(name="test", slug="test", provider=provider) @@ -68,7 +68,7 @@ class TestJWKS(OAuthTestCase): name="test", client_id="test", authorization_flow=create_test_flow(), - redirect_uris="http://local.invalid", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "http://local.invalid")], ) app = Application.objects.create(name="test", slug="test", provider=provider) response = self.client.get( @@ -82,7 +82,7 @@ class TestJWKS(OAuthTestCase): name="test", client_id="test", authorization_flow=create_test_flow(), - redirect_uris="http://local.invalid", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "http://local.invalid")], signing_key=create_test_cert(PrivateKeyAlg.ECDSA), ) app = Application.objects.create(name="test", slug="test", provider=provider) @@ -104,7 +104,7 @@ class TestJWKS(OAuthTestCase): name="test", client_id="test", authorization_flow=create_test_flow(), - redirect_uris="http://local.invalid", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "http://local.invalid")], signing_key=cert, ) app = Application.objects.create(name="test", slug="test", provider=provider) diff --git a/authentik/providers/oauth2/tests/test_revoke.py b/authentik/providers/oauth2/tests/test_revoke.py index a0312ef71a..3b08688d3e 100644 --- a/authentik/providers/oauth2/tests/test_revoke.py +++ b/authentik/providers/oauth2/tests/test_revoke.py @@ -10,7 +10,14 @@ from django.utils import timezone from authentik.core.models import Application from authentik.core.tests.utils import create_test_admin_user, create_test_cert, create_test_flow from authentik.lib.generators import generate_id -from authentik.providers.oauth2.models import AccessToken, IDToken, OAuth2Provider, RefreshToken +from authentik.providers.oauth2.models import ( + AccessToken, + IDToken, + OAuth2Provider, + RedirectURI, + RedirectURIMatchingMode, + RefreshToken, +) from authentik.providers.oauth2.tests.utils import OAuthTestCase @@ -22,7 +29,7 @@ class TesOAuth2Revoke(OAuthTestCase): self.provider: OAuth2Provider = OAuth2Provider.objects.create( name=generate_id(), authorization_flow=create_test_flow(), - redirect_uris="", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "")], signing_key=create_test_cert(), ) self.app = Application.objects.create( diff --git a/authentik/providers/oauth2/tests/test_token.py b/authentik/providers/oauth2/tests/test_token.py index 0e3f074f6c..6b08ecaa8e 100644 --- a/authentik/providers/oauth2/tests/test_token.py +++ b/authentik/providers/oauth2/tests/test_token.py @@ -22,6 +22,8 @@ from authentik.providers.oauth2.models import ( AccessToken, AuthorizationCode, OAuth2Provider, + RedirectURI, + RedirectURIMatchingMode, RefreshToken, ScopeMapping, ) @@ -42,7 +44,7 @@ class TestToken(OAuthTestCase): provider = OAuth2Provider.objects.create( name=generate_id(), authorization_flow=create_test_flow(), - redirect_uris="http://TestServer", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "http://TestServer")], signing_key=self.keypair, ) header = b64encode(f"{provider.client_id}:{provider.client_secret}".encode()).decode() @@ -69,7 +71,7 @@ class TestToken(OAuthTestCase): provider = OAuth2Provider.objects.create( name=generate_id(), authorization_flow=create_test_flow(), - redirect_uris="http://testserver", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "http://testserver")], signing_key=self.keypair, ) header = b64encode(f"{provider.client_id}:{provider.client_secret}".encode()).decode() @@ -90,7 +92,7 @@ class TestToken(OAuthTestCase): provider = OAuth2Provider.objects.create( name=generate_id(), authorization_flow=create_test_flow(), - redirect_uris="http://local.invalid", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "http://local.invalid")], signing_key=self.keypair, ) header = b64encode(f"{provider.client_id}:{provider.client_secret}".encode()).decode() @@ -118,7 +120,7 @@ class TestToken(OAuthTestCase): provider = OAuth2Provider.objects.create( name=generate_id(), authorization_flow=create_test_flow(), - redirect_uris="http://local.invalid", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "http://local.invalid")], signing_key=self.keypair, ) # Needs to be assigned to an application for iss to be set @@ -158,7 +160,7 @@ class TestToken(OAuthTestCase): provider = OAuth2Provider.objects.create( name=generate_id(), authorization_flow=create_test_flow(), - redirect_uris="http://local.invalid", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "http://local.invalid")], signing_key=self.keypair, ) provider.property_mappings.set( @@ -220,7 +222,7 @@ class TestToken(OAuthTestCase): provider = OAuth2Provider.objects.create( name=generate_id(), authorization_flow=create_test_flow(), - redirect_uris="http://local.invalid", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "http://local.invalid")], signing_key=self.keypair, ) provider.property_mappings.set( @@ -278,7 +280,7 @@ class TestToken(OAuthTestCase): provider = OAuth2Provider.objects.create( name=generate_id(), authorization_flow=create_test_flow(), - redirect_uris="http://testserver", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "http://testserver")], signing_key=self.keypair, ) provider.property_mappings.set( diff --git a/authentik/providers/oauth2/tests/test_token_cc_jwt_source.py b/authentik/providers/oauth2/tests/test_token_cc_jwt_source.py index df508ce311..ff7d51b36d 100644 --- a/authentik/providers/oauth2/tests/test_token_cc_jwt_source.py +++ b/authentik/providers/oauth2/tests/test_token_cc_jwt_source.py @@ -19,7 +19,12 @@ from authentik.providers.oauth2.constants import ( SCOPE_OPENID_PROFILE, TOKEN_TYPE, ) -from authentik.providers.oauth2.models import OAuth2Provider, ScopeMapping +from authentik.providers.oauth2.models import ( + OAuth2Provider, + RedirectURI, + RedirectURIMatchingMode, + ScopeMapping, +) from authentik.providers.oauth2.tests.utils import OAuthTestCase from authentik.providers.oauth2.views.jwks import JWKSView from authentik.sources.oauth.models import OAuthSource @@ -54,7 +59,7 @@ class TestTokenClientCredentialsJWTSource(OAuthTestCase): self.provider: OAuth2Provider = OAuth2Provider.objects.create( name="test", authorization_flow=create_test_flow(), - redirect_uris="http://testserver", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "http://testserver")], signing_key=self.cert, ) self.provider.jwks_sources.add(self.source) diff --git a/authentik/providers/oauth2/tests/test_token_cc_standard.py b/authentik/providers/oauth2/tests/test_token_cc_standard.py index a0abf3b7a6..f917c96617 100644 --- a/authentik/providers/oauth2/tests/test_token_cc_standard.py +++ b/authentik/providers/oauth2/tests/test_token_cc_standard.py @@ -19,7 +19,13 @@ from authentik.providers.oauth2.constants import ( TOKEN_TYPE, ) from authentik.providers.oauth2.errors import TokenError -from authentik.providers.oauth2.models import AccessToken, OAuth2Provider, ScopeMapping +from authentik.providers.oauth2.models import ( + AccessToken, + OAuth2Provider, + RedirectURI, + RedirectURIMatchingMode, + ScopeMapping, +) from authentik.providers.oauth2.tests.utils import OAuthTestCase @@ -33,7 +39,7 @@ class TestTokenClientCredentialsStandard(OAuthTestCase): self.provider = OAuth2Provider.objects.create( name="test", authorization_flow=create_test_flow(), - redirect_uris="http://testserver", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "http://testserver")], signing_key=create_test_cert(), ) self.provider.property_mappings.set(ScopeMapping.objects.all()) diff --git a/authentik/providers/oauth2/tests/test_token_cc_standard_compat.py b/authentik/providers/oauth2/tests/test_token_cc_standard_compat.py index 1c54ad38f1..8e4b1bbfe2 100644 --- a/authentik/providers/oauth2/tests/test_token_cc_standard_compat.py +++ b/authentik/providers/oauth2/tests/test_token_cc_standard_compat.py @@ -20,7 +20,12 @@ from authentik.providers.oauth2.constants import ( TOKEN_TYPE, ) from authentik.providers.oauth2.errors import TokenError -from authentik.providers.oauth2.models import OAuth2Provider, ScopeMapping +from authentik.providers.oauth2.models import ( + OAuth2Provider, + RedirectURI, + RedirectURIMatchingMode, + ScopeMapping, +) from authentik.providers.oauth2.tests.utils import OAuthTestCase @@ -34,7 +39,7 @@ class TestTokenClientCredentialsStandardCompat(OAuthTestCase): self.provider = OAuth2Provider.objects.create( name="test", authorization_flow=create_test_flow(), - redirect_uris="http://testserver", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "http://testserver")], signing_key=create_test_cert(), ) self.provider.property_mappings.set(ScopeMapping.objects.all()) diff --git a/authentik/providers/oauth2/tests/test_token_cc_user_pw.py b/authentik/providers/oauth2/tests/test_token_cc_user_pw.py index 0af554c2b2..bf57eca32b 100644 --- a/authentik/providers/oauth2/tests/test_token_cc_user_pw.py +++ b/authentik/providers/oauth2/tests/test_token_cc_user_pw.py @@ -19,7 +19,12 @@ from authentik.providers.oauth2.constants import ( TOKEN_TYPE, ) from authentik.providers.oauth2.errors import TokenError -from authentik.providers.oauth2.models import OAuth2Provider, ScopeMapping +from authentik.providers.oauth2.models import ( + OAuth2Provider, + RedirectURI, + RedirectURIMatchingMode, + ScopeMapping, +) from authentik.providers.oauth2.tests.utils import OAuthTestCase @@ -33,7 +38,7 @@ class TestTokenClientCredentialsUserNamePassword(OAuthTestCase): self.provider = OAuth2Provider.objects.create( name="test", authorization_flow=create_test_flow(), - redirect_uris="http://testserver", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "http://testserver")], signing_key=create_test_cert(), ) self.provider.property_mappings.set(ScopeMapping.objects.all()) diff --git a/authentik/providers/oauth2/tests/test_token_device.py b/authentik/providers/oauth2/tests/test_token_device.py index b1b7aef5f3..212828897e 100644 --- a/authentik/providers/oauth2/tests/test_token_device.py +++ b/authentik/providers/oauth2/tests/test_token_device.py @@ -14,7 +14,14 @@ from authentik.providers.oauth2.constants import ( SCOPE_OPENID, SCOPE_OPENID_EMAIL, ) -from authentik.providers.oauth2.models import AccessToken, DeviceToken, OAuth2Provider, ScopeMapping +from authentik.providers.oauth2.models import ( + AccessToken, + DeviceToken, + OAuth2Provider, + RedirectURI, + RedirectURIMatchingMode, + ScopeMapping, +) from authentik.providers.oauth2.tests.utils import OAuthTestCase @@ -28,7 +35,7 @@ class TestTokenDeviceCode(OAuthTestCase): self.provider = OAuth2Provider.objects.create( name="test", authorization_flow=create_test_flow(), - redirect_uris="http://testserver", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "http://testserver")], signing_key=create_test_cert(), ) self.provider.property_mappings.set(ScopeMapping.objects.all()) diff --git a/authentik/providers/oauth2/tests/test_token_pkce.py b/authentik/providers/oauth2/tests/test_token_pkce.py index 1f64476a9e..b296eac65e 100644 --- a/authentik/providers/oauth2/tests/test_token_pkce.py +++ b/authentik/providers/oauth2/tests/test_token_pkce.py @@ -10,7 +10,12 @@ from authentik.core.models import Application from authentik.core.tests.utils import create_test_admin_user, create_test_flow from authentik.lib.generators import generate_id from authentik.providers.oauth2.constants import GRANT_TYPE_AUTHORIZATION_CODE -from authentik.providers.oauth2.models import AuthorizationCode, OAuth2Provider +from authentik.providers.oauth2.models import ( + AuthorizationCode, + OAuth2Provider, + RedirectURI, + RedirectURIMatchingMode, +) from authentik.providers.oauth2.tests.utils import OAuthTestCase @@ -30,7 +35,7 @@ class TestTokenPKCE(OAuthTestCase): name=generate_id(), client_id="test", authorization_flow=flow, - redirect_uris="foo://localhost", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "foo://localhost")], access_code_validity="seconds=100", ) Application.objects.create(name="app", slug="app", provider=provider) @@ -93,7 +98,7 @@ class TestTokenPKCE(OAuthTestCase): name=generate_id(), client_id="test", authorization_flow=flow, - redirect_uris="foo://localhost", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "foo://localhost")], access_code_validity="seconds=100", ) Application.objects.create(name="app", slug="app", provider=provider) @@ -154,7 +159,7 @@ class TestTokenPKCE(OAuthTestCase): name=generate_id(), client_id="test", authorization_flow=flow, - redirect_uris="foo://localhost", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "foo://localhost")], access_code_validity="seconds=100", ) Application.objects.create(name="app", slug="app", provider=provider) @@ -210,7 +215,7 @@ class TestTokenPKCE(OAuthTestCase): name=generate_id(), client_id="test", authorization_flow=flow, - redirect_uris="foo://localhost", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "foo://localhost")], access_code_validity="seconds=100", ) Application.objects.create(name="app", slug="app", provider=provider) diff --git a/authentik/providers/oauth2/tests/test_userinfo.py b/authentik/providers/oauth2/tests/test_userinfo.py index 96e48754f7..e7cd423266 100644 --- a/authentik/providers/oauth2/tests/test_userinfo.py +++ b/authentik/providers/oauth2/tests/test_userinfo.py @@ -11,7 +11,14 @@ from authentik.core.models import Application from authentik.core.tests.utils import create_test_admin_user, create_test_cert, create_test_flow from authentik.events.models import Event, EventAction from authentik.lib.generators import generate_id -from authentik.providers.oauth2.models import AccessToken, IDToken, OAuth2Provider, ScopeMapping +from authentik.providers.oauth2.models import ( + AccessToken, + IDToken, + OAuth2Provider, + RedirectURI, + RedirectURIMatchingMode, + ScopeMapping, +) from authentik.providers.oauth2.tests.utils import OAuthTestCase @@ -25,7 +32,7 @@ class TestUserinfo(OAuthTestCase): self.provider: OAuth2Provider = OAuth2Provider.objects.create( name=generate_id(), authorization_flow=create_test_flow(), - redirect_uris="", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "")], signing_key=create_test_cert(), ) self.provider.property_mappings.set(ScopeMapping.objects.all()) diff --git a/authentik/providers/oauth2/views/authorize.py b/authentik/providers/oauth2/views/authorize.py index 5cf5145c03..1f613df45b 100644 --- a/authentik/providers/oauth2/views/authorize.py +++ b/authentik/providers/oauth2/views/authorize.py @@ -57,6 +57,8 @@ from authentik.providers.oauth2.models import ( AuthorizationCode, GrantTypes, OAuth2Provider, + RedirectURI, + RedirectURIMatchingMode, ResponseMode, ResponseTypes, ScopeMapping, @@ -188,40 +190,39 @@ class OAuthAuthorizationParams: def check_redirect_uri(self): """Redirect URI validation.""" - allowed_redirect_urls = self.provider.redirect_uris.split() + allowed_redirect_urls = self.provider.redirect_uris if not self.redirect_uri: LOGGER.warning("Missing redirect uri.") raise RedirectUriError("", allowed_redirect_urls) - if self.provider.redirect_uris == "": + if len(allowed_redirect_urls) < 1: LOGGER.info("Setting redirect for blank redirect_uris", redirect=self.redirect_uri) - self.provider.redirect_uris = self.redirect_uri + self.provider.redirect_uris = [ + RedirectURI(RedirectURIMatchingMode.STRICT, self.redirect_uri) + ] self.provider.save() - allowed_redirect_urls = self.provider.redirect_uris.split() + allowed_redirect_urls = self.provider.redirect_uris - if self.provider.redirect_uris == "*": - LOGGER.info("Converting redirect_uris to regex", redirect=self.redirect_uri) - self.provider.redirect_uris = ".*" - self.provider.save() - allowed_redirect_urls = self.provider.redirect_uris.split() - - try: - if not any(fullmatch(x, self.redirect_uri) for x in allowed_redirect_urls): - LOGGER.warning( - "Invalid redirect uri (regex comparison)", - redirect_uri_given=self.redirect_uri, - redirect_uri_expected=allowed_redirect_urls, - ) - raise RedirectUriError(self.redirect_uri, allowed_redirect_urls) - except RegexError as exc: - LOGGER.info("Failed to parse regular expression, checking directly", exc=exc) - if not any(x == self.redirect_uri for x in allowed_redirect_urls): - LOGGER.warning( - "Invalid redirect uri (strict comparison)", - redirect_uri_given=self.redirect_uri, - redirect_uri_expected=allowed_redirect_urls, - ) - raise RedirectUriError(self.redirect_uri, allowed_redirect_urls) from None + match_found = False + for allowed in allowed_redirect_urls: + if allowed.matching_mode == RedirectURIMatchingMode.STRICT: + if self.redirect_uri == allowed.url: + match_found = True + break + if allowed.matching_mode == RedirectURIMatchingMode.REGEX: + try: + if fullmatch(allowed.url, self.redirect_uri): + match_found = True + break + except RegexError as exc: + LOGGER.warning( + "Failed to parse regular expression", + exc=exc, + url=allowed.url, + provider=self.provider, + ) + if not match_found: + raise RedirectUriError(self.redirect_uri, allowed_redirect_urls) # Check against forbidden schemes if urlparse(self.redirect_uri).scheme in FORBIDDEN_URI_SCHEMES: raise RedirectUriError(self.redirect_uri, allowed_redirect_urls) diff --git a/authentik/providers/oauth2/views/provider.py b/authentik/providers/oauth2/views/provider.py index 06bdceab25..32e9881a54 100644 --- a/authentik/providers/oauth2/views/provider.py +++ b/authentik/providers/oauth2/views/provider.py @@ -158,5 +158,5 @@ class ProviderInfoView(View): OAuth2Provider, pk=application.provider_id ) response = super().dispatch(request, *args, **kwargs) - cors_allow(request, response, *self.provider.redirect_uris.split("\n")) + cors_allow(request, response, *[x.url for x in self.provider.redirect_uris]) return response diff --git a/authentik/providers/oauth2/views/token.py b/authentik/providers/oauth2/views/token.py index d1090402b7..99245ba548 100644 --- a/authentik/providers/oauth2/views/token.py +++ b/authentik/providers/oauth2/views/token.py @@ -58,6 +58,7 @@ from authentik.providers.oauth2.models import ( ClientTypes, DeviceToken, OAuth2Provider, + RedirectURIMatchingMode, RefreshToken, ScopeMapping, ) @@ -195,42 +196,7 @@ class TokenParams: LOGGER.warning("Missing authorization code") raise TokenError("invalid_grant") - 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 - try: - if not any(fullmatch(x, self.redirect_uri) for x in allowed_redirect_urls): - LOGGER.warning( - "Invalid redirect uri (regex comparison)", - redirect_uri=self.redirect_uri, - expected=allowed_redirect_urls, - ) - Event.new( - EventAction.CONFIGURATION_ERROR, - message="Invalid redirect URI used by provider", - provider=self.provider, - redirect_uri=self.redirect_uri, - expected=allowed_redirect_urls, - ).from_http(request) - raise TokenError("invalid_client") - except RegexError as exc: - LOGGER.info("Failed to parse regular expression, checking directly", exc=exc) - if not any(x == self.redirect_uri for x in allowed_redirect_urls): - LOGGER.warning( - "Invalid redirect uri (strict comparison)", - redirect_uri=self.redirect_uri, - expected=allowed_redirect_urls, - ) - Event.new( - EventAction.CONFIGURATION_ERROR, - message="Invalid redirect_uri configured", - provider=self.provider, - ).from_http(request) - raise TokenError("invalid_client") from None - - # Check against forbidden schemes - if urlparse(self.redirect_uri).scheme in FORBIDDEN_URI_SCHEMES: - raise TokenError("invalid_request") + self.__check_redirect_uri(request) self.authorization_code = AuthorizationCode.objects.filter(code=raw_code).first() if not self.authorization_code: @@ -270,6 +236,48 @@ class TokenParams: if not self.authorization_code.code_challenge and self.code_verifier: raise TokenError("invalid_grant") + def __check_redirect_uri(self, request: HttpRequest): + allowed_redirect_urls = self.provider.redirect_uris + # 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 + + match_found = False + for allowed in allowed_redirect_urls: + if allowed.matching_mode == RedirectURIMatchingMode.STRICT: + if self.redirect_uri == allowed.url: + match_found = True + break + if allowed.matching_mode == RedirectURIMatchingMode.REGEX: + try: + if fullmatch(allowed.url, self.redirect_uri): + match_found = True + break + except RegexError as exc: + LOGGER.warning( + "Failed to parse regular expression", + exc=exc, + url=allowed.url, + provider=self.provider, + ) + Event.new( + EventAction.CONFIGURATION_ERROR, + message="Invalid redirect_uri configured", + provider=self.provider, + ).from_http(request) + if not match_found: + Event.new( + EventAction.CONFIGURATION_ERROR, + message="Invalid redirect URI used by provider", + provider=self.provider, + redirect_uri=self.redirect_uri, + expected=allowed_redirect_urls, + ).from_http(request) + raise TokenError("invalid_client") + + # Check against forbidden schemes + if urlparse(self.redirect_uri).scheme in FORBIDDEN_URI_SCHEMES: + raise TokenError("invalid_request") + def __post_init_refresh(self, raw_token: str, request: HttpRequest): if not raw_token: LOGGER.warning("Missing refresh token") @@ -513,7 +521,7 @@ class TokenView(View): response = super().dispatch(request, *args, **kwargs) allowed_origins = [] if self.provider: - allowed_origins = self.provider.redirect_uris.split("\n") + allowed_origins = [x.url for x in self.provider.redirect_uris] cors_allow(self.request, response, *allowed_origins) return response diff --git a/authentik/providers/oauth2/views/userinfo.py b/authentik/providers/oauth2/views/userinfo.py index ad3c263643..cf151cf6d6 100644 --- a/authentik/providers/oauth2/views/userinfo.py +++ b/authentik/providers/oauth2/views/userinfo.py @@ -108,7 +108,7 @@ class UserInfoView(View): response = super().dispatch(request, *args, **kwargs) allowed_origins = [] if self.token: - allowed_origins = self.token.provider.redirect_uris.split("\n") + allowed_origins = [x.url for x in self.token.provider.redirect_uris] cors_allow(self.request, response, *allowed_origins) return response diff --git a/authentik/providers/proxy/api.py b/authentik/providers/proxy/api.py index 88ff9fe01e..bd52be08d5 100644 --- a/authentik/providers/proxy/api.py +++ b/authentik/providers/proxy/api.py @@ -121,7 +121,6 @@ class ProxyProviderViewSet(UsedByMixin, ModelViewSet): "basic_auth_password_attribute": ["iexact"], "basic_auth_user_attribute": ["iexact"], "mode": ["iexact"], - "redirect_uris": ["iexact"], "cookie_domain": ["iexact"], } search_fields = ["name"] diff --git a/authentik/providers/proxy/models.py b/authentik/providers/proxy/models.py index f824495309..55d6091f62 100644 --- a/authentik/providers/proxy/models.py +++ b/authentik/providers/proxy/models.py @@ -13,7 +13,13 @@ from rest_framework.serializers import Serializer from authentik.crypto.models import CertificateKeyPair from authentik.lib.models import DomainlessURLValidator from authentik.outposts.models import OutpostModel -from authentik.providers.oauth2.models import ClientTypes, OAuth2Provider, ScopeMapping +from authentik.providers.oauth2.models import ( + ClientTypes, + OAuth2Provider, + RedirectURI, + RedirectURIMatchingMode, + ScopeMapping, +) SCOPE_AK_PROXY = "ak_proxy" OUTPOST_CALLBACK_SIGNATURE = "X-authentik-auth-callback" @@ -24,14 +30,15 @@ def get_cookie_secret(): return "".join(SystemRandom().choice(string.ascii_uppercase + string.digits) for _ in range(32)) -def _get_callback_url(uri: str) -> str: - return "\n".join( - [ +def _get_callback_url(uri: str) -> list[RedirectURI]: + return [ + RedirectURI( + RedirectURIMatchingMode.STRICT, urljoin(uri, "outpost.goauthentik.io/callback") + f"\\?{OUTPOST_CALLBACK_SIGNATURE}=true", - uri + f"\\?{OUTPOST_CALLBACK_SIGNATURE}=true", - ] - ) + ), + RedirectURI(RedirectURIMatchingMode.STRICT, uri + f"\\?{OUTPOST_CALLBACK_SIGNATURE}=true"), + ] class ProxyMode(models.TextChoices): diff --git a/blueprints/schema.json b/blueprints/schema.json index 0c5480201b..b92295c9e6 100644 --- a/blueprints/schema.json +++ b/blueprints/schema.json @@ -5345,9 +5345,30 @@ "description": "Key used to sign the tokens. Only required when JWT Algorithm is set to RS256." }, "redirect_uris": { - "type": "string", - "title": "Redirect URIs", - "description": "Enter each URI on a new line." + "type": "array", + "items": { + "type": "object", + "properties": { + "matching_mode": { + "type": "string", + "enum": [ + "strict", + "regex" + ], + "title": "Matching mode" + }, + "url": { + "type": "string", + "minLength": 1, + "title": "Url" + } + }, + "required": [ + "matching_mode", + "url" + ] + }, + "title": "Redirect uris" }, "sub_mode": { "type": "string", diff --git a/schema.yml b/schema.yml index 05bcb0f099..2597e6339e 100644 --- a/schema.yml +++ b/schema.yml @@ -19493,10 +19493,6 @@ paths: format: uuid explode: true style: form - - in: query - name: redirect_uris - schema: - type: string - in: query name: refresh_token_validity schema: @@ -19912,10 +19908,6 @@ paths: format: uuid explode: true style: form - - in: query - name: redirect_uris__iexact - schema: - type: string - name: search required: false in: query @@ -41665,6 +41657,11 @@ components: required: - challenge - name + MatchingModeEnum: + enum: + - strict + - regex + type: string Metadata: type: object description: Serializer for blueprint metadata @@ -42353,8 +42350,9 @@ components: description: Key used to sign the tokens. Only required when JWT Algorithm is set to RS256. redirect_uris: - type: string - description: Enter each URI on a new line. + type: array + items: + $ref: '#/components/schemas/RedirectURI' sub_mode: allOf: - $ref: '#/components/schemas/SubModeEnum' @@ -42382,6 +42380,7 @@ components: - meta_model_name - name - pk + - redirect_uris - verbose_name - verbose_name_plural OAuth2ProviderRequest: @@ -42444,8 +42443,9 @@ components: description: Key used to sign the tokens. Only required when JWT Algorithm is set to RS256. redirect_uris: - type: string - description: Enter each URI on a new line. + type: array + items: + $ref: '#/components/schemas/RedirectURIRequest' sub_mode: allOf: - $ref: '#/components/schemas/SubModeEnum' @@ -42466,6 +42466,7 @@ components: required: - authorization_flow - name + - redirect_uris OAuth2ProviderSetupURLs: type: object description: OAuth2 Provider Metadata serializer @@ -46220,8 +46221,9 @@ components: description: Key used to sign the tokens. Only required when JWT Algorithm is set to RS256. redirect_uris: - type: string - description: Enter each URI on a new line. + type: array + items: + $ref: '#/components/schemas/RedirectURIRequest' sub_mode: allOf: - $ref: '#/components/schemas/SubModeEnum' @@ -49357,6 +49359,29 @@ components: type: string required: - to + RedirectURI: + type: object + description: A single allowed redirect URI entry + properties: + matching_mode: + $ref: '#/components/schemas/MatchingModeEnum' + url: + type: string + required: + - matching_mode + - url + RedirectURIRequest: + type: object + description: A single allowed redirect URI entry + properties: + matching_mode: + $ref: '#/components/schemas/MatchingModeEnum' + url: + type: string + minLength: 1 + required: + - matching_mode + - url Reputation: type: object description: Reputation Serializer diff --git a/tests/e2e/test_provider_oauth2_github.py b/tests/e2e/test_provider_oauth2_github.py index 64eecd032d..5389e49c75 100644 --- a/tests/e2e/test_provider_oauth2_github.py +++ b/tests/e2e/test_provider_oauth2_github.py @@ -13,7 +13,12 @@ from authentik.flows.models import Flow from authentik.lib.generators import generate_id, generate_key from authentik.policies.expression.models import ExpressionPolicy from authentik.policies.models import PolicyBinding -from authentik.providers.oauth2.models import ClientTypes, OAuth2Provider +from authentik.providers.oauth2.models import ( + ClientTypes, + OAuth2Provider, + RedirectURI, + RedirectURIMatchingMode, +) from tests.e2e.utils import SeleniumTestCase, retry @@ -79,7 +84,9 @@ class TestProviderOAuth2Github(SeleniumTestCase): client_id=self.client_id, client_secret=self.client_secret, client_type=ClientTypes.CONFIDENTIAL, - redirect_uris="http://localhost:3000/login/github", + redirect_uris=[ + RedirectURI(RedirectURIMatchingMode.STRICT, "http://localhost:3000/login/github") + ], authorization_flow=authorization_flow, ) Application.objects.create( @@ -134,7 +141,9 @@ class TestProviderOAuth2Github(SeleniumTestCase): client_id=self.client_id, client_secret=self.client_secret, client_type=ClientTypes.CONFIDENTIAL, - redirect_uris="http://localhost:3000/login/github", + redirect_uris=[ + RedirectURI(RedirectURIMatchingMode.STRICT, "http://localhost:3000/login/github") + ], authorization_flow=authorization_flow, ) app = Application.objects.create( @@ -205,7 +214,9 @@ class TestProviderOAuth2Github(SeleniumTestCase): client_id=self.client_id, client_secret=self.client_secret, client_type=ClientTypes.CONFIDENTIAL, - redirect_uris="http://localhost:3000/login/github", + redirect_uris=[ + RedirectURI(RedirectURIMatchingMode.STRICT, "http://localhost:3000/login/github") + ], authorization_flow=authorization_flow, ) app = Application.objects.create( diff --git a/tests/e2e/test_provider_oauth2_grafana.py b/tests/e2e/test_provider_oauth2_grafana.py index 0bdd167560..7b540013d2 100644 --- a/tests/e2e/test_provider_oauth2_grafana.py +++ b/tests/e2e/test_provider_oauth2_grafana.py @@ -20,7 +20,13 @@ from authentik.providers.oauth2.constants import ( SCOPE_OPENID_EMAIL, SCOPE_OPENID_PROFILE, ) -from authentik.providers.oauth2.models import ClientTypes, OAuth2Provider, ScopeMapping +from authentik.providers.oauth2.models import ( + ClientTypes, + OAuth2Provider, + RedirectURI, + RedirectURIMatchingMode, + ScopeMapping, +) from tests.e2e.utils import SeleniumTestCase, retry @@ -87,7 +93,7 @@ class TestProviderOAuth2OAuth(SeleniumTestCase): client_id=self.client_id, client_secret=self.client_secret, signing_key=create_test_cert(), - redirect_uris="http://localhost:3000/", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "http://localhost:3000/")], authorization_flow=authorization_flow, ) provider.property_mappings.set( @@ -136,7 +142,11 @@ class TestProviderOAuth2OAuth(SeleniumTestCase): client_id=self.client_id, client_secret=self.client_secret, signing_key=create_test_cert(), - redirect_uris="http://localhost:3000/login/generic_oauth", + redirect_uris=[ + RedirectURI( + RedirectURIMatchingMode.STRICT, "http://localhost:3000/login/generic_oauth" + ) + ], authorization_flow=authorization_flow, ) provider.property_mappings.set( @@ -198,7 +208,11 @@ class TestProviderOAuth2OAuth(SeleniumTestCase): client_id=self.client_id, client_secret=self.client_secret, signing_key=create_test_cert(), - redirect_uris="http://localhost:3000/login/generic_oauth", + redirect_uris=[ + RedirectURI( + RedirectURIMatchingMode.STRICT, "http://localhost:3000/login/generic_oauth" + ) + ], authorization_flow=authorization_flow, ) provider.property_mappings.set( @@ -270,7 +284,11 @@ class TestProviderOAuth2OAuth(SeleniumTestCase): client_id=self.client_id, client_secret=self.client_secret, signing_key=create_test_cert(), - redirect_uris="http://localhost:3000/login/generic_oauth", + redirect_uris=[ + RedirectURI( + RedirectURIMatchingMode.STRICT, "http://localhost:3000/login/generic_oauth" + ) + ], ) provider.property_mappings.set( ScopeMapping.objects.filter( @@ -350,7 +368,11 @@ class TestProviderOAuth2OAuth(SeleniumTestCase): client_id=self.client_id, client_secret=self.client_secret, signing_key=create_test_cert(), - redirect_uris="http://localhost:3000/login/generic_oauth", + redirect_uris=[ + RedirectURI( + RedirectURIMatchingMode.STRICT, "http://localhost:3000/login/generic_oauth" + ) + ], ) provider.property_mappings.set( ScopeMapping.objects.filter( diff --git a/tests/e2e/test_provider_oidc.py b/tests/e2e/test_provider_oidc.py index 5af7e3f2a9..350c4eaa0d 100644 --- a/tests/e2e/test_provider_oidc.py +++ b/tests/e2e/test_provider_oidc.py @@ -21,7 +21,13 @@ from authentik.providers.oauth2.constants import ( SCOPE_OPENID_EMAIL, SCOPE_OPENID_PROFILE, ) -from authentik.providers.oauth2.models import ClientTypes, OAuth2Provider, ScopeMapping +from authentik.providers.oauth2.models import ( + ClientTypes, + OAuth2Provider, + RedirectURI, + RedirectURIMatchingMode, + ScopeMapping, +) from tests.e2e.utils import SeleniumTestCase, retry @@ -73,7 +79,7 @@ class TestProviderOAuth2OIDC(SeleniumTestCase): client_id=self.client_id, client_secret=self.client_secret, signing_key=create_test_cert(), - redirect_uris="http://localhost:9009/", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "http://localhost:9009/")], authorization_flow=authorization_flow, ) provider.property_mappings.set( @@ -122,7 +128,9 @@ class TestProviderOAuth2OIDC(SeleniumTestCase): client_id=self.client_id, client_secret=self.client_secret, signing_key=create_test_cert(), - redirect_uris="http://localhost:9009/auth/callback", + redirect_uris=[ + RedirectURI(RedirectURIMatchingMode.STRICT, "http://localhost:9009/auth/callback") + ], authorization_flow=authorization_flow, ) provider.property_mappings.set( @@ -193,7 +201,9 @@ class TestProviderOAuth2OIDC(SeleniumTestCase): client_id=self.client_id, client_secret=self.client_secret, signing_key=create_test_cert(), - redirect_uris="http://localhost:9009/auth/callback", + redirect_uris=[ + RedirectURI(RedirectURIMatchingMode.STRICT, "http://localhost:9009/auth/callback") + ], ) provider.property_mappings.set( ScopeMapping.objects.filter( @@ -264,7 +274,9 @@ class TestProviderOAuth2OIDC(SeleniumTestCase): client_id=self.client_id, client_secret=self.client_secret, signing_key=create_test_cert(), - redirect_uris="http://localhost:9009/auth/callback", + redirect_uris=[ + RedirectURI(RedirectURIMatchingMode.STRICT, "http://localhost:9009/auth/callback") + ], ) provider.property_mappings.set( ScopeMapping.objects.filter( diff --git a/tests/e2e/test_provider_oidc_implicit.py b/tests/e2e/test_provider_oidc_implicit.py index 9d35c2cc43..5168a27cf7 100644 --- a/tests/e2e/test_provider_oidc_implicit.py +++ b/tests/e2e/test_provider_oidc_implicit.py @@ -21,7 +21,13 @@ from authentik.providers.oauth2.constants import ( SCOPE_OPENID_EMAIL, SCOPE_OPENID_PROFILE, ) -from authentik.providers.oauth2.models import ClientTypes, OAuth2Provider, ScopeMapping +from authentik.providers.oauth2.models import ( + ClientTypes, + OAuth2Provider, + RedirectURI, + RedirectURIMatchingMode, + ScopeMapping, +) from tests.e2e.utils import SeleniumTestCase, retry @@ -74,7 +80,7 @@ class TestProviderOAuth2OIDCImplicit(SeleniumTestCase): client_id=self.client_id, client_secret=self.client_secret, signing_key=create_test_cert(), - redirect_uris="http://localhost:9009/", + redirect_uris=[RedirectURI(RedirectURIMatchingMode.STRICT, "http://localhost:9009/")], authorization_flow=authorization_flow, ) provider.property_mappings.set( @@ -123,7 +129,9 @@ class TestProviderOAuth2OIDCImplicit(SeleniumTestCase): client_id=self.client_id, client_secret=self.client_secret, signing_key=create_test_cert(), - redirect_uris="http://localhost:9009/implicit/", + redirect_uris=[ + RedirectURI(RedirectURIMatchingMode.STRICT, "http://localhost:9009/implicit/") + ], authorization_flow=authorization_flow, ) provider.property_mappings.set( @@ -176,7 +184,9 @@ class TestProviderOAuth2OIDCImplicit(SeleniumTestCase): client_id=self.client_id, client_secret=self.client_secret, signing_key=create_test_cert(), - redirect_uris="http://localhost:9009/implicit/", + redirect_uris=[ + RedirectURI(RedirectURIMatchingMode.STRICT, "http://localhost:9009/implicit/") + ], ) provider.property_mappings.set( ScopeMapping.objects.filter( @@ -244,7 +254,9 @@ class TestProviderOAuth2OIDCImplicit(SeleniumTestCase): client_id=self.client_id, client_secret=self.client_secret, signing_key=create_test_cert(), - redirect_uris="http://localhost:9009/implicit/", + redirect_uris=[ + RedirectURI(RedirectURIMatchingMode.STRICT, "http://localhost:9009/implicit/") + ], ) provider.property_mappings.set( ScopeMapping.objects.filter( diff --git a/web/src/admin/providers/oauth2/OAuth2ProviderForm.ts b/web/src/admin/providers/oauth2/OAuth2ProviderForm.ts index f9ece5a3ad..fbdf094e35 100644 --- a/web/src/admin/providers/oauth2/OAuth2ProviderForm.ts +++ b/web/src/admin/providers/oauth2/OAuth2ProviderForm.ts @@ -1,11 +1,16 @@ import "@goauthentik/admin/common/ak-crypto-certificate-search"; import "@goauthentik/admin/common/ak-flow-search/ak-flow-search"; import { BaseProviderForm } from "@goauthentik/admin/providers/BaseProviderForm"; +import { + IRedirectURIInput, + akOAuthRedirectURIInput, +} from "@goauthentik/admin/providers/oauth2/OAuth2ProviderRedirectURI"; import { DEFAULT_CONFIG } from "@goauthentik/common/api/config"; import { ascii_letters, digits, first, randomString } from "@goauthentik/common/utils"; import "@goauthentik/components/ak-radio-input"; import "@goauthentik/components/ak-text-input"; import "@goauthentik/components/ak-textarea-input"; +import "@goauthentik/elements/ak-array-input.js"; import "@goauthentik/elements/ak-dual-select/ak-dual-select-dynamic-selected-provider.js"; import "@goauthentik/elements/ak-dual-select/ak-dual-select-provider.js"; import "@goauthentik/elements/forms/FormGroup"; @@ -15,7 +20,7 @@ import "@goauthentik/elements/forms/SearchSelect"; import "@goauthentik/elements/utils/TimeDeltaHelp"; import { msg } from "@lit/localize"; -import { TemplateResult, html } from "lit"; +import { TemplateResult, css, html } from "lit"; import { customElement, state } from "lit/decorators.js"; import { ifDefined } from "lit/directives/if-defined.js"; @@ -23,8 +28,10 @@ import { ClientTypeEnum, FlowsInstancesListDesignationEnum, IssuerModeEnum, + MatchingModeEnum, OAuth2Provider, ProvidersApi, + RedirectURI, SubModeEnum, } from "@goauthentik/api"; @@ -98,13 +105,13 @@ export const issuerModeOptions = [ const redirectUriHelpMessages = [ msg( - "Valid redirect URLs after a successful authorization flow. Also specify any origins here for Implicit flows.", + "Valid redirect URIs after a successful authorization flow. Also specify any origins here for Implicit flows.", ), msg( "If no explicit redirect URIs are specified, the first successfully used redirect URI will be saved.", ), msg( - 'To allow any redirect URI, set this value to ".*". Be aware of the possible security implications this can have.', + 'To allow any redirect URI, set the mode to Regex and the value to ".*". Be aware of the possible security implications this can have.', ), ]; @@ -124,11 +131,23 @@ export class OAuth2ProviderFormPage extends BaseProviderForm { @state() showClientSecret = true; + @state() + redirectUris: RedirectURI[] = []; + + static get styles() { + return super.styles.concat(css` + ak-array-input { + width: 100%; + } + `); + } + async loadInstance(pk: number): Promise { const provider = await new ProvidersApi(DEFAULT_CONFIG).providersOauth2Retrieve({ id: pk, }); this.showClientSecret = provider.clientType === ClientTypeEnum.Confidential; + this.redirectUris = provider.redirectUris; return provider; } @@ -216,13 +235,23 @@ export class OAuth2ProviderFormPage extends BaseProviderForm { ?hidden=${!this.showClientSecret} > - - + ({ matchingMode: MatchingModeEnum.Strict, url: "" })} + .row=${(f?: RedirectURI) => + akOAuthRedirectURIInput({ + ".redirectURI": f, + "style": "width: 100%", + } as unknown as IRedirectURIInput)} + > + + ${redirectUriHelp} + diff --git a/web/src/admin/providers/oauth2/OAuth2ProviderRedirectURI.ts b/web/src/admin/providers/oauth2/OAuth2ProviderRedirectURI.ts new file mode 100644 index 0000000000..0420b68b00 --- /dev/null +++ b/web/src/admin/providers/oauth2/OAuth2ProviderRedirectURI.ts @@ -0,0 +1,104 @@ +import "@goauthentik/admin/providers/oauth2/OAuth2ProviderRedirectURI"; +import { AkControlElement } from "@goauthentik/elements/AkControlElement.js"; +import { type Spread } from "@goauthentik/elements/types"; +import { spread } from "@open-wc/lit-helpers"; + +import { msg } from "@lit/localize"; +import { css, html } from "lit"; +import { customElement, property, queryAll } from "lit/decorators.js"; +import { ifDefined } from "lit/directives/if-defined.js"; + +import PFFormControl from "@patternfly/patternfly/components/FormControl/form-control.css"; +import PFInputGroup from "@patternfly/patternfly/components/InputGroup/input-group.css"; +import PFBase from "@patternfly/patternfly/patternfly-base.css"; + +import { MatchingModeEnum, RedirectURI } from "@goauthentik/api"; + +export interface IRedirectURIInput { + redirectURI: RedirectURI; +} + +@customElement("ak-provider-oauth2-redirect-uri") +export class OAuth2ProviderRedirectURI extends AkControlElement { + static get styles() { + return [ + PFBase, + PFInputGroup, + PFFormControl, + css` + .pf-c-input-group select { + width: 10em; + } + `, + ]; + } + + @property({ type: Object, attribute: false }) + redirectURI: RedirectURI = { + matchingMode: MatchingModeEnum.Strict, + url: "", + }; + + @queryAll(".ak-form-control") + controls?: HTMLInputElement[]; + + json() { + return Object.fromEntries( + Array.from(this.controls ?? []).map((control) => [control.name, control.value]), + ) as unknown as RedirectURI; + } + + get isValid() { + return true; + } + + render() { + const onChange = () => { + this.dispatchEvent(new Event("change", { composed: true, bubbles: true })); + }; + + return html`
+ + +
`; + } +} + +export function akOAuthRedirectURIInput(properties: IRedirectURIInput) { + return html``; +} + +declare global { + interface HTMLElementTagNameMap { + "ak-provider-oauth2-redirect-uri": OAuth2ProviderRedirectURI; + } +} diff --git a/web/src/elements/forms/HorizontalFormElement.ts b/web/src/elements/forms/HorizontalFormElement.ts index 5e4b9bcc6e..8d995a48e7 100644 --- a/web/src/elements/forms/HorizontalFormElement.ts +++ b/web/src/elements/forms/HorizontalFormElement.ts @@ -2,7 +2,7 @@ import { convertToSlug } from "@goauthentik/common/utils"; import { AKElement } from "@goauthentik/elements/Base"; import { FormGroup } from "@goauthentik/elements/forms/FormGroup"; -import { msg } from "@lit/localize"; +import { msg, str } from "@lit/localize"; import { CSSResult, css } from "lit"; import { TemplateResult, html } from "lit"; import { customElement, property } from "lit/decorators.js"; @@ -33,7 +33,7 @@ import PFBase from "@patternfly/patternfly/patternfly-base.css"; * where the field isn't available for the user to view unless they explicitly request to be able * to see the content; otherwise, a dead password field is shown. There are 10 uses of this * feature. - * + * */ const isAkControl = (el: unknown): boolean => @@ -86,7 +86,7 @@ export class HorizontalFormElement extends AKElement { writeOnlyActivated = false; @property({ attribute: false }) - errorMessages: string[] = []; + errorMessages: string[] | string[][] = []; @property({ type: Boolean }) slugMode = false; @@ -183,6 +183,16 @@ export class HorizontalFormElement extends AKElement {

` : html``} ${this.errorMessages.map((message) => { + if (message instanceof Object) { + return html`${Object.entries(message).map(([field, errMsg]) => { + return html`

+ ${msg(str`${field}: ${errMsg}`)} +

`; + })}`; + } return html`

${message}

`; diff --git a/website/docs/security/CVE-2024-52289.md b/website/docs/security/CVE-2024-52289.md new file mode 100644 index 0000000000..c9443d64c9 --- /dev/null +++ b/website/docs/security/CVE-2024-52289.md @@ -0,0 +1,30 @@ +# CVE-2024-52289 + +_Reported by [@PontusHanssen](https://github.com/PontusHanssen)_ + +## Insecure default configuration for OAuth2 Redirect URIs + +### Summary + +Redirect URIs in the OAuth2 provider in authentik are checked by RegEx comparison. +When no Redirect URIs are configured in a provider, authentik will automatically use the first `redirect_uri` value received as an allowed redirect URI, without escaping characters that have a special meaning in RegEx. Similarly, the documentation did not take this into consideration either. + +Given a provider with the Redirect URIs set to `https://foo.example.com`, an attacker can register a domain `fooaexample.com`, and it will correctly pass validation. + +### Patches + +authentik 2024.8.5 and 2024.10.3 fix this issue. + +The patched versions remedy this issue by changing the format that the Redirect URIs are saved in, allowing for the explicit configuration if the URL should be checked strictly or as a RegEx. This means that these patches include a backwards-incompatible database change and API change. + +Manual action _is required_ if any provider is intended to use RegEx for Redirect URIs because the migration will set the comparison type to strict for every Redirect URI. + +### Workarounds + +When configuring OAuth2 providers, make sure to escape any wildcard characters that are not intended to function as a wildcard, for example replace `.` with `\.`. + +### For more information + +If you have any questions or comments about this advisory: + +- Email us at [security@goauthentik.io](mailto:security@goauthentik.io) diff --git a/website/sidebars.js b/website/sidebars.js index a1b92bb443..cfc4df6b9b 100644 --- a/website/sidebars.js +++ b/website/sidebars.js @@ -522,6 +522,7 @@ const docsSidebar = { "security/security-hardening", "security/policy", "security/CVE-2024-52307", + "security/CVE-2024-52289", "security/CVE-2024-52287", "security/CVE-2024-47077", "security/CVE-2024-47070",