From 728e1c6aa7397a6e4fb8ad49133ee9250e63ff71 Mon Sep 17 00:00:00 2001
From: Paul Marillonnet <pmarillonnet@entrouvert.com>
Date: Thu, 22 Dec 2022 11:10:01 +0100
Subject: [PATCH 3/4] manager: filter api client's assignable roles depending
 on its OU (#72703)

---
 src/authentic2/manager/apiclient_views.py |  7 +++++-
 tests/test_manager_apiclient.py           | 26 +++++++++++++++++++++--
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/src/authentic2/manager/apiclient_views.py b/src/authentic2/manager/apiclient_views.py
index b5d468cc2..7fd46048c 100644
--- a/src/authentic2/manager/apiclient_views.py
+++ b/src/authentic2/manager/apiclient_views.py
@@ -20,7 +20,7 @@ from django.urls import reverse, reverse_lazy
 from django.utils.translation import gettext_lazy as _
 from django.views.generic import CreateView, DeleteView, DetailView, ListView, UpdateView
 
-from authentic2.a2_rbac.models import OrganizationalUnit
+from authentic2.a2_rbac.models import OrganizationalUnit, Role
 from authentic2.manager import forms
 from authentic2.manager.views import MediaMixin, PermissionMixin, TitleMixin
 from authentic2.models import APIClient
@@ -57,6 +57,11 @@ class APIClientsFormViewMixin(APIClientsMixin):
             form.fields['ou'].queryset = OrganizationalUnit.objects.filter(id__in=allowed_ous)
             form.fields['ou'].required = True
             form.fields['ou'].empty_label = None
+        api_client = self.object
+        if api_client and api_client.ou is not None:
+            form.fields['apiclient_roles'].queryset = Role.objects.filter(ou=api_client.ou).exclude(
+                slug__startswith='_'
+            )
         return form
 
 
diff --git a/tests/test_manager_apiclient.py b/tests/test_manager_apiclient.py
index 869b53111..acf9abff8 100644
--- a/tests/test_manager_apiclient.py
+++ b/tests/test_manager_apiclient.py
@@ -241,9 +241,16 @@ def test_detail(superuser, app):
     assert delete_button.text() == 'Delete'
 
 
-def test_edit(superuser, app):
+def test_edit(superuser, app, ou1, ou2):
+    role_1 = Role.objects.create(name='role-1', ou=ou1)
+    role_2 = Role.objects.create(name='role-2', ou=ou2)
+    role_3 = Role.objects.create(name='role-3', ou=ou1)
     api_client = APIClient.objects.create(
-        name='foo', description='foo-description', identifier='foo-identifier', password='foo-password'
+        name='foo',
+        description='foo-description',
+        identifier='foo-identifier',
+        password='foo-password',
+        ou=ou1,
     )
     assert APIClient.objects.count() == 1
     resp = login(app, superuser, 'a2-manager-api-client-edit', kwargs={'pk': api_client.pk})
@@ -251,6 +258,12 @@ def test_edit(superuser, app):
     assert form.get('password').value == 'foo-password'
     assert ('', False, '---------') in form['ou'].options
     resp.form.set('password', 'easy')
+    with pytest.raises(KeyError):
+        # forcing values not presented by the Select2ModelMultipleChoiceField,
+        # should not happen in UI
+        form['apiclient_roles'].force_value([role_1.id, role_2.id])
+        form.submit()
+    form['apiclient_roles'].force_value([role_1.id, role_3.id])
     response = form.submit().follow()
     assert urlparse(response.request.url).path == api_client.get_absolute_url()
     assert APIClient.objects.count() == 1
@@ -259,6 +272,9 @@ def test_edit(superuser, app):
 
 
 def test_edit_local_admin(admin_ou1, app, ou1, ou2):
+    role_1 = Role.objects.create(name='role-1', ou=ou1)
+    role_2 = Role.objects.create(name='role-2', ou=ou2)
+    role_3 = Role.objects.create(name='role-3', ou=ou1)
     api_client_ou1 = APIClient.objects.create(
         name='foo',
         description='foo-description',
@@ -278,6 +294,12 @@ def test_edit_local_admin(admin_ou1, app, ou1, ou2):
     assert form.get('password').value == 'foo-password'
     resp.form.set('password', 'easy')
     assert ('', False, '---------') not in form['ou'].options
+    with pytest.raises(KeyError):
+        # forcing values not presented by the Select2ModelMultipleChoiceField,
+        # should not happen in UI
+        form['apiclient_roles'].force_value([role_1.id, role_2.id])
+        form.submit()
+    form['apiclient_roles'].force_value([role_1.id, role_3.id])
     response = form.submit().follow()
     assert urlparse(response.request.url).path == api_client_ou1.get_absolute_url()
     api_client = APIClient.objects.get(password='easy')
-- 
2.39.0

