From 4db383850fd9f11ec531c18ee2118de8830a21e3 Mon Sep 17 00:00:00 2001 From: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Date: Wed, 17 Apr 2024 11:03:48 +1000 Subject: [PATCH] [AC-2172] Member modal - limit admin access (#8343) * limit admin permissions to assign members to collections that the admin doesn't have can manage permissions for --- .../member-dialog/member-dialog.component.ts | 97 ++++++++++++++----- .../vault/core/collection-admin.service.ts | 3 + .../models/response/collection.response.ts | 11 +-- 3 files changed, 81 insertions(+), 30 deletions(-) diff --git a/apps/web/src/app/admin-console/organizations/members/components/member-dialog/member-dialog.component.ts b/apps/web/src/app/admin-console/organizations/members/components/member-dialog/member-dialog.component.ts index 752122de00..771b8cc505 100644 --- a/apps/web/src/app/admin-console/organizations/members/components/member-dialog/member-dialog.component.ts +++ b/apps/web/src/app/admin-console/organizations/members/components/member-dialog/member-dialog.component.ts @@ -31,6 +31,7 @@ import { CollectionView } from "@bitwarden/common/vault/models/view/collection.v import { DialogService } from "@bitwarden/components"; import { CollectionAdminService } from "../../../../../vault/core/collection-admin.service"; +import { CollectionAdminView } from "../../../../../vault/core/views/collection-admin.view"; import { CollectionAccessSelectionView, GroupService, @@ -206,25 +207,52 @@ export class MemberDialogComponent implements OnDestroy { collections: this.collectionAdminService.getAll(this.params.organizationId), userDetails: userDetails$, groups: groups$, + flexibleCollectionsV1Enabled: this.configService.getFeatureFlag$( + FeatureFlag.FlexibleCollectionsV1, + false, + ), }) .pipe(takeUntil(this.destroy$)) - .subscribe(({ organization, collections, userDetails, groups }) => { - this.setFormValidators(organization); + .subscribe( + ({ organization, collections, userDetails, groups, flexibleCollectionsV1Enabled }) => { + this.setFormValidators(organization); - this.collectionAccessItems = [].concat( - collections.map((c) => mapCollectionToAccessItemView(c)), - ); + // Groups tab: populate available groups + this.groupAccessItems = [].concat( + groups.map((g) => mapGroupToAccessItemView(g)), + ); - this.groupAccessItems = [].concat( - groups.map((g) => mapGroupToAccessItemView(g)), - ); + // Collections tab: Populate all available collections (including current user access where applicable) + this.collectionAccessItems = collections + .map((c) => + mapCollectionToAccessItemView( + c, + organization, + flexibleCollectionsV1Enabled, + userDetails == null + ? undefined + : c.users.find((access) => access.id === userDetails.id), + ), + ) + // But remove collections that we can't assign access to, unless the user is already assigned + .filter( + (item) => + !item.readonly || userDetails?.collections.some((access) => access.id == item.id), + ); - if (this.params.organizationUserId) { - this.loadOrganizationUser(userDetails, groups, collections); - } + if (userDetails != null) { + this.loadOrganizationUser( + userDetails, + groups, + collections, + organization, + flexibleCollectionsV1Enabled, + ); + } - this.loading = false; - }); + this.loading = false; + }, + ); } private setFormValidators(organization: Organization) { @@ -246,7 +274,9 @@ export class MemberDialogComponent implements OnDestroy { private loadOrganizationUser( userDetails: OrganizationUserAdminView, groups: GroupView[], - collections: CollectionView[], + collections: CollectionAdminView[], + organization: Organization, + flexibleCollectionsV1Enabled: boolean, ) { if (!userDetails) { throw new Error("Could not find user to edit."); @@ -295,13 +325,22 @@ export class MemberDialogComponent implements OnDestroy { }), ); + // Populate additional collection access via groups (rendered as separate rows from user access) this.collectionAccessItems = this.collectionAccessItems.concat( collectionsFromGroups.map(({ collection, accessSelection, group }) => - mapCollectionToAccessItemView(collection, accessSelection, group), + mapCollectionToAccessItemView( + collection, + organization, + flexibleCollectionsV1Enabled, + accessSelection, + group, + ), ), ); - const accessSelections = mapToAccessSelections(userDetails); + // Set current collections and groups the user has access to (excluding collections the current user doesn't have + // permissions to change - they are included as readonly via the CollectionAccessItems) + const accessSelections = mapToAccessSelections(userDetails, this.collectionAccessItems); const groupAccessSelections = mapToGroupAccessSelections(userDetails.groups); this.formGroup.removeControl("emails"); @@ -573,6 +612,8 @@ export class MemberDialogComponent implements OnDestroy { function mapCollectionToAccessItemView( collection: CollectionView, + organization: Organization, + flexibleCollectionsV1Enabled: boolean, accessSelection?: CollectionAccessSelectionView, group?: GroupView, ): AccessItemView { @@ -581,7 +622,8 @@ function mapCollectionToAccessItemView( id: group ? `${collection.id}-${group.id}` : collection.id, labelName: collection.name, listName: collection.name, - readonly: group !== undefined, + readonly: + group !== undefined || !collection.canEdit(organization, flexibleCollectionsV1Enabled), readonlyPermission: accessSelection ? convertToPermission(accessSelection) : undefined, viaGroupName: group?.name, }; @@ -596,16 +638,23 @@ function mapGroupToAccessItemView(group: GroupView): AccessItemView { }; } -function mapToAccessSelections(user: OrganizationUserAdminView): AccessItemValue[] { +function mapToAccessSelections( + user: OrganizationUserAdminView, + items: AccessItemView[], +): AccessItemValue[] { if (user == undefined) { return []; } - return [].concat( - user.collections.map((selection) => ({ - id: selection.id, - type: AccessItemType.Collection, - permission: convertToPermission(selection), - })), + + return ( + user.collections + // The FormControl value only represents editable collection access - exclude readonly access selections + .filter((selection) => !items.find((item) => item.id == selection.id).readonly) + .map((selection) => ({ + id: selection.id, + type: AccessItemType.Collection, + permission: convertToPermission(selection), + })) ); } diff --git a/apps/web/src/app/vault/core/collection-admin.service.ts b/apps/web/src/app/vault/core/collection-admin.service.ts index 74f825e1ac..7f78ab214a 100644 --- a/apps/web/src/app/vault/core/collection-admin.service.ts +++ b/apps/web/src/app/vault/core/collection-admin.service.ts @@ -124,6 +124,9 @@ export class CollectionAdminService { view.groups = c.groups; view.users = c.users; view.assigned = c.assigned; + view.readOnly = c.readOnly; + view.hidePasswords = c.hidePasswords; + view.manage = c.manage; } return view; diff --git a/libs/common/src/vault/models/response/collection.response.ts b/libs/common/src/vault/models/response/collection.response.ts index f16fe547e0..ac4781df71 100644 --- a/libs/common/src/vault/models/response/collection.response.ts +++ b/libs/common/src/vault/models/response/collection.response.ts @@ -21,6 +21,10 @@ export class CollectionDetailsResponse extends CollectionResponse { readOnly: boolean; manage: boolean; hidePasswords: boolean; + + /** + * Flag indicating the user has been explicitly assigned to this Collection + */ assigned: boolean; constructor(response: any) { @@ -35,15 +39,10 @@ export class CollectionDetailsResponse extends CollectionResponse { } } -export class CollectionAccessDetailsResponse extends CollectionResponse { +export class CollectionAccessDetailsResponse extends CollectionDetailsResponse { groups: SelectionReadOnlyResponse[] = []; users: SelectionReadOnlyResponse[] = []; - /** - * Flag indicating the user has been explicitly assigned to this Collection - */ - assigned: boolean; - constructor(response: any) { super(response); this.assigned = this.getResponseProperty("Assigned") || false;