diff --git a/src/documents/tests/test_permission_filtering_security.py b/src/documents/tests/test_permission_filtering_security.py index 018d5bb94..f48095d48 100644 --- a/src/documents/tests/test_permission_filtering_security.py +++ b/src/documents/tests/test_permission_filtering_security.py @@ -430,6 +430,53 @@ class TestBulkDownloadPermissionChecksRootDocument: ) # version-only grant must not substitute for root permission +@pytest.mark.django_db +class TestDocumentOperationPermissionChecksRootDocument: + @pytest.mark.parametrize( + ("endpoint", "payload"), + [ + pytest.param("/api/documents/merge/", {}, id="merge"), + pytest.param("/api/documents/rotate/", {"degrees": 90}, id="rotate"), + ], + ) + @pytest.mark.parametrize("version_owner", ["none", "requester"]) + def test_version_operation_acts_on_root( + self, + rest_api_client: APIClient, + endpoint: str, + payload: dict, + version_owner: str, + ) -> None: + owner = UserFactory(username="owner") + requester = UserFactory(username="requester") + grant_global(requester, "change_document") + grant_global(requester, "add_document") + rest_api_client.force_authenticate(user=requester) + root = DocumentFactory(owner=owner) + # A version whose owner went stale, e.g. created before the root changed hands + version = DocumentFactory( + owner=requester if version_owner == "requester" else None, + root_document=root, + version_index=1, + ) + + with ( + patch("documents.views.bulk_edit.merge") as mock_merge, + patch("documents.views.bulk_edit.rotate") as mock_rotate, + ): + mock_merge.__name__ = "merge" + mock_rotate.__name__ = "rotate" + response = rest_api_client.post( + endpoint, + {"documents": [version.pk], **payload}, + format="json", + ) + + assert response.status_code == HTTPStatus.FORBIDDEN + mock_merge.assert_not_called() + mock_rotate.assert_not_called() + + @pytest.mark.django_db @pytest.mark.usefixtures("_search_index") class TestTrashRestorePermissionBoundary: diff --git a/src/documents/views.py b/src/documents/views.py index e36f181a5..7a9b90406 100644 --- a/src/documents/views.py +++ b/src/documents/views.py @@ -2967,11 +2967,15 @@ class DocumentOperationPermissionMixin(PassUserMixin, DocumentSelectionMixin): if user.is_superuser: return True - document_objs = Document.objects.select_related("owner").filter( - pk__in=documents, - ) + root_docs = { + get_root_document(doc) + for doc in Document.objects.select_related( + "owner", + "root_document__owner", + ).filter(pk__in=documents) + } user_is_owner_of_all_documents = all( - (doc.owner == user or doc.owner is None) for doc in document_objs + (doc.owner == user or doc.owner is None) for doc in root_docs ) # check global and object permissions for all documents @@ -2979,9 +2983,13 @@ class DocumentOperationPermissionMixin(PassUserMixin, DocumentSelectionMixin): user.has_perm( "documents.change_document", ) - and not document_objs.exclude( + and not Document.global_objects.filter( + pk__in=[doc.pk for doc in root_docs], + ) + .exclude( pk__in=permitted_document_ids(user, perm="change_document"), - ).exists() + ) + .exists() ) # check ownership for methods that change original document