From 6b51c6a1cf98628da0c1dda5efc4ee7ebe926fd9 Mon Sep 17 00:00:00 2001
From: Paul Marillonnet <pmarillonnet@entrouvert.com>
Date: Thu, 17 Nov 2022 10:13:53 +0100
Subject: [PATCH 1/2] models: add ou field to api clients (#71275)

---
 .../migrations/0044_apiclient_ou.py           | 26 +++++++++++++++++++
 src/authentic2/models.py                      |  8 ++++++
 tests/test_a2_rbac.py                         | 10 ++++---
 tests/test_api_client.py                      | 16 ++++++++++++
 tests/test_manager.py                         | 24 +++++++++++++----
 tests/test_role_manager.py                    |  4 ++-
 6 files changed, 78 insertions(+), 10 deletions(-)
 create mode 100644 src/authentic2/migrations/0044_apiclient_ou.py

diff --git a/src/authentic2/migrations/0044_apiclient_ou.py b/src/authentic2/migrations/0044_apiclient_ou.py
new file mode 100644
index 000000000..a748dc23b
--- /dev/null
+++ b/src/authentic2/migrations/0044_apiclient_ou.py
@@ -0,0 +1,26 @@
+# Generated by Django 2.2.26 on 2022-11-17 09:11
+
+import django.db.models.deletion
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('a2_rbac', '0033_remove_old_operation_fk'),
+        ('authentic2', '0043_api_client_description'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='apiclient',
+            name='ou',
+            field=models.ForeignKey(
+                blank=True,
+                null=True,
+                on_delete=django.db.models.deletion.CASCADE,
+                to='a2_rbac.OrganizationalUnit',
+                verbose_name='organizational unit',
+            ),
+        ),
+    ]
diff --git a/src/authentic2/models.py b/src/authentic2/models.py
index 2db2d0e88..444d2a0ac 100644
--- a/src/authentic2/models.py
+++ b/src/authentic2/models.py
@@ -652,6 +652,14 @@ class APIClient(models.Model):
         related_name='apiclients',
         blank=True,
     )
+    ou = models.ForeignKey(
+        verbose_name=_('organizational unit'),
+        to='a2_rbac.OrganizationalUnit',
+        swappable=False,
+        on_delete=models.CASCADE,
+        blank=True,
+        null=True,
+    )
 
     class Meta:
         verbose_name = _('APIClient')
diff --git a/tests/test_a2_rbac.py b/tests/test_a2_rbac.py
index a3a60fc3c..2b5063c48 100644
--- a/tests/test_a2_rbac.py
+++ b/tests/test_a2_rbac.py
@@ -408,7 +408,7 @@ def test_no_managed_ct(transactional_db, settings):
     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() == 7 + 5 + 5
+    assert Role.objects.count() == 7 + 6 + 6
     settings.A2_RBAC_MANAGED_CONTENT_TYPES = ()
     call_command('flush', verbosity=0, interactive=False, database='default', reset_sequences=False)
     assert Role.objects.count() == 0
@@ -459,15 +459,17 @@ def test_manager_roles_multi_ou(db, ou1):
         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}')
+        apiclients_manager = Role.objects.get(ou=ou, slug=f'_a2-manager-of-api-clients-{ou.slug}')
 
         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() == 4
+        assert apiclients_manager in manager.parents()
+        assert manager.parents(include_self=False).count() == 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
+    # 7 global roles and 6 ou roles for both ous
+    assert Role.objects.count() == 7 + 6 + 6
 
 
 @pytest.mark.parametrize(
diff --git a/tests/test_api_client.py b/tests/test_api_client.py
index 11a410bf2..d7bd71cd6 100644
--- a/tests/test_api_client.py
+++ b/tests/test_api_client.py
@@ -34,6 +34,22 @@ def test_has_perm(api_client):
     assert api_client.has_perm('a2_rbac.add_role')
 
 
+def test_has_perm_ou(api_client, ou1):
+    role_ct = ContentType.objects.get_for_model(Role)
+    role_admin_role = Role.objects.get_admin_role(role_ct, 'admin %s' % role_ct, 'admin-role')
+    api_client = APIClient.objects.create(name='foo', ou=ou1)
+    assert not api_client.has_ou_perm('a2_rbac.change_role', ou1)
+    assert not api_client.has_ou_perm('a2_rbac.view_role', ou1)
+    assert not api_client.has_ou_perm('a2_rbac.delete_role', ou1)
+    assert not api_client.has_ou_perm('a2_rbac.add_role', ou1)
+    role_admin_role.apiclients.add(api_client)
+    del api_client._rbac_perms_cache
+    assert api_client.has_ou_perm('a2_rbac.change_role', ou1)
+    assert api_client.has_ou_perm('a2_rbac.view_role', ou1)
+    assert api_client.has_ou_perm('a2_rbac.delete_role', ou1)
+    assert api_client.has_ou_perm('a2_rbac.add_role', ou1)
+
+
 def test_api_users_list(app, api_client):
     User.objects.create(username='user1')
 
diff --git a/tests/test_manager.py b/tests/test_manager.py
index f388c63e5..b7bd123bc 100644
--- a/tests/test_manager.py
+++ b/tests/test_manager.py
@@ -592,7 +592,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')) == 19
+        assert len(q('table tbody tr')) == 21
         for elt in q('table tbody td.name a'):
             assert (
                 'OU1' in elt.text
@@ -653,9 +653,16 @@ 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')) == 5
+        assert len(q('table tbody tr')) == 6
         names = {elt.text for elt in q('table tbody td.name a')}
-        assert names == {'Roles - OU1', 'Users - OU1', 'Services - OU1', 'role_ou1', 'Authenticators - OU1'}
+        assert names == {
+            'Roles - OU1',
+            'Users - OU1',
+            'Services - OU1',
+            'role_ou1',
+            'Authenticators - OU1',
+            'API clients - OU1',
+        }
 
         # test role listing
         response = app.get('/manage/roles/')
@@ -674,9 +681,16 @@ 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')) == 5
+        assert len(q('table tbody tr')) == 6
         names = {elt.text for elt in q('table tbody td.name a')}
-        assert names == {'Roles - OU1', 'Users - OU1', 'Services - OU1', 'role_ou1', 'Authenticators - OU1'}
+        assert names == {
+            'Roles - OU1',
+            'Users - OU1',
+            'Services - OU1',
+            'role_ou1',
+            'Authenticators - OU1',
+            'API clients - OU1',
+        }
 
     test_user_listing_ou_admin(admin_ou1)
 
diff --git a/tests/test_role_manager.py b/tests/test_role_manager.py
index e088b2aa2..bdbbf96d2 100644
--- a/tests/test_role_manager.py
+++ b/tests/test_role_manager.py
@@ -524,14 +524,16 @@ 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']) == 21
+    assert len(select2_json['results']) == 23
     choices = [x['text'] for x in select2_json['results']]
     assert choices == [
+        'Default organizational unit - API clients - Default organizational unit',
         '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 - API clients - OU1',
         'OU1 - Authenticators - OU1',
         'OU1 - role_ou1',
         'OU1 - Roles - OU1',
-- 
2.38.1

