stages/consent: fix error when requests with identical empty permissions

closes #3280

Signed-off-by: Jens Langhammer <jens.langhammer@beryju.org>
This commit is contained in:
Jens Langhammer 2022-08-01 20:06:09 +02:00
parent 6ab85f7f18
commit 7a05c6faef
3 changed files with 86 additions and 18 deletions

View file

@ -21,7 +21,7 @@ from authentik.stages.consent.models import ConsentMode, ConsentStage, UserConse
PLAN_CONTEXT_CONSENT_TITLE = "consent_title" PLAN_CONTEXT_CONSENT_TITLE = "consent_title"
PLAN_CONTEXT_CONSENT_HEADER = "consent_header" PLAN_CONTEXT_CONSENT_HEADER = "consent_header"
PLAN_CONTEXT_CONSENT_PERMISSIONS = "consent_permissions" 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 SESSION_KEY_CONSENT_TOKEN = "authentik/stages/consent/token" # nosec
@ -54,7 +54,7 @@ class ConsentStageView(ChallengeStageView):
"type": ChallengeTypes.NATIVE.value, "type": ChallengeTypes.NATIVE.value,
"permissions": self.executor.plan.context.get(PLAN_CONTEXT_CONSENT_PERMISSIONS, []), "permissions": self.executor.plan.context.get(PLAN_CONTEXT_CONSENT_PERMISSIONS, []),
"additional_permissions": self.executor.plan.context.get( "additional_permissions": self.executor.plan.context.get(
PLAN_CONTEXT_CONSNET_EXTRA_PERMISSIONS, [] PLAN_CONTEXT_CONSENT_EXTRA_PERMISSIONS, []
), ),
"token": token, "token": token,
} }
@ -92,14 +92,14 @@ class ConsentStageView(ChallengeStageView):
if consent: if consent:
perms = self.executor.plan.context.get(PLAN_CONTEXT_CONSENT_PERMISSIONS, []) 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) requested_perms = set(x["id"] for x in perms)
if allowed_perms != requested_perms: if allowed_perms != requested_perms:
self.executor.plan.context[PLAN_CONTEXT_CONSENT_PERMISSIONS] = [ self.executor.plan.context[PLAN_CONTEXT_CONSENT_PERMISSIONS] = [
x for x in perms if x["id"] in allowed_perms 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) x for x in perms if x["id"] in requested_perms.difference(allowed_perms)
] ]
return super().get(request, *args, **kwargs) return super().get(request, *args, **kwargs)
@ -117,7 +117,7 @@ class ConsentStageView(ChallengeStageView):
application = self.executor.plan.context[PLAN_CONTEXT_APPLICATION] application = self.executor.plan.context[PLAN_CONTEXT_APPLICATION]
permissions = self.executor.plan.context.get( permissions = self.executor.plan.context.get(
PLAN_CONTEXT_CONSENT_PERMISSIONS, [] 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) permissions_string = " ".join(x["id"] for x in permissions)
# Make this StageView work when injected, in which case `current_stage` is an instance # 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 # 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): if not isinstance(current_stage, ConsentStage):
return self.executor.stage_ok() return self.executor.stage_ok()
# Since we only get here when no consent exists, we can create it without update # 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: if current_stage.mode == ConsentMode.PERMANENT:
UserConsent.objects.create( consent.expiring = False
user=self.request.user,
application=application,
expiring=False,
permissions=permissions_string,
)
if current_stage.mode == ConsentMode.EXPIRING: if current_stage.mode == ConsentMode.EXPIRING:
UserConsent.objects.create( consent.expires = now() + timedelta_from_string(current_stage.consent_expire_in)
user=self.request.user, consent.save()
application=application,
expires=now() + timedelta_from_string(current_stage.consent_expire_in),
permissions=permissions_string,
)
return self.executor.stage_ok() return self.executor.stage_ok()

View file

@ -225,3 +225,70 @@ class TestConsentStage(FlowTestCase):
user=self.user, application=self.application, permissions="foo bar" user=self.user, application=self.application, permissions="foo bar"
).exists() ).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")

View file

@ -32,6 +32,7 @@ export class UserConsentList extends Table<UserConsent> {
return [ return [
new TableColumn(t`Application`, "application"), new TableColumn(t`Application`, "application"),
new TableColumn(t`Expires`, "expires"), new TableColumn(t`Expires`, "expires"),
new TableColumn(t`Permissions`, "permissions"),
]; ];
} }
@ -58,6 +59,10 @@ export class UserConsentList extends Table<UserConsent> {
} }
row(item: UserConsent): TemplateResult[] { 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 || "-"}`,
];
} }
} }