[PS-1134] Folder fixes, including revamped auth logic (#3118)

apps/browser/src/background/main.background.ts

@@ -44,7 +44,6 @@ import { ApiService } from "@bitwarden/common/services/api.service";
 import { AppIdService } from "@bitwarden/common/services/appId.service";
 import { AuditService } from "@bitwarden/common/services/audit.service";
 import { AuthService } from "@bitwarden/common/services/auth.service";
-import { BroadcasterService } from "@bitwarden/common/services/broadcaster.service";
 import { CipherService } from "@bitwarden/common/services/cipher.service";
 import { CollectionService } from "@bitwarden/common/services/collection.service";
 import { ConsoleLogService } from "@bitwarden/common/services/consoleLog.service";
@@ -150,7 +149,6 @@ export default class MainBackground {
   vaultFilterService: VaultFilterService;
   usernameGenerationService: UsernameGenerationServiceAbstraction;
   encryptService: EncryptService;
-  broadcasterService: BroadcasterService;
   folderApiService: FolderApiServiceAbstraction;
 
   // Passed to the popup for Safari to workaround issues with theming, downloading, etc.
@@ -272,13 +270,11 @@ export default class MainBackground {
       this.logService,
       this.stateService
     );
-    this.broadcasterService = new BroadcasterService();
     this.folderService = new FolderService(
       this.cryptoService,
       this.i18nService,
       this.cipherService,
-      this.stateService,
-      this.broadcasterService
+      this.stateService
     );
     this.folderApiService = new FolderApiService(this.folderService, this.apiService);
     this.collectionService = new CollectionService(

apps/browser/src/popup/services/services.module.ts

@@ -13,7 +13,6 @@ import { ApiService } from "@bitwarden/common/abstractions/api.service";
 import { AppIdService } from "@bitwarden/common/abstractions/appId.service";
 import { AuditService } from "@bitwarden/common/abstractions/audit.service";
 import { AuthService as AuthServiceAbstraction } from "@bitwarden/common/abstractions/auth.service";
-import { BroadcasterService as BroadcasterServiceAbstraction } from "@bitwarden/common/abstractions/broadcaster.service";
 import { CipherService } from "@bitwarden/common/abstractions/cipher.service";
 import { CollectionService } from "@bitwarden/common/abstractions/collection.service";
 import { CryptoService } from "@bitwarden/common/abstractions/crypto.service";
@@ -114,10 +113,6 @@ function getBgService<T>(service: keyof MainBackground) {
           : new BrowserMessagingService();
       },
     },
-    {
-      provide: BroadcasterServiceAbstraction,
-      useFactory: getBgService<BroadcasterServiceAbstraction>("broadcasterService"),
-    },
     {
       provide: TwoFactorService,
       useFactory: getBgService<TwoFactorService>("twoFactorService"),

apps/cli/src/bw.ts

@@ -206,8 +206,7 @@ export class Main {
       this.cryptoService,
       this.i18nService,
       this.cipherService,
-      this.stateService,
-      this.broadcasterService
+      this.stateService
     );
 
     this.folderApiService = new FolderApiService(this.folderService, this.apiService);

apps/cli/src/commands/get.command.ts

@@ -1,5 +1,3 @@
-import { firstValueFrom } from "rxjs";
-
 import { ApiService } from "@bitwarden/common/abstractions/api.service";
 import { AuditService } from "@bitwarden/common/abstractions/audit.service";
 import { CipherService } from "@bitwarden/common/abstractions/cipher.service";
@@ -360,7 +358,7 @@ export class GetCommand extends DownloadCommand {
         decFolder = await folder.decrypt();
       }
     } else if (id.trim() !== "") {
-      let folders = await firstValueFrom(this.folderService.folderViews$);
+      let folders = await this.folderService.getAllDecryptedFromState();
       folders = CliUtils.searchFolders(folders, id);
       if (folders.length > 1) {
         return Response.multipleResults(folders.map((f) => f.id));

apps/cli/src/commands/list.command.ts

@@ -1,5 +1,3 @@
-import { firstValueFrom } from "rxjs";
-
 import { ApiService } from "@bitwarden/common/abstractions/api.service";
 import { CipherService } from "@bitwarden/common/abstractions/cipher.service";
 import { CollectionService } from "@bitwarden/common/abstractions/collection.service";
@@ -128,7 +126,7 @@ export class ListCommand {
   }
 
   private async listFolders(options: Options) {
-    let folders = await firstValueFrom(this.folderService.folderViews$);
+    let folders = await this.folderService.getAllDecryptedFromState();
 
     if (options.search != null && options.search.trim() !== "") {
       folders = CliUtils.searchFolders(folders, options.search);

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

@@ -1,5 +1,5 @@
 import { Injectable } from "@angular/core";
-import { from, mergeMap, Observable } from "rxjs";
+import { firstValueFrom, from, mergeMap, Observable } from "rxjs";
 
 import { CipherService } from "@bitwarden/common/abstractions/cipher.service";
 import { CollectionService } from "@bitwarden/common/abstractions/collection.service";
@@ -90,7 +90,7 @@ export class VaultFilterService {
     return await this.policyService.policyAppliesToUser(PolicyType.PersonalOwnership);
   }
 
-  protected async getAllFoldersNested(folders?: FolderView[]): Promise<TreeNode<FolderView>[]> {
+  protected async getAllFoldersNested(folders: FolderView[]): Promise<TreeNode<FolderView>[]> {
     const nodes: TreeNode<FolderView>[] = [];
     folders.forEach((f) => {
       const folderCopy = new FolderView();
@@ -103,7 +103,9 @@ export class VaultFilterService {
   }
 
   async getFolderNested(id: string): Promise<TreeNode<FolderView>> {
-    const folders = await this.getAllFoldersNested();
+    const folders = await this.getAllFoldersNested(
+      await firstValueFrom(this.folderService.folderViews$)
+    );
     return ServiceUtils.getTreeNodeObject(folders, id) as TreeNode<FolderView>;
   }
 }

libs/angular/src/services/jslib-services.module.ts

@@ -223,7 +223,6 @@ export const LOG_MAC_FAILURES = new InjectionToken<string>("LOG_MAC_FAILURES");
         I18nServiceAbstraction,
         CipherServiceAbstraction,
         StateServiceAbstraction,
-        BroadcasterServiceAbstraction,
       ],
     },
     {

libs/common/src/abstractions/folder/folder.service.abstraction.ts

@@ -12,6 +12,10 @@ export abstract class FolderService {
   clearCache: () => Promise<void>;
   encrypt: (model: FolderView, key?: SymmetricCryptoKey) => Promise<Folder>;
   get: (id: string) => Promise<Folder>;
+  /**
+   * @deprecated Only use in CLI!
+   */
+  getAllDecryptedFromState: () => Promise<FolderView[]>;
 }
 
 export abstract class InternalFolderService extends FolderService {

libs/common/src/abstractions/state.service.ts

@@ -1,4 +1,4 @@
-import { BehaviorSubject } from "rxjs";
+import { BehaviorSubject, Observable } from "rxjs";
 
 import { KdfType } from "../enums/kdfType";
 import { ThemeType } from "../enums/themeType";
@@ -28,6 +28,8 @@ export abstract class StateService<T extends Account = Account> {
   accounts: BehaviorSubject<{ [userId: string]: T }>;
   activeAccount: BehaviorSubject<string>;
 
+  activeAccountUnlocked: Observable<boolean>;
+
   addAccount: (account: T) => Promise<void>;
   setActiveUser: (userId: string) => Promise<void>;
   clean: (options?: StorageOptions) => Promise<void>;

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

@@ -1,9 +1,8 @@
 import { BehaviorSubject } from "rxjs";
 
-import { BroadcasterService } from "../../abstractions/broadcaster.service";
 import { CipherService } from "../../abstractions/cipher.service";
 import { CryptoService } from "../../abstractions/crypto.service";
-import { FolderService as FolderServiceAbstraction } from "../../abstractions/folder/folder.service.abstraction";
+import { InternalFolderService as InternalFolderServiceAbstraction } from "../../abstractions/folder/folder.service.abstraction";
 import { I18nService } from "../../abstractions/i18n.service";
 import { StateService } from "../../abstractions/state.service";
 import { Utils } from "../../misc/utils";
@@ -13,9 +12,7 @@ import { Folder } from "../../models/domain/folder";
 import { SymmetricCryptoKey } from "../../models/domain/symmetricCryptoKey";
 import { FolderView } from "../../models/view/folderView";
 
-const BroadcasterSubscriptionId = "FolderService";
-
-export class FolderService implements FolderServiceAbstraction {
+export class FolderService implements InternalFolderServiceAbstraction {
   private _folders: BehaviorSubject<Folder[]> = new BehaviorSubject([]);
   private _folderViews: BehaviorSubject<FolderView[]> = new BehaviorSubject([]);
 
@@ -26,15 +23,14 @@ export class FolderService implements FolderServiceAbstraction {
     private cryptoService: CryptoService,
     private i18nService: I18nService,
     private cipherService: CipherService,
-    private stateService: StateService,
-    private broadcasterService: BroadcasterService
+    private stateService: StateService
   ) {
-    this.stateService.activeAccount.subscribe(async (activeAccount) => {
+    this.stateService.activeAccountUnlocked.subscribe(async (unlocked) => {
       if ((Utils.global as any).bitwardenContainerService == null) {
         return;
       }
 
-      if (activeAccount == null) {
+      if (!unlocked) {
         this._folders.next([]);
         this._folderViews.next([]);
         return;
@@ -44,20 +40,6 @@ export class FolderService implements FolderServiceAbstraction {
 
       await this.updateObservables(data);
     });
-
-    // TODO: Broadcasterservice should be removed or replaced with observables
-    this.broadcasterService.subscribe(BroadcasterSubscriptionId, async (message: any) => {
-      switch (message.command) {
-        case "unlocked": {
-          const data = await this.stateService.getEncryptedFolders();
-
-          await this.updateObservables(data);
-          break;
-        }
-        default:
-          break;
-      }
-    });
   }
 
   async clearCache(): Promise<void> {
@@ -78,6 +60,16 @@ export class FolderService implements FolderServiceAbstraction {
     return folders.find((folder) => folder.id === id);
   }
 
+  /**
+   * @deprecated Only use in CLI!
+   */
+  async getAllDecryptedFromState(): Promise<FolderView[]> {
+    const data = await this.stateService.getEncryptedFolders();
+    const folders = Object.values(data || {}).map((f) => new Folder(f));
+
+    return this.decryptFolders(folders);
+  }
+
   async upsert(folder: FolderData | FolderData[]): Promise<void> {
     let folders = await this.stateService.getEncryptedFolders();
     if (folders == null) {
@@ -149,6 +141,14 @@ export class FolderService implements FolderServiceAbstraction {
   private async updateObservables(foldersMap: { [id: string]: FolderData }) {
     const folders = Object.values(foldersMap || {}).map((f) => new Folder(f));
 
+    this._folders.next(folders);
+
+    if (await this.cryptoService.hasKey()) {
+      this._folderViews.next(await this.decryptFolders(folders));
+    }
+  }
+
+  private async decryptFolders(folders: Folder[]) {
     const decryptFolderPromises = folders.map((f) => f.decrypt());
     const decryptedFolders = await Promise.all(decryptFolderPromises);
 
@@ -158,7 +158,6 @@ export class FolderService implements FolderServiceAbstraction {
     noneFolder.name = this.i18nService.t("noneFolder");
     decryptedFolders.push(noneFolder);
 
-    this._folders.next(folders);
-    this._folderViews.next(decryptedFolders);
+    return decryptedFolders;
   }
 }

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

@@ -56,6 +56,7 @@ export class StateService<
 {
   accounts = new BehaviorSubject<{ [userId: string]: TAccount }>({});
   activeAccount = new BehaviorSubject<string>(null);
+  activeAccountUnlocked = new BehaviorSubject<boolean>(false);
 
   private hasBeenInited = false;
   private isRecoveredSession = false;
@@ -70,7 +71,21 @@ export class StateService<
     protected stateMigrationService: StateMigrationService,
     protected stateFactory: StateFactory<TGlobalState, TAccount>,
     protected useAccountCache: boolean = true
-  ) {}
+  ) {
+    // If the account gets changed, verify the new account is unlocked
+    this.activeAccount.subscribe(async (userId) => {
+      if (userId == null && this.activeAccountUnlocked.getValue() == false) {
+        return;
+      } else if (userId == null) {
+        this.activeAccountUnlocked.next(false);
+      }
+
+      // FIXME: This should be refactored into AuthService or a similar service,
+      //  as checking for the existance of the crypto key is a low level
+      //  implementation detail.
+      this.activeAccountUnlocked.next((await this.getCryptoMasterKey()) != null);
+    });
+  }
 
   async init(): Promise<void> {
     if (this.hasBeenInited) {
@@ -499,6 +514,15 @@ export class StateService<
       account,
       this.reconcileOptions(options, await this.defaultInMemoryOptions())
     );
+
+    if (options.userId == this.activeAccount.getValue()) {
+      const nextValue = value != null;
+
+      // Avoid emitting if we are already unlocked
+      if (this.activeAccountUnlocked.getValue() != nextValue) {
+        this.activeAccountUnlocked.next(nextValue);
+      }
+    }
   }
 
   async getCryptoMasterKeyAuto(options?: StorageOptions): Promise<string> {