From 1afb4a7a768ef966a8df2adba4fe331fe2f23b30 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Thu, 11 Feb 2021 20:36:48 +0100 Subject: [PATCH] policies: add ability to directly assign groups in bindings --- authentik/policies/api.py | 20 ++++- authentik/policies/forms.py | 6 +- .../migrations/0002_auto_20210211_1924.py | 20 +++++ authentik/policies/group_membership/models.py | 5 +- .../policies/migrations/0005_binding_group.py | 76 +++++++++++++++++++ authentik/policies/models.py | 40 +++++++++- authentik/policies/process.py | 17 +++-- authentik/policies/tests/test_process.py | 48 +++++++++++- swagger.yaml | 13 +++- website/docs/releases/2021.2.md | 9 +++ 10 files changed, 232 insertions(+), 22 deletions(-) create mode 100644 authentik/policies/group_membership/migrations/0002_auto_20210211_1924.py create mode 100644 authentik/policies/migrations/0005_binding_group.py diff --git a/authentik/policies/api.py b/authentik/policies/api.py index 1428b433a..4f43fe927 100644 --- a/authentik/policies/api.py +++ b/authentik/policies/api.py @@ -50,12 +50,12 @@ class PolicySerializer(ModelSerializer): _resolve_inheritance: bool + object_type = SerializerMethodField() + def __init__(self, *args, resolve_inheritance: bool = True, **kwargs): super().__init__(*args, **kwargs) self._resolve_inheritance = resolve_inheritance - object_type = SerializerMethodField() - def get_object_type(self, obj): """Get object type so that we know which API Endpoint to use to get the full object""" return obj._meta.object_name.lower().replace("provider", "") @@ -64,7 +64,9 @@ class PolicySerializer(ModelSerializer): # pyright: reportGeneralTypeIssues=false if instance.__class__ == Policy or not self._resolve_inheritance: return super().to_representation(instance) - return instance.serializer(instance=instance, resolve_inheritance=False).data + return dict( + instance.serializer(instance=instance, resolve_inheritance=False).data + ) class Meta: @@ -102,7 +104,17 @@ class PolicyBindingSerializer(ModelSerializer): class Meta: model = PolicyBinding - fields = ["pk", "policy", "policy_obj", "target", "enabled", "order", "timeout"] + fields = [ + "pk", + "policy", + "policy_obj", + "group", + "user", + "target", + "enabled", + "order", + "timeout", + ] class PolicyBindingViewSet(ModelViewSet): diff --git a/authentik/policies/forms.py b/authentik/policies/forms.py index 5148e349d..36f7d183f 100644 --- a/authentik/policies/forms.py +++ b/authentik/policies/forms.py @@ -14,10 +14,10 @@ class PolicyBindingForm(forms.ModelForm): to_field_name="pbm_uuid", ) policy = GroupedModelChoiceField( - queryset=Policy.objects.all().select_subclasses(), + queryset=Policy.objects.all().select_subclasses(), required=False ) - def __init__(self, *args, **kwargs): + def __init__(self, *args, **kwargs): # pragma: no cover super().__init__(*args, **kwargs) if "target" in self.initial: self.fields["target"].widget = forms.HiddenInput() @@ -25,7 +25,7 @@ class PolicyBindingForm(forms.ModelForm): class Meta: model = PolicyBinding - fields = ["enabled", "policy", "target", "order", "timeout"] + fields = ["enabled", "policy", "group", "user", "target", "order", "timeout"] class PolicyForm(forms.ModelForm): diff --git a/authentik/policies/group_membership/migrations/0002_auto_20210211_1924.py b/authentik/policies/group_membership/migrations/0002_auto_20210211_1924.py new file mode 100644 index 000000000..090b597de --- /dev/null +++ b/authentik/policies/group_membership/migrations/0002_auto_20210211_1924.py @@ -0,0 +1,20 @@ +# Generated by Django 3.1.6 on 2021-02-11 19:24 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ("authentik_policies_group_membership", "0001_initial"), + ] + + operations = [ + migrations.AlterModelOptions( + name="groupmembershippolicy", + options={ + "verbose_name": "Group Membership Policy (deprecated)", + "verbose_name_plural": "Group Membership Policies", + }, + ), + ] diff --git a/authentik/policies/group_membership/models.py b/authentik/policies/group_membership/models.py index c53b3550d..076bfd7be 100644 --- a/authentik/policies/group_membership/models.py +++ b/authentik/policies/group_membership/models.py @@ -12,7 +12,8 @@ from authentik.policies.types import PolicyRequest, PolicyResult class GroupMembershipPolicy(Policy): - """Check that the user is member of the selected group.""" + """Check that the user is member of the selected group. **DEPRECATED** + Assign the group directly in a binding instead of using this policy.""" group = models.ForeignKey(Group, null=True, blank=True, on_delete=models.SET_NULL) @@ -35,5 +36,5 @@ class GroupMembershipPolicy(Policy): class Meta: - verbose_name = _("Group Membership Policy") + verbose_name = _("Group Membership Policy (deprecated)") verbose_name_plural = _("Group Membership Policies") diff --git a/authentik/policies/migrations/0005_binding_group.py b/authentik/policies/migrations/0005_binding_group.py new file mode 100644 index 000000000..f73ec953c --- /dev/null +++ b/authentik/policies/migrations/0005_binding_group.py @@ -0,0 +1,76 @@ +# Generated by Django 3.1.6 on 2021-02-08 18:36 + +import django.db.models.deletion +from django.apps.registry import Apps +from django.conf import settings +from django.db import migrations, models +from django.db.backends.base.schema import BaseDatabaseSchemaEditor + +import authentik.lib.models + + +def migrate_from_groupmembership(apps: Apps, schema_editor: BaseDatabaseSchemaEditor): + try: + GroupMembershipPolicy = apps.get_model( + "authentik_policies_group_membership", "GroupMembershipPolicy" + ) + except LookupError: + # GroupMembership app isn't installed, ignore migration + return + PolicyBinding = apps.get_model("authentik_policies", "PolicyBinding") + + db_alias = schema_editor.connection.alias + + for membership in GroupMembershipPolicy.objects.using(db_alias).all(): + for binding in PolicyBinding.objects.using(db_alias).filter(policy=membership): + binding.group = membership.group + binding.policy = None + binding.save() + membership.delete() + + +class Migration(migrations.Migration): + + dependencies = [ + ("authentik_core", "0017_managed"), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ("authentik_policies", "0004_policy_execution_logging"), + ] + + operations = [ + migrations.AddField( + model_name="policybinding", + name="group", + field=models.ForeignKey( + blank=True, + default=None, + null=True, + on_delete=django.db.models.deletion.CASCADE, + to="authentik_core.group", + ), + ), + migrations.AddField( + model_name="policybinding", + name="user", + field=models.ForeignKey( + blank=True, + default=None, + null=True, + on_delete=django.db.models.deletion.CASCADE, + to=settings.AUTH_USER_MODEL, + ), + ), + migrations.AlterField( + model_name="policybinding", + name="policy", + field=authentik.lib.models.InheritanceForeignKey( + blank=True, + default=None, + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="+", + to="authentik_policies.policy", + ), + ), + migrations.RunPython(migrate_from_groupmembership), + ] diff --git a/authentik/policies/models.py b/authentik/policies/models.py index f5ca6ff7c..a495cc73d 100644 --- a/authentik/policies/models.py +++ b/authentik/policies/models.py @@ -43,7 +43,32 @@ class PolicyBinding(SerializerModel): enabled = models.BooleanField(default=True) - policy = InheritanceForeignKey("Policy", on_delete=models.CASCADE, related_name="+") + policy = InheritanceForeignKey( + "Policy", + on_delete=models.CASCADE, + related_name="+", + default=None, + null=True, + blank=True, + ) + group = models.ForeignKey( + # This is quite an ugly hack to prevent pylint from trying + # to resolve authentik_core.models.Group + # as python import path + "authentik_core." + "Group", + on_delete=models.CASCADE, + default=None, + null=True, + blank=True, + ) + user = models.ForeignKey( + "authentik_core." + "User", + on_delete=models.CASCADE, + default=None, + null=True, + blank=True, + ) + target = InheritanceForeignKey( PolicyBindingModel, on_delete=models.CASCADE, related_name="+" ) @@ -57,6 +82,17 @@ class PolicyBinding(SerializerModel): order = models.IntegerField() + def passes(self, request: PolicyRequest) -> PolicyResult: + """Check if request passes this PolicyBinding, check policy, group or user""" + if self.policy: + self.policy: Policy + return self.policy.passes(request) + if self.group: + return PolicyResult(self.group.users.filter(pk=request.user.pk).exists()) + if self.user: + return PolicyResult(request.user == self.user) + return PolicyResult(False) + @property def serializer(self) -> BaseSerializer: from authentik.policies.api import PolicyBindingSerializer @@ -105,7 +141,7 @@ class Policy(SerializerModel, CreatedUpdatedModel): return f"{self.__class__.__name__} {self.name}" def passes(self, request: PolicyRequest) -> PolicyResult: # pragma: no cover - """Check if user instance passes this policy""" + """Check if request passes this policy""" raise PolicyException() class Meta: diff --git a/authentik/policies/process.py b/authentik/policies/process.py index 3d12ab3d4..589173cf2 100644 --- a/authentik/policies/process.py +++ b/authentik/policies/process.py @@ -23,7 +23,7 @@ PROCESS_CLASS = FORK_CTX.Process def cache_key(binding: PolicyBinding, request: PolicyRequest) -> str: """Generate Cache key for policy""" - prefix = f"policy_{binding.policy_binding_uuid.hex}_{binding.policy.pk.hex}" + prefix = f"policy_{binding.policy_binding_uuid.hex}_" if request.http_request and hasattr(request.http_request, "session"): prefix += f"_{request.http_request.session.session_key}" if request.user: @@ -79,13 +79,14 @@ class PolicyProcess(PROCESS_CLASS): process="PolicyProcess", ) try: - policy_result = self.binding.policy.passes(self.request) - if self.binding.policy.execution_logging and not self.request.debug: - self.create_event( - EventAction.POLICY_EXECUTION, - message="Policy Execution", - result=policy_result, - ) + policy_result = self.binding.passes(self.request) + if self.binding.policy and not self.request.debug: + if self.binding.policy.execution_logging: + self.create_event( + EventAction.POLICY_EXECUTION, + message="Policy Execution", + result=policy_result, + ) except PolicyException as exc: # Either use passed original exception or whatever we have src_exc = exc.src_exc if exc.src_exc else exc diff --git a/authentik/policies/tests/test_process.py b/authentik/policies/tests/test_process.py index e1908fb78..a4ca22800 100644 --- a/authentik/policies/tests/test_process.py +++ b/authentik/policies/tests/test_process.py @@ -1,8 +1,9 @@ """policy process tests""" from django.core.cache import cache from django.test import RequestFactory, TestCase +from guardian.shortcuts import get_anonymous_user -from authentik.core.models import Application, User +from authentik.core.models import Application, Group, User from authentik.events.models import Event, EventAction from authentik.policies.dummy.models import DummyPolicy from authentik.policies.expression.models import ExpressionPolicy @@ -25,6 +26,51 @@ class TestPolicyProcess(TestCase): self.factory = RequestFactory() self.user = User.objects.create_user(username="policyuser") + def test_group_passing(self): + """Test binding to group""" + group = Group.objects.create(name="test-group") + group.users.add(self.user) + group.save() + binding = PolicyBinding(group=group) + + request = PolicyRequest(self.user) + response = PolicyProcess(binding, request, None).execute() + self.assertEqual(response.passing, True) + + def test_group_negative(self): + """Test binding to group""" + group = Group.objects.create(name="test-group") + group.save() + binding = PolicyBinding(group=group) + + request = PolicyRequest(self.user) + response = PolicyProcess(binding, request, None).execute() + self.assertEqual(response.passing, False) + + def test_user_passing(self): + """Test binding to user""" + binding = PolicyBinding(user=self.user) + + request = PolicyRequest(self.user) + response = PolicyProcess(binding, request, None).execute() + self.assertEqual(response.passing, True) + + def test_user_negative(self): + """Test binding to user""" + binding = PolicyBinding(user=get_anonymous_user()) + + request = PolicyRequest(self.user) + response = PolicyProcess(binding, request, None).execute() + self.assertEqual(response.passing, False) + + def test_empty(self): + """Test binding to user""" + binding = PolicyBinding() + + request = PolicyRequest(self.user) + response = PolicyProcess(binding, request, None).execute() + self.assertEqual(response.passing, False) + def test_invalid(self): """Test Process with invalid arguments""" policy = DummyPolicy.objects.create(result=True, wait_min=0, wait_max=1) diff --git a/swagger.yaml b/swagger.yaml index bc842bad9..9db897ffe 100755 --- a/swagger.yaml +++ b/swagger.yaml @@ -3272,7 +3272,7 @@ paths: parameters: - name: policy_uuid in: path - description: A UUID string identifying this Group Membership Policy. + description: A UUID string identifying this Group Membership Policy (deprecated). required: true type: string format: uuid @@ -8633,7 +8633,6 @@ definitions: PolicyBinding: description: PolicyBinding Serializer required: - - policy - target - order type: object @@ -8647,8 +8646,18 @@ definitions: title: Policy type: string format: uuid + x-nullable: true policy_obj: $ref: '#/definitions/Policy' + group: + title: Group + type: string + format: uuid + x-nullable: true + user: + title: User + type: integer + x-nullable: true target: title: Target type: string diff --git a/website/docs/releases/2021.2.md b/website/docs/releases/2021.2.md index 8d76aa6c7..31552fd55 100644 --- a/website/docs/releases/2021.2.md +++ b/website/docs/releases/2021.2.md @@ -21,6 +21,15 @@ title: Release 2021.1.2 - Add test view to debug property-mappings. +- Simplify role-based access + + Instead of having to create a Group Membership policy for every group you want to use, you can now select a Group and even a User directly in a binding. + + When a group is selected, the binding behaves the same as if a Group Membership policy exists. + + When a user is selected, the binding checks the user of the request, and denies the request when the user doesn't match. + + ## Fixes - admin: add test view for property mappings