save consented permissions in user consent, re-prompt when new permis…

…sions are required

Signed-off-by: Jens Langhammer <jens.langhammer@beryju.org>

authentik/flows/challenge.py

@@ -1,6 +1,6 @@
 """Challenge helpers"""
 from enum import Enum
-from typing import TYPE_CHECKING, Optional
+from typing import TYPE_CHECKING, Optional, TypedDict
 
 from django.db import models
 from django.http import JsonResponse
@@ -95,6 +95,13 @@ class AccessDeniedChallenge(WithUserInfoChallenge):
     component = CharField(default="ak-stage-access-denied")
 
 
+class PermissionDict(TypedDict):
+    """Consent Permission"""
+
+    id: str
+    name: str
+
+
 class PermissionSerializer(PassiveSerializer):
     """Permission used for consent"""
 

authentik/providers/oauth2/views/userinfo.py

@@ -9,6 +9,7 @@
 
 from authentik.core.exceptions import PropertyMappingExpressionException
 from authentik.events.models import Event, EventAction
+from authentik.flows.challenge import PermissionDict
 from authentik.providers.oauth2.constants import (
     SCOPE_AUTHENTIK_API,
     SCOPE_GITHUB_ORG_READ,
@@ -28,12 +29,13 @@ class UserInfoView(View):
 
     token: Optional[RefreshToken]
 
-    def get_scope_descriptions(self, scopes: list[str]) -> list[dict[str, str]]:
+    def get_scope_descriptions(self, scopes: list[str]) -> list[PermissionDict]:
         """Get a list of all Scopes's descriptions"""
         scope_descriptions = []
         for scope in ScopeMapping.objects.filter(scope_name__in=scopes).order_by("scope_name"):
-            if scope.description != "":
-                scope_descriptions.append({"id": scope.scope_name, "name": scope.description})
+            if scope.description == "":
+                continue
+            scope_descriptions.append(PermissionDict(id=scope.scope_name, name=scope.description))
         # GitHub Compatibility Scopes are handled differently, since they required custom paths
         # Hence they don't exist as Scope objects
         special_scope_map = {
@@ -45,7 +47,7 @@ def get_scope_descriptions(self, scopes: list[str]) -> list[dict[str, str]]:
         }
         for scope in scopes:
             if scope in special_scope_map:
-                scope_descriptions.append({"id": scope, "name": special_scope_map[scope]})
+                scope_descriptions.append(PermissionDict(id=scope, name=special_scope_map[scope]))
         return scope_descriptions
 
     def get_claims(self, token: RefreshToken) -> dict[str, Any]:

authentik/stages/consent/api.py

@@ -40,7 +40,7 @@ class UserConsentSerializer(StageSerializer):
     class Meta:
 
         model = UserConsent
-        fields = ["pk", "expires", "user", "application"]
+        fields = ["pk", "expires", "user", "application", "permissions"]
 
 
 class UserConsentViewSet(

authentik/stages/consent/migrations/0004_alter_userconsent_unique_together_and_more.py

@@ -0,0 +1,29 @@
+# Generated by Django 4.0.5 on 2022-06-26 10:42
+
+from django.conf import settings
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("authentik_core", "0021_source_user_path_user_path"),
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+        ("authentik_stages_consent", "0003_auto_20200924_1403"),
+    ]
+
+    operations = [
+        migrations.AlterUniqueTogether(
+            name="userconsent",
+            unique_together=set(),
+        ),
+        migrations.AddField(
+            model_name="userconsent",
+            name="permissions",
+            field=models.TextField(default=""),
+        ),
+        migrations.AlterUniqueTogether(
+            name="userconsent",
+            unique_together={("user", "application", "permissions")},
+        ),
+    ]

authentik/stages/consent/models.py

@@ -56,12 +56,13 @@ class UserConsent(ExpiringModel):
 
     user = models.ForeignKey(User, on_delete=models.CASCADE)
     application = models.ForeignKey(Application, on_delete=models.CASCADE)
+    permissions = models.TextField(default="")
 
     def __str__(self):
         return f"User Consent {self.application} by {self.user}"
 
     class Meta:
 
-        unique_together = (("user", "application"),)
+        unique_together = (("user", "application", "permissions"),)
         verbose_name = _("User Consent")
         verbose_name_plural = _("User Consents")

authentik/stages/consent/stage.py

@@ -1,4 +1,6 @@
 """authentik consent stage"""
+from typing import Optional, TypedDict
+
 from django.http import HttpRequest, HttpResponse
 from django.utils.timezone import now
 from rest_framework.fields import CharField
@@ -18,13 +20,15 @@
 PLAN_CONTEXT_CONSENT_TITLE = "consent_title"
 PLAN_CONTEXT_CONSENT_HEADER = "consent_header"
 PLAN_CONTEXT_CONSENT_PERMISSIONS = "consent_permissions"
+PLAN_CONTEXT_CONSNET_ADDITIONAL_PERMISSIONS = "consent_additional_permissions"
 
 
 class ConsentChallenge(WithUserInfoChallenge):
     """Challenge info for consent screens"""
 
-    header_text = CharField()
+    header_text = CharField(required=False)
     permissions = PermissionSerializer(many=True)
+    additional_permissions = PermissionSerializer(many=True)
     component = CharField(default="ak-stage-consent")
 
 
@@ -43,6 +47,9 @@ def get_challenge(self) -> Challenge:
         data = {
             "type": ChallengeTypes.NATIVE.value,
             "permissions": self.executor.plan.context.get(PLAN_CONTEXT_CONSENT_PERMISSIONS, []),
+            "additional_permissions": self.executor.plan.context.get(
+                PLAN_CONTEXT_CONSNET_ADDITIONAL_PERMISSIONS, []
+            ),
         }
         if PLAN_CONTEXT_CONSENT_TITLE in self.executor.plan.context:
             data["title"] = self.executor.plan.context[PLAN_CONTEXT_CONSENT_TITLE]
@@ -72,17 +79,37 @@ def get(self, request: HttpRequest, *args, **kwargs) -> HttpResponse:
         if PLAN_CONTEXT_PENDING_USER in self.executor.plan.context:
             user = self.executor.plan.context[PLAN_CONTEXT_PENDING_USER]
 
-        if UserConsent.filter_not_expired(user=user, application=application).exists():
+        consent: Optional[UserConsent] = UserConsent.filter_not_expired(
+            user=user, application=application
+        ).first()
+
+        if consent:
+            perms = self.executor.plan.context.get(PLAN_CONTEXT_CONSENT_PERMISSIONS, [])
+            allowed_perms = set(consent.permissions.split(" "))
+            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_ADDITIONAL_PERMISSIONS] = [
+                    x for x in perms if x["id"] in requested_perms.difference(allowed_perms)
+                ]
+                return super().get(request, *args, **kwargs)
             return self.executor.stage_ok()
 
