[PM-12049] Remove usage of ActiveUserState from folder service (#11880)

* Migrated folder service from using active user state to single user state Added extra test cases for encrypted folder and decrypted folders Updated derived state to use decrypt with key * Update callers in the web * Update callers in the browser * Update callers in libs * Update callers in cli * Fixed test * Fixed folder state test * Fixed test * removed duplicate activeUserId * Added takewhile operator to only make calls when userId is present * Simplified to accept a single user id instead of an observable * Required userid to be passed from notification service * [PM-15635] Folders not working on desktop (#12333) * Added folders memory state definition * added decrypted folders state * Refactored service to remove derived state * removed combinedstate and added clear decrypted folders to methods * Fixed test * Fixed issue with editing folder on the desktop app * Fixed test * Changed state name * fixed ts strict issue * fixed ts strict issue * fixed ts strict issue * removed unnecessasry null encrypteed folder check * Handle null folderdata * [PM-16197] "Items with No Folder" shows as a folder to edit name and delete (#12470) * Force redcryption anytime encryption state changes * Fixed text file * revert changes * create new object with nofolder instead of modifying exisiting object * Fixed failing test * switched to use memory-large-object * Fixed ts sctrict issue --------- Co-authored-by: Matt Bishop <[email protected]> Co-authored-by: bnagawiecki <[email protected]>
bitwarden · Jan 2, 2025 · 10c8a21 · 10c8a21
1 parent b966019
commit 10c8a21
Show file tree

Hide file tree

Showing 49 changed files with 592 additions and 387 deletions.
diff --git a/apps/browser/src/autofill/background/notification.background.spec.ts b/apps/browser/src/autofill/background/notification.background.spec.ts
@@ -60,10 +60,18 @@ describe("NotificationBackground", () => {
   const configService = mock<ConfigService>();
   const accountService = mock<AccountService>();
 
+  const activeAccountSubject = new BehaviorSubject<{ id: UserId } & AccountInfo>({
+    id: "testId" as UserId,
+    email: "[email protected]",
+    emailVerified: true,
+    name: "Test User",
+  });
+
   beforeEach(() => {
     activeAccountStatusMock$ = new BehaviorSubject(AuthenticationStatus.Locked);
     authService = mock<AuthService>();
     authService.activeAccountStatus$ = activeAccountStatusMock$;
+    accountService.activeAccount$ = activeAccountSubject;
     notificationBackground = new NotificationBackground(
       autofillService,
       cipherService,
@@ -683,13 +691,6 @@ describe("NotificationBackground", () => {
       });
 
       describe("saveOrUpdateCredentials", () => {
-        const activeAccountSubject = new BehaviorSubject<{ id: UserId } & AccountInfo>({
-          id: "testId" as UserId,
-          email: "[email protected]",
-          emailVerified: true,
-          name: "Test User",
-        });
-
         let getDecryptedCipherByIdSpy: jest.SpyInstance;
         let getAllDecryptedForUrlSpy: jest.SpyInstance;
         let updatePasswordSpy: jest.SpyInstance;

diff --git a/apps/browser/src/autofill/background/notification.background.ts b/apps/browser/src/autofill/background/notification.background.ts
@@ -83,6 +83,8 @@ export default class NotificationBackground {
     getWebVaultUrlForNotification: () => this.getWebVaultUrl(),
   };
 
+  private activeUserId$ = this.accountService.activeAccount$.pipe(map((a) => a?.id));
+
   constructor(
     private autofillService: AutofillService,
     private cipherService: CipherService,
@@ -569,9 +571,7 @@ export default class NotificationBackground {
         return;
       }
 
-      const activeUserId = await firstValueFrom(
-        this.accountService.activeAccount$.pipe(map((a) => a?.id)),
-      );
+      const activeUserId = await firstValueFrom(this.activeUserId$);
 
       const cipher = await this.cipherService.encrypt(newCipher, activeUserId);
       try {
@@ -611,10 +611,7 @@ export default class NotificationBackground {
       return;
     }
 
-    const activeUserId = await firstValueFrom(
-      this.accountService.activeAccount$.pipe(map((a) => a?.id)),
-    );
-
+    const activeUserId = await firstValueFrom(this.activeUserId$);
     const cipher = await this.cipherService.encrypt(cipherView, activeUserId);
     try {
       // We've only updated the password, no need to broadcast editedCipher message
@@ -647,17 +644,15 @@ export default class NotificationBackground {
     if (Utils.isNullOrWhitespace(folderId) || folderId === "null") {
       return false;
     }
-
-    const folders = await firstValueFrom(this.folderService.folderViews$);
+    const activeUserId = await firstValueFrom(this.activeUserId$);
+    const folders = await firstValueFrom(this.folderService.folderViews$(activeUserId));
     return folders.some((x) => x.id === folderId);
   }
 
   private async getDecryptedCipherById(cipherId: string) {
     const cipher = await this.cipherService.get(cipherId);
     if (cipher != null && cipher.type === CipherType.Login) {
-      const activeUserId = await firstValueFrom(
-        this.accountService.activeAccount$.pipe(map((a) => a?.id)),
-      );
+      const activeUserId = await firstValueFrom(this.activeUserId$);
 
       return await cipher.decrypt(
         await this.cipherService.getKeyForCipherKeyDecryption(cipher, activeUserId),
@@ -697,7 +692,8 @@ export default class NotificationBackground {
    * Returns the first value found from the folder service's folderViews$ observable.
    */
   private async getFolderData() {
-    return await firstValueFrom(this.folderService.folderViews$);
+    const activeUserId = await firstValueFrom(this.activeUserId$);
+    return await firstValueFrom(this.folderService.folderViews$(activeUserId));
   }
 
   private async getWebVaultUrl(): Promise<string> {

diff --git a/apps/browser/src/platform/sync/foreground-sync.service.spec.ts b/apps/browser/src/platform/sync/foreground-sync.service.spec.ts
@@ -3,7 +3,6 @@ import { Subject } from "rxjs";
 
 import { CollectionService } from "@bitwarden/admin-console/common";
 import { ApiService } from "@bitwarden/common/abstractions/api.service";
-import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
 import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
 import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
 import { StateService } from "@bitwarden/common/platform/abstractions/state.service";
@@ -30,12 +29,12 @@ describe("ForegroundSyncService", () => {
   const cipherService = mock<CipherService>();
   const collectionService = mock<CollectionService>();
   const apiService = mock<ApiService>();
-  const accountService = mock<AccountService>();
+  const accountService = mockAccountServiceWith(userId);
   const authService = mock<AuthService>();
   const sendService = mock<InternalSendService>();
   const sendApiService = mock<SendApiService>();
   const messageListener = mock<MessageListener>();
-  const stateProvider = new FakeStateProvider(mockAccountServiceWith(userId));
+  const stateProvider = new FakeStateProvider(accountService);
 
   const sut = new ForegroundSyncService(
     stateService,

diff --git a/...popup/components/vault-v2/add-edit-folder-dialog/add-edit-folder-dialog.component.spec.ts b/...popup/components/vault-v2/add-edit-folder-dialog/add-edit-folder-dialog.component.spec.ts
@@ -171,7 +171,7 @@ describe("AddEditFolderDialogComponent", () => {
     it("deletes the folder", async () => {
       await component.deleteFolder();
 
-      expect(deleteFolder).toHaveBeenCalledWith(folderView.id);
+      expect(deleteFolder).toHaveBeenCalledWith(folderView.id, "");
       expect(showToast).toHaveBeenCalledWith({
         variant: "success",
         title: null,

diff --git a/...ault/popup/components/vault-v2/add-edit-folder-dialog/add-edit-folder-dialog.component.ts b/...ault/popup/components/vault-v2/add-edit-folder-dialog/add-edit-folder-dialog.component.ts
@@ -13,7 +13,7 @@ import {
 } from "@angular/core";
 import { takeUntilDestroyed } from "@angular/core/rxjs-interop";
 import { FormBuilder, ReactiveFormsModule, Validators } from "@angular/forms";
-import { firstValueFrom } from "rxjs";
+import { firstValueFrom, map } from "rxjs";
 
 import { JslibModule } from "@bitwarden/angular/jslib.module";
 import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
@@ -67,6 +67,7 @@ export class AddEditFolderDialogComponent implements AfterViewInit, OnInit {
     name: ["", Validators.required],
   });
 
+  private activeUserId$ = this.accountService.activeAccount$.pipe(map((a) => a?.id));
   private destroyRef = inject(DestroyRef);
 
   constructor(
@@ -114,10 +115,10 @@ export class AddEditFolderDialogComponent implements AfterViewInit, OnInit {
     this.folder.name = this.folderForm.controls.name.value;
 
     try {
-      const activeUserId = await firstValueFrom(this.accountService.activeAccount$);
-      const userKey = await this.keyService.getUserKeyWithLegacySupport(activeUserId.id);
+      const activeUserId = await firstValueFrom(this.activeUserId$);
+      const userKey = await this.keyService.getUserKeyWithLegacySupport(activeUserId);
       const folder = await this.folderService.encrypt(this.folder, userKey);
-      await this.folderApiService.save(folder);
+      await this.folderApiService.save(folder, activeUserId);
 
       this.toastService.showToast({
         variant: "success",
@@ -144,7 +145,8 @@ export class AddEditFolderDialogComponent implements AfterViewInit, OnInit {
     }
 
     try {
-      await this.folderApiService.delete(this.folder.id);
+      const activeUserId = await firstValueFrom(this.activeUserId$);
+      await this.folderApiService.delete(this.folder.id, activeUserId);
       this.toastService.showToast({
         variant: "success",
         title: null,

diff --git a/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.spec.ts b/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.spec.ts
@@ -7,9 +7,12 @@ import { OrganizationService } from "@bitwarden/common/admin-console/abstraction
 import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
 import { PolicyType } from "@bitwarden/common/admin-console/enums";
 import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
+import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
 import { ProductTierType } from "@bitwarden/common/billing/enums";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { StateProvider } from "@bitwarden/common/platform/state";
+import { mockAccountServiceWith } from "@bitwarden/common/spec";
+import { UserId } from "@bitwarden/common/types/guid";
 import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
 import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction";
 import { CipherType } from "@bitwarden/common/vault/enums";
@@ -32,7 +35,7 @@ describe("VaultPopupListFiltersService", () => {
   } as unknown as CollectionService;
 
   const folderService = {
-    folderViews$,
+    folderViews$: () => folderViews$,
   } as unknown as FolderService;
 
   const cipherService = {
@@ -60,6 +63,8 @@ describe("VaultPopupListFiltersService", () => {
     policyAppliesToActiveUser$.next(false);
     policyService.policyAppliesToActiveUser$.mockClear();
 
+    const accountService = mockAccountServiceWith("userId" as UserId);
+
     collectionService.getAllNested = () => Promise.resolve([]);
     TestBed.configureTestingModule({
       providers: [
@@ -92,6 +97,10 @@ describe("VaultPopupListFiltersService", () => {
           useValue: { getGlobal: () => ({ state$, update }) },
         },
         { provide: FormBuilder, useClass: FormBuilder },
+        {
+          provide: AccountService,
+          useValue: accountService,
+        },
       ],
     });
 

diff --git a/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.ts b/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.ts
@@ -20,6 +20,7 @@ import { OrganizationService } from "@bitwarden/common/admin-console/abstraction
 import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
 import { PolicyType } from "@bitwarden/common/admin-console/enums";
 import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
+import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
 import { ProductTierType } from "@bitwarden/common/billing/enums";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { Utils } from "@bitwarden/common/platform/misc/utils";
@@ -102,6 +103,8 @@ export class VaultPopupListFiltersService {
     map((ciphers) => Object.values(ciphers)),
   );
 
+  private activeUserId$ = this.accountService.activeAccount$.pipe(map((a) => a?.id));
+
   constructor(
     private folderService: FolderService,
     private cipherService: CipherService,
@@ -111,6 +114,7 @@ export class VaultPopupListFiltersService {
     private formBuilder: FormBuilder,
     private policyService: PolicyService,
     private stateProvider: StateProvider,
+    private accountService: AccountService,
   ) {
     this.filterForm.controls.organization.valueChanges
       .pipe(takeUntilDestroyed())
@@ -264,60 +268,67 @@ export class VaultPopupListFiltersService {
   /**
    * Folder array structured to be directly passed to `ChipSelectComponent`
    */
-  folders$: Observable<ChipSelectOption<FolderView>[]> = combineLatest([
-    this.filters$.pipe(
-      distinctUntilChanged(
-        (previousFilter, currentFilter) =>
-          // Only update the collections when the organizationId filter changes
-          previousFilter.organization?.id === currentFilter.organization?.id,
-      ),
-    ),
-    this.folderService.folderViews$,
-    this.cipherViews$,
-  ]).pipe(
-    map(([filters, folders, cipherViews]): [PopupListFilter, FolderView[], CipherView[]] => {
-      if (folders.length === 1 && folders[0].id === null) {
-        // Do not display folder selections when only the "no folder" option is available.
-        return [filters, [], cipherViews];
-      }
+  folders$: Observable<ChipSelectOption<FolderView>[]> = this.activeUserId$.pipe(
+    switchMap((userId) =>
+      combineLatest([
+        this.filters$.pipe(
+          distinctUntilChanged(
+            (previousFilter, currentFilter) =>
+              // Only update the collections when the organizationId filter changes
+              previousFilter.organization?.id === currentFilter.organization?.id,
+          ),
+        ),
+        this.folderService.folderViews$(userId),
+        this.cipherViews$,
+      ]).pipe(
+        map(([filters, folders, cipherViews]): [PopupListFilter, FolderView[], CipherView[]] => {
+          if (folders.length === 1 && folders[0].id === null) {
+            // Do not display folder selections when only the "no folder" option is available.
+            return [filters, [], cipherViews];
+          }
 
-      // Sort folders by alphabetic name
-      folders.sort(Utils.getSortFunction(this.i18nService, "name"));
-      let arrangedFolders = folders;
+          // Sort folders by alphabetic name
+          folders.sort(Utils.getSortFunction(this.i18nService, "name"));
+          let arrangedFolders = folders;
 
-      const noFolder = folders.find((f) => f.id === null);
+          const noFolder = folders.find((f) => f.id === null);
 
-      if (noFolder) {
-        // Update `name` of the "no folder" option to "Items with no folder"
-        noFolder.name = this.i18nService.t("itemsWithNoFolder");
+          if (noFolder) {
+            // Update `name` of the "no folder" option to "Items with no folder"
+            const updatedNoFolder = {
+              ...noFolder,
+              name: this.i18nService.t("itemsWithNoFolder"),
+            };
 
-        // Move the "no folder" option to the end of the list
-        arrangedFolders = [...folders.filter((f) => f.id !== null), noFolder];
-      }
-      return [filters, arrangedFolders, cipherViews];
-    }),
-    map(([filters, folders, cipherViews]) => {
-      const organizationId = filters.organization?.id ?? null;
+            // Move the "no folder" option to the end of the list
+            arrangedFolders = [...folders.filter((f) => f.id !== null), updatedNoFolder];
+          }
+          return [filters, arrangedFolders, cipherViews];
+        }),
+        map(([filters, folders, cipherViews]) => {
+          const organizationId = filters.organization?.id ?? null;
 
-      // When no org or "My vault" is selected, return all folders
-      if (organizationId === null || organizationId === MY_VAULT_ID) {
-        return folders;
-      }
+          // When no org or "My vault" is selected, return all folders
+          if (organizationId === null || organizationId === MY_VAULT_ID) {
+            return folders;
+          }
 
-      const orgCiphers = cipherViews.filter((c) => c.organizationId === organizationId);
+          const orgCiphers = cipherViews.filter((c) => c.organizationId === organizationId);
 
-      // Return only the folders that have ciphers within the filtered organization
-      return folders.filter((f) => orgCiphers.some((oc) => oc.folderId === f.id));
-    }),
-    map((folders) => {
-      const nestedFolders = this.getAllFoldersNested(folders);
-      return new DynamicTreeNode<FolderView>({
-        fullList: folders,
-        nestedList: nestedFolders,
-      });
-    }),
-    map((folders) =>
-      folders.nestedList.map((f) => this.convertToChipSelectOption(f, "bwi-folder")),
+          // Return only the folders that have ciphers within the filtered organization
+          return folders.filter((f) => orgCiphers.some((oc) => oc.folderId === f.id));
+        }),
+        map((folders) => {
+          const nestedFolders = this.getAllFoldersNested(folders);
+          return new DynamicTreeNode<FolderView>({
+            fullList: folders,
+            nestedList: nestedFolders,
+          });
+        }),
+        map((folders) =>
+          folders.nestedList.map((f) => this.convertToChipSelectOption(f, "bwi-folder")),
+        ),
+      ),
     ),
   );
 

diff --git a/apps/browser/src/vault/popup/settings/folders-v2.component.spec.ts b/apps/browser/src/vault/popup/settings/folders-v2.component.spec.ts
@@ -4,10 +4,13 @@ import { By } from "@angular/platform-browser";
 import { mock } from "jest-mock-extended";
 import { BehaviorSubject } from "rxjs";
 
+import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
 import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
 import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
+import { mockAccountServiceWith } from "@bitwarden/common/spec";
+import { UserId } from "@bitwarden/common/types/guid";
 import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction";
 import { FolderView } from "@bitwarden/common/vault/models/view/folder.view";
 import { DialogService } from "@bitwarden/components";
@@ -52,8 +55,9 @@ describe("FoldersV2Component", () => {
         { provide: PlatformUtilsService, useValue: mock<PlatformUtilsService>() },
         { provide: ConfigService, useValue: mock<ConfigService>() },
         { provide: LogService, useValue: mock<LogService>() },
-        { provide: FolderService, useValue: { folderViews$ } },
+        { provide: FolderService, useValue: { folderViews$: () => folderViews$ } },
         { provide: I18nService, useValue: { t: (key: string) => key } },
+        { provide: AccountService, useValue: mockAccountServiceWith("UserId" as UserId) },
       ],
     })
       .overrideComponent(FoldersV2Component, {