api: returns no user if service-slug is unknown (#35189)

src/authentic2/api_views.py

@@ -629,16 +629,19 @@ class UsersAPI(api_mixins.GetOrCreateMixinView, HookMixin, ExceptionHandlerMixin
             qs = qs.prefetch_related('attribute_values', 'attribute_values__attribute')
         qs = self.request.user.filter_by_perm(['custom_user.view_user'], qs)
         # filter users authorized for a specified service
-        if 'service-slug' in self.request.GET and 'service-ou' in self.request.GET:
+        if 'service-slug' in self.request.GET:
             service_slug = self.request.GET['service-slug']
-            service_ou = self.request.GET['service-ou']
+            service_ou = self.request.GET.get('service-ou', '')
             service = Service.objects.filter(
                 slug=service_slug,
                 ou__slug=service_ou
             ).prefetch_related('authorized_roles').first()
-            if service and service.authorized_roles.all():
-                qs = qs.filter(roles__in=service.authorized_roles.children())
-                qs = qs.distinct()
+            if service:
+                if service.authorized_roles.all():
+                    qs = qs.filter(roles__in=service.authorized_roles.children())
+                    qs = qs.distinct()
+            else:
+                qs = qs.none()
         new_qs = hooks.call_hooks_first_result('api_modify_queryset', self, qs)
         if new_qs is not None:
             return new_qs

tests/conftest.py

@@ -28,6 +28,7 @@ from django.utils.six.moves.urllib import parse as urlparse
 
 from pytest_django.migrations import DisableMigrations
 
+from authentic2.models import Service
 from authentic2.a2_rbac.utils import get_default_ou
 from authentic2_idp_oidc.models import OIDCClient
 from authentic2.authentication import OIDCUser
@@ -369,3 +370,11 @@ def french_translation():
 @pytest.fixture
 def media(settings, tmpdir):
     settings.MEDIA_ROOT = str(tmpdir.mkdir('media'))
+
+
+@pytest.fixture
+def service(db):
+    return Service.objects.create(
+        ou=get_default_ou(),
+        slug='service',
+        name='Service')

tests/test_api.py

@@ -35,7 +35,7 @@ from django_rbac.utils import get_role_model, get_ou_model
 
 from authentic2.a2_rbac.models import Role
 from authentic2.a2_rbac.utils import get_default_ou
-from authentic2.models import Service, Attribute, AttributeValue
+from authentic2.models import Service, Attribute, AttributeValue, AuthorizedRole
 from authentic2.utils import good_next_url
 
 from utils import login, basic_authorization_header, get_link_from_mail
@@ -1376,3 +1376,22 @@ def test_api_user_required_drf_attribute(settings, app, admin, simple_user):
 
     Attribute.objects.filter(name='prefered_color').update(required=False)
     resp = app.put_json('/api/users/{}/'.format(simple_user.uuid), params=payload, headers=headers, status=200)
+
+
+def test_filter_users_by_service(app, admin, simple_user, role_random, service):
+    app.authorization = ('Basic', (admin.username, admin.username))
+
+    resp = app.get('/api/users/')
+    assert len(resp.json['results']) == 2
+
+    resp = app.get('/api/users/?service-slug=xxx')
+    assert len(resp.json['results']) == 0
+
+    resp = app.get('/api/users/?service-slug=service&service-ou=default')
+    assert len(resp.json['results']) == 2
+
+    role_random.members.add(simple_user)
+    AuthorizedRole.objects.get_or_create(service=service, role=role_random)
+
+    resp = app.get('/api/users/?service-slug=service&service-ou=default')
+    assert len(resp.json['results']) == 1