Merge branch 'beta' into beta-sec-autocomplete-perms

src/documents/tests/test_api_bulk_edit.py

@@ -1419,6 +1419,30 @@ class TestBulkEditAPI(DirectoriesMixin, APITestCase):
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         m.assert_called_once()
 
+    @mock.patch("documents.views.bulk_edit.merge")
+    def test_merge_and_delete_requires_change_permission(self, m) -> None:
+        self.setup_mock(m, "merge")
+        user = User.objects.create_user(username="no-change")
+        user.user_permissions.add(
+            Permission.objects.get(codename="add_document"),
+            Permission.objects.get(codename="delete_document"),
+        )
+        self.client.force_authenticate(user=user)
+
+        response = self.client.post(
+            "/api/documents/merge/",
+            json.dumps(
+                {
+                    "documents": [self.doc2.id, self.doc3.id],
+                    "delete_originals": True,
+                },
+            ),
+            content_type="application/json",
+        )
+
+        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+        m.assert_not_called()
+
     @mock.patch("documents.views.bulk_edit.merge")
     def test_merge_invalid_parameters(self, m) -> None:
         self.setup_mock(m, "merge")
@@ -1668,6 +1692,74 @@ class TestBulkEditAPI(DirectoriesMixin, APITestCase):
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         m.assert_called_once()
 
+    @mock.patch("documents.views.bulk_edit.edit_pdf")
+    def test_edit_pdf_update_requires_change_permission(self, m) -> None:
+        self.setup_mock(m, "edit_pdf")
+        user = User.objects.create_user(username="no-change")
+        self.client.force_authenticate(user=user)
+
+        response = self.client.post(
+            "/api/documents/edit_pdf/",
+            json.dumps(
+                {
+                    "documents": [self.doc2.id],
+                    "operations": [{"page": 1}],
+                    "update_document": True,
+                },
+            ),
+            content_type="application/json",
+        )
+
+        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+        m.assert_not_called()
+
+    @mock.patch("documents.views.bulk_edit.remove_password")
+    @mock.patch("documents.views.bulk_edit.edit_pdf")
+    def test_delete_original_requires_delete_permission(
+        self,
+        edit_pdf_mock,
+        remove_password_mock,
+    ) -> None:
+        self.setup_mock(edit_pdf_mock, "edit_pdf")
+        self.setup_mock(remove_password_mock, "remove_password")
+        user = User.objects.create_user(username="no-delete")
+        user.user_permissions.add(
+            Permission.objects.get(codename="add_document"),
+            Permission.objects.get(codename="change_document"),
+        )
+        self.client.force_authenticate(user=user)
+
+        cases = [
+            (
+                "/api/documents/edit_pdf/",
+                {
+                    "documents": [self.doc2.id],
+                    "operations": [{"page": 1}],
+                    "delete_original": True,
+                },
+                edit_pdf_mock,
+            ),
+            (
+                "/api/documents/remove_password/",
+                {
+                    "documents": [self.doc2.id],
+                    "password": "secret",
+                    "delete_original": True,
+                },
+                remove_password_mock,
+            ),
+        ]
+        for endpoint, payload, operation_mock in cases:
+            with self.subTest(endpoint=endpoint):
+                response = self.client.post(
+                    endpoint,
+                    json.dumps(payload),
+                    content_type="application/json",
+                )
+
+                self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+                operation_mock.assert_not_called()
+
     @mock.patch("documents.views.bulk_edit.remove_password")
     def test_remove_password(self, m) -> None:
         self.setup_mock(m, "remove_password")

src/documents/tests/test_api_document_versions.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import datetime
 from typing import TYPE_CHECKING
 from unittest import TestCase
 from unittest import mock
@@ -10,6 +11,7 @@ from django.contrib.auth.models import User
 from django.contrib.contenttypes.models import ContentType
 from django.core.exceptions import FieldError
 from django.core.files.uploadedfile import SimpleUploadedFile
+from django.utils import timezone
 from rest_framework import status
 from rest_framework.test import APITestCase
 
@@ -137,6 +139,8 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase):
             root_document=root,
             content="v2-content",
         )
+        original_modified = timezone.now() - datetime.timedelta(days=1)
+        Document.objects.filter(pk=root.pk).update(modified=original_modified)
 
         with mock.patch("documents.search.get_backend"):
             resp = self.client.delete(f"/api/documents/{root.id}/versions/{v2.id}/")
@@ -146,6 +150,7 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase):
         self.assertEqual(resp.data["current_version_id"], v1.id)
         root.refresh_from_db()
         self.assertEqual(root.content, "root-content")
+        self.assertGreater(root.modified, original_modified)
 
         with mock.patch("documents.search.get_backend"):
             resp = self.client.delete(f"/api/documents/{root.id}/versions/{v1.id}/")
