From 0cffe0c9539be7118777425c81de32684ba7544b Mon Sep 17 00:00:00 2001 From: Marc 'risson' Schmitt Date: Tue, 12 Nov 2024 14:42:53 +0100 Subject: [PATCH] core: add ability to provide reason for impersonation (#11951) * core: add ability to provide reason for impersonation Signed-off-by: Marc 'risson' Schmitt * tenants api things Signed-off-by: Marc 'risson' Schmitt * add missing implem Signed-off-by: Marc 'risson' Schmitt * A tooltip needs a DOM object to determine the coordinates where it should render. A solitary string is not enough; a is needed here. * web: user impersonation reason To determine where to render the Tooltip content, the object associated with the Tooltip must be a DOM object with an HTML tag. A naked string is not enough; a `` will do nicely here. Also, fixed a build failure: PFSize was not defined in RelatedUserList. * add and fix tests Signed-off-by: Marc 'risson' Schmitt * avoid migration change Signed-off-by: Marc 'risson' Schmitt * small fixes Signed-off-by: Marc 'risson' Schmitt --------- Signed-off-by: Marc 'risson' Schmitt Co-authored-by: Ken Sternberg --- authentik/core/api/users.py | 15 ++++++- authentik/core/tests/test_impersonation.py | 32 ++++++++++++--- authentik/events/models.py | 2 +- authentik/tenants/api/settings.py | 1 + ...004_tenant_impersonation_require_reason.py | 21 ++++++++++ authentik/tenants/models.py | 4 ++ authentik/tenants/utils.py | 6 ++- schema.yml | 26 ++++++++++++ .../admin/admin-settings/AdminSettingsForm.ts | 7 ++++ web/src/admin/groups/RelatedUserList.ts | 32 ++++++++------- web/src/admin/users/UserImpersonateForm.ts | 40 +++++++++++++++++++ web/src/admin/users/UserListPage.ts | 31 +++++++------- web/src/admin/users/UserViewPage.ts | 37 ++++++++--------- 13 files changed, 195 insertions(+), 59 deletions(-) create mode 100644 authentik/tenants/migrations/0004_tenant_impersonation_require_reason.py create mode 100644 web/src/admin/users/UserImpersonateForm.ts diff --git a/authentik/core/api/users.py b/authentik/core/api/users.py index 37656933d5..7280a05881 100644 --- a/authentik/core/api/users.py +++ b/authentik/core/api/users.py @@ -666,7 +666,12 @@ class UserViewSet(UsedByMixin, ModelViewSet): @permission_required("authentik_core.impersonate") @extend_schema( - request=OpenApiTypes.NONE, + request=inline_serializer( + "ImpersonationSerializer", + { + "reason": CharField(required=True), + }, + ), responses={ "204": OpenApiResponse(description="Successfully started impersonation"), "401": OpenApiResponse(description="Access denied"), @@ -679,6 +684,7 @@ class UserViewSet(UsedByMixin, ModelViewSet): LOGGER.debug("User attempted to impersonate", user=request.user) return Response(status=401) user_to_be = self.get_object() + reason = request.data.get("reason", "") # Check both object-level perms and global perms if not request.user.has_perm( "authentik_core.impersonate", user_to_be @@ -688,11 +694,16 @@ class UserViewSet(UsedByMixin, ModelViewSet): if user_to_be.pk == self.request.user.pk: LOGGER.debug("User attempted to impersonate themselves", user=request.user) return Response(status=401) + if not reason and request.tenant.impersonation_require_reason: + LOGGER.debug( + "User attempted to impersonate without providing a reason", user=request.user + ) + return Response(status=401) 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) + Event.new(EventAction.IMPERSONATION_STARTED, reason=reason).from_http(request, user_to_be) return Response(status=201) diff --git a/authentik/core/tests/test_impersonation.py b/authentik/core/tests/test_impersonation.py index d877e55c9e..7333971fab 100644 --- a/authentik/core/tests/test_impersonation.py +++ b/authentik/core/tests/test_impersonation.py @@ -29,7 +29,8 @@ class TestImpersonation(APITestCase): reverse( "authentik_api:user-impersonate", kwargs={"pk": self.other_user.pk}, - ) + ), + data={"reason": "some reason"}, ) response = self.client.get(reverse("authentik_api:user-me")) @@ -55,7 +56,8 @@ class TestImpersonation(APITestCase): reverse( "authentik_api:user-impersonate", kwargs={"pk": self.other_user.pk}, - ) + ), + data={"reason": "some reason"}, ) self.assertEqual(response.status_code, 201) @@ -75,7 +77,8 @@ class TestImpersonation(APITestCase): reverse( "authentik_api:user-impersonate", kwargs={"pk": self.other_user.pk}, - ) + ), + data={"reason": "some reason"}, ) self.assertEqual(response.status_code, 201) @@ -89,7 +92,8 @@ class TestImpersonation(APITestCase): self.client.force_login(self.other_user) response = self.client.post( - reverse("authentik_api:user-impersonate", kwargs={"pk": self.user.pk}) + reverse("authentik_api:user-impersonate", kwargs={"pk": self.user.pk}), + data={"reason": "some reason"}, ) self.assertEqual(response.status_code, 403) @@ -105,7 +109,8 @@ class TestImpersonation(APITestCase): self.client.force_login(self.user) response = self.client.post( - reverse("authentik_api:user-impersonate", kwargs={"pk": self.other_user.pk}) + reverse("authentik_api:user-impersonate", kwargs={"pk": self.other_user.pk}), + data={"reason": "some reason"}, ) self.assertEqual(response.status_code, 401) @@ -118,7 +123,22 @@ class TestImpersonation(APITestCase): self.client.force_login(self.user) response = self.client.post( - reverse("authentik_api:user-impersonate", kwargs={"pk": self.user.pk}) + reverse("authentik_api:user-impersonate", kwargs={"pk": self.user.pk}), + data={"reason": "some reason"}, + ) + self.assertEqual(response.status_code, 401) + + response = self.client.get(reverse("authentik_api:user-me")) + response_body = loads(response.content.decode()) + self.assertEqual(response_body["user"]["username"], self.user.username) + + def test_impersonate_reason_required(self): + """test impersonation that user must provide reason""" + self.client.force_login(self.user) + + response = self.client.post( + reverse("authentik_api:user-impersonate", kwargs={"pk": self.user.pk}), + data={"reason": ""}, ) self.assertEqual(response.status_code, 401) diff --git a/authentik/events/models.py b/authentik/events/models.py index 2f6a5b9d7e..1a21462b2d 100644 --- a/authentik/events/models.py +++ b/authentik/events/models.py @@ -60,7 +60,7 @@ def default_event_duration(): """Default duration an Event is saved. This is used as a fallback when no brand is available""" try: - tenant = get_current_tenant() + tenant = get_current_tenant(only=["event_retention"]) return now() + timedelta_from_string(tenant.event_retention) except Tenant.DoesNotExist: return now() + timedelta(days=365) diff --git a/authentik/tenants/api/settings.py b/authentik/tenants/api/settings.py index ad98165c05..569995699d 100644 --- a/authentik/tenants/api/settings.py +++ b/authentik/tenants/api/settings.py @@ -23,6 +23,7 @@ class SettingsSerializer(ModelSerializer): "footer_links", "gdpr_compliance", "impersonation", + "impersonation_require_reason", "default_token_duration", "default_token_length", ] diff --git a/authentik/tenants/migrations/0004_tenant_impersonation_require_reason.py b/authentik/tenants/migrations/0004_tenant_impersonation_require_reason.py new file mode 100644 index 0000000000..ebd13e73c4 --- /dev/null +++ b/authentik/tenants/migrations/0004_tenant_impersonation_require_reason.py @@ -0,0 +1,21 @@ +# Generated by Django 5.0.9 on 2024-11-07 15:08 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("authentik_tenants", "0003_alter_tenant_default_token_duration"), + ] + + operations = [ + migrations.AddField( + model_name="tenant", + name="impersonation_require_reason", + field=models.BooleanField( + default=True, + help_text="Require administrators to provide a reason for impersonating a user.", + ), + ), + ] diff --git a/authentik/tenants/models.py b/authentik/tenants/models.py index df44159c37..654009055b 100644 --- a/authentik/tenants/models.py +++ b/authentik/tenants/models.py @@ -85,6 +85,10 @@ class Tenant(TenantMixin, SerializerModel): impersonation = models.BooleanField( help_text=_("Globally enable/disable impersonation."), default=True ) + impersonation_require_reason = models.BooleanField( + help_text=_("Require administrators to provide a reason for impersonating a user."), + default=True, + ) default_token_duration = models.TextField( help_text=_("Default token duration"), default=DEFAULT_TOKEN_DURATION, diff --git a/authentik/tenants/utils.py b/authentik/tenants/utils.py index 7b5e000a8c..7204f39706 100644 --- a/authentik/tenants/utils.py +++ b/authentik/tenants/utils.py @@ -8,9 +8,11 @@ from authentik.root.install_id import get_install_id from authentik.tenants.models import Tenant -def get_current_tenant() -> Tenant: +def get_current_tenant(only: list[str] | None = None) -> Tenant: """Get tenant for current request""" - return Tenant.objects.get(schema_name=connection.schema_name) + if only is None: + only = [] + return Tenant.objects.only(*only).get(schema_name=connection.schema_name) def get_unique_identifier() -> str: diff --git a/schema.yml b/schema.yml index 6ba28e7c2b..a46794370b 100644 --- a/schema.yml +++ b/schema.yml @@ -5295,6 +5295,12 @@ paths: required: true tags: - core + requestBody: + content: + application/json: + schema: + $ref: '#/components/schemas/ImpersonationRequest' + required: true security: - authentik: [] responses: @@ -42720,6 +42726,14 @@ components: incorrect user info is entered. required: - name + ImpersonationRequest: + type: object + properties: + reason: + type: string + minLength: 1 + required: + - reason InstallID: type: object properties: @@ -50029,6 +50043,10 @@ components: impersonation: type: boolean description: Globally enable/disable impersonation. + impersonation_require_reason: + type: boolean + description: Require administrators to provide a reason for impersonating + a user. default_token_duration: type: string minLength: 1 @@ -53758,6 +53776,10 @@ components: impersonation: type: boolean description: Globally enable/disable impersonation. + impersonation_require_reason: + type: boolean + description: Require administrators to provide a reason for impersonating + a user. default_token_duration: type: string description: Default token duration @@ -53797,6 +53819,10 @@ components: impersonation: type: boolean description: Globally enable/disable impersonation. + impersonation_require_reason: + type: boolean + description: Require administrators to provide a reason for impersonating + a user. default_token_duration: type: string minLength: 1 diff --git a/web/src/admin/admin-settings/AdminSettingsForm.ts b/web/src/admin/admin-settings/AdminSettingsForm.ts index 4689e092dc..92c576d8d6 100644 --- a/web/src/admin/admin-settings/AdminSettingsForm.ts +++ b/web/src/admin/admin-settings/AdminSettingsForm.ts @@ -193,6 +193,13 @@ export class AdminSettingsForm extends Form { help=${msg("Globally enable/disable impersonation.")} > + + ${canImpersonate ? html` - { - return new CoreApi(DEFAULT_CONFIG) - .coreUsersImpersonateCreate({ - id: item.pk, - }) - .then(() => { - window.location.href = "/"; - }); - }} - > - ${msg("Impersonate")} - + + ${msg("Impersonate")} + ${msg("Impersonate")} ${item.username} + + + ` : html``}`, ]; diff --git a/web/src/admin/users/UserImpersonateForm.ts b/web/src/admin/users/UserImpersonateForm.ts new file mode 100644 index 0000000000..756f1ae7ab --- /dev/null +++ b/web/src/admin/users/UserImpersonateForm.ts @@ -0,0 +1,40 @@ +import { DEFAULT_CONFIG } from "@goauthentik/common/api/config"; +import "@goauthentik/components/ak-text-input"; +import { Form } from "@goauthentik/elements/forms/Form"; + +import { msg } from "@lit/localize"; +import { TemplateResult, html } from "lit"; +import { customElement, property } from "lit/decorators.js"; + +import { CoreApi, ImpersonationRequest } from "@goauthentik/api"; + +@customElement("ak-user-impersonate-form") +export class UserImpersonateForm extends Form { + @property({ type: Number }) + instancePk?: number; + + async send(data: ImpersonationRequest): Promise { + return new CoreApi(DEFAULT_CONFIG) + .coreUsersImpersonateCreate({ + id: this.instancePk || 0, + impersonationRequest: data, + }) + .then(() => { + window.location.href = "/"; + }); + } + + renderForm(): TemplateResult { + return html``; + } +} + +declare global { + interface HTMLElementTagNameMap { + "ak-user-impersonate-form": UserImpersonateForm; + } +} diff --git a/web/src/admin/users/UserListPage.ts b/web/src/admin/users/UserListPage.ts index 1264c00814..42f3cf64c4 100644 --- a/web/src/admin/users/UserListPage.ts +++ b/web/src/admin/users/UserListPage.ts @@ -2,6 +2,7 @@ import { AdminInterface } from "@goauthentik/admin/AdminInterface"; import "@goauthentik/admin/users/ServiceAccountForm"; import "@goauthentik/admin/users/UserActiveForm"; import "@goauthentik/admin/users/UserForm"; +import "@goauthentik/admin/users/UserImpersonateForm"; import "@goauthentik/admin/users/UserPasswordForm"; import "@goauthentik/admin/users/UserResetEmailForm"; import { DEFAULT_CONFIG } from "@goauthentik/common/api/config"; @@ -266,20 +267,22 @@ export class UserListPage extends WithBrandConfig(WithCapabilitiesConfig(TablePa ${canImpersonate ? html` - { - return new CoreApi(DEFAULT_CONFIG) - .coreUsersImpersonateCreate({ - id: item.pk, - }) - .then(() => { - window.location.href = "/"; - }); - }} - > - ${msg("Impersonate")} - + + ${msg("Impersonate")} + ${msg("Impersonate")} ${item.username} + + + ` : html``}`, ]; diff --git a/web/src/admin/users/UserViewPage.ts b/web/src/admin/users/UserViewPage.ts index 119ffdb371..80f0ceed17 100644 --- a/web/src/admin/users/UserViewPage.ts +++ b/web/src/admin/users/UserViewPage.ts @@ -5,6 +5,7 @@ import "@goauthentik/admin/users/UserActiveForm"; import "@goauthentik/admin/users/UserApplicationTable"; import "@goauthentik/admin/users/UserChart"; import "@goauthentik/admin/users/UserForm"; +import "@goauthentik/admin/users/UserImpersonateForm"; import { renderRecoveryEmailRequest, requestRecoveryLink, @@ -208,26 +209,22 @@ export class UserViewPage extends WithCapabilitiesConfig(AKElement) { ${canImpersonate ? html` - { - return new CoreApi(DEFAULT_CONFIG) - .coreUsersImpersonateCreate({ - id: user.pk, - }) - .then(() => { - window.location.href = "/"; - }); - }} - > - - ${msg("Impersonate")} - - + + ${msg("Impersonate")} + ${msg("Impersonate")} ${user.username} + + + ` : nothing} `;