Fix: dedupe permission-visible documents when combined with multi-tag ALL filtering (#13331) (#13345)

The permission filter OR'd three querysets together on top of a
queryset that could already carry two independent tags__id__all
joins, letting a document that matched more than one branch (e.g.
unowned + group-permissioned) come back twice. Replaced it with a
single id__in filter against the existing permitted_document_ids
helper, which is join-free and can't hit this.

src/documents/filters.py

@@ -37,6 +37,7 @@ from drf_spectacular.utils import extend_schema_field
 from guardian.utils import get_group_obj_perms_model
 from guardian.utils import get_user_obj_perms_model
 from rest_framework import serializers
+from rest_framework.filters import BaseFilterBackend
 from rest_framework.filters import OrderingFilter
 from rest_framework_guardian.filters import ObjectPermissionsFilter
 
@@ -50,6 +51,7 @@ from documents.models import ShareLink
 from documents.models import ShareLinkBundle
 from documents.models import StoragePath
 from documents.models import Tag
+from documents.permissions import permitted_document_ids
 
 if TYPE_CHECKING:
     from collections.abc import Callable
@@ -1038,6 +1040,31 @@ class ObjectOwnedOrGrantedPermissionsFilter(ObjectPermissionsFilter):
         return objects_with_perms | objects_owned | objects_unowned
 
 
+class DocumentPermissionsFilter(BaseFilterBackend):
+    """
+    A filter backend limiting Document results to those the requesting user
+    owns, are unowned, or has explicit (user- or group-level) view
+    permission on.
+
+    Unlike ``ObjectOwnedOrGrantedPermissionsFilter``, this does not build an
+    ``objects_with_perms | objects_owned | objects_unowned`` union of
+    querysets derived from the same base queryset. When that base queryset
+    already carries independent joins on a multi-valued relation (e.g. two
+    separate joins from ``tags__id__all`` filtering on two tags), each
+    OR-ed branch can end up pairing those joins' aliases differently,
+    letting more than one row out of the join's cross product satisfy the
+    combined WHERE -- returning the same document more than once. Filtering
+    via a single ``id__in`` against ``permitted_document_ids`` (a plain
+    subquery, not a join) sidesteps that entirely and is also cheaper than
+    guardian's join-based permission check.
+    """
+
+    def filter_queryset(self, request, queryset, view):
+        if request.user.is_superuser:
+            return queryset
+        return queryset.filter(id__in=permitted_document_ids(request.user))
+
+
 class ObjectOwnedPermissionsFilter(ObjectPermissionsFilter):
     """
     A filter backend that limits results to those where the requesting user

src/documents/permissions.py

@@ -163,7 +163,7 @@ def set_permissions_for_object(
                         )
 
 
-def _permitted_document_ids(user):
+def permitted_document_ids(user):
     """
     Return a queryset of document IDs the user may view, limited to non-deleted
     documents. This intentionally avoids ``get_objects_for_user`` to keep the
@@ -220,7 +220,7 @@ def get_document_count_filter_for_user(user, related_name: str = "documents"):
         # Superuser: no permission filtering needed
         return Q(**{f"{related_name}__deleted_at__isnull": True})
 
-    permitted_ids = _permitted_document_ids(user)
+    permitted_ids = permitted_document_ids(user)
     return Q(**{f"{related_name}__id__in": permitted_ids})
 
 
@@ -311,7 +311,7 @@ def annotate_document_count_for_related_queryset(
         queryset,
         through_model=through_model,
         related_object_field=related_object_field,
-        document_ids=_permitted_document_ids(user),
+        document_ids=permitted_document_ids(user),
         target_field=target_field,
     )
 

src/documents/tests/test_api_documents.py

@@ -14,6 +14,7 @@ from unittest import mock
 import celery
 from dateutil import parser
 from django.conf import settings
+from django.contrib.auth.models import Group
 from django.contrib.auth.models import Permission
 from django.contrib.auth.models import User
 from django.core import mail
@@ -47,6 +48,8 @@ from documents.models import Workflow
 from documents.models import WorkflowAction
 from documents.models import WorkflowTrigger
 from documents.signals.handlers import run_workflows
+from documents.tests.factories import DocumentFactory
+from documents.tests.factories import TagFactory
 from documents.tests.utils import ConsumeTaskMixin
 from documents.tests.utils import DirectoriesMixin
 from documents.tests.utils import read_streaming_response
@@ -1212,6 +1215,91 @@ class TestDocumentApi(DirectoriesMixin, ConsumeTaskMixin, APITestCase):
             [u1_doc1.id],
         )
 
