From 8543e140ef90c85c142a38fe5835241bbad29a8f Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Mon, 1 Aug 2022 20:06:09 +0200 Subject: [PATCH] stages/consent: fix error when requests with identical empty permissions closes #3280 Signed-off-by: Jens Langhammer --- authentik/stages/consent/stage.py | 30 +++++------ authentik/stages/consent/tests.py | 67 ++++++++++++++++++++++++ web/src/elements/user/UserConsentList.ts | 7 ++- 3 files changed, 86 insertions(+), 18 deletions(-) diff --git a/authentik/stages/consent/stage.py b/authentik/stages/consent/stage.py index a95f8c55d..e91e7c8bf 100644 --- a/authentik/stages/consent/stage.py +++ b/authentik/stages/consent/stage.py @@ -21,7 +21,7 @@ from authentik.stages.consent.models import ConsentMode, ConsentStage, UserConse PLAN_CONTEXT_CONSENT_TITLE = "consent_title" PLAN_CONTEXT_CONSENT_HEADER = "consent_header" PLAN_CONTEXT_CONSENT_PERMISSIONS = "consent_permissions" -PLAN_CONTEXT_CONSNET_EXTRA_PERMISSIONS = "consent_additional_permissions" +PLAN_CONTEXT_CONSENT_EXTRA_PERMISSIONS = "consent_additional_permissions" SESSION_KEY_CONSENT_TOKEN = "authentik/stages/consent/token" # nosec @@ -54,7 +54,7 @@ class ConsentStageView(ChallengeStageView): "type": ChallengeTypes.NATIVE.value, "permissions": self.executor.plan.context.get(PLAN_CONTEXT_CONSENT_PERMISSIONS, []), "additional_permissions": self.executor.plan.context.get( - PLAN_CONTEXT_CONSNET_EXTRA_PERMISSIONS, [] + PLAN_CONTEXT_CONSENT_EXTRA_PERMISSIONS, [] ), "token": token, } @@ -92,14 +92,14 @@ class ConsentStageView(ChallengeStageView): if consent: perms = self.executor.plan.context.get(PLAN_CONTEXT_CONSENT_PERMISSIONS, []) - allowed_perms = set(consent.permissions.split(" ")) + allowed_perms = set(consent.permissions.split(" ") if consent.permissions != "" else []) requested_perms = set(x["id"] for x in perms) if allowed_perms != requested_perms: self.executor.plan.context[PLAN_CONTEXT_CONSENT_PERMISSIONS] = [ x for x in perms if x["id"] in allowed_perms ] - self.executor.plan.context[PLAN_CONTEXT_CONSNET_EXTRA_PERMISSIONS] = [ + self.executor.plan.context[PLAN_CONTEXT_CONSENT_EXTRA_PERMISSIONS] = [ x for x in perms if x["id"] in requested_perms.difference(allowed_perms) ] return super().get(request, *args, **kwargs) @@ -117,7 +117,7 @@ class ConsentStageView(ChallengeStageView): application = self.executor.plan.context[PLAN_CONTEXT_APPLICATION] permissions = self.executor.plan.context.get( PLAN_CONTEXT_CONSENT_PERMISSIONS, [] - ) + self.executor.plan.context.get(PLAN_CONTEXT_CONSNET_EXTRA_PERMISSIONS, []) + ) + self.executor.plan.context.get(PLAN_CONTEXT_CONSENT_EXTRA_PERMISSIONS, []) permissions_string = " ".join(x["id"] for x in permissions) # Make this StageView work when injected, in which case `current_stage` is an instance # of the base class, and we don't save any consent, as it is assumed to be a one-time @@ -125,18 +125,14 @@ class ConsentStageView(ChallengeStageView): if not isinstance(current_stage, ConsentStage): return self.executor.stage_ok() # Since we only get here when no consent exists, we can create it without update + consent = UserConsent( + user=self.request.user, + application=application, + permissions=permissions_string, + ) if current_stage.mode == ConsentMode.PERMANENT: - UserConsent.objects.create( - user=self.request.user, - application=application, - expiring=False, - permissions=permissions_string, - ) + consent.expiring = False if current_stage.mode == ConsentMode.EXPIRING: - UserConsent.objects.create( - user=self.request.user, - application=application, - expires=now() + timedelta_from_string(current_stage.consent_expire_in), - permissions=permissions_string, - ) + consent.expires = now() + timedelta_from_string(current_stage.consent_expire_in) + consent.save() return self.executor.stage_ok() diff --git a/authentik/stages/consent/tests.py b/authentik/stages/consent/tests.py index 1645ee2cd..f15d2372c 100644 --- a/authentik/stages/consent/tests.py +++ b/authentik/stages/consent/tests.py @@ -225,3 +225,70 @@ class TestConsentStage(FlowTestCase): user=self.user, application=self.application, permissions="foo bar" ).exists() ) + + def test_permanent_same(self): + """Test permanent consent from user""" + self.client.force_login(self.user) + flow = create_test_flow(FlowDesignation.AUTHENTICATION) + stage = ConsentStage.objects.create(name=generate_id(), mode=ConsentMode.PERMANENT) + binding = FlowStageBinding.objects.create(target=flow, stage=stage, order=2) + + plan = FlowPlan( + flow_pk=flow.pk.hex, + bindings=[binding], + markers=[StageMarker()], + context={ + PLAN_CONTEXT_APPLICATION: self.application, + }, + ) + session = self.client.session + session[SESSION_KEY_PLAN] = plan + session.save() + + # First, consent with a single permission + response = self.client.get( + reverse("authentik_api:flow-executor", kwargs={"flow_slug": flow.slug}), + {}, + ) + self.assertEqual(response.status_code, 200) + raw_res = self.assertStageResponse( + response, + flow, + self.user, + permissions=[], + additional_permissions=[], + ) + response = self.client.post( + reverse("authentik_api:flow-executor", kwargs={"flow_slug": flow.slug}), + { + "token": raw_res["token"], + }, + ) + self.assertEqual(response.status_code, 200) + self.assertStageRedirects(response, reverse("authentik_core:root-redirect")) + self.assertTrue( + UserConsent.objects.filter( + user=self.user, application=self.application, permissions="" + ).exists() + ) + + # Request again with the same perms + plan = FlowPlan( + flow_pk=flow.pk.hex, + bindings=[binding], + markers=[StageMarker()], + context={ + PLAN_CONTEXT_APPLICATION: self.application, + PLAN_CONTEXT_CONSENT_PERMISSIONS: [], + }, + ) + session = self.client.session + session[SESSION_KEY_PLAN] = plan + session.save() + + response = self.client.get( + reverse("authentik_api:flow-executor", kwargs={"flow_slug": flow.slug}), + {}, + ) + self.assertEqual(response.status_code, 200) + self.assertStageResponse(response, component="xak-flow-redirect") diff --git a/web/src/elements/user/UserConsentList.ts b/web/src/elements/user/UserConsentList.ts index 7accd8da1..87ffcdac9 100644 --- a/web/src/elements/user/UserConsentList.ts +++ b/web/src/elements/user/UserConsentList.ts @@ -32,6 +32,7 @@ export class UserConsentList extends Table { return [ new TableColumn(t`Application`, "application"), new TableColumn(t`Expires`, "expires"), + new TableColumn(t`Permissions`, "permissions"), ]; } @@ -58,6 +59,10 @@ export class UserConsentList extends Table { } row(item: UserConsent): TemplateResult[] { - return [html`${item.application.name}`, html`${item.expires?.toLocaleString()}`]; + return [ + html`${item.application.name}`, + html`${item.expires?.toLocaleString()}`, + html`${item.permissions || "-"}`, + ]; } }