sources/scim: return a SCIM error for unsupported filters (#25095)

* fix(sources/scim): return a SCIM error for unsupported filters

The `members` attribute was mapped to the `group__users` relation itself, so
`members eq "<id>"` transpiled to `group__users__iexact`, which Django rejects
with a FieldError and which surfaced as an HTML 500 page. Map `members` to the
related user's `uuid` field instead, and map `id` for both users and groups so
that filters combining an ID and a member resolve correctly. Filters that can't
be parsed or can't be mapped to any field now raise a SCIM `invalidFilter`
error with status 400 instead of crashing.

Closes #23211

* fix(sources/scim): give each user in the filter test a unique external ID

test_user_list_filter created two SCIMSourceUser rows on the same source
without setting external_id. That field has no default, so both rows got an
empty string and the second insert hit the unique_together constraint on
(external_id, source), failing the test with an IntegrityError. Set a distinct
external_id on each row, matching the group filter tests.

---------

Co-authored-by: Jens L. <jens@goauthentik.io>

authentik/sources/scim/tests/test_groups.py

@@ -1,6 +1,6 @@
 """Test SCIM Group"""
 
-from json import dumps
+from json import dumps, loads
 from uuid import uuid4
 
 from django.urls import reverse
@@ -60,6 +60,96 @@ class TestSCIMGroups(APITestCase):
         self.assertEqual(response.status_code, second=200)
         SCIMGroupSchema.model_validate_json(response.content, strict=True)
 
+    def test_group_list_filter_members(self):
+        """Test group list filtered by ID and member"""
+        user = create_test_user()
+        group = Group.objects.create(name=generate_id())
+        group.users.add(user)
+        other_group = Group.objects.create(name=generate_id())
+        other_group.users.add(user)
+        for _group in [group, other_group]:
+            SCIMSourceGroup.objects.create(
+                source=self.source, group=_group, external_id=str(uuid4())
+            )
+        response = self.client.get(
+            reverse(
+                "authentik_sources_scim:v2-groups",
+                kwargs={
+                    "source_slug": self.source.slug,
+                },
+            ),
+            data={"filter": f'id eq "{group.pk}" and members eq "{user.uuid}"'},
+            HTTP_AUTHORIZATION=f"Bearer {self.source.token.key}",
+        )
+        self.assertEqual(response.status_code, 200, response.content)
+        body = loads(response.content)
+        self.assertEqual(body["totalResults"], 1)
+        self.assertEqual(body["Resources"][0]["id"], str(group.pk))
+
+    def test_group_list_filter_members_no_match(self):
+        """Test group list filtered by a member that isn't part of the group"""
+        group = Group.objects.create(name=generate_id())
+        group.users.add(create_test_user())
+        SCIMSourceGroup.objects.create(source=self.source, group=group, external_id=str(uuid4()))
+        response = self.client.get(
+            reverse(
+                "authentik_sources_scim:v2-groups",
+                kwargs={
+                    "source_slug": self.source.slug,
+                },
+            ),
+            data={"filter": f'id eq "{group.pk}" and members eq "{uuid4()}"'},
+            HTTP_AUTHORIZATION=f"Bearer {self.source.token.key}",
+        )
+        self.assertEqual(response.status_code, 200, response.content)
+        self.assertEqual(loads(response.content)["totalResults"], 0)
+
+    def test_group_list_filter_unknown_attribute(self):
+        """Test group list filtered by an attribute that can't be mapped"""
+        response = self.client.get(
+            reverse(
+                "authentik_sources_scim:v2-groups",
+                kwargs={
+                    "source_slug": self.source.slug,
+                },
+            ),
+            data={"filter": f'urn:foo:bar eq "{generate_id()}"'},
+            HTTP_AUTHORIZATION=f"Bearer {self.source.token.key}",
+        )
+        self.assertEqual(response.status_code, 400)
+        self.assertJSONEqual(
+            response.content,
+            {
+                "detail": "Unsupported filter attribute.",
+                "schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"],
+                "scimType": "invalidFilter",
+                "status": 400,
+            },
+        )
+
+    def test_group_list_filter_invalid(self):
+        """Test group list filtered by an unparsable filter"""
+        response = self.client.get(
+            reverse(
+                "authentik_sources_scim:v2-groups",
+                kwargs={
+                    "source_slug": self.source.slug,
+                },
+            ),
+            data={"filter": "displayName eq"},
+            HTTP_AUTHORIZATION=f"Bearer {self.source.token.key}",
+        )
+        self.assertEqual(response.status_code, 400)
+        self.assertJSONEqual(
+            response.content,
+            {
+                "detail": "Invalid filter.",
+                "schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"],
+                "scimType": "invalidFilter",
+                "status": 400,
+            },
+        )
+
     def test_group_create(self):
         """Test group create"""
         ext_id = generate_id()

