Compare commits

..
Author SHA1 Message Date
Trenton Holmes 0ff4b2d8dd Fix: check bulk mail delete permissions for the whole batch up front
ProcessedMailViewSet.bulk_delete checked permissions inside the delete
loop, so an unpermitted id returned 403 only after the mails ahead of it
had already been deleted. Resolve the permitted set once via
permitted_object_ids and reject before deleting anything, which also
drops the per-mail permission queries.
2026-08-08 14:48:49 -07:00
6 changed files with 47 additions and 71 deletions
+6 -13
View File
@@ -1703,7 +1703,7 @@
</context-group> </context-group>
<context-group purpose="location"> <context-group purpose="location">
<context context-type="sourcefile">src/app/components/common/suggestions-dropdown/suggestions-dropdown.component.html</context> <context context-type="sourcefile">src/app/components/common/suggestions-dropdown/suggestions-dropdown.component.html</context>
<context context-type="linenumber">34</context> <context context-type="linenumber">28</context>
</context-group> </context-group>
<context-group purpose="location"> <context-group purpose="location">
<context context-type="sourcefile">src/app/components/dashboard/widgets/statistics-widget/statistics-widget.component.html</context> <context context-type="sourcefile">src/app/components/dashboard/widgets/statistics-widget/statistics-widget.component.html</context>
@@ -3279,7 +3279,7 @@
</context-group> </context-group>
<context-group purpose="location"> <context-group purpose="location">
<context context-type="sourcefile">src/app/components/common/suggestions-dropdown/suggestions-dropdown.component.html</context> <context context-type="sourcefile">src/app/components/common/suggestions-dropdown/suggestions-dropdown.component.html</context>
<context context-type="linenumber">46</context> <context context-type="linenumber">40</context>
</context-group> </context-group>
<context-group purpose="location"> <context-group purpose="location">
<context context-type="sourcefile">src/app/components/dashboard/widgets/statistics-widget/statistics-widget.component.html</context> <context context-type="sourcefile">src/app/components/dashboard/widgets/statistics-widget/statistics-widget.component.html</context>
@@ -7070,39 +7070,32 @@
<context context-type="linenumber">143</context> <context context-type="linenumber">143</context>
</context-group> </context-group>
</trans-unit> </trans-unit>
<trans-unit id="8336346011691074629" datatype="html">
<source>No suggestions</source>
<context-group purpose="location">
<context context-type="sourcefile">src/app/components/common/suggestions-dropdown/suggestions-dropdown.component.html</context>
<context context-type="linenumber">11,12</context>
</context-group>
</trans-unit>
<trans-unit id="5320136382998259826" datatype="html"> <trans-unit id="5320136382998259826" datatype="html">
<source>Suggest</source> <source>Suggest</source>
<context-group purpose="location"> <context-group purpose="location">
<context context-type="sourcefile">src/app/components/common/suggestions-dropdown/suggestions-dropdown.component.html</context> <context context-type="sourcefile">src/app/components/common/suggestions-dropdown/suggestions-dropdown.component.html</context>
<context context-type="linenumber">13,14</context> <context context-type="linenumber">8,9</context>
</context-group> </context-group>
</trans-unit> </trans-unit>
<trans-unit id="6934085657687954669" datatype="html"> <trans-unit id="6934085657687954669" datatype="html">
<source>Show suggestions</source> <source>Show suggestions</source>
<context-group purpose="location"> <context-group purpose="location">
<context context-type="sourcefile">src/app/components/common/suggestions-dropdown/suggestions-dropdown.component.html</context> <context context-type="sourcefile">src/app/components/common/suggestions-dropdown/suggestions-dropdown.component.html</context>
<context context-type="linenumber">23,24</context> <context context-type="linenumber">17,18</context>
</context-group> </context-group>
</trans-unit> </trans-unit>
<trans-unit id="3834115140127576673" datatype="html"> <trans-unit id="3834115140127576673" datatype="html">
<source>No novel suggestions</source> <source>No novel suggestions</source>
<context-group purpose="location"> <context-group purpose="location">
<context context-type="sourcefile">src/app/components/common/suggestions-dropdown/suggestions-dropdown.component.html</context> <context context-type="sourcefile">src/app/components/common/suggestions-dropdown/suggestions-dropdown.component.html</context>
<context context-type="linenumber">30,31</context> <context context-type="linenumber">24,25</context>
</context-group> </context-group>
</trans-unit> </trans-unit>
<trans-unit id="4369111787961525769" datatype="html"> <trans-unit id="4369111787961525769" datatype="html">
<source>Document Types</source> <source>Document Types</source>
<context-group purpose="location"> <context-group purpose="location">
<context context-type="sourcefile">src/app/components/common/suggestions-dropdown/suggestions-dropdown.component.html</context> <context context-type="sourcefile">src/app/components/common/suggestions-dropdown/suggestions-dropdown.component.html</context>
<context context-type="linenumber">40</context> <context context-type="linenumber">34</context>
</context-group> </context-group>
<context-group purpose="location"> <context-group purpose="location">
<context context-type="sourcefile">src/app/components/dashboard/widgets/statistics-widget/statistics-widget.component.html</context> <context context-type="sourcefile">src/app/components/dashboard/widgets/statistics-widget/statistics-widget.component.html</context>
@@ -2,16 +2,10 @@
<button type="button" class="btn btn-sm btn-outline-primary" (click)="clickSuggest()" [disabled]="disabled() || loading() || (suggestions() && !aiEnabled())"> <button type="button" class="btn btn-sm btn-outline-primary" (click)="clickSuggest()" [disabled]="disabled() || loading() || (suggestions() && !aiEnabled())">
@if (loading()) { @if (loading()) {
<div class="spinner-border spinner-border-sm" role="status"></div> <div class="spinner-border spinner-border-sm" role="status"></div>
} @else if (noSuggestions) {
<i-bs width="1.2em" height="1.2em" name="check-circle"></i-bs>
} @else { } @else {
<i-bs width="1.2em" height="1.2em" name="stars"></i-bs> <i-bs width="1.2em" height="1.2em" name="stars"></i-bs>
} }
@if (noSuggestions) { <span class="d-none d-lg-inline ps-1" i18n>Suggest</span>
<span class="d-none d-lg-inline ps-1" i18n>No suggestions</span>
} @else {
<span class="d-none d-lg-inline ps-1" i18n>Suggest</span>
}
@if (totalSuggestions > 0) { @if (totalSuggestions > 0) {
<span class="badge bg-primary ms-2">{{ totalSuggestions }}</span> <span class="badge bg-primary ms-2">{{ totalSuggestions }}</span>
} }
@@ -25,7 +19,7 @@
<div ngbDropdownMenu aria-labelledby="suggestionsDropdown" class="shadow suggestions-dropdown"> <div ngbDropdownMenu aria-labelledby="suggestionsDropdown" class="shadow suggestions-dropdown">
<div class="list-group list-group-flush small pb-0"> <div class="list-group list-group-flush small pb-0">
@if (totalSuggestions === 0) { @if (!suggestions()?.suggested_tags && !suggestions()?.suggested_document_types && !suggestions()?.suggested_correspondents) {
<div class="list-group-item text-muted fst-italic"> <div class="list-group-item text-muted fst-italic">
<small class="text-muted small fst-italic" i18n>No novel suggestions</small> <small class="text-muted small fst-italic" i18n>No novel suggestions</small>
</div> </div>
@@ -30,34 +30,6 @@ describe('SuggestionsDropdownComponent', () => {
expect(component.totalSuggestions).toBe(4) expect(component.totalSuggestions).toBe(4)
}) })
it('should show when a completed request returned no suggestions', () => {
fixture.componentRef.setInput('suggestions', {
correspondents: [],
tags: [],
document_types: [],
storage_paths: [],
dates: [],
})
fixture.detectChanges()
expect(component.noSuggestions).toBeTruthy()
expect(fixture.nativeElement.textContent).toContain('No suggestions')
})
it('should not show the empty state before a request or with suggestions', () => {
expect(component.noSuggestions).toBeFalsy()
fixture.componentRef.setInput('suggestions', {
correspondents: [],
tags: [42],
document_types: [],
storage_paths: [],
dates: [],
})
expect(component.noSuggestions).toBeFalsy()
})
it('should emit getSuggestions when clickSuggest is called and suggestions are null', () => { it('should emit getSuggestions when clickSuggest is called and suggestions are null', () => {
jest.spyOn(component.getSuggestions, 'emit') jest.spyOn(component.getSuggestions, 'emit')
fixture.componentRef.setInput('suggestions', null) fixture.componentRef.setInput('suggestions', null)
@@ -87,6 +59,5 @@ describe('SuggestionsDropdownComponent', () => {
}) })
component.clickSuggest() component.clickSuggest()
expect(component.dropdown.open).toBeTruthy() expect(component.dropdown.open).toBeTruthy()
expect(fixture.nativeElement.textContent).toContain('No novel suggestions')
}) })
}) })
@@ -61,21 +61,4 @@ export class SuggestionsDropdownComponent {
this.suggestions()?.suggested_document_types?.length || 0 this.suggestions()?.suggested_document_types?.length || 0
) )
} }
get noSuggestions(): boolean {
const suggestions = this.suggestions()
return (
suggestions != null &&
!suggestions.title &&
!suggestions.tags?.length &&
!suggestions.suggested_tags?.length &&
!suggestions.correspondents?.length &&
!suggestions.suggested_correspondents?.length &&
!suggestions.document_types?.length &&
!suggestions.suggested_document_types?.length &&
!suggestions.storage_paths?.length &&
!suggestions.suggested_storage_paths?.length &&
!suggestions.dates?.length
)
}
} }
+27
View File
@@ -757,3 +757,30 @@ class TestAPIProcessedMails(DirectoriesMixin, APITestCase):
format="json", format="json",
) )
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) 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())
+12 -4
View File
@@ -27,6 +27,7 @@ from documents.filters import PermittedObjectsFilter
from documents.models import PaperlessTask from documents.models import PaperlessTask
from documents.permissions import PaperlessObjectPermissions from documents.permissions import PaperlessObjectPermissions
from documents.permissions import has_perms_owner_aware from documents.permissions import has_perms_owner_aware
from documents.permissions import permitted_object_ids
from documents.views import PassUserMixin from documents.views import PassUserMixin
from paperless.views import StandardPagination from paperless.views import StandardPagination
from paperless_mail.filters import ProcessedMailFilterSet 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") return HttpResponseBadRequest("mail_ids must be a list of integers")
mails = ProcessedMail.objects.filter(id__in=mail_ids) mails = ProcessedMail.objects.filter(id__in=mail_ids)
for mail in mails: # Check every id up front so an unpermitted one rejects the whole
if not has_perms_owner_aware(request.user, "delete_processedmail", mail): # request rather than deleting the mails ahead of it first.
return HttpResponseForbidden("Insufficient permissions") if mails.exclude(
mail.delete() 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}) return Response({"result": "OK", "deleted_mail_ids": mail_ids})