From d3c347018997a48cd620f3dab2a5f8293d5d0f73 Mon Sep 17 00:00:00 2001
From: Valentin Deniaud <vdeniaud@entrouvert.com>
Date: Fri, 4 Oct 2019 16:08:59 +0200
Subject: [PATCH] manager: add manage members permission for role admins
 (#20513)

---
 src/authentic2/a2_rbac/models.py                  |  4 +++-
 src/authentic2/a2_rbac/signal_handlers.py         |  4 +++-
 src/authentic2/manager/role_views.py              |  5 -----
 .../authentic2/manager/role_members.html          |  2 +-
 src/authentic2/settings.py                        |  1 +
 tests/test_a2_rbac.py                             |  2 +-
 tests/test_manager.py                             | 15 +++++++++++++++
 7 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/src/authentic2/a2_rbac/models.py b/src/authentic2/a2_rbac/models.py
index 0a77ae06..712b3acb 100644
--- a/src/authentic2/a2_rbac/models.py
+++ b/src/authentic2/a2_rbac/models.py
@@ -221,7 +221,8 @@ class Role(RoleAbstractBase):
             self_administered=True,
             update_name=True,
             update_slug=True,
-            create=create)
+            create=create,
+            operation=MANAGE_MEMBERS_OP)
         return admin_role
 
     def validate_unique(self, exclude=None):
@@ -393,3 +394,4 @@ CHANGE_PASSWORD_OP = Operation(name=_('Change password'), slug='change_password'
 RESET_PASSWORD_OP = Operation(name=_('Reset password'), slug='reset_password')
 ACTIVATE_OP = Operation(name=_('Activate'), slug='activate')
 CHANGE_EMAIL_OP = Operation(name=_('Change email'), slug='change_email')
+MANAGE_MEMBERS_OP = Operation(name=_('Manage role members'), slug='manage_members')
diff --git a/src/authentic2/a2_rbac/signal_handlers.py b/src/authentic2/a2_rbac/signal_handlers.py
index 87667d89..0c401296 100644
--- a/src/authentic2/a2_rbac/signal_handlers.py
+++ b/src/authentic2/a2_rbac/signal_handlers.py
@@ -86,7 +86,8 @@ def update_service_role_ou(sender, instance, created, raw, **kwargs):
 
 def create_default_permissions(app_config, verbosity=2, interactive=True, using=DEFAULT_DB_ALIAS,
                                **kwargs):
-    from .models import CHANGE_PASSWORD_OP, RESET_PASSWORD_OP, ACTIVATE_OP, CHANGE_EMAIL_OP
+    from .models import (CHANGE_PASSWORD_OP, RESET_PASSWORD_OP, ACTIVATE_OP, CHANGE_EMAIL_OP,
+                         MANAGE_MEMBERS_OP)
 
     if not router.allow_migrate(using, get_ou_model()):
         return
@@ -96,3 +97,4 @@ def create_default_permissions(app_config, verbosity=2, interactive=True, using=
         get_operation(RESET_PASSWORD_OP)
         get_operation(ACTIVATE_OP)
         get_operation(CHANGE_EMAIL_OP)
+        get_operation(MANAGE_MEMBERS_OP)
diff --git a/src/authentic2/manager/role_views.py b/src/authentic2/manager/role_views.py
index 7df448a0..713cc816 100644
--- a/src/authentic2/manager/role_views.py
+++ b/src/authentic2/manager/role_views.py
@@ -77,11 +77,6 @@ class RolesView(views.HideOUColumnMixin, RolesMixin, views.BaseTableView):
         kwargs['queryset'] = self.get_queryset()
         return kwargs
 
-    def authorize(self, request, *args, **kwargs):
-        super(RolesView, self).authorize(request, *args, **kwargs)
-        self.can_add = bool(request.user.ous_with_perm('a2_rbac.add_role'))
-
-
 listing = RolesView.as_view()
 
 
diff --git a/src/authentic2/manager/templates/authentic2/manager/role_members.html b/src/authentic2/manager/templates/authentic2/manager/role_members.html
index de5ca308..7388e084 100644
--- a/src/authentic2/manager/templates/authentic2/manager/role_members.html
+++ b/src/authentic2/manager/templates/authentic2/manager/role_members.html
@@ -53,7 +53,7 @@
  {% include "authentic2/manager/export_include.html" with export_view_name="a2-manager-role-members-export" %}
 
  {% if view.can_change %}
-   <form method="post" class="manager-m2m-add-form">
+   <form method="post" class="manager-m2m-add-form" id="add-user">
            {% csrf_token %}
            {{ form }}
            <button>{% trans "Add" %}</button>
diff --git a/src/authentic2/settings.py b/src/authentic2/settings.py
index f09c7fc6..de58caf3 100644
--- a/src/authentic2/settings.py
+++ b/src/authentic2/settings.py
@@ -331,6 +331,7 @@ DJANGO_RBAC_PERMISSIONS_HIERARCHY = {
     'change': ['view', 'search'],
     'delete': ['view', 'search'],
     'add': ['view', 'search'],
+    'manage_members': ['change', 'view', 'search', 'delete'],
 }
 
 SILENCED_SYSTEM_CHECKS = ["auth.W004"]
diff --git a/tests/test_a2_rbac.py b/tests/test_a2_rbac.py
index cb7756c9..0e7fd7f1 100644
--- a/tests/test_a2_rbac.py
+++ b/tests/test_a2_rbac.py
@@ -62,7 +62,7 @@ def test_delete_role(db):
     # 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) \
-        .get(operation__slug='admin')
+        .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)
diff --git a/tests/test_manager.py b/tests/test_manager.py
index 518b289f..c9532024 100644
--- a/tests/test_manager.py
+++ b/tests/test_manager.py
@@ -896,3 +896,18 @@ def test_manager_edit_user_email_verified(app, simple_user, superuser_or_admin):
 
     user = User.objects.get(id=simple_user.id)
     assert not user.email_verified
+
+
+def test_manager_role_admin_permissions(app, simple_user):
+    administered_role = Role.objects.create(name='Coucou')
+    admin_role = administered_role.get_admin_role()
+    simple_user.roles.add(admin_role)
+    login(app, simple_user, '/manage/')
+
+    response = app.get('/manage/roles/%s/' % administered_role.pk)
+    form = response.forms['add-user']
+    form['user'].force_value(simple_user.pk)
+    response = form.submit().follow()
+    assert administered_role in simple_user.roles.all()
+
+    response = app.get('/manage/roles/add/', status=403)
-- 
2.20.1

