[PM-11869] Adjust CollectionService to be reactive to keys being available (#11144)

libs/common/src/vault/abstractions/collection.service.ts

@@ -1,6 +1,7 @@
 import { Observable } from "rxjs";
 
-import { CollectionId, UserId } from "../../types/guid";
+import { CollectionId, OrganizationId, UserId } from "../../types/guid";
+import { OrgKey } from "../../types/key";
 import { CollectionData } from "../models/data/collection.data";
 import { Collection } from "../models/domain/collection";
 import { TreeNode } from "../models/domain/tree-node";
@@ -13,9 +14,13 @@ export abstract class CollectionService {
   encrypt: (model: CollectionView) => Promise<Collection>;
   decryptedCollectionViews$: (ids: CollectionId[]) => Observable<CollectionView[]>;
   /**
-   * @deprecated This method will soon be made private, use `decryptedCollectionViews$` instead.
+   * @deprecated This method will soon be made private
+   * See PM-12375
    */
-  decryptMany: (collections: Collection[]) => Promise<CollectionView[]>;
+  decryptMany: (
+    collections: Collection[],
+    orgKeys?: Record<OrganizationId, OrgKey>,
+  ) => Promise<CollectionView[]>;
   get: (id: string) => Promise<Collection>;
   getAll: () => Promise<Collection[]>;
   getAllDecrypted: () => Promise<CollectionView[]>;

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

@@ -0,0 +1,135 @@
+import { mock } from "jest-mock-extended";
+import { firstValueFrom, of } from "rxjs";
+
+import {
+  FakeStateProvider,
+  makeEncString,
+  makeSymmetricCryptoKey,
+  mockAccountServiceWith,
+} from "../../../spec";
+import { CryptoService } from "../../platform/abstractions/crypto.service";
+import { EncryptService } from "../../platform/abstractions/encrypt.service";
+import { I18nService } from "../../platform/abstractions/i18n.service";
+import { Utils } from "../../platform/misc/utils";
+import { EncString } from "../../platform/models/domain/enc-string";
+import { ContainerService } from "../../platform/services/container.service";
+import { CollectionId, OrganizationId, UserId } from "../../types/guid";
+import { OrgKey } from "../../types/key";
+import { CollectionData } from "../models/data/collection.data";
+
+import { CollectionService, ENCRYPTED_COLLECTION_DATA_KEY } from "./collection.service";
+
+describe("CollectionService", () => {
+  afterEach(() => {
+    delete (window as any).bitwardenContainerService;
+  });
+
+  describe("decryptedCollections$", () => {
+    it("emits decrypted collections from state", async () => {
+      // Arrange test collections
+      const org1 = Utils.newGuid() as OrganizationId;
+      const org2 = Utils.newGuid() as OrganizationId;
+
+      const collection1 = collectionDataFactory(org1);
+      const collection2 = collectionDataFactory(org2);
+
+      // Arrange state provider
+      const fakeStateProvider = mockStateProvider();
+      await fakeStateProvider.setUserState(ENCRYPTED_COLLECTION_DATA_KEY, {
+        [collection1.id]: collection1,
+        [collection2.id]: collection2,
+      });
+
+      // Arrange cryptoService - orgKeys and mock decryption
+      const cryptoService = mockCryptoService();
+      cryptoService.orgKeys$.mockReturnValue(
+        of({
+          [org1]: makeSymmetricCryptoKey<OrgKey>(),
+          [org2]: makeSymmetricCryptoKey<OrgKey>(),
+        }),
+      );
+
+      const collectionService = new CollectionService(
+        cryptoService,
+        mock<EncryptService>(),
+        mockI18nService(),
+        fakeStateProvider,
+      );
+
+      const result = await firstValueFrom(collectionService.decryptedCollections$);
+      expect(result.length).toBe(2);
+      expect(result[0]).toMatchObject({
+        id: collection1.id,
+        name: "DECRYPTED_STRING",
+      });
+      expect(result[1]).toMatchObject({
+        id: collection2.id,
+        name: "DECRYPTED_STRING",
+      });
+    });
+
+    it("handles null collection state", async () => {
+      // Arrange test collections
+      const org1 = Utils.newGuid() as OrganizationId;
+      const org2 = Utils.newGuid() as OrganizationId;
+
+      // Arrange state provider
+      const fakeStateProvider = mockStateProvider();
+      await fakeStateProvider.setUserState(ENCRYPTED_COLLECTION_DATA_KEY, null);
+
+      // Arrange cryptoService - orgKeys and mock decryption
+      const cryptoService = mockCryptoService();
+      cryptoService.orgKeys$.mockReturnValue(
+        of({
+          [org1]: makeSymmetricCryptoKey<OrgKey>(),
+          [org2]: makeSymmetricCryptoKey<OrgKey>(),
+        }),
+      );
+
+      const collectionService = new CollectionService(
+        cryptoService,
+        mock<EncryptService>(),
+        mockI18nService(),
+        fakeStateProvider,
+      );
+
+      const decryptedCollections = await firstValueFrom(collectionService.decryptedCollections$);
+      expect(decryptedCollections.length).toBe(0);
+
+      const encryptedCollections = await firstValueFrom(collectionService.encryptedCollections$);
+      expect(encryptedCollections.length).toBe(0);
+    });
+  });
+});
+
+const mockI18nService = () => {
+  const i18nService = mock<I18nService>();
+  i18nService.collator = null; // this is a mock only, avoid use of this object
+  return i18nService;
+};
+
+const mockStateProvider = () => {
+  const userId = Utils.newGuid() as UserId;
+  return new FakeStateProvider(mockAccountServiceWith(userId));
+};
+
+const mockCryptoService = () => {
+  const cryptoService = mock<CryptoService>();
+  const encryptService = mock<EncryptService>();
+  encryptService.decryptToUtf8
+    .calledWith(expect.any(EncString), expect.anything())
+    .mockResolvedValue("DECRYPTED_STRING");
+
+  (window as any).bitwardenContainerService = new ContainerService(cryptoService, encryptService);
+
+  return cryptoService;
+};
+
+const collectionDataFactory = (orgId: OrganizationId) => {
+  const collection = new CollectionData({} as any);
+  collection.id = Utils.newGuid() as CollectionId;
+  collection.organizationId = orgId;
+  collection.name = makeEncString("ENC_STRING").encryptedString;
+
+  return collection;
+};

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

@@ -1,4 +1,4 @@
-import { firstValueFrom, map, Observable } from "rxjs";
+import { combineLatest, firstValueFrom, map, Observable, of, switchMap } from "rxjs";
 import { Jsonify } from "type-fest";
 
 import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service";
@@ -15,6 +15,7 @@ import {
   UserKeyDefinition,
 } from "../../platform/state";
 import { CollectionId, OrganizationId, UserId } from "../../types/guid";
+import { OrgKey } from "../../types/key";
 import { CollectionService as CollectionServiceAbstraction } from "../../vault/abstractions/collection.service";
 import { CollectionData } from "../models/data/collection.data";
 import { Collection } from "../models/domain/collection";
@@ -22,7 +23,7 @@ import { TreeNode } from "../models/domain/tree-node";
 import { CollectionView } from "../models/view/collection.view";
 import { ServiceUtils } from "../service-utils";
 
-const ENCRYPTED_COLLECTION_DATA_KEY = UserKeyDefinition.record<CollectionData, CollectionId>(
+export const ENCRYPTED_COLLECTION_DATA_KEY = UserKeyDefinition.record<CollectionData, CollectionId>(
   COLLECTION_DATA,
   "collections",
   {
@@ -31,19 +32,19 @@ const ENCRYPTED_COLLECTION_DATA_KEY = UserKeyDefinition.record<CollectionData, C
   },
 );
 
-const DECRYPTED_COLLECTION_DATA_KEY = DeriveDefinition.from<
-  Record<CollectionId, CollectionData>,
+const DECRYPTED_COLLECTION_DATA_KEY = new DeriveDefinition<
+  [Record<CollectionId, CollectionData>, Record<OrganizationId, OrgKey>],
   CollectionView[],
   { collectionService: CollectionService }
->(ENCRYPTED_COLLECTION_DATA_KEY, {
+>(COLLECTION_DATA, "decryptedCollections", {
   deserializer: (obj) => obj.map((collection) => CollectionView.fromJSON(collection)),
-  derive: async (collections: Record<CollectionId, CollectionData>, { collectionService }) => {
-    const data: Collection[] = [];
-    for (const id in collections ?? {}) {
-      const collectionId = id as CollectionId;
-      data.push(new Collection(collections[collectionId]));
+  derive: async ([collections, orgKeys], { collectionService }) => {
+    if (collections == null) {
+      return [];
     }
-    return await collectionService.decryptMany(data);
+
+    const data = Object.values(collections).map((c) => new Collection(c));
+    return await collectionService.decryptMany(data, orgKeys);
   },
 });
 
@@ -68,18 +69,25 @@ export class CollectionService implements CollectionServiceAbstraction {
     protected stateProvider: StateProvider,
   ) {
     this.encryptedCollectionDataState = this.stateProvider.getActive(ENCRYPTED_COLLECTION_DATA_KEY);
+
     this.encryptedCollections$ = this.encryptedCollectionDataState.state$.pipe(
       map((collections) => {
-        const response: Collection[] = [];
-        for (const id in collections ?? {}) {
-          response.push(new Collection(collections[id as CollectionId]));
+        if (collections == null) {
+          return [];
         }
-        return response;
+
+        return Object.values(collections).map((c) => new Collection(c));
       }),
     );
 
+    const encryptedCollectionsWithKeys = this.encryptedCollectionDataState.combinedState$.pipe(
+      switchMap(([userId, collectionData]) =>
+        combineLatest([of(collectionData), this.cryptoService.orgKeys$(userId)]),
+      ),
+    );
+
     this.decryptedCollectionDataState = this.stateProvider.getDerived(
-      this.encryptedCollectionDataState.state$,
+      encryptedCollectionsWithKeys,
       DECRYPTED_COLLECTION_DATA_KEY,
       { collectionService: this },
     );
@@ -108,19 +116,24 @@ export class CollectionService implements CollectionServiceAbstraction {
     return collection;
   }
 
-  async decryptMany(collections: Collection[]): Promise<CollectionView[]> {
-    if (collections == null) {
+  // TODO: this should be private and orgKeys should be required.
+  // See https://bitwarden.atlassian.net/browse/PM-12375
+  async decryptMany(
+    collections: Collection[],
+    orgKeys?: Record<OrganizationId, OrgKey>,
+  ): Promise<CollectionView[]> {
+    if (collections == null || collections.length === 0) {
       return [];
     }
     const decCollections: CollectionView[] = [];
 
-    const organizationKeys = await firstValueFrom(this.cryptoService.activeUserOrgKeys$);
+    orgKeys ??= await firstValueFrom(this.cryptoService.activeUserOrgKeys$);
 
     const promises: Promise<any>[] = [];
     collections.forEach((collection) => {
       promises.push(
         collection
-          .decrypt(organizationKeys[collection.organizationId as OrganizationId])
+          .decrypt(orgKeys[collection.organizationId as OrganizationId])
           .then((c) => decCollections.push(c)),
       );
     });