+    def test_document_owned_and_group_shared_not_duplicated_when_filtering_by_tags(
+        self,
+    ) -> None:
+        """
+        GIVEN:
+            - A document owned by a user and also shared with a group the user belongs to
+        WHEN:
+            - The user filters documents by more than one tag (tags__id__all)
+        THEN:
+            - The document is returned exactly once, not once per permission path
+            (regression test for https://github.com/paperless-ngx/paperless-ngx/issues/13331)
+        """
+        user = User.objects.create_user("user1")
+        user.user_permissions.add(*Permission.objects.filter(codename="view_document"))
+        group = Group.objects.create(name="group1")
+        user.groups.add(group)
+
+        tag1 = TagFactory()
+        tag2 = TagFactory()
+        doc = DocumentFactory(title="shared", owner=user)
+        doc.tags.add(tag1, tag2)
+        assign_perm("view_document", group, doc)
+
+        self.client.force_authenticate(user=user)
+        response = self.client.get(
+            f"/api/documents/?tags__id__all={tag1.id},{tag2.id}",
+        )
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertEqual(response.data["count"], 1)
+        self.assertEqual(response.data["results"][0]["id"], doc.id)
+
+    def test_document_permission_filter_excludes_unrelated_documents(self) -> None:
+        """
+        GIVEN:
+            - A document owned by one user, with no permission granted to another user
+        WHEN:
+            - The unrelated user requests the document list
+        THEN:
+            - The document does not appear in their results
+        """
+        owner = User.objects.create_user("owner1")
+        stranger = User.objects.create_user("stranger1")
+        stranger.user_permissions.add(
+            *Permission.objects.filter(codename="view_document"),
+        )
+
+        DocumentFactory(title="private", owner=owner)
+
+        self.client.force_authenticate(user=stranger)
+        response = self.client.get("/api/documents/")
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertEqual(response.data["count"], 0)
+
+    def test_document_permission_filter_only_visible_to_group_members(self) -> None:
+        """
+        GIVEN:
+            - A document shared with a group via object permissions
+        WHEN:
+            - A group member and a non-member both request the document list
+        THEN:
+            - Only the group member sees the document
+        """
+        owner = User.objects.create_user("owner2")
+        member = User.objects.create_user("member1")
+        non_member = User.objects.create_user("nonmember1")
+        for u in (member, non_member):
+            u.user_permissions.add(*Permission.objects.filter(codename="view_document"))
+
+        group = Group.objects.create(name="group2")
+        member.groups.add(group)
+
+        doc = DocumentFactory(title="shared2", owner=owner)
+        assign_perm("view_document", group, doc)
+
+        self.client.force_authenticate(user=member)
+        response = self.client.get("/api/documents/")
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertEqual(response.data["count"], 1)
+        self.assertEqual(response.data["results"][0]["id"], doc.id)
+
+        self.client.force_authenticate(user=non_member)
+        response = self.client.get("/api/documents/")
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertEqual(response.data["count"], 0)
+
     def test_pagination_results(self) -> None:
         """
         GIVEN:

src/documents/views.py

@@ -133,6 +133,7 @@ from documents.file_handling import format_filename
 from documents.filters import CorrespondentFilterSet
 from documents.filters import CustomFieldFilterSet
 from documents.filters import DocumentFilterSet
+from documents.filters import DocumentPermissionsFilter
 from documents.filters import DocumentsOrderingFilter
 from documents.filters import DocumentTypeFilterSet
 from documents.filters import ObjectOwnedOrGrantedPermissionsFilter
@@ -986,7 +987,7 @@ class DocumentViewSet(
         DjangoFilterBackend,
         SearchFilter,
         DocumentsOrderingFilter,
-        ObjectOwnedOrGrantedPermissionsFilter,
+        DocumentPermissionsFilter,
     )
     filterset_class = DocumentFilterSet
     search_fields = ("title", "correspondent__name", "effective_content")