[PM-6991] Improve reactivity of cipherViews$ observable in cipher ser…

…vice (#11141)

* [PM-6991] Remove self reference to cipherViews$

* [PM-6991] Update cipherViews$ observable to be reactive to encrypted ciphers and localData

* [PM-6991] Use the cipherViews$ observable in the Web vault

* [PM-6991] Update the vault filter service to use cipherViews$ observable

* [PM-6991] Add deprecation notice to getAllDecrypted

* [PM-6991] Ensure cipherViews$ emits an empty array whenever the decrypted cipher cache is cleared

* [PM-6991] Fix cipher service test

* [PM-6991] Use shareReplay instead of share

* [PM-6991] Remove ciphersExpectingUpdate and replace with a null filter instead for cipherViews$

* [PM-6991] Skip refreshing on null cipherViews$ to avoid flashing an empty vault after modifying ciphers

apps/web/src/app/vault/individual-vault/vault-filter/services/vault-filter.service.spec.ts

@@ -35,6 +35,7 @@ describe("vault filter service", () => {
   let organizations: ReplaySubject<Organization[]>;
   let folderViews: ReplaySubject<FolderView[]>;
   let collectionViews: ReplaySubject<CollectionView[]>;
+  let cipherViews: ReplaySubject<CipherView[]>;
   let personalOwnershipPolicy: ReplaySubject<boolean>;
   let singleOrgPolicy: ReplaySubject<boolean>;
   let stateProvider: FakeStateProvider;
@@ -57,6 +58,7 @@ describe("vault filter service", () => {
     organizations = new ReplaySubject<Organization[]>(1);
     folderViews = new ReplaySubject<FolderView[]>(1);
     collectionViews = new ReplaySubject<CollectionView[]>(1);
+    cipherViews = new ReplaySubject<CipherView[]>(1);
     personalOwnershipPolicy = new ReplaySubject<boolean>(1);
     singleOrgPolicy = new ReplaySubject<boolean>(1);
 
@@ -69,6 +71,7 @@ describe("vault filter service", () => {
     policyService.policyAppliesToActiveUser$
       .calledWith(PolicyType.SingleOrg)
       .mockReturnValue(singleOrgPolicy);
+    cipherService.cipherViews$ = cipherViews;
 
     vaultFilterService = new VaultFilterService(
       organizationService,
@@ -162,7 +165,7 @@ describe("vault filter service", () => {
           createCipherView("1", "org test id", "folder test id"),
           createCipherView("2", "non matching org id", "non matching folder id"),
         ];
-        cipherService.getAllDecrypted.mockResolvedValue(storedCiphers);
+        cipherViews.next(storedCiphers);
 
         const storedFolders = [
           createFolderView("folder test id", "test"),
@@ -191,6 +194,7 @@ describe("vault filter service", () => {
           createFolderView("Folder 3 Id", "Folder 1/Folder 3"),
         ];
         folderViews.next(storedFolders);
+        cipherViews.next([]);
 
         const result = await firstValueFrom(vaultFilterService.folderTree$);
 

apps/web/src/app/vault/individual-vault/vault-filter/services/vault-filter.service.ts

@@ -25,6 +25,7 @@ import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.servi
 import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction";
 import { CipherType } from "@bitwarden/common/vault/enums";
 import { TreeNode } from "@bitwarden/common/vault/models/domain/tree-node";
+import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
 import { FolderView } from "@bitwarden/common/vault/models/view/folder.view";
 import { ServiceUtils } from "@bitwarden/common/vault/service-utils";
 import { COLLAPSED_GROUPINGS } from "@bitwarden/common/vault/services/key-state/collapsed-groupings.state";
@@ -55,9 +56,9 @@ export class VaultFilterService implements VaultFilterServiceAbstraction {
   protected _organizationFilter = new BehaviorSubject<Organization>(null);
 
   filteredFolders$: Observable<FolderView[]> = this.folderService.folderViews$.pipe(
-    combineLatestWith(this._organizationFilter),
-    switchMap(([folders, org]) => {
-      return this.filterFolders(folders, org);
+    combineLatestWith(this.cipherService.cipherViews$, this._organizationFilter),
+    switchMap(([folders, ciphers, org]) => {
+      return this.filterFolders(folders, ciphers, org);
     }),
   );
   folderTree$: Observable<TreeNode<FolderFilter>> = this.filteredFolders$.pipe(
@@ -231,6 +232,7 @@ export class VaultFilterService implements VaultFilterServiceAbstraction {
 
   protected async filterFolders(
     storedFolders: FolderView[],
+    ciphers: CipherView[],
     org?: Organization,
   ): Promise<FolderView[]> {
     // If no org or "My Vault" is selected, show all folders
@@ -239,7 +241,6 @@ export class VaultFilterService implements VaultFilterServiceAbstraction {
     }
 
     // Otherwise, show only folders that have ciphers from the selected org and the "no folder" folder
-    const ciphers = await this.cipherService.getAllDecrypted();
     const orgCiphers = ciphers.filter((c) => c.organizationId == org?.id);
     return storedFolders.filter(
       (f) => orgCiphers.some((oc) => oc.folderId == f.id) || f.id == null,

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

@@ -281,7 +281,7 @@ export class VaultComponent implements OnInit, OnDestroy {
     this.currentSearchText$ = this.route.queryParams.pipe(map((queryParams) => queryParams.search));
 
     const ciphers$ = combineLatest([
-      Utils.asyncToObservable(() => this.cipherService.getAllDecrypted()),
+      this.cipherService.cipherViews$.pipe(filter((c) => c !== null)),
       filter$,
       this.currentSearchText$,
     ]).pipe(

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

@@ -17,7 +17,7 @@ import { FieldView } from "../models/view/field.view";
 import { AddEditCipherInfo } from "../types/add-edit-cipher-info";
 
 export abstract class CipherService implements UserKeyRotationDataProvider<CipherWithIdRequest> {
-  cipherViews$: Observable<Record<CipherId, CipherView>>;
+  cipherViews$: Observable<CipherView[]>;
   ciphers$: Observable<Record<CipherId, CipherData>>;
   localData$: Observable<Record<CipherId, LocalData>>;
   /**

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

@@ -1,5 +1,5 @@
 import { mock } from "jest-mock-extended";
-import { BehaviorSubject, of } from "rxjs";
+import { BehaviorSubject, map, of } from "rxjs";
 
 import { BulkEncryptService } from "@bitwarden/common/platform/abstractions/bulk-encrypt.service";
 
@@ -381,7 +381,7 @@ describe("Cipher Service", () => {
         Cipher1: cipher1,
         Cipher2: cipher2,
       });
-      cipherService.cipherViews$ = decryptedCiphers;
+      cipherService.cipherViews$ = decryptedCiphers.pipe(map((ciphers) => Object.values(ciphers)));
 
       encryptService.decryptToBytes.mockResolvedValue(new Uint8Array(32));
       encryptedKey = new EncString("Re-encrypted Cipher Key");

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

@@ -1,4 +1,14 @@
-import { firstValueFrom, map, Observable, skipWhile, switchMap } from "rxjs";
+import {
+  combineLatest,
+  filter,
+  firstValueFrom,
+  map,
+  merge,
+  Observable,
+  shareReplay,
+  Subject,
+  switchMap,
+} from "rxjs";
 import { SemVer } from "semver";
 
 import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
@@ -24,13 +34,7 @@ import Domain from "../../platform/models/domain/domain-base";
 import { EncArrayBuffer } from "../../platform/models/domain/enc-array-buffer";
 import { EncString } from "../../platform/models/domain/enc-string";
 import { SymmetricCryptoKey } from "../../platform/models/domain/symmetric-crypto-key";
-import {
-  ActiveUserState,
-  CIPHERS_MEMORY,
-  DeriveDefinition,
-  DerivedState,
-  StateProvider,
-} from "../../platform/state";
+import { ActiveUserState, StateProvider } from "../../platform/state";
 import { CipherId, CollectionId, OrganizationId, UserId } from "../../types/guid";
 import { OrgKey, UserKey } from "../../types/key";
 import { CipherService as CipherServiceAbstraction } from "../abstractions/cipher.service";
@@ -81,14 +85,25 @@ export class CipherService implements CipherServiceAbstraction {
   private sortedCiphersCache: SortedCiphersCache = new SortedCiphersCache(
     this.sortCiphersByLastUsed,
   );
-  private ciphersExpectingUpdate: DerivedState<boolean>;
+  /**
+   * Observable that forces the `cipherViews$` observable to re-emit with the provided value.
+   * Used to let subscribers of `cipherViews$` know that the decrypted ciphers have been cleared for the active user.
+   * @private
+   */
+  private forceCipherViews$: Subject<CipherView[]> = new Subject<CipherView[]>();
 
   localData$: Observable<Record<CipherId, LocalData>>;
   ciphers$: Observable<Record<CipherId, CipherData>>;
-  cipherViews$: Observable<Record<CipherId, CipherView>>;
-  viewFor$(id: CipherId) {
-    return this.cipherViews$.pipe(map((views) => views[id]));
-  }
+
+  /**
+   * Observable that emits an array of decrypted ciphers for the active user.
+   * This observable will not emit until the encrypted ciphers have either been loaded from state or after sync.
+   *
+   * A `null` value indicates that the latest encrypted ciphers have not been decrypted yet and that
+   * decryption is in progress. The latest decrypted ciphers will be emitted once decryption is complete.
+   *
+   */
+  cipherViews$: Observable<CipherView[] | null>;
   addEditCipherInfo$: Observable<AddEditCipherInfo>;
 
   private localDataState: ActiveUserState<Record<CipherId, LocalData>>;
@@ -115,23 +130,16 @@ export class CipherService implements CipherServiceAbstraction {
     this.encryptedCiphersState = this.stateProvider.getActive(ENCRYPTED_CIPHERS);
     this.decryptedCiphersState = this.stateProvider.getActive(DECRYPTED_CIPHERS);
     this.addEditCipherInfoState = this.stateProvider.getActive(ADD_EDIT_CIPHER_INFO_KEY);
-    this.ciphersExpectingUpdate = this.stateProvider.getDerived(
-      this.encryptedCiphersState.state$,
-      new DeriveDefinition(CIPHERS_MEMORY, "ciphersExpectingUpdate", {
-        derive: (_: Record<CipherId, CipherData>) => false,
-        deserializer: (value) => value,
-      }),
-      {},
-    );
 
     this.localData$ = this.localDataState.state$.pipe(map((data) => data ?? {}));
-    // First wait for ciphersExpectingUpdate to be false before emitting ciphers
-    this.ciphers$ = this.ciphersExpectingUpdate.state$.pipe(
-      skipWhile((expectingUpdate) => expectingUpdate),
-      switchMap(() => this.encryptedCiphersState.state$),
-      map((ciphers) => ciphers ?? {}),
+    this.ciphers$ = this.encryptedCiphersState.state$.pipe(map((ciphers) => ciphers ?? {}));
+
+    // Decrypted ciphers depend on both ciphers and local data and need to be updated when either changes
+    this.cipherViews$ = combineLatest([this.encryptedCiphersState.state$, this.localData$]).pipe(
+      filter(([ciphers]) => ciphers != null), // Skip if ciphers haven't been loaded yor synced yet
+      switchMap(() => merge(this.forceCipherViews$, this.getAllDecrypted())),
+      shareReplay({ bufferSize: 1, refCount: true }),
     );
-    this.cipherViews$ = this.decryptedCiphersState.state$.pipe(map((views) => views ?? {}));
     this.addEditCipherInfo$ = this.addEditCipherInfoState.state$;
   }
 
@@ -160,8 +168,14 @@ export class CipherService implements CipherServiceAbstraction {
   }
 
   async clearCache(userId?: UserId): Promise<void> {
-    userId ??= await firstValueFrom(this.stateProvider.activeUserId$);
+    const activeUserId = await firstValueFrom(this.stateProvider.activeUserId$);
+    userId ??= activeUserId;
     await this.clearDecryptedCiphersState(userId);
+
+    // Force the cipherView$ observable (which always tracks the active user) to re-emit
+    if (userId == activeUserId) {
+      this.forceCipherViews$.next(null);
+    }
   }
 
   async encrypt(
@@ -354,6 +368,11 @@ export class CipherService implements CipherServiceAbstraction {
     return response;
   }
 
+  /**
+   * Decrypts all ciphers for the active user and caches them in memory. If the ciphers have already been decrypted and
+   * cached, the cached ciphers are returned.
+   * @deprecated Use `cipherViews$` observable instead
+   */
   @sequentialize(() => "getAllDecrypted")
   async getAllDecrypted(): Promise<CipherView[]> {
     let decCiphers = await this.getDecryptedCiphers();
@@ -375,7 +394,9 @@ export class CipherService implements CipherServiceAbstraction {
   }
 
   private async getDecryptedCiphers() {
-    return Object.values(await firstValueFrom(this.cipherViews$));
+    return Object.values(
+      await firstValueFrom(this.decryptedCiphersState.state$.pipe(map((c) => c ?? {}))),
+    );
   }
 
   private async decryptCiphers(ciphers: Cipher[], userId: UserId) {
@@ -932,8 +953,6 @@ export class CipherService implements CipherServiceAbstraction {
     userId: UserId = null,
   ): Promise<Record<CipherId, CipherData>> {
     userId ||= await firstValueFrom(this.stateProvider.activeUserId$);
-    // Store that we should wait for an update to return any ciphers
-    await this.ciphersExpectingUpdate.forceValue(true);
     await this.clearDecryptedCiphersState(userId);
     const updatedCiphers = await this.stateProvider
       .getUser(userId, ENCRYPTED_CIPHERS)
@@ -1254,7 +1273,7 @@ export class CipherService implements CipherServiceAbstraction {
 
     let encryptedCiphers: CipherWithIdRequest[] = [];
 
-    const ciphers = await this.getAllDecrypted();
+    const ciphers = await firstValueFrom(this.cipherViews$);
     if (!ciphers) {
       return encryptedCiphers;
     }