[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 <mbishop@bitwarden.com> Co-authored-by: bnagawiecki <107435978+bnagawiecki@users.noreply.github.com>
2026-01-09 12:03:33 +00:00 · 2025-01-02 17:16:33 -05:00
parent b9660194be
commit 10c8a2101a
49 changed files with 600 additions and 395 deletions
--- 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: "test@example.com",
+    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: "test@example.com",
-          emailVerified: true,
-          name: "Test User",
-        });
-
        let getDecryptedCipherByIdSpy: jest.SpyInstance;
        let getAllDecryptedForUrlSpy: jest.SpyInstance;
        let updatePasswordSpy: jest.SpyInstance;
--- 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> {
--- 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,
--- a/apps/browser/src/vault/popup/components/vault-v2/add-edit-folder-dialog/add-edit-folder-dialog.component.spec.ts
+++ b/apps/browser/src/vault/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,
--- a/apps/browser/src/vault/popup/components/vault-v2/add-edit-folder-dialog/add-edit-folder-dialog.component.ts
+++ b/apps/browser/src/vault/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,
--- 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,
+        },
      ],
    });

--- 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,61 +268,68 @@ 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,
+  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;
+
+          const noFolder = folders.find((f) => f.id === null);
+
+          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), 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;
+          }
+
+          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")),
+        ),
      ),
    ),
-    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];
-      }
-
-      // Sort folders by alphabetic name
-      folders.sort(Utils.getSortFunction(this.i18nService, "name"));
-      let arrangedFolders = folders;
-
-      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");
-
-        // 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;
-
-      // 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);
-
-      // 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")),
-    ),
  );

  /**
--- 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, {
--- a/apps/browser/src/vault/popup/settings/folders-v2.component.ts
+++ b/apps/browser/src/vault/popup/settings/folders-v2.component.ts
@@ -1,8 +1,10 @@
 import { CommonModule } from "@angular/common";
 import { Component } from "@angular/core";
-import { map, Observable } from "rxjs";
+import { filter, map, Observable, switchMap } from "rxjs";

 import { JslibModule } from "@bitwarden/angular/jslib.module";
+import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
+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 {
@@ -45,18 +47,21 @@ export class FoldersV2Component {
  folders$: Observable<FolderView[]>;

  NoFoldersIcon = VaultIcons.NoFolders;
+  private activeUserId$ = this.accountService.activeAccount$.pipe(map((a) => a?.id));

  constructor(
    private folderService: FolderService,
    private dialogService: DialogService,
+    private accountService: AccountService,
  ) {
-    this.folders$ = this.folderService.folderViews$.pipe(
+    this.folders$ = this.activeUserId$.pipe(
+      filter((userId): userId is UserId => userId !== null),
+      switchMap((userId) => this.folderService.folderViews$(userId)),
      map((folders) => {
        // Remove the last folder, which is the "no folder" option folder
        if (folders.length > 0) {
          return folders.slice(0, folders.length - 1);
        }
-
        return folders;
      }),
    );
--- a/apps/browser/src/vault/popup/settings/folders.component.ts
+++ b/apps/browser/src/vault/popup/settings/folders.component.ts
@@ -1,7 +1,9 @@
 import { Component } from "@angular/core";
 import { Router } from "@angular/router";
-import { map, Observable } from "rxjs";
+import { filter, map, Observable, switchMap } from "rxjs";

+import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
+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";

@@ -12,16 +14,21 @@ import { FolderView } from "@bitwarden/common/vault/models/view/folder.view";
 export class FoldersComponent {
  folders$: Observable<FolderView[]>;

+  private activeUserId$ = this.accountService.activeAccount$.pipe(map((a) => a?.id));
+
  constructor(
    private folderService: FolderService,
    private router: Router,
+    private accountService: AccountService,
  ) {
-    this.folders$ = this.folderService.folderViews$.pipe(
+    this.folders$ = this.activeUserId$.pipe(
+      filter((userId): userId is UserId => userId != null),
+      switchMap((userId) => this.folderService.folderViews$(userId)),
      map((folders) => {
+        // Remove the last folder, which is the "no folder" option folder
        if (folders.length > 0) {
-          folders = folders.slice(0, folders.length - 1);
+          return folders.slice(0, folders.length - 1);
        }
-
        return folders;
      }),
    );
--- a/apps/browser/src/vault/services/vault-filter.service.ts
+++ b/apps/browser/src/vault/services/vault-filter.service.ts
@@ -24,7 +24,7 @@ export class VaultFilterService extends BaseVaultFilterService {
    collectionService: CollectionService,
    policyService: PolicyService,
    stateProvider: StateProvider,
-    private accountService: AccountService,
+    accountService: AccountService,
  ) {
    super(
      organizationService,
@@ -33,6 +33,7 @@ export class VaultFilterService extends BaseVaultFilterService {
      collectionService,
      policyService,
      stateProvider,
+      accountService,
    );
    this.vaultFilter.myVaultOnly = false;
    this.vaultFilter.selectedOrganizationId = null;