@@ -326,6 +331,8 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase):
             root_document=root,
             version_label="old",
         )
+        original_modified = timezone.now() - datetime.timedelta(days=1)
+        Document.objects.filter(pk=root.pk).update(modified=original_modified)
 
         resp = self.client.patch(
             f"/api/documents/{root.id}/versions/{version.id}/",
@@ -339,6 +346,8 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase):
         self.assertEqual(resp.data["version_label"], "Label 1")
         self.assertEqual(resp.data["id"], version.id)
         self.assertFalse(resp.data["is_root"])
+        root.refresh_from_db()
+        self.assertGreater(root.modified, original_modified)
 
     def test_update_version_label_clears_on_blank(self) -> None:
         root = Document.objects.create(
@@ -660,6 +669,26 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase):
 
         self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)
 
+    def test_update_version_requires_global_change_permission(self) -> None:
+        user = User.objects.create_user(username="add-only")
+        user.user_permissions.add(Permission.objects.get(codename="add_document"))
+        root = Document.objects.create(
+            title="root",
+            checksum="root",
+            mime_type="application/pdf",
+        )
+        self.client.force_authenticate(user=user)
+
+        with mock.patch("documents.views.consume_file") as consume_mock:
+            resp = self.client.post(
+                f"/api/documents/{root.id}/update_version/",
+                {"document": self._make_pdf_upload()},
+                format="multipart",
+            )
+
+        self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)
+        consume_mock.apply_async.assert_not_called()
+
     def test_update_version_returns_404_for_missing_document(self) -> None:
         resp = self.client.post(
             "/api/documents/9999/update_version/",

src/documents/tests/test_consumer.py

@@ -767,6 +767,8 @@ class TestConsumer(
         root_doc.archive_serial_number = 42
         root_doc.save()
 
+        original_modified = timezone.now() - datetime.timedelta(days=1)
+        Document.objects.filter(pk=root_doc.pk).update(modified=original_modified)
         actor = User.objects.create_user(
             username="actor",
             email="actor@example.com",
@@ -818,6 +820,8 @@ class TestConsumer(
         self.assertIsNone(version.archive_serial_number)
         self.assertEqual(version.original_filename, version_file.name)
         self.assertTrue(bool(version.content))
+        root_doc.refresh_from_db()
+        self.assertGreater(root_doc.modified, original_modified)
 
     @override_settings(AUDIT_LOG_ENABLED=True)
     @mock.patch("documents.consumer.load_classifier")

src/documents/tests/test_version_conditionals.py

@@ -1,8 +1,12 @@
 from datetime import timedelta
 from types import SimpleNamespace
+from typing import TYPE_CHECKING
+from typing import cast
 from unittest import mock
 
+from django.db import connection
 from django.test import TestCase
+from django.test.utils import CaptureQueriesContext
 from django.utils import timezone
 
 from documents.conditionals import metadata_etag
@@ -13,6 +17,9 @@ from documents.models import Document
 from documents.tests.utils import DirectoriesMixin
 from documents.versioning import resolve_effective_document_by_pk
 
+if TYPE_CHECKING:
+    from rest_framework.request import Request
+
 
 class TestConditionals(DirectoriesMixin, TestCase):
     def test_metadata_etag_uses_latest_version_for_root_request(self) -> None:
@@ -29,14 +36,18 @@ class TestConditionals(DirectoriesMixin, TestCase):
             mime_type="application/pdf",
             root_document=root,
         )
-        request = SimpleNamespace(query_params={})
+        request = cast("Request", SimpleNamespace(query_params={}))
 
         self.assertEqual(
             metadata_etag(request, root.id),
             f"{latest.checksum}:{latest.modified.isoformat()}",
         )
-        self.assertEqual(preview_etag(request, root.id), latest.archive_checksum)
-        self.assertEqual(thumbnail_etag(request, root.id), latest.checksum)
+        # preview_etag/thumbnail_etag resolve the same (pk, request) and should
+        # reuse metadata_etag's cached resolution instead of re-querying.
+        with CaptureQueriesContext(connection) as ctx:
+            self.assertEqual(preview_etag(request, root.id), latest.archive_checksum)
+            self.assertEqual(thumbnail_etag(request, root.id), latest.checksum)
+        self.assertEqual(len(ctx.captured_queries), 0)
 
     def test_metadata_etag_changes_when_document_modified_changes(self) -> None:
         doc = Document.objects.create(
@@ -44,15 +55,20 @@ class TestConditionals(DirectoriesMixin, TestCase):
             checksum="same-checksum",
             mime_type="application/pdf",
         )
-        request = SimpleNamespace(query_params={})
-
-        original_etag = metadata_etag(request, doc.id)
+        # Each call simulates a separate incoming HTTP request, so it gets its
+        # own request object -- the per-request resolution cache must not leak
+        # a stale result across genuinely different requests.
+        original_etag = metadata_etag(
+            cast("Request", SimpleNamespace(query_params={})),
+            doc.id,
+        )
         new_modified = timezone.now() + timedelta(seconds=5)
         Document.objects.filter(id=doc.id).update(modified=new_modified)
 
-        self.assertNotEqual(metadata_etag(request, doc.id), original_etag)
+        second_request = cast("Request", SimpleNamespace(query_params={}))
+        self.assertNotEqual(metadata_etag(second_request, doc.id), original_etag)
         self.assertEqual(
-            metadata_etag(request, doc.id),
+            metadata_etag(second_request, doc.id),
             f"{doc.checksum}:{new_modified.isoformat()}",
         )
 
@@ -76,9 +92,13 @@ class TestConditionals(DirectoriesMixin, TestCase):
             root_document=other_root,
         )
 
-        invalid_request = SimpleNamespace(query_params={"version": "not-a-number"})
-        unrelated_request = SimpleNamespace(
-            query_params={"version": str(other_version.id)},
+        invalid_request = cast(
+            "Request",
+            SimpleNamespace(query_params={"version": "not-a-number"}),
+        )
+        unrelated_request = cast(
+            "Request",
+            SimpleNamespace(query_params={"version": str(other_version.id)}),
         )
 
         self.assertIsNone(
@@ -105,7 +125,7 @@ class TestConditionals(DirectoriesMixin, TestCase):
         latest.thumbnail_path.parent.mkdir(parents=True, exist_ok=True)
         latest.thumbnail_path.write_bytes(b"thumb")
 
-        request = SimpleNamespace(query_params={})
+        request = cast("Request", SimpleNamespace(query_params={}))
         with mock.patch(
             "documents.conditionals.get_thumbnail_modified_key",
             return_value="thumb-modified-key",

src/documents/tests/test_workflows.py

@@ -2938,6 +2938,69 @@ class TestWorkflows(
         self.assertFalse(doc.tags.filter(pk=self.t1.pk).exists())
         self.assertTrue(doc.tags.filter(pk=self.t2.pk).exists())
 
+    def test_document_updated_workflow_assignment_storage_path_persists_with_tag_assignment(
+        self,
+    ) -> None:
+        """
+        GIVEN:
+            - A document updated workflow filtered on a tag
+            - One assignment action assigns a storage path, a second (later-ordered)
+              assignment action adds a tag
+        WHEN:
+            - The document is updated and the workflow is triggered
+        THEN:
+            - Both the tag and the storage path are persisted
+        """
+        trigger = WorkflowTrigger.objects.create(
+            type=WorkflowTrigger.WorkflowTriggerType.DOCUMENT_UPDATED,
+        )
+        trigger.filter_has_tags.add(self.t1)
+        assign_storage_path = WorkflowAction.objects.create(
+            type=WorkflowAction.WorkflowActionType.ASSIGNMENT,
+            assign_storage_path=self.sp,
+            order=0,
+        )
+        assign_tag = WorkflowAction.objects.create(
+            type=WorkflowAction.WorkflowActionType.ASSIGNMENT,
+            order=1,
+        )
+        assign_tag.assign_tags.add(self.t2)
+        assign_tag.save()
+
+        workflow = Workflow.objects.create(
+            name="Workflow assign storage path then tag",
+            order=0,
+        )
+        workflow.triggers.add(trigger)
+        workflow.actions.add(assign_storage_path, assign_tag)
+        workflow.save()
+
+        doc = Document.objects.create(
+            title="sample test",
+            mime_type="application/pdf",
+            checksum="assign-tag-and-storage-path",
+            original_filename="sample.pdf",
+        )
+        generated = generate_unique_filename(doc)
+        destination = (settings.ORIGINALS_DIR / generated).resolve()
+        create_source_path_directory(destination)
+        shutil.copy(self.SAMPLE_DIR / "simple.pdf", destination)
+        Document.objects.filter(pk=doc.pk).update(filename=generated.as_posix())
+        doc.refresh_from_db()
+        doc.tags.set([self.t1])
+
+        superuser = User.objects.create_superuser("superuser")
+        self.client.force_authenticate(user=superuser)
+        self.client.patch(
+            f"/api/documents/{doc.id}/",
+            {"title": "user update to trigger workflow"},
+            format="json",
+        )
+
+        doc.refresh_from_db()
+        self.assertEqual(doc.storage_path, self.sp)
+        self.assertTrue(doc.tags.filter(pk=self.t2.pk).exists())
+
     def test_removal_action_document_updated_removeall(self) -> None:
         """
         GIVEN: