[PM-8004] [AC-2603] [AC-2616] [AC-2621] [AC-2622] Unmanaged collection fixes (#9301)

* [AC-2603] Add unmanaged property to CollectionAdminView and response models * [AC-2603] Cleanup CollectionViews - Remove getters that have been replaced with Unmanaged property - Remove AddAccess that is also being replaced - Add canEditUnmanagedCollections() helper to organization * [AC-2603] Replace old AddAccess logic with Unmanaged flag * [AC-2603] Fix failing test * [AC-2603] Ensure Add Access badge/toggle only shows when V1 flag is enabled * [AC-2603] Undo change to canEditUserAccess and canEditGroupAccess Custom users should not get access to an unmanaged collection with only Manage Groups and Manage User permissions. That is still reserved for admin/owners and EditAnyCollection custom users.
2024-05-22 11:58:04 -07:00 · 2024-05-22 11:58:04 -07:00 · 0b950080ca
parent d6f78f2225
commit 0b950080ca
11 changed files with 66 additions and 143 deletions
--- a/apps/web/src/app/vault/components/vault-items/vault-collection-row.component.html
+++ b/apps/web/src/app/vault/components/vault-items/vault-collection-row.component.html
@ -30,12 +30,7 @@
  >
    <span class="tw-truncate tw-mr-1">{{ collection.name }}</span>
    <div>
-      <span
-        *ngIf="collection.addAccess && collection.id !== Unassigned"
-        bitBadge
-        variant="warning"
-        >{{ "addAccess" | i18n }}</span
-      >
+      <span *ngIf="showAddAccess" bitBadge variant="warning">{{ "addAccess" | i18n }}</span>
    </div>
  </button>
 </td>
--- a/apps/web/src/app/vault/components/vault-items/vault-collection-row.component.ts
+++ b/apps/web/src/app/vault/components/vault-items/vault-collection-row.component.ts
@ -56,6 +56,28 @@ export class VaultCollectionRowComponent {
    return this.organizations.find((o) => o.id === this.collection.organizationId);
  }

+  get showAddAccess() {
+    if (!this.flexibleCollectionsV1Enabled) {
+      return false;
+    }
+
+    if (this.collection.id == Unassigned) {
+      return false;
+    }
+
+    // Only show AddAccess when viewing the Org vault (implied by CollectionAdminView)
+    if (this.collection instanceof CollectionAdminView) {
+      // Only show AddAccess if unmanaged and allowAdminAccessToAllCollectionItems is disabled
+      return (
+        !this.organization.allowAdminAccessToAllCollectionItems &&
+        this.collection.unmanaged &&
+        this.organization.canEditUnmanagedCollections()
+      );
+    }
+
+    return false;
+  }
+
  get permissionText() {
    if (
      this.collection.id == Unassigned &&
--- a/apps/web/src/app/vault/components/vault-items/vault-items.component.ts
+++ b/apps/web/src/app/vault/components/vault-items/vault-items.component.ts
@ -1,7 +1,6 @@
 import { SelectionModel } from "@angular/cdk/collections";
 import { Component, EventEmitter, Input, Output } from "@angular/core";

-import { OrganizationUserType } from "@bitwarden/common/admin-console/enums";
 import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
 import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
 import { CollectionView } from "@bitwarden/common/vault/models/view/collection.view";
@ -106,27 +105,6 @@ export class VaultItemsComponent {

    const organization = this.allOrganizations.find((o) => o.id === collection.organizationId);

-    if (this.flexibleCollectionsV1Enabled) {
-      //Custom user without edit access should not see the Edit option unless that user has "Can Manage" access to a collection
-      if (
-        !collection.manage &&
-        organization?.type === OrganizationUserType.Custom &&
-        !organization?.permissions.editAnyCollection
-      ) {
-        return false;
-      }
-      //Owner/Admin and Custom Users with Edit can see Edit and Access of Orphaned Collections
-      if (
-        collection.addAccess &&
-        collection.id !== Unassigned &&
-        ((organization?.type === OrganizationUserType.Custom &&
-          organization?.permissions.editAnyCollection) ||
-          organization.isAdmin ||
-          organization.isOwner)
-      ) {
-        return true;
-      }
-    }
    return collection.canEdit(organization, this.flexibleCollectionsV1Enabled);
  }

@ -138,31 +116,6 @@ export class VaultItemsComponent {

    const organization = this.allOrganizations.find((o) => o.id === collection.organizationId);

-    if (this.flexibleCollectionsV1Enabled) {
-      //Custom user with only edit access should not see the Delete button for orphaned collections
-      if (
-        collection.addAccess &&
-        organization?.type === OrganizationUserType.Custom &&
-        !organization?.permissions.deleteAnyCollection &&
-        organization?.permissions.editAnyCollection
-      ) {
-        return false;
-      }
-
-      // Owner/Admin with no access to a collection will not see Delete
-      if (
-        !collection.assigned &&
-        !collection.addAccess &&
-        (organization.isAdmin || organization.isOwner) &&
-        !(
-          organization?.type === OrganizationUserType.Custom &&
-          organization?.permissions.deleteAnyCollection
-        )
-      ) {
-        return false;
-      }
-    }
-
    return collection.canDelete(organization, this.flexibleCollectionsV1Enabled);
  }

--- a/apps/web/src/app/vault/core/collection-admin.service.ts
+++ b/apps/web/src/app/vault/core/collection-admin.service.ts
@ -127,6 +127,7 @@ export class CollectionAdminService {
        view.readOnly = c.readOnly;
        view.hidePasswords = c.hidePasswords;
        view.manage = c.manage;
+        view.unmanaged = c.unmanaged;
      }

      return view;
--- a/apps/web/src/app/vault/core/views/collection-admin.view.ts
+++ b/apps/web/src/app/vault/core/views/collection-admin.view.ts
@ -1,4 +1,3 @@
-import { OrganizationUserUserDetailsResponse } from "@bitwarden/common/admin-console/abstractions/organization-user/responses";
 import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
 import { CollectionAccessDetailsResponse } from "@bitwarden/common/src/vault/models/response/collection.response";
 import { CollectionView } from "@bitwarden/common/vault/models/view/collection.view";
@ -9,7 +8,12 @@ import { Unassigned } from "../../individual-vault/vault-filter/shared/models/ro
 export class CollectionAdminView extends CollectionView {
  groups: CollectionAccessSelectionView[] = [];
  users: CollectionAccessSelectionView[] = [];
-  addAccess: boolean;
+
+  /**
+   * Flag indicating the collection has no active user or group assigned to it with CanManage permissions
+   * In this case, the collection can be managed by admins/owners or custom users with appropriate permissions
+   */
+  unmanaged: boolean;

  /**
   * Flag indicating the user has been explicitly assigned to this Collection
@ -34,39 +38,13 @@ export class CollectionAdminView extends CollectionView {
    this.assigned = response.assigned;
  }

-  groupsCanManage() {
-    if (this.groups.length === 0) {
-      return this.groups;
-    }
-
-    const returnedGroups = this.groups.filter((group) => {
-      if (group.manage) {
-        return group;
-      }
-    });
-    return returnedGroups;
-  }
-
-  usersCanManage(revokedUsers: OrganizationUserUserDetailsResponse[]) {
-    if (this.users.length === 0) {
-      return this.users;
-    }
-
-    const returnedUsers = this.users.filter((user) => {
-      const isRevoked = revokedUsers.some((revoked) => revoked.id === user.id);
-      if (user.manage && !isRevoked) {
-        return user;
-      }
-    });
-    return returnedUsers;
-  }
-
  /**
   * Returns true if the user can edit a collection (including user and group access) from the Admin Console.
   */
  override canEdit(org: Organization, flexibleCollectionsV1Enabled: boolean): boolean {
    return (
      org?.canEditAnyCollection(flexibleCollectionsV1Enabled) ||
+      (flexibleCollectionsV1Enabled && this.unmanaged && org?.canEditUnmanagedCollections()) ||
      super.canEdit(org, flexibleCollectionsV1Enabled)
    );
  }
@ -125,6 +103,10 @@ export class CollectionAdminView extends CollectionView {
    return this.manage || org?.isAdmin || org?.permissions.editAnyCollection;
  }

+  /**
+   * True if this collection represents the pseudo "Unassigned" collection
+   * This is different from the "unmanaged" flag, which indicates that no users or groups have access to the collection
+   */
  get isUnassignedCollection() {
    return this.id === Unassigned;
  }
--- a/apps/web/src/app/vault/org-vault/vault.component.ts
+++ b/apps/web/src/app/vault/org-vault/vault.component.ts
@ -38,7 +38,6 @@ import { SearchService } from "@bitwarden/common/abstractions/search.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 { OrganizationUserUserDetailsResponse } from "@bitwarden/common/admin-console/abstractions/organization-user/responses";
-import { OrganizationUserType } from "@bitwarden/common/admin-console/enums";
 import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
 import { EventType } from "@bitwarden/common/enums";
 import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
@ -433,13 +432,13 @@ export class VaultComponent implements OnInit, OnDestroy {
        this.showAddAccessToggle = false;
        let collectionsToReturn = [];
        if (filter.collectionId === undefined || filter.collectionId === All) {
-          collectionsToReturn = await this.addAccessCollectionsMap(collections);
+          collectionsToReturn = collections.map((c) => c.node);
        } else {
          const selectedCollection = ServiceUtils.getTreeNodeObjectFromList(
            collections,
            filter.collectionId,
          );
-          collectionsToReturn = await this.addAccessCollectionsMap(selectedCollection?.children);
+          collectionsToReturn = selectedCollection?.children.map((c) => c.node) ?? [];
        }

        if (await this.searchService.isSearchable(searchText)) {
@ -451,8 +450,15 @@ export class VaultComponent implements OnInit, OnDestroy {
          );
        }

+        // Add access toggle is only shown if allowAdminAccessToAllCollectionItems is false and there are unmanaged collections the user can edit
+        this.showAddAccessToggle =
+          this.flexibleCollectionsV1Enabled &&
+          !this.organization.allowAdminAccessToAllCollectionItems &&
+          this.organization.canEditUnmanagedCollections() &&
+          collectionsToReturn.some((c) => c.unmanaged);
+
        if (addAccessStatus === 1 && this.showAddAccessToggle) {
-          collectionsToReturn = collectionsToReturn.filter((c: any) => c.addAccess);
+          collectionsToReturn = collectionsToReturn.filter((c) => c.unmanaged);
        }
        return collectionsToReturn;
      }),
@ -663,57 +669,7 @@ export class VaultComponent implements OnInit, OnDestroy {
      );
  }

-  // Update the list of collections to see if any collection is orphaned
-  // and will receive the addAccess badge / be filterable by the user
-  async addAccessCollectionsMap(collections: TreeNode<CollectionAdminView>[]) {
-    let mappedCollections;
-    const { type, allowAdminAccessToAllCollectionItems, permissions } = this.organization;
-
-    const canEditCiphersCheck =
-      this._flexibleCollectionsV1FlagEnabled &&
-      !this.organization.canEditAllCiphers(
-        this._flexibleCollectionsV1FlagEnabled,
-        this.restrictProviderAccessEnabled,
-      );
-
-    // This custom type check will show addAccess badge for
-    // Custom users with canEdit access AND owner/admin manage access setting is OFF
-    const customUserCheck =
-      this._flexibleCollectionsV1FlagEnabled &&
-      !allowAdminAccessToAllCollectionItems &&
-      type === OrganizationUserType.Custom &&
-      permissions.editAnyCollection;
-
-    // If Custom user has Delete Only access they will not see Add Access toggle
-    const customUserOnlyDelete =
-      this.flexibleCollectionsV1Enabled &&
-      type === OrganizationUserType.Custom &&
-      permissions.deleteAnyCollection &&
-      !permissions.editAnyCollection;
-
-    if (!customUserOnlyDelete && (canEditCiphersCheck || customUserCheck)) {
-      mappedCollections = collections.map((c: TreeNode<CollectionAdminView>) => {
-        const groupsCanManage = c.node.groupsCanManage();
-        const usersCanManage = c.node.usersCanManage(this.orgRevokedUsers);
-        if (
-          groupsCanManage.length === 0 &&
-          usersCanManage.length === 0 &&
-          c.node.id !== Unassigned
-        ) {
-          c.node.addAccess = true;
-          this.showAddAccessToggle = true;
-        } else {
-          c.node.addAccess = false;
-        }
-        return c.node;
-      });
-    } else {
-      mappedCollections = collections.map((c: TreeNode<CollectionAdminView>) => c.node);
-    }
-    return mappedCollections;
-  }
-
-  addAccessToggle(e: any) {
+  addAccessToggle(e: AddAccessStatusType) {
    this.addAccessStatus$.next(e);
  }

@ -758,9 +714,17 @@ export class VaultComponent implements OnInit, OnDestroy {
      } else if (event.type === "copyField") {
        await this.copy(event.item, event.field);
      } else if (event.type === "editCollection") {
-        await this.editCollection(event.item, CollectionDialogTabType.Info, event.readonly);
+        await this.editCollection(
+          event.item as CollectionAdminView,
+          CollectionDialogTabType.Info,
+          event.readonly,
+        );
      } else if (event.type === "viewCollectionAccess") {
-        await this.editCollection(event.item, CollectionDialogTabType.Access, event.readonly);
+        await this.editCollection(
+          event.item as CollectionAdminView,
+          CollectionDialogTabType.Access,
+          event.readonly,
+        );
      } else if (event.type === "bulkEditCollectionAccess") {
        await this.bulkEditCollectionAccess(event.items, this.organization);
      } else if (event.type === "assignToCollections") {
@ -1273,7 +1237,7 @@ export class VaultComponent implements OnInit, OnDestroy {
  }

  async editCollection(
-    c: CollectionView,
+    c: CollectionAdminView,
    tab: CollectionDialogTabType,
    readonly: boolean,
  ): Promise<void> {
@ -1283,7 +1247,7 @@ export class VaultComponent implements OnInit, OnDestroy {
        organizationId: this.organization?.id,
        initialTab: tab,
        readonly: readonly,
-        isAddAccessCollection: c.addAccess,
+        isAddAccessCollection: c.unmanaged,
        limitNestedCollections: !this.organization.canEditAnyCollection(
          this.flexibleCollectionsV1Enabled,
        ),
--- a/apps/web/src/app/vault/utils/collection-utils.ts
+++ b/apps/web/src/app/vault/utils/collection-utils.ts
@ -44,6 +44,7 @@ function cloneCollection(
    cloned.groups = [...collection.groups];
    cloned.users = [...collection.users];
    cloned.assigned = collection.assigned;
+    cloned.unmanaged = collection.unmanaged;
  } else {
    cloned = new CollectionView();
  }
--- a/libs/common/src/admin-console/models/domain/organization.ts
+++ b/libs/common/src/admin-console/models/domain/organization.ts
@ -203,6 +203,11 @@ export class Organization {
    );
  }

+  canEditUnmanagedCollections() {
+    // Any admin or custom user with editAnyCollection permission can edit unmanaged collections
+    return this.isAdmin || this.permissions.editAnyCollection;
+  }
+
  canEditUnassignedCiphers(restrictProviderAccessFlagEnabled: boolean) {
    if (this.isProviderUser) {
      return !restrictProviderAccessFlagEnabled;
--- a/libs/common/src/vault/models/domain/collection.spec.ts
+++ b/libs/common/src/vault/models/domain/collection.spec.ts
@ -64,7 +64,6 @@ describe("Collection", () => {
    const view = await collection.decrypt(key);

    expect(view).toEqual({
-      addAccess: false,
      externalId: "extId",
      hidePasswords: false,
      id: "id",
--- a/libs/common/src/vault/models/response/collection.response.ts
+++ b/libs/common/src/vault/models/response/collection.response.ts
@ -42,10 +42,12 @@ export class CollectionDetailsResponse extends CollectionResponse {
 export class CollectionAccessDetailsResponse extends CollectionDetailsResponse {
  groups: SelectionReadOnlyResponse[] = [];
  users: SelectionReadOnlyResponse[] = [];
+  unmanaged: boolean;

  constructor(response: any) {
    super(response);
    this.assigned = this.getResponseProperty("Assigned") || false;
+    this.unmanaged = this.getResponseProperty("Unmanaged") || false;

    const groups = this.getResponseProperty("Groups");
    if (groups != null) {
--- a/libs/common/src/vault/models/view/collection.view.ts
+++ b/libs/common/src/vault/models/view/collection.view.ts
@ -17,7 +17,6 @@ export class CollectionView implements View, ITreeNodeObject {
  readOnly: boolean = null;
  hidePasswords: boolean = null;
  manage: boolean = null;
-  addAccess: boolean = false;
  assigned: boolean = null;

  constructor(c?: Collection | CollectionAccessDetailsResponse) {