manager: filter api client qs on user's admin ou permissions (#72688)

src/authentic2/manager/apiclient_views.py

@@ -20,6 +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.manager import forms
 from authentic2.manager.views import MediaMixin, PermissionMixin, TitleMixin
 from authentic2.models import APIClient
@@ -28,10 +29,21 @@ from authentic2.models import APIClient
 class APIClientsMixin(PermissionMixin, MediaMixin, TitleMixin):
     model = APIClient
     permissions = ['authentic2.admin_apiclient']
-    permissions_global = True
+    permissions_global = False
 
     def get_queryset(self):
-        return self.model.objects.all()
+        if not self.request.user:
+            return self.model.objects.none()
+
+        qs = self.model.objects.all()
+        if self.request.user.has_perm('authentic2.admin_apiclient'):
+            return qs
+
+        allowed_ous = []
+        for ou in OrganizationalUnit.objects.all():
+            if self.request.user.has_ou_perm('authentic2.admin_apiclient', ou):
+                allowed_ous.append(ou)
+        return qs.filter(ou__in=allowed_ous)
 
 
 class APIClientsView(APIClientsMixin, ListView):

src/authentic2/manager/templates/authentic2/manager/api_client_detail.html

@@ -23,6 +23,7 @@
     <ul>
       <li>{% trans "Identifier" %}{% trans ":" %} {{api_client.identifier}}</li>
       <li>{% trans "Password" %}{% trans ":" %} {{api_client.password}}</li>
+      <li>{% trans "Organizational unit" %}{% trans ":" %} {{ api_client.ou.name }}</li>
       {% if api_client.restrict_to_anonymised_data %}<li>{% trans "Restricted to anonymised data" %}</li>{% endif %}
       {% if api_client.apiclient_roles.count %}
         <li>{% trans "Roles:" %}

src/authentic2/manager/templates/authentic2/manager/api_clients.html

@@ -13,7 +13,7 @@
     <ul class="objects-list single-links">
       {% for api_client in object_list %}
         <li>
-          <a href="{% url 'a2-manager-api-client-detail' pk=api_client.pk %}">{{api_client.name}} ({{api_client.identifier}})</a>
+          <a href="{% url 'a2-manager-api-client-detail' pk=api_client.pk %}">{{ api_client.name }} ({{ api_client.identifier }}) - {{ api_client.ou.name }}</a>
         </li>
       {% endfor %}
     </ul>

tests/test_manager_apiclient.py

@@ -32,13 +32,18 @@ class TestAuthorization:
         return app
 
     @pytest.fixture
-    def api_client(self, db):
+    def api_client(self, db, ou1):
         return APIClient.objects.create(
-            name='foo', description='foo-description', identifier='foo-description', password='foo-password'
+            name='foo',
+            description='foo-description',
+            identifier='foo-description',
+            password='foo-password',
+            ou=ou1,
         )
 
     class Mixin:
         status_code = -1
+        existing_client_status_code = -1
 
         def test_list(self, app):
             app.get(reverse('a2-manager-api-clients'), status=self.status_code)
@@ -48,28 +53,49 @@ class TestAuthorization:
 
         def test_detail(self, app, api_client):
             app.get(
-                reverse('a2-manager-api-client-detail', kwargs={'pk': api_client.pk}), status=self.status_code
+                reverse('a2-manager-api-client-detail', kwargs={'pk': api_client.pk}),
+                status=self.existing_client_status_code,
             )
 
         def test_edit(self, app, api_client):
             app.get(
-                reverse('a2-manager-api-client-edit', kwargs={'pk': api_client.pk}), status=self.status_code
+                reverse('a2-manager-api-client-edit', kwargs={'pk': api_client.pk}),
+                status=self.existing_client_status_code,
             )
 
         def test_delete(self, app, api_client):
             app.get(
-                reverse('a2-manager-api-client-delete', kwargs={'pk': api_client.pk}), status=self.status_code
+                reverse('a2-manager-api-client-delete', kwargs={'pk': api_client.pk}),
+                status=self.existing_client_status_code,
             )
 
     class TestAuthorization(Mixin):
         status_code = 403
+        existing_client_status_code = 404
 
         @pytest.fixture
         def user(self, simple_user):
             return simple_user
 
+    class TestAuthorizationLocalAdminWrongOu(Mixin):
+        status_code = 200
+        existing_client_status_code = 404
+
+        @pytest.fixture
+        def user(self, admin_ou2):
+            return admin_ou2
+
+    class TestAuthorizationLocalAdminRightOu(Mixin):
+        status_code = 200
+        existing_client_status_code = 200
+
+        @pytest.fixture
+        def user(self, admin_ou1):
+            return admin_ou1
+
     class TestAuthorizationAdmin(Mixin):
         status_code = 200
+        existing_client_status_code = 200
 
         @pytest.fixture
         def user(self, simple_user):
@@ -95,7 +121,40 @@ def test_list_show_objects(superuser, app):
     url = '/manage/api-clients/%s/' % api_client.pk
     resp = login(app, superuser, 'a2-manager-api-clients')
     anchor = resp.pyquery('div.content ul.objects-list a[href="%s"]' % url)
-    assert anchor.text() == 'foo (foo-description)'
+    assert anchor.text() == 'foo (foo-description) - Default organizational unit'
+
+
+def test_list_show_objects_local_admin(admin_ou1, app, ou1, ou2):
+    api_client_ou1 = APIClient.objects.create(
+        name='foo',
+        description='foo-description',
+        identifier='foo-description',
+        password='foo-password',
+        ou=ou1,
+    )
+    api_client_ou2 = APIClient.objects.create(
+        name='bar',
+        description='bar-description',
+        identifier='bar-description',
+        password='bar-password',
+        ou=ou2,
+    )
+    url = '/manage/api-clients/%s/' % api_client_ou1.pk
+    resp = login(app, admin_ou1, 'a2-manager-api-clients')
+    assert len(resp.pyquery('div.content ul.objects-list li')) == 1
+    anchor = resp.pyquery('div.content ul.objects-list a[href="%s"]' % url)
+    assert anchor.text() == 'foo (foo-description) - OU1'
+
+    role = Role.objects.get(slug='_a2-manager-of-api-clients-%s' % ou2.slug)
+    admin_ou1.roles.add(role)
+    admin_ou1.save()
+    resp = app.get(reverse('a2-manager-api-clients'))
+    assert len(resp.pyquery('div.content ul.objects-list li')) == 2
+    anchor = resp.pyquery('div.content ul.objects-list a[href="%s"]' % url)
+    assert anchor.text() == 'foo (foo-description) - OU1'
+    url = '/manage/api-clients/%s/' % api_client_ou2.pk
+    anchor = resp.pyquery('div.content ul.objects-list a[href="%s"]' % url)
+    assert anchor.text() == 'bar (bar-description) - OU2'
 
 
 def test_add(superuser, app):