paradizelost/paperless-ngx
1
0
Fork 0
mirror of https://github.com/paperless-ngx/paperless-ngx.git synced 2025-04-02 13:45:10 -05:00
Code Issues Packages Projects Releases Wiki Activity

Fix: bulk edit objects does not respect global permissions (#5888)

Browse Source
This commit is contained in:
shamoon 2024-02-25 16:59:59 -08:00 committed by GitHub
parent 4948438378
commit db0a2eb1a3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 101 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src-ui/src/app/components/manage/management-list/management-list.component.html
View File

@ -2,10 +2,10 @@
<button class="btn btn-sm btn-outline-secondary me-2" (click)="clearSelection()" [hidden]="selectedObjects.size === 0">
<i-bs name="x"></i-bs>&nbsp;<ng-container i18n>Clear selection</ng-container>
</button>
<button type="button" class="btn btn-sm btn-outline-primary me-2" (click)="setPermissions()" [disabled]="!userOwnsAll || selectedObjects.size === 0">
<button type="button" class="btn btn-sm btn-outline-primary me-2" (click)="setPermissions()" [disabled]="!userCanBulkEdit(PermissionAction.Change) || selectedObjects.size === 0">
<i-bs name="person-fill-lock"></i-bs>&nbsp;<ng-container i18n>Permissions</ng-container>
</button>
<button type="button" class="btn btn-sm btn-outline-danger me-5" (click)="delete()" [disabled]="!userOwnsAll || selectedObjects.size === 0">
<button type="button" class="btn btn-sm btn-outline-danger me-5" (click)="delete()" [disabled]="!userCanBulkEdit(PermissionAction.Delete) || selectedObjects.size === 0">
<i-bs name="trash"></i-bs>&nbsp;<ng-container i18n>Delete</ng-container>
</button>
<button type="button" class="btn btn-sm btn-outline-primary" (click)="openCreateDialog()" *pngxIfPermissions="{ action: PermissionAction.Add, type: permissionType }">

33
src-ui/src/app/components/manage/management-list/management-list.component.spec.ts
View File

@ -23,7 +23,10 @@ import { TagService } from 'src/app/services/rest/tag.service'
import { PageHeaderComponent } from '../../common/page-header/page-header.component'
import { TagListComponent } from '../tag-list/tag-list.component'
import { ManagementListComponent } from './management-list.component'
import { PermissionsService } from 'src/app/services/permissions.service'
import {
PermissionAction,
PermissionsService,
} from 'src/app/services/permissions.service'
import { ToastService } from 'src/app/services/toast.service'
import { EditDialogComponent } from '../../common/edit-dialog/edit-dialog.component'
import { ConfirmDialogComponent } from '../../common/confirm-dialog/confirm-dialog.component'
@ -65,6 +68,7 @@ describe('ManagementListComponent', () => {
let modalService: NgbModal
let toastService: ToastService
let documentListViewService: DocumentListViewService
let permissionsService: PermissionsService
beforeEach(async () => {
TestBed.configureTestingModule({
@ -77,18 +81,7 @@ describe('ManagementListComponent', () => {
ConfirmDialogComponent,
PermissionsDialogComponent,
],
providers: [
{
provide: PermissionsService,
useValue: {
currentUserCan: () => true,
currentUserHasObjectPermissions: () => true,
currentUserOwnsObject: () => true,
},
},
DatePipe,
PermissionsGuard,
],
providers: [DatePipe, PermissionsGuard],
imports: [
HttpClientTestingModule,
NgbPaginationModule,
@ -115,6 +108,14 @@ describe('ManagementListComponent', () => {
})
}
)
permissionsService = TestBed.inject(PermissionsService)
jest.spyOn(permissionsService, 'currentUserCan').mockReturnValue(true)
jest
.spyOn(permissionsService, 'currentUserHasObjectPermissions')
.mockReturnValue(true)
jest
.spyOn(permissionsService, 'currentUserOwnsObject')
.mockReturnValue(true)
modalService = TestBed.inject(NgbModal)
toastService = TestBed.inject(ToastService)
documentListViewService = TestBed.inject(DocumentListViewService)
@ -312,4 +313,10 @@ describe('ManagementListComponent', () => {
expect(bulkEditSpy).toHaveBeenCalled()
expect(successToastSpy).toHaveBeenCalled()
})
it('should disallow bulk permissions or delete objects if no global perms', () => {
jest.spyOn(permissionsService, 'currentUserCan').mockReturnValue(false)
expect(component.userCanBulkEdit(PermissionAction.Delete)).toBeFalsy()
expect(component.userCanBulkEdit(PermissionAction.Change)).toBeFalsy()
})
})

10
src-ui/src/app/components/manage/management-list/management-list.component.ts
View File

@ -15,16 +15,14 @@ import {
MATCH_NONE,
} from 'src/app/data/matching-model'
import { ObjectWithId } from 'src/app/data/object-with-id'
import {
ObjectWithPermissions,
PermissionsObject,
} from 'src/app/data/object-with-permissions'
import { ObjectWithPermissions } from 'src/app/data/object-with-permissions'
import {
SortableDirective,
SortEvent,
} from 'src/app/directives/sortable.directive'
import { DocumentListViewService } from 'src/app/services/document-list-view.service'
import {
PermissionAction,
PermissionsService,
PermissionType,
} from 'src/app/services/permissions.service'
@ -250,7 +248,9 @@ export abstract class ManagementListComponent<T extends ObjectWithId>
)
}
get userOwnsAll(): boolean {
userCanBulkEdit(action: PermissionAction): boolean {
if (!this.permissionsService.currentUserCan(action, this.permissionType))
return false
let ownsAll: boolean = true
const objects = this.data.filter((o) => this.selectedObjects.has(o.id))
ownsAll = objects.every((o) =>

69
src/documents/tests/test_api_objects.py
View File

@ -1,6 +1,7 @@
import json
from unittest import mock
from django.contrib.auth.models import Permission
from django.contrib.auth.models import User
from rest_framework import status
from rest_framework.test import APITestCase
@ -310,17 +311,77 @@ class TestBulkEditObjects(APITestCase):
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(StoragePath.objects.count(), 0)
def test_bulk_edit_object_permissions_insufficient_perms(self):
def test_bulk_edit_object_permissions_insufficient_global_perms(self):
"""
GIVEN:
- Objects owned by user other than logged in user
- Existing objects, user does not have global delete permissions
WHEN:
- bulk_edit_objects API endpoint is called with delete operation
THEN:
- User is not able to delete objects
"""
self.t1.owner = User.objects.get(username="temp_admin")
self.t1.save()
self.client.force_authenticate(user=self.user1)
response = self.client.post(
"/api/bulk_edit_objects/",
json.dumps(
{
"objects": [self.t1.id, self.t2.id],
"object_type": "tags",
"operation": "delete",
},
),
content_type="application/json",
)
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
self.assertEqual(response.content, b"Insufficient permissions")
def test_bulk_edit_object_permissions_sufficient_global_perms(self):
"""
GIVEN:
- Existing objects, user does have global delete permissions
WHEN:
- bulk_edit_objects API endpoint is called with delete operation
THEN:
- User is able to delete objects
"""
self.user1.user_permissions.add(
*Permission.objects.filter(codename="delete_tag"),
)
self.user1.save()
self.client.force_authenticate(user=self.user1)
response = self.client.post(
"/api/bulk_edit_objects/",
json.dumps(
{
"objects": [self.t1.id, self.t2.id],
"object_type": "tags",
"operation": "delete",
},
),
content_type="application/json",
)
self.assertEqual(response.status_code, status.HTTP_200_OK)
def test_bulk_edit_object_permissions_insufficient_object_perms(self):
"""
GIVEN:
- Objects owned by user other than logged in user
WHEN:
- bulk_edit_objects API endpoint is called with delete operation
THEN:
- User is not able to delete objects
"""
self.t2.owner = User.objects.get(username="temp_admin")
self.t2.save()
self.user1.user_permissions.add(
*Permission.objects.filter(codename="delete_tag"),
)
self.user1.save()
self.client.force_authenticate(user=self.user1)
response = self.client.post(

10
src/documents/views.py
View File

@ -1419,7 +1419,15 @@ class BulkEditObjectsView(GenericAPIView, PassUserMixin):
objs = object_class.objects.filter(pk__in=object_ids)
if not user.is_superuser:
has_perms = all((obj.owner == user or obj.owner is None) for obj in objs)
model_name = object_class._meta.verbose_name
perm = (
f"documents.change_{model_name}"
if operation == "set_permissions"
else f"documents.delete_{model_name}"
)
has_perms = user.has_perm(perm) and all(
(obj.owner == user or obj.owner is None) for obj in objs
)
if not has_perms:
return HttpResponseForbidden("Insufficient permissions")