-        # No consent found, return consent
+        # No consent found, return consent prompt
         return super().get(request, *args, **kwargs)
 
     def challenge_valid(self, response: ChallengeResponse) -> HttpResponse:
         current_stage: ConsentStage = self.executor.current_stage
         if PLAN_CONTEXT_APPLICATION not in self.executor.plan.context:
             return self.executor.stage_ok()
         application = self.executor.plan.context[PLAN_CONTEXT_APPLICATION]
+        permissions = self.executor.plan.context[
+            PLAN_CONTEXT_CONSENT_PERMISSIONS
+        ] + self.executor.plan.context.get(PLAN_CONTEXT_CONSNET_ADDITIONAL_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
         # prompt
@@ -91,12 +118,16 @@ def challenge_valid(self, response: ChallengeResponse) -> HttpResponse:
         # Since we only get here when no consent exists, we can create it without update
         if current_stage.mode == ConsentMode.PERMANENT:
             UserConsent.objects.create(
-                user=self.request.user, application=application, expiring=False
+                user=self.request.user,
+                application=application,
+                expiring=False,
+                permissions=permissions_string,
             )
         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,
             )
         return self.executor.stage_ok()

authentik/stages/consent/tests.py

@@ -6,12 +6,15 @@
 from authentik.core.models import Application
 from authentik.core.tasks import clean_expired_models
 from authentik.core.tests.utils import create_test_admin_user, create_test_flow
+from authentik.flows.challenge import PermissionDict
 from authentik.flows.markers import StageMarker
 from authentik.flows.models import FlowDesignation, FlowStageBinding
 from authentik.flows.planner import PLAN_CONTEXT_APPLICATION, FlowPlan
 from authentik.flows.tests import FlowTestCase
 from authentik.flows.views.executor import SESSION_KEY_PLAN
+from authentik.lib.generators import generate_id
 from authentik.stages.consent.models import ConsentMode, ConsentStage, UserConsent
+from authentik.stages.consent.stage import PLAN_CONTEXT_CONSENT_PERMISSIONS
 
 
 class TestConsentStage(FlowTestCase):
