[EC-267] Unassigned collection has disappeared in web vault (#3000)

* feat: add unassigned collection to decrypted collections

* feat: add support for unassigned in individual vault

* fix: dont activate collection when not selected

* fix: remove collection selection completely when pruning

* feat: prune collection selection if selecting my vault

* fix: filter and only show organization ciphers when unassigned collection is selected

* fix: only show unassigned for admins

* feat: add unassigned logic to organizational vault buildFilter

* refactor: move buildFilter to VaultFilterModel

* chore: add buildFilter tests

* fix: bugs in filtering logic

* refactor: use VaultFilter.buildFilter on desktop

* chore: group and reword tests for better readability

* feat: add additional test

* fix: connect unassigned collection to organization

* fix: test by adding missing

* chore: tweak test group naming

* fix: change undefined to null to better reflect real values

apps/desktop/src/app/vault/vault.component.ts

@@ -128,7 +128,7 @@ export class VaultComponent implements OnInit, OnDestroy {
             await this.openGenerator(false);
             break;
           case "syncCompleted":
-            await this.ciphersComponent.reload(this.buildFilter());
+            await this.ciphersComponent.reload(this.activeFilter.buildFilter());
             await this.vaultFilterComponent.reloadCollectionsAndFolders(this.activeFilter);
             await this.vaultFilterComponent.reloadOrganizations();
             break;
@@ -241,7 +241,7 @@ export class VaultComponent implements OnInit, OnDestroy {
         selectedOrganizationId: params.selectedOrganizationId,
         myVaultOnly: params.myVaultOnly ?? false,
       });
-      await this.ciphersComponent.reload(this.buildFilter());
+      await this.ciphersComponent.reload(this.activeFilter.buildFilter());
     });
   }
 
@@ -540,7 +540,10 @@ export class VaultComponent implements OnInit, OnDestroy {
       this.i18nService.t(this.calculateSearchBarLocalizationString(vaultFilter))
     );
     this.activeFilter = vaultFilter;
-    await this.ciphersComponent.reload(this.buildFilter(), vaultFilter.status === "trash");
+    await this.ciphersComponent.reload(
+      this.activeFilter.buildFilter(),
+      vaultFilter.status === "trash"
+    );
     this.go();
   }
 
@@ -570,40 +573,6 @@ export class VaultComponent implements OnInit, OnDestroy {
     return "searchVault";
   }
 
