From 6cdd161991f9644f3e643995ac84be5ac2f0f458 Mon Sep 17 00:00:00 2001
From: Valentin Deniaud <vdeniaud@entrouvert.com>
Date: Tue, 22 Oct 2019 17:30:57 +0200
Subject: [PATCH 4/4] manager: use new manage_members permission (#20513)

---
 src/authentic2/manager/role_views.py          |  8 +-
 .../authentic2/manager/role_members.html      |  6 +-
 .../manager/role_members_table.html           |  2 +-
 src/authentic2/manager/user_views.py          |  4 +-
 src/authentic2/manager/views.py               | 13 ++-
 tests/test_manager.py                         | 84 ++++++++++++++++++-
 6 files changed, 98 insertions(+), 19 deletions(-)

diff --git a/src/authentic2/manager/role_views.py b/src/authentic2/manager/role_views.py
index 713cc816..cbd2f207 100644
--- a/src/authentic2/manager/role_views.py
+++ b/src/authentic2/manager/role_views.py
@@ -155,7 +155,7 @@ class RoleMembersView(views.HideOUColumnMixin, RoleViewMixin, views.BaseSubTable
     def form_valid(self, form):
         user = form.cleaned_data['user']
         action = form.cleaned_data['action']
-        if self.can_change:
+        if self.can_manage_members:
             if action == 'add':
                 if self.object.members.filter(pk=user.pk).exists():
                     messages.warning(self.request, _('User already in this role.'))
@@ -277,7 +277,7 @@ class RoleAddChildView(views.AjaxFormViewMixin, views.TitleMixin,
     form_class = forms.RolesForm
     success_url = '..'
     template_name = 'authentic2/manager/form.html'
-    permissions = ['a2_rbac.change_role']
+    permissions = ['a2_rbac.manage_members_role']
 
     def dispatch(self, request, *args, **kwargs):
         self.object = self.get_object()
@@ -325,7 +325,7 @@ class RoleRemoveChildView(views.AjaxFormViewMixin, SingleObjectMixin,
     model = get_role_model()
     success_url = '../..'
     template_name = 'authentic2/manager/role_remove_child.html'
-    permissions = ['a2_rbac.change_role']
+    permissions = ['a2_rbac.manage_members_role']
 
     def dispatch(self, request, *args, **kwargs):
         self.object = self.get_object()
@@ -366,7 +366,7 @@ class RoleRemoveParentView(views.AjaxFormViewMixin, SingleObjectMixin,
         return ctx
 
     def post(self, request, *args, **kwargs):
-        if not self.request.user.has_perm('a2_rbac.change_role', self.parent):
+        if not self.request.user.has_perm('a2_rbac.manage_members_role', self.parent):
             raise PermissionDenied
         self.object.remove_parent(self.parent)
         hooks.call_hooks('event', name='manager-remove-child-role', user=self.request.user,
diff --git a/src/authentic2/manager/templates/authentic2/manager/role_members.html b/src/authentic2/manager/templates/authentic2/manager/role_members.html
index de5ca308..340eb6c4 100644
--- a/src/authentic2/manager/templates/authentic2/manager/role_members.html
+++ b/src/authentic2/manager/templates/authentic2/manager/role_members.html
@@ -52,8 +52,8 @@
 
  {% 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">
+ {% if view.can_manage_members %}
+   <form method="post" class="manager-m2m-add-form" id="add-user">
            {% csrf_token %}
            {{ form }}
            <button>{% trans "Add" %}</button>
@@ -103,7 +103,7 @@
        <a title="{% trans "Indirect child role" %}" class="disabled role-remove icon-minus-sign"></a>
      {% endif %}
    {% endfor %}
-  {% if view.can_change %}
+  {% if view.can_manage_members %}
     <a rel="popup" href="{% url "a2-manager-role-add-child" pk=object.pk %}" class="role-add icon-add-sign"></a>
   {% else %}
     <a title="{% trans "Permission denied" %}" class="disabled role-add icon-add-sign"></a>
diff --git a/src/authentic2/manager/templates/authentic2/manager/role_members_table.html b/src/authentic2/manager/templates/authentic2/manager/role_members_table.html
index cffdaa1e..4a5b63cb 100644
--- a/src/authentic2/manager/templates/authentic2/manager/role_members_table.html
+++ b/src/authentic2/manager/templates/authentic2/manager/role_members_table.html
@@ -7,6 +7,6 @@
     <th></th>
   {% endblock %}
   {% block table.tbody.last.column %}
-  <td>{% if table.context.view.can_change and row.record.direct %}<a class="icon-remove-sign js-remove-object" data-confirm="{% blocktrans with username=row.record.get_full_name role=table.context.object %}Do you really want to remove user &quot;{{ username }}&quot; from role &quot;{{ role }}&quot;&nbsp;?{% endblocktrans %}" href="#" data-pk-arg="user"></a>{% endif %}</td>
+  <td>{% if table.context.view.can_manage_members and row.record.direct %}<a class="icon-remove-sign js-remove-object" data-confirm="{% blocktrans with username=row.record.get_full_name role=table.context.object %}Do you really want to remove user &quot;{{ username }}&quot; from role &quot;{{ role }}&quot;&nbsp;?{% endblocktrans %}" href="#" data-pk-arg="user"></a>{% endif %}</td>
   {% endblock %}
 {% endif %}
diff --git a/src/authentic2/manager/user_views.py b/src/authentic2/manager/user_views.py
index 5faebe90..4e0411b9 100644
--- a/src/authentic2/manager/user_views.py
+++ b/src/authentic2/manager/user_views.py
@@ -553,7 +553,7 @@ class UserRolesView(HideOUColumnMixin, BaseSubTableView):
             qs = qs.prefetch_related(models.Prefetch(
                 'members', queryset=User.objects.filter(pk=self.object.pk),
                 to_attr='member'))
-            qs2 = self.request.user.filter_by_perm('a2_rbac.change_role', qs)
+            qs2 = self.request.user.filter_by_perm('a2_rbac.manage_members_role', qs)
             managable_ids = map(str, qs2.values_list('pk', flat=True))
             qs = qs.extra(select={'has_perm': 'a2_rbac_role.id in (%s)' % ', '.join(managable_ids)})
             qs = qs.exclude(slug__startswith='_a2-managers-of-role')
@@ -578,7 +578,7 @@ class UserRolesView(HideOUColumnMixin, BaseSubTableView):
         user = self.object
         role = form.cleaned_data['role']
         action = form.cleaned_data['action']
-        if self.request.user.has_perm('a2_rbac.change_role', role):
+        if self.request.user.has_perm('a2_rbac.manage_members_role', role):
             if action == 'add':
                 if user.roles.filter(pk=role.pk):
                     messages.warning(
diff --git a/src/authentic2/manager/views.py b/src/authentic2/manager/views.py
index 6c7a4a30..f994a8d8 100644
--- a/src/authentic2/manager/views.py
+++ b/src/authentic2/manager/views.py
@@ -115,14 +115,11 @@ class PermissionMixin(object):
                          or (hasattr(self, 'slug_url_kwarg')
                              and self.slug_url_kwarg in self.kwargs)):
                 self.object = self.get_object()
-                view_perm = '%s.view_%s' % (app_label, model_name)
-                change_perm = '%s.change_%s' % (app_label, model_name)
-                delete_perm = '%s.delete_%s' % (app_label, model_name)
-                self.can_view = request.user.has_perm(view_perm, self.object)
-                self.can_change = request.user.has_perm(change_perm,
-                                                        self.object)
-                self.can_delete = request.user.has_perm(delete_perm,
-                                                        self.object)
+                permissions = ('view', 'change', 'delete', 'manage_members')
+                for permission in permissions:
+                    perm = '%s.%s_%s' % (app_label, permission, model_name)
+                    setattr(self, 'can_' + permission,
+                            request.user.has_perm(perm, self.object))
                 if self.permissions \
                         and not request.user.has_perms(
                             self.permissions, self.object):
diff --git a/tests/test_manager.py b/tests/test_manager.py
index 518b289f..2e9bd47a 100644
--- a/tests/test_manager.py
+++ b/tests/test_manager.py
@@ -25,8 +25,11 @@ from webtest import Upload
 
 from authentic2.a2_rbac.utils import get_default_ou
 
-from django_rbac.utils import get_ou_model, get_role_model
+from django_rbac.utils import (get_ou_model, get_role_model,
+                               get_permission_model, get_operation)
+from django_rbac.models import VIEW_OP
 from django.contrib.auth import get_user_model
+from django.contrib.contenttypes.models import ContentType
 from django.utils.six.moves.urllib.parse import urlparse
 from utils import login, get_link_from_mail
 
@@ -896,3 +899,82 @@ 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, admin, simple_role):
+    admin_role = simple_role.get_admin_role()
+    simple_user.roles.add(admin_role)
+    login(app, simple_user, '/manage/')
+
+    # user can view users
+    response = app.get('/manage/users/')
+    q = response.pyquery.remove_namespaces()
+    assert len(q('table tbody tr')) == 2
+    assert set([e.text for e in q('table tbody td.username')]) == {'admin', 'user'}
+
+    # user can view administered roles
+    response = app.get('/manage/roles/')
+    q = response.pyquery.remove_namespaces()
+    assert len(q('table tbody tr')) == 1
+    assert q('table tbody td.name').text() == 'simple role'
+
+    # user can add members
+    response = app.get('/manage/roles/%s/' % simple_role.pk)
+    form = response.forms['add-user']
+    form['user'].force_value(admin.pk)
+    response = form.submit().follow()
+    assert simple_role in admin.roles.all()
+
+    # user can delete members
+    q = response.pyquery.remove_namespaces()
+    assert q('table tbody tr td .icon-remove-sign')
+    token = str(response.context['csrf_token'])
+    params = {'action': 'remove', 'user': admin.pk, 'csrfmiddlewaretoken': token}
+    app.post('/manage/roles/%s/' % simple_role.pk, params=params)
+    assert simple_role not in admin.roles.all()
+
+    # user can act on role inheritance
+    role = Role.objects.create(name='test_role')
+    view_role_perm = get_permission_model().objects.create(
+        operation=get_operation(VIEW_OP), ou=role.ou,
+        target_ct=ContentType.objects.get_for_model(Role),
+        target_id=role.pk)
+    simple_role.permissions.add(view_role_perm)
+    simple_user.roles.add(simple_role)
+    admin.roles.add(role)
+    response = app.get('/manage/roles/%s/add-child/' % simple_role.pk)
+    form = response.form
+    form['roles'].force_value(role.pk)
+    form.submit().follow()
+    assert role in simple_role.children()
+
+    url = '/manage/roles/%s/remove-child/%s/' % (simple_role.pk, role.pk)
+    token = str(response.context['csrf_token'])
+    app.post(url, params={'csrfmiddlewaretoken': token})
+    assert not role in simple_role.children()
+
+    response = app.get('/manage/roles/%s/add-parent/' % role.pk)
+    form = response.form
+    form['roles'].force_value(simple_role.pk)
+    form.submit().follow()
+    assert simple_role in role.parents()
+
+    url = '/manage/roles/%s/remove-parent/%s/' % (role.pk, simple_role.pk)
+    token = str(response.context['csrf_token'])
+    app.post(url, params={'csrfmiddlewaretoken': token})
+    assert not simple_role in role.parents()
+
+    # user roles view works
+    response = app.get('/manage/users/%s/roles/' % admin.pk)
+    q = response.pyquery.remove_namespaces()
+    assert len(q('table tbody tr')) == 1
+    assert q('table tbody td.name').text() == 'simple role'
+
+    token = str(response.context['csrf_token'])
+    params = {'action': 'add', 'role': simple_role.pk, 'csrfmiddlewaretoken': token}
+    response = app.post('/manage/users/%s/roles/' % admin.pk, params=params)
+    assert simple_role in admin.roles.all()
+
+    app.get('/manage/roles/add/', status=403)
+    app.get('/manage/roles/%s/edit/' % simple_role.pk, status=403)
+    app.get('/manage/roles/%s/delete/' % simple_role.pk, status=403)
-- 
2.20.1

