From 548b316f59ecdbce2b18af4ed3b7dab32648720c Mon Sep 17 00:00:00 2001 From: Valentin Deniaud 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 5ed9932e..b5a3cb64 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 %} -
+ {% if view.can_manage_members %} + {% csrf_token %} {{ form }} @@ -103,7 +103,7 @@ {% endif %} {% endfor %} - {% if view.can_change %} + {% if view.can_manage_members %} {% else %} 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 @@ {% endblock %} {% block table.tbody.last.column %} - {% if table.context.view.can_change and row.record.direct %}{% endif %} + {% if table.context.view.can_manage_members and row.record.direct %}{% endif %} {% endblock %} {% endif %} diff --git a/src/authentic2/manager/user_views.py b/src/authentic2/manager/user_views.py index 64fe239d..83641b05 100644 --- a/src/authentic2/manager/user_views.py +++ b/src/authentic2/manager/user_views.py @@ -554,7 +554,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') @@ -579,7 +579,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 84d78c60..825b5dee 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 7452b80a..82d34fca 100644 --- a/tests/test_manager.py +++ b/tests/test_manager.py @@ -26,8 +26,11 @@ from webtest import Upload from authentic2.a2_rbac.utils import get_default_ou from authentic2.validators import EmailValidator -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.encoding import force_bytes from django.utils.six.moves.urllib.parse import urlparse from utils import login, get_link_from_mail @@ -935,3 +938,82 @@ def test_manager_ajax_form_view_mixin_response(superuser_or_admin, app): resp = app.get('/manage/roles/add/', xhr=True, status=200) assert resp.content_type == 'application/json' assert resp.json['content'] + + +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