@@ -21,14 +24,14 @@ def setUp(self):
         super().setUp()
         self.user = create_test_admin_user()
         self.application = Application.objects.create(
-            name="test-application",
-            slug="test-application",
+            name=generate_id(),
+            slug=generate_id(),
         )
 
     def test_always_required(self):
         """Test always required consent"""
         flow = create_test_flow(FlowDesignation.AUTHENTICATION)
-        stage = ConsentStage.objects.create(name="consent", mode=ConsentMode.ALWAYS_REQUIRE)
+        stage = ConsentStage.objects.create(name=generate_id(), mode=ConsentMode.ALWAYS_REQUIRE)
         binding = FlowStageBinding.objects.create(target=flow, stage=stage, order=2)
 
         plan = FlowPlan(flow_pk=flow.pk.hex, bindings=[binding], markers=[StageMarker()])
@@ -48,7 +51,7 @@ def test_permanent(self):
         """Test permanent consent from user"""
         self.client.force_login(self.user)
         flow = create_test_flow(FlowDesignation.AUTHENTICATION)
-        stage = ConsentStage.objects.create(name="consent", mode=ConsentMode.PERMANENT)
+        stage = ConsentStage.objects.create(name=generate_id(), mode=ConsentMode.PERMANENT)
         binding = FlowStageBinding.objects.create(target=flow, stage=stage, order=2)
 
         plan = FlowPlan(
@@ -75,7 +78,7 @@ def test_expire(self):
         self.client.force_login(self.user)
         flow = create_test_flow(FlowDesignation.AUTHENTICATION)
         stage = ConsentStage.objects.create(
-            name="consent", mode=ConsentMode.EXPIRING, consent_expire_in="seconds=1"
+            name=generate_id(), mode=ConsentMode.EXPIRING, consent_expire_in="seconds=1"
         )
         binding = FlowStageBinding.objects.create(target=flow, stage=stage, order=2)
 
@@ -88,6 +91,18 @@ def test_expire(self):
         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,
+            flow,
+            self.user,
+            permissions=[],
+            additional_permissions=[],
+        )
         response = self.client.post(
             reverse("authentik_api:flow-executor", kwargs={"flow_slug": flow.slug}),
             {},
@@ -102,3 +117,95 @@ def test_expire(self):
         self.assertFalse(
             UserConsent.objects.filter(user=self.user, application=self.application).exists()
         )
+
+    def test_permanent_more_perms(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,
+                PLAN_CONTEXT_CONSENT_PERMISSIONS: [PermissionDict(id="foo", name="foo-desc")],
+            },
+        )
+        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)
+        self.assertStageResponse(
+            response,
+            flow,
+            self.user,
+            permissions=[
+                {"id": "foo", "name": "foo-desc"},
+            ],
+            additional_permissions=[],
+        )
+        response = self.client.post(
+            reverse("authentik_api:flow-executor", kwargs={"flow_slug": flow.slug}),
+            {},
+        )
+        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="foo"
+            ).exists()
+        )
+
+        # Request again with more perms
+        plan = FlowPlan(
+            flow_pk=flow.pk.hex,
+            bindings=[binding],
+            markers=[StageMarker()],
+            context={
+                PLAN_CONTEXT_APPLICATION: self.application,
+                PLAN_CONTEXT_CONSENT_PERMISSIONS: [
+                    PermissionDict(id="foo", name="foo-desc"),
+                    PermissionDict(id="bar", name="bar-desc"),
+                ],
+            },
+        )
+        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,
+            flow,
+            self.user,
+            permissions=[
+                {"id": "foo", "name": "foo-desc"},
+            ],
+            additional_permissions=[
+                {"id": "bar", "name": "bar-desc"},
+            ],
+        )
+        response = self.client.post(
+            reverse("authentik_api:flow-executor", kwargs={"flow_slug": flow.slug}),
+            {},
+        )
+        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="foo bar"
+            ).exists()
+        )

schema.yml

@@ -20510,8 +20510,12 @@ components:
           type: array
           items:
             $ref: '#/components/schemas/Permission'
+        additional_permissions:
+          type: array
+          items:
+            $ref: '#/components/schemas/Permission'
       required:
-      - header_text
+      - additional_permissions
       - pending_user
       - pending_user_avatar
       - permissions
@@ -31215,6 +31219,9 @@ components:
           $ref: '#/components/schemas/User'
         application:
           $ref: '#/components/schemas/Application'
+        permissions:
+          type: string
+          default: ''
       required:
       - application
       - pk