From d0d1359ff4fd4d40edaa5f1096373d642bafc40d Mon Sep 17 00:00:00 2001 From: Brandon Treston Date: Wed, 23 Jul 2025 19:05:15 -0400 Subject: [PATCH] [PM-12048] Wire up vNextCollectionService (#14871) * remove derived state, add cache in service. Fix ts strict errors * cleanup * promote vNextCollectionService * wip * replace callers in web WIP * refactor tests for web * update callers to use vNextCollectionServcie methods in CLI * WIP make decryptMany public again, fix callers, imports * wip cli * wip desktop * update callers in browser, fix tests * remove in service cache * cleanup * fix test * clean up * address cr feedback * remove duplicate userId * clean up * remove unused import * fix vault-settings-import-nudge.service * fix caching issue * clean up * refactor decryption, cleanup, update callers * clean up * Use in-memory statedefinition * Ac/pm 12048 v next collection service pairing (#15239) * Draft from pairing with Gibson * Add todos * Add comment * wip * refactor upsert --------- Co-authored-by: Brandon * clean up * fix state definitions * fix linter error * cleanup * add test, fix shareReplay * fix item-more-options component * fix desktop build * refactor state to account for null as an initial value, remove caching * add proper cache, add unit test, update callers * clean up * fix routing when deleting collections * cleanup * use combineLatest * fix ts-strict errors, fix error handling * refactor Collection and CollectionView properties for ts-strict * Revert "refactor Collection and CollectionView properties for ts-strict" This reverts commit a5c63aab76ba5663f4136000fe31fbb9171b8d4b. --------- Co-authored-by: Thomas Rittson Co-authored-by: Thomas Rittson <31796059+eliykat@users.noreply.github.com> --- .../background/notification.background.ts | 29 +- .../browser/src/background/main.background.ts | 1 - .../assign-collections.component.ts | 8 +- .../item-more-options.component.ts | 2 +- .../vault-popup-items.service.spec.ts | 2 +- .../services/vault-popup-items.service.ts | 9 +- .../vault-popup-list-filters.service.spec.ts | 28 +- .../vault-popup-list-filters.service.ts | 18 +- apps/cli/src/commands/get.command.ts | 10 +- apps/cli/src/commands/list.command.ts | 17 +- apps/cli/src/oss-serve-configurator.ts | 1 + .../service-container/service-container.ts | 1 - apps/cli/src/vault.program.ts | 1 + apps/desktop/src/app/app.component.ts | 1 - .../src/vault/app/vault/vault-v2.component.ts | 20 +- .../collections/vault.component.ts | 3 + .../organizations/manage/groups.component.ts | 26 +- .../members/members.component.ts | 27 +- .../collection-dialog.component.ts | 26 +- ...ree-org-collection-limit.validator.spec.ts | 10 +- .../free-org-collection-limit.validator.ts | 20 +- apps/web/src/app/app.component.ts | 1 - .../bulk-delete-dialog.component.ts | 9 +- .../services/vault-filter.service.spec.ts | 2 +- .../services/vault-filter.service.ts | 15 +- .../vault/individual-vault/vault.component.ts | 33 +- .../abstractions/collection-admin.service.ts | 19 +- .../abstractions/collection.service.ts | 40 +- .../abstractions/vnext-collection.service.ts | 43 -- .../collections/models/collection.data.ts | 5 +- .../common/collections/models/collection.ts | 26 +- .../collections/models/collection.view.ts | 2 +- .../collections/services/collection.state.ts | 28 ++ .../default-collection-admin.service.ts | 8 +- .../default-collection.service.spec.ts | 433 ++++++++++++++---- .../services/default-collection.service.ts | 338 +++++++------- .../default-vnext-collection.service.spec.ts | 345 -------------- .../default-vnext-collection.service.ts | 194 -------- .../services/vnext-collection.state.ts | 36 -- .../empty-vault-nudge.service.ts | 2 +- .../vault-settings-import-nudge.service.ts | 2 +- .../services/vault-filter.service.ts | 7 +- .../services/vault-timeout.service.ts | 4 - .../src/platform/misc/rxjs-operators.ts | 6 +- .../src/platform/models/domain/domain-base.ts | 2 +- .../src/platform/state/state-definitions.ts | 6 +- .../src/platform/sync/core-sync.service.ts | 6 +- .../cipher-authorization.service.spec.ts | 9 +- .../services/cipher-authorization.service.ts | 18 +- .../src/components/import.component.ts | 12 +- libs/importer/src/services/import.service.ts | 2 +- .../src/services/org-vault-export.service.ts | 35 +- .../src/components/export.component.ts | 40 +- .../default-cipher-form-config.service.ts | 5 +- .../src/cipher-view/cipher-view.component.ts | 13 +- .../assign-collections.component.ts | 12 +- 56 files changed, 906 insertions(+), 1112 deletions(-) delete mode 100644 libs/admin-console/src/common/collections/abstractions/vnext-collection.service.ts create mode 100644 libs/admin-console/src/common/collections/services/collection.state.ts delete mode 100644 libs/admin-console/src/common/collections/services/default-vnext-collection.service.spec.ts delete mode 100644 libs/admin-console/src/common/collections/services/default-vnext-collection.service.ts delete mode 100644 libs/admin-console/src/common/collections/services/vnext-collection.state.ts diff --git a/apps/browser/src/autofill/background/notification.background.ts b/apps/browser/src/autofill/background/notification.background.ts index 65c1ca0277f..15baccf3560 100644 --- a/apps/browser/src/autofill/background/notification.background.ts +++ b/apps/browser/src/autofill/background/notification.background.ts @@ -1030,18 +1030,23 @@ export default class NotificationBackground { private async getCollectionData( message: NotificationBackgroundExtensionMessage, ): Promise { - const collections = (await this.collectionService.getAllDecrypted()).reduce( - (acc, collection) => { - if (collection.organizationId === message?.orgId) { - acc.push({ - id: collection.id, - name: collection.name, - organizationId: collection.organizationId, - }); - } - return acc; - }, - [], + const collections = await firstValueFrom( + this.accountService.activeAccount$.pipe( + getUserId, + switchMap((userId) => this.collectionService.decryptedCollections$(userId)), + map((collections) => + collections.reduce((acc, collection) => { + if (collection.organizationId === message?.orgId) { + acc.push({ + id: collection.id, + name: collection.name, + organizationId: collection.organizationId, + }); + } + return acc; + }, []), + ), + ), ); return collections; } diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 2565f366870..3aaf24c07a4 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -1620,7 +1620,6 @@ export default class MainBackground { this.keyService.clearKeys(userBeingLoggedOut), this.cipherService.clear(userBeingLoggedOut), this.folderService.clear(userBeingLoggedOut), - this.collectionService.clear(userBeingLoggedOut), this.vaultTimeoutSettingsService.clear(userBeingLoggedOut), this.vaultFilterService.clear(), this.biometricStateService.logout(userBeingLoggedOut), diff --git a/apps/browser/src/vault/popup/components/vault-v2/assign-collections/assign-collections.component.ts b/apps/browser/src/vault/popup/components/vault-v2/assign-collections/assign-collections.component.ts index 8374cc254a9..0b7346c8613 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/assign-collections/assign-collections.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/assign-collections/assign-collections.component.ts @@ -10,6 +10,7 @@ import { Observable, combineLatest, filter, first, map, switchMap } from "rxjs"; import { CollectionService } from "@bitwarden/admin-console/common"; import { JslibModule } from "@bitwarden/angular/jslib.module"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { OrganizationId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; @@ -70,7 +71,12 @@ export class AssignCollections { ), ); - combineLatest([cipher$, this.collectionService.decryptedCollections$]) + const decryptedCollection$ = this.accountService.activeAccount$.pipe( + getUserId, + switchMap((userId) => this.collectionService.decryptedCollections$(userId)), + ); + + combineLatest([cipher$, decryptedCollection$]) .pipe(takeUntilDestroyed(), first()) .subscribe(([cipherView, collections]) => { let availableCollections = collections; diff --git a/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.ts b/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.ts index ce16ec2f3e0..6c3a7243309 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.ts @@ -93,7 +93,7 @@ export class ItemMoreOptionsComponent { switchMap((userId) => { return combineLatest([ this.organizationService.hasOrganizations(userId), - this.collectionService.decryptedCollections$, + this.collectionService.decryptedCollections$(userId), ]).pipe( map(([hasOrgs, collections]) => { const canEditCollections = collections.some((c) => !c.readOnly); diff --git a/apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts b/apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts index 48788ea5ae9..32974da162d 100644 --- a/apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts +++ b/apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts @@ -139,7 +139,7 @@ describe("VaultPopupItemsService", () => { ]; organizationServiceMock.organizations$.mockReturnValue(new BehaviorSubject([mockOrg])); - collectionService.decryptedCollections$ = new BehaviorSubject(mockCollections); + collectionService.decryptedCollections$.mockReturnValue(new BehaviorSubject(mockCollections)); activeUserLastSync$ = new BehaviorSubject(new Date()); syncServiceMock.activeUserLastSync$.mockReturnValue(activeUserLastSync$); diff --git a/apps/browser/src/vault/popup/services/vault-popup-items.service.ts b/apps/browser/src/vault/popup/services/vault-popup-items.service.ts index b2d4fd1b262..9d44eef2e47 100644 --- a/apps/browser/src/vault/popup/services/vault-popup-items.service.ts +++ b/apps/browser/src/vault/popup/services/vault-popup-items.service.ts @@ -72,6 +72,11 @@ export class VaultPopupItemsService { private organizations$ = this.activeUserId$.pipe( switchMap((userId) => this.organizationService.organizations$(userId)), ); + + private decryptedCollections$ = this.activeUserId$.pipe( + switchMap((userId) => this.collectionService.decryptedCollections$(userId)), + ); + /** * Observable that contains the list of other cipher types that should be shown * in the autofill section of the Vault tab. Depends on vault settings. @@ -130,7 +135,7 @@ export class VaultPopupItemsService { private _activeCipherList$: Observable = this._allDecryptedCiphers$.pipe( switchMap((ciphers) => - combineLatest([this.organizations$, this.collectionService.decryptedCollections$]).pipe( + combineLatest([this.organizations$, this.decryptedCollections$]).pipe( map(([organizations, collections]) => { const orgMap = Object.fromEntries(organizations.map((org) => [org.id, org])); const collectionMap = Object.fromEntries(collections.map((col) => [col.id, col])); @@ -291,7 +296,7 @@ export class VaultPopupItemsService { */ deletedCiphers$: Observable = this._allDecryptedCiphers$.pipe( switchMap((ciphers) => - combineLatest([this.organizations$, this.collectionService.decryptedCollections$]).pipe( + combineLatest([this.organizations$, this.decryptedCollections$]).pipe( map(([organizations, collections]) => { const orgMap = Object.fromEntries(organizations.map((org) => [org.id, org])); const collectionMap = Object.fromEntries(collections.map((col) => [col.id, col])); 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 index 9f1bd6e6e55..ebaeaeb6076 100644 --- 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 @@ -20,6 +20,7 @@ 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"; +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 { @@ -58,7 +59,7 @@ describe("VaultPopupListFiltersService", () => { }; const collectionService = { - decryptedCollections$, + decryptedCollections$: () => decryptedCollections$, getAllNested: () => Promise.resolve([]), } as unknown as CollectionService; @@ -106,7 +107,7 @@ describe("VaultPopupListFiltersService", () => { signal: jest.fn(() => mockCachedSignal), }; - collectionService.getAllNested = () => Promise.resolve([]); + collectionService.getAllNested = () => []; TestBed.configureTestingModule({ providers: [ { @@ -382,14 +383,7 @@ describe("VaultPopupListFiltersService", () => { beforeEach(() => { decryptedCollections$.next(testCollections); - collectionService.getAllNested = () => - Promise.resolve( - testCollections.map((c) => ({ - children: [], - node: c, - parent: null, - })), - ); + collectionService.getAllNested = () => testCollections.map((c) => new TreeNode(c, null)); }); it("returns all collections", (done) => { @@ -755,15 +749,13 @@ function createSeededVaultPopupListFiltersService( } as any; const collectionServiceMock = { - decryptedCollections$: seededCollections$, + decryptedCollections$: () => seededCollections$, getAllNested: () => - Promise.resolve( - seededCollections$.value.map((c) => ({ - children: [], - node: c, - parent: null, - })), - ), + seededCollections$.value.map((c) => ({ + children: [], + node: c, + parent: null, + })), } as any; const folderServiceMock = { 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 index 7af6fb5f212..adc0589e7e8 100644 --- 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 @@ -6,7 +6,6 @@ import { debounceTime, distinctUntilChanged, filter, - from, map, Observable, shareReplay, @@ -446,7 +445,7 @@ export class VaultPopupListFiltersService { this.filters$.pipe( distinctUntilChanged((prev, curr) => prev.organization?.id === curr.organization?.id), ), - this.collectionService.decryptedCollections$, + this.collectionService.decryptedCollections$(userId), this.organizationService.memberOrganizations$(userId), this.configService.getFeatureFlag$(FeatureFlag.CreateDefaultLocation), ]), @@ -463,16 +462,11 @@ export class VaultPopupListFiltersService { } return sortDefaultCollections(filtered, orgs, this.i18nService.collator); }), - switchMap((collections) => { - return from(this.collectionService.getAllNested(collections)).pipe( - map( - (nested) => - new DynamicTreeNode({ - fullList: collections, - nestedList: nested, - }), - ), - ); + map((fullList) => { + return new DynamicTreeNode({ + fullList, + nestedList: this.collectionService.getAllNested(fullList), + }); }), map((tree) => tree.nestedList.map((c) => this.convertToChipSelectOption(c, "bwi-collection-shared")), diff --git a/apps/cli/src/commands/get.command.ts b/apps/cli/src/commands/get.command.ts index b20052fbb53..756316cba43 100644 --- a/apps/cli/src/commands/get.command.ts +++ b/apps/cli/src/commands/get.command.ts @@ -24,6 +24,7 @@ import { LoginUriExport } from "@bitwarden/common/models/export/login-uri.export import { LoginExport } from "@bitwarden/common/models/export/login.export"; import { SecureNoteExport } from "@bitwarden/common/models/export/secure-note.export"; import { ErrorResponse } from "@bitwarden/common/models/response/error.response"; +import { getById } from "@bitwarden/common/platform/misc"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { CipherId, OrganizationId, UserId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; @@ -442,8 +443,11 @@ export class GetCommand extends DownloadCommand { private async getCollection(id: string) { let decCollection: CollectionView = null; + const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); if (Utils.isGuid(id)) { - const collection = await this.collectionService.get(id); + const collection = await firstValueFrom( + this.collectionService.encryptedCollections$(activeUserId).pipe(getById(id)), + ); if (collection != null) { const orgKeys = await firstValueFrom(this.keyService.activeUserOrgKeys$); decCollection = await collection.decrypt( @@ -451,7 +455,9 @@ export class GetCommand extends DownloadCommand { ); } } else if (id.trim() !== "") { - let collections = await this.collectionService.getAllDecrypted(); + let collections = await firstValueFrom( + this.collectionService.decryptedCollections$(activeUserId), + ); collections = CliUtils.searchCollections(collections, id); if (collections.length > 1) { return Response.multipleResults(collections.map((c) => c.id)); diff --git a/apps/cli/src/commands/list.command.ts b/apps/cli/src/commands/list.command.ts index 517050728c0..94abd97d6eb 100644 --- a/apps/cli/src/commands/list.command.ts +++ b/apps/cli/src/commands/list.command.ts @@ -20,6 +20,7 @@ import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.servi import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction"; import { SearchService } from "@bitwarden/common/vault/abstractions/search.service"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; +import { KeyService } from "@bitwarden/key-management"; import { CollectionResponse } from "../admin-console/models/response/collection.response"; import { OrganizationUserResponse } from "../admin-console/models/response/organization-user.response"; @@ -42,6 +43,7 @@ export class ListCommand { private apiService: ApiService, private eventCollectionService: EventCollectionService, private accountService: AccountService, + private keyService: KeyService, private cliRestrictedItemTypesService: CliRestrictedItemTypesService, ) {} @@ -158,7 +160,10 @@ export class ListCommand { } private async listCollections(options: Options) { - let collections = await this.collectionService.getAllDecrypted(); + const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); + let collections = await firstValueFrom( + this.collectionService.decryptedCollections$(activeUserId), + ); if (options.organizationId != null) { collections = collections.filter((c) => { @@ -178,13 +183,13 @@ export class ListCommand { } private async listOrganizationCollections(options: Options) { + const userId = await firstValueFrom(getUserId(this.accountService.activeAccount$)); if (options.organizationId == null || options.organizationId === "") { return Response.badRequest("`organizationid` option is required."); } if (!Utils.isGuid(options.organizationId)) { return Response.badRequest("`" + options.organizationId + "` is not a GUID."); } - const userId = await firstValueFrom(getUserId(this.accountService.activeAccount$)); if (!userId) { return Response.badRequest("No user found."); } @@ -207,7 +212,13 @@ export class ListCommand { const collections = response.data .filter((c) => c.organizationId === options.organizationId) .map((r) => new Collection(new CollectionData(r as ApiCollectionDetailsResponse))); - let decCollections = await this.collectionService.decryptMany(collections); + const orgKeys = await firstValueFrom(this.keyService.orgKeys$(userId)); + if (orgKeys == null) { + throw new Error("Organization keys not found."); + } + let decCollections = await firstValueFrom( + this.collectionService.decryptMany$(collections, orgKeys), + ); if (options.search != null && options.search.trim() !== "") { decCollections = CliUtils.searchCollections(decCollections, options.search); } diff --git a/apps/cli/src/oss-serve-configurator.ts b/apps/cli/src/oss-serve-configurator.ts index 14e6ace3b34..848627b703f 100644 --- a/apps/cli/src/oss-serve-configurator.ts +++ b/apps/cli/src/oss-serve-configurator.ts @@ -79,6 +79,7 @@ export class OssServeConfigurator { this.serviceContainer.apiService, this.serviceContainer.eventCollectionService, this.serviceContainer.accountService, + this.serviceContainer.keyService, this.serviceContainer.cliRestrictedItemTypesService, ); this.createCommand = new CreateCommand( diff --git a/apps/cli/src/service-container/service-container.ts b/apps/cli/src/service-container/service-container.ts index 78f961973d9..ddab6fb7cf1 100644 --- a/apps/cli/src/service-container/service-container.ts +++ b/apps/cli/src/service-container/service-container.ts @@ -901,7 +901,6 @@ export class ServiceContainer { this.keyService.clearKeys(userId), this.cipherService.clear(userId), this.folderService.clear(userId), - this.collectionService.clear(userId), ]); await this.stateEventRunnerService.handleEvent("logout", userId as UserId); diff --git a/apps/cli/src/vault.program.ts b/apps/cli/src/vault.program.ts index d5615d0bb1c..2b08bc67ec1 100644 --- a/apps/cli/src/vault.program.ts +++ b/apps/cli/src/vault.program.ts @@ -114,6 +114,7 @@ export class VaultProgram extends BaseProgram { this.serviceContainer.apiService, this.serviceContainer.eventCollectionService, this.serviceContainer.accountService, + this.serviceContainer.keyService, this.serviceContainer.cliRestrictedItemTypesService, ); const response = await command.run(object, cmd); diff --git a/apps/desktop/src/app/app.component.ts b/apps/desktop/src/app/app.component.ts index 10aa7ff9eeb..72bad5befe9 100644 --- a/apps/desktop/src/app/app.component.ts +++ b/apps/desktop/src/app/app.component.ts @@ -677,7 +677,6 @@ export class AppComponent implements OnInit, OnDestroy { await this.keyService.clearKeys(userBeingLoggedOut); await this.cipherService.clear(userBeingLoggedOut); await this.folderService.clear(userBeingLoggedOut); - await this.collectionService.clear(userBeingLoggedOut); await this.vaultTimeoutSettingsService.clear(userBeingLoggedOut); await this.biometricStateService.logout(userBeingLoggedOut); diff --git a/apps/desktop/src/vault/app/vault/vault-v2.component.ts b/apps/desktop/src/vault/app/vault/vault-v2.component.ts index 62ca41a3379..3c4a85f96b7 100644 --- a/apps/desktop/src/vault/app/vault/vault-v2.component.ts +++ b/apps/desktop/src/vault/app/vault/vault-v2.component.ts @@ -30,8 +30,9 @@ import { ConfigService } from "@bitwarden/common/platform/abstractions/config/co import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { getByIds } from "@bitwarden/common/platform/misc"; import { SyncService } from "@bitwarden/common/platform/sync"; -import { CipherId, CollectionId, OrganizationId, UserId } from "@bitwarden/common/types/guid"; +import { CipherId, OrganizationId, 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 { PremiumUpgradePromptService } from "@bitwarden/common/vault/abstractions/premium-upgrade-prompt.service"; @@ -360,7 +361,12 @@ export class VaultV2Component this.allOrganizations = orgs; }); - this.collectionService.decryptedCollections$ + if (!this.activeUserId) { + throw new Error("No user found."); + } + + this.collectionService + .decryptedCollections$(this.activeUserId) .pipe(takeUntil(this.componentIsDestroyed$)) .subscribe((collections) => { this.allCollections = collections; @@ -701,9 +707,17 @@ export class VaultV2Component this.cipherId = null; this.action = "view"; await this.vaultItemsComponent?.refresh().catch(() => {}); + + if (!this.activeUserId) { + throw new Error("No userId provided."); + } + this.collections = await firstValueFrom( - this.collectionService.decryptedCollectionViews$(cipher.collectionIds as CollectionId[]), + this.collectionService + .decryptedCollections$(this.activeUserId) + .pipe(getByIds(cipher.collectionIds)), ); + this.cipherId = cipher.id; this.cipher = cipher; if (this.activeUserId) { diff --git a/apps/web/src/app/admin-console/organizations/collections/vault.component.ts b/apps/web/src/app/admin-console/organizations/collections/vault.component.ts index 47ad93d81e2..5dc217b7a50 100644 --- a/apps/web/src/app/admin-console/organizations/collections/vault.component.ts +++ b/apps/web/src/app/admin-console/organizations/collections/vault.component.ts @@ -29,6 +29,7 @@ import { import { CollectionAdminService, CollectionAdminView, + CollectionService, CollectionView, Unassigned, } from "@bitwarden/admin-console/common"; @@ -264,6 +265,7 @@ export class VaultComponent implements OnInit, OnDestroy { private accountService: AccountService, private billingNotificationService: BillingNotificationService, private organizationWarningsService: OrganizationWarningsService, + private collectionService: CollectionService, ) {} async ngOnInit() { @@ -1133,6 +1135,7 @@ export class VaultComponent implements OnInit, OnDestroy { } try { await this.apiService.deleteCollection(this.organization?.id, collection.id); + await this.collectionService.delete([collection.id as CollectionId], this.userId); this.toastService.showToast({ variant: "success", title: null, diff --git a/apps/web/src/app/admin-console/organizations/manage/groups.component.ts b/apps/web/src/app/admin-console/organizations/manage/groups.component.ts index 6459cd1f857..1c40ec462d3 100644 --- a/apps/web/src/app/admin-console/organizations/manage/groups.component.ts +++ b/apps/web/src/app/admin-console/organizations/manage/groups.component.ts @@ -11,6 +11,7 @@ import { from, lastValueFrom, map, + Observable, switchMap, tap, } from "rxjs"; @@ -25,10 +26,13 @@ import { CollectionView, } from "@bitwarden/admin-console/common"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { ListResponse } from "@bitwarden/common/models/response/list.response"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { DialogService, TableDataSource, ToastService } from "@bitwarden/components"; +import { KeyService } from "@bitwarden/key-management"; import { GroupDetailsView, InternalGroupApiService as GroupService } from "../core"; @@ -100,6 +104,8 @@ export class GroupsComponent { private logService: LogService, private collectionService: CollectionService, private toastService: ToastService, + private keyService: KeyService, + private accountService: AccountService, ) { this.route.params .pipe( @@ -244,16 +250,22 @@ export class GroupsComponent { this.dataSource.data = this.dataSource.data.filter((g) => g !== groupRow); } - private async toCollectionMap(response: ListResponse) { + private toCollectionMap( + response: ListResponse, + ): Observable> { const collections = response.data.map( (r) => new Collection(new CollectionData(r as CollectionDetailsResponse)), ); - const decryptedCollections = await this.collectionService.decryptMany(collections); - // Convert to an object using collection Ids as keys for faster name lookups - const collectionMap: Record = {}; - decryptedCollections.forEach((c) => (collectionMap[c.id] = c)); - - return collectionMap; + return this.accountService.activeAccount$.pipe( + getUserId, + switchMap((userId) => this.keyService.orgKeys$(userId)), + switchMap((orgKeys) => this.collectionService.decryptMany$(collections, orgKeys)), + map((collections) => { + const collectionMap: Record = {}; + collections.forEach((c) => (collectionMap[c.id] = c)); + return collectionMap; + }), + ); } } diff --git a/apps/web/src/app/admin-console/organizations/members/members.component.ts b/apps/web/src/app/admin-console/organizations/members/members.component.ts index 0eec835e15f..77a0cecce8e 100644 --- a/apps/web/src/app/admin-console/organizations/members/members.component.ts +++ b/apps/web/src/app/admin-console/organizations/members/members.component.ts @@ -13,6 +13,7 @@ import { Observable, shareReplay, switchMap, + withLatestFrom, tap, } from "rxjs"; @@ -307,17 +308,27 @@ export class MembersComponent extends BaseMembersComponent * Retrieve a map of all collection IDs <-> names for the organization. */ async getCollectionNameMap() { - const collectionMap = new Map(); - const response = await this.apiService.getCollections(this.organization.id); - - const collections = response.data.map( - (r) => new Collection(new CollectionData(r as CollectionDetailsResponse)), + const response = from(this.apiService.getCollections(this.organization.id)).pipe( + map((res) => + res.data.map((r) => new Collection(new CollectionData(r as CollectionDetailsResponse))), + ), ); - const decryptedCollections = await this.collectionService.decryptMany(collections); - decryptedCollections.forEach((c) => collectionMap.set(c.id, c.name)); + const decryptedCollections$ = this.accountService.activeAccount$.pipe( + getUserId, + switchMap((userId) => this.keyService.orgKeys$(userId)), + withLatestFrom(response), + switchMap(([orgKeys, collections]) => + this.collectionService.decryptMany$(collections, orgKeys), + ), + map((collections) => { + const collectionMap = new Map(); + collections.forEach((c) => collectionMap.set(c.id, c.name)); + return collectionMap; + }), + ); - return collectionMap; + return await firstValueFrom(decryptedCollections$); } removeUser(id: string): Promise { diff --git a/apps/web/src/app/admin-console/organizations/shared/components/collection-dialog/collection-dialog.component.ts b/apps/web/src/app/admin-console/organizations/shared/components/collection-dialog/collection-dialog.component.ts index 8763b75ffca..fabfb65fc6b 100644 --- a/apps/web/src/app/admin-console/organizations/shared/components/collection-dialog/collection-dialog.component.ts +++ b/apps/web/src/app/admin-console/organizations/shared/components/collection-dialog/collection-dialog.component.ts @@ -26,7 +26,6 @@ import { CollectionResponse, CollectionView, CollectionService, - Collection, } from "@bitwarden/admin-console/common"; import { getOrganizationById, @@ -38,6 +37,7 @@ import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { getById } from "@bitwarden/common/platform/misc"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { DIALOG_DATA, @@ -141,7 +141,6 @@ export class CollectionDialogComponent implements OnInit, OnDestroy { protected PermissionMode = PermissionMode; protected showDeleteButton = false; protected showAddAccessWarning = false; - protected collections: Collection[]; protected buttonDisplayName: ButtonType = ButtonType.Save; private orgExceedingCollectionLimit!: Organization; @@ -166,14 +165,12 @@ export class CollectionDialogComponent implements OnInit, OnDestroy { async ngOnInit() { // Opened from the individual vault + const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(map((a) => a?.id))); if (this.params.showOrgSelector) { this.showOrgSelector = true; this.formGroup.controls.selectedOrg.valueChanges .pipe(takeUntil(this.destroy$)) .subscribe((id) => this.loadOrg(id)); - const userId = await firstValueFrom( - this.accountService.activeAccount$.pipe(map((a) => a?.id)), - ); this.organizations$ = this.organizationService.organizations$(userId).pipe( first(), map((orgs) => @@ -195,9 +192,14 @@ export class CollectionDialogComponent implements OnInit, OnDestroy { ); if (isBreadcrumbEventLogsEnabled) { - this.collections = await this.collectionService.getAll(); this.organizationSelected.setAsyncValidators( - freeOrgCollectionLimitValidator(this.organizations$, this.collections, this.i18nService), + freeOrgCollectionLimitValidator( + this.organizations$, + this.collectionService + .encryptedCollections$(userId) + .pipe(map((collections) => collections ?? [])), + this.i18nService, + ), ); this.formGroup.updateValueAndValidity(); } @@ -212,7 +214,7 @@ export class CollectionDialogComponent implements OnInit, OnDestroy { } }), filter(() => this.organizationSelected.errors?.cannotCreateCollections), - switchMap((value) => this.findOrganizationById(value)), + switchMap((organizationId) => this.organizations$.pipe(getById(organizationId))), takeUntil(this.destroy$), ) .subscribe((org) => { @@ -222,11 +224,6 @@ export class CollectionDialogComponent implements OnInit, OnDestroy { }); } - async findOrganizationById(orgId: string): Promise { - const organizations = await firstValueFrom(this.organizations$); - return organizations.find((org) => org.id === orgId); - } - async loadOrg(orgId: string) { const userId = await firstValueFrom(getUserId(this.accountService.activeAccount$)); const organization$ = this.organizationService @@ -413,7 +410,8 @@ export class CollectionDialogComponent implements OnInit, OnDestroy { collectionView.name = this.formGroup.controls.name.value; } - const savedCollection = await this.collectionAdminService.save(collectionView); + const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); + const savedCollection = await this.collectionAdminService.save(collectionView, userId); this.toastService.showToast({ variant: "success", diff --git a/apps/web/src/app/admin-console/organizations/shared/validators/free-org-collection-limit.validator.spec.ts b/apps/web/src/app/admin-console/organizations/shared/validators/free-org-collection-limit.validator.spec.ts index 120a58d6b1a..f9d389c979f 100644 --- a/apps/web/src/app/admin-console/organizations/shared/validators/free-org-collection-limit.validator.spec.ts +++ b/apps/web/src/app/admin-console/organizations/shared/validators/free-org-collection-limit.validator.spec.ts @@ -18,7 +18,7 @@ describe("freeOrgCollectionLimitValidator", () => { it("returns null if organization is not found", async () => { const orgs: Organization[] = []; - const validator = freeOrgCollectionLimitValidator(of(orgs), [], i18nService); + const validator = freeOrgCollectionLimitValidator(of(orgs), of([]), i18nService); const control = new FormControl("org-id"); const result: Observable = validator(control) as Observable; @@ -28,7 +28,7 @@ describe("freeOrgCollectionLimitValidator", () => { }); it("returns null if control is not an instance of FormControl", async () => { - const validator = freeOrgCollectionLimitValidator(of([]), [], i18nService); + const validator = freeOrgCollectionLimitValidator(of([]), of([]), i18nService); const control = {} as AbstractControl; const result: Observable = validator( @@ -40,7 +40,7 @@ describe("freeOrgCollectionLimitValidator", () => { }); it("returns null if control is not provided", async () => { - const validator = freeOrgCollectionLimitValidator(of([]), [], i18nService); + const validator = freeOrgCollectionLimitValidator(of([]), of([]), i18nService); const result: Observable = validator( undefined as any, @@ -53,7 +53,7 @@ describe("freeOrgCollectionLimitValidator", () => { it("returns null if organization has not reached collection limit (Observable)", async () => { const org = { id: "org-id", maxCollections: 2 } as Organization; const collections = [{ organizationId: "org-id" } as Collection]; - const validator = freeOrgCollectionLimitValidator(of([org]), collections, i18nService); + const validator = freeOrgCollectionLimitValidator(of([org]), of(collections), i18nService); const control = new FormControl("org-id"); const result$ = validator(control) as Observable; @@ -65,7 +65,7 @@ describe("freeOrgCollectionLimitValidator", () => { it("returns error if organization has reached collection limit (Observable)", async () => { const org = { id: "org-id", maxCollections: 1 } as Organization; const collections = [{ organizationId: "org-id" } as Collection]; - const validator = freeOrgCollectionLimitValidator(of([org]), collections, i18nService); + const validator = freeOrgCollectionLimitValidator(of([org]), of(collections), i18nService); const control = new FormControl("org-id"); const result$ = validator(control) as Observable; diff --git a/apps/web/src/app/admin-console/organizations/shared/validators/free-org-collection-limit.validator.ts b/apps/web/src/app/admin-console/organizations/shared/validators/free-org-collection-limit.validator.ts index 75919d31c1a..7132428c375 100644 --- a/apps/web/src/app/admin-console/organizations/shared/validators/free-org-collection-limit.validator.ts +++ b/apps/web/src/app/admin-console/organizations/shared/validators/free-org-collection-limit.validator.ts @@ -1,13 +1,14 @@ import { AbstractControl, AsyncValidatorFn, FormControl, ValidationErrors } from "@angular/forms"; -import { map, Observable, of } from "rxjs"; +import { combineLatest, map, Observable, of } from "rxjs"; import { Collection } from "@bitwarden/admin-console/common"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { getById } from "@bitwarden/common/platform/misc"; export function freeOrgCollectionLimitValidator( - orgs: Observable, - collections: Collection[], + organizations$: Observable, + collections$: Observable, i18nService: I18nService, ): AsyncValidatorFn { return (control: AbstractControl): Observable => { @@ -21,15 +22,16 @@ export function freeOrgCollectionLimitValidator( return of(null); } - return orgs.pipe( - map((organizations) => organizations.find((org) => org.id === orgId)), - map((org) => { - if (!org) { + return combineLatest([organizations$.pipe(getById(orgId)), collections$]).pipe( + map(([organization, collections]) => { + if (!organization) { return null; } - const orgCollections = collections.filter((c) => c.organizationId === org.id); - const hasReachedLimit = org.maxCollections === orgCollections.length; + const orgCollections = collections.filter( + (collection: Collection) => collection.organizationId === organization.id, + ); + const hasReachedLimit = organization.maxCollections === orgCollections.length; if (hasReachedLimit) { return { diff --git a/apps/web/src/app/app.component.ts b/apps/web/src/app/app.component.ts index ada73dd0059..ceb2c788e75 100644 --- a/apps/web/src/app/app.component.ts +++ b/apps/web/src/app/app.component.ts @@ -285,7 +285,6 @@ export class AppComponent implements OnDestroy, OnInit { this.keyService.clearKeys(userId), this.cipherService.clear(userId), this.folderService.clear(userId), - this.collectionService.clear(userId), this.biometricStateService.logout(userId), ]); diff --git a/apps/web/src/app/vault/individual-vault/bulk-action-dialogs/bulk-delete-dialog/bulk-delete-dialog.component.ts b/apps/web/src/app/vault/individual-vault/bulk-action-dialogs/bulk-delete-dialog/bulk-delete-dialog.component.ts index 128afdcccfc..78abad1ebf8 100644 --- a/apps/web/src/app/vault/individual-vault/bulk-action-dialogs/bulk-delete-dialog/bulk-delete-dialog.component.ts +++ b/apps/web/src/app/vault/individual-vault/bulk-action-dialogs/bulk-delete-dialog/bulk-delete-dialog.component.ts @@ -9,7 +9,7 @@ import { Organization } from "@bitwarden/common/admin-console/models/domain/orga import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; -import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { CollectionId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { CipherBulkDeleteRequest } from "@bitwarden/common/vault/models/request/cipher-bulk-delete.request"; import { UnionOfValues } from "@bitwarden/common/vault/types/union-of-values"; @@ -68,7 +68,6 @@ export class BulkDeleteDialogComponent { @Inject(DIALOG_DATA) params: BulkDeleteDialogParams, private dialogRef: DialogRef, private cipherService: CipherService, - private platformUtilsService: PlatformUtilsService, private i18nService: I18nService, private apiService: ApiService, private collectionService: CollectionService, @@ -116,7 +115,11 @@ export class BulkDeleteDialogComponent { }); } if (this.collections.length) { - await this.collectionService.delete(this.collections.map((c) => c.id)); + const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); + await this.collectionService.delete( + this.collections.map((c) => c.id as CollectionId), + userId, + ); this.toastService.showToast({ variant: "success", title: null, diff --git a/apps/web/src/app/vault/individual-vault/vault-filter/services/vault-filter.service.spec.ts b/apps/web/src/app/vault/individual-vault/vault-filter/services/vault-filter.service.spec.ts index 93189f2bf1c..b7a19bf2e76 100644 --- a/apps/web/src/app/vault/individual-vault/vault-filter/services/vault-filter.service.spec.ts +++ b/apps/web/src/app/vault/individual-vault/vault-filter/services/vault-filter.service.spec.ts @@ -78,7 +78,7 @@ describe("vault filter service", () => { configService.getFeatureFlag$.mockReturnValue(of(true)); organizationService.memberOrganizations$.mockReturnValue(organizations); folderService.folderViews$.mockReturnValue(folderViews); - collectionService.decryptedCollections$ = collectionViews; + collectionService.decryptedCollections$.mockReturnValue(collectionViews); policyService.policyAppliesToUser$ .calledWith(PolicyType.OrganizationDataOwnership, mockUserId) .mockReturnValue(organizationDataOwnershipPolicy); diff --git a/apps/web/src/app/vault/individual-vault/vault-filter/services/vault-filter.service.ts b/apps/web/src/app/vault/individual-vault/vault-filter/services/vault-filter.service.ts index 1fe618c6c4e..266676e418b 100644 --- a/apps/web/src/app/vault/individual-vault/vault-filter/services/vault-filter.service.ts +++ b/apps/web/src/app/vault/individual-vault/vault-filter/services/vault-filter.service.ts @@ -4,7 +4,6 @@ import { Injectable } from "@angular/core"; import { BehaviorSubject, combineLatest, - combineLatestWith, filter, firstValueFrom, map, @@ -100,13 +99,13 @@ export class VaultFilterService implements VaultFilterServiceAbstraction { map((folders) => this.buildFolderTree(folders)), ); - filteredCollections$: Observable = - this.collectionService.decryptedCollections$.pipe( - combineLatestWith(this._organizationFilter), - switchMap(([collections, org]) => { - return this.filterCollections(collections, org); - }), - ); + filteredCollections$: Observable = combineLatest([ + this.accountService.activeAccount$.pipe( + getUserId, + switchMap((userId) => this.collectionService.decryptedCollections$(userId)), + ), + this._organizationFilter, + ]).pipe(switchMap(([collections, org]) => this.filterCollections(collections, org))); collectionTree$: Observable> = combineLatest([ this.filteredCollections$, diff --git a/apps/web/src/app/vault/individual-vault/vault.component.ts b/apps/web/src/app/vault/individual-vault/vault.component.ts index c8c2f681bb4..09284cec9c9 100644 --- a/apps/web/src/app/vault/individual-vault/vault.component.ts +++ b/apps/web/src/app/vault/individual-vault/vault.component.ts @@ -334,7 +334,8 @@ export class VaultComponent implements OnInit, OnDestr }); const filter$ = this.routedVaultFilterService.filter$; - const allCollections$ = this.collectionService.decryptedCollections$; + + const allCollections$ = this.collectionService.decryptedCollections$(activeUserId); const nestedCollections$ = allCollections$.pipe( map((collections) => getNestedCollectionTree(collections)), ); @@ -861,7 +862,10 @@ export class VaultComponent implements OnInit, OnDestr if (result.collection) { // Update CollectionService with the new collection const c = new CollectionData(result.collection as CollectionDetailsResponse); - await this.collectionService.upsert(c); + const activeUserId = await firstValueFrom( + this.accountService.activeAccount$.pipe(getUserId), + ); + await this.collectionService.upsert(c, activeUserId); } this.refresh(); } @@ -878,20 +882,23 @@ export class VaultComponent implements OnInit, OnDestr }); const result = await lastValueFrom(dialog.closed); + const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); if (result.action === CollectionDialogAction.Saved) { if (result.collection) { // Update CollectionService with the new collection const c = new CollectionData(result.collection as CollectionDetailsResponse); - await this.collectionService.upsert(c); + await this.collectionService.upsert(c, activeUserId); } this.refresh(); } else if (result.action === CollectionDialogAction.Deleted) { - await this.collectionService.delete(result.collection?.id); - this.refresh(); + const parent = this.selectedCollection?.parent; // Navigate away if we deleted the collection we were viewing - if (this.selectedCollection?.node.id === c?.id) { + const navigateAway = this.selectedCollection && this.selectedCollection.node.id === c.id; + await this.collectionService.delete([result.collection?.id as CollectionId], activeUserId); + this.refresh(); + if (navigateAway) { await this.router.navigate([], { - queryParams: { collectionId: this.selectedCollection.parent?.node.id ?? null }, + queryParams: { collectionId: parent?.node.id ?? null }, queryParamsHandling: "merge", replaceUrl: true, }); @@ -916,18 +923,22 @@ export class VaultComponent implements OnInit, OnDestr return; } try { + const parent = this.selectedCollection?.parent; + // Navigate away if we deleted the collection we were viewing + const navigateAway = + this.selectedCollection && this.selectedCollection.node.id === collection.id; await this.apiService.deleteCollection(collection.organizationId, collection.id); - await this.collectionService.delete(collection.id); + const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); + await this.collectionService.delete([collection.id as CollectionId], activeUserId); this.toastService.showToast({ variant: "success", title: null, message: this.i18nService.t("deletedCollectionId", collection.name), }); - // Navigate away if we deleted the collection we were viewing - if (this.selectedCollection?.node.id === collection.id) { + if (navigateAway) { await this.router.navigate([], { - queryParams: { collectionId: this.selectedCollection.parent?.node.id ?? null }, + queryParams: { collectionId: parent?.node.id ?? null }, queryParamsHandling: "merge", replaceUrl: true, }); diff --git a/libs/admin-console/src/common/collections/abstractions/collection-admin.service.ts b/libs/admin-console/src/common/collections/abstractions/collection-admin.service.ts index 36222b16794..083539e9f6e 100644 --- a/libs/admin-console/src/common/collections/abstractions/collection-admin.service.ts +++ b/libs/admin-console/src/common/collections/abstractions/collection-admin.service.ts @@ -1,15 +1,20 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { CollectionDetailsResponse } from "@bitwarden/admin-console/common"; +import { UserId } from "@bitwarden/common/types/guid"; import { CollectionAccessSelectionView, CollectionAdminView } from "../models"; export abstract class CollectionAdminService { - getAll: (organizationId: string) => Promise; - get: (organizationId: string, collectionId: string) => Promise; - save: (collection: CollectionAdminView) => Promise; - delete: (organizationId: string, collectionId: string) => Promise; - bulkAssignAccess: ( + abstract getAll: (organizationId: string) => Promise; + abstract get: ( + organizationId: string, + collectionId: string, + ) => Promise; + abstract save: ( + collection: CollectionAdminView, + userId: UserId, + ) => Promise; + abstract delete: (organizationId: string, collectionId: string) => Promise; + abstract bulkAssignAccess: ( organizationId: string, collectionIds: string[], users: CollectionAccessSelectionView[], diff --git a/libs/admin-console/src/common/collections/abstractions/collection.service.ts b/libs/admin-console/src/common/collections/abstractions/collection.service.ts index 61fc94b271c..e69f96232da 100644 --- a/libs/admin-console/src/common/collections/abstractions/collection.service.ts +++ b/libs/admin-console/src/common/collections/abstractions/collection.service.ts @@ -1,5 +1,3 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { Observable } from "rxjs"; import { CollectionId, OrganizationId, UserId } from "@bitwarden/common/types/guid"; @@ -9,27 +7,25 @@ import { TreeNode } from "@bitwarden/common/vault/models/domain/tree-node"; import { CollectionData, Collection, CollectionView } from "../models"; export abstract class CollectionService { - encryptedCollections$: Observable; - decryptedCollections$: Observable; - - clearActiveUserCache: () => Promise; - encrypt: (model: CollectionView) => Promise; - decryptedCollectionViews$: (ids: CollectionId[]) => Observable; + abstract encryptedCollections$: (userId: UserId) => Observable; + abstract decryptedCollections$: (userId: UserId) => Observable; + abstract upsert: (collection: CollectionData, userId: UserId) => Promise; + abstract replace: (collections: { [id: string]: CollectionData }, userId: UserId) => Promise; /** - * @deprecated This method will soon be made private - * See PM-12375 + * @deprecated This method will soon be made private, use `decryptedCollections$` instead. */ - decryptMany: ( + abstract decryptMany$: ( collections: Collection[], - orgKeys?: Record, - ) => Promise; - get: (id: string) => Promise; - getAll: () => Promise; - getAllDecrypted: () => Promise; - getAllNested: (collections?: CollectionView[]) => Promise[]>; - getNested: (id: string) => Promise>; - upsert: (collection: CollectionData | CollectionData[]) => Promise; - replace: (collections: { [id: string]: CollectionData }, userId: UserId) => Promise; - clear: (userId?: string) => Promise; - delete: (id: string | string[]) => Promise; + orgKeys: Record, + ) => Observable; + abstract delete: (ids: CollectionId[], userId: UserId) => Promise; + abstract encrypt: (model: CollectionView, userId: UserId) => Promise; + /** + * Transforms the input CollectionViews into TreeNodes + */ + abstract getAllNested: (collections: CollectionView[]) => TreeNode[]; + /* + * Transforms the input CollectionViews into TreeNodes and then returns the Treenode with the specified id + */ + abstract getNested: (collections: CollectionView[], id: string) => TreeNode; } diff --git a/libs/admin-console/src/common/collections/abstractions/vnext-collection.service.ts b/libs/admin-console/src/common/collections/abstractions/vnext-collection.service.ts deleted file mode 100644 index e1b2a5759a1..00000000000 --- a/libs/admin-console/src/common/collections/abstractions/vnext-collection.service.ts +++ /dev/null @@ -1,43 +0,0 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore -import { Observable } from "rxjs"; - -import { OrganizationId, UserId } from "@bitwarden/common/types/guid"; -import { OrgKey } from "@bitwarden/common/types/key"; -import { TreeNode } from "@bitwarden/common/vault/models/domain/tree-node"; - -import { CollectionData, Collection, CollectionView } from "../models"; - -export abstract class vNextCollectionService { - encryptedCollections$: (userId: UserId) => Observable; - decryptedCollections$: (userId: UserId) => Observable; - upsert: (collection: CollectionData | CollectionData[], userId: UserId) => Promise; - replace: (collections: { [id: string]: CollectionData }, userId: UserId) => Promise; - /** - * Clear decrypted state without affecting encrypted state. - * Used for locking the vault. - */ - clearDecryptedState: (userId: UserId) => Promise; - /** - * Clear decrypted and encrypted state. - * Used for logging out. - */ - clear: (userId: UserId) => Promise; - delete: (id: string | string[], userId: UserId) => Promise; - encrypt: (model: CollectionView) => Promise; - /** - * @deprecated This method will soon be made private, use `decryptedCollections$` instead. - */ - decryptMany: ( - collections: Collection[], - orgKeys?: Record | null, - ) => Promise; - /** - * Transforms the input CollectionViews into TreeNodes - */ - getAllNested: (collections: CollectionView[]) => TreeNode[]; - /** - * Transforms the input CollectionViews into TreeNodes and then returns the Treenode with the specified id - */ - getNested: (collections: CollectionView[], id: string) => TreeNode; -} diff --git a/libs/admin-console/src/common/collections/models/collection.data.ts b/libs/admin-console/src/common/collections/models/collection.data.ts index b28a066509c..27c5c0c0efa 100644 --- a/libs/admin-console/src/common/collections/models/collection.data.ts +++ b/libs/admin-console/src/common/collections/models/collection.data.ts @@ -26,7 +26,10 @@ export class CollectionData { this.type = response.type; } - static fromJSON(obj: Jsonify) { + static fromJSON(obj: Jsonify): CollectionData | null { + if (obj == null) { + return null; + } return Object.assign(new CollectionData(new CollectionDetailsResponse({})), obj); } } diff --git a/libs/admin-console/src/common/collections/models/collection.ts b/libs/admin-console/src/common/collections/models/collection.ts index 75d68222b38..7bbd018fa96 100644 --- a/libs/admin-console/src/common/collections/models/collection.ts +++ b/libs/admin-console/src/common/collections/models/collection.ts @@ -1,7 +1,5 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string"; -import Domain from "@bitwarden/common/platform/models/domain/domain-base"; +import Domain, { EncryptableKeys } from "@bitwarden/common/platform/models/domain/domain-base"; import { OrgKey } from "@bitwarden/common/types/key"; import { CollectionData } from "./collection.data"; @@ -15,16 +13,16 @@ export const CollectionTypes = { export type CollectionType = (typeof CollectionTypes)[keyof typeof CollectionTypes]; export class Collection extends Domain { - id: string; - organizationId: string; - name: EncString; - externalId: string; - readOnly: boolean; - hidePasswords: boolean; - manage: boolean; - type: CollectionType; + id: string | undefined; + organizationId: string | undefined; + name: EncString | undefined; + externalId: string | undefined; + readOnly: boolean = false; + hidePasswords: boolean = false; + manage: boolean = false; + type: CollectionType = CollectionTypes.SharedCollection; - constructor(obj?: CollectionData) { + constructor(obj?: CollectionData | null) { super(); if (obj == null) { return; @@ -51,8 +49,8 @@ export class Collection extends Domain { return this.decryptObj( this, new CollectionView(this), - ["name"], - this.organizationId, + ["name"] as EncryptableKeys[], + this.organizationId ?? null, orgKey, ); } diff --git a/libs/admin-console/src/common/collections/models/collection.view.ts b/libs/admin-console/src/common/collections/models/collection.view.ts index bce1d558f96..f75ff565100 100644 --- a/libs/admin-console/src/common/collections/models/collection.view.ts +++ b/libs/admin-console/src/common/collections/models/collection.view.ts @@ -12,7 +12,7 @@ export const NestingDelimiter = "/"; export class CollectionView implements View, ITreeNodeObject { id: string | undefined; organizationId: string | undefined; - name: string | undefined; + name: string = ""; externalId: string | undefined; // readOnly applies to the items within a collection readOnly: boolean = false; diff --git a/libs/admin-console/src/common/collections/services/collection.state.ts b/libs/admin-console/src/common/collections/services/collection.state.ts new file mode 100644 index 00000000000..ebb620c2354 --- /dev/null +++ b/libs/admin-console/src/common/collections/services/collection.state.ts @@ -0,0 +1,28 @@ +import { Jsonify } from "type-fest"; + +import { + COLLECTION_DISK, + COLLECTION_MEMORY, + UserKeyDefinition, +} from "@bitwarden/common/platform/state"; +import { CollectionId } from "@bitwarden/common/types/guid"; + +import { CollectionData, CollectionView } from "../models"; + +export const ENCRYPTED_COLLECTION_DATA_KEY = UserKeyDefinition.record< + CollectionData | null, + CollectionId +>(COLLECTION_DISK, "collections", { + deserializer: (jsonData: Jsonify) => CollectionData.fromJSON(jsonData), + clearOn: ["logout"], +}); + +export const DECRYPTED_COLLECTION_DATA_KEY = new UserKeyDefinition( + COLLECTION_MEMORY, + "decryptedCollections", + { + deserializer: (obj: Jsonify) => + obj?.map((f) => CollectionView.fromJSON(f)) ?? null, + clearOn: ["logout", "lock"], + }, +); diff --git a/libs/admin-console/src/common/collections/services/default-collection-admin.service.ts b/libs/admin-console/src/common/collections/services/default-collection-admin.service.ts index 325b17cbd56..a00be4e5174 100644 --- a/libs/admin-console/src/common/collections/services/default-collection-admin.service.ts +++ b/libs/admin-console/src/common/collections/services/default-collection-admin.service.ts @@ -1,9 +1,11 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore + import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { SelectionReadOnlyRequest } from "@bitwarden/common/admin-console/models/request/selection-read-only.request"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string"; +import { CollectionId, UserId } from "@bitwarden/common/types/guid"; import { KeyService } from "@bitwarden/key-management"; import { CollectionAdminService, CollectionService } from "../abstractions"; @@ -55,7 +57,7 @@ export class DefaultCollectionAdminService implements CollectionAdminService { return view; } - async save(collection: CollectionAdminView): Promise { + async save(collection: CollectionAdminView, userId: UserId): Promise { const request = await this.encrypt(collection); let response: CollectionDetailsResponse; @@ -71,9 +73,9 @@ export class DefaultCollectionAdminService implements CollectionAdminService { } if (response.assigned) { - await this.collectionService.upsert(new CollectionData(response)); + await this.collectionService.upsert(new CollectionData(response), userId); } else { - await this.collectionService.delete(collection.id); + await this.collectionService.delete([collection.id as CollectionId], userId); } return response; diff --git a/libs/admin-console/src/common/collections/services/default-collection.service.spec.ts b/libs/admin-console/src/common/collections/services/default-collection.service.spec.ts index 57bed5e4ca5..c2c0332a486 100644 --- a/libs/admin-console/src/common/collections/services/default-collection.service.spec.ts +++ b/libs/admin-console/src/common/collections/services/default-collection.service.spec.ts @@ -1,10 +1,11 @@ -import { mock } from "jest-mock-extended"; -import { firstValueFrom, of } from "rxjs"; +import { mock, MockProxy } from "jest-mock-extended"; +import { combineLatest, first, firstValueFrom, of, ReplaySubject, takeWhile } from "rxjs"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; +import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { ContainerService } from "@bitwarden/common/platform/services/container.service"; import { FakeStateProvider, @@ -16,124 +17,382 @@ import { CollectionId, OrganizationId, UserId } from "@bitwarden/common/types/gu import { OrgKey } from "@bitwarden/common/types/key"; import { KeyService } from "@bitwarden/key-management"; -import { CollectionData } from "../models"; +import { CollectionData, CollectionView } from "../models"; -import { - DefaultCollectionService, - ENCRYPTED_COLLECTION_DATA_KEY, -} from "./default-collection.service"; +import { DECRYPTED_COLLECTION_DATA_KEY, ENCRYPTED_COLLECTION_DATA_KEY } from "./collection.state"; +import { DefaultCollectionService } from "./default-collection.service"; describe("DefaultCollectionService", () => { + let keyService: MockProxy; + let encryptService: MockProxy; + let i18nService: MockProxy; + let stateProvider: FakeStateProvider; + + let userId: UserId; + + let cryptoKeys: ReplaySubject | null>; + + let collectionService: DefaultCollectionService; + + beforeEach(() => { + userId = Utils.newGuid() as UserId; + + keyService = mock(); + encryptService = mock(); + i18nService = mock(); + stateProvider = new FakeStateProvider(mockAccountServiceWith(userId)); + + cryptoKeys = new ReplaySubject(1); + keyService.orgKeys$.mockReturnValue(cryptoKeys); + + // Set up mock decryption + encryptService.decryptString + .calledWith(expect.any(EncString), expect.any(SymmetricCryptoKey)) + .mockImplementation((encString, key) => + Promise.resolve(encString.data.replace("ENC_", "DEC_")), + ); + + (window as any).bitwardenContainerService = new ContainerService(keyService, encryptService); + + // Arrange i18nService so that sorting algorithm doesn't throw + i18nService.collator = null; + + collectionService = new DefaultCollectionService( + keyService, + encryptService, + i18nService, + stateProvider, + ); + }); + afterEach(() => { delete (window as any).bitwardenContainerService; }); describe("decryptedCollections$", () => { it("emits decrypted collections from state", async () => { - // Arrange test collections + // Arrange test data const org1 = Utils.newGuid() as OrganizationId; - const org2 = Utils.newGuid() as OrganizationId; - + const orgKey1 = makeSymmetricCryptoKey(64, 1); const collection1 = collectionDataFactory(org1); + + const org2 = Utils.newGuid() as OrganizationId; + const orgKey2 = makeSymmetricCryptoKey(64, 2); const collection2 = collectionDataFactory(org2); - // Arrange state provider - const fakeStateProvider = mockStateProvider(); - await fakeStateProvider.setUserState(ENCRYPTED_COLLECTION_DATA_KEY, { - [collection1.id]: collection1, - [collection2.id]: collection2, + // Arrange dependencies + await setEncryptedState([collection1, collection2]); + cryptoKeys.next({ + [org1]: orgKey1, + [org2]: orgKey2, }); - // Arrange cryptoService - orgKeys and mock decryption - const cryptoService = mockCryptoService(); - cryptoService.orgKeys$.mockReturnValue( - of({ - [org1]: makeSymmetricCryptoKey(), - [org2]: makeSymmetricCryptoKey(), - }), - ); + const result = await firstValueFrom(collectionService.decryptedCollections$(userId)); - const collectionService = new DefaultCollectionService( - cryptoService, - mock(), - mockI18nService(), - fakeStateProvider, - ); - - const result = await firstValueFrom(collectionService.decryptedCollections$); + // Assert emitted values 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", - }); + expect(result).toContainPartialObjects([ + { + id: collection1.id, + name: "DEC_NAME_" + collection1.id, + }, + { + id: collection2.id, + name: "DEC_NAME_" + collection2.id, + }, + ]); + + // Assert that the correct org keys were used for each encrypted string + // This should be replaced with decryptString when the platform PR (https://github.com/bitwarden/clients/pull/14544) is merged + expect(encryptService.decryptString).toHaveBeenCalledWith( + expect.objectContaining(new EncString(collection1.name)), + orgKey1, + ); + expect(encryptService.decryptString).toHaveBeenCalledWith( + expect.objectContaining(new EncString(collection2.name)), + orgKey2, + ); + }); + + it("emits decrypted collections from in-memory state when available", async () => { + // Arrange test data + const org1 = Utils.newGuid() as OrganizationId; + const collection1 = collectionViewDataFactory(org1); + + const org2 = Utils.newGuid() as OrganizationId; + const collection2 = collectionViewDataFactory(org2); + + await setDecryptedState([collection1, collection2]); + + const result = await firstValueFrom(collectionService.decryptedCollections$(userId)); + + // Assert emitted values + expect(result.length).toBe(2); + expect(result).toContainPartialObjects([ + { + id: collection1.id, + name: "DEC_NAME_" + collection1.id, + }, + { + id: collection2.id, + name: "DEC_NAME_" + collection2.id, + }, + ]); + + // Ensure that the returned data came from the in-memory state, rather than from decryption. + expect(encryptService.decryptString).not.toHaveBeenCalled(); }); it("handles null collection state", async () => { - // Arrange test collections + // Arrange dependencies + await setEncryptedState(null); + cryptoKeys.next({}); + + const encryptedCollections = await firstValueFrom( + collectionService.encryptedCollections$(userId), + ); + + expect(encryptedCollections).toBe(null); + }); + + it("handles undefined orgKeys", (done) => { + // Arrange test data const org1 = Utils.newGuid() as OrganizationId; + const collection1 = collectionDataFactory(org1); + const org2 = Utils.newGuid() as OrganizationId; + const collection2 = collectionDataFactory(org2); - // Arrange state provider - const fakeStateProvider = mockStateProvider(); - await fakeStateProvider.setUserState(ENCRYPTED_COLLECTION_DATA_KEY, null); + // Emit a non-null value after the first undefined value has propagated + // This will cause the collections to emit, calling done() + cryptoKeys.pipe(first()).subscribe((val) => { + cryptoKeys.next({}); + }); - // Arrange cryptoService - orgKeys and mock decryption - const cryptoService = mockCryptoService(); - cryptoService.orgKeys$.mockReturnValue( - of({ - [org1]: makeSymmetricCryptoKey(), - [org2]: makeSymmetricCryptoKey(), - }), - ); + collectionService + .decryptedCollections$(userId) + .pipe(takeWhile((val) => val.length != 2)) + .subscribe({ complete: () => done() }); - const collectionService = new DefaultCollectionService( - cryptoService, - mock(), - mockI18nService(), - fakeStateProvider, - ); + // Arrange dependencies + void setEncryptedState([collection1, collection2]).then(() => { + // Act: emit undefined + cryptoKeys.next(undefined); + keyService.activeUserOrgKeys$ = of(undefined); + }); + }); - const decryptedCollections = await firstValueFrom(collectionService.decryptedCollections$); - expect(decryptedCollections.length).toBe(0); + it("Decrypts one time for multiple simultaneous callers", async () => { + const decryptedMock: CollectionView[] = [{ id: "col1" }] as CollectionView[]; + const decryptManySpy = jest + .spyOn(collectionService, "decryptMany$") + .mockReturnValue(of(decryptedMock)); - const encryptedCollections = await firstValueFrom(collectionService.encryptedCollections$); - expect(encryptedCollections.length).toBe(0); + jest + .spyOn(collectionService as any, "encryptedCollections$") + .mockReturnValue(of([{ id: "enc1" }])); + jest.spyOn(keyService, "orgKeys$").mockReturnValue(of({ key: "fake-key" })); + + // Simulate multiple subscribers + const obs1 = collectionService.decryptedCollections$(userId); + const obs2 = collectionService.decryptedCollections$(userId); + const obs3 = collectionService.decryptedCollections$(userId); + + await firstValueFrom(combineLatest([obs1, obs2, obs3])); + + // Expect decryptMany$ to be called only once + expect(decryptManySpy).toHaveBeenCalledTimes(1); }); }); + + describe("encryptedCollections$", () => { + it("emits encrypted collections from state", async () => { + // Arrange test data + const collection1 = collectionDataFactory(); + const collection2 = collectionDataFactory(); + + // Arrange dependencies + await setEncryptedState([collection1, collection2]); + + const result = await firstValueFrom(collectionService.encryptedCollections$(userId)); + + expect(result!.length).toBe(2); + expect(result).toContainPartialObjects([ + { + id: collection1.id, + name: makeEncString("ENC_NAME_" + collection1.id), + }, + { + id: collection2.id, + name: makeEncString("ENC_NAME_" + collection2.id), + }, + ]); + }); + + it("handles null collection state", async () => { + await setEncryptedState(null); + + const decryptedCollections = await firstValueFrom( + collectionService.encryptedCollections$(userId), + ); + expect(decryptedCollections).toBe(null); + }); + }); + + describe("upsert", () => { + it("upserts to existing collections", async () => { + const org1 = Utils.newGuid() as OrganizationId; + const orgKey1 = makeSymmetricCryptoKey(64, 1); + const collection1 = collectionDataFactory(org1); + + await setEncryptedState([collection1]); + cryptoKeys.next({ + [collection1.organizationId]: orgKey1, + }); + + const updatedCollection1 = Object.assign(new CollectionData({} as any), collection1, { + name: makeEncString("UPDATED_ENC_NAME_" + collection1.id).encryptedString, + }); + + await collectionService.upsert(updatedCollection1, userId); + + const encryptedResult = await firstValueFrom(collectionService.encryptedCollections$(userId)); + + expect(encryptedResult!.length).toBe(1); + expect(encryptedResult).toContainPartialObjects([ + { + id: collection1.id, + name: makeEncString("UPDATED_ENC_NAME_" + collection1.id), + }, + ]); + + const decryptedResult = await firstValueFrom(collectionService.decryptedCollections$(userId)); + expect(decryptedResult.length).toBe(1); + expect(decryptedResult).toContainPartialObjects([ + { + id: collection1.id, + name: "UPDATED_DEC_NAME_" + collection1.id, + }, + ]); + }); + + it("upserts to a null state", async () => { + const org1 = Utils.newGuid() as OrganizationId; + const orgKey1 = makeSymmetricCryptoKey(64, 1); + const collection1 = collectionDataFactory(org1); + + cryptoKeys.next({ + [collection1.organizationId]: orgKey1, + }); + + await setEncryptedState(null); + + await collectionService.upsert(collection1, userId); + + const encryptedResult = await firstValueFrom(collectionService.encryptedCollections$(userId)); + expect(encryptedResult!.length).toBe(1); + expect(encryptedResult).toContainPartialObjects([ + { + id: collection1.id, + name: makeEncString("ENC_NAME_" + collection1.id), + }, + ]); + + const decryptedResult = await firstValueFrom(collectionService.decryptedCollections$(userId)); + expect(decryptedResult.length).toBe(1); + expect(decryptedResult).toContainPartialObjects([ + { + id: collection1.id, + name: "DEC_NAME_" + collection1.id, + }, + ]); + }); + }); + + describe("replace", () => { + it("replaces all collections", async () => { + await setEncryptedState([collectionDataFactory(), collectionDataFactory()]); + + const newCollection3 = collectionDataFactory(); + await collectionService.replace( + { + [newCollection3.id]: newCollection3, + }, + userId, + ); + + const result = await firstValueFrom(collectionService.encryptedCollections$(userId)); + expect(result!.length).toBe(1); + expect(result).toContainPartialObjects([ + { + id: newCollection3.id, + name: makeEncString("ENC_NAME_" + newCollection3.id), + }, + ]); + }); + }); + + describe("delete", () => { + it("deletes a collection", async () => { + const collection1 = collectionDataFactory(); + const collection2 = collectionDataFactory(); + await setEncryptedState([collection1, collection2]); + + await collectionService.delete([collection1.id], userId); + + const result = await firstValueFrom(collectionService.encryptedCollections$(userId)); + expect(result!.length).toEqual(1); + expect(result![0]).toMatchObject({ id: collection2.id }); + }); + + it("deletes several collections", async () => { + const collection1 = collectionDataFactory(); + const collection2 = collectionDataFactory(); + const collection3 = collectionDataFactory(); + await setEncryptedState([collection1, collection2, collection3]); + + await collectionService.delete([collection1.id, collection3.id], userId); + + const result = await firstValueFrom(collectionService.encryptedCollections$(userId)); + expect(result!.length).toEqual(1); + expect(result![0]).toMatchObject({ id: collection2.id }); + }); + + it("handles null collections", async () => { + const collection1 = collectionDataFactory(); + await setEncryptedState(null); + + await collectionService.delete([collection1.id], userId); + + const result = await firstValueFrom(collectionService.encryptedCollections$(userId)); + expect(result!.length).toEqual(0); + }); + }); + + const setEncryptedState = (collectionData: CollectionData[] | null) => + stateProvider.setUserState( + ENCRYPTED_COLLECTION_DATA_KEY, + collectionData == null ? null : Object.fromEntries(collectionData.map((c) => [c.id, c])), + userId, + ); + + const setDecryptedState = (collectionViews: CollectionView[] | null) => + stateProvider.setUserState(DECRYPTED_COLLECTION_DATA_KEY, collectionViews, userId); }); -const mockI18nService = () => { - const i18nService = mock(); - 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 keyService = mock(); - const encryptService = mock(); - encryptService.decryptString - .calledWith(expect.any(EncString), expect.anything()) - .mockResolvedValue("DECRYPTED_STRING"); - - (window as any).bitwardenContainerService = new ContainerService(keyService, encryptService); - - return keyService; -}; - -const collectionDataFactory = (orgId: OrganizationId) => { +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; + collection.organizationId = orgId ?? (Utils.newGuid() as OrganizationId); + collection.name = makeEncString("ENC_NAME_" + collection.id).encryptedString ?? ""; return collection; }; + +function collectionViewDataFactory(orgId?: OrganizationId): CollectionView { + const collectionView = new CollectionView(); + collectionView.id = Utils.newGuid() as CollectionId; + collectionView.organizationId = orgId ?? (Utils.newGuid() as OrganizationId); + collectionView.name = "DEC_NAME_" + collectionView.id; + return collectionView; +} diff --git a/libs/admin-console/src/common/collections/services/default-collection.service.ts b/libs/admin-console/src/common/collections/services/default-collection.service.ts index a1dd0419e2c..4978b06df35 100644 --- a/libs/admin-console/src/common/collections/services/default-collection.service.ts +++ b/libs/admin-console/src/common/collections/services/default-collection.service.ts @@ -1,113 +1,193 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore -import { combineLatest, firstValueFrom, map, Observable, of, shareReplay, switchMap } from "rxjs"; -import { Jsonify } from "type-fest"; +import { + combineLatest, + delayWhen, + filter, + firstValueFrom, + from, + map, + NEVER, + Observable, + of, + shareReplay, + switchMap, +} from "rxjs"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { - ActiveUserState, - COLLECTION_DATA, - DeriveDefinition, - DerivedState, - StateProvider, - UserKeyDefinition, -} from "@bitwarden/common/platform/state"; +import { SingleUserState, StateProvider } from "@bitwarden/common/platform/state"; import { CollectionId, OrganizationId, UserId } from "@bitwarden/common/types/guid"; import { OrgKey } from "@bitwarden/common/types/key"; import { TreeNode } from "@bitwarden/common/vault/models/domain/tree-node"; import { ServiceUtils } from "@bitwarden/common/vault/service-utils"; import { KeyService } from "@bitwarden/key-management"; -import { CollectionService } from "../abstractions"; +import { CollectionService } from "../abstractions/collection.service"; import { Collection, CollectionData, CollectionView } from "../models"; -export const ENCRYPTED_COLLECTION_DATA_KEY = UserKeyDefinition.record( - COLLECTION_DATA, - "collections", - { - deserializer: (jsonData: Jsonify) => CollectionData.fromJSON(jsonData), - clearOn: ["logout"], - }, -); - -const DECRYPTED_COLLECTION_DATA_KEY = new DeriveDefinition< - [Record, Record], - CollectionView[], - { collectionService: DefaultCollectionService } ->(COLLECTION_DATA, "decryptedCollections", { - deserializer: (obj) => obj.map((collection) => CollectionView.fromJSON(collection)), - derive: async ([collections, orgKeys], { collectionService }) => { - if (collections == null) { - return []; - } - - const data = Object.values(collections).map((c) => new Collection(c)); - return await collectionService.decryptMany(data, orgKeys); - }, -}); +import { DECRYPTED_COLLECTION_DATA_KEY, ENCRYPTED_COLLECTION_DATA_KEY } from "./collection.state"; const NestingDelimiter = "/"; export class DefaultCollectionService implements CollectionService { - private encryptedCollectionDataState: ActiveUserState>; - encryptedCollections$: Observable; - private decryptedCollectionDataState: DerivedState; - decryptedCollections$: Observable; - - decryptedCollectionViews$(ids: CollectionId[]): Observable { - return this.decryptedCollections$.pipe( - map((collections) => collections.filter((c) => ids.includes(c.id as CollectionId))), - ); - } - constructor( private keyService: KeyService, private encryptService: EncryptService, private i18nService: I18nService, protected stateProvider: StateProvider, - ) { - this.encryptedCollectionDataState = this.stateProvider.getActive(ENCRYPTED_COLLECTION_DATA_KEY); + ) {} - this.encryptedCollections$ = this.encryptedCollectionDataState.state$.pipe( + private collectionViewCache = new Map>(); + + /** + * @returns a SingleUserState for encrypted collection data. + */ + private encryptedState( + userId: UserId, + ): SingleUserState> { + return this.stateProvider.getUser(userId, ENCRYPTED_COLLECTION_DATA_KEY); + } + + /** + * @returns a SingleUserState for decrypted collection data. + */ + private decryptedState(userId: UserId): SingleUserState { + return this.stateProvider.getUser(userId, DECRYPTED_COLLECTION_DATA_KEY); + } + + encryptedCollections$(userId: UserId): Observable { + return this.encryptedState(userId).state$.pipe( map((collections) => { if (collections == null) { - return []; + return null; } return Object.values(collections).map((c) => new Collection(c)); }), ); + } - const encryptedCollectionsWithKeys = this.encryptedCollectionDataState.combinedState$.pipe( - switchMap(([userId, collectionData]) => - combineLatest([of(collectionData), this.keyService.orgKeys$(userId)]), + decryptedCollections$(userId: UserId): Observable { + const cachedResult = this.collectionViewCache.get(userId); + if (cachedResult) { + return cachedResult; + } + + const result$ = this.decryptedState(userId).state$.pipe( + switchMap((decryptedState) => { + // If decrypted state is already populated, return that + if (decryptedState !== null) { + return of(decryptedState ?? []); + } + + return this.initializeDecryptedState(userId).pipe(switchMap(() => NEVER)); + }), + shareReplay({ bufferSize: 1, refCount: true }), + ); + + this.collectionViewCache.set(userId, result$); + return result$; + } + + private initializeDecryptedState(userId: UserId): Observable { + return combineLatest([ + this.encryptedCollections$(userId), + this.keyService.orgKeys$(userId).pipe(filter((orgKeys) => !!orgKeys)), + ]).pipe( + switchMap(([collections, orgKeys]) => + this.decryptMany$(collections, orgKeys).pipe( + delayWhen((collections) => this.setDecryptedCollections(collections, userId)), + ), ), - shareReplay({ refCount: false, bufferSize: 1 }), ); - - this.decryptedCollectionDataState = this.stateProvider.getDerived( - encryptedCollectionsWithKeys, - DECRYPTED_COLLECTION_DATA_KEY, - { collectionService: this }, - ); - - this.decryptedCollections$ = this.decryptedCollectionDataState.state$; } - async clearActiveUserCache(): Promise { - await this.decryptedCollectionDataState.forceValue(null); + async upsert(toUpdate: CollectionData, userId: UserId): Promise { + if (toUpdate == null) { + return; + } + await this.encryptedState(userId).update((collections) => { + if (collections == null) { + collections = {}; + } + collections[toUpdate.id] = toUpdate; + + return collections; + }); + + const decryptedCollections = await firstValueFrom( + this.keyService.orgKeys$(userId).pipe( + switchMap((orgKeys) => { + if (!orgKeys) { + throw new Error("No key for this collection's organization."); + } + return this.decryptMany$([new Collection(toUpdate)], orgKeys); + }), + ), + ); + + await this.decryptedState(userId).update((collections) => { + if (collections == null) { + collections = []; + } + + if (!decryptedCollections?.length) { + return collections; + } + + const decryptedCollection = decryptedCollections[0]; + const existingIndex = collections.findIndex((collection) => collection.id == toUpdate.id); + if (existingIndex >= 0) { + collections[existingIndex] = decryptedCollection; + } else { + collections.push(decryptedCollection); + } + + return collections; + }); } - async encrypt(model: CollectionView): Promise { + async replace(collections: Record, userId: UserId): Promise { + await this.encryptedState(userId).update(() => collections); + await this.decryptedState(userId).update(() => null); + } + + async delete(ids: CollectionId[], userId: UserId): Promise { + await this.encryptedState(userId).update((collections) => { + if (collections == null) { + collections = {}; + } + ids.forEach((i) => { + delete collections[i]; + }); + return collections; + }); + + await this.decryptedState(userId).update((collections) => { + if (collections == null) { + collections = []; + } + ids.forEach((i) => { + if (collections?.length) { + collections = collections.filter((c) => c.id != i) ?? []; + } + }); + return collections; + }); + } + + async encrypt(model: CollectionView, userId: UserId): Promise { if (model.organizationId == null) { throw new Error("Collection has no organization id."); } - const key = await this.keyService.getOrgKey(model.organizationId); - if (key == null) { - throw new Error("No key for this collection's organization."); - } + + const key = await firstValueFrom( + this.keyService.orgKeys$(userId).pipe( + filter((orgKeys) => !!orgKeys), + map((k) => k[model.organizationId as OrganizationId]), + ), + ); + const collection = new Collection(); collection.id = model.id; collection.organizationId = model.organizationId; @@ -117,58 +197,37 @@ export class DefaultCollectionService implements CollectionService { return collection; } - // TODO: this should be private and orgKeys should be required. + // TODO: this should be private. // See https://bitwarden.atlassian.net/browse/PM-12375 - async decryptMany( - collections: Collection[], - orgKeys?: Record, - ): Promise { - if (collections == null || collections.length === 0) { - return []; + decryptMany$( + collections: Collection[] | null, + orgKeys: Record, + ): Observable { + if (collections === null || collections.length == 0 || orgKeys === null) { + return of([]); } - const decCollections: CollectionView[] = []; - orgKeys ??= await firstValueFrom(this.keyService.activeUserOrgKeys$); + const decCollections: Observable[] = []; - const promises: Promise[] = []; collections.forEach((collection) => { - promises.push( - collection - .decrypt(orgKeys[collection.organizationId as OrganizationId]) - .then((c) => decCollections.push(c)), + decCollections.push( + from(collection.decrypt(orgKeys[collection.organizationId as OrganizationId])), ); }); - await Promise.all(promises); - return decCollections.sort(Utils.getSortFunction(this.i18nService, "name")); - } - async get(id: string): Promise { - return ( - (await firstValueFrom( - this.encryptedCollections$.pipe(map((cs) => cs.find((c) => c.id === id))), - )) ?? null + return combineLatest(decCollections).pipe( + map((collections) => collections.sort(Utils.getSortFunction(this.i18nService, "name"))), ); } - async getAll(): Promise { - return await firstValueFrom(this.encryptedCollections$); - } - - async getAllDecrypted(): Promise { - return await firstValueFrom(this.decryptedCollections$); - } - - async getAllNested(collections: CollectionView[] = null): Promise[]> { - if (collections == null) { - collections = await this.getAllDecrypted(); - } + getAllNested(collections: CollectionView[]): TreeNode[] { const nodes: TreeNode[] = []; collections.forEach((c) => { const collectionCopy = new CollectionView(); collectionCopy.id = c.id; collectionCopy.organizationId = c.organizationId; const parts = c.name != null ? c.name.replace(/^\/+|\/+$/g, "").split(NestingDelimiter) : []; - ServiceUtils.nestedTraverse(nodes, 0, parts, collectionCopy, null, NestingDelimiter); + ServiceUtils.nestedTraverse(nodes, 0, parts, collectionCopy, undefined, NestingDelimiter); }); return nodes; } @@ -177,58 +236,23 @@ export class DefaultCollectionService implements CollectionService { * @deprecated August 30 2022: Moved to new Vault Filter Service * Remove when Desktop and Browser are updated */ - async getNested(id: string): Promise> { - const collections = await this.getAllNested(); - return ServiceUtils.getTreeNodeObjectFromList(collections, id) as TreeNode; + getNested(collections: CollectionView[], id: string): TreeNode { + const nestedCollections = this.getAllNested(collections); + return ServiceUtils.getTreeNodeObjectFromList( + nestedCollections, + id, + ) as TreeNode; } - async upsert(toUpdate: CollectionData | CollectionData[]): Promise { - if (toUpdate == null) { - return; - } - await this.encryptedCollectionDataState.update((collections) => { - if (collections == null) { - collections = {}; - } - if (Array.isArray(toUpdate)) { - toUpdate.forEach((c) => { - collections[c.id] = c; - }); - } else { - collections[toUpdate.id] = toUpdate; - } - return collections; - }); - } - - async replace(collections: Record, userId: UserId): Promise { - await this.stateProvider - .getUser(userId, ENCRYPTED_COLLECTION_DATA_KEY) - .update(() => collections); - } - - async clear(userId?: UserId): Promise { - if (userId == null) { - await this.encryptedCollectionDataState.update(() => null); - await this.decryptedCollectionDataState.forceValue(null); - } else { - await this.stateProvider.getUser(userId, ENCRYPTED_COLLECTION_DATA_KEY).update(() => null); - } - } - - async delete(id: CollectionId | CollectionId[]): Promise { - await this.encryptedCollectionDataState.update((collections) => { - if (collections == null) { - collections = {}; - } - if (typeof id === "string") { - delete collections[id]; - } else { - (id as CollectionId[]).forEach((i) => { - delete collections[i]; - }); - } - return collections; - }); + /** + * Sets the decrypted collections state for a user. + * @param collections the decrypted collections + * @param userId the user id + */ + private async setDecryptedCollections( + collections: CollectionView[], + userId: UserId, + ): Promise { + await this.stateProvider.setUserState(DECRYPTED_COLLECTION_DATA_KEY, collections, userId); } } diff --git a/libs/admin-console/src/common/collections/services/default-vnext-collection.service.spec.ts b/libs/admin-console/src/common/collections/services/default-vnext-collection.service.spec.ts deleted file mode 100644 index 256157a03c1..00000000000 --- a/libs/admin-console/src/common/collections/services/default-vnext-collection.service.spec.ts +++ /dev/null @@ -1,345 +0,0 @@ -import { mock, MockProxy } from "jest-mock-extended"; -import { first, firstValueFrom, of, ReplaySubject, takeWhile } from "rxjs"; - -import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; -import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string"; -import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; -import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; -import { ContainerService } from "@bitwarden/common/platform/services/container.service"; -import { - FakeStateProvider, - makeEncString, - makeSymmetricCryptoKey, - mockAccountServiceWith, -} from "@bitwarden/common/spec"; -import { CollectionId, OrganizationId, UserId } from "@bitwarden/common/types/guid"; -import { OrgKey } from "@bitwarden/common/types/key"; -import { KeyService } from "@bitwarden/key-management"; - -import { CollectionData } from "../models"; - -import { DefaultvNextCollectionService } from "./default-vnext-collection.service"; -import { ENCRYPTED_COLLECTION_DATA_KEY } from "./vnext-collection.state"; - -describe("DefaultvNextCollectionService", () => { - let keyService: MockProxy; - let encryptService: MockProxy; - let i18nService: MockProxy; - let stateProvider: FakeStateProvider; - - let userId: UserId; - - let cryptoKeys: ReplaySubject | null>; - - let collectionService: DefaultvNextCollectionService; - - beforeEach(() => { - userId = Utils.newGuid() as UserId; - - keyService = mock(); - encryptService = mock(); - i18nService = mock(); - stateProvider = new FakeStateProvider(mockAccountServiceWith(userId)); - - cryptoKeys = new ReplaySubject(1); - keyService.orgKeys$.mockReturnValue(cryptoKeys); - - // Set up mock decryption - encryptService.decryptString - .calledWith(expect.any(EncString), expect.any(SymmetricCryptoKey)) - .mockImplementation((encString, key) => - Promise.resolve(encString.data.replace("ENC_", "DEC_")), - ); - - (window as any).bitwardenContainerService = new ContainerService(keyService, encryptService); - - // Arrange i18nService so that sorting algorithm doesn't throw - i18nService.collator = null; - - collectionService = new DefaultvNextCollectionService( - keyService, - encryptService, - i18nService, - stateProvider, - ); - }); - - afterEach(() => { - delete (window as any).bitwardenContainerService; - }); - - describe("decryptedCollections$", () => { - it("emits decrypted collections from state", async () => { - // Arrange test data - const org1 = Utils.newGuid() as OrganizationId; - const orgKey1 = makeSymmetricCryptoKey(64, 1); - const collection1 = collectionDataFactory(org1); - - const org2 = Utils.newGuid() as OrganizationId; - const orgKey2 = makeSymmetricCryptoKey(64, 2); - const collection2 = collectionDataFactory(org2); - - // Arrange dependencies - await setEncryptedState([collection1, collection2]); - cryptoKeys.next({ - [org1]: orgKey1, - [org2]: orgKey2, - }); - - const result = await firstValueFrom(collectionService.decryptedCollections$(userId)); - - // Assert emitted values - expect(result.length).toBe(2); - expect(result).toContainPartialObjects([ - { - id: collection1.id, - name: "DEC_NAME_" + collection1.id, - }, - { - id: collection2.id, - name: "DEC_NAME_" + collection2.id, - }, - ]); - - // Assert that the correct org keys were used for each encrypted string - // This should be replaced with decryptString when the platform PR (https://github.com/bitwarden/clients/pull/14544) is merged - expect(encryptService.decryptString).toHaveBeenCalledWith( - expect.objectContaining(new EncString(collection1.name)), - orgKey1, - ); - expect(encryptService.decryptString).toHaveBeenCalledWith( - expect.objectContaining(new EncString(collection2.name)), - orgKey2, - ); - }); - - it("handles null collection state", async () => { - // Arrange dependencies - await setEncryptedState(null); - cryptoKeys.next({}); - - const encryptedCollections = await firstValueFrom( - collectionService.encryptedCollections$(userId), - ); - - expect(encryptedCollections.length).toBe(0); - }); - - it("handles undefined orgKeys", (done) => { - // Arrange test data - const org1 = Utils.newGuid() as OrganizationId; - const collection1 = collectionDataFactory(org1); - - const org2 = Utils.newGuid() as OrganizationId; - const collection2 = collectionDataFactory(org2); - - // Emit a non-null value after the first undefined value has propagated - // This will cause the collections to emit, calling done() - cryptoKeys.pipe(first()).subscribe((val) => { - cryptoKeys.next({}); - }); - - collectionService - .decryptedCollections$(userId) - .pipe(takeWhile((val) => val.length != 2)) - .subscribe({ complete: () => done() }); - - // Arrange dependencies - void setEncryptedState([collection1, collection2]).then(() => { - // Act: emit undefined - cryptoKeys.next(undefined); - keyService.activeUserOrgKeys$ = of(undefined); - }); - }); - }); - - describe("encryptedCollections$", () => { - it("emits encrypted collections from state", async () => { - // Arrange test data - const collection1 = collectionDataFactory(); - const collection2 = collectionDataFactory(); - - // Arrange dependencies - await setEncryptedState([collection1, collection2]); - - const result = await firstValueFrom(collectionService.encryptedCollections$(userId)); - - expect(result.length).toBe(2); - expect(result).toContainPartialObjects([ - { - id: collection1.id, - name: makeEncString("ENC_NAME_" + collection1.id), - }, - { - id: collection2.id, - name: makeEncString("ENC_NAME_" + collection2.id), - }, - ]); - }); - - it("handles null collection state", async () => { - await setEncryptedState(null); - - const decryptedCollections = await firstValueFrom( - collectionService.encryptedCollections$(userId), - ); - expect(decryptedCollections.length).toBe(0); - }); - }); - - describe("upsert", () => { - it("upserts to existing collections", async () => { - const collection1 = collectionDataFactory(); - const collection2 = collectionDataFactory(); - - await setEncryptedState([collection1, collection2]); - - const updatedCollection1 = Object.assign(new CollectionData({} as any), collection1, { - name: makeEncString("UPDATED_ENC_NAME_" + collection1.id).encryptedString, - }); - const newCollection3 = collectionDataFactory(); - - await collectionService.upsert([updatedCollection1, newCollection3], userId); - - const result = await firstValueFrom(collectionService.encryptedCollections$(userId)); - expect(result.length).toBe(3); - expect(result).toContainPartialObjects([ - { - id: collection1.id, - name: makeEncString("UPDATED_ENC_NAME_" + collection1.id), - }, - { - id: collection2.id, - name: makeEncString("ENC_NAME_" + collection2.id), - }, - { - id: newCollection3.id, - name: makeEncString("ENC_NAME_" + newCollection3.id), - }, - ]); - }); - - it("upserts to a null state", async () => { - const collection1 = collectionDataFactory(); - - await setEncryptedState(null); - - await collectionService.upsert(collection1, userId); - - const result = await firstValueFrom(collectionService.encryptedCollections$(userId)); - expect(result.length).toBe(1); - expect(result).toContainPartialObjects([ - { - id: collection1.id, - name: makeEncString("ENC_NAME_" + collection1.id), - }, - ]); - }); - }); - - describe("replace", () => { - it("replaces all collections", async () => { - await setEncryptedState([collectionDataFactory(), collectionDataFactory()]); - - const newCollection3 = collectionDataFactory(); - await collectionService.replace( - { - [newCollection3.id]: newCollection3, - }, - userId, - ); - - const result = await firstValueFrom(collectionService.encryptedCollections$(userId)); - expect(result.length).toBe(1); - expect(result).toContainPartialObjects([ - { - id: newCollection3.id, - name: makeEncString("ENC_NAME_" + newCollection3.id), - }, - ]); - }); - }); - - it("clearDecryptedState", async () => { - await setEncryptedState([collectionDataFactory(), collectionDataFactory()]); - - await collectionService.clearDecryptedState(userId); - - // Encrypted state remains - const encryptedState = await firstValueFrom(collectionService.encryptedCollections$(userId)); - expect(encryptedState.length).toEqual(2); - - // Decrypted state is cleared - const decryptedState = await firstValueFrom(collectionService.decryptedCollections$(userId)); - expect(decryptedState.length).toEqual(0); - }); - - it("clear", async () => { - await setEncryptedState([collectionDataFactory(), collectionDataFactory()]); - cryptoKeys.next({}); - - await collectionService.clear(userId); - - // Encrypted state is cleared - const encryptedState = await firstValueFrom(collectionService.encryptedCollections$(userId)); - expect(encryptedState.length).toEqual(0); - - // Decrypted state is cleared - const decryptedState = await firstValueFrom(collectionService.decryptedCollections$(userId)); - expect(decryptedState.length).toEqual(0); - }); - - describe("delete", () => { - it("deletes a collection", async () => { - const collection1 = collectionDataFactory(); - const collection2 = collectionDataFactory(); - await setEncryptedState([collection1, collection2]); - - await collectionService.delete(collection1.id, userId); - - const result = await firstValueFrom(collectionService.encryptedCollections$(userId)); - expect(result.length).toEqual(1); - expect(result[0]).toMatchObject({ id: collection2.id }); - }); - - it("deletes several collections", async () => { - const collection1 = collectionDataFactory(); - const collection2 = collectionDataFactory(); - const collection3 = collectionDataFactory(); - await setEncryptedState([collection1, collection2, collection3]); - - await collectionService.delete([collection1.id, collection3.id], userId); - - const result = await firstValueFrom(collectionService.encryptedCollections$(userId)); - expect(result.length).toEqual(1); - expect(result[0]).toMatchObject({ id: collection2.id }); - }); - - it("handles null collections", async () => { - const collection1 = collectionDataFactory(); - await setEncryptedState(null); - - await collectionService.delete(collection1.id, userId); - - const result = await firstValueFrom(collectionService.encryptedCollections$(userId)); - expect(result.length).toEqual(0); - }); - }); - - const setEncryptedState = (collectionData: CollectionData[] | null) => - stateProvider.setUserState( - ENCRYPTED_COLLECTION_DATA_KEY, - collectionData == null ? null : Object.fromEntries(collectionData.map((c) => [c.id, c])), - userId, - ); -}); - -const collectionDataFactory = (orgId?: OrganizationId) => { - const collection = new CollectionData({} as any); - collection.id = Utils.newGuid() as CollectionId; - collection.organizationId = orgId ?? (Utils.newGuid() as OrganizationId); - collection.name = makeEncString("ENC_NAME_" + collection.id).encryptedString; - - return collection; -}; diff --git a/libs/admin-console/src/common/collections/services/default-vnext-collection.service.ts b/libs/admin-console/src/common/collections/services/default-vnext-collection.service.ts deleted file mode 100644 index 4dcda795afe..00000000000 --- a/libs/admin-console/src/common/collections/services/default-vnext-collection.service.ts +++ /dev/null @@ -1,194 +0,0 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore -import { combineLatest, filter, firstValueFrom, map } from "rxjs"; - -import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; -import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; -import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { StateProvider, DerivedState } from "@bitwarden/common/platform/state"; -import { CollectionId, OrganizationId, UserId } from "@bitwarden/common/types/guid"; -import { OrgKey } from "@bitwarden/common/types/key"; -import { TreeNode } from "@bitwarden/common/vault/models/domain/tree-node"; -import { ServiceUtils } from "@bitwarden/common/vault/service-utils"; -import { KeyService } from "@bitwarden/key-management"; - -import { vNextCollectionService } from "../abstractions/vnext-collection.service"; -import { Collection, CollectionData, CollectionView } from "../models"; - -import { - DECRYPTED_COLLECTION_DATA_KEY, - ENCRYPTED_COLLECTION_DATA_KEY, -} from "./vnext-collection.state"; - -const NestingDelimiter = "/"; - -export class DefaultvNextCollectionService implements vNextCollectionService { - constructor( - private keyService: KeyService, - private encryptService: EncryptService, - private i18nService: I18nService, - protected stateProvider: StateProvider, - ) {} - - encryptedCollections$(userId: UserId) { - return this.encryptedState(userId).state$.pipe( - map((collections) => { - if (collections == null) { - return []; - } - - return Object.values(collections).map((c) => new Collection(c)); - }), - ); - } - - decryptedCollections$(userId: UserId) { - return this.decryptedState(userId).state$.pipe(map((collections) => collections ?? [])); - } - - async upsert(toUpdate: CollectionData | CollectionData[], userId: UserId): Promise { - if (toUpdate == null) { - return; - } - await this.encryptedState(userId).update((collections) => { - if (collections == null) { - collections = {}; - } - if (Array.isArray(toUpdate)) { - toUpdate.forEach((c) => { - collections[c.id] = c; - }); - } else { - collections[toUpdate.id] = toUpdate; - } - return collections; - }); - } - - async replace(collections: Record, userId: UserId): Promise { - await this.encryptedState(userId).update(() => collections); - } - - async clearDecryptedState(userId: UserId): Promise { - if (userId == null) { - throw new Error("User ID is required."); - } - - await this.decryptedState(userId).forceValue([]); - } - - async clear(userId: UserId): Promise { - await this.encryptedState(userId).update(() => null); - // This will propagate from the encrypted state update, but by doing it explicitly - // the promise doesn't resolve until the update is complete. - await this.decryptedState(userId).forceValue([]); - } - - async delete(id: CollectionId | CollectionId[], userId: UserId): Promise { - await this.encryptedState(userId).update((collections) => { - if (collections == null) { - collections = {}; - } - if (typeof id === "string") { - delete collections[id]; - } else { - (id as CollectionId[]).forEach((i) => { - delete collections[i]; - }); - } - return collections; - }); - } - - async encrypt(model: CollectionView): Promise { - if (model.organizationId == null) { - throw new Error("Collection has no organization id."); - } - const key = await this.keyService.getOrgKey(model.organizationId); - if (key == null) { - throw new Error("No key for this collection's organization."); - } - const collection = new Collection(); - collection.id = model.id; - collection.organizationId = model.organizationId; - collection.readOnly = model.readOnly; - collection.externalId = model.externalId; - collection.name = await this.encryptService.encryptString(model.name, key); - return collection; - } - - // TODO: this should be private and orgKeys should be required. - // See https://bitwarden.atlassian.net/browse/PM-12375 - async decryptMany( - collections: Collection[], - orgKeys?: Record | null, - ): Promise { - if (collections == null || collections.length === 0) { - return []; - } - const decCollections: CollectionView[] = []; - - orgKeys ??= await firstValueFrom(this.keyService.activeUserOrgKeys$); - - const promises: Promise[] = []; - collections.forEach((collection) => { - promises.push( - collection - .decrypt(orgKeys[collection.organizationId as OrganizationId]) - .then((c) => decCollections.push(c)), - ); - }); - await Promise.all(promises); - return decCollections.sort(Utils.getSortFunction(this.i18nService, "name")); - } - - getAllNested(collections: CollectionView[]): TreeNode[] { - const nodes: TreeNode[] = []; - collections.forEach((c) => { - const collectionCopy = new CollectionView(); - collectionCopy.id = c.id; - collectionCopy.organizationId = c.organizationId; - const parts = c.name != null ? c.name.replace(/^\/+|\/+$/g, "").split(NestingDelimiter) : []; - ServiceUtils.nestedTraverse(nodes, 0, parts, collectionCopy, undefined, NestingDelimiter); - }); - return nodes; - } - - /** - * @deprecated August 30 2022: Moved to new Vault Filter Service - * Remove when Desktop and Browser are updated - */ - getNested(collections: CollectionView[], id: string): TreeNode { - const nestedCollections = this.getAllNested(collections); - return ServiceUtils.getTreeNodeObjectFromList( - nestedCollections, - id, - ) as TreeNode; - } - - /** - * @returns a SingleUserState for encrypted collection data. - */ - private encryptedState(userId: UserId) { - return this.stateProvider.getUser(userId, ENCRYPTED_COLLECTION_DATA_KEY); - } - - /** - * @returns a SingleUserState for decrypted collection data. - */ - private decryptedState(userId: UserId): DerivedState { - const encryptedCollectionsWithKeys$ = combineLatest([ - this.encryptedCollections$(userId), - // orgKeys$ can emit null during brief moments on unlock and lock/logout, we want to ignore those intermediate states - this.keyService.orgKeys$(userId).pipe(filter((orgKeys) => orgKeys != null)), - ]); - - return this.stateProvider.getDerived( - encryptedCollectionsWithKeys$, - DECRYPTED_COLLECTION_DATA_KEY, - { - collectionService: this, - }, - ); - } -} diff --git a/libs/admin-console/src/common/collections/services/vnext-collection.state.ts b/libs/admin-console/src/common/collections/services/vnext-collection.state.ts deleted file mode 100644 index 331c80436f7..00000000000 --- a/libs/admin-console/src/common/collections/services/vnext-collection.state.ts +++ /dev/null @@ -1,36 +0,0 @@ -import { Jsonify } from "type-fest"; - -import { - COLLECTION_DATA, - DeriveDefinition, - UserKeyDefinition, -} from "@bitwarden/common/platform/state"; -import { CollectionId, OrganizationId } from "@bitwarden/common/types/guid"; -import { OrgKey } from "@bitwarden/common/types/key"; - -import { vNextCollectionService } from "../abstractions/vnext-collection.service"; -import { Collection, CollectionData, CollectionView } from "../models"; - -export const ENCRYPTED_COLLECTION_DATA_KEY = UserKeyDefinition.record( - COLLECTION_DATA, - "collections", - { - deserializer: (jsonData: Jsonify) => CollectionData.fromJSON(jsonData), - clearOn: ["logout"], - }, -); - -export const DECRYPTED_COLLECTION_DATA_KEY = new DeriveDefinition< - [Collection[], Record | null], - CollectionView[], - { collectionService: vNextCollectionService } ->(COLLECTION_DATA, "decryptedCollections", { - deserializer: (obj) => obj.map((collection) => CollectionView.fromJSON(collection)), - derive: async ([collections, orgKeys], { collectionService }) => { - if (collections == null) { - return []; - } - - return await collectionService.decryptMany(collections, orgKeys); - }, -}); diff --git a/libs/angular/src/vault/services/custom-nudges-services/empty-vault-nudge.service.ts b/libs/angular/src/vault/services/custom-nudges-services/empty-vault-nudge.service.ts index d90ae06a75f..57df2d03398 100644 --- a/libs/angular/src/vault/services/custom-nudges-services/empty-vault-nudge.service.ts +++ b/libs/angular/src/vault/services/custom-nudges-services/empty-vault-nudge.service.ts @@ -27,7 +27,7 @@ export class EmptyVaultNudgeService extends DefaultSingleNudgeService { this.getNudgeStatus$(nudgeType, userId), this.cipherService.cipherListViews$(userId), this.organizationService.organizations$(userId), - this.collectionService.decryptedCollections$, + this.collectionService.decryptedCollections$(userId), ]).pipe( switchMap(([nudgeStatus, ciphers, orgs, collections]) => { const vaultHasContents = !(ciphers == null || ciphers.length === 0); diff --git a/libs/angular/src/vault/services/custom-nudges-services/vault-settings-import-nudge.service.ts b/libs/angular/src/vault/services/custom-nudges-services/vault-settings-import-nudge.service.ts index df0403ba4ab..2529fc40b73 100644 --- a/libs/angular/src/vault/services/custom-nudges-services/vault-settings-import-nudge.service.ts +++ b/libs/angular/src/vault/services/custom-nudges-services/vault-settings-import-nudge.service.ts @@ -27,7 +27,7 @@ export class VaultSettingsImportNudgeService extends DefaultSingleNudgeService { this.getNudgeStatus$(nudgeType, userId), this.cipherService.cipherViews$(userId), this.organizationService.organizations$(userId), - this.collectionService.decryptedCollections$, + this.collectionService.decryptedCollections$(userId), ]).pipe( switchMap(([nudgeStatus, ciphers, orgs, collections]) => { const vaultHasMoreThanOneItem = (ciphers?.length ?? 0) > 1; diff --git a/libs/angular/src/vault/vault-filter/services/vault-filter.service.ts b/libs/angular/src/vault/vault-filter/services/vault-filter.service.ts index 0d633be868e..9bc10e5ffc5 100644 --- a/libs/angular/src/vault/vault-filter/services/vault-filter.service.ts +++ b/libs/angular/src/vault/vault-filter/services/vault-filter.service.ts @@ -109,7 +109,12 @@ export class VaultFilterService implements DeprecatedVaultFilterServiceAbstracti } async buildCollections(organizationId?: string): Promise> { - const storedCollections = await this.collectionService.getAllDecrypted(); + const storedCollections = await firstValueFrom( + this.accountService.activeAccount$.pipe( + getUserId, + switchMap((userId) => this.collectionService.decryptedCollections$(userId)), + ), + ); const orgs = await this.buildOrganizations(); const defaulCollectionsFlagEnabled = await this.configService.getFeatureFlag( FeatureFlag.CreateDefaultLocation, diff --git a/libs/common/src/key-management/vault-timeout/services/vault-timeout.service.ts b/libs/common/src/key-management/vault-timeout/services/vault-timeout.service.ts index b5ee6a1fc0f..6d71bad0b0a 100644 --- a/libs/common/src/key-management/vault-timeout/services/vault-timeout.service.ts +++ b/libs/common/src/key-management/vault-timeout/services/vault-timeout.service.ts @@ -143,10 +143,6 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction { ), ); - if (userId == null || userId === currentUserId) { - await this.collectionService.clearActiveUserCache(); - } - await this.searchService.clearIndex(lockingUserId); await this.folderService.clearDecryptedFolderState(lockingUserId); diff --git a/libs/common/src/platform/misc/rxjs-operators.ts b/libs/common/src/platform/misc/rxjs-operators.ts index 689b928cd29..423bcbb790f 100644 --- a/libs/common/src/platform/misc/rxjs-operators.ts +++ b/libs/common/src/platform/misc/rxjs-operators.ts @@ -13,9 +13,9 @@ export const getById = (id: TId) => * @param id The IDs of the objects to return. * @returns An array containing objects with matching IDs, or an empty array if there are no matching objects. */ -export const getByIds = (ids: TId[]) => { - const idSet = new Set(ids); +export const getByIds = (ids: TId[]) => { + const idSet = new Set(ids.filter((id) => id != null)); return map((objects) => { - return objects.filter((o) => idSet.has(o.id)); + return objects.filter((o) => o.id && idSet.has(o.id)); }); }; diff --git a/libs/common/src/platform/models/domain/domain-base.ts b/libs/common/src/platform/models/domain/domain-base.ts index 08f5aca3a3c..58282a665af 100644 --- a/libs/common/src/platform/models/domain/domain-base.ts +++ b/libs/common/src/platform/models/domain/domain-base.ts @@ -14,7 +14,7 @@ export type DecryptedObject< > = Record & Omit; // extracts shared keys from the domain and view types -type EncryptableKeys = (keyof D & +export type EncryptableKeys = (keyof D & ConditionalKeys) & (keyof V & ConditionalKeys); diff --git a/libs/common/src/platform/state/state-definitions.ts b/libs/common/src/platform/state/state-definitions.ts index a1c3ee35c5c..5fcb39e0356 100644 --- a/libs/common/src/platform/state/state-definitions.ts +++ b/libs/common/src/platform/state/state-definitions.ts @@ -164,9 +164,13 @@ export const SEND_ACCESS_AUTH_MEMORY = new StateDefinition("sendAccessAuth", "me // Vault -export const COLLECTION_DATA = new StateDefinition("collection", "disk", { +export const COLLECTION_DISK = new StateDefinition("collection", "disk", { web: "memory", }); +export const COLLECTION_MEMORY = new StateDefinition("decryptedCollections", "memory", { + browser: "memory-large-object", +}); + export const FOLDER_DISK = new StateDefinition("folder", "disk", { web: "memory" }); export const FOLDER_MEMORY = new StateDefinition("decryptedFolders", "memory", { browser: "memory-large-object", diff --git a/libs/common/src/platform/sync/core-sync.service.ts b/libs/common/src/platform/sync/core-sync.service.ts index 63f9ab17fb3..40419a343da 100644 --- a/libs/common/src/platform/sync/core-sync.service.ts +++ b/libs/common/src/platform/sync/core-sync.service.ts @@ -172,7 +172,11 @@ export abstract class CoreSyncService implements SyncService { notification.collectionIds != null && notification.collectionIds.length > 0 ) { - const collections = await this.collectionService.getAll(); + const collections = await firstValueFrom( + this.collectionService + .encryptedCollections$(userId) + .pipe(map((collections) => collections ?? [])), + ); if (collections != null) { for (let i = 0; i < collections.length; i++) { if (notification.collectionIds.indexOf(collections[i].id) > -1) { diff --git a/libs/common/src/vault/services/cipher-authorization.service.spec.ts b/libs/common/src/vault/services/cipher-authorization.service.spec.ts index 43e68bfc71f..78fe6f18913 100644 --- a/libs/common/src/vault/services/cipher-authorization.service.spec.ts +++ b/libs/common/src/vault/services/cipher-authorization.service.spec.ts @@ -119,7 +119,7 @@ describe("CipherAuthorizationService", () => { cipherAuthorizationService.canRestoreCipher$(cipher, false).subscribe((result) => { expect(result).toBe(false); - expect(mockCollectionService.decryptedCollectionViews$).not.toHaveBeenCalled(); + expect(mockCollectionService.decryptedCollections$).not.toHaveBeenCalled(); done(); }); }); @@ -133,7 +133,7 @@ describe("CipherAuthorizationService", () => { cipherAuthorizationService.canRestoreCipher$(cipher, false).subscribe((result) => { expect(result).toBe(true); - expect(mockCollectionService.decryptedCollectionViews$).not.toHaveBeenCalled(); + expect(mockCollectionService.decryptedCollections$).not.toHaveBeenCalled(); done(); }); }); @@ -198,6 +198,7 @@ describe("CipherAuthorizationService", () => { cipherAuthorizationService.canDeleteCipher$(cipher, false).subscribe((result) => { expect(result).toBe(false); + expect(mockCollectionService.decryptedCollections$).not.toHaveBeenCalled(); done(); }); }); @@ -251,7 +252,7 @@ describe("CipherAuthorizationService", () => { createMockCollection("col1", true), createMockCollection("col2", false), ]; - mockCollectionService.decryptedCollectionViews$.mockReturnValue( + mockCollectionService.decryptedCollections$.mockReturnValue( of(allCollections as CollectionView[]), ); @@ -270,7 +271,7 @@ describe("CipherAuthorizationService", () => { createMockCollection("col1", false), createMockCollection("col2", false), ]; - mockCollectionService.decryptedCollectionViews$.mockReturnValue( + mockCollectionService.decryptedCollections$.mockReturnValue( of(allCollections as CollectionView[]), ); diff --git a/libs/common/src/vault/services/cipher-authorization.service.ts b/libs/common/src/vault/services/cipher-authorization.service.ts index 2933e94c302..06177629de5 100644 --- a/libs/common/src/vault/services/cipher-authorization.service.ts +++ b/libs/common/src/vault/services/cipher-authorization.service.ts @@ -1,11 +1,11 @@ -import { map, Observable, of, shareReplay, switchMap } from "rxjs"; +import { combineLatest, map, Observable, of, shareReplay, switchMap } from "rxjs"; // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. // eslint-disable-next-line no-restricted-imports import { CollectionService } from "@bitwarden/admin-console/common"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; -import { CollectionId } from "@bitwarden/common/types/guid"; +import { getByIds } from "@bitwarden/common/platform/misc"; import { getUserId } from "../../auth/services/account.service"; import { CipherLike } from "../types/cipher-like"; @@ -125,8 +125,11 @@ export class DefaultCipherAuthorizationService implements CipherAuthorizationSer return of(true); } - return this.organization$(cipher).pipe( - switchMap((organization) => { + return combineLatest([ + this.organization$(cipher), + this.accountService.activeAccount$.pipe(getUserId), + ]).pipe( + switchMap(([organization, userId]) => { // Admins and custom users can always clone when in the Admin Console if ( isAdminConsoleAction && @@ -136,9 +139,10 @@ export class DefaultCipherAuthorizationService implements CipherAuthorizationSer return of(true); } - return this.collectionService - .decryptedCollectionViews$(cipher.collectionIds as CollectionId[]) - .pipe(map((allCollections) => allCollections.some((collection) => collection.manage))); + return this.collectionService.decryptedCollections$(userId).pipe( + getByIds(cipher.collectionIds), + map((allCollections) => allCollections.some((collection) => collection.manage)), + ); }), shareReplay({ bufferSize: 1, refCount: false }), ); diff --git a/libs/importer/src/components/import.component.ts b/libs/importer/src/components/import.component.ts index 7bac6b0e0a5..63b35e979bd 100644 --- a/libs/importer/src/components/import.component.ts +++ b/libs/importer/src/components/import.component.ts @@ -300,7 +300,7 @@ export class ImportComponent implements OnInit, OnDestroy, AfterViewInit { // Retrieve all organizations a user is a member of and has collections they can manage const userId = await firstValueFrom(getUserId(this.accountService.activeAccount$)); this.organizations$ = this.organizationService.memberOrganizations$(userId).pipe( - combineLatestWith(this.collectionService.decryptedCollections$), + combineLatestWith(this.collectionService.decryptedCollections$(userId)), map(([organizations, collections]) => organizations .filter((org) => collections.some((c) => c.organizationId === org.id && c.manage)) @@ -318,15 +318,15 @@ export class ImportComponent implements OnInit, OnDestroy, AfterViewInit { } if (value) { - this.collections$ = Utils.asyncToObservable(() => - this.collectionService - .getAllDecrypted() - .then((decryptedCollections) => + this.collections$ = this.collectionService + .decryptedCollections$(userId) + .pipe( + map((decryptedCollections) => decryptedCollections .filter((c2) => c2.organizationId === value && c2.manage) .sort(Utils.getSortFunction(this.i18nService, "name")), ), - ); + ); } }); this.formGroup.controls.vaultSelector.setValue("myVault"); diff --git a/libs/importer/src/services/import.service.ts b/libs/importer/src/services/import.service.ts index c6bff607633..5be597c0591 100644 --- a/libs/importer/src/services/import.service.ts +++ b/libs/importer/src/services/import.service.ts @@ -406,7 +406,7 @@ export class ImportService implements ImportServiceAbstraction { if (importResult.collections != null) { for (let i = 0; i < importResult.collections.length; i++) { importResult.collections[i].organizationId = organizationId; - const c = await this.collectionService.encrypt(importResult.collections[i]); + const c = await this.collectionService.encrypt(importResult.collections[i], activeUserId); request.collections.push(new CollectionWithIdRequest(c)); } } diff --git a/libs/tools/export/vault-export/vault-export-core/src/services/org-vault-export.service.ts b/libs/tools/export/vault-export/vault-export-core/src/services/org-vault-export.service.ts index 61fbcd261f4..8a518cb1304 100644 --- a/libs/tools/export/vault-export/vault-export-core/src/services/org-vault-export.service.ts +++ b/libs/tools/export/vault-export/vault-export-core/src/services/org-vault-export.service.ts @@ -1,7 +1,7 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore import * as papa from "papaparse"; -import { firstValueFrom } from "rxjs"; +import { firstValueFrom, map } from "rxjs"; import { CollectionService, @@ -225,15 +225,8 @@ export class OrganizationVaultExportService ): Promise { let decCiphers: CipherView[] = []; let allDecCiphers: CipherView[] = []; - let decCollections: CollectionView[] = []; const promises = []; - promises.push( - this.collectionService.getAllDecrypted().then(async (collections) => { - decCollections = collections.filter((c) => c.organizationId == organizationId && c.manage); - }), - ); - promises.push( this.cipherService.getAllDecrypted(activeUserId).then((ciphers) => { allDecCiphers = ciphers; @@ -241,6 +234,16 @@ export class OrganizationVaultExportService ); await Promise.all(promises); + const decCollections: CollectionView[] = await firstValueFrom( + this.collectionService + .decryptedCollections$(activeUserId) + .pipe( + map((collections) => + collections.filter((c) => c.organizationId == organizationId && c.manage), + ), + ), + ); + const restrictions = await firstValueFrom(this.restrictedItemTypesService.restricted$); decCiphers = allDecCiphers.filter( @@ -263,15 +266,8 @@ export class OrganizationVaultExportService ): Promise { let encCiphers: Cipher[] = []; let allCiphers: Cipher[] = []; - let encCollections: Collection[] = []; const promises = []; - promises.push( - this.collectionService.getAll().then((collections) => { - encCollections = collections.filter((c) => c.organizationId == organizationId && c.manage); - }), - ); - promises.push( this.cipherService.getAll(activeUserId).then((ciphers) => { allCiphers = ciphers; @@ -280,6 +276,15 @@ export class OrganizationVaultExportService await Promise.all(promises); + const encCollections: Collection[] = await firstValueFrom( + this.collectionService.encryptedCollections$(activeUserId).pipe( + map((collections) => collections ?? []), + map((collections) => + collections.filter((c) => c.organizationId == organizationId && c.manage), + ), + ), + ); + const restrictions = await firstValueFrom(this.restrictedItemTypesService.restricted$); encCiphers = allCiphers.filter( diff --git a/libs/tools/export/vault-export/vault-export-ui/src/components/export.component.ts b/libs/tools/export/vault-export/vault-export-ui/src/components/export.component.ts index 6af6d5121fb..0b5d3d70834 100644 --- a/libs/tools/export/vault-export/vault-export-ui/src/components/export.component.ts +++ b/libs/tools/export/vault-export/vault-export-ui/src/components/export.component.ts @@ -272,25 +272,29 @@ export class ExportComponent implements OnInit, OnDestroy, AfterViewInit { return; } - this.organizations$ = combineLatest({ - collections: this.collectionService.decryptedCollections$, - memberOrganizations: this.accountService.activeAccount$.pipe( + this.organizations$ = this.accountService.activeAccount$ + .pipe( getUserId, - switchMap((userId) => this.organizationService.memberOrganizations$(userId)), - ), - }).pipe( - map(({ collections, memberOrganizations }) => { - const managedCollectionsOrgIds = new Set( - collections.filter((c) => c.manage).map((c) => c.organizationId), - ); - // Filter organizations that exist in managedCollectionsOrgIds - const filteredOrgs = memberOrganizations.filter((org) => - managedCollectionsOrgIds.has(org.id), - ); - // Sort the filtered organizations based on the name - return filteredOrgs.sort(Utils.getSortFunction(this.i18nService, "name")); - }), - ); + switchMap((userId) => + combineLatest({ + collections: this.collectionService.decryptedCollections$(userId), + memberOrganizations: this.organizationService.memberOrganizations$(userId), + }), + ), + ) + .pipe( + map(({ collections, memberOrganizations }) => { + const managedCollectionsOrgIds = new Set( + collections.filter((c) => c.manage).map((c) => c.organizationId), + ); + // Filter organizations that exist in managedCollectionsOrgIds + const filteredOrgs = memberOrganizations.filter((org) => + managedCollectionsOrgIds.has(org.id), + ); + // Sort the filtered organizations based on the name + return filteredOrgs.sort(Utils.getSortFunction(this.i18nService, "name")); + }), + ); combineLatest([ this.disablePersonalVaultExportPolicy$, diff --git a/libs/vault/src/cipher-form/services/default-cipher-form-config.service.ts b/libs/vault/src/cipher-form/services/default-cipher-form-config.service.ts index c47e5842987..04a2bb957ec 100644 --- a/libs/vault/src/cipher-form/services/default-cipher-form-config.service.ts +++ b/libs/vault/src/cipher-form/services/default-cipher-form-config.service.ts @@ -48,9 +48,10 @@ export class DefaultCipherFormConfigService implements CipherFormConfigService { await firstValueFrom( combineLatest([ this.organizations$(activeUserId), - this.collectionService.encryptedCollections$.pipe( + this.collectionService.encryptedCollections$(activeUserId).pipe( + map((collections) => collections ?? []), switchMap((c) => - this.collectionService.decryptedCollections$.pipe( + this.collectionService.decryptedCollections$(activeUserId).pipe( filter((d) => d.length === c.length), // Ensure all collections have been decrypted ), ), diff --git a/libs/vault/src/cipher-view/cipher-view.component.ts b/libs/vault/src/cipher-view/cipher-view.component.ts index 66910ad8ac7..4f54e5d393a 100644 --- a/libs/vault/src/cipher-view/cipher-view.component.ts +++ b/libs/vault/src/cipher-view/cipher-view.component.ts @@ -16,7 +16,8 @@ import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { isCardExpired } from "@bitwarden/common/autofill/utils"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; -import { CipherId, CollectionId, EmergencyAccessId, UserId } from "@bitwarden/common/types/guid"; +import { getByIds } from "@bitwarden/common/platform/misc"; +import { CipherId, EmergencyAccessId, 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"; @@ -143,6 +144,8 @@ export class CipherViewComponent implements OnChanges, OnDestroy { return; } + const userId = await firstValueFrom(this.activeUserId$); + // Load collections if not provided and the cipher has collectionIds if ( this.cipher.collectionIds && @@ -150,14 +153,12 @@ export class CipherViewComponent implements OnChanges, OnDestroy { (!this.collections || this.collections.length === 0) ) { this.collections = await firstValueFrom( - this.collectionService.decryptedCollectionViews$( - this.cipher.collectionIds as CollectionId[], - ), + this.collectionService + .decryptedCollections$(userId) + .pipe(getByIds(this.cipher.collectionIds)), ); } - const userId = await firstValueFrom(this.activeUserId$); - if (this.cipher.organizationId) { this.organization$ = this.organizationService .organizations$(userId) diff --git a/libs/vault/src/components/assign-collections.component.ts b/libs/vault/src/components/assign-collections.component.ts index 124dc783034..b2bd6e31ee5 100644 --- a/libs/vault/src/components/assign-collections.component.ts +++ b/libs/vault/src/components/assign-collections.component.ts @@ -435,12 +435,14 @@ export class AssignCollectionsComponent implements OnInit, OnDestroy, AfterViewI * @returns An observable of the collections for the organization. */ private getCollectionsForOrganization(orgId: OrganizationId): Observable { - return combineLatest([ - this.collectionService.decryptedCollections$, - this.accountService.activeAccount$.pipe( - switchMap((account) => this.organizationService.organizations$(account?.id)), + return this.accountService.activeAccount$.pipe( + getUserId, + switchMap((userId) => + combineLatest([ + this.collectionService.decryptedCollections$(userId), + this.organizationService.organizations$(userId), + ]), ), - ]).pipe( map(([collections, organizations]) => { const org = organizations.find((o) => o.id === orgId); this.orgName = org.name;