From fe28d216fec269ed52acbd501a9ebd35a2a58433 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Tue, 27 Apr 2021 14:07:04 +0200 Subject: [PATCH 01/13] providers/oauth2: always test JWT keys in tests Signed-off-by: Jens Langhammer --- .../providers/oauth2/tests/test_authorize.py | 27 +++------------- .../providers/oauth2/tests/test_token.py | 18 ++++++++--- authentik/providers/oauth2/tests/utils.py | 31 +++++++++++++++++++ authentik/providers/oauth2/views/token.py | 3 +- 4 files changed, 51 insertions(+), 28 deletions(-) create mode 100644 authentik/providers/oauth2/tests/utils.py diff --git a/authentik/providers/oauth2/tests/test_authorize.py b/authentik/providers/oauth2/tests/test_authorize.py index 9407b13e9..da169ed4e 100644 --- a/authentik/providers/oauth2/tests/test_authorize.py +++ b/authentik/providers/oauth2/tests/test_authorize.py @@ -1,8 +1,7 @@ """Test authorize view""" -from django.test import RequestFactory, TestCase +from django.test import RequestFactory from django.urls import reverse from django.utils.encoding import force_str -from jwt import decode from authentik.core.models import Application, User from authentik.flows.challenge import ChallengeTypes @@ -22,10 +21,11 @@ from authentik.providers.oauth2.models import ( OAuth2Provider, RefreshToken, ) +from authentik.providers.oauth2.tests.utils import OAuthTestCase from authentik.providers.oauth2.views.authorize import OAuthAuthorizationParams -class TestAuthorize(TestCase): +class TestAuthorize(OAuthTestCase): """Test authorize view""" def setUp(self) -> None: @@ -238,23 +238,4 @@ class TestAuthorize(TestCase): ), }, ) - jwt = decode( - token.access_token, - provider.client_secret, - algorithms=[provider.jwt_alg], - audience=provider.client_id, - ) - self.assertIsNotNone(jwt["exp"]) - self.assertIsNotNone(jwt["iat"]) - self.assertIsNotNone(jwt["auth_time"]) - self.assertIsNotNone(jwt["acr"]) - self.assertIsNotNone(jwt["sub"]) - self.assertIsNotNone(jwt["iss"]) - # Check id_token - id_token = token.id_token.to_dict() - self.assertIsNotNone(id_token["exp"]) - self.assertIsNotNone(id_token["iat"]) - self.assertIsNotNone(id_token["auth_time"]) - self.assertIsNotNone(id_token["acr"]) - self.assertIsNotNone(id_token["sub"]) - self.assertIsNotNone(id_token["iss"]) + self.validate_jwt(token, provider) diff --git a/authentik/providers/oauth2/tests/test_token.py b/authentik/providers/oauth2/tests/test_token.py index dccd9c10c..fc9c39c67 100644 --- a/authentik/providers/oauth2/tests/test_token.py +++ b/authentik/providers/oauth2/tests/test_token.py @@ -1,11 +1,11 @@ """Test token view""" from base64 import b64encode -from django.test import RequestFactory, TestCase +from django.test import RequestFactory from django.urls import reverse from django.utils.encoding import force_str -from authentik.core.models import User +from authentik.core.models import Application, User from authentik.flows.models import Flow from authentik.providers.oauth2.constants import ( GRANT_TYPE_AUTHORIZATION_CODE, @@ -20,15 +20,17 @@ from authentik.providers.oauth2.models import ( OAuth2Provider, RefreshToken, ) +from authentik.providers.oauth2.tests.utils import OAuthTestCase from authentik.providers.oauth2.views.token import TokenParams -class TestToken(TestCase): +class TestToken(OAuthTestCase): """Test token view""" def setUp(self) -> None: super().setUp() self.factory = RequestFactory() + self.app = Application.objects.create(name="test", slug="test") def test_request_auth_code(self): """test request param""" @@ -97,12 +99,15 @@ class TestToken(TestCase): authorization_flow=Flow.objects.first(), redirect_uris="http://local.invalid", ) + # Needs to be assigned to an application for iss to be set + self.app.provider = provider + self.app.save() header = b64encode( f"{provider.client_id}:{provider.client_secret}".encode() ).decode() user = User.objects.get(username="akadmin") code = AuthorizationCode.objects.create( - code="foobar", provider=provider, user=user + code="foobar", provider=provider, user=user, is_open_id=True ) response = self.client.post( reverse("authentik_providers_oauth2:token"), @@ -126,6 +131,7 @@ class TestToken(TestCase): ), }, ) + self.validate_jwt(new_token, provider) def test_refresh_token_view(self): """test request param""" @@ -136,6 +142,9 @@ class TestToken(TestCase): authorization_flow=Flow.objects.first(), redirect_uris="http://local.invalid", ) + # Needs to be assigned to an application for iss to be set + self.app.provider = provider + self.app.save() header = b64encode( f"{provider.client_id}:{provider.client_secret}".encode() ).decode() @@ -174,6 +183,7 @@ class TestToken(TestCase): ), }, ) + self.validate_jwt(new_token, provider) def test_refresh_token_view_invalid_origin(self): """test request param""" diff --git a/authentik/providers/oauth2/tests/utils.py b/authentik/providers/oauth2/tests/utils.py new file mode 100644 index 000000000..0f1264ebf --- /dev/null +++ b/authentik/providers/oauth2/tests/utils.py @@ -0,0 +1,31 @@ +"""OAuth test helpers""" +from django.test import TestCase +from jwt import decode + +from authentik.providers.oauth2.models import OAuth2Provider, RefreshToken + + +class OAuthTestCase(TestCase): + """OAuth test helpers""" + + required_jwt_keys = [ + "exp", + "iat", + "auth_time", + "acr", + "sub", + "iss", + ] + + def validate_jwt(self, token: RefreshToken, provider: OAuth2Provider): + """Validate that all required fields are set""" + jwt = decode( + token.access_token, + provider.client_secret, + algorithms=[provider.jwt_alg], + audience=provider.client_id, + ) + id_token = token.id_token.to_dict() + for key in self.required_jwt_keys: + self.assertIsNotNone(jwt[key], f"Key {key} is missing in access_token") + self.assertIsNotNone(id_token[key], f"Key {key} is missing in id_token") diff --git a/authentik/providers/oauth2/views/token.py b/authentik/providers/oauth2/views/token.py index dc71beec9..c0d85a345 100644 --- a/authentik/providers/oauth2/views/token.py +++ b/authentik/providers/oauth2/views/token.py @@ -16,6 +16,7 @@ from authentik.providers.oauth2.constants import ( from authentik.providers.oauth2.errors import TokenError, UserAuthError from authentik.providers.oauth2.models import ( AuthorizationCode, + ClientTypes, OAuth2Provider, RefreshToken, ) @@ -75,7 +76,7 @@ class TokenParams: LOGGER.warning("OAuth2Provider does not exist", client_id=self.client_id) raise TokenError("invalid_client") - if self.provider.client_type == "confidential": + if self.provider.client_type == ClientTypes.CONFIDENTIAL: if self.provider.client_secret != self.client_secret: LOGGER.warning( "Invalid client secret: client does not have secret", From 7b29a1e4859a4f2537ffc47c10c00ec7030cc87f Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Tue, 27 Apr 2021 14:52:42 +0200 Subject: [PATCH 02/13] stages/user_login: add tests for explicit session length Signed-off-by: Jens Langhammer --- authentik/stages/user_login/tests.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/authentik/stages/user_login/tests.py b/authentik/stages/user_login/tests.py index 98da7ab45..33c921e17 100644 --- a/authentik/stages/user_login/tests.py +++ b/authentik/stages/user_login/tests.py @@ -1,4 +1,5 @@ """login tests""" +from time import sleep from unittest.mock import patch from django.test import Client, TestCase @@ -51,6 +52,31 @@ class TestUserLoginStage(TestCase): {"to": reverse("authentik_core:root-redirect"), "type": "redirect"}, ) + def test_expiry(self): + """Test with expiry""" + self.stage.session_duration = "seconds=2" + self.stage.save() + plan = FlowPlan( + flow_pk=self.flow.pk.hex, stages=[self.stage], markers=[StageMarker()] + ) + plan.context[PLAN_CONTEXT_PENDING_USER] = self.user + session = self.client.session + session[SESSION_KEY_PLAN] = plan + session.save() + + response = self.client.get( + reverse("authentik_api:flow-executor", kwargs={"flow_slug": self.flow.slug}) + ) + self.assertEqual(response.status_code, 200) + self.assertJSONEqual( + force_str(response.content), + {"to": reverse("authentik_core:root-redirect"), "type": "redirect"}, + ) + self.assertNotEqual(list(self.client.session.keys()), []) + sleep(3) + self.client.session.clear_expired() + self.assertEqual(list(self.client.session.keys()), []) + @patch( "authentik.flows.views.to_stage_response", TO_STAGE_RESPONSE_MOCK, From 48c0c0bacae6fb26c18ed4433b54c101c694bbd4 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Tue, 27 Apr 2021 14:53:01 +0200 Subject: [PATCH 03/13] */api: simplify lookups for per-user Signed-off-by: Jens Langhammer --- authentik/events/api/notification.py | 6 +++--- authentik/providers/oauth2/api/tokens.py | 15 +++++++-------- authentik/sources/oauth/api/source_connection.py | 8 ++++---- authentik/stages/authenticator_static/api.py | 6 +++--- authentik/stages/authenticator_totp/api.py | 6 +++--- authentik/stages/authenticator_webauthn/api.py | 6 +++--- authentik/stages/consent/api.py | 8 ++++---- 7 files changed, 27 insertions(+), 28 deletions(-) diff --git a/authentik/events/api/notification.py b/authentik/events/api/notification.py index 9908533ca..70066ae0e 100644 --- a/authentik/events/api/notification.py +++ b/authentik/events/api/notification.py @@ -1,4 +1,5 @@ """Notification API Views""" +from guardian.utils import get_anonymous_user from rest_framework import mixins from rest_framework.fields import ReadOnlyField from rest_framework.serializers import ModelSerializer @@ -48,6 +49,5 @@ class NotificationViewSet( ] def get_queryset(self): - if not self.request: - return super().get_queryset() - return Notification.objects.filter(user=self.request.user) + user = self.request.user if self.request else get_anonymous_user() + return Notification.objects.filter(user=user) diff --git a/authentik/providers/oauth2/api/tokens.py b/authentik/providers/oauth2/api/tokens.py index f8ec0d79a..f7e2ea3f2 100644 --- a/authentik/providers/oauth2/api/tokens.py +++ b/authentik/providers/oauth2/api/tokens.py @@ -1,4 +1,5 @@ """OAuth2Provider API Views""" +from guardian.utils import get_anonymous_user from rest_framework import mixins from rest_framework.fields import CharField, ListField from rest_framework.serializers import ModelSerializer @@ -38,11 +39,10 @@ class AuthorizationCodeViewSet( ordering = ["provider", "expires"] def get_queryset(self): - if not self.request: + user = self.request.user if self.request else get_anonymous_user() + if user.is_superuser: return super().get_queryset() - if self.request.user.is_superuser: - return super().get_queryset() - return super().get_queryset().filter(user=self.request.user) + return super().get_queryset().filter(user=user) class RefreshTokenViewSet( @@ -59,8 +59,7 @@ class RefreshTokenViewSet( ordering = ["provider", "expires"] def get_queryset(self): - if not self.request: + user = self.request.user if self.request else get_anonymous_user() + if user.is_superuser: return super().get_queryset() - if self.request.user.is_superuser: - return super().get_queryset() - return super().get_queryset().filter(user=self.request.user) + return super().get_queryset().filter(user=user) diff --git a/authentik/sources/oauth/api/source_connection.py b/authentik/sources/oauth/api/source_connection.py index 67d34db3b..763608150 100644 --- a/authentik/sources/oauth/api/source_connection.py +++ b/authentik/sources/oauth/api/source_connection.py @@ -1,4 +1,5 @@ """OAuth Source Serializer""" +from guardian.utils import get_anonymous_user from rest_framework.viewsets import ModelViewSet from authentik.core.api.sources import SourceSerializer @@ -26,8 +27,7 @@ class UserOAuthSourceConnectionViewSet(ModelViewSet): filterset_fields = ["source__slug"] def get_queryset(self): - if not self.request: + user = self.request.user if self.request else get_anonymous_user() + if user.is_superuser: return super().get_queryset() - if self.request.user.is_superuser: - return super().get_queryset() - return super().get_queryset().filter(user=self.request.user) + return super().get_queryset().filter(user=user) diff --git a/authentik/stages/authenticator_static/api.py b/authentik/stages/authenticator_static/api.py index 9a27d31e5..a7eeaee88 100644 --- a/authentik/stages/authenticator_static/api.py +++ b/authentik/stages/authenticator_static/api.py @@ -1,5 +1,6 @@ """AuthenticatorStaticStage API Views""" from django_otp.plugins.otp_static.models import StaticDevice +from guardian.utils import get_anonymous_user from rest_framework.permissions import IsAdminUser from rest_framework.serializers import ModelSerializer from rest_framework.viewsets import ModelViewSet, ReadOnlyModelViewSet @@ -44,9 +45,8 @@ class StaticDeviceViewSet(ModelViewSet): ordering = ["name"] def get_queryset(self): - if not self.request: - return super().get_queryset() - return StaticDevice.objects.filter(user=self.request.user) + user = self.request.user if self.request else get_anonymous_user() + return StaticDevice.objects.filter(user=user) class StaticAdminDeviceViewSet(ReadOnlyModelViewSet): diff --git a/authentik/stages/authenticator_totp/api.py b/authentik/stages/authenticator_totp/api.py index 12e748e90..f6aa67e57 100644 --- a/authentik/stages/authenticator_totp/api.py +++ b/authentik/stages/authenticator_totp/api.py @@ -1,5 +1,6 @@ """AuthenticatorTOTPStage API Views""" from django_otp.plugins.otp_totp.models import TOTPDevice +from guardian.utils import get_anonymous_user from rest_framework.permissions import IsAdminUser from rest_framework.serializers import ModelSerializer from rest_framework.viewsets import ModelViewSet, ReadOnlyModelViewSet @@ -47,9 +48,8 @@ class TOTPDeviceViewSet(ModelViewSet): ordering = ["name"] def get_queryset(self): - if not self.request: - return super().get_queryset() - return TOTPDevice.objects.filter(user=self.request.user) + user = self.request.user if self.request else get_anonymous_user() + return TOTPDevice.objects.filter(user=user) class TOTPAdminDeviceViewSet(ReadOnlyModelViewSet): diff --git a/authentik/stages/authenticator_webauthn/api.py b/authentik/stages/authenticator_webauthn/api.py index 3830fed6c..2ea373efe 100644 --- a/authentik/stages/authenticator_webauthn/api.py +++ b/authentik/stages/authenticator_webauthn/api.py @@ -1,4 +1,5 @@ """AuthenticateWebAuthnStage API Views""" +from guardian.utils import get_anonymous_user from rest_framework.permissions import IsAdminUser from rest_framework.serializers import ModelSerializer from rest_framework.viewsets import ModelViewSet, ReadOnlyModelViewSet @@ -46,9 +47,8 @@ class WebAuthnDeviceViewSet(ModelViewSet): ordering = ["name"] def get_queryset(self): - if not self.request: - return super().get_queryset() - return WebAuthnDevice.objects.filter(user=self.request.user) + user = self.request.user if self.request else get_anonymous_user() + return WebAuthnDevice.objects.filter(user=user) class WebAuthnAdminDeviceViewSet(ReadOnlyModelViewSet): diff --git a/authentik/stages/consent/api.py b/authentik/stages/consent/api.py index 7a66efa3e..ffe600f3b 100644 --- a/authentik/stages/consent/api.py +++ b/authentik/stages/consent/api.py @@ -1,4 +1,5 @@ """ConsentStage API Views""" +from guardian.utils import get_anonymous_user from rest_framework import mixins from rest_framework.viewsets import GenericViewSet, ModelViewSet @@ -50,8 +51,7 @@ class UserConsentViewSet( ordering = ["application", "expires"] def get_queryset(self): - if not self.request: + user = self.request.user if self.request else get_anonymous_user() + if user.is_superuser: return super().get_queryset() - if self.request.user.is_superuser: - return super().get_queryset() - return super().get_queryset().filter(user=self.request.user) + return super().get_queryset().filter(user=user) From 68d120b3b4c015f72c37df396e5480b0f08f1ab9 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Tue, 27 Apr 2021 15:19:54 +0200 Subject: [PATCH 04/13] sources/oauth: add tests for google type Signed-off-by: Jens Langhammer --- .../sources/oauth/tests/test_type_discord.py | 4 +- .../sources/oauth/tests/test_type_google.py | 40 +++++++++++++++++++ authentik/sources/oauth/views/dispatcher.py | 10 ++--- 3 files changed, 45 insertions(+), 9 deletions(-) create mode 100644 authentik/sources/oauth/tests/test_type_google.py diff --git a/authentik/sources/oauth/tests/test_type_discord.py b/authentik/sources/oauth/tests/test_type_discord.py index 0e01056db..6b337d1bd 100644 --- a/authentik/sources/oauth/tests/test_type_discord.py +++ b/authentik/sources/oauth/tests/test_type_discord.py @@ -18,7 +18,7 @@ DISCORD_USER = { } -class TestTypeGitHub(TestCase): +class TestTypeDiscord(TestCase): """OAuth Source tests""" def setUp(self): @@ -32,7 +32,7 @@ class TestTypeGitHub(TestCase): ) def test_enroll_context(self): - """Test GitHub Enrollment context""" + """Test discord Enrollment context""" ak_context = DiscordOAuth2Callback().get_user_enroll_context( self.source, UserOAuthSourceConnection(), DISCORD_USER ) diff --git a/authentik/sources/oauth/tests/test_type_google.py b/authentik/sources/oauth/tests/test_type_google.py new file mode 100644 index 000000000..6f43812ad --- /dev/null +++ b/authentik/sources/oauth/tests/test_type_google.py @@ -0,0 +1,40 @@ +"""google Type tests""" +from django.test import TestCase + +from authentik.sources.oauth.models import OAuthSource, UserOAuthSourceConnection +from authentik.sources.oauth.types.google import GoogleOAuth2Callback + +# https://developers.google.com/identity/protocols/oauth2/openid-connect?hl=en +GOOGLE_USER = { + "id": "1324813249123401234", + "email": "foo@bar.baz", + "verified_email": True, + "name": "foo bar", + "given_name": "foo", + "family_name": "bar", + "picture": "", + "locale": "en", +} + + +class TestTypeGoogle(TestCase): + """OAuth Source tests""" + + def setUp(self): + self.source = OAuthSource.objects.create( + name="test", + slug="test", + provider_type="google", + authorization_url="", + profile_url="", + consumer_key="", + ) + + def test_enroll_context(self): + """Test Google Enrollment context""" + ak_context = GoogleOAuth2Callback().get_user_enroll_context( + self.source, UserOAuthSourceConnection(), GOOGLE_USER + ) + self.assertEqual(ak_context["username"], GOOGLE_USER["email"]) + self.assertEqual(ak_context["email"], GOOGLE_USER["email"]) + self.assertEqual(ak_context["name"], GOOGLE_USER["name"]) diff --git a/authentik/sources/oauth/views/dispatcher.py b/authentik/sources/oauth/views/dispatcher.py index 63a0769b5..4ee0cf3b2 100644 --- a/authentik/sources/oauth/views/dispatcher.py +++ b/authentik/sources/oauth/views/dispatcher.py @@ -1,5 +1,4 @@ """Dispatch OAuth views to respective views""" -from django.http import Http404 from django.shortcuts import get_object_or_404 from django.views import View from structlog.stdlib import get_logger @@ -15,12 +14,9 @@ class DispatcherView(View): kind = "" - def dispatch(self, *args, **kwargs): + def dispatch(self, *args, source_slug: str, **kwargs): """Find Source by slug and forward request""" - slug = kwargs.get("source_slug", None) - if not slug: - raise Http404 - source = get_object_or_404(OAuthSource, slug=slug) + source = get_object_or_404(OAuthSource, slug=source_slug) view = MANAGER.find(source.provider_type, kind=RequestKind(self.kind)) LOGGER.debug("dispatching OAuth2 request to", view=view, kind=self.kind) - return view.as_view()(*args, **kwargs) + return view.as_view()(*args, source_slug=source_slug, **kwargs) From d2abe6d455a116e0b9cbc0e95a158a7aad9cf8c1 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Tue, 27 Apr 2021 15:20:09 +0200 Subject: [PATCH 05/13] stages/email: catch ValueError when global email settings are invalid Signed-off-by: Jens Langhammer --- authentik/stages/email/tasks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/authentik/stages/email/tasks.py b/authentik/stages/email/tasks.py index 38fafdeeb..462f3dae6 100644 --- a/authentik/stages/email/tasks.py +++ b/authentik/stages/email/tasks.py @@ -68,7 +68,7 @@ def send_mail( messages=["Successfully sent Mail."], ) ) - except (SMTPException, ConnectionError) as exc: + except (SMTPException, ConnectionError, ValueError) as exc: LOGGER.debug("Error sending email, retrying...", exc=exc) self.set_status(TaskResult(TaskResultStatus.ERROR).with_error(exc)) raise exc From cad6c42fdd3693dfffe1e5b28511dc69ac9c40b7 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Tue, 27 Apr 2021 15:20:23 +0200 Subject: [PATCH 06/13] lib: add more tests Signed-off-by: Jens Langhammer --- authentik/crypto/builder.py | 2 +- authentik/crypto/tests.py | 2 ++ authentik/lib/tests/test_utils_reflection.py | 16 ++++++++++++++++ 3 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 authentik/lib/tests/test_utils_reflection.py diff --git a/authentik/crypto/builder.py b/authentik/crypto/builder.py index 55799a496..51ad4defb 100644 --- a/authentik/crypto/builder.py +++ b/authentik/crypto/builder.py @@ -33,7 +33,7 @@ class CertificateBuilder: def save(self) -> Optional[CertificateKeyPair]: """Save generated certificate as model""" if not self.__certificate: - return None + raise ValueError("Certificated hasn't been built yet") return CertificateKeyPair.objects.create( name=self.common_name, certificate_data=self.certificate, diff --git a/authentik/crypto/tests.py b/authentik/crypto/tests.py index 59df86b32..00dd0c30a 100644 --- a/authentik/crypto/tests.py +++ b/authentik/crypto/tests.py @@ -37,6 +37,8 @@ class TestCrypto(TestCase): """Test Builder""" builder = CertificateBuilder() builder.common_name = "test-cert" + with self.assertRaises(ValueError): + builder.save() builder.build( subject_alt_names=[], validity_days=3, diff --git a/authentik/lib/tests/test_utils_reflection.py b/authentik/lib/tests/test_utils_reflection.py new file mode 100644 index 000000000..4123cba61 --- /dev/null +++ b/authentik/lib/tests/test_utils_reflection.py @@ -0,0 +1,16 @@ +"""Test Reflection utils""" + +from datetime import datetime + +from django.test import TestCase + +from authentik.lib.utils.reflection import path_to_class + + +class TestReflectionUtils(TestCase): + """Test Reflection-utils""" + + def test_path_to_class(self): + """Test path_to_class""" + self.assertIsNone(path_to_class(None)) + self.assertEqual(path_to_class("datetime.datetime"), datetime) From ccef7b4233b0d3ab247b4508b6533c839055f7d6 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Tue, 27 Apr 2021 15:43:26 +0200 Subject: [PATCH 07/13] *: make logger not use .error Signed-off-by: Jens Langhammer --- authentik/flows/transfer/importer.py | 4 ++-- authentik/outposts/models.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/authentik/flows/transfer/importer.py b/authentik/flows/transfer/importer.py index 04ed65220..5c9fde10e 100644 --- a/authentik/flows/transfer/importer.py +++ b/authentik/flows/transfer/importer.py @@ -160,7 +160,7 @@ class FlowImporter: try: model: SerializerModel = apps.get_model(model_app_label, model_name) except LookupError: - self.logger.error( + self.logger.warning( "app or model does not exist", app=model_app_label, model=model_name ) return False @@ -168,7 +168,7 @@ class FlowImporter: try: serializer = self._validate_single(entry) except EntryInvalidError as exc: - self.logger.error("entry not valid", entry=entry, error=exc) + self.logger.warning("entry not valid", entry=entry, error=exc) return False model = serializer.save() diff --git a/authentik/outposts/models.py b/authentik/outposts/models.py index 33e653d5e..d8e75cdf4 100644 --- a/authentik/outposts/models.py +++ b/authentik/outposts/models.py @@ -201,7 +201,7 @@ class DockerServiceConnection(OutpostServiceConnection): ) client.containers.list() except DockerException as exc: - LOGGER.error(exc) + LOGGER.warning(exc) raise ServiceConnectionInvalid from exc return client From ceace0282b2eea51ab446fe3bbe296312a877b03 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Tue, 27 Apr 2021 15:43:40 +0200 Subject: [PATCH 08/13] web/admin: don't show docker certs as required Signed-off-by: Jens Langhammer --- web/src/pages/outposts/ServiceConnectionDockerForm.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/web/src/pages/outposts/ServiceConnectionDockerForm.ts b/web/src/pages/outposts/ServiceConnectionDockerForm.ts index 584dd4513..db8ceafa8 100644 --- a/web/src/pages/outposts/ServiceConnectionDockerForm.ts +++ b/web/src/pages/outposts/ServiceConnectionDockerForm.ts @@ -70,7 +70,6 @@ export class ServiceConnectionDockerForm extends Form { From 69af788b0fa4ea085caf306234afb24158da4641 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Tue, 27 Apr 2021 15:54:57 +0200 Subject: [PATCH 09/13] web: ignore network errors for sentry Signed-off-by: Jens Langhammer --- web/src/api/Sentry.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/web/src/api/Sentry.ts b/web/src/api/Sentry.ts index e012bfd05..7f0f39847 100644 --- a/web/src/api/Sentry.ts +++ b/web/src/api/Sentry.ts @@ -23,6 +23,11 @@ export function configureSentry(canDoPpi: boolean = false): Promise { if (hint.originalException instanceof SentryIgnoredError) { return null; } + if (hint.originalException instanceof Error) { + if (hint.originalException.name == 'NetworkError') { + return null; + } + } if (hint.originalException instanceof Response) { const response = hint.originalException as Response; // We only care about server errors From e9e0992dce0e61276a22acba3560f2551ce89e01 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Tue, 27 Apr 2021 16:21:44 +0200 Subject: [PATCH 10/13] root: add middleware to properly report websocket connection to sentry Signed-off-by: Jens Langhammer --- authentik/lib/sentry.py | 17 +++++++++++++++++ authentik/root/websocket.py | 7 +++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/authentik/lib/sentry.py b/authentik/lib/sentry.py index df3704ba9..27c5bd020 100644 --- a/authentik/lib/sentry.py +++ b/authentik/lib/sentry.py @@ -5,6 +5,7 @@ from aioredis.errors import ConnectionClosedError, ReplyError from billiard.exceptions import WorkerLostError from botocore.client import ClientError from celery.exceptions import CeleryError +from channels.middleware import BaseMiddleware from channels_redis.core import ChannelFull from django.core.exceptions import SuspiciousOperation, ValidationError from django.db import InternalError, OperationalError, ProgrammingError @@ -14,12 +15,28 @@ from ldap3.core.exceptions import LDAPException from redis.exceptions import ConnectionError as RedisConnectionError from redis.exceptions import RedisError, ResponseError from rest_framework.exceptions import APIException +from sentry_sdk import Hub +from sentry_sdk.tracing import Transaction from structlog.stdlib import get_logger from websockets.exceptions import WebSocketException +from authentik.lib.utils.reflection import class_to_path + LOGGER = get_logger() +class SentryWSMiddleware(BaseMiddleware): + """Sentry Websocket middleweare to set the transaction name based on + consumer class path""" + + async def __call__(self, scope, receive, send): + transaction: Optional[Transaction] = Hub.current.scope.transaction + class_path = class_to_path(self.inner.consumer_class) + if transaction: + transaction.name = class_path + return await self.inner(scope, receive, send) + + class SentryIgnoredException(Exception): """Base Class for all errors that are suppressed, and not sent to sentry.""" diff --git a/authentik/root/websocket.py b/authentik/root/websocket.py index d53b52a12..398b47f62 100644 --- a/authentik/root/websocket.py +++ b/authentik/root/websocket.py @@ -2,10 +2,13 @@ from channels.auth import AuthMiddlewareStack from django.urls import path +from authentik.lib.sentry import SentryWSMiddleware from authentik.outposts.channels import OutpostConsumer from authentik.root.messages.consumer import MessageConsumer websocket_urlpatterns = [ - path("ws/outpost//", OutpostConsumer.as_asgi()), - path("ws/client/", AuthMiddlewareStack(MessageConsumer.as_asgi())), + path("ws/outpost//", SentryWSMiddleware(OutpostConsumer.as_asgi())), + path( + "ws/client/", AuthMiddlewareStack(SentryWSMiddleware(MessageConsumer.as_asgi())) + ), ] From 776c3128b8e905a39eb678231015654664181ab5 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Tue, 27 Apr 2021 16:50:05 +0200 Subject: [PATCH 11/13] flows: add tests for stage type, component and ui_user_settings Signed-off-by: Jens Langhammer --- authentik/api/tests/test_config.py | 16 +++++++++++ authentik/core/api/utils.py | 6 ++-- authentik/flows/tests/test_stage_model.py | 35 +++++++++++++++++++++++ 3 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 authentik/api/tests/test_config.py create mode 100644 authentik/flows/tests/test_stage_model.py diff --git a/authentik/api/tests/test_config.py b/authentik/api/tests/test_config.py new file mode 100644 index 000000000..8e31e3ee0 --- /dev/null +++ b/authentik/api/tests/test_config.py @@ -0,0 +1,16 @@ +"""Test config API""" +from json import loads + +from django.urls import reverse +from rest_framework.test import APITestCase + + +class TestConfig(APITestCase): + """Test config API""" + + def test_config(self): + """Test YAML generation""" + response = self.client.get( + reverse("authentik_api:configs-list"), + ) + self.assertTrue(loads(response.content.decode())) diff --git a/authentik/core/api/utils.py b/authentik/core/api/utils.py index 5005090c3..7b8a98872 100644 --- a/authentik/core/api/utils.py +++ b/authentik/core/api/utils.py @@ -20,10 +20,12 @@ def is_dict(value: Any): class PassiveSerializer(Serializer): """Base serializer class which doesn't implement create/update methods""" - def create(self, validated_data: dict) -> Model: + def create(self, validated_data: dict) -> Model: # pragma: no cover return Model() - def update(self, instance: Model, validated_data: dict) -> Model: + def update( + self, instance: Model, validated_data: dict + ) -> Model: # pragma: no cover return Model() diff --git a/authentik/flows/tests/test_stage_model.py b/authentik/flows/tests/test_stage_model.py new file mode 100644 index 000000000..fe887ee89 --- /dev/null +++ b/authentik/flows/tests/test_stage_model.py @@ -0,0 +1,35 @@ +"""base model tests""" +from typing import Callable, Type + +from django.test import TestCase + +from authentik.flows.models import Stage +from authentik.flows.stage import StageView +from authentik.lib.utils.reflection import all_subclasses + + +class TestModels(TestCase): + """Generic model properties tests""" + + +def model_tester_factory(test_model: Type[Stage]) -> Callable: + """Test a form""" + + def tester(self: TestModels): + try: + model_class = None + if test_model._meta.abstract: + model_class = test_model.__bases__[0]() + else: + model_class = test_model() + self.assertTrue(issubclass(model_class.type, StageView)) + self.assertIsNotNone(test_model.component) + _ = test_model.ui_user_settings + except NotImplementedError: + pass + + return tester + + +for model in all_subclasses(Stage): + setattr(TestModels, f"test_model_{model.__name__}", model_tester_factory(model)) From 04f06e00ff94cd25b61cc1cea3ae38ec0ef7dda9 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Tue, 27 Apr 2021 17:04:38 +0200 Subject: [PATCH 12/13] api: add tests for permission_required decorator Signed-off-by: Jens Langhammer --- authentik/api/tests/test_decorators.py | 33 ++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 authentik/api/tests/test_decorators.py diff --git a/authentik/api/tests/test_decorators.py b/authentik/api/tests/test_decorators.py new file mode 100644 index 000000000..d7091280a --- /dev/null +++ b/authentik/api/tests/test_decorators.py @@ -0,0 +1,33 @@ +"""test decorators api""" +from django.urls import reverse +from guardian.shortcuts import assign_perm +from rest_framework.test import APITestCase + +from authentik.core.models import Application, User + + +class TestAPIDecorators(APITestCase): + """test decorators api""" + + def setUp(self) -> None: + super().setUp() + self.user = User.objects.create(username="test-user") + + def test_obj_perm_denied(self): + """Test object perm denied""" + self.client.force_login(self.user) + app = Application.objects.create(name="denied", slug="denied") + response = self.client.get( + reverse("authentik_api:application-metrics", kwargs={"slug": app.slug}) + ) + self.assertEqual(response.status_code, 403) + + def test_other_perm_denied(self): + """Test other perm denied""" + self.client.force_login(self.user) + app = Application.objects.create(name="denied", slug="denied") + assign_perm("authentik_core.view_application", self.user, app) + response = self.client.get( + reverse("authentik_api:application-metrics", kwargs={"slug": app.slug}) + ) + self.assertEqual(response.status_code, 403) From 6f0792ccfebc63ffea9bfeda047363e134e70bad Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Tue, 27 Apr 2021 17:04:51 +0200 Subject: [PATCH 13/13] api: remove legacy basic auth for 2021.3 outposts Signed-off-by: Jens Langhammer --- authentik/api/auth.py | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/authentik/api/auth.py b/authentik/api/auth.py index 27060a7e8..1992061f8 100644 --- a/authentik/api/auth.py +++ b/authentik/api/auth.py @@ -1,5 +1,5 @@ """API Authentication""" -from base64 import b64decode, b64encode +from base64 import b64decode from binascii import Error from typing import Any, Optional, Union @@ -19,14 +19,6 @@ def token_from_header(raw_header: bytes) -> Optional[Token]: auth_credentials = raw_header.decode() if auth_credentials == "": return None - # Legacy, accept basic auth thats fully encoded (2021.3 outposts) - if " " not in auth_credentials: - try: - plain = b64decode(auth_credentials.encode()).decode() - auth_type, body = plain.split() - auth_credentials = f"{auth_type} {b64encode(body.encode()).decode()}" - except (UnicodeDecodeError, Error): - raise AuthenticationFailed("Malformed header") auth_type, auth_credentials = auth_credentials.split() if auth_type.lower() not in ["basic", "bearer"]: LOGGER.debug("Unsupported authentication type, denying", type=auth_type.lower())