diff --git a/apps/browser/src/vault/popup/components/vault/assign-collections/assign-collections.component.ts b/apps/browser/src/vault/popup/components/vault/assign-collections/assign-collections.component.ts index 546a352111ef..6972e77ac5ab 100644 --- a/apps/browser/src/vault/popup/components/vault/assign-collections/assign-collections.component.ts +++ b/apps/browser/src/vault/popup/components/vault/assign-collections/assign-collections.component.ts @@ -5,13 +5,13 @@ import { Component } from "@angular/core"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { ReactiveFormsModule } from "@angular/forms"; import { ActivatedRoute } from "@angular/router"; -import { Observable, combineLatest, filter, first, map, switchMap } from "rxjs"; +import { Observable, combineLatest, filter, first, 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 { CipherId, 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"; import { @@ -62,21 +62,18 @@ export class AssignCollections { private accountService: AccountService, route: ActivatedRoute, ) { - const cipher$: Observable = this.accountService.activeAccount$.pipe( - map((account) => account?.id), - filter((userId) => userId != null), + const userId$ = this.accountService.activeAccount$.pipe(getUserId); + + const cipher$: Observable = userId$.pipe( switchMap((userId) => route.queryParams.pipe( - switchMap(async ({ cipherId }) => { - const cipherDomain = await this.cipherService.get(cipherId, userId); - return await this.cipherService.decrypt(cipherDomain, userId); - }), + switchMap(({ cipherId }) => this.cipherService.cipherView$(userId, cipherId as CipherId)), ), ), + filter((cipher): cipher is CipherView => cipher != null), ); - const decryptedCollection$ = this.accountService.activeAccount$.pipe( - getUserId, + const decryptedCollection$ = userId$.pipe( switchMap((userId) => this.collectionService.decryptedCollections$(userId)), ); diff --git a/apps/browser/src/vault/popup/components/vault/attachments/open-attachments/open-attachments.component.spec.ts b/apps/browser/src/vault/popup/components/vault/attachments/open-attachments/open-attachments.component.spec.ts index b88b435c702e..dfa6e7a5ff62 100644 --- a/apps/browser/src/vault/popup/components/vault/attachments/open-attachments/open-attachments.component.spec.ts +++ b/apps/browser/src/vault/popup/components/vault/attachments/open-attachments/open-attachments.component.spec.ts @@ -38,16 +38,14 @@ describe("OpenAttachmentsComponent", () => { }, } as CipherView; - const cipherDomain = { - decrypt: () => cipherView, - }; - const org = { name: "Test Org", productTierType: ProductTierType.Enterprise, } as Organization; - const getCipher = jest.fn().mockResolvedValue(cipherDomain); + const mockCipherService = { + cipherView$: jest.fn().mockReturnValue(of(cipherView)), + }; const organizations$ = jest.fn().mockReturnValue(of([org])); const mockUserId = Utils.newGuid() as UserId; @@ -63,7 +61,7 @@ describe("OpenAttachmentsComponent", () => { const formStatusChange$ = new BehaviorSubject<"enabled" | "disabled">("enabled"); beforeEach(async () => { - getCipher.mockClear(); + mockCipherService.cipherView$.mockClear(); showToast.mockClear(); organizations$.mockClear(); hasPremiumFromAnySource$.next(true); @@ -76,11 +74,7 @@ describe("OpenAttachmentsComponent", () => { { provide: BillingAccountProfileStateService, useValue: { hasPremiumFromAnySource$ } }, { provide: CipherService, - useValue: { - get: getCipher, - getKeyForCipherKeyDecryption: () => Promise.resolve(null), - decrypt: jest.fn().mockResolvedValue(cipherView), - }, + useValue: mockCipherService, }, { provide: CipherFormContainer, diff --git a/apps/browser/src/vault/popup/components/vault/attachments/open-attachments/open-attachments.component.ts b/apps/browser/src/vault/popup/components/vault/attachments/open-attachments/open-attachments.component.ts index b5e2b588ad85..63f9792c18f7 100644 --- a/apps/browser/src/vault/popup/components/vault/attachments/open-attachments/open-attachments.component.ts +++ b/apps/browser/src/vault/popup/components/vault/attachments/open-attachments/open-attachments.component.ts @@ -80,8 +80,9 @@ export class OpenAttachmentsComponent implements OnInit { const activeUserId = await firstValueFrom( this.accountService.activeAccount$.pipe(map((a) => a?.id)), ); - const cipherDomain = await this.cipherService.get(this.cipherId, activeUserId); - const cipher = await this.cipherService.decrypt(cipherDomain, activeUserId); + const cipher = await firstValueFrom( + this.cipherService.cipherView$(activeUserId, this.cipherId), + ); if (!cipher.organizationId) { this.cipherIsAPartOfFreeOrg = false; diff --git a/apps/browser/src/vault/popup/components/vault/vault-list-items-container/vault-list-items-container.component.ts b/apps/browser/src/vault/popup/components/vault/vault-list-items-container/vault-list-items-container.component.ts index 0fd257d33bbb..e0155f8c4beb 100644 --- a/apps/browser/src/vault/popup/components/vault/vault-list-items-container/vault-list-items-container.component.ts +++ b/apps/browser/src/vault/popup/components/vault/vault-list-items-container/vault-list-items-container.component.ts @@ -430,8 +430,13 @@ export class VaultListItemsContainerComponent implements AfterViewInit { // When only the `CipherListView` is available, fetch the full cipher details const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); - const _cipher = await this.cipherService.get(uuidAsString(cipher.id!), activeUserId); - const cipherView = await this.cipherService.decrypt(_cipher, activeUserId); + const cipherView = await firstValueFrom( + this.cipherService.cipherView$(activeUserId, uuidAsString(cipher.id!) as CipherId), + ); + + if (!cipherView) { + return; + } await this.vaultPopupAutofillService.doAutofill(cipherView); } diff --git a/apps/browser/src/vault/popup/components/vault/vault-password-history/vault-password-history.component.spec.ts b/apps/browser/src/vault/popup/components/vault/vault-password-history/vault-password-history.component.spec.ts index 3b8a6db25b1a..90b6388a1344 100644 --- a/apps/browser/src/vault/popup/components/vault/vault-password-history/vault-password-history.component.spec.ts +++ b/apps/browser/src/vault/popup/components/vault/vault-password-history/vault-password-history.component.spec.ts @@ -1,7 +1,7 @@ import { ComponentFixture, fakeAsync, TestBed, tick } from "@angular/core/testing"; import { ActivatedRoute } from "@angular/router"; import { mock } from "jest-mock-extended"; -import { Subject } from "rxjs"; +import { BehaviorSubject, Subject } from "rxjs"; import { WINDOW } from "@bitwarden/angular/services/injection-tokens"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; @@ -33,11 +33,13 @@ describe("PasswordHistoryComponent", () => { } as unknown as Cipher; const back = jest.fn().mockResolvedValue(undefined); - const getCipher = jest.fn().mockResolvedValue(mockCipher); + const cipherView$ = jest + .fn() + .mockReturnValue(new BehaviorSubject(mockCipher)); beforeEach(async () => { back.mockClear(); - getCipher.mockClear(); + cipherView$.mockClear(); await TestBed.configureTestingModule({ imports: [PasswordHistoryComponent], @@ -45,7 +47,12 @@ describe("PasswordHistoryComponent", () => { { provide: WINDOW, useValue: window }, { provide: PlatformUtilsService, useValue: mock() }, { provide: ConfigService, useValue: mock() }, - { provide: CipherService, useValue: mock({ get: getCipher }) }, + { + provide: CipherService, + useValue: mock({ + cipherView$, + }), + }, { provide: AccountService, useValue: mockAccountServiceWith(mockUserId), @@ -65,13 +72,13 @@ describe("PasswordHistoryComponent", () => { tick(100); - expect(getCipher).toHaveBeenCalledWith(mockCipherView.id, mockUserId); + expect(cipherView$).toHaveBeenCalledWith(mockUserId, mockCipherView.id); })); it("navigates back when a cipherId is not in the params", () => { params$.next({}); expect(back).toHaveBeenCalledTimes(1); - expect(getCipher).not.toHaveBeenCalled(); + expect(cipherView$).not.toHaveBeenCalled(); }); }); diff --git a/apps/browser/src/vault/popup/components/vault/vault-password-history/vault-password-history.component.ts b/apps/browser/src/vault/popup/components/vault/vault-password-history/vault-password-history.component.ts index e5909937b126..2942c62a5cf0 100644 --- a/apps/browser/src/vault/popup/components/vault/vault-password-history/vault-password-history.component.ts +++ b/apps/browser/src/vault/popup/components/vault/vault-password-history/vault-password-history.component.ts @@ -7,7 +7,7 @@ import { first, map } from "rxjs/operators"; import { JslibModule } from "@bitwarden/angular/jslib.module"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; -import { UserId } from "@bitwarden/common/types/guid"; +import { CipherId, UserId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { PasswordHistoryViewComponent } from "@bitwarden/vault"; @@ -67,7 +67,8 @@ export class PasswordHistoryComponent implements OnInit { const activeUserId = activeAccount.id as UserId; - const cipher = await this.cipherService.get(cipherId, activeUserId); - this.cipher = await this.cipherService.decrypt(cipher, activeUserId); + this.cipher = await firstValueFrom( + this.cipherService.cipherView$(activeUserId, cipherId as CipherId), + ); } } 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 5a6ca11f4e99..e86ae27e3b40 100644 --- a/apps/web/src/app/vault/individual-vault/vault.component.ts +++ b/apps/web/src/app/vault/individual-vault/vault.component.ts @@ -1666,9 +1666,10 @@ export class VaultComponent implements OnInit, OnDestr } const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); - const _cipher = await this.cipherService.get(uuidAsString(cipher.id), activeUserId); - const cipherView = await this.cipherService.decrypt(_cipher, activeUserId); - return cipherView.login.password; + const cipherView = await firstValueFrom( + this.cipherService.cipherView$(activeUserId, uuidAsString(cipher.id) as CipherId), + ); + return cipherView?.login.password; } } diff --git a/libs/common/src/vault/abstractions/cipher.service.ts b/libs/common/src/vault/abstractions/cipher.service.ts index 51b12f91d2a7..318d40d56c93 100644 --- a/libs/common/src/vault/abstractions/cipher.service.ts +++ b/libs/common/src/vault/abstractions/cipher.service.ts @@ -30,6 +30,17 @@ export type EncryptionContext = { export abstract class CipherService implements UserKeyRotationDataProvider { abstract cipherViews$(userId: UserId): Observable; + /** + * Observable that emits the decrypted {@link CipherView} for a single cipher, or `undefined` + * when no cipher with the given id exists in the user's vault. + * + * Encapsulates the common pattern of subscribing to {@link cipherViews$} and finding a single + * cipher by id. The observable re-emits whenever the user's ciphers change. + * + * @param userId The id of the user whose vault should be searched. + * @param cipherId The id of the cipher to retrieve. + */ + abstract cipherView$(userId: UserId, cipherId: CipherId): Observable; abstract cipherListViews$(userId: UserId): Observable; abstract ciphers$(userId: UserId): Observable>; abstract localData$(userId: UserId): Observable>; diff --git a/libs/common/src/vault/services/cipher.service.spec.ts b/libs/common/src/vault/services/cipher.service.spec.ts index cb536140c05b..461e1f6dd004 100644 --- a/libs/common/src/vault/services/cipher.service.spec.ts +++ b/libs/common/src/vault/services/cipher.service.spec.ts @@ -752,6 +752,52 @@ describe("Cipher Service", () => { }); }); + describe("cipherView$", () => { + let cipher1: CipherView; + let cipher2: CipherView; + + beforeEach(() => { + cipher1 = new CipherView(encryptionContext.cipher); + cipher1.id = "cipher-1" as CipherId; + cipher2 = new CipherView(encryptionContext.cipher); + cipher2.id = "cipher-2" as CipherId; + + jest + .spyOn(cipherService, "cipherViews$") + .mockReturnValue(of([cipher1, cipher2]) as Observable); + }); + + it("emits the decrypted cipher matching the given id", async () => { + const result = await firstValueFrom( + cipherService.cipherView$(mockUserId, "cipher-2" as CipherId), + ); + + expect(result).toBe(cipher2); + }); + + it("emits undefined when no cipher matches the given id", async () => { + const result = await firstValueFrom( + cipherService.cipherView$(mockUserId, "missing" as CipherId), + ); + + expect(result).toBeUndefined(); + }); + + it("does not emit while cipherViews$ is null", async () => { + (cipherService.cipherViews$ as jest.Mock).mockReturnValue( + of(null) as unknown as Observable, + ); + + const emitted = jest.fn(); + const subscription = cipherService + .cipherView$(mockUserId, "cipher-1" as CipherId) + .subscribe(emitted); + + expect(emitted).not.toHaveBeenCalled(); + subscription.unsubscribe(); + }); + }); + describe("getDecryptedAttachmentBuffer", () => { const mockEncryptedContent = new Uint8Array([1, 2, 3]); const mockDecryptedContent = new Uint8Array([4, 5, 6]); diff --git a/libs/common/src/vault/services/cipher.service.ts b/libs/common/src/vault/services/cipher.service.ts index f6531f8bfb70..8309500ce5d0 100644 --- a/libs/common/src/vault/services/cipher.service.ts +++ b/libs/common/src/vault/services/cipher.service.ts @@ -225,6 +225,13 @@ export class CipherService implements CipherServiceAbstraction { ); }, this.clearCipherViewsForUser$); + cipherView$(userId: UserId, cipherId: CipherId): Observable { + return this.cipherViews$(userId).pipe( + filterOutNullish(), + map((ciphers) => ciphers.find((cipher) => cipher.id === cipherId)), + ); + } + addEditCipherInfo$(userId: UserId): Observable { return this.addEditCipherInfoState(userId).state$; } diff --git a/libs/vault/src/vault-item-dialog/vault-item-dialog.component.spec.ts b/libs/vault/src/vault-item-dialog/vault-item-dialog.component.spec.ts index 4d544d9efa93..3e0a71e3f884 100644 --- a/libs/vault/src/vault-item-dialog/vault-item-dialog.component.spec.ts +++ b/libs/vault/src/vault-item-dialog/vault-item-dialog.component.spec.ts @@ -464,26 +464,22 @@ describe("VaultItemDialogComponent", () => { }); it("refreshes the cipher from local state and switches to view mode", async () => { - const latestCipher = { id: "cipher-id" } as any; const refreshedCipherView = { id: "cipher-id", attachments: [] } as any; - cipherServiceMock.get.mockResolvedValue(latestCipher); - cipherServiceMock.decrypt.mockResolvedValue(refreshedCipherView); + cipherServiceMock.cipherView$.mockReturnValue(of(refreshedCipherView)); await component.cancel(); - expect(cipherServiceMock.get).toHaveBeenCalledWith("cipher-id", "test-user-id"); - expect(cipherServiceMock.decrypt).toHaveBeenCalledWith(latestCipher, "test-user-id"); + expect(cipherServiceMock.cipherView$).toHaveBeenCalledWith("test-user-id", "cipher-id"); expect(component["cipher"]).toBe(refreshedCipherView); expect((component as any).changeMode).toHaveBeenCalledWith("view"); }); it("leaves the existing cipher in place when local state has no cipher", async () => { const originalCipher = component["cipher"]; - cipherServiceMock.get.mockResolvedValue(null as any); + cipherServiceMock.cipherView$.mockReturnValue(of(undefined)); await component.cancel(); - expect(cipherServiceMock.decrypt).not.toHaveBeenCalled(); expect(component["cipher"]).toBe(originalCipher); expect((component as any).changeMode).toHaveBeenCalledWith("view"); }); diff --git a/libs/vault/src/vault-item-dialog/vault-item-dialog.component.ts b/libs/vault/src/vault-item-dialog/vault-item-dialog.component.ts index 1da690d307c4..6042afb866cc 100644 --- a/libs/vault/src/vault-item-dialog/vault-item-dialog.component.ts +++ b/libs/vault/src/vault-item-dialog/vault-item-dialog.component.ts @@ -549,12 +549,12 @@ export class VaultItemDialogComponent implements OnInit, OnDestroy { await this.cipherService.getKeyForCipherKeyDecryption(cipher, activeUserId), ); } else { - const updatedCipher = await this.cipherService.get( - this.formConfig.originalCipher?.id, - activeUserId, + updatedCipherView = await firstValueFrom( + this.cipherService.cipherView$( + activeUserId, + this.formConfig.originalCipher?.id as CipherId, + ), ); - - updatedCipherView = await this.cipherService.decrypt(updatedCipher, activeUserId); } this.cipherFormComponent().patchCipher((currentCipher) => { @@ -584,9 +584,11 @@ export class VaultItemDialogComponent implements OnInit, OnDestroy { // Refresh from local state so attachments modified during edit aren't stale in view mode. const activeUserId = await firstValueFrom(this.userId$); - const latestCipher = await this.cipherService.get(this.cipher.id, activeUserId); + const latestCipher = await firstValueFrom( + this.cipherService.cipherView$(activeUserId, this.cipher.id as CipherId), + ); if (latestCipher != null) { - this.cipher = await this.cipherService.decrypt(latestCipher, activeUserId); + this.cipher = latestCipher; } // We're in Form mode, and we have a cipher, switch back to View mode.