src/authentic2/a2_rbac/management.py

@@ -53,8 +53,8 @@ def update_ou_admin_roles(ou):
             ou_ct_admin_role.add_child(ou_admin_role)
         else:
             ou_ct_admin_role.remove_child(ou_admin_role)
-        if info.get('must_view_user'):
-            ou_ct_admin_role.permissions.add(utils.get_view_user_perm(ou))
+        if info.get('must_search_user'):
+            ou_ct_admin_role.permissions.add(utils.get_search_user_perm(ou))
         ou_ct_admin_role.permissions.add(utils.get_search_ou_perm(ou))
 
 
@@ -76,7 +76,7 @@ MANAGED_CT = {
     ('a2_rbac', 'role'): {
         'name': _('Manager of roles'),
         'scoped_name': _('Roles - {ou}'),
-        'must_view_user': True,
+        'must_search_user': True,
     },
     ('a2_rbac', 'organizationalunit'): {
         'name': _('Manager of organizational units'),
@@ -107,7 +107,7 @@ def update_content_types_roles():
     types.
     """
     cts = ContentType.objects.all()
-    view_user_perm = utils.get_view_user_perm()
+    search_user_perm = utils.get_search_user_perm()
     search_ou_perm = utils.get_search_ou_perm()
     manage_authorizations_user_perm = utils.get_manage_authorizations_user_perm()
     slug = '_a2-manager'
@@ -136,8 +136,8 @@ def update_content_types_roles():
         ct_admin_role = Role.objects.get_admin_role(
             instance=ct, name=name, slug=slug, update_name=True, update_slug=True, create=True
         )
-        if MANAGED_CT[ct_tuple].get('must_view_user'):
-            ct_admin_role.permissions.add(view_user_perm)
+        if MANAGED_CT[ct_tuple].get('must_search_user'):
+            ct_admin_role.permissions.add(search_user_perm)
         if MANAGED_CT[ct_tuple].get('must_manage_authorizations_user'):
             ct_admin_role.permissions.add(manage_authorizations_user_perm)
         ct_admin_role.permissions.add(search_ou_perm)

src/authentic2/a2_rbac/migrations/0032_copy_operations_data.py

@@ -10,7 +10,7 @@ def copy_operations_data(apps, schema_editor):
 
     operation_map = {}
     for operation in OldOperation.objects.all():
-        operation_map[operation.pk] = NewOperation.objects.create(slug=operation.slug)
+        operation_map[operation.pk], _ = NewOperation.objects.get_or_create(slug=operation.slug)
 
     for permission in Permission.objects.all():
         permission.operation_new = operation_map[permission.operation_id]
@@ -24,7 +24,7 @@ def reverse_copy_operations_data(apps, schema_editor):
 
     operation_map = {}
     for operation in NewOperation.objects.all():
-        operation_map[operation.pk] = OldOperation.objects.create(slug=operation.slug)
+        operation_map[operation.pk], _ = OldOperation.objects.get_or_create(slug=operation.slug)
 
     for permission in Permission.objects.all():
         permission.operation = operation_map[permission.operation_new_id]

src/authentic2/a2_rbac/migrations/0039_set_user_view_permissions_by_ou.py

@@ -6,7 +6,7 @@ from django.db import migrations
 def update_admin_roles_permissions(apps, schema_editor):
     Operation = apps.get_model('a2_rbac', 'Operation')
     try:
-        view_operation = Operation.objects.get(slug='view')
+        search_operation = Operation.objects.get(slug='search')
     except Operation.DoesNotExist:
         return
 
@@ -17,21 +17,21 @@ def update_admin_roles_permissions(apps, schema_editor):
 
     target_ct = ContentType.objects.get_for_model(ContentType)
     target_id = ContentType.objects.get_for_model(User).pk
-    user_view_permission_qs = Permission.objects.filter(
-        operation=view_operation, target_ct=target_ct, target_id=target_id
+    user_search_permission_qs = Permission.objects.filter(
+        operation=search_operation, target_ct=target_ct, target_id=target_id
     )
-    global_user_view_permission = user_view_permission_qs.get(ou__isnull=True)
+    global_user_search_permission = user_search_permission_qs.get(ou__isnull=True)
     for ou in OrganizationalUnit.objects.exclude(default=True):
         try:
-            ou_user_view_permission = user_view_permission_qs.get(ou=ou)
+            ou_user_search_permission = user_search_permission_qs.get(ou=ou)
         except Permission.DoesNotExist:
             continue
         # admin roles start with _
         for r in ou.role_set.filter(slug__startswith='_'):
-            if ou_user_view_permission not in r.permissions.all():
+            if ou_user_search_permission not in r.permissions.all():
                 continue
-            r.permissions.remove(global_user_view_permission)
-            r.permissions.add(ou_user_view_permission)
+            r.permissions.remove(global_user_search_permission)
+            r.permissions.add(ou_user_search_permission)
 
 
 class Migration(migrations.Migration):

src/authentic2/a2_rbac/migrations/0040_update_role_administration_permissions.py

@@ -0,0 +1,64 @@
+from django.db import migrations
+
+
+def update_admin_roles_permissions(apps, schema_editor):
+    ContentType = apps.get_model('contenttypes', 'ContentType')
+    OrganizationalUnit = apps.get_model('a2_rbac', 'OrganizationalUnit')
+    Operation = apps.get_model('a2_rbac', 'Operation')
+    Role = apps.get_model('a2_rbac', 'Role')
+    Permission = apps.get_model('a2_rbac', 'Permission')
+    User = apps.get_model('custom_user', 'User')
+
+    view_operation, _ = Operation.objects.get_or_create(slug='view')
+    search_operation, _ = Operation.objects.get_or_create(slug='search')
+
+    target_ct = ContentType.objects.get_for_model(Role)
+
+    def all_ous_iterator():
+        yield from OrganizationalUnit.objects.all()
+        yield None  # global administration not restrained to any OU
+
+    for ou in all_ous_iterator():
+        try:
+            view_user_perm = Permission.objects.get(
+                operation=view_operation,
+                target_ct=ContentType.objects.get_for_model(ContentType),
+                target_id=ContentType.objects.get_for_model(User).pk,
+                ou__isnull=ou is None,
+                ou=ou,
+            )
+        except Permission.DoesNotExist:
+            # The permission does not exist, implying that role administration roles have
+            # not been created in this OU yet, no migration needed.
+            continue
+
+        search_user_perm, _ = Permission.objects.get_or_create(
+            operation=search_operation,
+            target_ct=ContentType.objects.get_for_model(ContentType),
+            target_id=ContentType.objects.get_for_model(User).pk,
+            ou__isnull=ou is None,
+            ou=ou,
+        )
+
+        view_user_perm_roles = [role.id for role in view_user_perm.roles.all()]
+        search_user_perm_roles = [role.id for role in search_user_perm.roles.all()]
+
+        roles_qs = (
+            Role.objects.prefetch_related('permissions')
+            .filter(admin_scope_ct=target_ct, admin_scope_id__isnull=False, id__in=view_user_perm_roles)
+            .exclude(id__in=search_user_perm_roles)
+        )
+        for role in roles_qs:
+            role.permissions.remove(view_user_perm)
+            role.permissions.add(search_user_perm)
+            role.save()
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ('a2_rbac', '0039_set_user_view_permissions_by_ou'),
+    ]
+
+    operations = [
+        migrations.RunPython(update_admin_roles_permissions, reverse_code=migrations.RunPython.noop)
+    ]

src/authentic2/a2_rbac/models.py

@@ -448,12 +448,12 @@ class Role(AbstractBase):
     def get_admin_role(self, create=True):
         from . import utils
 
-        view_user_perm = utils.get_view_user_perm(ou=self.ou)
+        search_user_perm = utils.get_search_user_perm(ou=self.ou)
         admin_role = self.__class__.objects.get_admin_role(
             self,
             name=_('Managers of role "{role}"').format(role=str(self)),
             slug=f'_a2-managers-of-role-{slugify(str(self))}',
-            permissions=(view_user_perm,),
+            permissions=(search_user_perm,),
             self_administered=True,
             update_name=True,
             update_slug=True,

src/authentic2/a2_rbac/utils.py

@@ -55,6 +55,18 @@ def get_view_user_perm(ou=None):
     return view_user_perm
 
 
+def get_search_user_perm(ou=None):
+    User = get_user_model()
+    search_user_perm, dummy = models.Permission.objects.get_or_create(
+        operation=get_operation(models.SEARCH_OP),
+        target_ct=ContentType.objects.get_for_model(ContentType),
+        target_id=ContentType.objects.get_for_model(User).pk,
+        ou__isnull=ou is None,
+        ou=ou,
+    )
+    return search_user_perm
+
+
 def get_search_ou_perm(ou=None):
     if ou:
         view_ou_perm, dummy = models.Permission.objects.get_or_create(

src/authentic2/api_views.py

@@ -1139,14 +1139,14 @@ class RoleMembershipAPI(ExceptionHandlerMixin, RolesMixin, APIView):
             user_qs = self.role.all_members()
         else:
             user_qs = self.role.members.all()
-        user_qs = request.user.filter_by_perm('custom_user.view_user', user_qs)
+        user_qs = request.user.filter_by_perm('custom_user.search_user', user_qs)
         member = get_object_or_404(user_qs, uuid=self.member_uuid)
         return Response(BaseUserSerializer(member).data)
 
     def post(self, request, *args, **kwargs):
         if not request.user.has_perm('a2_rbac.manage_members_role', obj=self.role):
             raise PermissionDenied('User not allowed to manage role members')
-        user_qs = request.user.filter_by_perm('custom_user.view_user', User.objects.all())
+        user_qs = request.user.filter_by_perm('custom_user.search_user', User.objects.all())
         new_member = get_object_or_404(user_qs, uuid=self.member_uuid)
         self.role.members.add(new_member)
         request.journal.record('manager.role.membership.grant', role=self.role, member=new_member, api=True)
@@ -1157,7 +1157,7 @@ class RoleMembershipAPI(ExceptionHandlerMixin, RolesMixin, APIView):
     def delete(self, request, *args, **kwargs):
         if not request.user.has_perm('a2_rbac.manage_members_role', obj=self.role):
             raise PermissionDenied('User not allowed to manage role members')
-        user_qs = request.user.filter_by_perm('custom_user.view_user', User.objects.all())
+        user_qs = request.user.filter_by_perm('custom_user.search_user', User.objects.all())
         member = get_object_or_404(user_qs, uuid=self.member_uuid)
         self.role.members.remove(member)
         request.journal.record('manager.role.membership.removal', role=self.role, member=member, api=True)

tests/test_a2_rbac.py

@@ -109,20 +109,20 @@ def test_admin_roles_update_slug(db):
     assert admin_role3.slug == slug2
 
 
-def test_role_view_user_perm_on_ou_update(db):
+def test_role_search_user_perm_on_ou_update(db):
     role = Role.objects.create(name='Admin')
     admin_role = role.get_admin_role()
-    assert admin_role.permissions.get(operation__slug='view').ou is None
+    assert admin_role.permissions.get(operation__slug='search').ou is None
 
     default_ou = get_default_ou()
     role.ou = default_ou
     role.save()
-    assert admin_role.permissions.get(operation__slug='view').ou == default_ou
+    assert admin_role.permissions.get(operation__slug='search').ou == default_ou
 
     new_ou = OU.objects.create(name='New OU')
     role.ou = new_ou
     role.save()
-    assert admin_role.permissions.get(operation__slug='view').ou == new_ou
+    assert admin_role.permissions.get(operation__slug='search').ou == new_ou
 
 
 def test_role_clean(db):
@@ -722,7 +722,7 @@ def test_a2_rbac_operation_migration(migration, settings):
     Permission = old_apps.get_model('a2_rbac', 'Permission')
 
     # check objects created by signal handlers
-    base_operation = Operation.objects.get(slug='view')
+    base_operation = Operation.objects.get(slug='search')
     base_permission = Permission.objects.filter(operation=base_operation).first()
 
     # check other objects
@@ -738,7 +738,7 @@ def test_a2_rbac_operation_migration(migration, settings):
     Operation = new_apps.get_model('a2_rbac', 'Operation')
     Permission = new_apps.get_model('a2_rbac', 'Permission')
 
-    base_operation = Operation.objects.get(slug='view')
+    base_operation = Operation.objects.get(slug='search')
     assert (
         Permission.objects.filter(
             operation_id=base_operation,
@@ -822,3 +822,89 @@ def test_a2_rbac_role_attribute_migration(migration, settings):
     assert role.emails == []
     assert role.emails_to_members is True
     assert role.is_superuser is False
+
+
+def test_a2_rbac_0040_migration_update_admin_roles_permission(migration, settings):
+    migrate_from = [('a2_rbac', '0039_set_user_view_permissions_by_ou')]
+    migrate_to = [('a2_rbac', '0040_update_role_administration_permissions')]
+
+    old_apps = migration.before(migrate_from)
+
+    ContentType = old_apps.get_model('contenttypes', 'ContentType')
+    Operation = old_apps.get_model('a2_rbac', 'Operation')
+    OrganizationalUnit = old_apps.get_model('a2_rbac', 'OrganizationalUnit')
+    Permission = old_apps.get_model('a2_rbac', 'Permission')
+    Role = old_apps.get_model('a2_rbac', 'Role')
+    User = old_apps.get_model('custom_user', 'User')
+
+    watched_roles = []
+
+    view_operation, _ = Operation.objects.get_or_create(slug='view')
+
+    role = Role.objects.create(slug='main-role', ou=None)
+    view_user_perm, _ = Permission.objects.get_or_create(
+        operation=view_operation,
+        target_ct=ContentType.objects.get_for_model(ContentType),
+        target_id=ContentType.objects.get_for_model(User).pk,
+        ou__isnull=True,
+        ou=None,
+    )
+    admin_role = Role.objects.create(
+        admin_scope_ct=ContentType.objects.get_for_model(Role),
+        admin_scope_id=role.id,
+        ou=None,
+        slug=f'admin-role-{role.slug}',
+    )
+    admin_role.permissions.add(view_user_perm)
+    admin_role.save()
+
+    watched_roles.append(admin_role.id)
+
+    for i in range(4):
+        ou = OrganizationalUnit.objects.create(slug=f'ou-{i}', name=f'OU {i}')
+        role = Role.objects.create(slug=f'role-{i}', name=f'Role {i}', ou=ou)
+        view_user_perm, _ = Permission.objects.get_or_create(
+            operation=view_operation,
+            target_ct=ContentType.objects.get_for_model(ContentType),
+            target_id=ContentType.objects.get_for_model(User).pk,
+            ou__isnull=ou is None,
+            ou=ou,
+        )
+        admin_role = Role.objects.create(
+            admin_scope_ct=ContentType.objects.get_for_model(Role),
+            admin_scope_id=role.id,
+            ou=role.ou,
+            slug=f'admin-role-{role.slug}',
+        )
+        admin_role.permissions.add(view_user_perm)
+        admin_role.save()
+        watched_roles.append(admin_role.id)
+
+    new_apps = migration.apply(migrate_to)
+
+    ContentType = new_apps.get_model('contenttypes', 'ContentType')
+    Operation = new_apps.get_model('a2_rbac', 'Operation')
+    Permission = new_apps.get_model('a2_rbac', 'Permission')
+    Role = new_apps.get_model('a2_rbac', 'Role')
+    User = new_apps.get_model('custom_user', 'User')
+
+    view_operation, _ = Operation.objects.get_or_create(slug='view')
+    search_operation, _ = Operation.objects.get_or_create(slug='search')
+
+    for role in Role.objects.filter(id__in=watched_roles):
+        view_user_perm, _ = Permission.objects.get_or_create(
+            operation=view_operation,
+            target_ct=ContentType.objects.get_for_model(ContentType),
+            target_id=ContentType.objects.get_for_model(User).pk,
+            ou__isnull=role.ou is None,
+            ou=role.ou,
+        )
+        search_user_perm, _ = Permission.objects.get_or_create(
+            operation=search_operation,
+            target_ct=ContentType.objects.get_for_model(ContentType),
+            target_id=ContentType.objects.get_for_model(User).pk,
+            ou__isnull=role.ou is None,
+            ou=role.ou,
+        )
+        assert role not in view_user_perm.roles.all()
+        assert role in search_user_perm.roles.all()

tests/test_commands.py

@@ -31,7 +31,7 @@ from mellon.models import Issuer, UserSAMLIdentifier
 from authentic2.a2_rbac.models import (
     ADMIN_OP,
     MANAGE_MEMBERS_OP,
-    VIEW_OP,
+    SEARCH_OP,
     Operation,
     OrganizationalUnit,
     Permission,
@@ -694,7 +694,7 @@ def test_check_and_repair_managers_of_roles(db, capsys):
     )
     assert manager_role1.permissions.get(operation=get_operation(MANAGE_MEMBERS_OP), target_id=role1.id)
     assert manager_role1.permissions.get(
-        operation=get_operation(VIEW_OP),
+        operation=get_operation(SEARCH_OP),
         target_ct=ContentType.objects.get_for_model(ContentType),
         target_id=ContentType.objects.get_for_model(User).pk,
     )

tests/test_user_manager.py

@@ -33,7 +33,7 @@ from webtest import Upload
 from authentic2.a2_rbac.models import VIEW_OP
 from authentic2.a2_rbac.models import OrganizationalUnit as OU
 from authentic2.a2_rbac.models import Permission, Role
-from authentic2.a2_rbac.utils import get_default_ou, get_operation, get_view_user_perm
+from authentic2.a2_rbac.utils import get_default_ou, get_operation, get_search_user_perm, get_view_user_perm
 from authentic2.apps.journal.models import Event
 from authentic2.custom_user.models import User
 from authentic2.manager import user_import
@@ -1372,19 +1372,18 @@ def test_manager_user_roles_visibility(app, simple_user, admin, ou1, ou2):
         target_ct=ContentType.objects.get_for_model(Role),
         target_id=role1.pk,
     )
-    other_role.permissions.add(get_view_user_perm())
+    other_role.permissions.add(get_search_user_perm())
     other_role.permissions.add(view_role1_perm)
     other_role.save()
     other_user.roles.add(other_role)
     other_user.save()
 
     login(app, other_user, '/manage/', password='auietsrn')
-    resp = app.get(reverse('a2-manager-user-detail', kwargs={'pk': simple_user.id}))
-    assert '/manage/roles/%s/' % role1.pk in resp.text
-    assert 'Role 1' in resp.text
-    assert '/manage/roles/%s/' % role2.pk not in resp.text
-    assert 'Role 2' in resp.text
-    app.get('/manage/roles/%s/' % role2.pk, status=403)
+    app.get(
+        reverse('a2-manager-user-detail', kwargs={'pk': simple_user.id}), status=403
+    )  # search_user in not enough to view users -> NOK
+    app.get('/manage/roles/%s/' % role1.pk)  # OK
+    app.get('/manage/roles/%s/' % role2.pk, status=403)  # NOK
 
 
 def test_manager_user_authorizations(app, superuser, simple_user):