diff --git a/src/paperless_mail/tests/test_api.py b/src/paperless_mail/tests/test_api.py index 088ee0eb6..64f456795 100644 --- a/src/paperless_mail/tests/test_api.py +++ b/src/paperless_mail/tests/test_api.py @@ -757,3 +757,30 @@ class TestAPIProcessedMails(DirectoriesMixin, APITestCase): format="json", ) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + def test_bulk_delete_processed_mails_rejects_mixed_batch_atomically(self) -> None: + """ + GIVEN: + - A permitted processed mail and one the user may not delete + WHEN: + - API call bulk deletes both in a single request + THEN: + - The request is rejected and neither mail is deleted + """ + user2 = User.objects.create_user(username="temp_admin2") + rule = MailRuleFactory() + # Created first so it sorts ahead of the forbidden mail, i.e. the + # permission check has to cover the whole batch before deleting rather + # than rejecting only once it reaches the forbidden one. + pm_owned = ProcessedMailFactory(rule=rule, owner=self.user) + pm_forbidden = ProcessedMailFactory(rule=rule, owner=user2) + + response = self.client.post( + f"{self.ENDPOINT}bulk_delete/", + data={"mail_ids": [pm_owned.id, pm_forbidden.id]}, + format="json", + ) + + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertTrue(ProcessedMail.objects.filter(id=pm_owned.id).exists()) + self.assertTrue(ProcessedMail.objects.filter(id=pm_forbidden.id).exists()) diff --git a/src/paperless_mail/views.py b/src/paperless_mail/views.py index ec510b4e4..2a8f93b9e 100644 --- a/src/paperless_mail/views.py +++ b/src/paperless_mail/views.py @@ -27,6 +27,7 @@ from documents.filters import PermittedObjectsFilter from documents.models import PaperlessTask from documents.permissions import PaperlessObjectPermissions from documents.permissions import has_perms_owner_aware +from documents.permissions import permitted_object_ids from documents.views import PassUserMixin from paperless.views import StandardPagination from paperless_mail.filters import ProcessedMailFilterSet @@ -211,10 +212,17 @@ class ProcessedMailViewSet(PassUserMixin, ReadOnlyModelViewSet[ProcessedMail]): ): return HttpResponseBadRequest("mail_ids must be a list of integers") mails = ProcessedMail.objects.filter(id__in=mail_ids) - for mail in mails: - if not has_perms_owner_aware(request.user, "delete_processedmail", mail): - return HttpResponseForbidden("Insufficient permissions") - mail.delete() + # Check every id up front so an unpermitted one rejects the whole + # request rather than deleting the mails ahead of it first. + if mails.exclude( + pk__in=permitted_object_ids( + request.user, + ProcessedMail, + "delete_processedmail", + ), + ).exists(): + return HttpResponseForbidden("Insufficient permissions") + mails.delete() return Response({"result": "OK", "deleted_mail_ids": mail_ids})