From a6c0a9e750539429abb507ec7a5345180843c54d Mon Sep 17 00:00:00 2001
From: Paul Marillonnet <pmarillonnet@entrouvert.com>
Date: Mon, 14 Nov 2022 12:26:17 +0100
Subject: [PATCH] a2_rbac: add global management role for api clients (#71267)

    ou-wise api-client management roles will be added in #71275.
---
 src/authentic2/a2_rbac/management.py      |  4 ++++
 src/authentic2/manager/apiclient_views.py |  2 +-
 src/authentic2/manager/views.py           |  2 +-
 tests/test_a2_rbac.py                     | 22 +++++++++++++---------
 tests/test_manager.py                     | 16 ++++++++--------
 tests/test_manager_apiclient.py           |  2 +-
 tests/test_role_manager.py                |  7 ++++---
 7 files changed, 32 insertions(+), 23 deletions(-)

diff --git a/src/authentic2/a2_rbac/management.py b/src/authentic2/a2_rbac/management.py
index 79ae1c43c..35c5cbf1a 100644
--- a/src/authentic2/a2_rbac/management.py
+++ b/src/authentic2/a2_rbac/management.py
@@ -95,6 +95,10 @@ MANAGED_CT = {
         'name': _('Manager of authenticators'),
         'scoped_name': _('Authenticators - {ou}'),
     },
+    ('authentic2', 'apiclient'): {
+        'name': _('Manager of API clients'),
+        'scoped_name': _('API clients - {ou}'),
+    },
 }
 
 
diff --git a/src/authentic2/manager/apiclient_views.py b/src/authentic2/manager/apiclient_views.py
index b02564354..17b3725d4 100644
--- a/src/authentic2/manager/apiclient_views.py
+++ b/src/authentic2/manager/apiclient_views.py
@@ -27,7 +27,7 @@ from authentic2.models import APIClient
 
 class APIClientsMixin(PermissionMixin, MediaMixin, TitleMixin):
     model = APIClient
-    permissions = ['authentic2.admin_service']
+    permissions = ['authentic2.admin_apiclient']
     permissions_global = True
 
     def get_queryset(self):
diff --git a/src/authentic2/manager/views.py b/src/authentic2/manager/views.py
index dbd36ff64..edbe459ca 100644
--- a/src/authentic2/manager/views.py
+++ b/src/authentic2/manager/views.py
@@ -692,7 +692,7 @@ class HomepageView(TitleMixin, PermissionMixin, MediaMixin, TemplateView):
             'label': _('API Clients'),
             'slug': 'api-clients',
             'href': reverse_lazy('a2-manager-api-clients'),
-            'permissions': ['authentic2.admin_service'],
+            'permissions': ['authentic2.admin_apiclient'],
             'place': 'sidebar',
         },
     ]
diff --git a/tests/test_a2_rbac.py b/tests/test_a2_rbac.py
index 4288ec294..81c21d95e 100644
--- a/tests/test_a2_rbac.py
+++ b/tests/test_a2_rbac.py
@@ -31,13 +31,13 @@ from tests.utils import login, request_select2, scoped_db_fixture
 
 def test_update_rbac(db):
     # 5 content types managers and 1 global manager
-    assert Role.objects.count() == 6
+    assert Role.objects.count() == 7
     # 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() == 9
+    assert Permission.objects.count() == 10
 
 
 def test_delete_role(db):
@@ -423,10 +423,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() == 6
+    assert Role.objects.count() == 7
     OU.objects.create(name='OU1', slug='ou1')
     emit_post_migrate_signal(verbosity=0, interactive=False, db='default', created_models=[])
-    assert Role.objects.count() == 6 + 5 + 5
+    assert Role.objects.count() == 7 + 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
@@ -443,13 +443,15 @@ def test_global_manager_roles(db):
     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')
+    apiclients_manager = Role.objects.get(ou__isnull=True, slug='_a2-manager-of-api-clients')
     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 authenticator_manager in manager.parents()
-    assert manager.parents(include_self=False).count() == 5
-    assert Role.objects.count() == 6
+    assert apiclients_manager in manager.parents()
+    assert manager.parents(include_self=False).count() == 6
+    assert Role.objects.count() == 7
     assert OU.objects.count() == 1
 
 
