From b0fbd576fc69eeceb23e926890b1323efc35cb73 Mon Sep 17 00:00:00 2001 From: Jens L Date: Thu, 22 Jun 2023 22:25:04 +0200 Subject: [PATCH] security: cure53 fix (#6039) * ATH-01-001: resolve path and check start before loading blueprints This is even less of an issue since 411ef239f63e4d3beacd8297d4be54b29fb30127, since with that commit we only allow files that the listing returns Signed-off-by: Jens Langhammer * ATH-01-010: fix missing user filter for webauthn device This prevents an attack that is only possible when an attacker can intercept HTTP traffic and in the case of HTTPS decrypt it. * ATH-01-008: fix web forms not submitting correctly when pressing enter When submitting some forms with the Enter key instead of clicking "Confirm"/etc, the form would not get submitted correctly This would in the worst case is when setting a user's password, where the new password can end up in the URL, but the password was not actually saved to the user. * ATH-01-004: remove env from admin system endpoint this endpoint already required admin access, but for debugging the env variables are used very little Signed-off-by: Jens Langhammer * ATH-01-003 / ATH-01-012: disable htmlLabels in mermaid Signed-off-by: Jens Langhammer * ATH-01-005: use hmac.compare_digest for secret_key authentication Signed-off-by: Jens Langhammer * ATH-01-009: migrate impersonation to use API Signed-off-by: Jens Langhammer * ATH-01-010: rework Signed-off-by: Jens Langhammer * ATH-01-014: save authenticator validation state in flow context Signed-off-by: Jens Langhammer bugfixes Signed-off-by: Jens Langhammer * ATH-01-012: escape quotation marks Signed-off-by: Jens Langhammer * add website Signed-off-by: Jens Langhammer * update release ntoes Signed-off-by: Jens Langhammer * update with all notes Signed-off-by: Jens Langhammer * fix format Signed-off-by: Jens Langhammer --------- Signed-off-by: Jens Langhammer --- authentik/admin/api/system.py | 6 -- authentik/api/authentication.py | 3 +- authentik/blueprints/models.py | 5 +- authentik/blueprints/tests/test_models.py | 33 ++------- .../tests/test_serializer_models.py | 34 +++++++++ authentik/core/api/users.py | 55 +++++++++++++- authentik/core/tests/test_impersonation.py | 20 +++--- authentik/core/urls.py | 13 +--- authentik/core/views/impersonate.py | 60 ---------------- authentik/flows/api/flows_diagram.py | 3 +- authentik/flows/stage.py | 12 ++-- authentik/policies/tests/test_process.py | 4 +- authentik/sources/ldap/signals.py | 2 +- .../authenticator_validate/challenge.py | 6 ++ .../stages/authenticator_validate/stage.py | 40 +++++------ .../tests/test_stage.py | 72 ++++++++++++------- .../tests/test_webauthn.py | 10 +-- schema.yml | 62 ++++++++++++++-- web/src/admin/AdminInterface.ts | 11 +-- .../ApplicationCheckAccessForm.ts | 12 ++-- .../admin/crypto/CertificateGenerateForm.ts | 8 +-- web/src/admin/flows/FlowImportForm.ts | 8 +-- web/src/admin/groups/RelatedGroupList.ts | 66 +++++++++-------- web/src/admin/policies/PolicyTestForm.ts | 8 +-- .../PropertyMappingTestForm.ts | 8 +-- .../providers/saml/SAMLProviderImportForm.ts | 8 +-- web/src/admin/users/RelatedUserList.ts | 24 ++++--- web/src/admin/users/ServiceAccountForm.ts | 14 ++-- web/src/admin/users/UserListPage.ts | 16 +++-- web/src/admin/users/UserPasswordForm.ts | 14 ++-- web/src/admin/users/UserResetEmailForm.ts | 58 ++++++++------- web/src/admin/users/UserViewPage.ts | 16 +++-- web/src/elements/Diagram.ts | 1 + web/src/elements/forms/Form.ts | 14 ++++ web/src/user/UserInterface.ts | 32 +++++---- website/docs/releases/2023/v2023.4.md | 4 ++ website/docs/releases/2023/v2023.5.md | 4 ++ website/docs/security/2023-06-cure53.md | 67 +++++++++++++++++ website/sidebars.js | 1 + 39 files changed, 505 insertions(+), 329 deletions(-) create mode 100644 authentik/blueprints/tests/test_serializer_models.py delete mode 100644 authentik/core/views/impersonate.py create mode 100644 website/docs/security/2023-06-cure53.md diff --git a/authentik/admin/api/system.py b/authentik/admin/api/system.py index c1e74672a..11dc5dfec 100644 --- a/authentik/admin/api/system.py +++ b/authentik/admin/api/system.py @@ -1,5 +1,4 @@ """authentik administration overview""" -import os import platform from datetime import datetime from sys import version as python_version @@ -34,7 +33,6 @@ class RuntimeDict(TypedDict): class SystemSerializer(PassiveSerializer): """Get system information.""" - env = SerializerMethodField() http_headers = SerializerMethodField() http_host = SerializerMethodField() http_is_secure = SerializerMethodField() @@ -43,10 +41,6 @@ class SystemSerializer(PassiveSerializer): server_time = SerializerMethodField() embedded_outpost_host = SerializerMethodField() - def get_env(self, request: Request) -> dict[str, str]: - """Get Environment""" - return os.environ.copy() - def get_http_headers(self, request: Request) -> dict[str, str]: """Get HTTP Request headers""" headers = {} diff --git a/authentik/api/authentication.py b/authentik/api/authentication.py index 1e4870828..8aa402384 100644 --- a/authentik/api/authentication.py +++ b/authentik/api/authentication.py @@ -1,4 +1,5 @@ """API Authentication""" +from hmac import compare_digest from typing import Any, Optional from django.conf import settings @@ -78,7 +79,7 @@ def token_secret_key(value: str) -> Optional[User]: and return the service account for the managed outpost""" from authentik.outposts.apps import MANAGED_OUTPOST - if value != settings.SECRET_KEY: + if not compare_digest(value, settings.SECRET_KEY): return None outposts = Outpost.objects.filter(managed=MANAGED_OUTPOST) if not outposts: diff --git a/authentik/blueprints/models.py b/authentik/blueprints/models.py index 130b13264..e1099d2de 100644 --- a/authentik/blueprints/models.py +++ b/authentik/blueprints/models.py @@ -82,7 +82,10 @@ class BlueprintInstance(SerializerModel, ManagedModel, CreatedUpdatedModel): def retrieve_file(self) -> str: """Get blueprint from path""" try: - full_path = Path(CONFIG.y("blueprints_dir")).joinpath(Path(self.path)) + base = Path(CONFIG.y("blueprints_dir")) + full_path = base.joinpath(Path(self.path)).resolve() + if not str(full_path).startswith(str(base.resolve())): + raise BlueprintRetrievalFailed("Invalid blueprint path") with full_path.open("r", encoding="utf-8") as _file: return _file.read() except (IOError, OSError) as exc: diff --git a/authentik/blueprints/tests/test_models.py b/authentik/blueprints/tests/test_models.py index 718caa502..2c04b64ef 100644 --- a/authentik/blueprints/tests/test_models.py +++ b/authentik/blueprints/tests/test_models.py @@ -1,34 +1,15 @@ """authentik managed models tests""" -from typing import Callable, Type - -from django.apps import apps from django.test import TestCase -from authentik.blueprints.v1.importer import is_model_allowed -from authentik.lib.models import SerializerModel +from authentik.blueprints.models import BlueprintInstance, BlueprintRetrievalFailed +from authentik.lib.generators import generate_id class TestModels(TestCase): """Test Models""" - -def serializer_tester_factory(test_model: Type[SerializerModel]) -> Callable: - """Test serializer""" - - def tester(self: TestModels): - if test_model._meta.abstract: # pragma: no cover - return - model_class = test_model() - self.assertTrue(isinstance(model_class, SerializerModel)) - self.assertIsNotNone(model_class.serializer) - - return tester - - -for app in apps.get_app_configs(): - if not app.label.startswith("authentik"): - continue - for model in app.get_models(): - if not is_model_allowed(model): - continue - setattr(TestModels, f"test_{app.label}_{model.__name__}", serializer_tester_factory(model)) + def test_retrieve_file(self): + """Test retrieve_file""" + instance = BlueprintInstance.objects.create(name=generate_id(), path="../etc/hosts") + with self.assertRaises(BlueprintRetrievalFailed): + instance.retrieve() diff --git a/authentik/blueprints/tests/test_serializer_models.py b/authentik/blueprints/tests/test_serializer_models.py new file mode 100644 index 000000000..718caa502 --- /dev/null +++ b/authentik/blueprints/tests/test_serializer_models.py @@ -0,0 +1,34 @@ +"""authentik managed models tests""" +from typing import Callable, Type + +from django.apps import apps +from django.test import TestCase + +from authentik.blueprints.v1.importer import is_model_allowed +from authentik.lib.models import SerializerModel + + +class TestModels(TestCase): + """Test Models""" + + +def serializer_tester_factory(test_model: Type[SerializerModel]) -> Callable: + """Test serializer""" + + def tester(self: TestModels): + if test_model._meta.abstract: # pragma: no cover + return + model_class = test_model() + self.assertTrue(isinstance(model_class, SerializerModel)) + self.assertIsNotNone(model_class.serializer) + + return tester + + +for app in apps.get_app_configs(): + if not app.label.startswith("authentik"): + continue + for model in app.get_models(): + if not is_model_allowed(model): + continue + setattr(TestModels, f"test_{app.label}_{model.__name__}", serializer_tester_factory(model)) diff --git a/authentik/core/api/users.py b/authentik/core/api/users.py index 911948fd0..108714fc3 100644 --- a/authentik/core/api/users.py +++ b/authentik/core/api/users.py @@ -68,11 +68,12 @@ from authentik.core.models import ( TokenIntents, User, ) -from authentik.events.models import EventAction +from authentik.events.models import Event, EventAction from authentik.flows.exceptions import FlowNonApplicableException from authentik.flows.models import FlowToken from authentik.flows.planner import PLAN_CONTEXT_PENDING_USER, FlowPlanner from authentik.flows.views.executor import QS_KEY_TOKEN +from authentik.lib.config import CONFIG from authentik.stages.email.models import EmailStage from authentik.stages.email.tasks import send_mails from authentik.stages.email.utils import TemplateEmailMessage @@ -568,6 +569,58 @@ class UserViewSet(UsedByMixin, ModelViewSet): send_mails(email_stage, message) return Response(status=204) + @permission_required("authentik_core.impersonate") + @extend_schema( + request=OpenApiTypes.NONE, + responses={ + "204": OpenApiResponse(description="Successfully started impersonation"), + "401": OpenApiResponse(description="Access denied"), + }, + ) + @action(detail=True, methods=["POST"]) + def impersonate(self, request: Request, pk: int) -> Response: + """Impersonate a user""" + if not CONFIG.y_bool("impersonation"): + LOGGER.debug("User attempted to impersonate", user=request.user) + return Response(status=401) + if not request.user.has_perm("impersonate"): + LOGGER.debug("User attempted to impersonate without permissions", user=request.user) + return Response(status=401) + + user_to_be = self.get_object() + + request.session[SESSION_KEY_IMPERSONATE_ORIGINAL_USER] = request.user + request.session[SESSION_KEY_IMPERSONATE_USER] = user_to_be + + Event.new(EventAction.IMPERSONATION_STARTED).from_http(request, user_to_be) + + return Response(status=201) + + @extend_schema( + request=OpenApiTypes.NONE, + responses={ + "204": OpenApiResponse(description="Successfully started impersonation"), + }, + ) + @action(detail=False, methods=["GET"]) + def impersonate_end(self, request: Request) -> Response: + """End Impersonation a user""" + if ( + SESSION_KEY_IMPERSONATE_USER not in request.session + or SESSION_KEY_IMPERSONATE_ORIGINAL_USER not in request.session + ): + LOGGER.debug("Can't end impersonation", user=request.user) + return Response(status=204) + + original_user = request.session[SESSION_KEY_IMPERSONATE_ORIGINAL_USER] + + del request.session[SESSION_KEY_IMPERSONATE_USER] + del request.session[SESSION_KEY_IMPERSONATE_ORIGINAL_USER] + + Event.new(EventAction.IMPERSONATION_ENDED).from_http(request, original_user) + + return Response(status=204) + def _filter_queryset_for_list(self, queryset: QuerySet) -> QuerySet: """Custom filter_queryset method which ignores guardian, but still supports sorting""" for backend in list(self.filter_backends): diff --git a/authentik/core/tests/test_impersonation.py b/authentik/core/tests/test_impersonation.py index 12d309d59..6d17393a8 100644 --- a/authentik/core/tests/test_impersonation.py +++ b/authentik/core/tests/test_impersonation.py @@ -1,14 +1,14 @@ """impersonation tests""" from json import loads -from django.test.testcases import TestCase from django.urls import reverse +from rest_framework.test import APITestCase from authentik.core.models import User from authentik.core.tests.utils import create_test_admin_user -class TestImpersonation(TestCase): +class TestImpersonation(APITestCase): """impersonation tests""" def setUp(self) -> None: @@ -23,10 +23,10 @@ class TestImpersonation(TestCase): self.other_user.save() self.client.force_login(self.user) - self.client.get( + self.client.post( reverse( - "authentik_core:impersonate-init", - kwargs={"user_id": self.other_user.pk}, + "authentik_api:user-impersonate", + kwargs={"pk": self.other_user.pk}, ) ) @@ -35,7 +35,7 @@ class TestImpersonation(TestCase): self.assertEqual(response_body["user"]["username"], self.other_user.username) self.assertEqual(response_body["original"]["username"], self.user.username) - self.client.get(reverse("authentik_core:impersonate-end")) + self.client.get(reverse("authentik_api:user-impersonate-end")) response = self.client.get(reverse("authentik_api:user-me")) response_body = loads(response.content.decode()) @@ -46,9 +46,7 @@ class TestImpersonation(TestCase): """test impersonation without permissions""" self.client.force_login(self.other_user) - self.client.get( - reverse("authentik_core:impersonate-init", kwargs={"user_id": self.user.pk}) - ) + self.client.get(reverse("authentik_api:user-impersonate", kwargs={"pk": self.user.pk})) response = self.client.get(reverse("authentik_api:user-me")) response_body = loads(response.content.decode()) @@ -58,5 +56,5 @@ class TestImpersonation(TestCase): """test un-impersonation without impersonating first""" self.client.force_login(self.other_user) - response = self.client.get(reverse("authentik_core:impersonate-end")) - self.assertRedirects(response, reverse("authentik_core:if-user")) + response = self.client.get(reverse("authentik_api:user-impersonate-end")) + self.assertEqual(response.status_code, 204) diff --git a/authentik/core/urls.py b/authentik/core/urls.py index 0fc6aab0a..c9aa748c5 100644 --- a/authentik/core/urls.py +++ b/authentik/core/urls.py @@ -16,7 +16,7 @@ from authentik.core.api.providers import ProviderViewSet from authentik.core.api.sources import SourceViewSet, UserSourceConnectionViewSet from authentik.core.api.tokens import TokenViewSet from authentik.core.api.users import UserViewSet -from authentik.core.views import apps, impersonate +from authentik.core.views import apps from authentik.core.views.debug import AccessDeniedView from authentik.core.views.interface import FlowInterfaceView, InterfaceView from authentik.core.views.session import EndSessionView @@ -38,17 +38,6 @@ urlpatterns = [ apps.RedirectToAppLaunch.as_view(), name="application-launch", ), - # Impersonation - path( - "-/impersonation//", - impersonate.ImpersonateInitView.as_view(), - name="impersonate-init", - ), - path( - "-/impersonation/end/", - impersonate.ImpersonateEndView.as_view(), - name="impersonate-end", - ), # Interfaces path( "if/admin/", diff --git a/authentik/core/views/impersonate.py b/authentik/core/views/impersonate.py deleted file mode 100644 index c19a47f62..000000000 --- a/authentik/core/views/impersonate.py +++ /dev/null @@ -1,60 +0,0 @@ -"""authentik impersonation views""" - -from django.http import HttpRequest, HttpResponse -from django.shortcuts import get_object_or_404, redirect -from django.views import View -from structlog.stdlib import get_logger - -from authentik.core.middleware import ( - SESSION_KEY_IMPERSONATE_ORIGINAL_USER, - SESSION_KEY_IMPERSONATE_USER, -) -from authentik.core.models import User -from authentik.events.models import Event, EventAction -from authentik.lib.config import CONFIG - -LOGGER = get_logger() - - -class ImpersonateInitView(View): - """Initiate Impersonation""" - - def get(self, request: HttpRequest, user_id: int) -> HttpResponse: - """Impersonation handler, checks permissions""" - if not CONFIG.y_bool("impersonation"): - LOGGER.debug("User attempted to impersonate", user=request.user) - return HttpResponse("Unauthorized", status=401) - if not request.user.has_perm("impersonate"): - LOGGER.debug("User attempted to impersonate without permissions", user=request.user) - return HttpResponse("Unauthorized", status=401) - - user_to_be = get_object_or_404(User, pk=user_id) - - request.session[SESSION_KEY_IMPERSONATE_ORIGINAL_USER] = request.user - request.session[SESSION_KEY_IMPERSONATE_USER] = user_to_be - - Event.new(EventAction.IMPERSONATION_STARTED).from_http(request, user_to_be) - - return redirect("authentik_core:if-user") - - -class ImpersonateEndView(View): - """End User impersonation""" - - def get(self, request: HttpRequest) -> HttpResponse: - """End Impersonation handler""" - if ( - SESSION_KEY_IMPERSONATE_USER not in request.session - or SESSION_KEY_IMPERSONATE_ORIGINAL_USER not in request.session - ): - LOGGER.debug("Can't end impersonation", user=request.user) - return redirect("authentik_core:if-user") - - original_user = request.session[SESSION_KEY_IMPERSONATE_ORIGINAL_USER] - - del request.session[SESSION_KEY_IMPERSONATE_USER] - del request.session[SESSION_KEY_IMPERSONATE_ORIGINAL_USER] - - Event.new(EventAction.IMPERSONATION_ENDED).from_http(request, original_user) - - return redirect("authentik_core:root-redirect") diff --git a/authentik/flows/api/flows_diagram.py b/authentik/flows/api/flows_diagram.py index e72bceea4..ab75fa235 100644 --- a/authentik/flows/api/flows_diagram.py +++ b/authentik/flows/api/flows_diagram.py @@ -23,7 +23,8 @@ class DiagramElement: style: list[str] = field(default_factory=lambda: ["[", "]"]) def __str__(self) -> str: - element = f'{self.identifier}{self.style[0]}"{self.description}"{self.style[1]}' + description = self.description.replace('"', "#quot;") + element = f'{self.identifier}{self.style[0]}"{description}"{self.style[1]}' if self.action is not None: if self.action != "": element = f"--{self.action}--> {element}" diff --git a/authentik/flows/stage.py b/authentik/flows/stage.py index 68589c8fd..135d54f4d 100644 --- a/authentik/flows/stage.py +++ b/authentik/flows/stage.py @@ -204,12 +204,12 @@ class ChallengeStageView(StageView): for field, errors in response.errors.items(): for error in errors: full_errors.setdefault(field, []) - full_errors[field].append( - { - "string": str(error), - "code": error.code, - } - ) + field_error = { + "string": str(error), + } + if hasattr(error, "code"): + field_error["code"] = error.code + full_errors[field].append(field_error) challenge_response.initial_data["response_errors"] = full_errors if not challenge_response.is_valid(): self.logger.error( diff --git a/authentik/policies/tests/test_process.py b/authentik/policies/tests/test_process.py index fa880e13b..dd2eafe04 100644 --- a/authentik/policies/tests/test_process.py +++ b/authentik/policies/tests/test_process.py @@ -132,9 +132,9 @@ class TestPolicyProcess(TestCase): ) binding = PolicyBinding(policy=policy, target=Application.objects.create(name="test")) - http_request = self.factory.get(reverse("authentik_core:impersonate-end")) + http_request = self.factory.get(reverse("authentik_api:user-impersonate-end")) http_request.user = self.user - http_request.resolver_match = resolve(reverse("authentik_core:impersonate-end")) + http_request.resolver_match = resolve(reverse("authentik_api:user-impersonate-end")) request = PolicyRequest(self.user) request.set_http_request(http_request) diff --git a/authentik/sources/ldap/signals.py b/authentik/sources/ldap/signals.py index e4279d40f..81d8c692e 100644 --- a/authentik/sources/ldap/signals.py +++ b/authentik/sources/ldap/signals.py @@ -66,8 +66,8 @@ def ldap_sync_password(sender, user: User, password: str, **_): if not sources.exists(): return source = sources.first() - changer = LDAPPasswordChanger(source) try: + changer = LDAPPasswordChanger(source) changer.change_password(user, password) except LDAPOperationResult as exc: LOGGER.warning("failed to set LDAP password", exc=exc) diff --git a/authentik/stages/authenticator_validate/challenge.py b/authentik/stages/authenticator_validate/challenge.py index beb7f3a90..96e5dc764 100644 --- a/authentik/stages/authenticator_validate/challenge.py +++ b/authentik/stages/authenticator_validate/challenge.py @@ -133,6 +133,12 @@ def validate_challenge_webauthn(data: dict, stage_view: StageView, user: User) - device = WebAuthnDevice.objects.filter(credential_id=credential_id).first() if not device: raise ValidationError("Invalid device") + # We can only check the device's user if the user we're given isn't anonymous + # as this validation is also used for password-less login where webauthn is the very first + # step done by a user. Only if this validation happens at a later stage we can check + # that the device belongs to the user + if not user.is_anonymous and device.user != user: + raise ValidationError("Invalid device") stage: AuthenticatorValidateStage = stage_view.executor.current_stage diff --git a/authentik/stages/authenticator_validate/stage.py b/authentik/stages/authenticator_validate/stage.py index 6a7257f9c..1e53cab6c 100644 --- a/authentik/stages/authenticator_validate/stage.py +++ b/authentik/stages/authenticator_validate/stage.py @@ -37,9 +37,9 @@ from authentik.stages.password.stage import PLAN_CONTEXT_METHOD, PLAN_CONTEXT_ME COOKIE_NAME_MFA = "authentik_mfa" -SESSION_KEY_STAGES = "authentik/stages/authenticator_validate/stages" -SESSION_KEY_SELECTED_STAGE = "authentik/stages/authenticator_validate/selected_stage" -SESSION_KEY_DEVICE_CHALLENGES = "authentik/stages/authenticator_validate/device_challenges" +PLAN_CONTEXT_STAGES = "goauthentik.io/stages/authenticator_validate/stages" +PLAN_CONTEXT_SELECTED_STAGE = "goauthentik.io/stages/authenticator_validate/selected_stage" +PLAN_CONTEXT_DEVICE_CHALLENGES = "goauthentik.io/stages/authenticator_validate/device_challenges" class SelectableStageSerializer(PassiveSerializer): @@ -73,8 +73,8 @@ class AuthenticatorValidationChallengeResponse(ChallengeResponse): component = CharField(default="ak-stage-authenticator-validate") def _challenge_allowed(self, classes: list): - device_challenges: list[dict] = self.stage.request.session.get( - SESSION_KEY_DEVICE_CHALLENGES, [] + device_challenges: list[dict] = self.stage.executor.plan.context.get( + PLAN_CONTEXT_DEVICE_CHALLENGES, [] ) if not any(x["device_class"] in classes for x in device_challenges): raise ValidationError("No compatible device class allowed") @@ -104,7 +104,9 @@ class AuthenticatorValidationChallengeResponse(ChallengeResponse): """Check which challenge the user has selected. Actual logic only used for SMS stage.""" # First check if the challenge is valid allowed = False - for device_challenge in self.stage.request.session.get(SESSION_KEY_DEVICE_CHALLENGES, []): + for device_challenge in self.stage.executor.plan.context.get( + PLAN_CONTEXT_DEVICE_CHALLENGES, [] + ): if device_challenge.get("device_class", "") == challenge.get( "device_class", "" ) and device_challenge.get("device_uid", "") == challenge.get("device_uid", ""): @@ -122,11 +124,11 @@ class AuthenticatorValidationChallengeResponse(ChallengeResponse): def validate_selected_stage(self, stage_pk: str) -> str: """Check that the selected stage is valid""" - stages = self.stage.request.session.get(SESSION_KEY_STAGES, []) + stages = self.stage.executor.plan.context.get(PLAN_CONTEXT_STAGES, []) if not any(str(stage.pk) == stage_pk for stage in stages): raise ValidationError("Selected stage is invalid") self.stage.logger.debug("Setting selected stage to ", stage=stage_pk) - self.stage.request.session[SESSION_KEY_SELECTED_STAGE] = stage_pk + self.stage.executor.plan.context[PLAN_CONTEXT_SELECTED_STAGE] = stage_pk return stage_pk def validate(self, attrs: dict): @@ -231,7 +233,7 @@ class AuthenticatorValidateStageView(ChallengeStageView): else: self.logger.debug("No pending user, continuing") return self.executor.stage_ok() - self.request.session[SESSION_KEY_DEVICE_CHALLENGES] = challenges + self.executor.plan.context[PLAN_CONTEXT_DEVICE_CHALLENGES] = challenges # No allowed devices if len(challenges) < 1: @@ -264,23 +266,23 @@ class AuthenticatorValidateStageView(ChallengeStageView): if stage.configuration_stages.count() == 1: next_stage = Stage.objects.get_subclass(pk=stage.configuration_stages.first().pk) self.logger.debug("Single stage configured, auto-selecting", stage=next_stage) - self.request.session[SESSION_KEY_SELECTED_STAGE] = next_stage + self.executor.plan.context[PLAN_CONTEXT_SELECTED_STAGE] = next_stage # Because that normal execution only happens on post, we directly inject it here and # return it self.executor.plan.insert_stage(next_stage) return self.executor.stage_ok() stages = Stage.objects.filter(pk__in=stage.configuration_stages.all()).select_subclasses() - self.request.session[SESSION_KEY_STAGES] = stages + self.executor.plan.context[PLAN_CONTEXT_STAGES] = stages return super().get(self.request, *args, **kwargs) def post(self, request: HttpRequest, *args, **kwargs) -> HttpResponse: res = super().post(request, *args, **kwargs) if ( - SESSION_KEY_SELECTED_STAGE in self.request.session + PLAN_CONTEXT_SELECTED_STAGE in self.executor.plan.context and self.executor.current_stage.not_configured_action == NotConfiguredAction.CONFIGURE ): - self.logger.debug("Got selected stage in session, running that") - stage_pk = self.request.session.get(SESSION_KEY_SELECTED_STAGE) + self.logger.debug("Got selected stage in context, running that") + stage_pk = self.executor.plan.context(PLAN_CONTEXT_SELECTED_STAGE) # Because the foreign key to stage.configuration_stage points to # a base stage class, we need to do another lookup stage = Stage.objects.get_subclass(pk=stage_pk) @@ -291,8 +293,8 @@ class AuthenticatorValidateStageView(ChallengeStageView): return res def get_challenge(self) -> AuthenticatorValidationChallenge: - challenges = self.request.session.get(SESSION_KEY_DEVICE_CHALLENGES, []) - stages = self.request.session.get(SESSION_KEY_STAGES, []) + challenges = self.executor.plan.context.get(PLAN_CONTEXT_DEVICE_CHALLENGES, []) + stages = self.executor.plan.context.get(PLAN_CONTEXT_STAGES, []) stage_challenges = [] for stage in stages: serializer = SelectableStageSerializer( @@ -307,6 +309,7 @@ class AuthenticatorValidateStageView(ChallengeStageView): stage_challenges.append(serializer.data) return AuthenticatorValidationChallenge( data={ + "component": "ak-stage-authenticator-validate", "type": ChallengeTypes.NATIVE.value, "device_challenges": challenges, "configuration_stages": stage_challenges, @@ -386,8 +389,3 @@ class AuthenticatorValidateStageView(ChallengeStageView): "device": webauthn_device, } return self.set_valid_mfa_cookie(response.device) - - def cleanup(self): - self.request.session.pop(SESSION_KEY_STAGES, None) - self.request.session.pop(SESSION_KEY_SELECTED_STAGE, None) - self.request.session.pop(SESSION_KEY_DEVICE_CHALLENGES, None) diff --git a/authentik/stages/authenticator_validate/tests/test_stage.py b/authentik/stages/authenticator_validate/tests/test_stage.py index df111993d..1a342752a 100644 --- a/authentik/stages/authenticator_validate/tests/test_stage.py +++ b/authentik/stages/authenticator_validate/tests/test_stage.py @@ -1,26 +1,19 @@ """Test validator stage""" from unittest.mock import MagicMock, patch -from django.contrib.sessions.middleware import SessionMiddleware from django.test.client import RequestFactory from django.urls.base import reverse -from rest_framework.exceptions import ValidationError from authentik.core.tests.utils import create_test_admin_user, create_test_flow from authentik.flows.models import FlowDesignation, FlowStageBinding, NotConfiguredAction from authentik.flows.planner import FlowPlan -from authentik.flows.stage import StageView from authentik.flows.tests import FlowTestCase -from authentik.flows.views.executor import SESSION_KEY_PLAN, FlowExecutorView +from authentik.flows.views.executor import SESSION_KEY_PLAN from authentik.lib.generators import generate_id, generate_key -from authentik.lib.tests.utils import dummy_get_response from authentik.stages.authenticator_duo.models import AuthenticatorDuoStage, DuoDevice from authentik.stages.authenticator_validate.api import AuthenticatorValidateStageSerializer from authentik.stages.authenticator_validate.models import AuthenticatorValidateStage, DeviceClasses -from authentik.stages.authenticator_validate.stage import ( - SESSION_KEY_DEVICE_CHALLENGES, - AuthenticatorValidationChallengeResponse, -) +from authentik.stages.authenticator_validate.stage import PLAN_CONTEXT_DEVICE_CHALLENGES from authentik.stages.identification.models import IdentificationStage, UserFields @@ -86,12 +79,17 @@ class AuthenticatorValidateStageTests(FlowTestCase): def test_validate_selected_challenge(self): """Test validate_selected_challenge""" - # Prepare request with session - request = self.request_factory.get("/") + flow = create_test_flow() + stage = AuthenticatorValidateStage.objects.create( + name=generate_id(), + not_configured_action=NotConfiguredAction.CONFIGURE, + device_classes=[DeviceClasses.STATIC, DeviceClasses.TOTP], + ) - middleware = SessionMiddleware(dummy_get_response) - middleware.process_request(request) - request.session[SESSION_KEY_DEVICE_CHALLENGES] = [ + session = self.client.session + plan = FlowPlan(flow_pk=flow.pk.hex) + plan.append_stage(stage) + plan.context[PLAN_CONTEXT_DEVICE_CHALLENGES] = [ { "device_class": "static", "device_uid": "1", @@ -101,23 +99,43 @@ class AuthenticatorValidateStageTests(FlowTestCase): "device_uid": "2", }, ] - request.session.save() + session[SESSION_KEY_PLAN] = plan + session.save() - res = AuthenticatorValidationChallengeResponse() - res.stage = StageView(FlowExecutorView()) - res.stage.request = request - with self.assertRaises(ValidationError): - res.validate_selected_challenge( - { + response = self.client.post( + reverse("authentik_api:flow-executor", kwargs={"flow_slug": flow.slug}), + data={ + "selected_challenge": { "device_class": "baz", "device_uid": "quox", + "challenge": {}, } - ) - res.validate_selected_challenge( - { - "device_class": "static", - "device_uid": "1", - } + }, + ) + self.assertStageResponse( + response, + flow, + response_errors={ + "selected_challenge": [{"string": "invalid challenge selected", "code": "invalid"}] + }, + component="ak-stage-authenticator-validate", + ) + + response = self.client.post( + reverse("authentik_api:flow-executor", kwargs={"flow_slug": flow.slug}), + data={ + "selected_challenge": { + "device_class": "static", + "device_uid": "1", + "challenge": {}, + }, + }, + ) + self.assertStageResponse( + response, + flow, + response_errors={"non_field_errors": [{"string": "Empty response", "code": "invalid"}]}, + component="ak-stage-authenticator-validate", ) @patch( diff --git a/authentik/stages/authenticator_validate/tests/test_webauthn.py b/authentik/stages/authenticator_validate/tests/test_webauthn.py index b4d383762..3a6071089 100644 --- a/authentik/stages/authenticator_validate/tests/test_webauthn.py +++ b/authentik/stages/authenticator_validate/tests/test_webauthn.py @@ -22,7 +22,7 @@ from authentik.stages.authenticator_validate.challenge import ( ) from authentik.stages.authenticator_validate.models import AuthenticatorValidateStage, DeviceClasses from authentik.stages.authenticator_validate.stage import ( - SESSION_KEY_DEVICE_CHALLENGES, + PLAN_CONTEXT_DEVICE_CHALLENGES, AuthenticatorValidateStageView, ) from authentik.stages.authenticator_webauthn.models import UserVerification, WebAuthnDevice @@ -211,14 +211,14 @@ class AuthenticatorValidateStageWebAuthnTests(FlowTestCase): plan.append_stage(stage) plan.append_stage(UserLoginStage(name=generate_id())) plan.context[PLAN_CONTEXT_PENDING_USER] = self.user - session[SESSION_KEY_PLAN] = plan - session[SESSION_KEY_DEVICE_CHALLENGES] = [ + plan.context[PLAN_CONTEXT_DEVICE_CHALLENGES] = [ { "device_class": device.__class__.__name__.lower().replace("device", ""), "device_uid": device.pk, "challenge": {}, } ] + session[SESSION_KEY_PLAN] = plan session[SESSION_KEY_WEBAUTHN_CHALLENGE] = base64url_to_bytes( "g98I51mQvZXo5lxLfhrD2zfolhZbLRyCgqkkYap1jwSaJ13BguoJWCF9_Lg3AgO4Wh-Bqa556JE20oKsYbl6RA" ) @@ -283,14 +283,14 @@ class AuthenticatorValidateStageWebAuthnTests(FlowTestCase): plan = FlowPlan(flow_pk=flow.pk.hex) plan.append_stage(stage) plan.append_stage(UserLoginStage(name=generate_id())) - session[SESSION_KEY_PLAN] = plan - session[SESSION_KEY_DEVICE_CHALLENGES] = [ + plan.context[PLAN_CONTEXT_DEVICE_CHALLENGES] = [ { "device_class": device.__class__.__name__.lower().replace("device", ""), "device_uid": device.pk, "challenge": {}, } ] + session[SESSION_KEY_PLAN] = plan session[SESSION_KEY_WEBAUTHN_CHALLENGE] = base64url_to_bytes( "g98I51mQvZXo5lxLfhrD2zfolhZbLRyCgqkkYap1jwSaJ13BguoJWCF9_Lg3AgO4Wh-Bqa556JE20oKsYbl6RA" ) diff --git a/schema.yml b/schema.yml index 4d96e34af..5d14bdf0f 100644 --- a/schema.yml +++ b/schema.yml @@ -4812,6 +4812,38 @@ paths: schema: $ref: '#/components/schemas/GenericError' description: '' + /core/users/{id}/impersonate/: + post: + operationId: core_users_impersonate_create + description: Impersonate a user + parameters: + - in: path + name: id + schema: + type: integer + description: A unique integer value identifying this User. + required: true + tags: + - core + security: + - authentik: [] + responses: + '204': + description: Successfully started impersonation + '401': + description: Access denied + '400': + content: + application/json: + schema: + $ref: '#/components/schemas/ValidationError' + description: '' + '403': + content: + application/json: + schema: + $ref: '#/components/schemas/GenericError' + description: '' /core/users/{id}/metrics/: get: operationId: core_users_metrics_retrieve @@ -4991,6 +5023,29 @@ paths: schema: $ref: '#/components/schemas/GenericError' description: '' + /core/users/impersonate_end/: + get: + operationId: core_users_impersonate_end_retrieve + description: End Impersonation a user + tags: + - core + security: + - authentik: [] + responses: + '204': + description: Successfully started impersonation + '400': + content: + application/json: + schema: + $ref: '#/components/schemas/ValidationError' + description: '' + '403': + content: + application/json: + schema: + $ref: '#/components/schemas/GenericError' + description: '' /core/users/me/: get: operationId: core_users_me_retrieve @@ -41003,12 +41058,6 @@ components: type: object description: Get system information. properties: - env: - type: object - additionalProperties: - type: string - description: Get Environment - readOnly: true http_headers: type: object additionalProperties: @@ -41062,7 +41111,6 @@ components: readOnly: true required: - embedded_outpost_host - - env - http_headers - http_host - http_is_secure diff --git a/web/src/admin/AdminInterface.ts b/web/src/admin/AdminInterface.ts index d6b19cc7f..d467ad08a 100644 --- a/web/src/admin/AdminInterface.ts +++ b/web/src/admin/AdminInterface.ts @@ -30,7 +30,7 @@ import PFDrawer from "@patternfly/patternfly/components/Drawer/drawer.css"; import PFPage from "@patternfly/patternfly/components/Page/page.css"; import PFBase from "@patternfly/patternfly/patternfly-base.css"; -import { AdminApi, SessionUser, UiThemeEnum, Version } from "@goauthentik/api"; +import { AdminApi, CoreApi, SessionUser, UiThemeEnum, Version } from "@goauthentik/api"; autoDetectLanguage(); @@ -178,10 +178,11 @@ export class AdminInterface extends Interface { ${this.user?.original ? html` { + new CoreApi(DEFAULT_CONFIG).coreUsersImpersonateEndRetrieve().then(() => { + window.location.reload(); + }); + }} > ${msg( diff --git a/web/src/admin/applications/ApplicationCheckAccessForm.ts b/web/src/admin/applications/ApplicationCheckAccessForm.ts index 42d106dd0..90beb3786 100644 --- a/web/src/admin/applications/ApplicationCheckAccessForm.ts +++ b/web/src/admin/applications/ApplicationCheckAccessForm.ts @@ -114,9 +114,12 @@ export class ApplicationCheckAccessForm extends Form<{ forUser: number }> { `; } - renderForm(): TemplateResult { - return html`
- + renderInlineForm(): TemplateResult { + return html` => { const args: CoreUsersListRequest = { @@ -143,7 +146,6 @@ export class ApplicationCheckAccessForm extends Form<{ forUser: number }> { > - ${this.result ? this.renderResult() : html``} - `; + ${this.result ? this.renderResult() : html``}`; } } diff --git a/web/src/admin/crypto/CertificateGenerateForm.ts b/web/src/admin/crypto/CertificateGenerateForm.ts index f936c63d2..000c18462 100644 --- a/web/src/admin/crypto/CertificateGenerateForm.ts +++ b/web/src/admin/crypto/CertificateGenerateForm.ts @@ -20,9 +20,8 @@ export class CertificateKeyPairForm extends Form { }); } - renderForm(): TemplateResult { - return html`
- { ?required=${true} > - -
`; +
`; } } diff --git a/web/src/admin/flows/FlowImportForm.ts b/web/src/admin/flows/FlowImportForm.ts index 86f284e2f..3db49d056 100644 --- a/web/src/admin/flows/FlowImportForm.ts +++ b/web/src/admin/flows/FlowImportForm.ts @@ -86,9 +86,8 @@ export class FlowImportForm extends Form { `; } - renderForm(): TemplateResult { - return html`
- + renderInlineForm(): TemplateResult { + return html`

${msg( @@ -96,7 +95,6 @@ export class FlowImportForm extends Form { )}

- ${this.result ? this.renderResult() : html``} - `; + ${this.result ? this.renderResult() : html``}`; } } diff --git a/web/src/admin/groups/RelatedGroupList.ts b/web/src/admin/groups/RelatedGroupList.ts index bcb2c3dad..35d7124fc 100644 --- a/web/src/admin/groups/RelatedGroupList.ts +++ b/web/src/admin/groups/RelatedGroupList.ts @@ -45,41 +45,39 @@ export class RelatedGroupAdd extends Form<{ groups: string[] }> { return data; } - renderForm(): TemplateResult { - return html`
- -
- { - this.groupsToAdd = items; - this.requestUpdate(); - return Promise.resolve(); - }} - > - - -
- - ${this.groupsToAdd.map((group) => { - return html` { - const idx = this.groupsToAdd.indexOf(group); - this.groupsToAdd.splice(idx, 1); - this.requestUpdate(); - }} - > - ${group.name} - `; - })} - -
+ renderInlineForm(): TemplateResult { + return html` +
+ { + this.groupsToAdd = items; + this.requestUpdate(); + return Promise.resolve(); + }} + > + + +
+ + ${this.groupsToAdd.map((group) => { + return html` { + const idx = this.groupsToAdd.indexOf(group); + this.groupsToAdd.splice(idx, 1); + this.requestUpdate(); + }} + > + ${group.name} + `; + })} +
- - `; +
+ `; } } diff --git a/web/src/admin/policies/PolicyTestForm.ts b/web/src/admin/policies/PolicyTestForm.ts index ff15e8984..b8dd38712 100644 --- a/web/src/admin/policies/PolicyTestForm.ts +++ b/web/src/admin/policies/PolicyTestForm.ts @@ -115,9 +115,8 @@ export class PolicyTestForm extends Form { `; } - renderForm(): TemplateResult { - return html`
- + renderInlineForm(): TemplateResult { + return html` => { const args: CoreUsersListRequest = { @@ -154,7 +153,6 @@ export class PolicyTestForm extends Form { ${msg("Set custom attributes using YAML or JSON.")}

- ${this.result ? this.renderResult() : html``} - `; + ${this.result ? this.renderResult() : html``}`; } } diff --git a/web/src/admin/property-mappings/PropertyMappingTestForm.ts b/web/src/admin/property-mappings/PropertyMappingTestForm.ts index f1b2777ec..10617238f 100644 --- a/web/src/admin/property-mappings/PropertyMappingTestForm.ts +++ b/web/src/admin/property-mappings/PropertyMappingTestForm.ts @@ -118,9 +118,8 @@ export class PolicyTestForm extends Form { `; } - renderForm(): TemplateResult { - return html`
- + renderInlineForm(): TemplateResult { + return html` => { const args: CoreUsersListRequest = { @@ -155,7 +154,6 @@ export class PolicyTestForm extends Form {

${this.renderExampleButtons()}

 - ${this.result ? this.renderResult() : html``} - `; + ${this.result ? this.renderResult() : html``}`; } } diff --git a/web/src/admin/providers/saml/SAMLProviderImportForm.ts b/web/src/admin/providers/saml/SAMLProviderImportForm.ts index 1ea6bd512..d2730a708 100644 --- a/web/src/admin/providers/saml/SAMLProviderImportForm.ts +++ b/web/src/admin/providers/saml/SAMLProviderImportForm.ts @@ -36,9 +36,8 @@ export class SAMLProviderImportForm extends Form { }); } - renderForm(): TemplateResult { - return html`
- + renderInlineForm(): TemplateResult { + return html` { - - `; + `; } } diff --git a/web/src/admin/users/RelatedUserList.ts b/web/src/admin/users/RelatedUserList.ts index b58e87681..28f01c223 100644 --- a/web/src/admin/users/RelatedUserList.ts +++ b/web/src/admin/users/RelatedUserList.ts @@ -58,9 +58,8 @@ export class RelatedUserAdd extends Form<{ users: number[] }> { return data; } - renderForm(): TemplateResult { - return html`
- ${this.group?.isSuperuser ? html`` : html``} + renderInlineForm(): TemplateResult { + return html`${this.group?.isSuperuser ? html`` : html``}
{
-
- `; +
`; } } @@ -194,12 +192,20 @@ export class RelatedUserList extends Table { ${rootInterface()?.config?.capabilities.includes(CapabilitiesEnum.CanImpersonate) ? html` - { + return new CoreApi(DEFAULT_CONFIG) + .coreUsersImpersonateCreate({ + id: item.pk, + }) + .then(() => { + window.location.href = "/"; + }); + }} > ${msg("Impersonate")} - + ` : html``}`, ]; diff --git a/web/src/admin/users/ServiceAccountForm.ts b/web/src/admin/users/ServiceAccountForm.ts index 409353949..b5a0dc9b7 100644 --- a/web/src/admin/users/ServiceAccountForm.ts +++ b/web/src/admin/users/ServiceAccountForm.ts @@ -34,9 +34,12 @@ export class ServiceAccountForm extends Form { this.result = undefined; } - renderRequestForm(): TemplateResult { - return html`
- + renderInlineForm(): TemplateResult { + return html`

${msg("User's primary identifier. 150 characters or fewer.")} @@ -81,8 +84,7 @@ export class ServiceAccountForm extends Form { value="${dateTimeLocal(new Date(Date.now() + 1000 * 60 ** 2 * 24 * 360))}" class="pf-c-form-control" /> - - `; + `; } renderResponseForm(): TemplateResult { @@ -120,6 +122,6 @@ export class ServiceAccountForm extends Form { if (this.result) { return this.renderResponseForm(); } - return this.renderRequestForm(); + return super.renderForm(); } } diff --git a/web/src/admin/users/UserListPage.ts b/web/src/admin/users/UserListPage.ts index 924026649..4a8681a7b 100644 --- a/web/src/admin/users/UserListPage.ts +++ b/web/src/admin/users/UserListPage.ts @@ -197,12 +197,20 @@ export class UserListPage extends TablePage { ${rootInterface()?.config?.capabilities.includes(CapabilitiesEnum.CanImpersonate) ? html` - { + return new CoreApi(DEFAULT_CONFIG) + .coreUsersImpersonateCreate({ + id: item.pk, + }) + .then(() => { + window.location.href = "/"; + }); + }} > ${msg("Impersonate")} - + ` : html``}`, ]; diff --git a/web/src/admin/users/UserPasswordForm.ts b/web/src/admin/users/UserPasswordForm.ts index 4b9b95965..e96d2486f 100644 --- a/web/src/admin/users/UserPasswordForm.ts +++ b/web/src/admin/users/UserPasswordForm.ts @@ -25,11 +25,13 @@ export class UserPasswordForm extends Form { }); } - renderForm(): TemplateResult { - return html`

- - - -
`; + renderInlineForm(): TemplateResult { + return html` + + `; } } diff --git a/web/src/admin/users/UserResetEmailForm.ts b/web/src/admin/users/UserResetEmailForm.ts index ad528e1a1..b3e9550f8 100644 --- a/web/src/admin/users/UserResetEmailForm.ts +++ b/web/src/admin/users/UserResetEmailForm.ts @@ -31,36 +31,34 @@ export class UserResetEmailForm extends Form - + => { + const args: StagesAllListRequest = { + ordering: "name", + }; + if (query !== undefined) { + args.search = query; + } + const stages = await new StagesApi(DEFAULT_CONFIG).stagesEmailList(args); + return stages.results; + }} + .groupBy=${(items: Stage[]) => { + return groupBy(items, (stage) => stage.verboseNamePlural); + }} + .renderElement=${(stage: Stage): string => { + return stage.name; + }} + .value=${(stage: Stage | undefined): string | undefined => { + return stage?.pk; + }} > - => { - const args: StagesAllListRequest = { - ordering: "name", - }; - if (query !== undefined) { - args.search = query; - } - const stages = await new StagesApi(DEFAULT_CONFIG).stagesEmailList(args); - return stages.results; - }} - .groupBy=${(items: Stage[]) => { - return groupBy(items, (stage) => stage.verboseNamePlural); - }} - .renderElement=${(stage: Stage): string => { - return stage.name; - }} - .value=${(stage: Stage | undefined): string | undefined => { - return stage?.pk; - }} - > - - - `; + + `; } } diff --git a/web/src/admin/users/UserViewPage.ts b/web/src/admin/users/UserViewPage.ts index 3de730f79..60e62c0c8 100644 --- a/web/src/admin/users/UserViewPage.ts +++ b/web/src/admin/users/UserViewPage.ts @@ -204,12 +204,20 @@ export class UserViewPage extends AKElement { ) ? html`
- { + return new CoreApi(DEFAULT_CONFIG) + .coreUsersImpersonateCreate({ + id: this.user?.pk || 0, + }) + .then(() => { + window.location.href = "/"; + }); + }} > ${msg("Impersonate")} - +
` : html``} diff --git a/web/src/elements/Diagram.ts b/web/src/elements/Diagram.ts index 92aab2231..872f04f47 100644 --- a/web/src/elements/Diagram.ts +++ b/web/src/elements/Diagram.ts @@ -46,6 +46,7 @@ export class Diagram extends AKElement { flowchart: { curve: "linear", }, + htmlLabels: false, }; mermaid.initialize(this.config); } diff --git a/web/src/elements/forms/Form.ts b/web/src/elements/forms/Form.ts index 807f7312c..338147c90 100644 --- a/web/src/elements/forms/Form.ts +++ b/web/src/elements/forms/Form.ts @@ -283,9 +283,23 @@ export abstract class Form extends AKElement { } renderForm(): TemplateResult { + const inline = this.renderInlineForm(); + if (inline) { + return html`
+ ${inline} +
`; + } return html``; } + /** + * Inline form render callback when inheriting this class, should be overwritten + * instead of `this.renderForm` + */ + renderInlineForm(): TemplateResult | undefined { + return undefined; + } + renderNonFieldErrors(): TemplateResult { if (!this.nonFieldErrors) { return html``; diff --git a/web/src/user/UserInterface.ts b/web/src/user/UserInterface.ts index 12bf01705..6ad9bd285 100644 --- a/web/src/user/UserInterface.ts +++ b/web/src/user/UserInterface.ts @@ -11,6 +11,7 @@ import { me } from "@goauthentik/common/users"; import { first } from "@goauthentik/common/utils"; import { WebsocketClient } from "@goauthentik/common/ws"; import { Interface } from "@goauthentik/elements/Base"; +import "@goauthentik/elements/buttons/ActionButton"; import "@goauthentik/elements/messages/MessageContainer"; import "@goauthentik/elements/notifications/APIDrawer"; import "@goauthentik/elements/notifications/NotificationDrawer"; @@ -35,7 +36,7 @@ import PFPage from "@patternfly/patternfly/components/Page/page.css"; import PFBase from "@patternfly/patternfly/patternfly-base.css"; import PFDisplay from "@patternfly/patternfly/utilities/Display/display.css"; -import { EventsApi, SessionUser } from "@goauthentik/api"; +import { CoreApi, EventsApi, SessionUser } from "@goauthentik/api"; autoDetectLanguage(); @@ -233,18 +234,23 @@ export class UserInterface extends Interface { : html``} ${this.me.original - ? html`
-
- - ${msg("Stop impersonation")} - -
-
` + ? html`  +
+
+ { + return new CoreApi(DEFAULT_CONFIG) + .coreUsersImpersonateEndRetrieve() + .then(() => { + window.location.reload(); + }); + }} + > + ${msg("Stop impersonation")} + +
+
` : html``}
diff --git a/website/docs/releases/2023/v2023.4.md b/website/docs/releases/2023/v2023.4.md index a63a1cc4b..439867d01 100644 --- a/website/docs/releases/2023/v2023.4.md +++ b/website/docs/releases/2023/v2023.4.md @@ -107,6 +107,10 @@ image: - lifecycle: fix worker healthcheck (#5259) - lifecycle: re-add exec to ak wrapper (#5253) +## Fixed in 2023.4.2 + +- security: Address pen-test findings from the [2023-06 Cure53 Code audit](../../security/2023-06-cure53.md) + ## API Changes #### What's New diff --git a/website/docs/releases/2023/v2023.5.md b/website/docs/releases/2023/v2023.5.md index d3e5fc17c..ab73725e2 100644 --- a/website/docs/releases/2023/v2023.5.md +++ b/website/docs/releases/2023/v2023.5.md @@ -144,6 +144,10 @@ image: - providers/ldap: fix LDAP Outpost application selection (#5812) - web/flows: fix RedirectStage not detecting absolute URLs correctly (#5781) +## Fixed in 2023.5.4 + +- security: Address pen-test findings from the [2023-06 Cure53 Code audit](../../security/2023-06-cure53.md) + ## API Changes #### What's Changed diff --git a/website/docs/security/2023-06-cure53.md b/website/docs/security/2023-06-cure53.md new file mode 100644 index 000000000..3df339e81 --- /dev/null +++ b/website/docs/security/2023-06-cure53.md @@ -0,0 +1,67 @@ +# 2023-06 Cure53 Code audit + +In May/June of 2023, we've had a Pen-test conducted by [Cure53](https://cure53.de). The following security updates, 2023.4.2 and 2023.5.3 were released as a response to the found issues. + +From the complete report, these are the points we're addressing with this update: + +### ATH-01-001: Path traversal on blueprints allows arbitrary file-read (Medium) + +This had accidentally been patched by a previous commit already; and was also only possible for users with superuser permissions. + +### ATH-01-003: CSS injection via faulty string replacement in Mermaid (Low) + +This is an unrelated issue that was found with a third-party dependency ([Mermaid](https://mermaid.js.org/)), fixed with https://github.com/mermaid-js/mermaid/releases/tag/v10.2.2 + +Additionally we've also taken steps to further mitigate possible issues that could be caused in this way. + +### ATH-01-008: User-passwords disclosed to third-party service (High) + +In certain circumstances, using the Enter key to submit some forms instead of clicking submit would cause the frontend to change the URL instead of calling the API, which could lead to sensitive data being disclosed. + +### ATH-01-009: Lack of CSRF protection in impersonate feature (Low) + +Previous the URL to start an impersonation was a simple GET URL request, which was susceptible to CSRF. This has been changed to an API Post request. + +### ATH-01-010: Web authentication bypass via key confusion (High) + +When using WebAuthn to authenticate, the owner of the WebAuthn device wasn't checked. However to exploit this, an attacker would need to be able to already intercept HTTP traffic and read the data. + +### ATH-01-014: Authentication challenges abused by foreign flow (Medium) + +Previously it was possible to use an MFA authenticator class that wasn't allowed in a flow, if another flow existed that allowed this class. The patch changes data to be isolated per flow to prevent this issue. + +### ATH-01-004: Information disclosure on system endpoint (Info) + +The `/api/v3/admin/system/` (only accessible to superusers) endpoint returns a large amount of system info (mostly used for debugging), like the HTTP headers sent to the server. It also included all environment variables set for authentik. The environment variables have been removed. + +### ATH-01-005: Timing-unsafe comparison in API authentication (Info) + +In the API authentication that is used by the embedded outpost (API authentication via Secret key), a timing-unsafe comparison was used. + +### ATH-01-012: Unintended diagram created due to unescaped quotes (Info) + +Related to ATH-01-003, it was possible to insert unintended diagrams into generated diagrams. + +## Additional info + +In addition to the points above, several of the findings are classified as intended features (such as the expression policies), however these are points where we do also see room for improvement that we will address in the future. + +### ATH-01-002: Stored XSS in help text of prompt module (Medium) + +Prompt help texts can use HTML to add markup, which also includes the option to include JavaScript. This is only possible to configure for superusers, and in the future we're planning to add an additional toggle to limit this. + +### ATH-01-006: Arbitrary code execution via expressions (Critical) + +This is the intended function of expression policies/property mappings, which also requires superuser permissions to edit. We're planning to also add a toggle to limit the functions that can be executed to the ones provided by authentik, and prevent the importing of modules. + +### ATH-01-007: SSRF via blueprints feature for fetching manifests (Medium) + +Blueprints can be fetched via OCI registries, which could be potentially used for server-side request forgery. This can only be accessed by superusers, and we're planning to add an option to limit the resolved IP ranges this functionality can connect to. + +### ATH-01-013: XSS via CAPTCHA JavaScript URL (Medium) + +Similar to ATH-01-002, any arbitrary JavaScript can be loaded using the Captcha stage. This is also limited to superusers. + +### ATH-01-011: Weak default configs in logout/change password flows (Info) + +The default logout flow does not do any additional validation and logs the user out with a single GET request. The default password-change flow does not verify the users current password, nor does it show the current users info. diff --git a/website/sidebars.js b/website/sidebars.js index 9ad4c50d9..777718b49 100644 --- a/website/sidebars.js +++ b/website/sidebars.js @@ -321,6 +321,7 @@ module.exports = { }, items: [ "security/policy", + "security/2023-06-cure53", "security/CVE-2022-23555", "security/CVE-2022-46145", "security/CVE-2022-46172",