authentik/sources/scim/tests/test_users.py

@@ -1,6 +1,6 @@
 """Test SCIM User"""
 
-from json import dumps
+from json import dumps, loads
 from uuid import uuid4
 
 from django.urls import reverse
@@ -56,6 +56,50 @@ class TestSCIMUsers(APITestCase):
         self.assertEqual(response.status_code, 200)
         SCIMUserSchema.model_validate_json(response.content, strict=True)
 
+    def test_user_list_filter(self):
+        """Test user list with a filter"""
+        user = create_test_user()
+        SCIMSourceUser.objects.create(source=self.source, user=user, external_id=str(uuid4()))
+        other_user = create_test_user()
+        SCIMSourceUser.objects.create(source=self.source, user=other_user, external_id=str(uuid4()))
+        response = self.client.get(
+            reverse(
+                "authentik_sources_scim:v2-users",
+                kwargs={
+                    "source_slug": self.source.slug,
+                },
+            ),
+            data={"filter": f'id eq "{user.uuid}" and userName eq "{user.username}"'},
+            HTTP_AUTHORIZATION=f"Bearer {self.source.token.key}",
+        )
+        self.assertEqual(response.status_code, 200, response.content)
+        body = loads(response.content)
+        self.assertEqual(body["totalResults"], 1)
+        self.assertEqual(body["Resources"][0]["id"], str(user.uuid))
+
+    def test_user_list_filter_invalid(self):
+        """Test user list with an unparsable filter"""
+        response = self.client.get(
+            reverse(
+                "authentik_sources_scim:v2-users",
+                kwargs={
+                    "source_slug": self.source.slug,
+                },
+            ),
+            data={"filter": "userName eq"},
+            HTTP_AUTHORIZATION=f"Bearer {self.source.token.key}",
+        )
+        self.assertEqual(response.status_code, 400)
+        self.assertJSONEqual(
+            response.content,
+            {
+                "detail": "Invalid filter.",
+                "schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"],
+                "scimType": "invalidFilter",
+                "status": 400,
+            },
+        )
+
     def test_user_create(self):
         """Test user create"""
         user = create_test_user()

authentik/sources/scim/views/v2/base.py

@@ -3,6 +3,7 @@
 from typing import Any
 from uuid import UUID
 
+from django.core.exceptions import FieldError
 from django.core.paginator import Page, Paginator
 from django.db.models import Q, QuerySet
 from django.http import HttpRequest
@@ -12,6 +13,7 @@ from rest_framework.renderers import JSONRenderer
 from rest_framework.request import Request
 from rest_framework.response import Response
 from rest_framework.views import APIView
+from scim2_filter_parser.parser import SCIMParserError
 from scim2_filter_parser.transpilers.django_q_object import get_query
 from structlog import BoundLogger
 from structlog.stdlib import get_logger
@@ -21,7 +23,7 @@ from authentik.core.sources.mapper import SourceMapper
 from authentik.lib.sync.mapper import PropertyMappingManager
 from authentik.sources.scim.models import SCIMSource
 from authentik.sources.scim.views.v2.auth import SCIMTokenAuth
