diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 32f6881aa..6e19d03a4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -184,7 +184,7 @@ jobs: cache-dependency-path: 'src-ui/package-lock.json' - name: Cache frontend dependencies id: cache-frontend-deps - uses: actions/cache@v3 + uses: actions/cache@v4 with: path: | ~/.npm @@ -221,7 +221,7 @@ jobs: cache-dependency-path: 'src-ui/package-lock.json' - name: Cache frontend dependencies id: cache-frontend-deps - uses: actions/cache@v3 + uses: actions/cache@v4 with: path: | ~/.npm @@ -283,7 +283,7 @@ jobs: merge-multiple: true - name: Upload frontend coverage to Codecov - uses: codecov/codecov-action@v3 + uses: codecov/codecov-action@v4 with: # not required for public repos, but intermittently fails otherwise token: ${{ secrets.CODECOV_TOKEN }} @@ -299,7 +299,7 @@ jobs: path: src/ - name: Upload coverage to Codecov - uses: codecov/codecov-action@v3 + uses: codecov/codecov-action@v4 with: # not required for public repos, but intermittently fails otherwise token: ${{ secrets.CODECOV_TOKEN }} diff --git a/docs/api.md b/docs/api.md index 97ccf4c3a..bd5154ada 100644 --- a/docs/api.md +++ b/docs/api.md @@ -139,7 +139,7 @@ document. Paperless only reports PDF metadata at this point. ## Authorization -The REST api provides three different forms of authentication. +The REST api provides four different forms of authentication. 1. Basic authentication @@ -177,6 +177,12 @@ The REST api provides three different forms of authentication. Tokens can also be managed in the Django admin. +4. Remote User authentication + + If enabled (see + [configuration](configuration.md#PAPERLESS_ENABLE_HTTP_REMOTE_USER_API)), + you can authenticate against the API using Remote User auth. + ## Searching for documents Full text searching is available on the `/api/documents/` endpoint. Two @@ -185,7 +191,7 @@ results: - `/api/documents/?query=your%20search%20query`: Search for a document using a full text query. For details on the syntax, see [Basic Usage - Searching](usage.md#basic-usage_searching). -- `/api/documents/?more_like=1234`: Search for documents similar to +- `/api/documents/?more_like_id=1234`: Search for documents similar to the document with id 1234. Pagination works exactly the same as it does for normal requests on this @@ -324,6 +330,64 @@ granted). You can pass the parameter `full_perms=true` to API calls to view the full permissions of objects in a format that mirrors the `set_permissions` parameter above. +## Bulk Editing + +The API supports various bulk-editing operations which are executed asynchronously. + +### Documents + +For bulk operations on documents, use the endpoint `/api/bulk_edit/` which accepts +a json payload of the format: + +```json +{ + "documents": [LIST_OF_DOCUMENT_IDS], + "method": METHOD, // see below + "parameters": args // see below +} +``` + +The following methods are supported: + +- `set_correspondent` + - Requires `parameters`: `{ "correspondent": CORRESPONDENT_ID }` +- `set_document_type` + - Requires `parameters`: `{ "document_type": DOCUMENT_TYPE_ID }` +- `set_storage_path` + - Requires `parameters`: `{ "storage_path": STORAGE_PATH_ID }` +- `add_tag` + - Requires `parameters`: `{ "tag": TAG_ID }` +- `remove_tag` + - Requires `parameters`: `{ "tag": TAG_ID }` +- `modify_tags` + - Requires `parameters`: `{ "add_tags": [LIST_OF_TAG_IDS] }` and / or `{ "remove_tags": [LIST_OF_TAG_IDS] }` +- `delete` + - No `parameters` required +- `redo_ocr` + - No `parameters` required +- `set_permissions` + - Requires `parameters`: + - `"permissions": PERMISSIONS_OBJ` (see format [above](#permissions)) and / or + - `"owner": OWNER_ID or null` + - `"merge": true or false` (defaults to false) + - The `merge` flag determines if the supplied permissions will overwrite all existing permissions (including + removing them) or be merged with existing permissions. + +### Objects + +Bulk editing for objects (tags, document types etc.) currently supports only updating permissions, using +the endpoint: `/api/bulk_edit_object_perms/` which requires a json payload of the format: + +```json +{ + "objects": [LIST_OF_OBJECT_IDS], + "object_type": "tags", "correspondents", "document_types" or "storage_paths" + "owner": OWNER_ID // optional + "permissions": { "view": { "users": [] ... }, "change": { ... } }, // (see 'set_permissions' format above) + "merge": true / false // defaults to false, see above +} +``` + ## API Versioning The REST API is versioned since Paperless-ngx 1.3.0. @@ -380,3 +444,13 @@ Initial API version. color to use for a specific tag, which is either black or white depending on the brightness of `Tag.color`. - Removed field `Tag.colour`. + +#### Version 3 + +- Permissions endpoints have been added. +- The format of the `/api/ui_settings/` has changed. + +#### Version 4 + +- Consumption templates were refactored to workflows and API endpoints + changed as such. diff --git a/docs/configuration.md b/docs/configuration.md index 94d7e515d..6dc341346 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -462,9 +462,21 @@ applications. Defaults to "false" which disables this feature. +#### [`PAPERLESS_ENABLE_HTTP_REMOTE_USER_API=`](#PAPERLESS_ENABLE_HTTP_REMOTE_USER_API) {#PAPERLESS_ENABLE_HTTP_REMOTE_USER_API} + +: Allows authentication via HTTP_REMOTE_USER directly against the API + + !!! warning + + See the warning above about securing your installation when using remote user header authentication. This setting is separate from + `PAPERLESS_ENABLE_HTTP_REMOTE_USER` to avoid introducing a security vulnerability to existing reverse proxy setups. As above, + ensure that your reverse proxy does not simply pass the `Remote-User` header from the internet to paperless. + + Defaults to "false" which disables this feature. + #### [`PAPERLESS_HTTP_REMOTE_USER_HEADER_NAME=`](#PAPERLESS_HTTP_REMOTE_USER_HEADER_NAME) {#PAPERLESS_HTTP_REMOTE_USER_HEADER_NAME} -: If "PAPERLESS_ENABLE_HTTP_REMOTE_USER" is enabled, this +: If "PAPERLESS_ENABLE_HTTP_REMOTE_USER" or `PAPERLESS_ENABLE_HTTP_REMOTE_USER_API` are enabled, this property allows to customize the name of the HTTP header from which the authenticated username is extracted. Values are in terms of [HttpRequest.META](https://docs.djangoproject.com/en/4.1/ref/request-response/#django.http.HttpRequest.META). diff --git a/src-ui/messages.xlf b/src-ui/messages.xlf index e7263e0f2..56866f512 100644 --- a/src-ui/messages.xlf +++ b/src-ui/messages.xlf @@ -620,7 +620,7 @@ src/app/components/common/permissions-dialog/permissions-dialog.component.html - 20 + 23 src/app/components/dashboard/dashboard.component.html @@ -2438,7 +2438,7 @@ src/app/components/common/permissions-dialog/permissions-dialog.component.html - 23 + 26 src/app/components/document-list/bulk-editor/bulk-editor.component.ts @@ -2505,7 +2505,7 @@ src/app/components/common/permissions-dialog/permissions-dialog.component.html - 22 + 25 src/app/components/common/profile-edit-dialog/profile-edit-dialog.component.html @@ -3923,6 +3923,13 @@ 15 + + Merge with existing permissions + + src/app/components/common/permissions-dialog/permissions-dialog.component.html + 14 + + Set permissions @@ -3937,11 +3944,18 @@ 33 - - Note that permissions set here will override any existing permissions + + Existing owner, user and group permissions will be merged with these settings. src/app/components/common/permissions-dialog/permissions-dialog.component.ts - 71 + 74 + + + + Any and all existing owner, user and group permissions will be replaced. + + src/app/components/common/permissions-dialog/permissions-dialog.component.ts + 75 @@ -6100,18 +6114,18 @@ Permissions updated src/app/components/manage/mail/mail.component.ts - 211 + 212 Error updating permissions src/app/components/manage/mail/mail.component.ts - 215 + 217 src/app/components/manage/management-list/management-list.component.ts - 300 + 301 @@ -6277,7 +6291,7 @@ Permissions updated successfully src/app/components/manage/management-list/management-list.component.ts - 293 + 294 diff --git a/src-ui/src/app/components/common/input/switch/switch.component.html b/src-ui/src/app/components/common/input/switch/switch.component.html index 489106fcb..5acef7a75 100644 --- a/src-ui/src/app/components/common/input/switch/switch.component.html +++ b/src-ui/src/app/components/common/input/switch/switch.component.html @@ -15,7 +15,7 @@ } } -
+
@if (horizontal) { diff --git a/src-ui/src/app/components/common/permissions-dialog/permissions-dialog.component.html b/src-ui/src/app/components/common/permissions-dialog/permissions-dialog.component.html index 37ab7efbd..fd88002ba 100644 --- a/src-ui/src/app/components/common/permissions-dialog/permissions-dialog.component.html +++ b/src-ui/src/app/components/common/permissions-dialog/permissions-dialog.component.html @@ -5,12 +5,15 @@
- @if (!object && message) { -

- } -
- +
+ +
+
+
+ +
+
@@ -20,5 +23,5 @@ } - +
diff --git a/src-ui/src/app/components/common/permissions-dialog/permissions-dialog.component.spec.ts b/src-ui/src/app/components/common/permissions-dialog/permissions-dialog.component.spec.ts index 3f601d771..bc8ccdecb 100644 --- a/src-ui/src/app/components/common/permissions-dialog/permissions-dialog.component.spec.ts +++ b/src-ui/src/app/components/common/permissions-dialog/permissions-dialog.component.spec.ts @@ -11,6 +11,7 @@ import { NgSelectModule } from '@ng-select/ng-select' import { FormsModule, ReactiveFormsModule } from '@angular/forms' import { PermissionsUserComponent } from '../input/permissions/permissions-user/permissions-user.component' import { PermissionsGroupComponent } from '../input/permissions/permissions-group/permissions-group.component' +import { SwitchComponent } from '../input/switch/switch.component' const set_permissions = { owner: 10, @@ -37,6 +38,7 @@ describe('PermissionsDialogComponent', () => { PermissionsDialogComponent, SafeHtmlPipe, SelectComponent, + SwitchComponent, PermissionsFormComponent, PermissionsUserComponent, PermissionsGroupComponent, @@ -112,4 +114,23 @@ describe('PermissionsDialogComponent', () => { expect(component.title).toEqual(`Edit permissions for ${obj.name}`) expect(component.permissions).toEqual(set_permissions) }) + + it('should toggle hint based on object existence (if editing) or merge flag', () => { + component.form.get('merge').setValue(true) + expect(component.hint.includes('Existing')).toBeTruthy() + component.form.get('merge').setValue(false) + expect(component.hint.includes('will be replaced')).toBeTruthy() + component.object = {} + expect(component.hint).toBeNull() + }) + + it('should emit permissions and merge flag on confirm', () => { + const confirmSpy = jest.spyOn(component.confirmClicked, 'emit') + component.form.get('permissions_form').setValue(set_permissions) + component.confirm() + expect(confirmSpy).toHaveBeenCalledWith({ + permissions: set_permissions, + merge: true, + }) + }) }) diff --git a/src-ui/src/app/components/common/permissions-dialog/permissions-dialog.component.ts b/src-ui/src/app/components/common/permissions-dialog/permissions-dialog.component.ts index 9a514387c..afe1bebb3 100644 --- a/src-ui/src/app/components/common/permissions-dialog/permissions-dialog.component.ts +++ b/src-ui/src/app/components/common/permissions-dialog/permissions-dialog.component.ts @@ -32,6 +32,7 @@ export class PermissionsDialogComponent { this.o = o this.title = $localize`Edit permissions for ` + o['name'] this.form.patchValue({ + merge: true, permissions_form: { owner: o.owner, set_permissions: o.permissions, @@ -43,8 +44,9 @@ export class PermissionsDialogComponent { return this.o } - form = new FormGroup({ + public form = new FormGroup({ permissions_form: new FormControl(), + merge: new FormControl(true), }) buttonsEnabled: boolean = true @@ -66,11 +68,21 @@ export class PermissionsDialogComponent { } } - @Input() - message = - $localize`Note that permissions set here will override any existing permissions` + get hint(): string { + if (this.object) return null + return this.form.get('merge').value + ? $localize`Existing owner, user and group permissions will be merged with these settings.` + : $localize`Any and all existing owner, user and group permissions will be replaced.` + } cancelClicked() { this.activeModal.close() } + + confirm() { + this.confirmClicked.emit({ + permissions: this.permissions, + merge: this.form.get('merge').value, + }) + } } diff --git a/src-ui/src/app/components/common/permissions-filter-dropdown/permissions-filter-dropdown.component.html b/src-ui/src/app/components/common/permissions-filter-dropdown/permissions-filter-dropdown.component.html index f95434c8f..d20986c57 100644 --- a/src-ui/src/app/components/common/permissions-filter-dropdown/permissions-filter-dropdown.component.html +++ b/src-ui/src/app/components/common/permissions-filter-dropdown/permissions-filter-dropdown.component.html @@ -62,22 +62,24 @@ }
-
- - -
+ @if (permissionsService.currentUserCan(PermissionAction.View, PermissionType.User)) { +
+ + +
+ } @if (selectionModel.ownerFilter === OwnerFilterType.NONE || selectionModel.ownerFilter === OwnerFilterType.NOT_SELF) {
diff --git a/src-ui/src/app/components/common/permissions-filter-dropdown/permissions-filter-dropdown.component.ts b/src-ui/src/app/components/common/permissions-filter-dropdown/permissions-filter-dropdown.component.ts index 3f5c3e68d..b0c3e8817 100644 --- a/src-ui/src/app/components/common/permissions-filter-dropdown/permissions-filter-dropdown.component.ts +++ b/src-ui/src/app/components/common/permissions-filter-dropdown/permissions-filter-dropdown.component.ts @@ -67,7 +67,7 @@ export class PermissionsFilterDropdownComponent extends ComponentWithPermissions } constructor( - permissionsService: PermissionsService, + public permissionsService: PermissionsService, userService: UserService, private settingsService: SettingsService ) { diff --git a/src-ui/src/app/components/dashboard/widgets/saved-view-widget/saved-view-widget.component.html b/src-ui/src/app/components/dashboard/widgets/saved-view-widget/saved-view-widget.component.html index 0a7a852ed..de46991d2 100644 --- a/src-ui/src/app/components/dashboard/widgets/saved-view-widget/saved-view-widget.component.html +++ b/src-ui/src/app/components/dashboard/widgets/saved-view-widget/saved-view-widget.component.html @@ -15,8 +15,14 @@ Created Title - Tags - Correspondent + @if (permissionsService.currentUserCan(PermissionAction.View, PermissionType.Tag)) { + Tags + } + @if (permissionsService.currentUserCan(PermissionAction.View, PermissionType.Correspondent)) { + Correspondent + } @else { + + } @@ -26,13 +32,15 @@ {{doc.title | documentTitle}} - - @for (t of doc.tags$ | async; track t) { - - } - + @if (permissionsService.currentUserCan(PermissionAction.View, PermissionType.Tag)) { + + @for (t of doc.tags$ | async; track t) { + + } + + } - @if (doc.correspondent !== null) { + @if (permissionsService.currentUserCan(PermissionAction.View, PermissionType.Correspondent) && doc.correspondent !== null) { {{(doc.correspondent$ | async)?.name}} }
diff --git a/src-ui/src/app/components/dashboard/widgets/saved-view-widget/saved-view-widget.component.ts b/src-ui/src/app/components/dashboard/widgets/saved-view-widget/saved-view-widget.component.ts index aa1b160cf..c81ea5484 100644 --- a/src-ui/src/app/components/dashboard/widgets/saved-view-widget/saved-view-widget.component.ts +++ b/src-ui/src/app/components/dashboard/widgets/saved-view-widget/saved-view-widget.component.ts @@ -22,6 +22,7 @@ import { DocumentListViewService } from 'src/app/services/document-list-view.ser import { ComponentWithPermissions } from 'src/app/components/with-permissions/with-permissions.component' import { NgbPopover } from '@ng-bootstrap/ng-bootstrap' import { queryParamsFromFilterRules } from 'src/app/utils/query-params' +import { PermissionsService } from 'src/app/services/permissions.service' @Component({ selector: 'pngx-saved-view-widget', @@ -40,7 +41,8 @@ export class SavedViewWidgetComponent private list: DocumentListViewService, private consumerStatusService: ConsumerStatusService, public openDocumentsService: OpenDocumentsService, - public documentListViewService: DocumentListViewService + public documentListViewService: DocumentListViewService, + public permissionsService: PermissionsService ) { super() } diff --git a/src-ui/src/app/components/document-detail/document-detail.component.spec.ts b/src-ui/src/app/components/document-detail/document-detail.component.spec.ts index af0e0e78e..a30588970 100644 --- a/src-ui/src/app/components/document-detail/document-detail.component.spec.ts +++ b/src-ui/src/app/components/document-detail/document-detail.component.spec.ts @@ -1,5 +1,8 @@ import { DatePipe } from '@angular/common' -import { HttpClientTestingModule } from '@angular/common/http/testing' +import { + HttpClientTestingModule, + HttpTestingController, +} from '@angular/common/http/testing' import { ComponentFixture, TestBed, @@ -71,6 +74,7 @@ import { CustomFieldDataType } from 'src/app/data/custom-field' import { CustomFieldsService } from 'src/app/services/rest/custom-fields.service' import { PdfViewerComponent } from '../common/pdf-viewer/pdf-viewer.component' import { NgxBootstrapIconsModule, allIcons } from 'ngx-bootstrap-icons' +import { environment } from 'src/environments/environment' const doc: Document = { id: 3, @@ -136,6 +140,7 @@ describe('DocumentDetailComponent', () => { let documentListViewService: DocumentListViewService let settingsService: SettingsService let customFieldsService: CustomFieldsService + let httpTestingController: HttpTestingController let currentUserCan = true let currentUserHasObjectPermissions = true @@ -266,6 +271,7 @@ describe('DocumentDetailComponent', () => { settingsService.currentUser = { id: 1 } customFieldsService = TestBed.inject(CustomFieldsService) fixture = TestBed.createComponent(DocumentDetailComponent) + httpTestingController = TestBed.inject(HttpTestingController) component = fixture.componentInstance }) @@ -350,6 +356,26 @@ describe('DocumentDetailComponent', () => { expect(component.documentForm.disabled).toBeTruthy() }) + it('should not attempt to retrieve objects if user does not have permissions', () => { + currentUserCan = false + initNormally() + expect(component.correspondents).toBeUndefined() + expect(component.documentTypes).toBeUndefined() + expect(component.storagePaths).toBeUndefined() + expect(component.users).toBeUndefined() + httpTestingController.expectNone(`${environment.apiBaseUrl}documents/tags/`) + httpTestingController.expectNone( + `${environment.apiBaseUrl}documents/correspondents/` + ) + httpTestingController.expectNone( + `${environment.apiBaseUrl}documents/document_types/` + ) + httpTestingController.expectNone( + `${environment.apiBaseUrl}documents/storage_paths/` + ) + currentUserCan = true + }) + it('should support creating document type', () => { initNormally() let openModal: NgbModalRef diff --git a/src-ui/src/app/components/document-detail/document-detail.component.ts b/src-ui/src/app/components/document-detail/document-detail.component.ts index 0ce9fa007..0ca458a21 100644 --- a/src-ui/src/app/components/document-detail/document-detail.component.ts +++ b/src-ui/src/app/components/document-detail/document-detail.component.ts @@ -250,25 +250,50 @@ export class DocumentDetailComponent Object.assign(this.document, docValues) }) - this.correspondentService - .listAll() - .pipe(first(), takeUntil(this.unsubscribeNotifier)) - .subscribe((result) => (this.correspondents = result.results)) - - this.documentTypeService - .listAll() - .pipe(first(), takeUntil(this.unsubscribeNotifier)) - .subscribe((result) => (this.documentTypes = result.results)) - - this.storagePathService - .listAll() - .pipe(first(), takeUntil(this.unsubscribeNotifier)) - .subscribe((result) => (this.storagePaths = result.results)) - - this.userService - .listAll() - .pipe(first(), takeUntil(this.unsubscribeNotifier)) - .subscribe((result) => (this.users = result.results)) + if ( + this.permissionsService.currentUserCan( + PermissionAction.View, + PermissionType.Correspondent + ) + ) { + this.correspondentService + .listAll() + .pipe(first(), takeUntil(this.unsubscribeNotifier)) + .subscribe((result) => (this.correspondents = result.results)) + } + if ( + this.permissionsService.currentUserCan( + PermissionAction.View, + PermissionType.DocumentType + ) + ) { + this.documentTypeService + .listAll() + .pipe(first(), takeUntil(this.unsubscribeNotifier)) + .subscribe((result) => (this.documentTypes = result.results)) + } + if ( + this.permissionsService.currentUserCan( + PermissionAction.View, + PermissionType.StoragePath + ) + ) { + this.storagePathService + .listAll() + .pipe(first(), takeUntil(this.unsubscribeNotifier)) + .subscribe((result) => (this.storagePaths = result.results)) + } + if ( + this.permissionsService.currentUserCan( + PermissionAction.View, + PermissionType.User + ) + ) { + this.userService + .listAll() + .pipe(first(), takeUntil(this.unsubscribeNotifier)) + .subscribe((result) => (this.users = result.results)) + } this.getCustomFields() diff --git a/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.html b/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.html index b101c2742..0c261df67 100644 --- a/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.html +++ b/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.html @@ -17,51 +17,59 @@
- - - - - - - - + @if (permissionService.currentUserCan(PermissionAction.View, PermissionType.Tag)) { + + + } + @if (permissionService.currentUserCan(PermissionAction.View, PermissionType.Correspondent)) { + + + } + @if (permissionService.currentUserCan(PermissionAction.View, PermissionType.DocumentType)) { + + + } + @if (permissionService.currentUserCan(PermissionAction.View, PermissionType.StoragePath)) { + + + }
diff --git a/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.spec.ts b/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.spec.ts index af41d298c..42f8b6d1d 100644 --- a/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.spec.ts +++ b/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.spec.ts @@ -41,6 +41,7 @@ import { PermissionsUserComponent } from '../../common/input/permissions/permiss import { NgSelectModule } from '@ng-select/ng-select' import { GroupService } from 'src/app/services/rest/group.service' import { NgxBootstrapIconsModule, allIcons } from 'ngx-bootstrap-icons' +import { SwitchComponent } from '../../common/input/switch/switch.component' const selectionData: SelectionData = { selected_tags: [ @@ -81,6 +82,7 @@ describe('BulkEditorComponent', () => { SelectComponent, PermissionsGroupComponent, PermissionsUserComponent, + SwitchComponent, ], providers: [ PermissionsService, @@ -851,7 +853,18 @@ describe('BulkEditorComponent', () => { fixture.detectChanges() component.setPermissions() expect(modal).not.toBeUndefined() - modal.componentInstance.confirmClicked.next() + const perms = { + permissions: { + view_users: [], + change_users: [], + view_groups: [], + change_groups: [], + }, + } + modal.componentInstance.confirmClicked.emit({ + permissions: perms, + merge: true, + }) let req = httpTestingController.expectOne( `${environment.apiBaseUrl}documents/bulk_edit/` ) @@ -859,7 +872,10 @@ describe('BulkEditorComponent', () => { expect(req.request.body).toEqual({ documents: [3, 4], method: 'set_permissions', - parameters: undefined, + parameters: { + permissions: perms.permissions, + merge: true, + }, }) httpTestingController.match( `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true` @@ -868,4 +884,22 @@ describe('BulkEditorComponent', () => { `${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id` ) // listAllFilteredIds }) + + it('should not attempt to retrieve objects if user does not have permissions', () => { + jest.spyOn(permissionsService, 'currentUserCan').mockReturnValue(true) + expect(component.tags).toBeUndefined() + expect(component.correspondents).toBeUndefined() + expect(component.documentTypes).toBeUndefined() + expect(component.storagePaths).toBeUndefined() + httpTestingController.expectNone(`${environment.apiBaseUrl}documents/tags/`) + httpTestingController.expectNone( + `${environment.apiBaseUrl}documents/correspondents/` + ) + httpTestingController.expectNone( + `${environment.apiBaseUrl}documents/document_types/` + ) + httpTestingController.expectNone( + `${environment.apiBaseUrl}documents/storage_paths/` + ) + }) }) diff --git a/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.ts b/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.ts index 91b714b24..49d4c070f 100644 --- a/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.ts +++ b/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.ts @@ -115,22 +115,50 @@ export class BulkEditorComponent } ngOnInit() { - this.tagService - .listAll() - .pipe(first()) - .subscribe((result) => (this.tags = result.results)) - this.correspondentService - .listAll() - .pipe(first()) - .subscribe((result) => (this.correspondents = result.results)) - this.documentTypeService - .listAll() - .pipe(first()) - .subscribe((result) => (this.documentTypes = result.results)) - this.storagePathService - .listAll() - .pipe(first()) - .subscribe((result) => (this.storagePaths = result.results)) + if ( + this.permissionService.currentUserCan( + PermissionAction.View, + PermissionType.Tag + ) + ) { + this.tagService + .listAll() + .pipe(first()) + .subscribe((result) => (this.tags = result.results)) + } + if ( + this.permissionService.currentUserCan( + PermissionAction.View, + PermissionType.Correspondent + ) + ) { + this.correspondentService + .listAll() + .pipe(first()) + .subscribe((result) => (this.correspondents = result.results)) + } + if ( + this.permissionService.currentUserCan( + PermissionAction.View, + PermissionType.DocumentType + ) + ) { + this.documentTypeService + .listAll() + .pipe(first()) + .subscribe((result) => (this.documentTypes = result.results)) + } + if ( + this.permissionService.currentUserCan( + PermissionAction.View, + PermissionType.StoragePath + ) + ) { + this.storagePathService + .listAll() + .pipe(first()) + .subscribe((result) => (this.storagePaths = result.results)) + } this.downloadForm .get('downloadFileTypeArchive') @@ -512,9 +540,14 @@ export class BulkEditorComponent let modal = this.modalService.open(PermissionsDialogComponent, { backdrop: 'static', }) - modal.componentInstance.confirmClicked.subscribe((permissions) => { - modal.componentInstance.buttonsEnabled = false - this.executeBulkOperation(modal, 'set_permissions', permissions) - }) + modal.componentInstance.confirmClicked.subscribe( + ({ permissions, merge }) => { + modal.componentInstance.buttonsEnabled = false + this.executeBulkOperation(modal, 'set_permissions', { + ...permissions, + merge, + }) + } + ) } } diff --git a/src-ui/src/app/components/document-list/document-card-small/document-card-small.component.ts b/src-ui/src/app/components/document-list/document-card-small/document-card-small.component.ts index bd565a9fb..2ca1a3408 100644 --- a/src-ui/src/app/components/document-list/document-card-small/document-card-small.component.ts +++ b/src-ui/src/app/components/document-list/document-card-small/document-card-small.component.ts @@ -79,7 +79,7 @@ export class DocumentCardSmallComponent extends ComponentWithPermissions { getTagsLimited$() { const limit = this.document.notes.length > 0 ? 6 : 7 - return this.document.tags$.pipe( + return this.document.tags$?.pipe( map((tags) => { if (tags.length > limit) { this.moreTags = tags.length - (limit - 1) diff --git a/src-ui/src/app/components/document-list/filter-editor/filter-editor.component.html b/src-ui/src/app/components/document-list/filter-editor/filter-editor.component.html index 54427ad31..89900e087 100644 --- a/src-ui/src/app/components/document-list/filter-editor/filter-editor.component.html +++ b/src-ui/src/app/components/document-list/filter-editor/filter-editor.component.html @@ -18,7 +18,7 @@ } @if (_textFilter) { - } @@ -29,7 +29,8 @@
- - + } + @if (permissionsService.currentUserCan(PermissionAction.View, PermissionType.Correspondent)) { + - - + } + @if (permissionsService.currentUserCan(PermissionAction.View, PermissionType.DocumentType)) { + + } + @if (permissionsService.currentUserCan(PermissionAction.View, PermissionType.StoragePath)) { + + [allowSelectNone]="true"> + }
{ let fixture: ComponentFixture let documentService: DocumentService let settingsService: SettingsService + let permissionsService: PermissionsService + let httpTestingController: HttpTestingController beforeEach(fakeAsync(() => { TestBed.configureTestingModule({ @@ -199,6 +209,15 @@ describe('FilterEditorComponent', () => { documentService = TestBed.inject(DocumentService) settingsService = TestBed.inject(SettingsService) settingsService.currentUser = users[0] + permissionsService = TestBed.inject(PermissionsService) + jest + .spyOn(permissionsService, 'currentUserCan') + .mockImplementation((action, type) => { + // a little hack-ish, permissions filter dropdown causes reactive forms issue due to ng-select + // trying to apply formControlName + return type !== PermissionType.User + }) + httpTestingController = TestBed.inject(HttpTestingController) fixture = TestBed.createComponent(FilterEditorComponent) component = fixture.componentInstance component.filterRules = [] @@ -206,6 +225,24 @@ describe('FilterEditorComponent', () => { tick() })) + it('should not attempt to retrieve objects if user does not have permissions', () => { + jest.spyOn(permissionsService, 'currentUserCan').mockReset() + jest + .spyOn(permissionsService, 'currentUserCan') + .mockImplementation((action, type) => false) + component.ngOnInit() + httpTestingController.expectNone(`${environment.apiBaseUrl}documents/tags/`) + httpTestingController.expectNone( + `${environment.apiBaseUrl}documents/correspondents/` + ) + httpTestingController.expectNone( + `${environment.apiBaseUrl}documents/document_types/` + ) + httpTestingController.expectNone( + `${environment.apiBaseUrl}documents/storage_paths/` + ) + }) + // SET filterRules it('should ingest text filter rules for doc title', fakeAsync(() => { diff --git a/src-ui/src/app/components/document-list/filter-editor/filter-editor.component.ts b/src-ui/src/app/components/document-list/filter-editor/filter-editor.component.ts index 03e1db539..b11874d7c 100644 --- a/src-ui/src/app/components/document-list/filter-editor/filter-editor.component.ts +++ b/src-ui/src/app/components/document-list/filter-editor/filter-editor.component.ts @@ -70,6 +70,12 @@ import { OwnerFilterType, PermissionsSelectionModel, } from '../../common/permissions-filter-dropdown/permissions-filter-dropdown.component' +import { + PermissionAction, + PermissionType, + PermissionsService, +} from 'src/app/services/permissions.service' +import { ComponentWithPermissions } from '../../with-permissions/with-permissions.component' const TEXT_FILTER_TARGET_TITLE = 'title' const TEXT_FILTER_TARGET_TITLE_CONTENT = 'title-content' @@ -155,7 +161,10 @@ const DEFAULT_TEXT_FILTER_MODIFIER_OPTIONS = [ templateUrl: './filter-editor.component.html', styleUrls: ['./filter-editor.component.scss'], }) -export class FilterEditorComponent implements OnInit, OnDestroy { +export class FilterEditorComponent + extends ComponentWithPermissions + implements OnInit, OnDestroy +{ generateFilterName() { if (this.filterRules.length == 1) { let rule = this.filterRules[0] @@ -224,8 +233,11 @@ export class FilterEditorComponent implements OnInit, OnDestroy { private tagService: TagService, private correspondentService: CorrespondentService, private documentService: DocumentService, - private storagePathService: StoragePathService - ) {} + private storagePathService: StoragePathService, + public permissionsService: PermissionsService + ) { + super() + } @ViewChild('textFilterInput') textFilterInput: ElementRef @@ -872,18 +884,46 @@ export class FilterEditorComponent implements OnInit, OnDestroy { subscription: Subscription ngOnInit() { - this.tagService - .listAll() - .subscribe((result) => (this.tags = result.results)) - this.correspondentService - .listAll() - .subscribe((result) => (this.correspondents = result.results)) - this.documentTypeService - .listAll() - .subscribe((result) => (this.documentTypes = result.results)) - this.storagePathService - .listAll() - .subscribe((result) => (this.storagePaths = result.results)) + if ( + this.permissionsService.currentUserCan( + PermissionAction.View, + PermissionType.Tag + ) + ) { + this.tagService + .listAll() + .subscribe((result) => (this.tags = result.results)) + } + if ( + this.permissionsService.currentUserCan( + PermissionAction.View, + PermissionType.Correspondent + ) + ) { + this.correspondentService + .listAll() + .subscribe((result) => (this.correspondents = result.results)) + } + if ( + this.permissionsService.currentUserCan( + PermissionAction.View, + PermissionType.DocumentType + ) + ) { + this.documentTypeService + .listAll() + .subscribe((result) => (this.documentTypes = result.results)) + } + if ( + this.permissionsService.currentUserCan( + PermissionAction.View, + PermissionType.StoragePath + ) + ) { + this.storagePathService + .listAll() + .subscribe((result) => (this.storagePaths = result.results)) + } this.textFilterDebounce = new Subject() diff --git a/src-ui/src/app/components/manage/mail/mail.component.spec.ts b/src-ui/src/app/components/manage/mail/mail.component.spec.ts index 5680d8faa..fcc5bcc6b 100644 --- a/src-ui/src/app/components/manage/mail/mail.component.spec.ts +++ b/src-ui/src/app/components/manage/mail/mail.component.spec.ts @@ -41,6 +41,7 @@ import { TagsComponent } from '../../common/input/tags/tags.component' import { FormsModule, ReactiveFormsModule } from '@angular/forms' import { EditDialogMode } from '../../common/edit-dialog/edit-dialog.component' import { NgxBootstrapIconsModule, allIcons } from 'ngx-bootstrap-icons' +import { SwitchComponent } from '../../common/input/switch/switch.component' const mailAccounts = [ { id: 1, name: 'account1' }, @@ -82,6 +83,7 @@ describe('MailComponent', () => { PermissionsGroupComponent, PermissionsDialogComponent, PermissionsFormComponent, + SwitchComponent, ], providers: [CustomDatePipe, DatePipe, PermissionsGuard], imports: [ @@ -267,11 +269,11 @@ describe('MailComponent', () => { rulePatchSpy.mockReturnValueOnce( throwError(() => new Error('error saving perms')) ) - dialog.confirmClicked.emit(perms) + dialog.confirmClicked.emit({ permissions: perms, merge: true }) expect(rulePatchSpy).toHaveBeenCalled() expect(toastErrorSpy).toHaveBeenCalled() rulePatchSpy.mockReturnValueOnce(of(mailRules[0] as MailRule)) - dialog.confirmClicked.emit(perms) + dialog.confirmClicked.emit({ permissions: perms, merge: true }) expect(toastInfoSpy).toHaveBeenCalledWith('Permissions updated') modalService.dismissAll() @@ -299,8 +301,7 @@ describe('MailComponent', () => { expect(modal).not.toBeUndefined() let dialog = modal.componentInstance as PermissionsDialogComponent expect(dialog.object).toEqual(mailAccounts[0]) - dialog = modal.componentInstance as PermissionsDialogComponent - dialog.confirmClicked.emit(perms) + dialog.confirmClicked.emit({ permissions: perms, merge: true }) expect(accountPatchSpy).toHaveBeenCalled() }) }) diff --git a/src-ui/src/app/components/manage/mail/mail.component.ts b/src-ui/src/app/components/manage/mail/mail.component.ts index 9bb33c03b..d8820ed38 100644 --- a/src-ui/src/app/components/manage/mail/mail.component.ts +++ b/src-ui/src/app/components/manage/mail/mail.component.ts @@ -200,22 +200,27 @@ export class MailComponent const dialog: PermissionsDialogComponent = modal.componentInstance as PermissionsDialogComponent dialog.object = object - modal.componentInstance.confirmClicked.subscribe((permissions) => { - modal.componentInstance.buttonsEnabled = false - const service: AbstractPaperlessService = - 'account' in object ? this.mailRuleService : this.mailAccountService - object.owner = permissions['owner'] - object['set_permissions'] = permissions['set_permissions'] - service.patch(object).subscribe({ - next: () => { - this.toastService.showInfo($localize`Permissions updated`) - modal.close() - }, - error: (e) => { - this.toastService.showError($localize`Error updating permissions`, e) - }, - }) - }) + modal.componentInstance.confirmClicked.subscribe( + ({ permissions, merge }) => { + modal.componentInstance.buttonsEnabled = false + const service: AbstractPaperlessService = + 'account' in object ? this.mailRuleService : this.mailAccountService + object.owner = permissions['owner'] + object['set_permissions'] = permissions['set_permissions'] + service.patch(object).subscribe({ + next: () => { + this.toastService.showInfo($localize`Permissions updated`) + modal.close() + }, + error: (e) => { + this.toastService.showError( + $localize`Error updating permissions`, + e + ) + }, + }) + } + ) } userCanEdit(obj: ObjectWithPermissions): boolean { diff --git a/src-ui/src/app/components/manage/management-list/management-list.component.spec.ts b/src-ui/src/app/components/manage/management-list/management-list.component.spec.ts index 44a3452d7..6196a3c8a 100644 --- a/src-ui/src/app/components/manage/management-list/management-list.component.spec.ts +++ b/src-ui/src/app/components/manage/management-list/management-list.component.spec.ts @@ -264,13 +264,19 @@ describe('ManagementListComponent', () => { throwError(() => new Error('error setting permissions')) ) const errorToastSpy = jest.spyOn(toastService, 'showError') - modal.componentInstance.confirmClicked.emit() + modal.componentInstance.confirmClicked.emit({ + permissions: {}, + merge: true, + }) expect(bulkEditPermsSpy).toHaveBeenCalled() expect(errorToastSpy).toHaveBeenCalled() const successToastSpy = jest.spyOn(toastService, 'showInfo') bulkEditPermsSpy.mockReturnValueOnce(of('OK')) - modal.componentInstance.confirmClicked.emit() + modal.componentInstance.confirmClicked.emit({ + permissions: {}, + merge: true, + }) expect(bulkEditPermsSpy).toHaveBeenCalled() expect(successToastSpy).toHaveBeenCalled() }) diff --git a/src-ui/src/app/components/manage/management-list/management-list.component.ts b/src-ui/src/app/components/manage/management-list/management-list.component.ts index e20b5d4a7..a89b5e4f6 100644 --- a/src-ui/src/app/components/manage/management-list/management-list.component.ts +++ b/src-ui/src/app/components/manage/management-list/management-list.component.ts @@ -279,12 +279,13 @@ export abstract class ManagementListComponent backdrop: 'static', }) modal.componentInstance.confirmClicked.subscribe( - (permissions: { owner: number; set_permissions: PermissionsObject }) => { + ({ permissions, merge }) => { modal.componentInstance.buttonsEnabled = false this.service .bulk_update_permissions( Array.from(this.selectedObjects), - permissions + permissions, + merge ) .subscribe({ next: () => { diff --git a/src-ui/src/app/services/rest/abstract-name-filter-service.spec.ts b/src-ui/src/app/services/rest/abstract-name-filter-service.spec.ts index 70ae211e5..e09270701 100644 --- a/src-ui/src/app/services/rest/abstract-name-filter-service.spec.ts +++ b/src-ui/src/app/services/rest/abstract-name-filter-service.spec.ts @@ -53,10 +53,14 @@ export const commonAbstractNameFilterPaperlessServiceTests = ( }, } subscription = service - .bulk_update_permissions([1, 2], { - owner, - set_permissions: permissions, - }) + .bulk_update_permissions( + [1, 2], + { + owner, + set_permissions: permissions, + }, + true + ) .subscribe() const req = httpTestingController.expectOne( `${environment.apiBaseUrl}bulk_edit_object_perms/` diff --git a/src-ui/src/app/services/rest/abstract-name-filter-service.ts b/src-ui/src/app/services/rest/abstract-name-filter-service.ts index 5e0377cb9..b38994086 100644 --- a/src-ui/src/app/services/rest/abstract-name-filter-service.ts +++ b/src-ui/src/app/services/rest/abstract-name-filter-service.ts @@ -26,13 +26,15 @@ export abstract class AbstractNameFilterService< bulk_update_permissions( objects: Array, - permissions: { owner: number; set_permissions: PermissionsObject } + permissions: { owner: number; set_permissions: PermissionsObject }, + merge: boolean ): Observable { return this.http.post(`${this.baseUrl}bulk_edit_object_perms/`, { objects, object_type: this.resourceName, owner: permissions.owner, permissions: permissions.set_permissions, + merge, }) } } diff --git a/src-ui/src/app/services/rest/document.service.ts b/src-ui/src/app/services/rest/document.service.ts index ee0f26187..9ff99031f 100644 --- a/src-ui/src/app/services/rest/document.service.ts +++ b/src-ui/src/app/services/rest/document.service.ts @@ -13,6 +13,11 @@ import { TagService } from './tag.service' import { DocumentSuggestions } from 'src/app/data/document-suggestions' import { queryParamsFromFilterRules } from '../../utils/query-params' import { StoragePathService } from './storage-path.service' +import { + PermissionAction, + PermissionType, + PermissionsService, +} from '../permissions.service' export const DOCUMENT_SORT_FIELDS = [ { field: 'archive_serial_number', name: $localize`ASN` }, @@ -57,21 +62,40 @@ export class DocumentService extends AbstractPaperlessService { private correspondentService: CorrespondentService, private documentTypeService: DocumentTypeService, private tagService: TagService, - private storagePathService: StoragePathService + private storagePathService: StoragePathService, + private permissionsService: PermissionsService ) { super(http, 'documents') } addObservablesToDocument(doc: Document) { - if (doc.correspondent) { + if ( + doc.correspondent && + this.permissionsService.currentUserCan( + PermissionAction.View, + PermissionType.Correspondent + ) + ) { doc.correspondent$ = this.correspondentService.getCached( doc.correspondent ) } - if (doc.document_type) { + if ( + doc.document_type && + this.permissionsService.currentUserCan( + PermissionAction.View, + PermissionType.DocumentType + ) + ) { doc.document_type$ = this.documentTypeService.getCached(doc.document_type) } - if (doc.tags) { + if ( + doc.tags && + this.permissionsService.currentUserCan( + PermissionAction.View, + PermissionType.Tag + ) + ) { doc.tags$ = this.tagService .getCachedMany(doc.tags) .pipe( @@ -80,7 +104,13 @@ export class DocumentService extends AbstractPaperlessService { ) ) } - if (doc.storage_path) { + if ( + doc.storage_path && + this.permissionsService.currentUserCan( + PermissionAction.View, + PermissionType.StoragePath + ) + ) { doc.storage_path$ = this.storagePathService.getCached(doc.storage_path) } return doc diff --git a/src/documents/bulk_edit.py b/src/documents/bulk_edit.py index ab0049eaa..ba001fd14 100644 --- a/src/documents/bulk_edit.py +++ b/src/documents/bulk_edit.py @@ -129,13 +129,17 @@ def redo_ocr(doc_ids): return "OK" -def set_permissions(doc_ids, set_permissions, owner=None): +def set_permissions(doc_ids, set_permissions, owner=None, merge=False): qs = Document.objects.filter(id__in=doc_ids) - qs.update(owner=owner) + if merge: + # If merging, only set owner for documents that don't have an owner + qs.filter(owner__isnull=True).update(owner=owner) + else: + qs.update(owner=owner) for doc in qs: - set_permissions_for_object(set_permissions, doc) + set_permissions_for_object(permissions=set_permissions, object=doc, merge=merge) affected_docs = [doc.id for doc in qs] diff --git a/src/documents/parsers.py b/src/documents/parsers.py index 12e5d6b33..b791d8322 100644 --- a/src/documents/parsers.py +++ b/src/documents/parsers.py @@ -140,6 +140,7 @@ def run_convert( type=None, depth=None, auto_orient=False, + use_cropbox=False, extra=None, logging_group=None, ) -> None: @@ -158,6 +159,7 @@ def run_convert( args += ["-type", str(type)] if type else [] args += ["-depth", str(depth)] if depth else [] args += ["-auto-orient"] if auto_orient else [] + args += ["-define", "pdf:use-cropbox=true"] if use_cropbox else [] args += [input_file, output_file] logger.debug("Execute: " + " ".join(args), extra={"group": logging_group}) @@ -229,6 +231,7 @@ def make_thumbnail_from_pdf(in_path, temp_dir, logging_group=None) -> str: strip=True, trim=False, auto_orient=True, + use_cropbox=True, input_file=f"{in_path}[0]", output_file=out_path, logging_group=logging_group, diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py index 942d22fe5..3c65e11d9 100644 --- a/src/documents/serialisers.py +++ b/src/documents/serialisers.py @@ -916,6 +916,8 @@ class BulkEditSerializer(DocumentListSerializer, SetPermissionsMixin): ) if "owner" in parameters and parameters["owner"] is not None: self._validate_owner(parameters["owner"]) + if "merge" not in parameters: + parameters["merge"] = False def validate(self, attrs): method = attrs["method"] @@ -1258,6 +1260,12 @@ class BulkEditObjectPermissionsSerializer(serializers.Serializer, SetPermissions write_only=True, ) + merge = serializers.BooleanField( + default=False, + write_only=True, + required=False, + ) + def get_object_class(self, object_type): object_class = None if object_type == "tags": diff --git a/src/documents/test_bulk_edit.py b/src/documents/test_bulk_edit.py new file mode 100644 index 000000000..ad622ca24 --- /dev/null +++ b/src/documents/test_bulk_edit.py @@ -0,0 +1,110 @@ +from unittest import mock + +from django.contrib.auth.models import Group +from django.contrib.auth.models import User +from django.test import TestCase +from guardian.shortcuts import assign_perm +from guardian.shortcuts import get_groups_with_perms +from guardian.shortcuts import get_users_with_perms + +from documents.bulk_edit import set_permissions +from documents.models import Document +from documents.tests.utils import DirectoriesMixin + + +class TestBulkEditPermissions(DirectoriesMixin, TestCase): + def setUp(self): + super().setUp() + + self.doc1 = Document.objects.create(checksum="A", title="A") + self.doc2 = Document.objects.create(checksum="B", title="B") + self.doc3 = Document.objects.create(checksum="C", title="C") + + self.owner = User.objects.create(username="test_owner") + self.user1 = User.objects.create(username="user1") + self.user2 = User.objects.create(username="user2") + self.group1 = Group.objects.create(name="group1") + self.group2 = Group.objects.create(name="group2") + + @mock.patch("documents.tasks.bulk_update_documents.delay") + def test_set_permissions(self, m): + doc_ids = [self.doc1.id, self.doc2.id, self.doc3.id] + + assign_perm("view_document", self.group1, self.doc1) + + permissions = { + "view": { + "users": [self.user1.id, self.user2.id], + "groups": [self.group2.id], + }, + "change": { + "users": [self.user1.id], + "groups": [self.group2.id], + }, + } + + set_permissions( + doc_ids, + set_permissions=permissions, + owner=self.owner, + merge=False, + ) + m.assert_called_once() + + self.assertEqual(Document.objects.filter(owner=self.owner).count(), 3) + self.assertEqual(Document.objects.filter(id__in=doc_ids).count(), 3) + + users_with_perms = get_users_with_perms( + self.doc1, + ) + self.assertEqual(users_with_perms.count(), 2) + + # group1 should be replaced by group2 + groups_with_perms = get_groups_with_perms( + self.doc1, + ) + self.assertEqual(groups_with_perms.count(), 1) + + @mock.patch("documents.tasks.bulk_update_documents.delay") + def test_set_permissions_merge(self, m): + doc_ids = [self.doc1.id, self.doc2.id, self.doc3.id] + + self.doc1.owner = self.user1 + self.doc1.save() + + assign_perm("view_document", self.user1, self.doc1) + assign_perm("view_document", self.group1, self.doc1) + + permissions = { + "view": { + "users": [self.user2.id], + "groups": [self.group2.id], + }, + "change": { + "users": [self.user2.id], + "groups": [self.group2.id], + }, + } + set_permissions( + doc_ids, + set_permissions=permissions, + owner=self.owner, + merge=True, + ) + m.assert_called_once() + + # when merge is true owner doesn't get replaced if its not empty + self.assertEqual(Document.objects.filter(owner=self.owner).count(), 2) + self.assertEqual(Document.objects.filter(id__in=doc_ids).count(), 3) + + # merge of user1 which was pre-existing and user2 + users_with_perms = get_users_with_perms( + self.doc1, + ) + self.assertEqual(users_with_perms.count(), 2) + + # group1 should be merged by group2 + groups_with_perms = get_groups_with_perms( + self.doc1, + ) + self.assertEqual(groups_with_perms.count(), 2) diff --git a/src/documents/tests/test_api_bulk_edit.py b/src/documents/tests/test_api_bulk_edit.py index c2dc69a1e..4cf9909c0 100644 --- a/src/documents/tests/test_api_bulk_edit.py +++ b/src/documents/tests/test_api_bulk_edit.py @@ -765,6 +765,58 @@ class TestBulkEdit(DirectoriesMixin, APITestCase): self.assertCountEqual(args[0], [self.doc2.id, self.doc3.id]) self.assertEqual(len(kwargs["set_permissions"]["view"]["users"]), 2) + @mock.patch("documents.serialisers.bulk_edit.set_permissions") + def test_set_permissions_merge(self, m): + m.return_value = "OK" + user1 = User.objects.create(username="user1") + user2 = User.objects.create(username="user2") + permissions = { + "view": { + "users": [user1.id, user2.id], + "groups": None, + }, + "change": { + "users": [user1.id], + "groups": None, + }, + } + + response = self.client.post( + "/api/documents/bulk_edit/", + json.dumps( + { + "documents": [self.doc2.id, self.doc3.id], + "method": "set_permissions", + "parameters": {"set_permissions": permissions}, + }, + ), + content_type="application/json", + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + + m.assert_called() + args, kwargs = m.call_args + self.assertEqual(kwargs["merge"], False) + + response = self.client.post( + "/api/documents/bulk_edit/", + json.dumps( + { + "documents": [self.doc2.id, self.doc3.id], + "method": "set_permissions", + "parameters": {"set_permissions": permissions, "merge": True}, + }, + ), + content_type="application/json", + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + + m.assert_called() + args, kwargs = m.call_args + self.assertEqual(kwargs["merge"], True) + @mock.patch("documents.serialisers.bulk_edit.set_permissions") def test_insufficient_permissions_ownership(self, m): """ diff --git a/src/documents/tests/test_api_permissions.py b/src/documents/tests/test_api_permissions.py index a0b22586b..851608f37 100644 --- a/src/documents/tests/test_api_permissions.py +++ b/src/documents/tests/test_api_permissions.py @@ -700,8 +700,8 @@ class TestBulkEditObjectPermissions(APITestCase): def setUp(self): super().setUp() - user = User.objects.create_superuser(username="temp_admin") - self.client.force_authenticate(user=user) + self.temp_admin = User.objects.create_superuser(username="temp_admin") + self.client.force_authenticate(user=self.temp_admin) self.t1 = Tag.objects.create(name="t1") self.t2 = Tag.objects.create(name="t2") @@ -822,6 +822,79 @@ class TestBulkEditObjectPermissions(APITestCase): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(StoragePath.objects.get(pk=self.sp1.id).owner, self.user3) + def test_bulk_object_set_permissions_merge(self): + """ + GIVEN: + - Existing objects + WHEN: + - bulk_edit_object_perms API endpoint is called with merge=True or merge=False (default) + THEN: + - Permissions and / or owner are replaced or merged, depending on the merge flag + """ + permissions = { + "view": { + "users": [self.user1.id, self.user2.id], + "groups": [], + }, + "change": { + "users": [self.user1.id], + "groups": [], + }, + } + + assign_perm("view_tag", self.user3, self.t1) + self.t1.owner = self.user3 + self.t1.save() + + # merge=True + response = self.client.post( + "/api/bulk_edit_object_perms/", + json.dumps( + { + "objects": [self.t1.id, self.t2.id], + "object_type": "tags", + "owner": self.user1.id, + "permissions": permissions, + "merge": True, + }, + ), + content_type="application/json", + ) + + self.t1.refresh_from_db() + self.t2.refresh_from_db() + + self.assertEqual(response.status_code, status.HTTP_200_OK) + # user3 should still be owner of t1 since was set prior + self.assertEqual(self.t1.owner, self.user3) + # user1 should now be owner of t2 since it didn't have an owner + self.assertEqual(self.t2.owner, self.user1) + + # user1 should be added + self.assertIn(self.user1, get_users_with_perms(self.t1)) + # user3 should be preserved + self.assertIn(self.user3, get_users_with_perms(self.t1)) + + # merge=False (default) + response = self.client.post( + "/api/bulk_edit_object_perms/", + json.dumps( + { + "objects": [self.t1.id, self.t2.id], + "object_type": "tags", + "permissions": permissions, + "merge": False, + }, + ), + content_type="application/json", + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + # user1 should be added + self.assertIn(self.user1, get_users_with_perms(self.t1)) + # user3 should be removed + self.assertNotIn(self.user3, get_users_with_perms(self.t1)) + def test_bulk_edit_object_permissions_insufficient_perms(self): """ GIVEN: diff --git a/src/documents/views.py b/src/documents/views.py index 9d44ecbc4..11fb5b1f2 100644 --- a/src/documents/views.py +++ b/src/documents/views.py @@ -1385,6 +1385,7 @@ class BulkEditObjectPermissionsView(GenericAPIView, PassUserMixin): object_class = serializer.get_object_class(object_type) permissions = serializer.validated_data.get("permissions") owner = serializer.validated_data.get("owner") + merge = serializer.validated_data.get("merge") if not user.is_superuser: objs = object_class.objects.filter(pk__in=object_ids) @@ -1396,12 +1397,21 @@ class BulkEditObjectPermissionsView(GenericAPIView, PassUserMixin): try: qs = object_class.objects.filter(id__in=object_ids) - if "owner" in serializer.validated_data: - qs.update(owner=owner) + # if merge is true, we dont want to remove the owner + if "owner" in serializer.validated_data and ( + not merge or (merge and owner is not None) + ): + # if merge is true, we dont want to overwrite the owner + qs_owner_update = qs.filter(owner__isnull=True) if merge else qs + qs_owner_update.update(owner=owner) if "permissions" in serializer.validated_data: for obj in qs: - set_permissions_for_object(permissions, obj) + set_permissions_for_object( + permissions=permissions, + object=obj, + merge=merge, + ) return Response({"result": "OK"}) except Exception as e: diff --git a/src/paperless/auth.py b/src/paperless/auth.py index a23b01cb4..98e2a8b30 100644 --- a/src/paperless/auth.py +++ b/src/paperless/auth.py @@ -47,3 +47,11 @@ class HttpRemoteUserMiddleware(PersistentRemoteUserMiddleware): """ header = settings.HTTP_REMOTE_USER_HEADER_NAME + + +class PaperlessRemoteUserAuthentication(authentication.RemoteUserAuthentication): + """ + REMOTE_USER authentication for DRF which overrides the default header. + """ + + header = settings.HTTP_REMOTE_USER_HEADER_NAME diff --git a/src/paperless/settings.py b/src/paperless/settings.py index bc815d4d5..c9d5848c0 100644 --- a/src/paperless/settings.py +++ b/src/paperless/settings.py @@ -420,19 +420,34 @@ if AUTO_LOGIN_USERNAME: # regular login in case the provided user does not exist. MIDDLEWARE.insert(_index + 1, "paperless.auth.AutoLoginMiddleware") -ENABLE_HTTP_REMOTE_USER = __get_boolean("PAPERLESS_ENABLE_HTTP_REMOTE_USER") -HTTP_REMOTE_USER_HEADER_NAME = os.getenv( - "PAPERLESS_HTTP_REMOTE_USER_HEADER_NAME", - "HTTP_REMOTE_USER", -) -if ENABLE_HTTP_REMOTE_USER: - MIDDLEWARE.append("paperless.auth.HttpRemoteUserMiddleware") - AUTHENTICATION_BACKENDS.insert(0, "django.contrib.auth.backends.RemoteUserBackend") - REST_FRAMEWORK["DEFAULT_AUTHENTICATION_CLASSES"].append( - "rest_framework.authentication.RemoteUserAuthentication", +def _parse_remote_user_settings() -> str: + global MIDDLEWARE, AUTHENTICATION_BACKENDS, REST_FRAMEWORK + enable = __get_boolean("PAPERLESS_ENABLE_HTTP_REMOTE_USER") + enable_api = __get_boolean("PAPERLESS_ENABLE_HTTP_REMOTE_USER_API") + if enable or enable_api: + MIDDLEWARE.append("paperless.auth.HttpRemoteUserMiddleware") + AUTHENTICATION_BACKENDS.insert( + 0, + "django.contrib.auth.backends.RemoteUserBackend", + ) + + if enable_api: + REST_FRAMEWORK["DEFAULT_AUTHENTICATION_CLASSES"].insert( + 0, + "paperless.auth.PaperlessRemoteUserAuthentication", + ) + + header_name = os.getenv( + "PAPERLESS_HTTP_REMOTE_USER_HEADER_NAME", + "HTTP_REMOTE_USER", ) + return header_name + + +HTTP_REMOTE_USER_HEADER_NAME = _parse_remote_user_settings() + # X-Frame options for embedded PDF display: X_FRAME_OPTIONS = "ANY" if DEBUG else "SAMEORIGIN" diff --git a/src/paperless/tests/test_remote_user.py b/src/paperless/tests/test_remote_user.py new file mode 100644 index 000000000..c5d7a6db4 --- /dev/null +++ b/src/paperless/tests/test_remote_user.py @@ -0,0 +1,110 @@ +import os +from unittest import mock + +from django.contrib.auth.models import User +from rest_framework import status +from rest_framework.test import APITestCase + +from documents.tests.utils import DirectoriesMixin +from paperless.settings import _parse_remote_user_settings + + +class TestRemoteUser(DirectoriesMixin, APITestCase): + def setUp(self): + super().setUp() + + self.user = User.objects.create_superuser( + username="temp_admin", + ) + + def test_remote_user(self): + """ + GIVEN: + - Configured user + - Remote user auth is enabled + WHEN: + - Call is made to root + THEN: + - Call succeeds + """ + + with mock.patch.dict( + os.environ, + { + "PAPERLESS_ENABLE_HTTP_REMOTE_USER": "True", + }, + ): + _parse_remote_user_settings() + + response = self.client.get("/documents/") + + self.assertEqual( + response.status_code, + status.HTTP_302_FOUND, + ) + + response = self.client.get( + "/documents/", + headers={ + "Remote-User": self.user.username, + }, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + + def test_remote_user_api(self): + """ + GIVEN: + - Configured user + - Remote user auth is enabled for the API + WHEN: + - API call is made to get documents + THEN: + - Call succeeds + """ + + with mock.patch.dict( + os.environ, + { + "PAPERLESS_ENABLE_HTTP_REMOTE_USER_API": "True", + }, + ): + _parse_remote_user_settings() + + response = self.client.get("/api/documents/") + + # 403 testing locally, 401 on ci... + self.assertIn( + response.status_code, + [status.HTTP_401_UNAUTHORIZED, status.HTTP_403_FORBIDDEN], + ) + + response = self.client.get( + "/api/documents/", + headers={ + "Remote-User": self.user.username, + }, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + + def test_remote_user_header_setting(self): + """ + GIVEN: + - Remote user header name is set + WHEN: + - Settings are parsed + THEN: + - Correct header name is returned + """ + + with mock.patch.dict( + os.environ, + { + "PAPERLESS_ENABLE_HTTP_REMOTE_USER": "True", + "PAPERLESS_HTTP_REMOTE_USER_HEADER_NAME": "HTTP_FOO", + }, + ): + header_name = _parse_remote_user_settings() + + self.assertEqual(header_name, "HTTP_FOO")