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:
parent
e2cf578afd
commit
8543e140ef
|
@ -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()
|
||||||
|
|
|
@ -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")
|
||||||
|
|
|
@ -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 || "-"}`,
|
||||||
|
];
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
Reference in a new issue