From 5e3d1b26e759185a4286249f90194adafe4ef2bf Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Thu, 1 Feb 2024 01:20:14 -0800 Subject: [PATCH] Fix: Dont attempt to retrieve objects for which user doesnt have global permissions (#5612) --- ...permissions-filter-dropdown.component.html | 34 ++++--- .../permissions-filter-dropdown.component.ts | 2 +- .../saved-view-widget.component.html | 24 +++-- .../saved-view-widget.component.ts | 4 +- .../document-detail.component.spec.ts | 28 +++++- .../document-detail.component.ts | 63 ++++++++---- .../bulk-editor/bulk-editor.component.html | 98 ++++++++++--------- .../bulk-editor/bulk-editor.component.spec.ts | 18 ++++ .../bulk-editor/bulk-editor.component.ts | 60 +++++++++--- .../document-card-small.component.ts | 2 +- .../filter-editor.component.html | 36 ++++--- .../filter-editor.component.spec.ts | 39 +++++++- .../filter-editor/filter-editor.component.ts | 70 ++++++++++--- .../src/app/services/rest/document.service.ts | 40 +++++++- 14 files changed, 375 insertions(+), 143 deletions(-) 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..8edfcc7e1 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 @@ -868,4 +868,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..1c7e0b9c6 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') 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 08f680b07..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 @@ -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/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