-from authentik.sources.scim.views.v2.exceptions import SCIMNotFoundError
+from authentik.sources.scim.views.v2.exceptions import SCIMInvalidFilterError, SCIMNotFoundError
 
 SCIM_CONTENT_TYPE = "application/scim+json"
 
@@ -62,27 +64,47 @@ class SCIMView(APIView):
             data.pop(key.strip(), None)
         return data
 
-    def filter_parse(self, request: Request):
-        """Parse the path of a Patch Operation"""
+    def filter_parse(self, request: Request) -> Q:
+        """Parse the `filter` query parameter into a Q object"""
         path = request.query_params.get("filter")
         if not path:
             return Q()
         attr_map = {}
         if self.model == User:
             attr_map = {
+                ("id", None, None): "user__uuid",
                 ("userName", None, None): "user__username",
                 ("active", None, None): "user__is_active",
                 ("name", "familyName", None): "attributes__familyName",
             }
         elif self.model == Group:
             attr_map = {
+                ("id", None, None): "group__group_uuid",
                 ("displayName", None, None): "group__name",
-                ("members", None, None): "group__users",
+                # `members` is a many-to-many relation, so it has to be filtered on a
+                # concrete field of the related user instead of on the relation itself
+                ("members", None, None): "group__users__uuid",
             }
-        return get_query(
-            path,
-            attr_map,
-        )
+        try:
+            query = get_query(path, attr_map)
+        except (SCIMParserError, ValueError) as exc:
+            self.logger.debug("Failed to parse filter", filter=path, exc=exc)
+            raise SCIMInvalidFilterError("Invalid filter.") from exc
+        if query is None:
+            # None is returned when none of the attributes in the filter can be mapped
+            # to a field, in which case we can't return any meaningful result
+            self.logger.debug("Failed to map filter to any field", filter=path)
+            raise SCIMInvalidFilterError("Unsupported filter attribute.")
+        return query
+
+    def filter_query(self, request: Request, query: QuerySet) -> QuerySet:
+        """Apply the `filter` query parameter to `query`"""
+        parsed = self.filter_parse(request)
+        try:
+            return query.filter(parsed)
+        except FieldError as exc:
+            self.logger.debug("Failed to apply filter", filter=parsed, exc=exc)
+            raise SCIMInvalidFilterError("Unsupported filter.") from exc
 
     def paginate_query(self, query: QuerySet) -> Page:
         per_page = int(self.request.tenant.pagination_default_page_size)

authentik/sources/scim/views/v2/exceptions.py

@@ -56,3 +56,16 @@ class SCIMNotFoundError(SCIMValidationError):
                 status=self.status_code,
             )
         )
+
+
+class SCIMInvalidFilterError(SCIMValidationError):
+    status_code = 400
+
+    def __init__(self, detail: str):
+        super().__init__(
+            SCIMError(
+                detail=detail,
+                scimType=SCIMErrorTypes.invalid_filter,
+                status=self.status_code,
+            )
+        )

authentik/sources/scim/views/v2/groups.py

@@ -72,8 +72,8 @@ class GroupsView(SCIMObjectView):
             if not connection:
                 raise SCIMNotFoundError("Group not found.")
             return Response(self.group_to_scim(connection))
-        connections = (
-            base_query.filter(source=self.source).order_by("pk").filter(self.filter_parse(request))
+        connections = self.filter_query(
+            request, base_query.filter(source=self.source).order_by("pk")
         )
         page = self.paginate_query(connections)
         return Response(

authentik/sources/scim/views/v2/users.py

@@ -74,10 +74,10 @@ class UsersView(SCIMObjectView):
             if not connection:
                 raise SCIMNotFoundError("User not found.")
             return Response(self.user_to_scim(connection))
-        connections = (
-            SCIMSourceUser.objects.filter(source=self.source).select_related("user").order_by("pk")
+        connections = self.filter_query(
+            request,
+            SCIMSourceUser.objects.filter(source=self.source).select_related("user").order_by("pk"),
         )
-        connections = connections.filter(self.filter_parse(request))
         page = self.paginate_query(connections)
         return Response(
             {