From 9dda5e8ee16ea537e6d892028cabad0c360619a1 Mon Sep 17 00:00:00 2001
From: Thomas Rittson <31796059+eliykat@users.noreply.github.com>
Date: Thu, 2 May 2024 09:54:18 +1000
Subject: [PATCH] [AC-2170] Group modal - limit admin access - collections tab
 (#8758)

* Update Group modal -> Collections tab to respect collection management settings,
  e.g. only allow admins to assign access to collections they can manage
* Update collectionAdminView getters for custom permissions
---
 .../manage/group-add-edit.component.html      |   7 +-
 .../manage/group-add-edit.component.ts        | 203 +++++++++++-------
 .../vault/core/views/collection-admin.view.ts |   9 +-
 apps/web/src/locales/en/messages.json         |   3 +
 4 files changed, 140 insertions(+), 82 deletions(-)

diff --git a/apps/web/src/app/admin-console/organizations/manage/group-add-edit.component.html b/apps/web/src/app/admin-console/organizations/manage/group-add-edit.component.html
index 3afb816e14..5fcf7b0f42 100644
--- a/apps/web/src/app/admin-console/organizations/manage/group-add-edit.component.html
+++ b/apps/web/src/app/admin-console/organizations/manage/group-add-edit.component.html
@@ -50,7 +50,12 @@
         </bit-tab>
 
         <bit-tab label="{{ 'collections' | i18n }}">
-          <p>{{ "editGroupCollectionsDesc" | i18n }}</p>
+          <p>
+            {{ "editGroupCollectionsDesc" | i18n }}
+            <span *ngIf="!(allowAdminAccessToAllCollectionItems$ | async)">
+              {{ "editGroupCollectionsRestrictionsDesc" | i18n }}
+            </span>
+          </p>
           <div *ngIf="!(flexibleCollectionsEnabled$ | async)" class="tw-my-3">
             <input type="checkbox" formControlName="accessAll" id="accessAll" />
             <label class="tw-mb-0 tw-text-lg" for="accessAll">{{
diff --git a/apps/web/src/app/admin-console/organizations/manage/group-add-edit.component.ts b/apps/web/src/app/admin-console/organizations/manage/group-add-edit.component.ts
index b18effac86..aabaac2f1c 100644
--- a/apps/web/src/app/admin-console/organizations/manage/group-add-edit.component.ts
+++ b/apps/web/src/app/admin-console/organizations/manage/group-add-edit.component.ts
@@ -11,13 +11,13 @@ import {
   of,
   shareReplay,
   Subject,
-  switchMap,
   takeUntil,
 } from "rxjs";
 
 import { ApiService } from "@bitwarden/common/abstractions/api.service";
 import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
 import { OrganizationUserService } from "@bitwarden/common/admin-console/abstractions/organization-user/organization-user.service";
+import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
 import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
 import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
 import { ErrorResponse } from "@bitwarden/common/models/response/error.response";
@@ -26,12 +26,10 @@ import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.servic
 import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
 import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
 import { UserId } from "@bitwarden/common/types/guid";
-import { CollectionService } from "@bitwarden/common/vault/abstractions/collection.service";
-import { CollectionData } from "@bitwarden/common/vault/models/data/collection.data";
-import { Collection } from "@bitwarden/common/vault/models/domain/collection";
-import { CollectionDetailsResponse } from "@bitwarden/common/vault/models/response/collection.response";
 import { DialogService } from "@bitwarden/components";
 
+import { CollectionAdminService } from "../../../vault/core/collection-admin.service";
+import { CollectionAdminView } from "../../../vault/core/views/collection-admin.view";
 import { InternalGroupService as GroupService, GroupView } from "../core";
 import {
   AccessItemType,
@@ -95,9 +93,15 @@ export const openGroupAddEditDialog = (
   templateUrl: "group-add-edit.component.html",
 })
 export class GroupAddEditComponent implements OnInit, OnDestroy {
-  protected flexibleCollectionsEnabled$ = this.organizationService
+  private organization$ = this.organizationService
     .get$(this.organizationId)
-    .pipe(map((o) => o?.flexibleCollections));
+    .pipe(shareReplay({ refCount: true }));
+  protected flexibleCollectionsEnabled$ = this.organization$.pipe(
+    map((o) => o?.flexibleCollections),
+  );
+  private flexibleCollectionsV1Enabled$ = this.configService.getFeatureFlag$(
+    FeatureFlag.FlexibleCollectionsV1,
+  );
 
   protected PermissionMode = PermissionMode;
   protected ResultType = GroupAddEditDialogResultType;
@@ -131,27 +135,9 @@ export class GroupAddEditComponent implements OnInit, OnDestroy {
 
   private destroy$ = new Subject<void>();
 
-  private get orgCollections$() {
-    return from(this.apiService.getCollections(this.organizationId)).pipe(
-      switchMap((response) => {
-        return from(
-          this.collectionService.decryptMany(
-            response.data.map(
-              (r) => new Collection(new CollectionData(r as CollectionDetailsResponse)),
-            ),
-          ),
-        );
-      }),
-      map((collections) =>
-        collections.map<AccessItemView>((c) => ({
-          id: c.id,
-          type: AccessItemType.Collection,
-          labelName: c.name,
-          listName: c.name,
-        })),
-      ),
-    );
-  }
+  private orgCollections$ = from(this.collectionAdminService.getAll(this.organizationId)).pipe(
+    shareReplay({ refCount: false }),
+  );
 
   private get orgMembers$(): Observable<Array<AccessItemView & { userId: UserId }>> {
     return from(this.organizationUserService.getAllUsers(this.organizationId)).pipe(
@@ -197,23 +183,24 @@ export class GroupAddEditComponent implements OnInit, OnDestroy {
     shareReplay({ refCount: true, bufferSize: 1 }),
   );
 
-  restrictGroupAccess$ = combineLatest([
-    this.organizationService.get$(this.organizationId),
-    this.configService.getFeatureFlag$(FeatureFlag.FlexibleCollectionsV1),
-    this.groupDetails$,
+  allowAdminAccessToAllCollectionItems$ = combineLatest([
+    this.organization$,
+    this.flexibleCollectionsV1Enabled$,
   ]).pipe(
-    map(
-      ([organization, flexibleCollectionsV1Enabled, group]) =>
-        // Feature flag conditionals
-        flexibleCollectionsV1Enabled &&
-        organization.flexibleCollections &&
-        // Business logic conditionals
-        !organization.allowAdminAccessToAllCollectionItems &&
-        group !== undefined,
-    ),
-    shareReplay({ refCount: true, bufferSize: 1 }),
+    map(([organization, flexibleCollectionsV1Enabled]) => {
+      if (!flexibleCollectionsV1Enabled || !organization.flexibleCollections) {
+        return true;
+      }
+
+      return organization.allowAdminAccessToAllCollectionItems;
+    }),
   );
 
+  restrictGroupAccess$ = combineLatest([
+    this.allowAdminAccessToAllCollectionItems$,
+    this.groupDetails$,
+  ]).pipe(map(([allowAdminAccess, groupDetails]) => !allowAdminAccess && groupDetails != null));
+
   constructor(
     @Inject(DIALOG_DATA) private params: GroupAddEditDialogParams,
     private dialogRef: DialogRef<GroupAddEditDialogResultType>,
@@ -221,7 +208,6 @@ export class GroupAddEditComponent implements OnInit, OnDestroy {
     private organizationUserService: OrganizationUserService,
     private groupService: GroupService,
     private i18nService: I18nService,
-    private collectionService: CollectionService,
     private platformUtilsService: PlatformUtilsService,
     private logService: LogService,
     private formBuilder: FormBuilder,
@@ -230,6 +216,7 @@ export class GroupAddEditComponent implements OnInit, OnDestroy {
     private organizationService: OrganizationService,
     private configService: ConfigService,
     private accountService: AccountService,
+    private collectionAdminService: CollectionAdminService,
   ) {
     this.tabIndex = params.initialTab ?? GroupAddEditTabType.Info;
   }
@@ -244,48 +231,61 @@ export class GroupAddEditComponent implements OnInit, OnDestroy {
       this.groupDetails$,
       this.restrictGroupAccess$,
       this.accountService.activeAccount$,
+      this.organization$,
+      this.flexibleCollectionsV1Enabled$,
     ])
       .pipe(takeUntil(this.destroy$))
-      .subscribe(([collections, members, group, restrictGroupAccess, activeAccount]) => {
-        this.collections = collections;
-        this.members = members;
-        this.group = group;
-
-        if (this.group != undefined) {
-          // Must detect changes so that AccessSelector @Inputs() are aware of the latest
-          // collections/members set above, otherwise no selected values will be patched below
-          this.changeDetectorRef.detectChanges();
-
-          this.groupForm.patchValue({
-            name: this.group.name,
-            externalId: this.group.externalId,
-            accessAll: this.group.accessAll,
-            members: this.group.members.map((m) => ({
-              id: m,
-              type: AccessItemType.Member,
-            })),
-            collections: this.group.collections.map((gc) => ({
-              id: gc.id,
-              type: AccessItemType.Collection,
-              permission: convertToPermission(gc),
-            })),
-          });
-        }
-
-        // If the current user is not already in the group and cannot add themselves, remove them from the list
-        if (restrictGroupAccess) {
-          const organizationUserId = this.members.find((m) => m.userId === activeAccount.id).id;
-          const isAlreadyInGroup = this.groupForm.value.members.some(
-            (m) => m.id === organizationUserId,
+      .subscribe(
+        ([
+          collections,
+          members,
+          group,
+          restrictGroupAccess,
+          activeAccount,
+          organization,
+          flexibleCollectionsV1Enabled,
+        ]) => {
+          this.members = members;
+          this.group = group;
+          this.collections = mapToAccessItemViews(
+            collections,
+            organization,
+            flexibleCollectionsV1Enabled,
+            group,
           );
 
-          if (!isAlreadyInGroup) {
-            this.members = this.members.filter((m) => m.id !== organizationUserId);
-          }
-        }
+          if (this.group != undefined) {
+            // Must detect changes so that AccessSelector @Inputs() are aware of the latest
+            // collections/members set above, otherwise no selected values will be patched below
+            this.changeDetectorRef.detectChanges();
 
-        this.loading = false;
-      });
+            this.groupForm.patchValue({
+              name: this.group.name,
+              externalId: this.group.externalId,
+              accessAll: this.group.accessAll,
+              members: this.group.members.map((m) => ({
+                id: m,
+                type: AccessItemType.Member,
+              })),
+              collections: mapToAccessSelections(group, this.collections),
+            });
+          }
+
+          // If the current user is not already in the group and cannot add themselves, remove them from the list
+          if (restrictGroupAccess) {
+            const organizationUserId = this.members.find((m) => m.userId === activeAccount.id).id;
+            const isAlreadyInGroup = this.groupForm.value.members.some(
+              (m) => m.id === organizationUserId,
+            );
+
+            if (!isAlreadyInGroup) {
+              this.members = this.members.filter((m) => m.id !== organizationUserId);
+            }
+          }
+
+          this.loading = false;
+        },
+      );
   }
 
   ngOnDestroy() {
@@ -355,3 +355,46 @@ export class GroupAddEditComponent implements OnInit, OnDestroy {
     this.dialogRef.close(GroupAddEditDialogResultType.Deleted);
   };
 }
+
+/**
+ * Maps the group's current collection access to AccessItemValues to populate the access-selector's FormControl
+ */
+function mapToAccessSelections(group: GroupView, items: AccessItemView[]): AccessItemValue[] {
+  return (
+    group.collections
+      // The FormControl value only represents editable collection access - exclude readonly access selections
+      .filter((selection) => !items.find((item) => item.id == selection.id).readonly)
+      .map((gc) => ({
+        id: gc.id,
+        type: AccessItemType.Collection,
+        permission: convertToPermission(gc),
+      }))
+  );
+}
+
+/**
+ * Maps the organization's collections to AccessItemViews to populate the access-selector's multi-select
+ */
+function mapToAccessItemViews(
+  collections: CollectionAdminView[],
+  organization: Organization,
+  flexibleCollectionsV1Enabled: boolean,
+  group?: GroupView,
+): AccessItemView[] {
+  return (
+    collections
+      .map<AccessItemView>((c) => {
+        const accessSelection = group?.collections.find((access) => access.id == c.id) ?? undefined;
+        return {
+          id: c.id,
+          type: AccessItemType.Collection,
+          labelName: c.name,
+          listName: c.name,
+          readonly: !c.canEditGroupAccess(organization, flexibleCollectionsV1Enabled),
+          readonlyPermission: accessSelection ? convertToPermission(accessSelection) : undefined,
+        };
+      })
+      // Remove any collection views that are not already assigned and that we don't have permissions to assign access to
+      .filter((item) => !item.readonly || group?.collections.some((access) => access.id == item.id))
+  );
+}
diff --git a/apps/web/src/app/vault/core/views/collection-admin.view.ts b/apps/web/src/app/vault/core/views/collection-admin.view.ts
index 369c78636c..2be84b0d24 100644
--- a/apps/web/src/app/vault/core/views/collection-admin.view.ts
+++ b/apps/web/src/app/vault/core/views/collection-admin.view.ts
@@ -51,6 +51,13 @@ export class CollectionAdminView extends CollectionView {
    * Whether the user can modify user access to this collection
    */
   canEditUserAccess(org: Organization, flexibleCollectionsV1Enabled: boolean): boolean {
-    return this.canEdit(org, flexibleCollectionsV1Enabled) || org.canManageUsers;
+    return this.canEdit(org, flexibleCollectionsV1Enabled) || org.permissions.manageUsers;
+  }
+
+  /**
+   * Whether the user can modify group access to this collection
+   */
+  canEditGroupAccess(org: Organization, flexibleCollectionsV1Enabled: boolean): boolean {
+    return this.canEdit(org, flexibleCollectionsV1Enabled) || org.permissions.manageGroups;
   }
 }
diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json
index e37ccaf536..f5fff9a81d 100644
--- a/apps/web/src/locales/en/messages.json
+++ b/apps/web/src/locales/en/messages.json
@@ -6480,6 +6480,9 @@
   "editGroupCollectionsDesc": {
     "message": "Grant access to collections by adding them to this group."
   },
+  "editGroupCollectionsRestrictionsDesc": {
+    "message": "You can only assign collections you manage."
+  },
   "accessAllCollectionsDesc": {
     "message": "Grant access to all current and future collections."
   },