Fix: require only change permissions for task dismissal, add frontend error handling (#11023)

src-ui/src/app/components/admin/tasks/tasks.component.spec.ts

@@ -16,6 +16,7 @@ import {
   NgbNavItem,
 } from '@ng-bootstrap/ng-bootstrap'
 import { allIcons, NgxBootstrapIconsModule } from 'ngx-bootstrap-icons'
+import { throwError } from 'rxjs'
 import { routes } from 'src/app/app-routing.module'
 import {
   PaperlessTask,
@@ -28,6 +29,7 @@ import { PermissionsGuard } from 'src/app/guards/permissions.guard'
 import { CustomDatePipe } from 'src/app/pipes/custom-date.pipe'
 import { PermissionsService } from 'src/app/services/permissions.service'
 import { TasksService } from 'src/app/services/tasks.service'
+import { ToastService } from 'src/app/services/toast.service'
 import { environment } from 'src/environments/environment'
 import { ConfirmDialogComponent } from '../../common/confirm-dialog/confirm-dialog.component'
 import { PageHeaderComponent } from '../../common/page-header/page-header.component'
@@ -123,6 +125,7 @@ describe('TasksComponent', () => {
   let router: Router
   let httpTestingController: HttpTestingController
   let reloadSpy
+  let toastService: ToastService
 
   beforeEach(async () => {
     TestBed.configureTestingModule({
@@ -157,6 +160,7 @@ describe('TasksComponent', () => {
     httpTestingController = TestBed.inject(HttpTestingController)
     modalService = TestBed.inject(NgbModal)
     router = TestBed.inject(Router)
+    toastService = TestBed.inject(ToastService)
     fixture = TestBed.createComponent(TasksComponent)
     component = fixture.componentInstance
     jest.useFakeTimers()
@@ -249,6 +253,42 @@ describe('TasksComponent', () => {
     expect(dismissSpy).toHaveBeenCalledWith(selected)
   })
 
+  it('should show an error and re-enable modal buttons when dismissing multiple tasks fails', () => {
+    component.selectedTasks = new Set([tasks[0].id, tasks[1].id])
+    const error = new Error('dismiss failed')
+    const toastSpy = jest.spyOn(toastService, 'showError')
+    const dismissSpy = jest
+      .spyOn(tasksService, 'dismissTasks')
+      .mockReturnValue(throwError(() => error))
+
+    let modal: NgbModalRef
+    modalService.activeInstances.subscribe((m) => (modal = m[m.length - 1]))
+
+    component.dismissTasks()
+    expect(modal).not.toBeUndefined()
+
+    modal.componentInstance.confirmClicked.emit()
+
+    expect(dismissSpy).toHaveBeenCalledWith(new Set([tasks[0].id, tasks[1].id]))
+    expect(toastSpy).toHaveBeenCalledWith('Error dismissing tasks', error)
+    expect(modal.componentInstance.buttonsEnabled).toBe(true)
+    expect(component.selectedTasks.size).toBe(0)
+  })
+
+  it('should show an error when dismissing a single task fails', () => {
+    const error = new Error('dismiss failed')
+    const toastSpy = jest.spyOn(toastService, 'showError')
+    const dismissSpy = jest
+      .spyOn(tasksService, 'dismissTasks')
+      .mockReturnValue(throwError(() => error))
+
+    component.dismissTask(tasks[0])
+
+    expect(dismissSpy).toHaveBeenCalledWith(new Set([tasks[0].id]))
+    expect(toastSpy).toHaveBeenCalledWith('Error dismissing task', error)
+    expect(component.selectedTasks.size).toBe(0)
+  })
+
   it('should support dismiss all tasks', () => {
     let modal: NgbModalRef
     modalService.activeInstances.subscribe((m) => (modal = m[m.length - 1]))

src-ui/src/app/components/admin/tasks/tasks.component.ts

@@ -24,6 +24,7 @@ import { PaperlessTask } from 'src/app/data/paperless-task'
 import { IfPermissionsDirective } from 'src/app/directives/if-permissions.directive'
 import { CustomDatePipe } from 'src/app/pipes/custom-date.pipe'
 import { TasksService } from 'src/app/services/tasks.service'
+import { ToastService } from 'src/app/services/toast.service'
 import { ConfirmDialogComponent } from '../../common/confirm-dialog/confirm-dialog.component'
 import { PageHeaderComponent } from '../../common/page-header/page-header.component'
 import { LoadingComponentWithPermissions } from '../../loading-component/loading.component'
@@ -72,6 +73,7 @@ export class TasksComponent
   tasksService = inject(TasksService)
   private modalService = inject(NgbModal)
   private readonly router = inject(Router)
+  private readonly toastService = inject(ToastService)
 
   public activeTab: TaskTab
   public selectedTasks: Set<number> = new Set()
@@ -154,11 +156,19 @@ export class TasksComponent
       modal.componentInstance.confirmClicked.pipe(first()).subscribe(() => {
         modal.componentInstance.buttonsEnabled = false
         modal.close()
-        this.tasksService.dismissTasks(tasks)
+        this.tasksService.dismissTasks(tasks).subscribe({
+          error: (e) => {
+            this.toastService.showError($localize`Error dismissing tasks`, e)
+            modal.componentInstance.buttonsEnabled = true
+          },
+        })
         this.clearSelection()
       })
     } else {
-      this.tasksService.dismissTasks(tasks)
+      this.tasksService.dismissTasks(tasks).subscribe({
+        error: (e) =>
+          this.toastService.showError($localize`Error dismissing task`, e),
+      })
       this.clearSelection()
     }
   }

src-ui/src/app/services/tasks.service.spec.ts

@@ -51,7 +51,7 @@ describe('TasksService', () => {
   })
 
   it('calls acknowledge_tasks api endpoint on dismiss and reloads', () => {
-    tasksService.dismissTasks(new Set([1, 2, 3]))
+    tasksService.dismissTasks(new Set([1, 2, 3])).subscribe()
     const req = httpTestingController.expectOne(
       `${environment.apiBaseUrl}tasks/acknowledge/`
     )

src-ui/src/app/services/tasks.service.ts

@@ -1,7 +1,7 @@
 import { HttpClient } from '@angular/common/http'
 import { Injectable, inject } from '@angular/core'
 import { Observable, Subject } from 'rxjs'
-import { first, takeUntil } from 'rxjs/operators'
+import { first, takeUntil, tap } from 'rxjs/operators'
 import {
   PaperlessTask,
   PaperlessTaskName,
@@ -68,14 +68,17 @@ export class TasksService {
   }
 
   public dismissTasks(task_ids: Set<number>) {
-    this.http
+    return this.http
       .post(`${this.baseUrl}tasks/acknowledge/`, {
         tasks: [...task_ids],
       })
-      .pipe(first())
-      .subscribe((r) => {
-        this.reload()
-      })
+      .pipe(
+        first(),
+        takeUntil(this.unsubscribeNotifer),
+        tap(() => {
+          this.reload()
+        })
+      )
   }
 
   public cancelPending(): void {

src/documents/permissions.py

@@ -161,3 +161,21 @@ class PaperlessNotePermissions(BasePermission):
         perms = self.perms_map[request.method]
 
         return request.user.has_perms(perms)
+
+
+class AcknowledgeTasksPermissions(BasePermission):
+    """
+    Permissions class that checks for model permissions for acknowledging tasks.
+    """
+
+    perms_map = {
+        "POST": ["documents.change_paperlesstask"],
+    }
+
+    def has_permission(self, request, view):
+        if not request.user or not request.user.is_authenticated:  # pragma: no cover
+            return False
+
+        perms = self.perms_map.get(request.method, [])
+
+        return request.user.has_perms(perms)

src/documents/tests/test_api_tasks.py

@@ -135,6 +135,44 @@ class TestTasks(DirectoriesMixin, APITestCase):
         response = self.client.get(self.ENDPOINT + "?acknowledged=false")
         self.assertEqual(len(response.data), 0)
 
+    def test_acknowledge_tasks_requires_change_permission(self):
+        """
+        GIVEN:
+            - A regular user initially without change permissions
+            - A regular user with change permissions
+        WHEN:
+            - API call is made to acknowledge tasks
+        THEN:
+            - The first user is forbidden from acknowledging tasks
+            - The second user is allowed to acknowledge tasks
+        """
+        regular_user = User.objects.create_user(username="test")
+        self.client.force_authenticate(user=regular_user)
+
+        task = PaperlessTask.objects.create(
+            task_id=str(uuid.uuid4()),
+            task_file_name="task_one.pdf",
+        )
+
+        response = self.client.post(
+            self.ENDPOINT + "acknowledge/",
+            {"tasks": [task.id]},
+        )
+        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+
+        regular_user2 = User.objects.create_user(username="test2")
+        regular_user2.user_permissions.add(
+            Permission.objects.get(codename="change_paperlesstask"),
+        )
+        regular_user2.save()
+        self.client.force_authenticate(user=regular_user2)
+
+        response = self.client.post(
+            self.ENDPOINT + "acknowledge/",
+            {"tasks": [task.id]},
+        )
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+
     def test_tasks_owner_aware(self):
         """
         GIVEN:

src/documents/views.py

@@ -136,6 +136,7 @@ from documents.models import WorkflowAction
 from documents.models import WorkflowTrigger
 from documents.parsers import get_parser_class_for_mime_type
 from documents.parsers import parse_date_generator
+from documents.permissions import AcknowledgeTasksPermissions
 from documents.permissions import PaperlessAdminPermissions
 from documents.permissions import PaperlessNotePermissions
 from documents.permissions import PaperlessObjectPermissions
@@ -2487,7 +2488,11 @@ class TasksViewSet(ReadOnlyModelViewSet):
             queryset = PaperlessTask.objects.filter(task_id=task_id)
         return queryset
 
-    @action(methods=["post"], detail=False)
+    @action(
+        methods=["post"],
+        detail=False,
+        permission_classes=[IsAuthenticated, AcknowledgeTasksPermissions],
+    )
     def acknowledge(self, request):
         serializer = AcknowledgeTasksViewSerializer(data=request.data)
         serializer.is_valid(raise_exception=True)