documents_without_permitted_root helper used x4

src/documents/permissions.py

@@ -14,6 +14,7 @@ from django.db.models import QuerySet
 from django.db.models import Value
 from django.db.models import When
 from django.db.models.functions import Cast
+from django.db.models.functions import Coalesce
 from guardian.core import ObjectPermissionChecker
 from guardian.models import GroupObjectPermission
 from guardian.models import UserObjectPermission
@@ -474,6 +475,29 @@ def permitted_document_ids(
     return permitted_object_ids(user, Document, perm, include_deleted=include_deleted)
 
 
+def documents_without_permitted_root(
+    documents: QuerySet[Document],
+    user: User | None,
+    *,
+    perm: str = "view_document",
+    include_deleted: bool = False,
+) -> QuerySet[Document]:
+    """
+    The documents the user lacks ``perm`` on. Versions are authorized by their
+    root document, so a version's own owner is ignored. A single query, without
+    loading the documents or joining the root.
+    """
+    return documents.annotate(
+        root_id=Coalesce("root_document_id", "id"),
+    ).exclude(
+        root_id__in=permitted_document_ids(
+            user,
+            perm=perm,
+            include_deleted=include_deleted,
+        ),
+    )
+
+
 def get_document_count_filter_for_user(user, related_name: str = "documents"):
     """
     Return the Q object used to filter document counts for the given user.

src/documents/tests/test_share_link_bundles.py

@@ -90,10 +90,13 @@ class ShareLinkBundleAPITests(DirectoriesMixin, APITestCase):
         self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
         self.assertIn("document_ids", response.data)
 
-    @mock.patch("documents.views.permitted_document_ids", return_value=set())
-    def test_create_bundle_rejects_insufficient_permissions(self, perms_mock) -> None:
+    def test_create_bundle_rejects_insufficient_permissions(self) -> None:
+        requester = UserFactory(username="bundle_creator")
+        grant_global(requester, "add_sharelinkbundle", "view_document")
+        self.client.force_authenticate(requester)
+        document = DocumentFactory(owner=UserFactory(username="document_owner"))
         payload = {
-            "document_ids": [self.document.pk],
+            "document_ids": [self.document.pk, document.pk],
             "file_version": ShareLink.FileVersion.ARCHIVE,
             "expiration_days": 7,
         }
@@ -101,8 +104,8 @@ class ShareLinkBundleAPITests(DirectoriesMixin, APITestCase):
         response = self.client.post(self.ENDPOINT, payload, format="json")
 
         self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
-        self.assertIn("document_ids", response.data)
-        perms_mock.assert_called()
+        self.assertIn(str(document.pk), str(response.data["document_ids"]))
+        self.assertFalse(ShareLinkBundle.objects.exists())
 
     @mock.patch("documents.views.build_share_link_bundle.apply_async")
     def test_rebuild_bundle_resets_state(self, delay_mock) -> None:

src/documents/views.py

@@ -174,6 +174,7 @@ from documents.permissions import TrashPermissions
 from documents.permissions import ViewDocumentsPermissions
 from documents.permissions import annotate_document_count_by_ids
 from documents.permissions import annotate_document_count_for_related_queryset
+from documents.permissions import documents_without_permitted_root
 from documents.permissions import get_document_count_filter_for_user
 from documents.permissions import get_objects_for_user_owner_aware
 from documents.permissions import has_global_statistics_permission
@@ -2113,13 +2114,12 @@ class DocumentViewSet(
         message = validated_data.get("message")
         use_archive_version = validated_data.get("use_archive_version", True)
 
-        documents = Document.objects.filter(pk__in=document_ids).select_related(
-            "root_document__owner",
-        )
-        if request.user is not None:
-            permitted_ids = set(permitted_document_ids(request.user))
-            if any(get_root_document(doc).pk not in permitted_ids for doc in documents):
-                return HttpResponseForbidden("Insufficient permissions")
+        documents = Document.objects.filter(pk__in=document_ids)
+        if (
+            request.user is not None
+            and documents_without_permitted_root(documents, request.user).exists()
+        ):
+            return HttpResponseForbidden("Insufficient permissions")
 
         try:
             attachments: list[EmailAttachment] = []
@@ -3624,13 +3624,11 @@ class SelectionDataView(DocumentSelectionMixin, GenericAPIView[Any]):
             user=request.user,
             validated_data=serializer.validated_data,
         )
-        # Versions are authorized by their root document
-        permitted_documents = Document.objects.annotate(
-            root_id=Coalesce("root_document_id", "id"),
-        ).filter(
-            root_id__in=permitted_document_ids(request.user),
-        )
-        if permitted_documents.filter(pk__in=ids).count() != len(ids):
+        documents = Document.objects.filter(pk__in=ids)
+        if (
+            documents.count() != len(ids)
+            or documents_without_permitted_root(documents, request.user).exists()
+        ):
             return HttpResponseForbidden("Insufficient permissions")
 
         correspondents = Correspondent.objects.annotate(
@@ -4799,7 +4797,6 @@ class ShareLinkBundleViewSet(PassUserMixin, ModelViewSet[ShareLinkBundle]):
         document_ids = serializer.validated_data["document_ids"]
         documents_qs = Document.objects.filter(pk__in=document_ids).select_related(
             "owner",
-            "root_document__owner",
         )
         found_ids = set(documents_qs.values_list("pk", flat=True))
         missing = sorted(set(document_ids) - found_ids)
@@ -4813,19 +4810,23 @@ class ShareLinkBundleViewSet(PassUserMixin, ModelViewSet[ShareLinkBundle]):
                 },
             )
 
-        documents = list(documents_qs)
-        permitted_ids = set(permitted_document_ids(request.user))
-        for document in documents:
-            if get_root_document(document).pk not in permitted_ids:
-                raise ValidationError(
-                    {
-                        "document_ids": _(
-                            "Insufficient permissions to share document %(id)s.",
-                        )
-                        % {"id": document.pk},
-                    },
-                )
+        denied_id = (
+            documents_without_permitted_root(documents_qs, request.user)
+            .order_by("pk")
+            .values_list("pk", flat=True)
+            .first()
+        )
+        if denied_id is not None:
+            raise ValidationError(
+                {
+                    "document_ids": _(
+                        "Insufficient permissions to share document %(id)s.",
+                    )
+                    % {"id": denied_id},
+                },
+            )
 
+        documents = list(documents_qs)
         document_map = {document.pk: document for document in documents}
         ordered_documents = [document_map[doc_id] for doc_id in document_ids]
 
@@ -5656,18 +5657,12 @@ class TrashView(ListModelMixin, PassUserMixin):
             if doc_ids is not None
             else self.filter_queryset(self.get_queryset()).all()
         )
-        # Versions are authorized by their root document
-        if (
-            docs.annotate(root_id=Coalesce("root_document_id", "id"))
-            .exclude(
-                root_id__in=permitted_document_ids(
-                    request.user,
-                    perm="delete_document",
-                    include_deleted=True,
-                ),
-            )
-            .exists()
-        ):
+        if documents_without_permitted_root(
+            docs,
+            request.user,
+            perm="delete_document",
+            include_deleted=True,
+        ).exists():
             return HttpResponseForbidden("Insufficient permissions")
         action = serializer.validated_data.get("action")
         if action == "restore":