From 9fef18115586b7e72ed13d55512929e591069758 Mon Sep 17 00:00:00 2001 From: Vito Date: Wed, 27 Jun 2018 15:15:57 +0100 Subject: [PATCH] [ADF-2671] added permission check on permissions (#3521) * [ADF-2671] start adding permission check for changing permissions * [ADF-2671] added permission check for inherit button and permission dialog * [ADF-2671] added permission check for inherit button and permission dialog * [ADF-2671] start fixing and adding test for new permission check * ] [ADF-2671] improved check for node-permission directive * [ADF-2671] fixed and added more test for permission on permissions * [ADF-2671] reverting change on node-permission directive * [ADF-2671] fixing test for permission check --- lib/content-services/i18n/en.json | 3 +- .../add-permission.component.html | 2 +- .../add-permission.component.spec.ts | 36 +++++++-- .../add-permission.component.ts | 29 ++++--- .../inherited-button.directive.spec.ts | 20 ++++- .../components/inherited-button.directive.ts | 17 ++-- .../node-permission-dialog.service.spec.ts | 81 ++++++++++++++----- .../node-permission-dialog.service.ts | 42 ++++++---- 8 files changed, 170 insertions(+), 60 deletions(-) diff --git a/lib/content-services/i18n/en.json b/lib/content-services/i18n/en.json index 4b7b9d49b4..932e5ccee8 100644 --- a/lib/content-services/i18n/en.json +++ b/lib/content-services/i18n/en.json @@ -283,7 +283,8 @@ "EVERYONE" : "EVERYONE" }, "ERROR": { - "DUPLICATE-PERMISSION": "One or more of the permissions you have set is already present : {{list}}" + "DUPLICATE-PERMISSION": "One or more of the permissions you have set is already present : {{list}}", + "NOT-ALLOWED": "You are not allowed to change permissions" } } } diff --git a/lib/content-services/permission-manager/components/add-permission/add-permission.component.html b/lib/content-services/permission-manager/components/add-permission/add-permission.component.html index 99d3792e9d..3400e9c2b5 100644 --- a/lib/content-services/permission-manager/components/add-permission/add-permission.component.html +++ b/lib/content-services/permission-manager/components/add-permission/add-permission.component.html @@ -5,7 +5,7 @@ diff --git a/lib/content-services/permission-manager/components/add-permission/add-permission.component.spec.ts b/lib/content-services/permission-manager/components/add-permission/add-permission.component.spec.ts index 71c3f28321..998ff25bea 100644 --- a/lib/content-services/permission-manager/components/add-permission/add-permission.component.spec.ts +++ b/lib/content-services/permission-manager/components/add-permission/add-permission.component.spec.ts @@ -19,7 +19,7 @@ import { async, ComponentFixture, TestBed } from '@angular/core/testing'; import { AddPermissionComponent } from './add-permission.component'; import { AddPermissionPanelComponent } from './add-permission-panel.component'; import { By } from '@angular/platform-browser'; -import { setupTestBed } from '@alfresco/adf-core'; +import { setupTestBed, NodesApiService } from '@alfresco/adf-core'; import { Observable } from 'rxjs/Observable'; import { fakeAuthorityResults } from '../../../mock/add-permission.component.mock'; import { ContentTestingModule } from '../../../testing/content.testing.module'; @@ -30,6 +30,7 @@ describe('AddPermissionComponent', () => { let fixture: ComponentFixture; let element: HTMLElement; let nodePermissionService: NodePermissionService; + let nodeApiService: NodesApiService; setupTestBed({ imports: [ @@ -38,6 +39,8 @@ describe('AddPermissionComponent', () => { }); beforeEach(() => { + nodeApiService = TestBed.get(NodesApiService); + spyOn(nodeApiService, 'getNode').and.returnValue(Observable.of({ id: 'fake-node', allowableOperations: ['updatePermissions']})); fixture = TestBed.createComponent(AddPermissionComponent); element = fixture.nativeElement; nodePermissionService = TestBed.get(NodePermissionService); @@ -67,12 +70,24 @@ describe('AddPermissionComponent', () => { }); })); - it('should emit a success event when the node is updated', async(() => { + it('should NOT enable the ADD button when a selection is sent but the user does not have the permissions', async(() => { + const addPermissionPanelComponent: AddPermissionPanelComponent = fixture.debugElement.query(By.directive(AddPermissionPanelComponent)).componentInstance; + addPermissionPanelComponent.select.emit(fakeAuthorityResults); + fixture.componentInstance.currentNode = {id: 'fake-node-id'}; + fixture.whenStable().then(() => { + fixture.detectChanges(); + const addButton: HTMLButtonElement = element.querySelector('#adf-add-permission-action-button'); + expect(addButton.disabled).toBeTruthy(); + }); + })); + + it('should emit a success event when the node is updated', (done) => { fixture.componentInstance.selectedItems = fakeAuthorityResults; spyOn(nodePermissionService, 'updateNodePermissions').and.returnValue(Observable.of({ id: 'fake-node-id'})); fixture.componentInstance.success.subscribe((node) => { expect(node.id).toBe('fake-node-id'); + done(); }); fixture.detectChanges(); @@ -81,14 +96,25 @@ describe('AddPermissionComponent', () => { const addButton: HTMLButtonElement = element.querySelector('#adf-add-permission-action-button'); addButton.click(); }); - })); + }); - it('should emit an error event when the node update fail', async(() => { + it('should NOT emit a success event when the user does not have permission to update the node', () => { + fixture.componentInstance.selectedItems = fakeAuthorityResults; + fixture.componentInstance.currentNode = { id: 'fake-node-id' }; + spyOn(nodePermissionService, 'updateNodePermissions').and.returnValue(Observable.of({ id: 'fake-node-id' })); + + let spySuccess = spyOn(fixture.componentInstance, 'success'); + fixture.componentInstance.applySelection(); + expect(spySuccess).not.toHaveBeenCalled(); + }); + + it('should emit an error event when the node update fail', (done) => { fixture.componentInstance.selectedItems = fakeAuthorityResults; spyOn(nodePermissionService, 'updateNodePermissions').and.returnValue(Observable.throw({ error: 'errored'})); fixture.componentInstance.error.subscribe((error) => { expect(error.error).toBe('errored'); + done(); }); fixture.detectChanges(); @@ -97,6 +123,6 @@ describe('AddPermissionComponent', () => { const addButton: HTMLButtonElement = element.querySelector('#adf-add-permission-action-button'); addButton.click(); }); - })); + }); }); diff --git a/lib/content-services/permission-manager/components/add-permission/add-permission.component.ts b/lib/content-services/permission-manager/components/add-permission/add-permission.component.ts index f5e60bd87f..1fae58ecad 100644 --- a/lib/content-services/permission-manager/components/add-permission/add-permission.component.ts +++ b/lib/content-services/permission-manager/components/add-permission/add-permission.component.ts @@ -18,6 +18,7 @@ import { Component, ViewEncapsulation, EventEmitter, Input, Output } from '@angular/core'; import { MinimalNodeEntity, MinimalNodeEntryEntity } from 'alfresco-js-api'; import { NodePermissionService } from '../../services/node-permission.service'; +import { NodesApiService, ContentService, PermissionsEnum } from '@alfresco/adf-core'; @Component({ selector: 'adf-add-permission', @@ -43,22 +44,32 @@ export class AddPermissionComponent { currentNode: MinimalNodeEntryEntity; currentNodeRoles: string[]; - constructor(private nodePermissionService: NodePermissionService) { + constructor(private nodePermissionService: NodePermissionService, + private nodeApiService: NodesApiService, + private contentService: ContentService) { + this.nodeApiService.getNode(this.nodeId).subscribe((node) => this.currentNode = node); } onSelect(selection: MinimalNodeEntity[]) { this.selectedItems = selection; } + isAddEnabled(): boolean { + return this.contentService.hasPermission(this.currentNode, PermissionsEnum.UPDATEPERMISSIONS) && + this.selectedItems.length !== 0; + } + applySelection() { - this.nodePermissionService.updateNodePermissions(this.nodeId, this.selectedItems) - .subscribe( - (node) => { - this.success.emit(node); - }, - (error) => { - this.error.emit(error); - }); + if (this.contentService.hasPermission(this.currentNode, PermissionsEnum.UPDATEPERMISSIONS)) { + this.nodePermissionService.updateNodePermissions(this.nodeId, this.selectedItems) + .subscribe( + (node) => { + this.success.emit(node); + }, + (error) => { + this.error.emit(error); + }); + } } } diff --git a/lib/content-services/permission-manager/components/inherited-button.directive.spec.ts b/lib/content-services/permission-manager/components/inherited-button.directive.spec.ts index 4828b39779..9c0fd82bbf 100644 --- a/lib/content-services/permission-manager/components/inherited-button.directive.spec.ts +++ b/lib/content-services/permission-manager/components/inherited-button.directive.spec.ts @@ -21,8 +21,9 @@ import { InheritPermissionDirective } from './inherited-button.directive'; import { NodesApiService, setupTestBed, CoreModule } from '@alfresco/adf-core'; import { Observable } from 'rxjs/Observable'; -const fakeNodeWithInherit: any = { id: 'fake-id', permissions : {isInheritanceEnabled : true}}; -const fakeNodeNoInherit: any = { id: 'fake-id', permissions : {isInheritanceEnabled : false}}; +const fakeNodeWithInherit: any = { id: 'fake-id', permissions : {isInheritanceEnabled : true}, allowableOperations: ['updatePermissions']}; +const fakeNodeNoInherit: any = { id: 'fake-id', permissions : {isInheritanceEnabled : false}, allowableOperations: ['updatePermissions']}; +const fakeNodeWithInheritNoPermission: any = { id: 'fake-id', permissions : {isInheritanceEnabled : true}}; describe('InheritPermissionDirective', () => { @@ -95,4 +96,19 @@ describe('InheritPermissionDirective', () => { }); })); + it('should not update the node when node has no permission', async(() => { + spyOn(nodeService, 'getNode').and.returnValue(Observable.of(fakeNodeWithInheritNoPermission)); + let spyUpdateNode = spyOn(nodeService, 'updateNode'); + component.updatedNode = true; + fixture.detectChanges(); + const buttonPermission: HTMLButtonElement = element.querySelector('#sample-button-permission'); + expect(buttonPermission).not.toBeNull(); + expect(element.querySelector('#update-notification')).not.toBeNull(); + buttonPermission.click(); + fixture.detectChanges(); + fixture.whenStable().then(() => { + expect(spyUpdateNode).not.toHaveBeenCalled(); + }); + })); + }); diff --git a/lib/content-services/permission-manager/components/inherited-button.directive.ts b/lib/content-services/permission-manager/components/inherited-button.directive.ts index 9bc1c3faa9..12d4adbb71 100644 --- a/lib/content-services/permission-manager/components/inherited-button.directive.ts +++ b/lib/content-services/permission-manager/components/inherited-button.directive.ts @@ -17,7 +17,7 @@ /* tslint:disable:no-input-rename */ import { Directive, Input, Output, EventEmitter } from '@angular/core'; -import { NodesApiService } from '@alfresco/adf-core'; +import { NodesApiService, ContentService, PermissionsEnum } from '@alfresco/adf-core'; import { MinimalNodeEntryEntity } from 'alfresco-js-api'; @Directive({ @@ -38,15 +38,20 @@ export class InheritPermissionDirective { @Output() error: EventEmitter = new EventEmitter(); - constructor(private nodeService: NodesApiService) { + constructor(private nodeService: NodesApiService, + private contentService: ContentService) { } onInheritPermissionClicked() { this.nodeService.getNode(this.nodeId).subscribe((node: MinimalNodeEntryEntity) => { - const nodeBody = { permissions: { isInheritanceEnabled: !node.permissions.isInheritanceEnabled } }; - this.nodeService.updateNode(this.nodeId, nodeBody, { include: ['permissions'] }).subscribe((nodeUpdated: MinimalNodeEntryEntity) => { - this.updated.emit(nodeUpdated); - }, (error) => this.error.emit(error)); + if (this.contentService.hasPermission(node, PermissionsEnum.UPDATEPERMISSIONS)) { + const nodeBody = { permissions: { isInheritanceEnabled: !node.permissions.isInheritanceEnabled } }; + this.nodeService.updateNode(this.nodeId, nodeBody, { include: ['permissions'] }).subscribe((nodeUpdated: MinimalNodeEntryEntity) => { + this.updated.emit(nodeUpdated); + }, (error) => this.error.emit(error)); + } else { + this.error.emit('PERMISSION_MANAGER.ERROR.NOT-ALLOWED'); + } }); } diff --git a/lib/content-services/permission-manager/services/node-permission-dialog.service.spec.ts b/lib/content-services/permission-manager/services/node-permission-dialog.service.spec.ts index afcd3b7c17..06b3465568 100644 --- a/lib/content-services/permission-manager/services/node-permission-dialog.service.spec.ts +++ b/lib/content-services/permission-manager/services/node-permission-dialog.service.spec.ts @@ -16,13 +16,14 @@ */ import { TestBed, async } from '@angular/core/testing'; -import { AppConfigService, setupTestBed } from '@alfresco/adf-core'; +import { AppConfigService, setupTestBed, ContentService } from '@alfresco/adf-core'; import { NodePermissionDialogService } from './node-permission-dialog.service'; import { MatDialog } from '@angular/material'; import { Observable } from 'rxjs/Observable'; import { Subject } from 'rxjs/Subject'; import { ContentTestingModule } from '../../testing/content.testing.module'; import { NodePermissionService } from './node-permission.service'; +import { Node } from 'alfresco-js-api'; describe('NodePermissionDialogService', () => { @@ -31,6 +32,7 @@ describe('NodePermissionDialogService', () => { let spyOnDialogOpen: jasmine.Spy; let afterOpenObservable: Subject; let nodePermissionService: NodePermissionService; + let contentService: ContentService; setupTestBed({ imports: [ContentTestingModule], @@ -44,6 +46,7 @@ describe('NodePermissionDialogService', () => { materialDialog = TestBed.get(MatDialog); afterOpenObservable = new Subject(); nodePermissionService = TestBed.get(NodePermissionService); + contentService = TestBed.get(ContentService); spyOnDialogOpen = spyOn(materialDialog, 'open').and.returnValue({ afterOpen: () => afterOpenObservable, afterClosed: () => Observable.of({}), @@ -57,27 +60,65 @@ describe('NodePermissionDialogService', () => { expect(service).not.toBeNull(); }); - it('should be able to open the dialog showing node permissions', () => { - service.openAddPermissionDialog('fake-node-id', 'fake-title'); - expect(spyOnDialogOpen).toHaveBeenCalled(); + describe('when node has permission to update permissions', () => { + + let fakePermissionNode = {}; + + beforeEach(() => { + fakePermissionNode = { id: 'fake-permission-node', allowableOperations: ['updatePermissions']}; + }); + + it('should be able to open the dialog showing node permissions', () => { + service.openAddPermissionDialog(fakePermissionNode, 'fake-title'); + expect(spyOnDialogOpen).toHaveBeenCalled(); + }); + + it('should return the updated node', (done) => { + spyOn(nodePermissionService, 'updateNodePermissions').and.returnValue(Observable.of({id : 'fake-node-updated'})); + spyOn(service, 'openAddPermissionDialog').and.returnValue(Observable.of({})); + spyOn(contentService, 'getNode').and.returnValue(Observable.of(fakePermissionNode)); + service.updateNodePermissionByDialog('fake-node-id', 'fake-title').subscribe((node) => { + expect(node.id).toBe('fake-node-updated'); + done(); + }); + }); + + it('should throw an error if the update of the node fails', (done) => { + spyOn(nodePermissionService, 'updateNodePermissions').and.returnValue(Observable.throw({error : 'error'})); + spyOn(service, 'openAddPermissionDialog').and.returnValue(Observable.of({})); + spyOn(contentService, 'getNode').and.returnValue(Observable.of(fakePermissionNode)); + service.updateNodePermissionByDialog('fake-node-id', 'fake-title').subscribe(() => { + Observable.throw('This call should fail'); + }, (error) => { + expect(error.error).toBe('error'); + done(); + }); + }); }); - it('should throw an error if the update of the node fails', async(() => { - spyOn(nodePermissionService, 'updateNodePermissions').and.returnValue(Observable.throw({error : 'error'})); - spyOn(service, 'openAddPermissionDialog').and.returnValue(Observable.of({})); - service.updateNodePermissionByDialog('fake-node-id', 'fake-title').subscribe(() => { - Observable.throw('This call should fail'); - }, (error) => { - expect(error.error).toBe('error'); - }); - })); + describe('when node does not have permission to update permissions', () => { - it('should return the updated node', async(() => { - spyOn(nodePermissionService, 'updateNodePermissions').and.returnValue(Observable.of({id : 'fake-node'})); - spyOn(service, 'openAddPermissionDialog').and.returnValue(Observable.of({})); - service.updateNodePermissionByDialog('fake-node-id', 'fake-title').subscribe((node) => { - expect(node.id).toBe('fake-node'); - }); - })); + let fakeForbiddenNode = {}; + beforeEach(() => { + fakeForbiddenNode = { id: 'fake-permission-node', allowableOperations: ['update']}; + }); + + it('should not be able to open the dialog showing node permissions', () => { + service.openAddPermissionDialog(fakeForbiddenNode, 'fake-title'); + expect(spyOnDialogOpen).not.toHaveBeenCalled(); + }); + + it('should return the updated node', (done) => { + spyOn(contentService, 'getNode').and.returnValue(Observable.of(fakeForbiddenNode)); + service.updateNodePermissionByDialog('fake-node-id', 'fake-title').subscribe((node) => { + Observable.throw('This call should fail'); + }, + (error) => { + expect(error.message).toBe('PERMISSION_MANAGER.ERROR.NOT-ALLOWED'); + done(); + }); + }); + + }); }); diff --git a/lib/content-services/permission-manager/services/node-permission-dialog.service.ts b/lib/content-services/permission-manager/services/node-permission-dialog.service.ts index fa6de27a9b..ded77fe82a 100644 --- a/lib/content-services/permission-manager/services/node-permission-dialog.service.ts +++ b/lib/content-services/permission-manager/services/node-permission-dialog.service.ts @@ -21,14 +21,16 @@ import { Subject } from 'rxjs/Subject'; import { Observable } from 'rxjs/Observable'; import { AddPermissionDialogComponent } from '../components/add-permission/add-permission-dialog.component'; import { AddPermissionDialogData } from '../components/add-permission/add-permission-dialog-data.interface'; -import { MinimalNodeEntity, MinimalNodeEntryEntity } from 'alfresco-js-api'; +import { MinimalNodeEntity, MinimalNodeEntryEntity, Node } from 'alfresco-js-api'; import { NodePermissionService } from './node-permission.service'; +import { ContentService, PermissionsEnum } from '@alfresco/adf-core'; @Injectable() export class NodePermissionDialogService { constructor(private dialog: MatDialog, - private nodePermissionService: NodePermissionService) { + private nodePermissionService: NodePermissionService, + private contentService: ContentService) { } /** @@ -37,21 +39,27 @@ export class NodePermissionDialogService { * @param title Dialog title * @returns Node with updated permissions */ - openAddPermissionDialog(nodeId: string, title?: string): Observable { - const confirm = new Subject(); + openAddPermissionDialog(node: Node, title?: string): Observable { + if (this.contentService.hasPermission(node, PermissionsEnum.UPDATEPERMISSIONS)) { + const confirm = new Subject(); - confirm.subscribe({ - complete: this.close.bind(this) - }); + confirm.subscribe({ + complete: this.close.bind(this) + }); - const data: AddPermissionDialogData = { - nodeId: nodeId, - title: title, - confirm : confirm - }; + const data: AddPermissionDialogData = { + nodeId: node.id, + title: title, + confirm: confirm + }; - this.openDialog(data, 'adf-add-permission-dialog', '630px'); - return confirm; + this.openDialog(data, 'adf-add-permission-dialog', '630px'); + return confirm; + } else { + let errors = new Error(JSON.stringify({ error: { statusCode: 403 } })); + errors.message = 'PERMISSION_MANAGER.ERROR.NOT-ALLOWED'; + return Observable.throw(errors); + } } private openDialog(data: any, currentPanelClass: string, chosenWidth: string) { @@ -72,8 +80,10 @@ export class NodePermissionDialogService { * @returns Node with updated permissions */ updateNodePermissionByDialog(nodeId?: string, title?: string): Observable { - return this.openAddPermissionDialog(nodeId, title).switchMap((selection) => { - return this.nodePermissionService.updateNodePermissions(nodeId, selection); + return this.contentService.getNode(nodeId, { include: ['allowableOperations'] }).switchMap((node) => { + return this.openAddPermissionDialog(node.entry, title).switchMap((selection) => { + return this.nodePermissionService.updateNodePermissions(nodeId, selection); + }); }); } }