From 9dd859af7a759b9769d64ecf753b12b1f145e13e Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Wed, 20 Oct 2021 16:17:27 -0500 Subject: [PATCH] Limit collection actions presented to permitted (#1247) * Limit collection actions presented to permitted * Revert useless move * Limit vault view to editable ciphers and collections * Update jslib * PR review --- .../organizations-routing.module.ts | 7 +- jslib | 2 +- .../manage/collection-add-edit.component.html | 22 +++--- .../manage/collection-add-edit.component.ts | 2 + .../manage/collections.component.html | 8 +-- .../manage/collections.component.ts | 67 ++++++++++++++++--- .../manage/manage.component.html | 2 +- .../organizations/vault/ciphers.component.ts | 2 +- .../vault/groupings.component.ts | 2 +- src/locales/en/messages.json | 3 + 10 files changed, 88 insertions(+), 29 deletions(-) diff --git a/bitwarden_license/src/app/organizations/organizations-routing.module.ts b/bitwarden_license/src/app/organizations/organizations-routing.module.ts index 1ad83403ca2..5ad731eb54c 100644 --- a/bitwarden_license/src/app/organizations/organizations-routing.module.ts +++ b/bitwarden_license/src/app/organizations/organizations-routing.module.ts @@ -24,8 +24,11 @@ const routes: Routes = [ canActivate: [OrganizationTypeGuardService], data: { permissions: [ - Permissions.ManageAssignedCollections, - Permissions.ManageAllCollections, + Permissions.CreateNewCollections, + Permissions.EditAnyCollection, + Permissions.DeleteAnyCollection, + Permissions.EditAssignedCollections, + Permissions.DeleteAssignedCollections, Permissions.AccessEventLogs, Permissions.ManageGroups, Permissions.ManageUsers, diff --git a/jslib b/jslib index f09fb698825..815b436f7ce 160000 --- a/jslib +++ b/jslib @@ -1 +1 @@ -Subproject commit f09fb69882525b3be7b2e257e7723eeb79b343d1 +Subproject commit 815b436f7ce9f8825575f288b1ae98c1dc54f1d2 diff --git a/src/app/organizations/manage/collection-add-edit.component.html b/src/app/organizations/manage/collection-add-edit.component.html index 18c71da9052..26cb81c8d13 100644 --- a/src/app/organizations/manage/collection-add-edit.component.html +++ b/src/app/organizations/manage/collection-add-edit.component.html @@ -15,17 +15,18 @@
+ appAutofocus [disabled]="!this.canSave">
- + {{'externalIdDesc' | i18n}}

{{'groupAccess' | i18n}} -
+
@@ -50,7 +51,7 @@ + [disabled]="g.accessAll || !this.canSave" appStopProp> {{g.name}} @@ -62,11 +63,11 @@ + name="Groups[{{i}}].HidePasswords" [disabled]="!g.checked || g.accessAll || !this.canSave"> + [disabled]="!g.checked || g.accessAll || !this.canSave"> @@ -74,22 +75,23 @@
- -
+
+
diff --git a/src/app/organizations/manage/collection-add-edit.component.ts b/src/app/organizations/manage/collection-add-edit.component.ts index 1f5561953fb..c4c66e6e79f 100644 --- a/src/app/organizations/manage/collection-add-edit.component.ts +++ b/src/app/organizations/manage/collection-add-edit.component.ts @@ -30,6 +30,8 @@ import { Utils } from 'jslib-common/misc/utils'; export class CollectionAddEditComponent implements OnInit { @Input() collectionId: string; @Input() organizationId: string; + @Input() canSave: boolean; + @Input() canDelete: boolean; @Output() onSavedCollection = new EventEmitter(); @Output() onDeletedCollection = new EventEmitter(); diff --git a/src/app/organizations/manage/collections.component.html b/src/app/organizations/manage/collections.component.html index cb466557146..3d85da045d5 100644 --- a/src/app/organizations/manage/collections.component.html +++ b/src/app/organizations/manage/collections.component.html @@ -6,7 +6,7 @@ - @@ -27,17 +27,17 @@ {{c.name}} -
+
- + {{'users' | i18n}} - + {{'delete' | i18n}} diff --git a/src/app/organizations/manage/collections.component.ts b/src/app/organizations/manage/collections.component.ts index 9ddf1149e49..14171cd1740 100644 --- a/src/app/organizations/manage/collections.component.ts +++ b/src/app/organizations/manage/collections.component.ts @@ -21,6 +21,7 @@ import { ModalService } from 'jslib-angular/services/modal.service'; import { CollectionData } from 'jslib-common/models/data/collectionData'; import { Collection } from 'jslib-common/models/domain/collection'; +import { Organization } from 'jslib-common/models/domain/organization'; import { CollectionDetailsResponse, CollectionResponse, @@ -40,8 +41,11 @@ export class CollectionsComponent implements OnInit { @ViewChild('usersTemplate', { read: ViewContainerRef, static: true }) usersModalRef: ViewContainerRef; loading = true; + organization: Organization; + canCreate: boolean = false; organizationId: string; collections: CollectionView[]; + assignedCollections: CollectionView[]; pagedCollections: CollectionView[]; searchText: string; @@ -67,16 +71,27 @@ export class CollectionsComponent implements OnInit { } async load() { - const organization = await this.userService.getOrganization(this.organizationId); - let response: ListResponse; - if (organization.canViewAllCollections) { - response = await this.apiService.getCollections(this.organizationId); - } else { - response = await this.apiService.getUserCollections(); + this.organization = await this.userService.getOrganization(this.organizationId); + this.canCreate = this.organization.canCreateNewCollections; + + const decryptCollections = async (r: ListResponse) => { + const collections = r.data.filter(c => c.organizationId === this.organizationId).map(d => + new Collection(new CollectionData(d as CollectionDetailsResponse))); + return await this.collectionService.decryptMany(collections); + }; + + if (this.organization.canViewAssignedCollections) { + const response = await this.apiService.getUserCollections(); + this.assignedCollections = await decryptCollections(response); } - const collections = response.data.filter(c => c.organizationId === this.organizationId).map(r => - new Collection(new CollectionData(r as CollectionDetailsResponse))); - this.collections = await this.collectionService.decryptMany(collections); + + if (this.organization.canViewAllCollections) { + const response = await this.apiService.getCollections(this.organizationId); + this.collections = await decryptCollections(response); + } else { + this.collections = this.assignedCollections; + } + this.resetPaging(); this.loading = false; } @@ -99,9 +114,20 @@ export class CollectionsComponent implements OnInit { } async edit(collection: CollectionView) { + const canCreate = collection == null && this.canCreate; + const canEdit = collection != null && this.canEdit(collection); + const canDelete = collection != null && this.canDelete(collection); + + if (!(canCreate || canEdit || canDelete)) { + this.toasterService.popAsync('error', null, this.i18nService.t('missingPermissions')); + return; + } + const [modal] = await this.modalService.openViewRef(CollectionAddEditComponent, this.addEditModalRef, comp => { comp.organizationId = this.organizationId; comp.collectionId = collection != null ? collection.id : null; + comp.canSave = canCreate || canEdit; + comp.canDelete = canDelete; comp.onSavedCollection.subscribe(() => { modal.close(); this.load(); @@ -131,6 +157,7 @@ export class CollectionsComponent implements OnInit { this.removeCollection(collection); } catch (e) { this.logService.error(e); + this.toasterService.popAsync('error', null, this.i18nService.t('missingPermissions')); } } @@ -165,6 +192,28 @@ export class CollectionsComponent implements OnInit { return !searching && this.collections && this.collections.length > this.pageSize; } + canEdit(collection: CollectionView) { + if (this.organization.canEditAnyCollection) { + return true; + } + + if (this.organization.canEditAssignedCollections && this.assignedCollections.some(c => c.id === collection.id)) { + return true; + } + return false; + } + + canDelete(collection: CollectionView) { + if (this.organization.canDeleteAnyCollection) { + return true; + } + + if (this.organization.canDeleteAssignedCollections && this.assignedCollections.some(c => c.id === collection.id)) { + return true; + } + return false; + } + private removeCollection(collection: CollectionView) { const index = this.collections.indexOf(collection); if (index > -1) { diff --git a/src/app/organizations/manage/manage.component.html b/src/app/organizations/manage/manage.component.html index 2182f1fbc1b..1cb4384b6ab 100644 --- a/src/app/organizations/manage/manage.component.html +++ b/src/app/organizations/manage/manage.component.html @@ -9,7 +9,7 @@ {{'people' | i18n}} + *ngIf="organization.canViewAllCollections || organization.canViewAssignedCollections"> {{'collections' | i18n}} boolean = null) { - if (this.organization.canViewAllCollections) { + if (this.organization.canEditAnyCollection) { this.accessEvents = this.organization.useEvents; this.allCiphers = await this.cipherService.getAllFromApiForOrganization(this.organization.id); } else { diff --git a/src/app/organizations/vault/groupings.component.ts b/src/app/organizations/vault/groupings.component.ts index 845b007df60..fc9dc8d0b5a 100644 --- a/src/app/organizations/vault/groupings.component.ts +++ b/src/app/organizations/vault/groupings.component.ts @@ -29,7 +29,7 @@ export class GroupingsComponent extends BaseGroupingsComponent { } async loadCollections() { - if (!this.organization.canViewAllCollections) { + if (!this.organization.canEditAnyCollection) { await super.loadCollections(this.organization.id); return; } diff --git a/src/locales/en/messages.json b/src/locales/en/messages.json index 8fbc4a67ffe..fbf9c008eb8 100644 --- a/src/locales/en/messages.json +++ b/src/locales/en/messages.json @@ -3817,6 +3817,9 @@ "accessReports": { "message": "Access Reports" }, + "missingPermissions": { + "message": "You lack the necessary permissions to perform this action." + }, "manageAllCollections": { "message": "Manage All Collections" },