From 4b2f527202f94dc1b91ae80b3949636fe8fd47bf Mon Sep 17 00:00:00 2001 From: Valentin Deniaud Date: Mon, 26 Jul 2021 16:03:12 +0200 Subject: [PATCH 2/2] manager: add separate role inheritance table views (#53481) --- src/authentic2/manager/forms.py | 6 +- src/authentic2/manager/role_views.py | 138 +++++++++++++----- src/authentic2/manager/tables.py | 21 +++ .../authentic2/manager/role_members.html | 4 +- .../authentic2/manager/roles_inheritance.html | 25 ++++ .../authentic2/manager/user_ou_roles.html | 2 +- tests/test_manager.py | 135 ++++++++--------- 7 files changed, 217 insertions(+), 114 deletions(-) create mode 100644 src/authentic2/manager/templates/authentic2/manager/roles_inheritance.html diff --git a/src/authentic2/manager/forms.py b/src/authentic2/manager/forms.py index 996b5846..7abf9e87 100644 --- a/src/authentic2/manager/forms.py +++ b/src/authentic2/manager/forms.py @@ -133,12 +133,12 @@ class UsersForm(LimitQuerysetFormMixin, CssClass, forms.Form): class RolesForm(LimitQuerysetFormMixin, CssClass, forms.Form): - roles = fields.ChooseRolesField(label=_('Add some roles')) -class RoleParentsForm(LimitQuerysetFormMixin, CssClass, forms.Form): - roles = fields.ChooseManageableMemberRolesField(label=_('Add some roles')) +class RoleParentForm(LimitQuerysetFormMixin, CssClass, forms.Form): + role = fields.ChooseManageableMemberRoleField(label=_('Add some roles')) + action = forms.CharField(initial='add', widget=forms.HiddenInput) class ChooseUserRoleForm(LimitQuerysetFormMixin, CssClass, forms.Form): diff --git a/src/authentic2/manager/role_views.py b/src/authentic2/manager/role_views.py index 286b7d0d..fefc898c 100644 --- a/src/authentic2/manager/role_views.py +++ b/src/authentic2/manager/role_views.py @@ -21,7 +21,8 @@ from django.contrib.auth import get_user_model from django.contrib.contenttypes.models import ContentType from django.core.exceptions import PermissionDenied, ValidationError from django.db import transaction -from django.db.models import Count, F +from django.db.models import BooleanField, Count, ExpressionWrapper, F, Prefetch +from django.db.models.functions import Cast from django.db.models.query import Prefetch, Q from django.shortcuts import get_object_or_404 from django.urls import reverse @@ -34,7 +35,7 @@ from authentic2 import data_transfer, hooks from authentic2.apps.journal.views import JournalViewWithContext from authentic2.forms.profile import modelform_factory from authentic2.utils.misc import redirect -from django_rbac.utils import get_ou_model, get_permission_model, get_role_model +from django_rbac.utils import get_ou_model, get_permission_model, get_role_model, get_role_parenting_model from . import app_settings, forms, resources, tables, views from .journal_views import BaseJournalView @@ -365,64 +366,129 @@ class RoleMembersExportView(views.ExportMixin, RoleMembersView): members_export = RoleMembersExportView.as_view() -class RoleAddChildView( - views.AjaxFormViewMixin, - views.TitleMixin, - views.PermissionMixin, - views.FormNeedsRequest, - SingleObjectMixin, - FormView, -): +class RoleAddChildView(RoleViewMixin, views.HideOUColumnMixin, views.BaseSubTableView): title = _('Add child role') - model = get_role_model() - form_class = forms.RolesForm - success_url = '..' - template_name = 'authentic2/manager/form.html' + form_class = forms.ChooseRoleForm + table_class = tables.InheritanceRolesTable + search_form_class = forms.RoleSearchForm + template_name = 'authentic2/manager/roles_inheritance.html' permissions = ['a2_rbac.manage_members_role'] + success_url = '.' + slug_field = 'uuid' - def dispatch(self, request, *args, **kwargs): - self.object = self.get_object() - return super().dispatch(request, *args, **kwargs) + def get_table_queryset(self): + qs = super().get_table_queryset() + qs = qs.exclude(pk=self.object.pk) + children = self.object.children(annotate=True, include_self=False) + children = children.annotate(is_direct=Cast('direct', output_field=BooleanField())) + qs = qs.annotate( + checked=ExpressionWrapper(Q(pk__in=children.filter(is_direct=True)), output_field=BooleanField()) + ) + qs = qs.annotate( + indeterminate=ExpressionWrapper( + Q(pk__in=children.filter(is_direct=False)), output_field=BooleanField() + ) + ) + RoleParenting = get_role_parenting_model() + rp_qs = RoleParenting.objects.filter(parent__in=children).annotate(name=F('parent__name')) + qs = qs.prefetch_related(Prefetch('parent_relation', queryset=rp_qs, to_attr='via')) + return qs def form_valid(self, form): - parent = self.get_object() - for role in form.cleaned_data['roles']: - parent.add_child(role) + role = form.cleaned_data['role'] + action = form.cleaned_data['action'] + if action == 'add': + self.object.add_child(role) hooks.call_hooks( - 'event', name='manager-add-child-role', user=self.request.user, parent=parent, child=role + 'event', name='manager-add-child-role', user=self.request.user, parent=self.object, child=role ) - self.request.journal.record('manager.role.inheritance.addition', parent=parent, child=role) + self.request.journal.record('manager.role.inheritance.addition', parent=self.object, child=role) + elif action == 'remove': + self.object.remove_child(role) + hooks.call_hooks( + 'event', + name='manager-remove-child-role', + user=self.request.user, + parent=self.object, + child=role, + ) + self.request.journal.record('manager.role.inheritance.removal', parent=self.object, child=role) return super().form_valid(form) + def get_search_form_kwargs(self): + kwargs = super().get_search_form_kwargs() + kwargs['show_all_ou'] = app_settings.SHOW_ALL_OU + kwargs['queryset'] = self.request.user.filter_by_perm( + 'a2_rbac.view_role', get_role_model().objects.all() + ) + return kwargs + add_child = RoleAddChildView.as_view() -class RoleAddParentView( - views.AjaxFormViewMixin, views.TitleMixin, views.FormNeedsRequest, SingleObjectMixin, FormView -): +class RoleAddParentView(RoleViewMixin, views.HideOUColumnMixin, views.BaseSubTableView): title = _('Add parent role') - model = get_role_model() - form_class = forms.RoleParentsForm - success_url = '..' - template_name = 'authentic2/manager/form.html' + form_class = forms.RoleParentForm + table_class = tables.InheritanceRolesTable + search_form_class = forms.RoleSearchForm + template_name = 'authentic2/manager/roles_inheritance.html' + success_url = '.' + slug_field = 'uuid' def dispatch(self, request, *args, **kwargs): - self.object = self.get_object() - if self.object.is_internal(): + if self.get_object().is_internal(): raise PermissionDenied return super().dispatch(request, *args, **kwargs) + def get_table_queryset(self): + qs = super().get_table_queryset() + qs = self.request.user.filter_by_perm('a2_rbac.manage_members_role', qs) + qs = qs.exclude(pk=self.object.pk) + parents = self.object.parents(annotate=True, include_self=False) + parents = parents.annotate(is_direct=Cast('direct', output_field=BooleanField())) + qs = qs.annotate( + checked=ExpressionWrapper(Q(pk__in=parents.filter(is_direct=True)), output_field=BooleanField()) + ) + qs = qs.annotate( + indeterminate=ExpressionWrapper( + Q(pk__in=parents.filter(is_direct=False)), output_field=BooleanField() + ) + ) + RoleParenting = get_role_parenting_model() + rp_qs = RoleParenting.objects.filter(child__in=parents).annotate(name=F('child__name')) + qs = qs.prefetch_related(Prefetch('child_relation', queryset=rp_qs, to_attr='via')) + return qs + def form_valid(self, form): - child = self.get_object() - for role in form.cleaned_data['roles']: - child.add_parent(role) + role = form.cleaned_data['role'] + action = form.cleaned_data['action'] + if action == 'add': + self.object.add_parent(role) + hooks.call_hooks( + 'event', name='manager-add-child-role', user=self.request.user, parent=role, child=self.object + ) + self.request.journal.record('manager.role.inheritance.addition', parent=role, child=self.object) + elif action == 'remove': + self.object.remove_parent(role) hooks.call_hooks( - 'event', name='manager-add-child-role', user=self.request.user, parent=role, child=child + 'event', + name='manager-remove-child-role', + user=self.request.user, + parent=role, + child=self.object, ) - self.request.journal.record('manager.role.inheritance.addition', parent=role, child=child) + self.request.journal.record('manager.role.inheritance.removal', parent=role, child=self.object) return super().form_valid(form) + def get_search_form_kwargs(self): + kwargs = super().get_search_form_kwargs() + kwargs['show_all_ou'] = app_settings.SHOW_ALL_OU + kwargs['queryset'] = self.request.user.filter_by_perm( + 'a2_rbac.manage_members_role', get_role_model().objects.all() + ) + return kwargs + add_parent = RoleAddParentView.as_view() diff --git a/src/authentic2/manager/tables.py b/src/authentic2/manager/tables.py index 930d751d..5af52638 100644 --- a/src/authentic2/manager/tables.py +++ b/src/authentic2/manager/tables.py @@ -240,3 +240,24 @@ class UserAuthorizationsTable(tables.Table): attrs = {'class': 'main plaintable', 'id': 'user-authorizations-table'} fields = ('client', 'created', 'expired') empty_text = _('This user has not granted profile data access to any service yet.') + + +class InheritanceRolesTable(tables.Table): + name = tables.LinkColumn( + viewname='a2-manager-role-members', kwargs={'pk': A('pk')}, accessor='name', verbose_name=_('label') + ) + via = tables.TemplateColumn( + '''{% for rel in record.via %}{{ rel.name }} {% if not forloop.last %}, {% endif %}{% endfor %}''', + verbose_name=_('Inherited from'), + orderable=False, + ) + member = tables.TemplateColumn( + '', + verbose_name='', + ) + + class Meta: + model = get_role_model() + attrs = {'class': 'main plaintable', 'id': 'inheritance-role-table'} + fields = ('name', 'ou') + empty_text = _('None') diff --git a/src/authentic2/manager/templates/authentic2/manager/role_members.html b/src/authentic2/manager/templates/authentic2/manager/role_members.html index b6594bf4..fff8cea7 100644 --- a/src/authentic2/manager/templates/authentic2/manager/role_members.html +++ b/src/authentic2/manager/templates/authentic2/manager/role_members.html @@ -116,7 +116,7 @@ {% endif %} {% endfor %} {% if view.can_manage_members %} - + {% else %} {% endif %} @@ -138,7 +138,7 @@ {% endif %} {% endfor %} {% if not object.is_internal %} - + {% else %} {% endif %} diff --git a/src/authentic2/manager/templates/authentic2/manager/roles_inheritance.html b/src/authentic2/manager/templates/authentic2/manager/roles_inheritance.html new file mode 100644 index 00000000..7d7d9dd8 --- /dev/null +++ b/src/authentic2/manager/templates/authentic2/manager/roles_inheritance.html @@ -0,0 +1,25 @@ +{% extends "authentic2/manager/role_common.html" %} +{% load i18n static django_tables2 %} + +{% block breadcrumb %} + {{ block.super }} + {{ object }} + {% trans "Role inheritance" %} +{% endblock %} + +{% block extrascripts %} + {{ block.super }} + +{% endblock %} + +{% block main %} + {% with row_link=0 %} + {% render_table table "authentic2/manager/table.html" %} + {% endwith %} +{% endblock %} + +{% block sidebar %} + +{% endblock %} diff --git a/src/authentic2/manager/templates/authentic2/manager/user_ou_roles.html b/src/authentic2/manager/templates/authentic2/manager/user_ou_roles.html index 6225e476..f73d20d2 100644 --- a/src/authentic2/manager/templates/authentic2/manager/user_ou_roles.html +++ b/src/authentic2/manager/templates/authentic2/manager/user_ou_roles.html @@ -1,5 +1,5 @@ {% extends "authentic2/manager/user_common_roles.html" %} -{% load django_tables2 %} +{% load static django_tables2 %} {% block extrascripts %} {{ block.super }} diff --git a/tests/test_manager.py b/tests/test_manager.py index d282d6fd..e6c1c1f9 100644 --- a/tests/test_manager.py +++ b/tests/test_manager.py @@ -861,15 +861,15 @@ def test_roles_widget(admin, app, db): def test_roles_for_change_widget(admin, app, db): - from authentic2.manager.forms import RoleParentsForm + from authentic2.manager.forms import RoleParentForm login(app, admin, '/manage/') Role.objects.create(name='admin 1') Role.objects.create(name='user 1') - form = RoleParentsForm(request=None) + form = RoleParentForm(request=None) assert form.as_p() - field_id = form.fields['roles'].widget.build_attrs({})['data-field_id'] + field_id = form.fields['role'].widget.build_attrs({})['data-field_id'] url = reverse('django_select2-json') response = app.get(url, params={'field_id': field_id, 'term': 'admin'}) assert len(response.json['results']) == 1 @@ -940,23 +940,34 @@ def test_manager_role_admin_permissions(app, simple_user, admin, simple_role): 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() + token = str(response.context['csrf_token']) + params = {'action': 'add', 'role': role.pk, 'csrfmiddlewaretoken': token} + response = app.post('/manage/roles/%s/add-child/' % simple_role.pk, params=params) assert role in simple_role.children() + response = app.get('/manage/roles/%s/' % simple_role.pk) 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() + token = str(response.context['csrf_token']) + params = {'action': 'add', 'role': simple_role.pk, 'csrfmiddlewaretoken': token} + response = app.post('/manage/roles/%s/add-parent/' % role.pk, params=params) assert simple_role in role.parents() + # try to add arbitrary role + admin_role = Role.objects.get(slug='_a2-manager') + response = app.get('/manage/roles/%s/add-parent/' % role.pk) + token = str(response.context['csrf_token']) + params = {'action': 'add', 'role': admin_role.pk, 'csrfmiddlewaretoken': token} + response = app.post('/manage/roles/%s/add-parent/' % simple_role.pk, params=params) + assert admin_role not in role.parents() + + response = app.get('/manage/roles/%s/' % simple_role.pk) url = '/manage/roles/%s/remove-parent/%s/' % (role.pk, simple_role.pk) token = str(response.context['csrf_token']) app.post(url, params={'csrfmiddlewaretoken': token}) @@ -978,33 +989,13 @@ def test_manager_role_admin_permissions(app, simple_user, admin, simple_role): app.get('/manage/roles/%s/delete/' % simple_role.pk, status=403) -def test_manager_permission_inheritance(app, simple_user, admin, simple_role): - admin_role = Role.objects.get(slug='_a2-manager') - view_role_perm = get_permission_model().objects.create( - operation=get_operation(VIEW_OP), - target_ct=ContentType.objects.get_for_model(Role), - target_id=simple_role.pk, - ) - simple_role.permissions.add(view_role_perm) - simple_user.roles.add(simple_role) - login(app, simple_user, '/manage/') - - response = app.get('/manage/roles/%s/add-parent/' % simple_role.pk) - form = response.form - form['roles'].force_value(admin_role.pk) - response = form.submit() - - assert response.status_code == 200 - assert not admin_role in simple_role.parents() - - def test_manager_widget_fields_validation(app, simple_user, simple_role): '''Verify that fields corresponding to widget implement queryset restrictions.''' from authentic2.manager.forms import ( ChooseRoleForm, ChooseUserForm, ChooseUserRoleForm, - RoleParentsForm, + RoleParentForm, RolesForm, UsersForm, ) @@ -1056,8 +1047,8 @@ def test_manager_widget_fields_validation(app, simple_user, simple_role): assert error_message in form.errors['roles'][0] # For those we need manage_members permission - form = RoleParentsForm(request=request, data={'roles': [visible_role.pk]}) - assert error_message in form.errors['roles'][0] + form = RoleParentForm(request=request, data={'role': visible_role.pk, 'action': 'add'}) + assert error_message in form.errors['role'][0] form = ChooseUserRoleForm(request=request, data={'role': visible_role.pk, 'action': 'add'}) assert error_message in form.errors['role'][0] @@ -1070,66 +1061,66 @@ def test_manager_widget_fields_validation(app, simple_user, simple_role): simple_role.permissions.add(change_role_perm) del simple_user._rbac_perms_cache - form = RoleParentsForm(request=request, data={'roles': [visible_role.pk]}) + form = RoleParentForm(request=request, data={'role': visible_role.pk, 'action': 'add'}) assert form.is_valid() form = ChooseUserRoleForm(request=request, data={'role': visible_role.pk, 'action': 'add'}) assert form.is_valid() -def test_manager_role_widgets_choices(app, simple_user, simple_role): - def get_choices(response): - select2_json = request_select2(app, response) - assert select2_json['more'] is False - return {result['id'] for result in select2_json['results']} - +def test_manager_role_inheritance_list(app, simple_user, simple_role, ou1): visible_role = Role.objects.create(name='visible_role', ou=simple_user.ou) + visible_role_2 = Role.objects.create(name='visible_role_2', ou=ou1) Role.objects.create(name='invisible_role', ou=simple_user.ou) admin_of_simple_role = simple_role.get_admin_role() admin_of_simple_role.members.add(simple_user) - view_role_perm = get_permission_model().objects.create( - operation=get_operation(VIEW_OP), - target_ct=ContentType.objects.get_for_model(Role), - target_id=visible_role.pk, - ) - simple_role.permissions.add(view_role_perm) + for role in (visible_role, visible_role_2): + view_role_perm = get_permission_model().objects.create( + operation=get_operation(VIEW_OP), + 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) response = login(app, simple_user, '/manage/roles/') - # all visible roles are shown + # all visible roles are shown, except current role response = app.get('/manage/roles/%s/add-child/' % simple_role.pk) - assert {visible_role.pk, simple_role.pk} == get_choices(response) - - # all roles with manage_members permissions are shown - response = app.get('/manage/roles/%s/add-parent/' % simple_role.pk) - assert {simple_role.pk, admin_of_simple_role.pk} == get_choices(response) - - response = app.get('/manage/roles/%s/add-parent/' % visible_role.pk) - assert {simple_role.pk, admin_of_simple_role.pk} == get_choices(response) - - -def test_manager_widgets_field_id_other_user(app, admin, simple_user, simple_role): - other_role = Role.objects.create(name='visible_role', ou=simple_user.ou) - simple_role.get_admin_role().members.add(simple_user) + q = response.pyquery.remove_namespaces() + assert len(q('table tbody tr')) == 2 + assert {e.text_content() for e in q('table tbody td.name')} == {visible_role.name, visible_role_2.name} - response = login(app, admin, '/manage/roles/%s/add-child/' % simple_role.pk) - select2_json = request_select2(app, response) - assert select2_json['more'] is False + # filter by ou + response.form['search-ou'] = ou1.pk + response = response.form.submit() + q = response.pyquery.remove_namespaces() + assert len(q('table tbody tr')) == 1 + assert {e.text_content() for e in q('table tbody td.name')} == {visible_role_2.name} - # admin can see every roles - assert {simple_role.pk, other_role.pk} == {result['id'] for result in select2_json['results']} + # filter by name + response.form['search-text'] = '2' + response.form['search-ou'] = 'all' + response = response.form.submit() + q = response.pyquery.remove_namespaces() + assert len(q('table tbody tr')) == 1 + assert {e.text_content() for e in q('table tbody td.name')} == {visible_role_2.name} - login(app, simple_user) - # same request from the page served for admin - select2_json = request_select2(app, response) - # simple_user doesn't see all roles - assert simple_role.pk == select2_json['results'][0]['id'] + # all roles with manage_members permissions are shown + response = app.get('/manage/roles/%s/add-parent/' % visible_role.pk) + q = response.pyquery.remove_namespaces() + assert len(q('table tbody tr')) == 1 + assert {e.text_content() for e in q('table tbody td.name')} == {simple_role.name} - # anymous user receive 404 - app.session.flush() - select2_json = request_select2(app, response, get_kwargs={'status': 404}) + response.form['search-internals'] = True + response = response.form.submit() + q = response.pyquery.remove_namespaces() + assert len(q('table tbody tr')) == 2 + assert {e.text_content() for e in q('table tbody td.name')} == { + simple_role.name, + admin_of_simple_role.name, + } def test_display_parent_roles_on_role_page(app, superuser, settings): -- 2.20.1