diff --git a/src/documents/tests/test_api_bulk_edit.py b/src/documents/tests/test_api_bulk_edit.py index ce3b24b5c..e4272e78f 100644 --- a/src/documents/tests/test_api_bulk_edit.py +++ b/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") diff --git a/src/documents/tests/test_api_document_versions.py b/src/documents/tests/test_api_document_versions.py index 3ff32998f..38aa78706 100644 --- a/src/documents/tests/test_api_document_versions.py +++ b/src/documents/tests/test_api_document_versions.py @@ -669,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/", diff --git a/src/documents/views.py b/src/documents/views.py index 3c49d6c76..ae83fe23c 100644 --- a/src/documents/views.py +++ b/src/documents/views.py @@ -1947,10 +1947,13 @@ class DocumentViewSet( "root_document", ).get(pk=pk) root_doc = get_root_document(request_doc) - if request.user is not None and not has_perms_owner_aware( - request.user, - "change_document", - root_doc, + if request.user is not None and ( + not request.user.has_perm("documents.change_document") + or not has_perms_owner_aware( + request.user, + "change_document", + root_doc, + ) ): return HttpResponseForbidden("Insufficient permissions") except Document.DoesNotExist: @@ -2756,7 +2759,7 @@ class DocumentOperationPermissionMixin(PassUserMixin, DocumentSelectionMixin): ) or (method == bulk_edit.edit_pdf and parameters.get("update_document")) ): - has_perms = user_is_owner_of_all_documents + has_perms = has_perms and user_is_owner_of_all_documents # check global add permissions for methods that create documents if ( @@ -2781,6 +2784,11 @@ class DocumentOperationPermissionMixin(PassUserMixin, DocumentSelectionMixin): method in [bulk_edit.merge, bulk_edit.split] and parameters.get("delete_originals") ) + or ( + method in [bulk_edit.edit_pdf, bulk_edit.remove_password] + and parameters.get("delete_original") + and not parameters.get("update_document") + ) ) and not user.has_perm("documents.delete_document") ):