@@ -460,12 +462,14 @@ def test_manager_roles_multi_ou(db, ou1):
     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')
+    apiclients_manager = Role.objects.get(ou__isnull=True, slug='_a2-manager-of-api-clients')
     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 authenticator_manager in manager.parents()
-    assert manager.parents(include_self=False).count() == 5
+    assert apiclients_manager in manager.parents()
+    assert manager.parents(include_self=False).count() == 6
 
     for ou in [get_default_ou(), ou1]:
         manager = Role.objects.get(ou__isnull=True, slug=f'_a2-managers-of-{ou.slug}')
@@ -480,8 +484,8 @@ def test_manager_roles_multi_ou(db, ou1):
         assert authenticator_manager in manager.parents()
         assert manager.parents(include_self=False).count() == 4
 
-    # 6 global roles and 5 ou roles for both ous
-    assert Role.objects.count() == 6 + 5 + 5
+    # 7 global roles and 5 ou roles for both ous (api clients aren't ou-managed yet)
+    assert Role.objects.count() == 7 + 5 + 5
 
 
 @pytest.mark.parametrize(
diff --git a/tests/test_manager.py b/tests/test_manager.py
index 525d9e144..11d48d88a 100644
--- a/tests/test_manager.py
+++ b/tests/test_manager.py
@@ -466,9 +466,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')) == 7
+        assert len(q('table tbody tr')) == 8
         # admin enroled only in the Manager role, other roles are inherited
-        assert len(q('table tbody tr td.via')) == 7
+        assert len(q('table tbody tr td.via')) == 8
         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'
@@ -490,7 +490,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')) == 7
+        assert len(q('table tbody tr')) == 8
         for elt in q('table tbody td.name a'):
             assert 'Manager' in elt.text or elt.text == 'simple role'
 
@@ -541,9 +541,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')) == 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')) == 1
         for elt in q('table tbody td.name a'):
             assert 'Manager' in elt.text
@@ -553,7 +553,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')) == 8
+        assert len(q('table tbody tr')) == 9
         for elt in q('table tbody td.name a'):
             assert 'Manager' in elt.text
 
@@ -585,7 +585,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')) == 18
+        assert len(q('table tbody tr')) == 19
         for elt in q('table tbody td.name a'):
             assert (
                 'OU1' in elt.text
@@ -599,7 +599,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')) == 8
+        assert len(q('table tbody tr')) == 9
         for elt in q('table tbody td.name a'):
             assert 'Manager' in elt.text
 
diff --git a/tests/test_manager_apiclient.py b/tests/test_manager_apiclient.py
index 2bda5f73f..de3d2e38b 100644
--- a/tests/test_manager_apiclient.py
+++ b/tests/test_manager_apiclient.py
@@ -73,7 +73,7 @@ class TestAuthorization:
 
         @pytest.fixture
         def user(self, simple_user):
-            simple_user.roles.add(Role.objects.get(ou__isnull=True, slug='_a2-manager-of-services'))
+            simple_user.roles.add(Role.objects.get(ou__isnull=True, slug='_a2-manager-of-api-clients'))
             return simple_user
 
 
diff --git a/tests/test_role_manager.py b/tests/test_role_manager.py
index e35545463..d648cd262 100644
--- a/tests/test_role_manager.py
+++ b/tests/test_role_manager.py
@@ -524,7 +524,7 @@ 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']) == 20
+    assert len(select2_json['results']) == 21
     choices = [x['text'] for x in select2_json['results']]
     assert choices == [
         'Default organizational unit - Authenticators - Default organizational unit',
@@ -538,6 +538,7 @@ def test_role_members_user_role_mixed_field_choices(
         'OU1 - Services - OU1',
         'OU1 - Users - OU1',
         'Manager',
+        'Manager of API clients',
         'Manager of authenticators',
         'Manager of organizational units',
         'Manager of roles',
@@ -561,9 +562,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']) == 9
+    assert len(select2_json['results']) == 10
     select2_json = request_select2(app, resp, term='Manager of')
-    assert len(select2_json['results']) == 8
+    assert len(select2_json['results']) == 9
     select2_json = request_select2(app, resp, term='Manager of serv')
     assert len(select2_json['results']) == 1
 
-- 
2.38.1

