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
@@ -55,6 +55,13 @@ class APIClientsFormViewMixin(APIClientsMixin):
                 if self.request.user.has_ou_perm('authentic2.admin_apiclient', ou):
                     allowed_ous.append(ou.id)
             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
 
 

src/authentic2/manager/forms.py

@@ -939,6 +939,20 @@ class APIClientForm(forms.ModelForm):
         'apiclient_roles',
     )
 
+    def clean(self):
+        ou = self.cleaned_data['ou']
+        if ou:
+            unauthorized_roles = self.cleaned_data['apiclient_roles'].exclude(ou=ou)
+            if unauthorized_roles:
+                unauthorized_roles = ', '.join(unauthorized_roles.values_list('name', flat=True))
+                self.add_error(
+                    'apiclient_roles',
+                    _(
+                        f'The following roles do not belong to organizational unit {ou.name}: {unauthorized_roles}.'
+                    ),
+                )
+        return super().clean()
+
     class Meta:
         model = APIClient
         fields = (

src/authentic2/migrations/0045_auto_20221222_1013.py

@@ -0,0 +1,28 @@
+# Generated by Django 2.2.26 on 2022-12-22 09:13
+
+import django.db.models.deletion
+from django.db import migrations, models
+
+import authentic2.a2_rbac.utils
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('authentic2', '0044_apiclient_ou'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='apiclient',
+            name='ou',
+            field=models.ForeignKey(
+                blank=True,
+                default=authentic2.a2_rbac.utils.get_default_ou_pk,
+                null=True,
+                on_delete=django.db.models.deletion.CASCADE,
+                to='a2_rbac.OrganizationalUnit',
+                verbose_name='organizational unit',
+            ),
+        ),
+    ]

src/authentic2/models.py

@@ -727,6 +727,8 @@ class APIClient(models.Model):
         swappable=False,
         on_delete=models.CASCADE,
         default=get_default_ou_pk,
+        null=True,
+        blank=True,
     )
 
     class Meta:

tests/test_manager_apiclient.py

@@ -20,6 +20,7 @@ import pytest
 from django.urls import reverse
 
 from authentic2.a2_rbac.models import Role
+from authentic2.a2_rbac.utils import get_default_ou
 from authentic2.models import APIClient
 
 from .utils import login
@@ -159,12 +160,13 @@ def test_list_show_objects_local_admin(admin_ou1, app, ou1, ou2):
 
 def test_add(superuser, app):
     assert APIClient.objects.count() == 0
-    role_1 = Role.objects.create(name='role-1')
+    role_1 = Role.objects.create(name='role-1', ou=get_default_ou())
-    role_2 = Role.objects.create(name='role-2')
+    role_2 = Role.objects.create(name='role-2', ou=get_default_ou())
     resp = login(app, superuser, 'a2-manager-api-client-add')
     form = resp.form
     # password is prefilled
     assert form.get('password').value
+    assert ('', False, '---------') in form['ou'].options
     form.set('name', 'api-client-name')
     form.set('description', 'api-client-description')
     form.set('identifier', 'api-client-identifier')
@@ -182,18 +184,20 @@ def test_add_local_admin(admin_ou1, app, ou1, ou2):
     resp = login(app, admin_ou1, 'a2-manager-api-client-add')
     form = resp.form
     assert len(form['ou'].options) == 1
+    assert ('', False, '---------') not in form['ou'].options
     assert form['ou'].options[0][2] == 'OU1'
 
     role = Role.objects.get(slug='_a2-manager-of-api-clients-%s' % ou2.slug)
     admin_ou1.roles.add(role)
     resp = app.get(reverse('a2-manager-api-client-add'))
     assert len(resp.form['ou'].options) == 2
+    assert ('', False, '---------') not in form['ou'].options
 
 
 def test_add_description_non_mandatory(superuser, app):
     assert APIClient.objects.count() == 0
-    role_1 = Role.objects.create(name='role-1')
+    role_1 = Role.objects.create(name='role-1', ou=get_default_ou())
-    role_2 = Role.objects.create(name='role-2')
+    role_2 = Role.objects.create(name='role-2', ou=get_default_ou())
     resp = login(app, superuser, 'a2-manager-api-client-add')
     form = resp.form
     form.set('name', 'api-client-name')
@@ -238,23 +242,61 @@ 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})
     form = resp.form
     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
     api_client = APIClient.objects.get(password='easy')
     assert api_client.identifier == 'foo-identifier'
 
+    resp = app.get(reverse('a2-manager-api-client-edit', kwargs={'pk': api_client.pk}))
+    form = resp.form
+    form.set('ou', ou2.id)
+    response = form.submit()
+    errmsg = response.pyquery('div.error')[0].text
+    assert "do not belong to organizational unit OU2: role-1, role-3." in errmsg
+    response.form.set('ou', ou2.id)
+    response.form['apiclient_roles'].force_value([])
+    response.form.submit().follow()
+    api_client = APIClient.objects.get()
+    assert set(api_client.apiclient_roles.all()) == set()
+    assert api_client.ou == ou2
+
+    resp = app.get(reverse('a2-manager-api-client-edit', kwargs={'pk': api_client.pk}))
+    form = resp.form
+    form['apiclient_roles'].force_value([role_2.id])
+    response = form.submit().follow()
+    api_client = APIClient.objects.get()
+    assert api_client.ou == ou2
+    assert set(api_client.apiclient_roles.all()) == {role_2}
+
 
 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',
@@ -273,6 +315,13 @@ def test_edit_local_admin(admin_ou1, app, ou1, ou2):
     form = resp.form
     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')
@@ -282,6 +331,7 @@ def test_edit_local_admin(admin_ou1, app, ou1, ou2):
     admin_ou1.roles.add(role)
     resp = app.get(reverse('a2-manager-api-client-edit', kwargs={'pk': api_client_ou2.pk}))
     assert resp.form.get('password').value == 'bar-password'
+    assert ('', False, '---------') not in form['ou'].options
     resp.form.set('ou', ou1.id)
     resp.form.submit().follow()
     assert APIClient.objects.filter(ou=ou1).count() == 2