-  private buildFilter(): (cipher: CipherView) => boolean {
-    return (cipher) => {
-      let cipherPassesFilter = true;
-      if (this.activeFilter.status === "favorites" && cipherPassesFilter) {
-        cipherPassesFilter = cipher.favorite;
-      }
-      if (this.activeFilter.status === "trash" && cipherPassesFilter) {
-        cipherPassesFilter = cipher.isDeleted;
-      }
-      if (this.activeFilter.cipherType != null && cipherPassesFilter) {
-        cipherPassesFilter = cipher.type === this.activeFilter.cipherType;
-      }
-      if (
-        this.activeFilter.selectedFolder &&
-        this.activeFilter.selectedFolderId != "none" &&
-        cipherPassesFilter
-      ) {
-        cipherPassesFilter = cipher.folderId === this.activeFilter.selectedFolderId;
-      }
-      if (this.activeFilter.selectedCollectionId != null && cipherPassesFilter) {
-        cipherPassesFilter =
-          cipher.collectionIds != null &&
-          cipher.collectionIds.indexOf(this.activeFilter.selectedCollectionId) > -1;
-      }
-      if (this.activeFilter.selectedOrganizationId != null && cipherPassesFilter) {
-        cipherPassesFilter = cipher.organizationId === this.activeFilter.selectedOrganizationId;
-      }
-      if (this.activeFilter.myVaultOnly && cipherPassesFilter) {
-        cipherPassesFilter = cipher.organizationId === null;
-      }
-      return cipherPassesFilter;
-    };
-  }
-
   async openGenerator(comingFromAddEdit: boolean, passwordType = true) {
     if (this.modal != null) {
       this.modal.close();

apps/web/src/app/modules/vault-filter/components/collection-filter.component.html

@@ -23,7 +23,7 @@
       <li
         *ngFor="let c of collections"
         [ngClass]="{
-          active: c.node.id === activeFilter.selectedCollectionId
+          active: c.node.id === activeFilter.selectedCollectionId && activeFilter.selectedCollection
         }"
         class="filter-option"
       >

apps/web/src/app/modules/vault-filter/vault-filter.service.ts

@@ -7,6 +7,7 @@ import { ApiService } from "@bitwarden/common/abstractions/api.service";
 import { CipherService } from "@bitwarden/common/abstractions/cipher.service";
 import { CollectionService } from "@bitwarden/common/abstractions/collection.service";
 import { FolderService } from "@bitwarden/common/abstractions/folder.service";
+import { I18nService } from "@bitwarden/common/abstractions/i18n.service";
 import { OrganizationService } from "@bitwarden/common/abstractions/organization.service";
 import { PolicyService } from "@bitwarden/common/abstractions/policy.service";
 import { StateService } from "@bitwarden/common/abstractions/state.service";
@@ -27,6 +28,7 @@ export class VaultFilterService extends BaseVaultFilterService {
     cipherService: CipherService,
     collectionService: CollectionService,
     policyService: PolicyService,
+    private i18nService: I18nService,
     protected apiService: ApiService
   ) {
     super(
@@ -69,6 +71,11 @@ export class VaultFilterService extends BaseVaultFilterService {
       result = await this.collectionService.decryptMany(collectionDomains);
     }
 
+    const noneCollection = new CollectionView();
+    noneCollection.name = this.i18nService.t("unassigned");
+    noneCollection.organizationId = organizationId;
+    result.push(noneCollection);
+
     const nestedCollections = await this.collectionService.getAllNested(result);
     return new DynamicTreeNode<CollectionView>({
       fullList: result,

apps/web/src/app/modules/vault/modules/individual-vault/individual-vault.component.ts

@@ -173,7 +173,10 @@ export class IndividualVaultComponent implements OnInit, OnDestroy {
   async applyVaultFilter(vaultFilter: VaultFilter) {
     this.ciphersComponent.showAddNew = vaultFilter.status !== "trash";
     this.activeFilter = vaultFilter;
-    await this.ciphersComponent.reload(this.buildFilter(), vaultFilter.status === "trash");
+    await this.ciphersComponent.reload(
+      this.activeFilter.buildFilter(),
+      vaultFilter.status === "trash"
+    );
     this.filterComponent.searchPlaceholder = this.vaultService.calculateSearchBarLocalizationString(
       this.activeFilter
     );
@@ -196,40 +199,6 @@ export class IndividualVaultComponent implements OnInit, OnDestroy {
     this.ciphersComponent.search(200);
   }
 
-  private buildFilter(): (cipher: CipherView) => boolean {
-    return (cipher) => {
-      let cipherPassesFilter = true;
-      if (this.activeFilter.status === "favorites" && cipherPassesFilter) {
-        cipherPassesFilter = cipher.favorite;
-      }
-      if (this.activeFilter.status === "trash" && cipherPassesFilter) {
-        cipherPassesFilter = cipher.isDeleted;
-      }
-      if (this.activeFilter.cipherType != null && cipherPassesFilter) {
-        cipherPassesFilter = cipher.type === this.activeFilter.cipherType;
-      }
-      if (
-        this.activeFilter.selectedFolder &&
-        this.activeFilter.selectedFolderId != "none" &&
-        cipherPassesFilter
-      ) {
-        cipherPassesFilter = cipher.folderId === this.activeFilter.selectedFolderId;
-      }
-      if (this.activeFilter.selectedCollectionId != null && cipherPassesFilter) {
-        cipherPassesFilter =
-          cipher.collectionIds != null &&
-          cipher.collectionIds.indexOf(this.activeFilter.selectedCollectionId) > -1;
-      }
-      if (this.activeFilter.selectedOrganizationId != null && cipherPassesFilter) {
-        cipherPassesFilter = cipher.organizationId === this.activeFilter.selectedOrganizationId;
-      }
-      if (this.activeFilter.myVaultOnly && cipherPassesFilter) {
-        cipherPassesFilter = cipher.organizationId === null;
-      }
-      return cipherPassesFilter;
-    };
-  }
-
   async editCipherAttachments(cipher: CipherView) {
     const canAccessPremium = await this.stateService.getCanAccessPremium();
     if (cipher.organizationId == null && !canAccessPremium) {

apps/web/src/app/modules/vault/modules/organization-vault/organization-vault.component.ts

@@ -162,46 +162,15 @@ export class OrganizationVaultComponent implements OnInit, OnDestroy {
   async applyVaultFilter(vaultFilter: VaultFilter) {
     this.ciphersComponent.showAddNew = vaultFilter.status !== "trash";
     this.activeFilter = vaultFilter;
-    await this.ciphersComponent.reload(this.buildFilter(), vaultFilter.status === "trash");
+    await this.ciphersComponent.reload(
+      this.activeFilter.buildFilter(),
+      vaultFilter.status === "trash"
+    );
     this.vaultFilterComponent.searchPlaceholder =
       this.vaultService.calculateSearchBarLocalizationString(this.activeFilter);
     this.go();
   }
 
-  private buildFilter(): (cipher: CipherView) => boolean {
-    return (cipher) => {
-      let cipherPassesFilter = true;
-      if (this.activeFilter.status === "favorites" && cipherPassesFilter) {
-        cipherPassesFilter = cipher.favorite;
-      }
-      if (this.deleted && cipherPassesFilter) {
-        cipherPassesFilter = cipher.isDeleted;
-      }
-      if (this.activeFilter.cipherType != null && cipherPassesFilter) {
-        cipherPassesFilter = cipher.type === this.activeFilter.cipherType;
-      }
-      if (
-        this.activeFilter.selectedFolder != null &&
-        this.activeFilter.selectedFolderId != "none" &&
-        cipherPassesFilter
-      ) {
-        cipherPassesFilter = cipher.folderId === this.activeFilter.selectedFolderId;
-      }
-      if (this.activeFilter.selectedCollectionId != null && cipherPassesFilter) {
-        cipherPassesFilter =
-          cipher.collectionIds != null &&
-          cipher.collectionIds.indexOf(this.activeFilter.selectedCollectionId) > -1;
-      }
-      if (this.activeFilter.selectedOrganizationId != null && cipherPassesFilter) {
-        cipherPassesFilter = cipher.organizationId === this.activeFilter.selectedOrganizationId;
-      }
-      if (this.activeFilter.myVaultOnly && cipherPassesFilter) {
-        cipherPassesFilter = cipher.organizationId === null;
-      }
-      return cipherPassesFilter;
-    };
-  }
-
   filterSearchText(searchText: string) {
     this.ciphersComponent.searchText = searchText;
     this.ciphersComponent.search(200);

apps/web/src/app/modules/vault/vault.service.ts

@@ -14,7 +14,7 @@ export class VaultService {
     if (vaultFilter.selectedFolderId != null && vaultFilter.selectedFolderId != "none") {
       return "searchFolder";
     }
-    if (vaultFilter.selectedCollectionId != null) {
+    if (vaultFilter.selectedCollection) {
       return "searchCollection";
     }
     if (vaultFilter.selectedOrganizationId != null) {

libs/angular/src/modules/vault-filter/components/collection-filter.component.ts

@@ -41,6 +41,7 @@ export class CollectionFilterComponent {
 
   applyFilter(collection: CollectionView) {
     this.activeFilter.resetFilter();
+    this.activeFilter.selectedCollection = true;
     this.activeFilter.selectedCollectionId = collection.id;
     this.onFilterChange.emit(this.activeFilter);
   }

libs/angular/src/modules/vault-filter/models/vault-filter.model.spec.ts

@@ -0,0 +1,237 @@
+import { CipherType } from "@bitwarden/common/enums/cipherType";
+import { CipherView } from "@bitwarden/common/models/view/cipherView";
+
+import { VaultFilter } from "./vault-filter.model";
+
+describe("VaultFilter", () => {
+  describe("filterFunction", () => {
+    describe("generic cipher", () => {
+      it("should return true when not filtering for anything specific", () => {
+        const cipher = createCipher();
+        const filterFunction = createFilterFunction({ status: "all" });
+
+        const result = filterFunction(cipher);
+
+        expect(result).toBe(true);
+      });
+    });
+
+    describe("given a favorite cipher", () => {
+      const cipher = createCipher({ favorite: true });
+
+      it("should return true when filtering for favorites", () => {
+        const filterFunction = createFilterFunction({ status: "favorites" });
+
+        const result = filterFunction(cipher);
+
+        expect(result).toBe(true);
+      });
+
+      it("should return false when filtering for trash", () => {
+        const filterFunction = createFilterFunction({ status: "trash" });
+
+        const result = filterFunction(cipher);
+
+        expect(result).toBe(false);
+      });
+    });
+
+    describe("given a deleted cipher", () => {
+      const cipher = createCipher({ deletedDate: new Date() });
+
+      it("should return true when filtering for trash", () => {
+        const filterFunction = createFilterFunction({ status: "trash" });
+
+        const result = filterFunction(cipher);
+
+        expect(result).toBe(true);
+      });
+
+      it("should return false when filtering for favorites", () => {
+        const filterFunction = createFilterFunction({ status: "favorites" });
+
+        const result = filterFunction(cipher);
+
+        expect(result).toBe(false);
+      });
+    });
+
+    describe("given a cipher with type", () => {
+      it("should return true when filter matches cipher type", () => {
+        const cipher = createCipher({ type: CipherType.Identity });
+        const filterFunction = createFilterFunction({ cipherType: CipherType.Identity });
+
+        const result = filterFunction(cipher);
+
+        expect(result).toBe(true);
+      });
+
+      it("should return false when filter does not match cipher type", () => {
+        const cipher = createCipher({ type: CipherType.Card });
+        const filterFunction = createFilterFunction({ cipherType: CipherType.Identity });
+
+        const result = filterFunction(cipher);
+
+        expect(result).toBe(false);
+      });
+    });
+
+    describe("given a cipher with folder id", () => {
+      it("should return true when filter matches folder id", () => {
+        const cipher = createCipher({ folderId: "folderId" });
+        const filterFunction = createFilterFunction({
+          selectedFolder: true,
+          selectedFolderId: "folderId",
+        });
+
+        const result = filterFunction(cipher);
+
+        expect(result).toBe(true);
+      });
+
+      it("should return false when filter does not match folder id", () => {
+        const cipher = createCipher({ folderId: "folderId" });
+        const filterFunction = createFilterFunction({
+          selectedFolder: true,
+          selectedFolderId: "anotherFolderId",
+        });
+
+        const result = filterFunction(cipher);
+
+        expect(result).toBe(false);
+      });
+    });
+
+    describe("given a cipher without folder", () => {
+      const cipher = createCipher({ folderId: null });
+
+      it("should return true when filtering on unassigned folder", () => {
+        const filterFunction = createFilterFunction({
+          selectedFolder: true,
+          selectedFolderId: null,
+        });
+
+        const result = filterFunction(cipher);
+
+        expect(result).toBe(true);
+      });
+    });
+
+    describe("given an organizational cipher (with organization and collections)", () => {
+      const cipher = createCipher({
+        organizationId: "organizationId",
+        collectionIds: ["collectionId", "anotherId"],
+      });
+
+      it("should return true when filter matches collection id", () => {
+        const filterFunction = createFilterFunction({
+          selectedCollection: true,
+          selectedCollectionId: "collectionId",
+        });
+
+        const result = filterFunction(cipher);
+
+        expect(result).toBe(true);
+      });
+
+      it("should return false when filter does not match collection id", () => {
+        const filterFunction = createFilterFunction({
+          selectedCollection: true,
+          selectedCollectionId: "nonMatchingId",
+        });
+
+        const result = filterFunction(cipher);
+
+        expect(result).toBe(false);
+      });
+
+      it("should return false when filter does not match organization id", () => {
+        const filterFunction = createFilterFunction({
+          selectedOrganizationId: "anotherOrganizationId",
+        });
+
+        const result = filterFunction(cipher);
+
+        expect(result).toBe(false);
+      });
+
+      it("should return false when filtering for my vault only", () => {
+        const filterFunction = createFilterFunction({
+          myVaultOnly: true,
+        });
+
+        const result = filterFunction(cipher);
+
+        expect(result).toBe(false);
+      });
+    });
+
+    describe("given an unassigned organizational cipher (with organization, without collection)", () => {
+      const cipher = createCipher({ organizationId: "organizationId", collectionIds: [] });
+
+      it("should return true when filtering for unassigned collection", () => {
+        const filterFunction = createFilterFunction({
+          selectedCollection: true,
+          selectedCollectionId: null,
+        });
+
+        const result = filterFunction(cipher);
+
+        expect(result).toBe(true);
+      });
+
+      it("should return true when filter matches organization id", () => {
+        const filterFunction = createFilterFunction({
+          selectedOrganizationId: "organizationId",
+        });
+
+        const result = filterFunction(cipher);
+
+        expect(result).toBe(true);
+      });
+    });
+
+    describe("given an individual cipher (without organization or collection)", () => {
+      const cipher = createCipher({ organizationId: null, collectionIds: [] });
+
+      it("should return false when filtering for unassigned collection", () => {
+        const filterFunction = createFilterFunction({
+          selectedCollection: true,
+          selectedCollectionId: null,
+        });
+
+        const result = filterFunction(cipher);
+
+        expect(result).toBe(false);
+      });
+
+      it("should return true when filtering for my vault only", () => {
+        const cipher = createCipher({ organizationId: null });
+        const filterFunction = createFilterFunction({
+          myVaultOnly: true,
+        });
+
+        const result = filterFunction(cipher);
+
+        expect(result).toBe(true);
+      });
+    });
+  });
+});
+
+function createFilterFunction(options: Partial<VaultFilter> = {}) {
+  return new VaultFilter(options).buildFilter();
+}
+
+function createCipher(options: Partial<CipherView> = {}) {
+  const cipher = new CipherView();
+
+  cipher.favorite = options.favorite ?? false;
+  cipher.deletedDate = options.deletedDate;
+  cipher.type = options.type;
+  cipher.folderId = options.folderId;
+  cipher.collectionIds = options.collectionIds;
+  cipher.organizationId = options.organizationId;
+
+  return cipher;
+}

libs/angular/src/modules/vault-filter/models/vault-filter.model.ts

@@ -1,9 +1,13 @@
 import { CipherType } from "@bitwarden/common/enums/cipherType";
+import { CipherView } from "@bitwarden/common/models/view/cipherView";
 
 import { CipherStatus } from "./cipher-status.model";
 
+export type VaultFilterFunction = (cipher: CipherView) => boolean;
+
 export class VaultFilter {
   cipherType?: CipherType;
+  selectedCollection = false; // This is needed because of how the "Unassigned" collection works. It has a null id.
   selectedCollectionId?: string;
   status?: CipherStatus;
   selectedFolder = false; // This is needed because of how the "No Folder" folder works. It has a null id.
@@ -19,6 +23,7 @@ export class VaultFilter {
   resetFilter() {
     this.cipherType = null;
     this.status = null;
+    this.selectedCollection = false;
     this.selectedCollectionId = null;
     this.selectedFolder = false;
     this.selectedFolderId = null;
@@ -29,4 +34,41 @@ export class VaultFilter {
     this.selectedOrganizationId = null;
     this.resetFilter();
   }
+
+  buildFilter(): VaultFilterFunction {
+    return (cipher) => {
+      let cipherPassesFilter = true;
+      if (this.status === "favorites" && cipherPassesFilter) {
+        cipherPassesFilter = cipher.favorite;
+      }
+      if (this.status === "trash" && cipherPassesFilter) {
+        cipherPassesFilter = cipher.isDeleted;
+      }
+      if (this.cipherType != null && cipherPassesFilter) {
+        cipherPassesFilter = cipher.type === this.cipherType;
+      }
+      if (this.selectedFolder && this.selectedFolderId == null && cipherPassesFilter) {
+        cipherPassesFilter = cipher.folderId == null;
+      }
+      if (this.selectedFolder && this.selectedFolderId != null && cipherPassesFilter) {
+        cipherPassesFilter = cipher.folderId === this.selectedFolderId;
+      }
+      if (this.selectedCollection && this.selectedCollectionId == null && cipherPassesFilter) {
+        cipherPassesFilter =
+          cipher.organizationId != null &&
+          (cipher.collectionIds == null || cipher.collectionIds.length === 0);
+      }
+      if (this.selectedCollection && this.selectedCollectionId != null && cipherPassesFilter) {
+        cipherPassesFilter =
+          cipher.collectionIds != null && cipher.collectionIds.includes(this.selectedCollectionId);
+      }
+      if (this.selectedOrganizationId != null && cipherPassesFilter) {
+        cipherPassesFilter = cipher.organizationId === this.selectedOrganizationId;
+      }
+      if (this.myVaultOnly && cipherPassesFilter) {
+        cipherPassesFilter = cipher.organizationId === null;
+      }
+      return cipherPassesFilter;
+    };
+  }
 }

libs/angular/src/modules/vault-filter/vault-filter.component.ts

@@ -111,9 +111,12 @@ export class VaultFilterComponent implements OnInit {
 
   protected pruneInvalidCollectionSelection(filter: VaultFilter): VaultFilter {
     if (
-      filter.selectedCollectionId != null &&
+      filter.myVaultOnly ||
-      !this.collections?.hasId(filter.selectedCollectionId)
+      (filter.selectedCollection &&
+        filter.selectedCollectionId != null &&
+        !this.collections?.hasId(filter.selectedCollectionId))
     ) {
+      filter.selectedCollection = false;
       filter.selectedCollectionId = null;
     }
     return filter;

libs/common/src/services/collection.service.ts

@@ -86,6 +86,7 @@ export class CollectionService implements CollectionServiceAbstraction {
 
     const collections = await this.getAll();
     decryptedCollections = await this.decryptMany(collections);
+
     await this.stateService.setDecryptedCollections(decryptedCollections);
     return decryptedCollections;
   }