From 14db0eb1c9fb975306846a9ec90000c5f23ed016 Mon Sep 17 00:00:00 2001 From: Valentin Deniaud Date: Wed, 26 Oct 2022 14:40:15 +0200 Subject: [PATCH] authenticators: add manager role (#66984) --- src/authentic2/a2_rbac/management.py | 4 + .../apps/authenticators/manager_urls.py | 137 +++++++----------- src/authentic2/apps/authenticators/views.py | 9 +- src/authentic2/manager/urls.py | 4 +- src/authentic2/manager/views.py | 2 +- tests/test_a2_rbac.py | 30 ++-- tests/test_manager.py | 29 ++-- tests/test_manager_authenticators.py | 27 +++- tests/test_role_manager.py | 9 +- 9 files changed, 129 insertions(+), 122 deletions(-) diff --git a/src/authentic2/a2_rbac/management.py b/src/authentic2/a2_rbac/management.py index 85b40a985..79ae1c43c 100644 --- a/src/authentic2/a2_rbac/management.py +++ b/src/authentic2/a2_rbac/management.py @@ -91,6 +91,10 @@ MANAGED_CT = { 'name': _('Manager of services'), 'scoped_name': _('Services - {ou}'), }, + ('authenticators', 'baseauthenticator'): { + 'name': _('Manager of authenticators'), + 'scoped_name': _('Authenticators - {ou}'), + }, } diff --git a/src/authentic2/apps/authenticators/manager_urls.py b/src/authentic2/apps/authenticators/manager_urls.py index 3a27ec862..7586e819f 100644 --- a/src/authentic2/apps/authenticators/manager_urls.py +++ b/src/authentic2/apps/authenticators/manager_urls.py @@ -14,89 +14,62 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . -from django.contrib.auth.decorators import user_passes_test -from django.core.exceptions import PermissionDenied from django.urls import path -from django.utils.functional import lazy - -from authentic2.decorators import required -from authentic2.utils import misc as utils_misc from . import views - -def superuser_required(function, login_url): - def check_superuser(user): - if user and user.is_superuser: - return True - if user and not user.is_anonymous: - raise PermissionDenied() - return False - - actual_decorator = user_passes_test(check_superuser, login_url=login_url) - return actual_decorator(function) - - -def superuser_login_required(func): - return superuser_required(func, login_url=lazy(utils_misc.get_manager_login_url, str)()) - - -urlpatterns = required( - superuser_login_required, - [ - # Authenticators - path('authenticators/', views.authenticators, name='a2-manager-authenticators'), - path( - 'authenticators/add/', - views.add, - name='a2-manager-authenticator-add', - ), - path( - 'authenticators//detail/', - views.detail, - name='a2-manager-authenticator-detail', - ), - path( - 'authenticators//edit/', - views.edit, - name='a2-manager-authenticator-edit', - ), - path( - 'authenticators//delete/', - views.delete, - name='a2-manager-authenticator-delete', - ), - path( - 'authenticators//toggle/', - views.toggle, - name='a2-manager-authenticator-toggle', - ), - path( - 'authenticators//journal/', - views.journal, - name='a2-manager-authenticator-journal', - ), - path('authenticators//export/', views.export_json, name='a2-manager-authenticator-export'), - path('authenticators/import/', views.import_json, name='a2-manager-authenticator-import'), - path( - 'authenticators/order/', - views.order, - name='a2-manager-authenticators-order', - ), - path( - 'authenticators///add/', - views.add_related_object, - name='a2-manager-authenticators-add-related-object', - ), - path( - 'authenticators////edit/', - views.edit_related_object, - name='a2-manager-authenticators-edit-related-object', - ), - path( - 'authenticators////delete/', - views.delete_related_object, - name='a2-manager-authenticators-delete-related-object', - ), - ], -) +urlpatterns = [ + path('authenticators/', views.authenticators, name='a2-manager-authenticators'), + path( + 'authenticators/add/', + views.add, + name='a2-manager-authenticator-add', + ), + path( + 'authenticators//detail/', + views.detail, + name='a2-manager-authenticator-detail', + ), + path( + 'authenticators//edit/', + views.edit, + name='a2-manager-authenticator-edit', + ), + path( + 'authenticators//delete/', + views.delete, + name='a2-manager-authenticator-delete', + ), + path( + 'authenticators//toggle/', + views.toggle, + name='a2-manager-authenticator-toggle', + ), + path( + 'authenticators//journal/', + views.journal, + name='a2-manager-authenticator-journal', + ), + path('authenticators//export/', views.export_json, name='a2-manager-authenticator-export'), + path('authenticators/import/', views.import_json, name='a2-manager-authenticator-import'), + path( + 'authenticators/order/', + views.order, + name='a2-manager-authenticators-order', + ), + path( + 'authenticators///add/', + views.add_related_object, + name='a2-manager-authenticators-add-related-object', + ), + path( + 'authenticators////edit/', + views.edit_related_object, + name='a2-manager-authenticators-edit-related-object', + ), + path( + 'authenticators////delete/', + views.delete_related_object, + name='a2-manager-authenticators-delete-related-object', + ), +] diff --git a/src/authentic2/apps/authenticators/views.py b/src/authentic2/apps/authenticators/views.py index 8d7293499..9209ddb9a 100644 --- a/src/authentic2/apps/authenticators/views.py +++ b/src/authentic2/apps/authenticators/views.py @@ -32,14 +32,15 @@ from django.views.generic.list import ListView from authentic2.apps.journal.views import JournalViewWithContext from authentic2.manager.journal_views import BaseJournalView -from authentic2.manager.views import MediaMixin, TitleMixin +from authentic2.manager.views import MediaMixin, PermissionMixin, TitleMixin from . import forms from .models import AuthenticatorImportError, BaseAuthenticator -class AuthenticatorsMixin(MediaMixin, TitleMixin): +class AuthenticatorsMixin(MediaMixin, TitleMixin, PermissionMixin): model = BaseAuthenticator + permissions = ['authenticators.search_baseauthenticator'] def get_queryset(self): return self.model.authenticators.all() @@ -268,7 +269,9 @@ class AuthenticatorsOrderView(AuthenticatorsMixin, FormView): order = AuthenticatorsOrderView.as_view() -class AuthenticatorRelatedObjectMixin(MediaMixin, TitleMixin): +class AuthenticatorRelatedObjectMixin(MediaMixin, TitleMixin, PermissionMixin): + permissions = ['authenticators.search_baseauthenticator'] + def dispatch(self, request, *args, **kwargs): self.authenticator = get_object_or_404( BaseAuthenticator.authenticators.all(), pk=kwargs.get('authenticator_pk') diff --git a/src/authentic2/manager/urls.py b/src/authentic2/manager/urls.py index 3c1c6160a..c5af9b762 100644 --- a/src/authentic2/manager/urls.py +++ b/src/authentic2/manager/urls.py @@ -207,10 +207,10 @@ urlpatterns = required( path('api-clients//', apiclient_views.detail, name='a2-manager-api-client-detail'), path('api-clients//edit/', apiclient_views.edit, name='a2-manager-api-client-edit'), path('api-clients//delete/', apiclient_views.delete, name='a2-manager-api-client-delete'), - ], + ] + + authenticator_urlpatterns, ) -urlpatterns += authenticator_urlpatterns urlpatterns += oidc_manager_urlpatterns urlpatterns += [ diff --git a/src/authentic2/manager/views.py b/src/authentic2/manager/views.py index 674f6db96..dbd36ff64 100644 --- a/src/authentic2/manager/views.py +++ b/src/authentic2/manager/views.py @@ -669,7 +669,7 @@ class HomepageView(TitleMixin, PermissionMixin, MediaMixin, TemplateView): 'label': _('Authentication frontends'), 'slug': 'authn', 'href': reverse_lazy('a2-manager-authenticators'), - 'permissions': 'superuser', + 'permissions': 'authenticators.search_baseauthenticator', 'place': 'sidebar', }, { diff --git a/tests/test_a2_rbac.py b/tests/test_a2_rbac.py index 2bd0c98d2..e42c6c991 100644 --- a/tests/test_a2_rbac.py +++ b/tests/test_a2_rbac.py @@ -30,14 +30,14 @@ from tests.utils import login, request_select2, scoped_db_fixture def test_update_rbac(db): - # 3 content types managers and 1 global manager - assert Role.objects.count() == 5 - # 3 content type global permissions, 1 role administration permissions (for the main manager + # 5 content types managers and 1 global manager + assert Role.objects.count() == 6 + # 4 content type global permissions, 1 role administration permissions (for the main manager # role which is self-administered) # and 1 user view permission (for the role administrator) # and 1 user manage authorizations permission (for the role administrator) # and 1 ou view permission (for the user and role administrators) - assert Permission.objects.count() == 8 + assert Permission.objects.count() == 9 def test_delete_role(db): @@ -405,10 +405,10 @@ def test_no_managed_ct(transactional_db, settings): from django.core.management.sql import emit_post_migrate_signal call_command('flush', verbosity=0, interactive=False, database='default', reset_sequences=False) - assert Role.objects.count() == 5 + assert Role.objects.count() == 6 OU.objects.create(name='OU1', slug='ou1') emit_post_migrate_signal(verbosity=0, interactive=False, db='default', created_models=[]) - assert Role.objects.count() == 5 + 4 + 4 + assert Role.objects.count() == 6 + 5 + 5 settings.A2_RBAC_MANAGED_CONTENT_TYPES = () call_command('flush', verbosity=0, interactive=False, database='default', reset_sequences=False) assert Role.objects.count() == 0 @@ -424,12 +424,14 @@ def test_global_manager_roles(db): user_manager = Role.objects.get(ou__isnull=True, slug='_a2-manager-of-users') role_manager = Role.objects.get(ou__isnull=True, slug='_a2-manager-of-roles') service_manager = Role.objects.get(ou__isnull=True, slug='_a2-manager-of-services') + authenticator_manager = Role.objects.get(ou__isnull=True, slug='_a2-manager-of-authenticators') assert ou_manager in manager.parents() assert user_manager in manager.parents() assert role_manager in manager.parents() assert service_manager in manager.parents() - assert manager.parents(include_self=False).count() == 4 - assert Role.objects.count() == 5 + assert authenticator_manager in manager.parents() + assert manager.parents(include_self=False).count() == 5 + assert Role.objects.count() == 6 assert OU.objects.count() == 1 @@ -439,25 +441,29 @@ def test_manager_roles_multi_ou(db, ou1): user_manager = Role.objects.get(ou__isnull=True, slug='_a2-manager-of-users') role_manager = Role.objects.get(ou__isnull=True, slug='_a2-manager-of-roles') service_manager = Role.objects.get(ou__isnull=True, slug='_a2-manager-of-services') + authenticator_manager = Role.objects.get(ou__isnull=True, slug='_a2-manager-of-authenticators') assert ou_manager in manager.parents() assert user_manager in manager.parents() assert role_manager in manager.parents() assert service_manager in manager.parents() - assert manager.parents(include_self=False).count() == 4 + assert authenticator_manager in manager.parents() + assert manager.parents(include_self=False).count() == 5 for ou in [get_default_ou(), ou1]: manager = Role.objects.get(ou__isnull=True, slug=f'_a2-managers-of-{ou.slug}') user_manager = Role.objects.get(ou=ou, slug=f'_a2-manager-of-users-{ou.slug}') role_manager = Role.objects.get(ou=ou, slug=f'_a2-manager-of-roles-{ou.slug}') service_manager = Role.objects.get(ou=ou, slug=f'_a2-manager-of-services-{ou.slug}') + authenticator_manager = Role.objects.get(ou=ou, slug=f'_a2-manager-of-authenticators-{ou.slug}') assert user_manager in manager.parents() assert role_manager in manager.parents() assert service_manager in manager.parents() - assert manager.parents(include_self=False).count() == 3 + assert authenticator_manager in manager.parents() + assert manager.parents(include_self=False).count() == 4 - # 5 global roles and 4 ou roles for both ous - assert Role.objects.count() == 5 + 4 + 4 + # 6 global roles and 5 ou roles for both ous + assert Role.objects.count() == 6 + 5 + 5 @pytest.mark.parametrize( diff --git a/tests/test_manager.py b/tests/test_manager.py index 28651a04f..382e94302 100644 --- a/tests/test_manager.py +++ b/tests/test_manager.py @@ -84,10 +84,11 @@ def test_manager_login_admin(admin, app): for section in sections: path = reverse('a2-manager-%s' % section) assert response.pyquery.remove_namespaces()('a.button[href=\'%s\']' % path) - # only the journal is visible in the sidebar - assert len(response.pyquery('div.a2-manager-id-tools_content').children()) == 2 + # only journal, api clients and authenticators are visible in the sidebar + assert len(response.pyquery('div.a2-manager-id-tools_content').children()) == 3 assert response.pyquery('a#journal') assert response.pyquery('a#api-clients') + assert response.pyquery('a#authn') def test_manager_create_ou(superuser_or_admin, app): @@ -467,9 +468,9 @@ def test_manager_one_ou(app, superuser, admin, simple_role, settings): form.set('search-internals', True) response = form.submit() q = response.pyquery.remove_namespaces() - assert len(q('table tbody tr')) == 6 + assert len(q('table tbody tr')) == 7 # admin enroled only in the Manager role, other roles are inherited - assert len(q('table tbody tr td.via')) == 6 + assert len(q('table tbody tr td.via')) == 7 assert len(q('table tbody tr td.via:empty')) == 2 for elt in q('table tbody td.name a'): assert 'Manager' in elt.text or elt.text == 'simple role' @@ -491,7 +492,7 @@ def test_manager_one_ou(app, superuser, admin, simple_role, settings): response.form.set('search-internals', True) response = response.form.submit() q = response.pyquery.remove_namespaces() - assert len(q('table tbody tr')) == 6 + assert len(q('table tbody tr')) == 7 for elt in q('table tbody td.name a'): assert 'Manager' in elt.text or elt.text == 'simple role' @@ -542,9 +543,9 @@ def test_manager_many_ou(app, superuser, admin, simple_role, role_ou1, admin_ou1 form.set('search-internals', True) response = form.submit() q = response.pyquery.remove_namespaces() - assert len(q('table tbody tr')) == 5 + assert len(q('table tbody tr')) == 6 # admin enroled only in the Manager role, other roles are inherited - assert len(q('table tbody tr td.via')) == 5 + assert len(q('table tbody tr td.via')) == 6 assert len(q('table tbody tr td.via:empty')) == 1 for elt in q('table tbody td.name a'): assert 'Manager' in elt.text @@ -554,7 +555,7 @@ def test_manager_many_ou(app, superuser, admin, simple_role, role_ou1, admin_ou1 form.set('search-internals', True) response = form.submit() q = response.pyquery.remove_namespaces() - assert len(q('table tbody tr')) == 7 + assert len(q('table tbody tr')) == 8 for elt in q('table tbody td.name a'): assert 'Manager' in elt.text @@ -586,7 +587,7 @@ def test_manager_many_ou(app, superuser, admin, simple_role, role_ou1, admin_ou1 response.form.set('search-internals', True) response = response.form.submit() q = response.pyquery.remove_namespaces() - assert len(q('table tbody tr')) == 15 + assert len(q('table tbody tr')) == 18 for elt in q('table tbody td.name a'): assert ( 'OU1' in elt.text @@ -600,7 +601,7 @@ def test_manager_many_ou(app, superuser, admin, simple_role, role_ou1, admin_ou1 response.form.set('search-internals', True) response = response.form.submit() q = response.pyquery.remove_namespaces() - assert len(q('table tbody tr')) == 7 + assert len(q('table tbody tr')) == 8 for elt in q('table tbody td.name a'): assert 'Manager' in elt.text @@ -647,9 +648,9 @@ def test_manager_many_ou(app, superuser, admin, simple_role, role_ou1, admin_ou1 response.form.set('search-internals', True) response = response.form.submit() q = response.pyquery.remove_namespaces() - assert len(q('table tbody tr')) == 4 + assert len(q('table tbody tr')) == 5 names = {elt.text for elt in q('table tbody td.name a')} - assert names == {'Roles - OU1', 'Users - OU1', 'Services - OU1', 'role_ou1'} + assert names == {'Roles - OU1', 'Users - OU1', 'Services - OU1', 'role_ou1', 'Authenticators - OU1'} # test role listing response = app.get('/manage/roles/') @@ -668,9 +669,9 @@ def test_manager_many_ou(app, superuser, admin, simple_role, role_ou1, admin_ou1 response.form.set('search-internals', True) response = response.form.submit() q = response.pyquery.remove_namespaces() - assert len(q('table tbody tr')) == 4 + assert len(q('table tbody tr')) == 5 names = {elt.text for elt in q('table tbody td.name a')} - assert names == {'Roles - OU1', 'Users - OU1', 'Services - OU1', 'role_ou1'} + assert names == {'Roles - OU1', 'Users - OU1', 'Services - OU1', 'role_ou1', 'Authenticators - OU1'} test_user_listing_ou_admin(admin_ou1) diff --git a/tests/test_manager_authenticators.py b/tests/test_manager_authenticators.py index 0c59e8da7..a9df01b53 100644 --- a/tests/test_manager_authenticators.py +++ b/tests/test_manager_authenticators.py @@ -21,6 +21,7 @@ from django import VERSION as DJ_VERSION from django.utils.html import escape from webtest import Upload +from authentic2.a2_rbac.models import Role from authentic2.a2_rbac.utils import get_default_ou from authentic2.apps.authenticators.models import AddRoleAction, BaseAuthenticator, LoginPasswordAuthenticator from authentic2.models import Attribute @@ -31,11 +32,27 @@ from authentic2_auth_saml.models import SAMLAuthenticator, SetAttributeAction from .utils import assert_event, login, logout -def test_authenticators_authorization(app, admin, superuser): - resp = login(app, admin, path='/manage/') +def test_authenticators_authorization(app, simple_user, simple_role, admin, superuser): + simple_user.roles.add(simple_role.get_admin_role()) # grant user access to /manage/ + + resp = login(app, simple_user, path='/manage/') assert 'Authenticators' not in resp.text app.get('/manage/authenticators/', status=403) + role = Role.objects.get(name='Manager of authenticators') + simple_user.roles.add(role) + + resp = app.get('/manage/') + resp = resp.click('Authentication frontends') + assert 'Authenticators' in resp.text + + logout(app) + resp = login(app, admin, path='/manage/') + assert 'Authentication frontends' in resp.text + + resp = resp.click('Authentication frontends') + assert 'Authenticators' in resp.text + logout(app) resp = login(app, superuser, path='/manage/') assert 'Authentication frontends' in resp.text @@ -44,8 +61,8 @@ def test_authenticators_authorization(app, admin, superuser): assert 'Authenticators' in resp.text -def test_authenticators_password(app, superuser): - resp = login(app, superuser, path='/manage/authenticators/') +def test_authenticators_password(app, superuser_or_admin): + resp = login(app, superuser_or_admin, path='/manage/authenticators/') # Password authenticator already exists assert 'Password' in resp.text authenticator = LoginPasswordAuthenticator.objects.get() @@ -85,7 +102,7 @@ def test_authenticators_password(app, superuser): "Show condition: 'backoffice' in login_hint or remotre_addr == '1.2.3.4'" in resp.text ) - assert_event('authenticator.edit', user=superuser, session=app.session) + assert_event('authenticator.edit', user=superuser_or_admin, session=app.session) resp = resp.click('Edit') resp.form['show_condition'] = "remote_addr in dnsbl('ddns.entrouvert.org')" diff --git a/tests/test_role_manager.py b/tests/test_role_manager.py index 6436c0f49..e35545463 100644 --- a/tests/test_role_manager.py +++ b/tests/test_role_manager.py @@ -524,18 +524,21 @@ def test_role_members_user_role_mixed_field_choices( assert select2_json['more'] is True select2_json = request_select2(app, resp, fetch_all=True) - assert len(select2_json['results']) == 17 + assert len(select2_json['results']) == 20 choices = [x['text'] for x in select2_json['results']] assert choices == [ + 'Default organizational unit - Authenticators - Default organizational unit', 'Default organizational unit - Managers of role "simple role"', 'Default organizational unit - Roles - Default organizational unit', 'Default organizational unit - Services - Default organizational unit', 'Default organizational unit - Users - Default organizational unit', + 'OU1 - Authenticators - OU1', 'OU1 - role_ou1', 'OU1 - Roles - OU1', 'OU1 - Services - OU1', 'OU1 - Users - OU1', 'Manager', + 'Manager of authenticators', 'Manager of organizational units', 'Manager of roles', 'Manager of services', @@ -558,9 +561,9 @@ def test_role_members_user_role_mixed_field_choices( assert select2_json['more'] is False select2_json = request_select2(app, resp, term='Manager') - assert len(select2_json['results']) == 8 + assert len(select2_json['results']) == 9 select2_json = request_select2(app, resp, term='Manager of') - assert len(select2_json['results']) == 7 + assert len(select2_json['results']) == 8 select2_json = request_select2(app, resp, term='Manager of serv') assert len(select2_json['results']) == 1 -- 2.35.1