From 2a8e1e2f949150d50de1cdd589b55ce7fccafd67 Mon Sep 17 00:00:00 2001 From: Valentin Deniaud Date: Mon, 27 Apr 2020 15:03:07 +0200 Subject: [PATCH] a2_rbac: change self admin permission to manage_members (#42086) --- src/authentic2/a2_rbac/management.py | 3 +- .../migrations/0023_fix_self_admin_perm.py | 45 ++++++++++++ src/authentic2/a2_rbac/models.py | 21 +++--- src/authentic2/manager/user_views.py | 2 +- tests/test_a2_rbac.py | 70 +++++++++++++++---- 5 files changed, 118 insertions(+), 23 deletions(-) create mode 100644 src/authentic2/a2_rbac/migrations/0023_fix_self_admin_perm.py diff --git a/src/authentic2/a2_rbac/management.py b/src/authentic2/a2_rbac/management.py index 627aa9af..0c68ecd3 100644 --- a/src/authentic2/a2_rbac/management.py +++ b/src/authentic2/a2_rbac/management.py @@ -152,7 +152,8 @@ def update_user_admin_roles_permission(): old_perm = role.permissions.get(operation__slug=ADMIN_OP.slug) administered_role = old_perm.target admin_role = administered_role.get_admin_role() - new_perm = admin_role.permissions.get(operation__slug=MANAGE_MEMBERS_OP.slug) + new_perm = admin_role.permissions.get(operation__slug=MANAGE_MEMBERS_OP.slug, + target_id=administered_role.pk) admin_role.delete() role.admin_scope_id = new_perm.pk role.save() diff --git a/src/authentic2/a2_rbac/migrations/0023_fix_self_admin_perm.py b/src/authentic2/a2_rbac/migrations/0023_fix_self_admin_perm.py new file mode 100644 index 00000000..3a3c3db7 --- /dev/null +++ b/src/authentic2/a2_rbac/migrations/0023_fix_self_admin_perm.py @@ -0,0 +1,45 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.18 on 2020-05-12 08:58 +from __future__ import unicode_literals + +from django.db import migrations +from django.db.utils import IntegrityError +from django.utils.six import text_type + +from authentic2.a2_rbac.models import MANAGE_MEMBERS_OP +from django_rbac.models import CHANGE_OP + + +def update_self_administration_perm(apps, schema_editor): + Role = apps.get_model('a2_rbac', 'Role') + Permission = apps.get_model('a2_rbac', 'Permission') + Operation = apps.get_model('django_rbac', 'Operation') + ContentType = apps.get_model('contenttypes', 'ContentType') + change_op, _ = Operation.objects.get_or_create(slug=text_type(CHANGE_OP.slug)) + manage_members_op, _ = Operation.objects.get_or_create(slug=text_type(MANAGE_MEMBERS_OP.slug)) + ct = ContentType.objects.get_for_model(Role) + perms_to_delete = [] + for role in Role.objects.all(): + try: + perm = role.permissions.get(operation=change_op, target_ct=ct, target_id=role.pk) + except Permission.DoesNotExist: + continue + + new_perm, _ = Permission.objects.get_or_create( + operation=manage_members_op, target_ct=ct, target_id=role.pk, ou__isnull=True + ) + role.permissions.add(new_perm) + role.permissions.remove(perm) + perms_to_delete.append(perm.pk) + Permission.objects.filter(pk__in=perms_to_delete, roles__isnull=True).delete() + + +class Migration(migrations.Migration): + + dependencies = [ + ('a2_rbac', '0022_auto_20200402_1101'), + ] + + operations = [ + migrations.RunPython(update_self_administration_perm, migrations.RunPython.noop) + ] diff --git a/src/authentic2/a2_rbac/models.py b/src/authentic2/a2_rbac/models.py index b78a2963..186efce3 100644 --- a/src/authentic2/a2_rbac/models.py +++ b/src/authentic2/a2_rbac/models.py @@ -25,7 +25,7 @@ from django.contrib.contenttypes.models import ContentType from django_rbac.models import (RoleAbstractBase, PermissionAbstractBase, OrganizationalUnitAbstractBase, RoleParentingAbstractBase, VIEW_OP, - CHANGE_OP, Operation) + Operation) from django_rbac import utils as rbac_utils from authentic2.decorators import errorcollector @@ -282,21 +282,26 @@ class Role(RoleAbstractBase): self.get_admin_role(create=False) return result - def has_self_administration(self, op=CHANGE_OP): + def has_self_administration(self, op=None): + if not op: + op = MANAGE_MEMBERS_OP Permission = rbac_utils.get_permission_model() - admin_op = rbac_utils.get_operation(op) + operation = rbac_utils.get_operation(op) self_perm, created = Permission.objects.get_or_create( - operation=admin_op, + operation=operation, target_ct=ContentType.objects.get_for_model(self), - target_id=self.pk) + target_id=self.pk, + ou__is_null=True) return self.permissions.filter(pk=self_perm.pk).exists() - def add_self_administration(self, op=CHANGE_OP): + def add_self_administration(self, op=None): 'Add permission to role so that it is self-administered' + if not op: + op = MANAGE_MEMBERS_OP Permission = rbac_utils.get_permission_model() - admin_op = rbac_utils.get_operation(op) + operation = rbac_utils.get_operation(op) self_perm, created = Permission.objects.get_or_create( - operation=admin_op, + operation=operation, target_ct=ContentType.objects.get_for_model(self), target_id=self.pk) self.permissions.through.objects.get_or_create(role=self, permission=self_perm) diff --git a/src/authentic2/manager/user_views.py b/src/authentic2/manager/user_views.py index 9b032a8c..d540589d 100644 --- a/src/authentic2/manager/user_views.py +++ b/src/authentic2/manager/user_views.py @@ -327,7 +327,7 @@ class UserDetailView(OtherActionsMixin, BaseDetailView): role_qs = get_role_model().objects.all() if app_settings.ROLE_MEMBERS_FROM_OU and instance.ou: role_qs = role_qs.filter(ou=instance.ou) - return user.filter_by_perm('a2_rbac.change_role', role_qs).exists() + return user.filter_by_perm('a2_rbac.manage_members_role', role_qs).exists() def get_context_data(self, **kwargs): kwargs['default_ou'] = get_default_ou diff --git a/tests/test_a2_rbac.py b/tests/test_a2_rbac.py index 85b22a84..b6abef3a 100644 --- a/tests/test_a2_rbac.py +++ b/tests/test_a2_rbac.py @@ -22,18 +22,19 @@ from django.core.management import call_command from django.contrib.contenttypes.models import ContentType from django_rbac.utils import get_permission_model -from django_rbac.models import Operation +from django_rbac.models import Operation, CHANGE_OP from authentic2.custom_user.models import User from authentic2.models import Service from django.core.management import call_command -from authentic2.a2_rbac.models import Role, OrganizationalUnit as OU, RoleAttribute from authentic2.a2_rbac.utils import get_default_ou, get_view_user_perm from authentic2.a2_rbac.models import ( Role, Permission, OrganizationalUnit as OU, - RoleAttribute) + RoleAttribute, + MANAGE_MEMBERS_OP +) from authentic2.utils import get_hex_uuid @@ -59,23 +60,23 @@ def test_delete_role(db): # There should two more roles, the role and its admin counterpart assert Role.objects.count() == rcount + 2 - # There should be two more permissions the admin permission on the role - # and the admin permission on the admin role - admin_perm = Permission.objects.by_target(new_role) \ + # There should be two more permissions the manage-members permission on the role + # and the manage-members permission on the admin role + manage_members_perm = Permission.objects.by_target(new_role) \ .get(operation__slug='manage_members') admin_role = Role.objects.get( - admin_scope_ct=ContentType.objects.get_for_model(admin_perm), - admin_scope_id=admin_perm.pk) - admin_admin_perm = Permission.objects.by_target(admin_role) \ - .get(operation__slug='change') + admin_scope_ct=ContentType.objects.get_for_model(manage_members_perm), + admin_scope_id=manage_members_perm.pk) + admin_manage_members_perm = Permission.objects.by_target(admin_role) \ + .get(operation__slug='manage_members') assert Permission.objects.count() == pcount + 2 new_role.delete() with pytest.raises(Permission.DoesNotExist): - Permission.objects.get(pk=admin_perm.pk) + Permission.objects.get(pk=manage_members_perm.pk) with pytest.raises(Role.DoesNotExist): Role.objects.get(pk=admin_role.pk) with pytest.raises(Permission.DoesNotExist): - Permission.objects.get(pk=admin_admin_perm.pk) + Permission.objects.get(pk=admin_manage_members_perm.pk) assert Role.objects.count() == rcount assert Permission.objects.count() == pcount @@ -260,7 +261,7 @@ def test_role_with_permission_export_json(db): name='other role name', slug='other-role-slug', uuid=get_hex_uuid(), ou=some_ou) ou = OU.objects.create(name='basic ou', slug='basic-ou', description='basic ou description') Permission = get_permission_model() - op = Operation.objects.first() + op = Operation.objects.get(slug='add') perm_saml = Permission.objects.create( operation=op, ou=ou, target_ct=ContentType.objects.get_for_model(ContentType), @@ -526,3 +527,46 @@ def test_update_role_admins_perm(transactional_db, simple_user): emit_post_migrate_signal(verbosity=0, interactive=False, db='default', created_models=[]) assert simple_user.get_all_permissions(role) == \ {'a2_rbac.manage_members_role', 'a2_rbac.search_role', 'a2_rbac.view_role'} + + +@pytest.mark.parametrize('new_perm_exists', [True, False]) +def test_update_self_admin_perm_migration(migration, new_perm_exists): + old_apps = migration.before([('a2_rbac', '0022_auto_20200402_1101')]) + Role = old_apps.get_model('a2_rbac', 'Role') + OrganizationalUnit = old_apps.get_model('a2_rbac', 'OrganizationalUnit') + Permission = old_apps.get_model('a2_rbac', 'Permission') + Operation = old_apps.get_model('django_rbac', 'Operation') + ContentType = old_apps.get_model('contenttypes', 'ContentType') + ct = ContentType.objects.get_for_model(Role) + change_op, _ = Operation.objects.get_or_create(slug=CHANGE_OP.slug) + manage_members_op, _ = Operation.objects.get_or_create(slug=MANAGE_MEMBERS_OP.slug) + + # add old self administration + role = Role.objects.create(name='name', slug='slug') + self_perm, _ = Permission.objects.get_or_create(operation=change_op, target_ct=ct, target_id=role.pk) + role.permissions.add(self_perm) + + if new_perm_exists: + new_self_perm, _ = Permission.objects.get_or_create(operation=manage_members_op, target_ct=ct, + target_id=role.pk) + else: + Permission.objects.filter( + operation=manage_members_op, target_ct=ct, target_id=role.pk + ).delete() + + new_apps = migration.apply([('a2_rbac', '0023_fix_self_admin_perm')]) + Role = new_apps.get_model('a2_rbac', 'Role') + Operation = old_apps.get_model('django_rbac', 'Operation') + + role = Role.objects.get(slug='slug') + assert role.permissions.count() == 1 + + perm = role.permissions.first() + assert perm.operation.pk == manage_members_op.pk + assert perm.target_ct.pk == ct.pk + assert perm.target_id == role.pk + + if new_perm_exists: + assert perm.pk == new_self_perm.pk + + assert not Permission.objects.filter(operation=change_op, target_ct=ct, target_id=role.pk).exists() -- 2.20.1