From cf416388d7c1e52bba0b42e2fa8ad1617b57435f Mon Sep 17 00:00:00 2001 From: Jeffrey Holland <124393578+jholland-livefront@users.noreply.github.com> Date: Wed, 3 Dec 2025 09:46:40 +0100 Subject: [PATCH 01/12] Fix stale data issue in new login popout (#17307) * Fix stale data issue in new login popout * Update the comments * Address critical claude code bot suggestions * Clean out all stale data from pop up * Fix cached cipher issue * Fix caching issue between tab and overlay flow * Address claude comments --- .../add-edit/add-edit-v2.component.spec.ts | 84 +++++++++++++++++++ .../add-edit/add-edit-v2.component.ts | 50 ++++++++++- .../popup/utils/vault-popout-window.spec.ts | 40 +++++++++ .../vault/popup/utils/vault-popout-window.ts | 24 +++++- .../components/cipher-form.component.spec.ts | 13 ++- .../components/cipher-form.component.ts | 24 +++++- .../default-cipher-form-cache.service.ts | 8 +- 7 files changed, 232 insertions(+), 11 deletions(-) diff --git a/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.spec.ts b/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.spec.ts index 1bffcd9ad51..f2c9d470816 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.spec.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.spec.ts @@ -381,4 +381,88 @@ describe("AddEditV2Component", () => { expect(navigate).toHaveBeenCalledWith(["/tabs/vault"]); }); }); + + describe("reloadAddEditCipherData", () => { + beforeEach(fakeAsync(() => { + addEditCipherInfo$.next({ + cipher: { + name: "InitialName", + type: CipherType.Login, + login: { + password: "initialPassword", + username: "initialUsername", + uris: [{ uri: "https://initial.com" }], + }, + }, + } as AddEditCipherInfo); + queryParams$.next({}); + tick(); + + cipherServiceMock.setAddEditCipherInfo.mockClear(); + })); + + it("replaces all initialValues with new data, clearing stale fields", fakeAsync(() => { + const newCipherInfo = { + cipher: { + name: "UpdatedName", + type: CipherType.Login, + login: { + password: "updatedPassword", + uris: [{ uri: "https://updated.com" }], + }, + }, + } as AddEditCipherInfo; + + addEditCipherInfo$.next(newCipherInfo); + + const messageListener = component["messageListener"]; + messageListener({ command: "reloadAddEditCipherData" }); + tick(); + + expect(component.config.initialValues).toEqual({ + name: "UpdatedName", + password: "updatedPassword", + loginUri: "https://updated.com", + } as OptionalInitialValues); + + expect(cipherServiceMock.setAddEditCipherInfo).toHaveBeenCalledWith(null, "UserId"); + })); + + it("does not reload data if config is not set", fakeAsync(() => { + component.config = null; + + const messageListener = component["messageListener"]; + messageListener({ command: "reloadAddEditCipherData" }); + tick(); + + expect(cipherServiceMock.setAddEditCipherInfo).not.toHaveBeenCalled(); + })); + + it("does not reload data if latestCipherInfo is null", fakeAsync(() => { + addEditCipherInfo$.next(null); + + const messageListener = component["messageListener"]; + messageListener({ command: "reloadAddEditCipherData" }); + tick(); + + expect(component.config.initialValues).toEqual({ + name: "InitialName", + password: "initialPassword", + username: "initialUsername", + loginUri: "https://initial.com", + } as OptionalInitialValues); + + expect(cipherServiceMock.setAddEditCipherInfo).not.toHaveBeenCalled(); + })); + + it("ignores messages with different commands", fakeAsync(() => { + const initialValues = component.config.initialValues; + + const messageListener = component["messageListener"]; + messageListener({ command: "someOtherCommand" }); + tick(); + + expect(component.config.initialValues).toBe(initialValues); + })); + }); }); diff --git a/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.ts b/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.ts index 60e44cefbdf..22aad854dd0 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/add-edit/add-edit-v2.component.ts @@ -1,7 +1,7 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore import { CommonModule } from "@angular/common"; -import { Component, OnInit } from "@angular/core"; +import { Component, OnInit, OnDestroy } from "@angular/core"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { FormsModule } from "@angular/forms"; import { ActivatedRoute, Params, Router } from "@angular/router"; @@ -158,7 +158,7 @@ export type AddEditQueryParams = Partial>; IconButtonModule, ], }) -export class AddEditV2Component implements OnInit { +export class AddEditV2Component implements OnInit, OnDestroy { headerText: string; config: CipherFormConfig; canDeleteCipher$: Observable; @@ -200,12 +200,58 @@ export class AddEditV2Component implements OnInit { this.subscribeToParams(); } + private messageListener: (message: any) => void; + async ngOnInit() { this.fido2PopoutSessionData = await firstValueFrom(this.fido2PopoutSessionData$); if (BrowserPopupUtils.inPopout(window)) { this.popupCloseWarningService.enable(); } + + // Listen for messages to reload cipher data when the pop up is already open + this.messageListener = async (message: any) => { + if (message?.command === "reloadAddEditCipherData") { + try { + await this.reloadCipherData(); + } catch (error) { + this.logService.error("Failed to reload cipher data", error); + } + } + }; + BrowserApi.addListener(chrome.runtime.onMessage, this.messageListener); + } + + ngOnDestroy() { + if (this.messageListener) { + BrowserApi.removeListener(chrome.runtime.onMessage, this.messageListener); + } + } + + /** + * Reloads the cipher data when the popup is already open and new form data is submitted. + * This completely replaces the initialValues to clear any stale data from the previous submission. + */ + private async reloadCipherData() { + if (!this.config) { + return; + } + + const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); + + const latestCipherInfo = await firstValueFrom( + this.cipherService.addEditCipherInfo$(activeUserId), + ); + + if (latestCipherInfo != null) { + this.config = { + ...this.config, + initialValues: mapAddEditCipherInfoToInitialValues(latestCipherInfo), + }; + + // Be sure to clear the "cached" cipher info, so it doesn't get used again + await this.cipherService.setAddEditCipherInfo(null, activeUserId); + } } /** diff --git a/apps/browser/src/vault/popup/utils/vault-popout-window.spec.ts b/apps/browser/src/vault/popup/utils/vault-popout-window.spec.ts index 4597c004290..3389228dda4 100644 --- a/apps/browser/src/vault/popup/utils/vault-popout-window.spec.ts +++ b/apps/browser/src/vault/popup/utils/vault-popout-window.spec.ts @@ -2,6 +2,7 @@ import { mock } from "jest-mock-extended"; import { CipherType } from "@bitwarden/common/vault/enums"; +import { BrowserApi } from "../../../platform/browser/browser-api"; import BrowserPopupUtils from "../../../platform/browser/browser-popup-utils"; import { @@ -23,6 +24,19 @@ describe("VaultPopoutWindow", () => { .spyOn(BrowserPopupUtils, "closeSingleActionPopout") .mockImplementation(); + beforeEach(() => { + jest.spyOn(BrowserApi, "tabsQuery").mockResolvedValue([]); + jest.spyOn(BrowserApi, "updateWindowProperties").mockResolvedValue(); + global.chrome = { + ...global.chrome, + runtime: { + ...global.chrome?.runtime, + sendMessage: jest.fn().mockResolvedValue(undefined), + getURL: jest.fn((path) => `chrome-extension://extension-id/${path}`), + }, + }; + }); + afterEach(() => { jest.clearAllMocks(); }); @@ -123,6 +137,32 @@ describe("VaultPopoutWindow", () => { }, ); }); + + it("sends a message to refresh data when the popup is already open", async () => { + const existingPopupTab = { + id: 123, + windowId: 456, + url: `chrome-extension://extension-id/popup/index.html#/edit-cipher?singleActionPopout=${VaultPopoutType.addEditVaultItem}_${CipherType.Login}`, + } as chrome.tabs.Tab; + + jest.spyOn(BrowserApi, "tabsQuery").mockResolvedValue([existingPopupTab]); + const sendMessageSpy = jest.spyOn(chrome.runtime, "sendMessage"); + const updateWindowSpy = jest.spyOn(BrowserApi, "updateWindowProperties"); + + await openAddEditVaultItemPopout( + mock({ windowId: 1, url: "https://jest-testing-website.com" }), + { + cipherType: CipherType.Login, + }, + ); + + expect(openPopoutSpy).not.toHaveBeenCalled(); + expect(sendMessageSpy).toHaveBeenCalledWith({ + command: "reloadAddEditCipherData", + data: { cipherId: undefined, cipherType: CipherType.Login }, + }); + expect(updateWindowSpy).toHaveBeenCalledWith(456, { focused: true }); + }); }); describe("closeAddEditVaultItemPopout", () => { diff --git a/apps/browser/src/vault/popup/utils/vault-popout-window.ts b/apps/browser/src/vault/popup/utils/vault-popout-window.ts index 3dae96b6cc7..cccf005cd2e 100644 --- a/apps/browser/src/vault/popup/utils/vault-popout-window.ts +++ b/apps/browser/src/vault/popup/utils/vault-popout-window.ts @@ -115,10 +115,26 @@ async function openAddEditVaultItemPopout( addEditCipherUrl += formatQueryString("uri", url); } - await BrowserPopupUtils.openPopout(addEditCipherUrl, { - singleActionKey, - senderWindowId: windowId, - }); + const extensionUrl = chrome.runtime.getURL("popup/index.html"); + const existingPopupTabs = await BrowserApi.tabsQuery({ url: `${extensionUrl}*` }); + const existingPopup = existingPopupTabs.find((tab) => + tab.url?.includes(`singleActionPopout=${singleActionKey}`), + ); + // Check if the an existing popup is already open + try { + await chrome.runtime.sendMessage({ + command: "reloadAddEditCipherData", + data: { cipherId, cipherType }, + }); + await BrowserApi.updateWindowProperties(existingPopup.windowId, { + focused: true, + }); + } catch { + await BrowserPopupUtils.openPopout(addEditCipherUrl, { + singleActionKey, + senderWindowId: windowId, + }); + } } /** diff --git a/libs/vault/src/cipher-form/components/cipher-form.component.spec.ts b/libs/vault/src/cipher-form/components/cipher-form.component.spec.ts index 1e60ad91fb1..9f3102239ae 100644 --- a/libs/vault/src/cipher-form/components/cipher-form.component.spec.ts +++ b/libs/vault/src/cipher-form/components/cipher-form.component.spec.ts @@ -42,9 +42,18 @@ describe("CipherFormComponent", () => { { provide: CipherFormService, useValue: mockAddEditFormService }, { provide: CipherFormCacheService, - useValue: { init: jest.fn(), getCachedCipherView: jest.fn() }, + useValue: { init: jest.fn(), getCachedCipherView: jest.fn(), clearCache: jest.fn() }, + }, + { + provide: ViewCacheService, + useValue: { + signal: jest.fn(() => { + const signalFn = (): any => null; + signalFn.set = jest.fn(); + return signalFn; + }), + }, }, - { provide: ViewCacheService, useValue: { signal: jest.fn(() => (): any => null) } }, { provide: ConfigService, useValue: mock() }, { provide: AccountService, useValue: mockAccountService }, { provide: CipherArchiveService, useValue: mockCipherArchiveService }, diff --git a/libs/vault/src/cipher-form/components/cipher-form.component.ts b/libs/vault/src/cipher-form/components/cipher-form.component.ts index f94af25e90a..c9e867f8d3a 100644 --- a/libs/vault/src/cipher-form/components/cipher-form.component.ts +++ b/libs/vault/src/cipher-form/components/cipher-form.component.ts @@ -304,13 +304,30 @@ export class CipherFormComponent implements AfterViewInit, OnInit, OnChanges, Ci * Updates `updatedCipherView` based on the value from the cache. */ setInitialCipherFromCache() { + // If we are coming from the overlay/popup flow clear the cache to avoid old cached data + const hasOverlayData = + this.config.initialValues && + (this.config.initialValues.username !== undefined || + this.config.initialValues.password !== undefined); + + if (hasOverlayData) { + this.cipherFormCacheService.clearCache(); + return; + } + const cachedCipher = this.cipherFormCacheService.getCachedCipherView(); if (cachedCipher === null) { return; } - // Use the cached cipher when it matches the cipher being edited - if (this.updatedCipherView.id === cachedCipher.id) { + const isEditingExistingCipher = + this.updatedCipherView.id && this.updatedCipherView.id === cachedCipher.id; + const isCreatingNewCipher = + !this.updatedCipherView.id && + !cachedCipher.id && + this.updatedCipherView.type === cachedCipher.type; + + if (isEditingExistingCipher || isCreatingNewCipher) { this.updatedCipherView = cachedCipher; } } @@ -382,6 +399,9 @@ export class CipherFormComponent implements AfterViewInit, OnInit, OnChanges, Ci this.config, ); + // Clear the cache after successful save + this.cipherFormCacheService.clearCache(); + this.toastService.showToast({ variant: "success", title: null, diff --git a/libs/vault/src/cipher-form/services/default-cipher-form-cache.service.ts b/libs/vault/src/cipher-form/services/default-cipher-form-cache.service.ts index 25581ae5ea1..d525dcd9afa 100644 --- a/libs/vault/src/cipher-form/services/default-cipher-form-cache.service.ts +++ b/libs/vault/src/cipher-form/services/default-cipher-form-cache.service.ts @@ -22,7 +22,6 @@ export class CipherFormCacheService { key: CIPHER_FORM_CACHE_KEY, initialValue: null, deserializer: CipherView.fromJSON, - clearOnTabChange: true, }); constructor() { @@ -45,4 +44,11 @@ export class CipherFormCacheService { getCachedCipherView(): CipherView | null { return this.cipherCache(); } + + /** + * Clear the cached CipherView. + */ + clearCache(): void { + this.cipherCache.set(null); + } } From a6100d8a0ebe67fbb453049627879fa54b7c45db Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Wed, 3 Dec 2025 13:11:03 +0100 Subject: [PATCH 02/12] Replace webcrypto RSA with PureCrypto RSA (#17742) --- .../abstractions/crypto-function.service.ts | 6 +- .../encrypt.service.implementation.ts | 8 +-- .../web-crypto-function.service.spec.ts | 3 +- .../services/web-crypto-function.service.ts | 47 ++++--------- .../node-crypto-function.service.spec.ts | 15 +++- .../services/node-crypto-function.service.ts | 70 +++++-------------- 6 files changed, 48 insertions(+), 101 deletions(-) diff --git a/libs/common/src/key-management/crypto/abstractions/crypto-function.service.ts b/libs/common/src/key-management/crypto/abstractions/crypto-function.service.ts index 705a1c1a24e..b16371198b3 100644 --- a/libs/common/src/key-management/crypto/abstractions/crypto-function.service.ts +++ b/libs/common/src/key-management/crypto/abstractions/crypto-function.service.ts @@ -91,7 +91,7 @@ export abstract class CryptoFunctionService { abstract rsaEncrypt( data: Uint8Array, publicKey: Uint8Array, - algorithm: "sha1" | "sha256", + algorithm: "sha1", ): Promise; /** * @deprecated HAZMAT WARNING: DO NOT USE THIS FOR NEW CODE. Implement low-level crypto operations @@ -100,10 +100,10 @@ export abstract class CryptoFunctionService { abstract rsaDecrypt( data: Uint8Array, privateKey: Uint8Array, - algorithm: "sha1" | "sha256", + algorithm: "sha1", ): Promise; abstract rsaExtractPublicKey(privateKey: Uint8Array): Promise; - abstract rsaGenerateKeyPair(length: 1024 | 2048 | 4096): Promise<[Uint8Array, Uint8Array]>; + abstract rsaGenerateKeyPair(length: 2048): Promise<[Uint8Array, Uint8Array]>; /** * Generates a key of the given length suitable for use in AES encryption */ diff --git a/libs/common/src/key-management/crypto/services/encrypt.service.implementation.ts b/libs/common/src/key-management/crypto/services/encrypt.service.implementation.ts index 132bbc306cb..a5da0c82382 100644 --- a/libs/common/src/key-management/crypto/services/encrypt.service.implementation.ts +++ b/libs/common/src/key-management/crypto/services/encrypt.service.implementation.ts @@ -252,15 +252,9 @@ export class EncryptServiceImplementation implements EncryptService { throw new Error("[Encrypt service] rsaDecrypt: No data provided for decryption."); } - let algorithm: "sha1" | "sha256"; switch (data.encryptionType) { case EncryptionType.Rsa2048_OaepSha1_B64: case EncryptionType.Rsa2048_OaepSha1_HmacSha256_B64: - algorithm = "sha1"; - break; - case EncryptionType.Rsa2048_OaepSha256_B64: - case EncryptionType.Rsa2048_OaepSha256_HmacSha256_B64: - algorithm = "sha256"; break; default: throw new Error("Invalid encryption type."); @@ -270,6 +264,6 @@ export class EncryptServiceImplementation implements EncryptService { throw new Error("[Encrypt service] rsaDecrypt: No private key provided for decryption."); } - return this.cryptoFunctionService.rsaDecrypt(data.dataBytes, privateKey, algorithm); + return this.cryptoFunctionService.rsaDecrypt(data.dataBytes, privateKey, "sha1"); } } diff --git a/libs/common/src/key-management/crypto/services/web-crypto-function.service.spec.ts b/libs/common/src/key-management/crypto/services/web-crypto-function.service.spec.ts index af23a515de2..c64926e0e5b 100644 --- a/libs/common/src/key-management/crypto/services/web-crypto-function.service.spec.ts +++ b/libs/common/src/key-management/crypto/services/web-crypto-function.service.spec.ts @@ -299,7 +299,6 @@ describe("WebCrypto Function Service", () => { }); describe("rsaGenerateKeyPair", () => { - testRsaGenerateKeyPair(1024); testRsaGenerateKeyPair(2048); // Generating 4096 bit keys can be slow. Commenting it out to save CI. @@ -495,7 +494,7 @@ function testHmac(algorithm: "sha1" | "sha256" | "sha512", mac: string) { }); } -function testRsaGenerateKeyPair(length: 1024 | 2048 | 4096) { +function testRsaGenerateKeyPair(length: 2048) { it( "should successfully generate a " + length + " bit key pair", async () => { diff --git a/libs/common/src/key-management/crypto/services/web-crypto-function.service.ts b/libs/common/src/key-management/crypto/services/web-crypto-function.service.ts index 829227cada9..ee0b5cab902 100644 --- a/libs/common/src/key-management/crypto/services/web-crypto-function.service.ts +++ b/libs/common/src/key-management/crypto/services/web-crypto-function.service.ts @@ -263,33 +263,19 @@ export class WebCryptoFunctionService implements CryptoFunctionService { async rsaEncrypt( data: Uint8Array, publicKey: Uint8Array, - algorithm: "sha1" | "sha256", + _algorithm: "sha1", ): Promise { - // Note: Edge browser requires that we specify name and hash for both key import and decrypt. - // We cannot use the proper types here. - const rsaParams = { - name: "RSA-OAEP", - hash: { name: this.toWebCryptoAlgorithm(algorithm) }, - }; - const impKey = await this.subtle.importKey("spki", publicKey, rsaParams, false, ["encrypt"]); - const buffer = await this.subtle.encrypt(rsaParams, impKey, data); - return new Uint8Array(buffer); + await SdkLoadService.Ready; + return PureCrypto.rsa_encrypt_data(data, publicKey); } async rsaDecrypt( data: Uint8Array, privateKey: Uint8Array, - algorithm: "sha1" | "sha256", + _algorithm: "sha1", ): Promise { - // Note: Edge browser requires that we specify name and hash for both key import and decrypt. - // We cannot use the proper types here. - const rsaParams = { - name: "RSA-OAEP", - hash: { name: this.toWebCryptoAlgorithm(algorithm) }, - }; - const impKey = await this.subtle.importKey("pkcs8", privateKey, rsaParams, false, ["decrypt"]); - const buffer = await this.subtle.decrypt(rsaParams, impKey, data); - return new Uint8Array(buffer); + await SdkLoadService.Ready; + return PureCrypto.rsa_decrypt_data(data, privateKey); } async rsaExtractPublicKey(privateKey: Uint8Array): Promise { @@ -297,6 +283,13 @@ export class WebCryptoFunctionService implements CryptoFunctionService { return PureCrypto.rsa_extract_public_key(privateKey) as UnsignedPublicKey; } + async rsaGenerateKeyPair(_length: 2048): Promise<[UnsignedPublicKey, Uint8Array]> { + await SdkLoadService.Ready; + const privateKey = PureCrypto.rsa_generate_keypair(); + const publicKey = await this.rsaExtractPublicKey(privateKey); + return [publicKey, privateKey]; + } + async aesGenerateKey(bitLength = 128 | 192 | 256 | 512): Promise { if (bitLength === 512) { // 512 bit keys are not supported in WebCrypto, so we concat two 256 bit keys @@ -314,20 +307,6 @@ export class WebCryptoFunctionService implements CryptoFunctionService { return new Uint8Array(rawKey) as CsprngArray; } - async rsaGenerateKeyPair(length: 1024 | 2048 | 4096): Promise<[Uint8Array, Uint8Array]> { - const rsaParams = { - name: "RSA-OAEP", - modulusLength: length, - publicExponent: new Uint8Array([0x01, 0x00, 0x01]), // 65537 - // Have to specify some algorithm - hash: { name: this.toWebCryptoAlgorithm("sha1") }, - }; - const keyPair = await this.subtle.generateKey(rsaParams, true, ["encrypt", "decrypt"]); - const publicKey = await this.subtle.exportKey("spki", keyPair.publicKey); - const privateKey = await this.subtle.exportKey("pkcs8", keyPair.privateKey); - return [new Uint8Array(publicKey), new Uint8Array(privateKey)]; - } - randomBytes(length: number): Promise { const arr = new Uint8Array(length); this.crypto.getRandomValues(arr); diff --git a/libs/node/src/services/node-crypto-function.service.spec.ts b/libs/node/src/services/node-crypto-function.service.spec.ts index 3256d85110f..28a6c127d44 100644 --- a/libs/node/src/services/node-crypto-function.service.spec.ts +++ b/libs/node/src/services/node-crypto-function.service.spec.ts @@ -1,9 +1,17 @@ +import { SdkLoadService } from "@bitwarden/common/platform/abstractions/sdk/sdk-load.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { EcbDecryptParameters } from "@bitwarden/common/platform/models/domain/decrypt-parameters"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { NodeCryptoFunctionService } from "./node-crypto-function.service"; +class TestSdkLoadService extends SdkLoadService { + protected override load(): Promise { + // Simulate successful WASM load + return Promise.resolve(); + } +} + const RsaPublicKey = "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAl0Vawl/toXzkEvB82FEtqHP" + "4xlU2ab/v0crqIfXfIoWF/XXdHGIdrZeilnRXPPJT1B9dTsasttEZNnua/0Rek/cjNDHtzT52irfoZYS7X6HNIfOi54Q+egP" + @@ -37,6 +45,10 @@ const Sha512Mac = "5ea7817a0b7c5d4d9b00364ccd214669131fc17fe4aca"; describe("NodeCrypto Function Service", () => { + beforeAll(async () => { + await new TestSdkLoadService().loadAndInit(); + }); + describe("pbkdf2", () => { const regular256Key = "pj9prw/OHPleXI6bRdmlaD+saJS4awrMiQsQiDjeu2I="; const utf8256Key = "yqvoFXgMRmHR3QPYr5pyR4uVuoHkltv9aHUP63p8n7I="; @@ -279,7 +291,6 @@ describe("NodeCrypto Function Service", () => { }); describe("rsaGenerateKeyPair", () => { - testRsaGenerateKeyPair(1024); testRsaGenerateKeyPair(2048); // Generating 4096 bit keys is really slow with Forge lib. @@ -514,7 +525,7 @@ function testCompare(fast = false) { }); } -function testRsaGenerateKeyPair(length: 1024 | 2048 | 4096) { +function testRsaGenerateKeyPair(length: 2048) { it( "should successfully generate a " + length + " bit key pair", async () => { diff --git a/libs/node/src/services/node-crypto-function.service.ts b/libs/node/src/services/node-crypto-function.service.ts index 22cc5756f30..49dbc65ca84 100644 --- a/libs/node/src/services/node-crypto-function.service.ts +++ b/libs/node/src/services/node-crypto-function.service.ts @@ -4,6 +4,7 @@ import * as forge from "node-forge"; import { CryptoFunctionService } from "@bitwarden/common/key-management/crypto/abstractions/crypto-function.service"; import { UnsignedPublicKey } from "@bitwarden/common/key-management/types"; +import { SdkLoadService } from "@bitwarden/common/platform/abstractions/sdk/sdk-load.service"; import { EncryptionType } from "@bitwarden/common/platform/enums"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { @@ -12,6 +13,7 @@ import { } from "@bitwarden/common/platform/models/domain/decrypt-parameters"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { CsprngArray } from "@bitwarden/common/types/csprng"; +import { PureCrypto } from "@bitwarden/sdk-internal"; export class NodeCryptoFunctionService implements CryptoFunctionService { pbkdf2( @@ -205,72 +207,34 @@ export class NodeCryptoFunctionService implements CryptoFunctionService { return Promise.resolve(this.toUint8Buffer(decBuf)); } - rsaEncrypt( + async rsaEncrypt( data: Uint8Array, publicKey: Uint8Array, - algorithm: "sha1" | "sha256", + _algorithm: "sha1", ): Promise { - if (algorithm === "sha256") { - throw new Error("Node crypto does not support RSA-OAEP SHA-256"); - } - - const pem = this.toPemPublicKey(publicKey); - const decipher = crypto.publicEncrypt(pem, this.toNodeBuffer(data)); - return Promise.resolve(this.toUint8Buffer(decipher)); + await SdkLoadService.Ready; + return PureCrypto.rsa_encrypt_data(data, publicKey); } - rsaDecrypt( + async rsaDecrypt( data: Uint8Array, privateKey: Uint8Array, - algorithm: "sha1" | "sha256", + _algorithm: "sha1", ): Promise { - if (algorithm === "sha256") { - throw new Error("Node crypto does not support RSA-OAEP SHA-256"); - } - - const pem = this.toPemPrivateKey(privateKey); - const decipher = crypto.privateDecrypt(pem, this.toNodeBuffer(data)); - return Promise.resolve(this.toUint8Buffer(decipher)); + await SdkLoadService.Ready; + return PureCrypto.rsa_decrypt_data(data, privateKey); } async rsaExtractPublicKey(privateKey: Uint8Array): Promise { - const privateKeyByteString = Utils.fromBufferToByteString(privateKey); - const privateKeyAsn1 = forge.asn1.fromDer(privateKeyByteString); - const forgePrivateKey: any = forge.pki.privateKeyFromAsn1(privateKeyAsn1); - const forgePublicKey = (forge.pki as any).setRsaPublicKey(forgePrivateKey.n, forgePrivateKey.e); - const publicKeyAsn1 = forge.pki.publicKeyToAsn1(forgePublicKey); - const publicKeyByteString = forge.asn1.toDer(publicKeyAsn1).data; - const publicKeyArray = Utils.fromByteStringToArray(publicKeyByteString); - return publicKeyArray as UnsignedPublicKey; + await SdkLoadService.Ready; + return PureCrypto.rsa_extract_public_key(privateKey) as UnsignedPublicKey; } - async rsaGenerateKeyPair(length: 1024 | 2048 | 4096): Promise<[UnsignedPublicKey, Uint8Array]> { - return new Promise<[UnsignedPublicKey, Uint8Array]>((resolve, reject) => { - forge.pki.rsa.generateKeyPair( - { - bits: length, - workers: -1, - e: 0x10001, // 65537 - }, - (error, keyPair) => { - if (error != null) { - reject(error); - return; - } - - const publicKeyAsn1 = forge.pki.publicKeyToAsn1(keyPair.publicKey); - const publicKeyByteString = forge.asn1.toDer(publicKeyAsn1).getBytes(); - const publicKey = Utils.fromByteStringToArray(publicKeyByteString); - - const privateKeyAsn1 = forge.pki.privateKeyToAsn1(keyPair.privateKey); - const privateKeyPkcs8 = forge.pki.wrapRsaPrivateKey(privateKeyAsn1); - const privateKeyByteString = forge.asn1.toDer(privateKeyPkcs8).getBytes(); - const privateKey = Utils.fromByteStringToArray(privateKeyByteString); - - resolve([publicKey as UnsignedPublicKey, privateKey]); - }, - ); - }); + async rsaGenerateKeyPair(_length: 2048): Promise<[UnsignedPublicKey, Uint8Array]> { + await SdkLoadService.Ready; + const privateKey = PureCrypto.rsa_generate_keypair(); + const publicKey = await this.rsaExtractPublicKey(privateKey); + return [publicKey, privateKey]; } aesGenerateKey(bitLength: 128 | 192 | 256 | 512): Promise { From 17ebae11d7a28d67b513a68e1477ac9a7a063e9d Mon Sep 17 00:00:00 2001 From: cyprain-okeke <108260115+cyprain-okeke@users.noreply.github.com> Date: Wed, 3 Dec 2025 16:08:48 +0100 Subject: [PATCH 03/12] Fix the bug by hiding the add button (#17744) --- .../clients/manage-clients.component.html | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/clients/manage-clients.component.html b/bitwarden_license/bit-web/src/app/admin-console/providers/clients/manage-clients.component.html index 2ab82bd837b..ce89b3c068d 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/clients/manage-clients.component.html +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/clients/manage-clients.component.html @@ -2,18 +2,20 @@ @let provider = provider$ | async; - + @if (provider?.type === ProviderUserType.ProviderAdmin) { + + } `; - const shadowRoot = document.getElementById("shadow-root").attachShadow({ mode: "open" }); + const shadowRoot = document.getElementById("shadow-root")!.attachShadow({ mode: "open" }); shadowRoot.innerHTML = ` `; @@ -1668,7 +1668,7 @@ describe("AutofillOverlayContentService", () => { pageDetailsMock, ); await flushPromises(); - buttonElement.dispatchEvent(new KeyboardEvent("keyup", { code: "Enter" })); + buttonElement?.dispatchEvent(new KeyboardEvent("keyup", { code: "Enter" })); expect(sendExtensionMessageSpy).toHaveBeenCalledWith( "formFieldSubmitted", @@ -1716,6 +1716,85 @@ describe("AutofillOverlayContentService", () => { }); }); + describe("refreshMenuLayerPosition", () => { + it("calls refreshTopLayerPosition on the inline menu content service", () => { + autofillOverlayContentService.refreshMenuLayerPosition(); + + expect(inlineMenuContentService.refreshTopLayerPosition).toHaveBeenCalled(); + }); + + it("does not throw if inline menu content service is not available", () => { + const serviceWithoutInlineMenu = new AutofillOverlayContentService( + domQueryService, + domElementVisibilityService, + inlineMenuFieldQualificationService, + ); + + expect(() => serviceWithoutInlineMenu.refreshMenuLayerPosition()).not.toThrow(); + }); + }); + + describe("getOwnedInlineMenuTagNames", () => { + it("returns tag names from the inline menu content service", () => { + inlineMenuContentService.getOwnedTagNames.mockReturnValue(["div", "span"]); + + const result = autofillOverlayContentService.getOwnedInlineMenuTagNames(); + + expect(result).toEqual(["div", "span"]); + }); + + it("returns an empty array if inline menu content service is not available", () => { + const serviceWithoutInlineMenu = new AutofillOverlayContentService( + domQueryService, + domElementVisibilityService, + inlineMenuFieldQualificationService, + ); + + const result = serviceWithoutInlineMenu.getOwnedInlineMenuTagNames(); + + expect(result).toEqual([]); + }); + }); + + describe("getUnownedTopLayerItems", () => { + it("returns unowned top layer items from the inline menu content service", () => { + const mockElements = document.querySelectorAll("div"); + inlineMenuContentService.getUnownedTopLayerItems.mockReturnValue(mockElements); + + const result = autofillOverlayContentService.getUnownedTopLayerItems(true); + + expect(result).toEqual(mockElements); + expect(inlineMenuContentService.getUnownedTopLayerItems).toHaveBeenCalledWith(true); + }); + + it("returns undefined if inline menu content service is not available", () => { + const serviceWithoutInlineMenu = new AutofillOverlayContentService( + domQueryService, + domElementVisibilityService, + inlineMenuFieldQualificationService, + ); + + const result = serviceWithoutInlineMenu.getUnownedTopLayerItems(); + + expect(result).toBeUndefined(); + }); + }); + + describe("clearUserFilledFields", () => { + it("deletes all user filled fields", () => { + const mockElement1 = document.createElement("input") as FillableFormFieldElement; + const mockElement2 = document.createElement("input") as FillableFormFieldElement; + autofillOverlayContentService["userFilledFields"] = { + username: mockElement1, + password: mockElement2, + }; + + autofillOverlayContentService.clearUserFilledFields(); + + expect(autofillOverlayContentService["userFilledFields"]).toEqual({}); + }); + }); + describe("handleOverlayRepositionEvent", () => { const repositionEvents = [EVENTS.SCROLL, EVENTS.RESIZE]; repositionEvents.forEach((repositionEvent) => { @@ -2049,7 +2128,7 @@ describe("AutofillOverlayContentService", () => { }); it("skips focusing an element if no recently focused field exists", async () => { - autofillOverlayContentService["mostRecentlyFocusedField"] = undefined; + (autofillOverlayContentService as any)["mostRecentlyFocusedField"] = null; sendMockExtensionMessage({ command: "redirectAutofillInlineMenuFocusOut", @@ -2149,7 +2228,6 @@ describe("AutofillOverlayContentService", () => { }); it("returns null if the sub frame URL cannot be parsed correctly", async () => { - delete globalThis.location; globalThis.location = { href: "invalid-base" } as Location; sendMockExtensionMessage( { diff --git a/apps/browser/src/autofill/services/inline-menu-field-qualification.service.ts b/apps/browser/src/autofill/services/inline-menu-field-qualification.service.ts index f7c46a9fa77..f6afaae202f 100644 --- a/apps/browser/src/autofill/services/inline-menu-field-qualification.service.ts +++ b/apps/browser/src/autofill/services/inline-menu-field-qualification.service.ts @@ -945,7 +945,8 @@ export class InlineMenuFieldQualificationService !fieldType || !this.usernameFieldTypes.has(fieldType) || this.isExcludedFieldType(field, this.excludedAutofillFieldTypesSet) || - this.fieldHasDisqualifyingAttributeValue(field) + this.fieldHasDisqualifyingAttributeValue(field) || + this.isTotpField(field) ) { return false; } From 5f9759fde13c86cf4cd580da61919f7e7bd50de7 Mon Sep 17 00:00:00 2001 From: Vince Grassia <593223+vgrassia@users.noreply.github.com> Date: Wed, 3 Dec 2025 12:10:42 -0500 Subject: [PATCH 05/12] Update Linux build job in Build Desktop workflow to free up space on disk (#17784) --- .github/workflows/build-desktop.yml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/.github/workflows/build-desktop.yml b/.github/workflows/build-desktop.yml index 949263b34b7..c973796207c 100644 --- a/.github/workflows/build-desktop.yml +++ b/.github/workflows/build-desktop.yml @@ -175,9 +175,23 @@ jobs: - name: Check out repo uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1 with: + fetch-depth: 1 ref: ${{ github.event.pull_request.head.sha }} persist-credentials: false + - name: Free disk space for build + run: | + sudo rm -rf /usr/share/dotnet + sudo rm -rf /usr/share/swift + sudo rm -rf /usr/local/.ghcup + sudo rm -rf /usr/share/miniconda + sudo rm -rf /usr/share/az_* + sudo rm -rf /usr/local/julia* + sudo rm -rf /usr/lib/mono + sudo rm -rf /usr/lib/heroku + sudo rm -rf /usr/local/aws-cli + sudo rm -rf /usr/local/aws-sam-cli + - name: Set up Node uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5.0.0 with: From 6ae096485af03192fb30f8a26af0428ffc0601f1 Mon Sep 17 00:00:00 2001 From: "Michael L." Date: Wed, 3 Dec 2025 18:14:07 +0100 Subject: [PATCH 06/12] Add support for Helium browser integration on mac (#17293) Co-authored-by: Addison Beck --- apps/desktop/resources/entitlements.mas.plist | 3 ++- apps/desktop/src/main/native-messaging.main.ts | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/desktop/resources/entitlements.mas.plist b/apps/desktop/resources/entitlements.mas.plist index 3ebd56f0fd7..2977e5fd786 100644 --- a/apps/desktop/resources/entitlements.mas.plist +++ b/apps/desktop/resources/entitlements.mas.plist @@ -32,8 +32,9 @@ /Library/Application Support/Microsoft Edge Beta/NativeMessagingHosts/ /Library/Application Support/Microsoft Edge Dev/NativeMessagingHosts/ /Library/Application Support/Microsoft Edge Canary/NativeMessagingHosts/ - /Library/Application Support/Vivaldi/NativeMessagingHosts/ + /Library/Application Support/Vivaldi/NativeMessagingHosts/ /Library/Application Support/Zen/NativeMessagingHosts/ + /Library/Application Support/net.imput.helium com.apple.security.cs.allow-jit diff --git a/apps/desktop/src/main/native-messaging.main.ts b/apps/desktop/src/main/native-messaging.main.ts index ba5d8616752..23d2e038635 100644 --- a/apps/desktop/src/main/native-messaging.main.ts +++ b/apps/desktop/src/main/native-messaging.main.ts @@ -314,6 +314,7 @@ export class NativeMessagingMain { "Microsoft Edge Canary": `${this.homedir()}/Library/Application\ Support/Microsoft\ Edge\ Canary/`, Vivaldi: `${this.homedir()}/Library/Application\ Support/Vivaldi/`, Zen: `${this.homedir()}/Library/Application\ Support/Zen/`, + Helium: `${this.homedir()}/Library/Application\ Support/net.imput.helium/`, }; /* eslint-enable no-useless-escape */ } From 6e2203d6d4b56ec33a39e26c68a97d5e869fe081 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Wed, 3 Dec 2025 19:04:18 +0100 Subject: [PATCH 07/12] [PM-18026] Implement forced, automatic KDF upgrades (#15937) * Implement automatic kdf upgrades * Fix kdf config not being updated * Update legacy kdf state on master password unlock sync * Fix cli build * Fix * Deduplicate prompts * Fix dismiss time * Fix default kdf setting * Fix build * Undo changes * Fix test * Fix prettier * Fix test * Update libs/angular/src/key-management/encrypted-migration/encrypted-migrations-scheduler.service.ts Co-authored-by: Maciej Zieniuk <167752252+mzieniukbw@users.noreply.github.com> * Update libs/common/src/key-management/master-password/abstractions/master-password.service.abstraction.ts Co-authored-by: Maciej Zieniuk <167752252+mzieniukbw@users.noreply.github.com> * Update libs/angular/src/key-management/encrypted-migration/encrypted-migrations-scheduler.service.ts Co-authored-by: Maciej Zieniuk <167752252+mzieniukbw@users.noreply.github.com> * Only sync when there is at least one migration * Relative imports * Add tech debt comment * Resolve inconsistent prefix * Clean up * Update docs * Use default PBKDF2 iteratinos instead of custom threshold * Undo type check * Fix build * Add comment * Cleanup * Cleanup * Address component feedback * Use isnullorwhitespace * Fix tests * Allow migration only on vault * Fix tests * Run prettier * Fix tests * Prevent await race condition * Fix min and default values in kdf migration * Run sync only when a migration was run * Update libs/common/src/key-management/encrypted-migrator/default-encrypted-migrator.ts Co-authored-by: Maciej Zieniuk <167752252+mzieniukbw@users.noreply.github.com> * Fix link not being blue * Fix later button on browser --------- Co-authored-by: Maciej Zieniuk <167752252+mzieniukbw@users.noreply.github.com> --- apps/browser/src/_locales/en/messages.json | 21 ++ apps/cli/src/auth/commands/login.command.ts | 4 + apps/cli/src/base-program.ts | 1 + .../commands/unlock.command.spec.ts | 3 + .../key-management/commands/unlock.command.ts | 4 + apps/cli/src/oss-serve-configurator.ts | 1 + apps/cli/src/program.ts | 2 + .../service-container/service-container.ts | 15 + apps/desktop/src/locales/en/messages.json | 18 ++ .../app/auth/recover-two-factor.component.ts | 2 +- .../change-kdf-confirmation.component.spec.ts | 2 +- .../change-kdf-confirmation.component.ts | 2 +- apps/web/src/locales/en/messages.json | 18 ++ .../login-via-webauthn.component.ts | 2 +- ...igrations-scheduler.service.abstraction.ts | 9 + ...ypted-migrations-scheduler.service.spec.ts | 270 ++++++++++++++++++ .../encrypted-migrations-scheduler.service.ts | 188 ++++++++++++ .../prompt-migration-password.component.html | 55 ++++ .../prompt-migration-password.component.ts | 85 ++++++ .../src/services/jslib-services.module.ts | 52 +++- .../login-via-auth-request.component.ts | 2 +- .../auth/src/angular/login/login.component.ts | 2 +- .../new-device-verification.component.ts | 4 +- .../registration-finish.component.ts | 5 +- libs/auth/src/angular/sso/sso.component.ts | 2 +- .../two-factor-auth.component.ts | 2 +- .../login-success-handler.service.ts | 3 +- .../login-strategies/login.strategy.spec.ts | 2 + .../common/login-strategies/login.strategy.ts | 5 + .../password-login.strategy.ts | 4 + ...ault-login-success-handler.service.spec.ts | 14 +- .../default-login-success-handler.service.ts | 10 +- .../src/auth/models/domain/auth-result.ts | 2 + .../default-encrypted-migrator.spec.ts | 194 +++++++++++++ .../default-encrypted-migrator.ts | 113 ++++++++ .../encrypted-migrator.abstraction.ts | 32 +++ .../migrations/encrypted-migration.ts | 36 +++ .../migrations/minimum-kdf-migration.spec.ts | 184 ++++++++++++ .../migrations/minimum-kdf-migration.ts | 68 +++++ ...n.ts => change-kdf.service.abstraction.ts} | 0 ...ice.spec.ts => change-kdf.service.spec.ts} | 2 +- ...e-kdf-service.ts => change-kdf.service.ts} | 2 +- .../master-password.service.abstraction.ts | 7 + .../services/fake-master-password.service.ts | 4 + .../services/master-password.service.ts | 15 + .../lock/components/lock.component.spec.ts | 3 + .../src/lock/components/lock.component.ts | 13 + libs/state/src/core/state-definitions.ts | 18 +- 48 files changed, 1471 insertions(+), 31 deletions(-) create mode 100644 libs/angular/src/key-management/encrypted-migration/encrypted-migrations-scheduler.service.abstraction.ts create mode 100644 libs/angular/src/key-management/encrypted-migration/encrypted-migrations-scheduler.service.spec.ts create mode 100644 libs/angular/src/key-management/encrypted-migration/encrypted-migrations-scheduler.service.ts create mode 100644 libs/angular/src/key-management/encrypted-migration/prompt-migration-password.component.html create mode 100644 libs/angular/src/key-management/encrypted-migration/prompt-migration-password.component.ts create mode 100644 libs/common/src/key-management/encrypted-migrator/default-encrypted-migrator.spec.ts create mode 100644 libs/common/src/key-management/encrypted-migrator/default-encrypted-migrator.ts create mode 100644 libs/common/src/key-management/encrypted-migrator/encrypted-migrator.abstraction.ts create mode 100644 libs/common/src/key-management/encrypted-migrator/migrations/encrypted-migration.ts create mode 100644 libs/common/src/key-management/encrypted-migrator/migrations/minimum-kdf-migration.spec.ts create mode 100644 libs/common/src/key-management/encrypted-migrator/migrations/minimum-kdf-migration.ts rename libs/common/src/key-management/kdf/{change-kdf-service.abstraction.ts => change-kdf.service.abstraction.ts} (100%) rename libs/common/src/key-management/kdf/{change-kdf-service.spec.ts => change-kdf.service.spec.ts} (99%) rename libs/common/src/key-management/kdf/{change-kdf-service.ts => change-kdf.service.ts} (97%) diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index 6a7df1678bf..bbdea838e62 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -1406,6 +1406,27 @@ "learnMore": { "message": "Learn more" }, + "migrationsFailed": { + "message": "An error occurred updating the encryption settings." + }, + "updateEncryptionSettingsTitle": { + "message": "Update your encryption settings" + }, + "updateEncryptionSettingsDesc": { + "message": "The new recommended encryption settings will improve your account security. Enter your master password to update now." + }, + "confirmIdentityToContinue": { + "message": "Confirm your identity to continue" + }, + "enterYourMasterPassword": { + "message": "Enter your master password" + }, + "updateSettings": { + "message": "Update settings" + }, + "later": { + "message": "Later" + }, "authenticatorKeyTotp": { "message": "Authenticator key (TOTP)" }, diff --git a/apps/cli/src/auth/commands/login.command.ts b/apps/cli/src/auth/commands/login.command.ts index d0ab062d0b3..661e052fb72 100644 --- a/apps/cli/src/auth/commands/login.command.ts +++ b/apps/cli/src/auth/commands/login.command.ts @@ -31,6 +31,7 @@ import { TwoFactorService, TwoFactorApiService } from "@bitwarden/common/auth/tw import { ClientType } from "@bitwarden/common/enums"; import { CryptoFunctionService } from "@bitwarden/common/key-management/crypto/abstractions/crypto-function.service"; import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string"; +import { EncryptedMigrator } from "@bitwarden/common/key-management/encrypted-migrator/encrypted-migrator.abstraction"; import { KeyConnectorService } from "@bitwarden/common/key-management/key-connector/abstractions/key-connector.service"; import { MasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; import { ErrorResponse } from "@bitwarden/common/models/response/error.response"; @@ -81,6 +82,7 @@ export class LoginCommand { protected ssoUrlService: SsoUrlService, protected i18nService: I18nService, protected masterPasswordService: MasterPasswordServiceAbstraction, + protected encryptedMigrator: EncryptedMigrator, ) {} async run(email: string, password: string, options: OptionValues) { @@ -367,6 +369,8 @@ export class LoginCommand { } } + await this.encryptedMigrator.runMigrations(response.userId, password); + return await this.handleSuccessResponse(response); } catch (e) { if ( diff --git a/apps/cli/src/base-program.ts b/apps/cli/src/base-program.ts index 69a5e4e1bde..71c3830b4cc 100644 --- a/apps/cli/src/base-program.ts +++ b/apps/cli/src/base-program.ts @@ -182,6 +182,7 @@ export abstract class BaseProgram { this.serviceContainer.organizationApiService, this.serviceContainer.logout, this.serviceContainer.i18nService, + this.serviceContainer.encryptedMigrator, this.serviceContainer.masterPasswordUnlockService, this.serviceContainer.configService, ); diff --git a/apps/cli/src/key-management/commands/unlock.command.spec.ts b/apps/cli/src/key-management/commands/unlock.command.spec.ts index 928a750dca6..70e9a8fd232 100644 --- a/apps/cli/src/key-management/commands/unlock.command.spec.ts +++ b/apps/cli/src/key-management/commands/unlock.command.spec.ts @@ -7,6 +7,7 @@ import { UserVerificationService } from "@bitwarden/common/auth/abstractions/use import { VerificationType } from "@bitwarden/common/auth/enums/verification-type"; import { MasterPasswordVerificationResponse } from "@bitwarden/common/auth/types/verification"; import { CryptoFunctionService } from "@bitwarden/common/key-management/crypto/abstractions/crypto-function.service"; +import { EncryptedMigrator } from "@bitwarden/common/key-management/encrypted-migrator/encrypted-migrator.abstraction"; import { KeyConnectorService } from "@bitwarden/common/key-management/key-connector/abstractions/key-connector.service"; import { MasterPasswordUnlockService } from "@bitwarden/common/key-management/master-password/abstractions/master-password-unlock.service"; import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; @@ -40,6 +41,7 @@ describe("UnlockCommand", () => { const organizationApiService = mock(); const logout = jest.fn(); const i18nService = mock(); + const encryptedMigrator = mock(); const masterPasswordUnlockService = mock(); const configService = mock(); @@ -92,6 +94,7 @@ describe("UnlockCommand", () => { organizationApiService, logout, i18nService, + encryptedMigrator, masterPasswordUnlockService, configService, ); diff --git a/apps/cli/src/key-management/commands/unlock.command.ts b/apps/cli/src/key-management/commands/unlock.command.ts index 4ae8ce823a4..c88d9ae1cc4 100644 --- a/apps/cli/src/key-management/commands/unlock.command.ts +++ b/apps/cli/src/key-management/commands/unlock.command.ts @@ -9,6 +9,7 @@ import { VerificationType } from "@bitwarden/common/auth/enums/verification-type import { MasterPasswordVerification } from "@bitwarden/common/auth/types/verification"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { CryptoFunctionService } from "@bitwarden/common/key-management/crypto/abstractions/crypto-function.service"; +import { EncryptedMigrator } from "@bitwarden/common/key-management/encrypted-migrator/encrypted-migrator.abstraction"; import { KeyConnectorService } from "@bitwarden/common/key-management/key-connector/abstractions/key-connector.service"; import { MasterPasswordUnlockService } from "@bitwarden/common/key-management/master-password/abstractions/master-password-unlock.service"; import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; @@ -38,6 +39,7 @@ export class UnlockCommand { private organizationApiService: OrganizationApiServiceAbstraction, private logout: () => Promise, private i18nService: I18nService, + private encryptedMigrator: EncryptedMigrator, private masterPasswordUnlockService: MasterPasswordUnlockService, private configService: ConfigService, ) {} @@ -116,6 +118,8 @@ export class UnlockCommand { } } + await this.encryptedMigrator.runMigrations(userId, password); + return this.successResponse(); } diff --git a/apps/cli/src/oss-serve-configurator.ts b/apps/cli/src/oss-serve-configurator.ts index bd51cf4dd91..dbe17224d07 100644 --- a/apps/cli/src/oss-serve-configurator.ts +++ b/apps/cli/src/oss-serve-configurator.ts @@ -176,6 +176,7 @@ export class OssServeConfigurator { this.serviceContainer.organizationApiService, async () => await this.serviceContainer.logout(), this.serviceContainer.i18nService, + this.serviceContainer.encryptedMigrator, this.serviceContainer.masterPasswordUnlockService, this.serviceContainer.configService, ); diff --git a/apps/cli/src/program.ts b/apps/cli/src/program.ts index a47278db089..3e5b5678629 100644 --- a/apps/cli/src/program.ts +++ b/apps/cli/src/program.ts @@ -195,6 +195,7 @@ export class Program extends BaseProgram { this.serviceContainer.ssoUrlService, this.serviceContainer.i18nService, this.serviceContainer.masterPasswordService, + this.serviceContainer.encryptedMigrator, ); const response = await command.run(email, password, options); this.processResponse(response, true); @@ -311,6 +312,7 @@ export class Program extends BaseProgram { this.serviceContainer.organizationApiService, async () => await this.serviceContainer.logout(), this.serviceContainer.i18nService, + this.serviceContainer.encryptedMigrator, this.serviceContainer.masterPasswordUnlockService, this.serviceContainer.configService, ); diff --git a/apps/cli/src/service-container/service-container.ts b/apps/cli/src/service-container/service-container.ts index c163b7581b4..e29bc517f24 100644 --- a/apps/cli/src/service-container/service-container.ts +++ b/apps/cli/src/service-container/service-container.ts @@ -76,6 +76,10 @@ import { import { EncryptServiceImplementation } from "@bitwarden/common/key-management/crypto/services/encrypt.service.implementation"; import { DeviceTrustServiceAbstraction } from "@bitwarden/common/key-management/device-trust/abstractions/device-trust.service.abstraction"; import { DeviceTrustService } from "@bitwarden/common/key-management/device-trust/services/device-trust.service.implementation"; +import { DefaultEncryptedMigrator } from "@bitwarden/common/key-management/encrypted-migrator/default-encrypted-migrator"; +import { EncryptedMigrator } from "@bitwarden/common/key-management/encrypted-migrator/encrypted-migrator.abstraction"; +import { DefaultChangeKdfApiService } from "@bitwarden/common/key-management/kdf/change-kdf-api.service"; +import { DefaultChangeKdfService } from "@bitwarden/common/key-management/kdf/change-kdf.service"; import { KeyConnectorService } from "@bitwarden/common/key-management/key-connector/services/key-connector.service"; import { MasterPasswordUnlockService } from "@bitwarden/common/key-management/master-password/abstractions/master-password-unlock.service"; import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; @@ -324,6 +328,7 @@ export class ServiceContainer { cipherEncryptionService: CipherEncryptionService; restrictedItemTypesService: RestrictedItemTypesService; cliRestrictedItemTypesService: CliRestrictedItemTypesService; + encryptedMigrator: EncryptedMigrator; securityStateService: SecurityStateService; masterPasswordUnlockService: MasterPasswordUnlockService; cipherArchiveService: CipherArchiveService; @@ -975,6 +980,16 @@ export class ServiceContainer { ); this.masterPasswordApiService = new MasterPasswordApiService(this.apiService, this.logService); + const changeKdfApiService = new DefaultChangeKdfApiService(this.apiService); + const changeKdfService = new DefaultChangeKdfService(changeKdfApiService, this.sdkService); + this.encryptedMigrator = new DefaultEncryptedMigrator( + this.kdfConfigService, + changeKdfService, + this.logService, + this.configService, + this.masterPasswordService, + this.syncService, + ); } async logout() { diff --git a/apps/desktop/src/locales/en/messages.json b/apps/desktop/src/locales/en/messages.json index 757059c4e41..8da3ba54844 100644 --- a/apps/desktop/src/locales/en/messages.json +++ b/apps/desktop/src/locales/en/messages.json @@ -1093,6 +1093,24 @@ "learnMore": { "message": "Learn more" }, + "migrationsFailed": { + "message": "An error occurred updating the encryption settings." + }, + "updateEncryptionSettingsTitle": { + "message": "Update your encryption settings" + }, + "updateEncryptionSettingsDesc": { + "message": "The new recommended encryption settings will improve your account security. Enter your master password to update now." + }, + "confirmIdentityToContinue": { + "message": "Confirm your identity to continue" + }, + "enterYourMasterPassword": { + "message": "Enter your master password" + }, + "updateSettings": { + "message": "Update settings" + }, "featureUnavailable": { "message": "Feature unavailable" }, diff --git a/apps/web/src/app/auth/recover-two-factor.component.ts b/apps/web/src/app/auth/recover-two-factor.component.ts index 9c033b88a75..20f40b5319a 100644 --- a/apps/web/src/app/auth/recover-two-factor.component.ts +++ b/apps/web/src/app/auth/recover-two-factor.component.ts @@ -108,7 +108,7 @@ export class RecoverTwoFactorComponent implements OnInit { message: this.i18nService.t("twoStepRecoverDisabled"), }); - await this.loginSuccessHandlerService.run(authResult.userId); + await this.loginSuccessHandlerService.run(authResult.userId, this.masterPassword); await this.router.navigate(["/settings/security/two-factor"]); } catch (error: unknown) { diff --git a/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.spec.ts b/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.spec.ts index 525ddd89675..2c2caba7b3a 100644 --- a/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.spec.ts +++ b/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.spec.ts @@ -4,7 +4,7 @@ import { mock, MockProxy } from "jest-mock-extended"; import { of } from "rxjs"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; -import { ChangeKdfService } from "@bitwarden/common/key-management/kdf/change-kdf-service.abstraction"; +import { ChangeKdfService } from "@bitwarden/common/key-management/kdf/change-kdf.service.abstraction"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; diff --git a/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.ts b/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.ts index b730a3597ba..ffeabffa019 100644 --- a/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.ts +++ b/apps/web/src/app/key-management/change-kdf/change-kdf-confirmation.component.ts @@ -5,7 +5,7 @@ import { firstValueFrom, Observable } from "rxjs"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; -import { ChangeKdfService } from "@bitwarden/common/key-management/kdf/change-kdf-service.abstraction"; +import { ChangeKdfService } from "@bitwarden/common/key-management/kdf/change-kdf.service.abstraction"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index 582efade7f4..19eec245885 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -4621,6 +4621,24 @@ "learnMore": { "message": "Learn more" }, + "migrationsFailed": { + "message": "An error occurred updating the encryption settings." + }, + "updateEncryptionSettingsTitle": { + "message": "Update your encryption settings" + }, + "updateEncryptionSettingsDesc": { + "message": "The new recommended encryption settings will improve your account security. Enter your master password to update now." + }, + "confirmIdentityToContinue": { + "message": "Confirm your identity to continue" + }, + "enterYourMasterPassword": { + "message": "Enter your master password" + }, + "updateSettings": { + "message": "Update settings" + }, "deleteRecoverDesc": { "message": "Enter your email address below to recover and delete your account." }, diff --git a/libs/angular/src/auth/login-via-webauthn/login-via-webauthn.component.ts b/libs/angular/src/auth/login-via-webauthn/login-via-webauthn.component.ts index fa2a01fe8e1..764d9fe7733 100644 --- a/libs/angular/src/auth/login-via-webauthn/login-via-webauthn.component.ts +++ b/libs/angular/src/auth/login-via-webauthn/login-via-webauthn.component.ts @@ -120,7 +120,7 @@ export class LoginViaWebAuthnComponent implements OnInit { // Only run loginSuccessHandlerService if webAuthn is used for vault decryption. const userKey = await firstValueFrom(this.keyService.userKey$(authResult.userId)); if (userKey) { - await this.loginSuccessHandlerService.run(authResult.userId); + await this.loginSuccessHandlerService.run(authResult.userId, null); } await this.router.navigate([this.successRoute]); diff --git a/libs/angular/src/key-management/encrypted-migration/encrypted-migrations-scheduler.service.abstraction.ts b/libs/angular/src/key-management/encrypted-migration/encrypted-migrations-scheduler.service.abstraction.ts new file mode 100644 index 00000000000..565cbb02cf0 --- /dev/null +++ b/libs/angular/src/key-management/encrypted-migration/encrypted-migrations-scheduler.service.abstraction.ts @@ -0,0 +1,9 @@ +import { UserId } from "@bitwarden/common/types/guid"; + +export abstract class EncryptedMigrationsSchedulerService { + /** + * Runs migrations for a user if needed, handling both interactive and non-interactive cases + * @param userId The user ID to run migrations for + */ + abstract runMigrationsIfNeeded(userId: UserId): Promise; +} diff --git a/libs/angular/src/key-management/encrypted-migration/encrypted-migrations-scheduler.service.spec.ts b/libs/angular/src/key-management/encrypted-migration/encrypted-migrations-scheduler.service.spec.ts new file mode 100644 index 00000000000..76cfbc0bfdd --- /dev/null +++ b/libs/angular/src/key-management/encrypted-migration/encrypted-migrations-scheduler.service.spec.ts @@ -0,0 +1,270 @@ +import { Router } from "@angular/router"; +import { mock } from "jest-mock-extended"; +import { of } from "rxjs"; + +import { AccountInfo } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; +import { EncryptedMigrator } from "@bitwarden/common/key-management/encrypted-migrator/encrypted-migrator.abstraction"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { SingleUserState, StateProvider } from "@bitwarden/common/platform/state"; +import { SyncService } from "@bitwarden/common/platform/sync"; +import { FakeAccountService } from "@bitwarden/common/spec"; +import { UserId } from "@bitwarden/common/types/guid"; +import { DialogService, ToastService } from "@bitwarden/components"; +import { LogService } from "@bitwarden/logging"; + +import { + DefaultEncryptedMigrationsSchedulerService, + ENCRYPTED_MIGRATION_DISMISSED, +} from "./encrypted-migrations-scheduler.service"; +import { PromptMigrationPasswordComponent } from "./prompt-migration-password.component"; + +const SomeUser = "SomeUser" as UserId; +const AnotherUser = "SomeOtherUser" as UserId; +const accounts: Record = { + [SomeUser]: { + name: "some user", + email: "some.user@example.com", + emailVerified: true, + }, + [AnotherUser]: { + name: "some other user", + email: "some.other.user@example.com", + emailVerified: true, + }, +}; + +describe("DefaultEncryptedMigrationsSchedulerService", () => { + let service: DefaultEncryptedMigrationsSchedulerService; + const mockAccountService = new FakeAccountService(accounts); + const mockAuthService = mock(); + const mockEncryptedMigrator = mock(); + const mockStateProvider = mock(); + const mockSyncService = mock(); + const mockDialogService = mock(); + const mockToastService = mock(); + const mockI18nService = mock(); + const mockLogService = mock(); + const mockRouter = mock(); + + const mockUserId = "test-user-id" as UserId; + const mockMasterPassword = "test-master-password"; + + const createMockUserState = (value: T): jest.Mocked> => + ({ + state$: of(value), + userId: mockUserId, + update: jest.fn(), + combinedState$: of([mockUserId, value]), + }) as any; + + beforeEach(() => { + const mockDialogRef = { + closed: of(mockMasterPassword), + }; + + jest.spyOn(PromptMigrationPasswordComponent, "open").mockReturnValue(mockDialogRef as any); + mockI18nService.t.mockReturnValue("translated_migrationsFailed"); + (mockRouter as any)["events"] = of({ url: "/vault" }) as any; + + service = new DefaultEncryptedMigrationsSchedulerService( + mockSyncService, + mockAccountService, + mockStateProvider, + mockEncryptedMigrator, + mockAuthService, + mockLogService, + mockDialogService, + mockToastService, + mockI18nService, + mockRouter, + ); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + describe("runMigrationsIfNeeded", () => { + it("should return early if user is not unlocked", async () => { + mockAuthService.authStatusFor$.mockReturnValue(of(AuthenticationStatus.Locked)); + + await service.runMigrationsIfNeeded(mockUserId); + + expect(mockEncryptedMigrator.needsMigrations).not.toHaveBeenCalled(); + expect(mockLogService.info).not.toHaveBeenCalled(); + }); + + it("should log and return when no migration is needed", async () => { + mockAuthService.authStatusFor$.mockReturnValue(of(AuthenticationStatus.Unlocked)); + mockEncryptedMigrator.needsMigrations.mockResolvedValue("noMigrationNeeded"); + + await service.runMigrationsIfNeeded(mockUserId); + + expect(mockEncryptedMigrator.needsMigrations).toHaveBeenCalledWith(mockUserId); + expect(mockLogService.info).toHaveBeenCalledWith( + `[EncryptedMigrationsScheduler] No migrations needed for user ${mockUserId}`, + ); + expect(mockEncryptedMigrator.runMigrations).not.toHaveBeenCalled(); + }); + + it("should run migrations without interaction when master password is not required", async () => { + mockAuthService.authStatusFor$.mockReturnValue(of(AuthenticationStatus.Unlocked)); + mockEncryptedMigrator.needsMigrations.mockResolvedValue("needsMigration"); + + await service.runMigrationsIfNeeded(mockUserId); + + expect(mockEncryptedMigrator.needsMigrations).toHaveBeenCalledWith(mockUserId); + expect(mockLogService.info).toHaveBeenCalledWith( + `[EncryptedMigrationsScheduler] User ${mockUserId} needs migrations with master password`, + ); + expect(mockEncryptedMigrator.runMigrations).toHaveBeenCalledWith(mockUserId, null); + }); + + it("should run migrations with interaction when migration is needed", async () => { + mockAuthService.authStatusFor$.mockReturnValue(of(AuthenticationStatus.Unlocked)); + mockEncryptedMigrator.needsMigrations.mockResolvedValue("needsMigrationWithMasterPassword"); + const mockUserState = createMockUserState(null); + mockStateProvider.getUser.mockReturnValue(mockUserState); + + await service.runMigrationsIfNeeded(mockUserId); + + expect(mockEncryptedMigrator.needsMigrations).toHaveBeenCalledWith(mockUserId); + expect(mockLogService.info).toHaveBeenCalledWith( + `[EncryptedMigrationsScheduler] User ${mockUserId} needs migrations with master password`, + ); + expect(PromptMigrationPasswordComponent.open).toHaveBeenCalledWith(mockDialogService); + expect(mockEncryptedMigrator.runMigrations).toHaveBeenCalledWith( + mockUserId, + mockMasterPassword, + ); + }); + }); + + describe("runMigrationsWithoutInteraction", () => { + it("should run migrations without master password", async () => { + mockAuthService.authStatusFor$.mockReturnValue(of(AuthenticationStatus.Unlocked)); + mockEncryptedMigrator.needsMigrations.mockResolvedValue("needsMigration"); + + await service.runMigrationsIfNeeded(mockUserId); + + expect(mockEncryptedMigrator.runMigrations).toHaveBeenCalledWith(mockUserId, null); + expect(mockLogService.error).not.toHaveBeenCalled(); + }); + + it("should handle errors during migration without interaction", async () => { + const mockError = new Error("Migration failed"); + mockAuthService.authStatusFor$.mockReturnValue(of(AuthenticationStatus.Unlocked)); + mockEncryptedMigrator.needsMigrations.mockResolvedValue("needsMigration"); + mockEncryptedMigrator.runMigrations.mockRejectedValue(mockError); + + await service.runMigrationsIfNeeded(mockUserId); + + expect(mockEncryptedMigrator.runMigrations).toHaveBeenCalledWith(mockUserId, null); + expect(mockLogService.error).toHaveBeenCalledWith( + "[EncryptedMigrationsScheduler] Error during migration without interaction", + mockError, + ); + }); + }); + + describe("runMigrationsWithInteraction", () => { + beforeEach(() => { + mockAuthService.authStatusFor$.mockReturnValue(of(AuthenticationStatus.Unlocked)); + mockEncryptedMigrator.needsMigrations.mockResolvedValue("needsMigrationWithMasterPassword"); + }); + + it("should skip if migration was dismissed recently", async () => { + const recentDismissDate = new Date(Date.now() - 12 * 60 * 60 * 1000); // 12 hours ago + const mockUserState = createMockUserState(recentDismissDate); + mockStateProvider.getUser.mockReturnValue(mockUserState); + + await service.runMigrationsIfNeeded(mockUserId); + + expect(mockStateProvider.getUser).toHaveBeenCalledWith( + mockUserId, + ENCRYPTED_MIGRATION_DISMISSED, + ); + expect(mockLogService.info).toHaveBeenCalledWith( + "[EncryptedMigrationsScheduler] Migration prompt dismissed recently, skipping for now.", + ); + expect(PromptMigrationPasswordComponent.open).not.toHaveBeenCalled(); + }); + + it("should prompt for migration if dismissed date is older than 24 hours", async () => { + const oldDismissDate = new Date(Date.now() - 25 * 60 * 60 * 1000); // 25 hours ago + const mockUserState = createMockUserState(oldDismissDate); + mockStateProvider.getUser.mockReturnValue(mockUserState); + + await service.runMigrationsIfNeeded(mockUserId); + + expect(mockStateProvider.getUser).toHaveBeenCalledWith( + mockUserId, + ENCRYPTED_MIGRATION_DISMISSED, + ); + expect(PromptMigrationPasswordComponent.open).toHaveBeenCalledWith(mockDialogService); + expect(mockEncryptedMigrator.runMigrations).toHaveBeenCalledWith( + mockUserId, + mockMasterPassword, + ); + }); + + it("should prompt for migration if no dismiss date exists", async () => { + const mockUserState = createMockUserState(null); + mockStateProvider.getUser.mockReturnValue(mockUserState); + + await service.runMigrationsIfNeeded(mockUserId); + + expect(PromptMigrationPasswordComponent.open).toHaveBeenCalledWith(mockDialogService); + expect(mockEncryptedMigrator.runMigrations).toHaveBeenCalledWith( + mockUserId, + mockMasterPassword, + ); + }); + + it("should set dismiss date when empty password is provided", async () => { + const mockUserState = createMockUserState(null); + mockStateProvider.getUser.mockReturnValue(mockUserState); + + const mockDialogRef = { + closed: of(""), // Empty password + }; + jest.spyOn(PromptMigrationPasswordComponent, "open").mockReturnValue(mockDialogRef as any); + + await service.runMigrationsIfNeeded(mockUserId); + + expect(PromptMigrationPasswordComponent.open).toHaveBeenCalledWith(mockDialogService); + expect(mockEncryptedMigrator.runMigrations).not.toHaveBeenCalled(); + expect(mockStateProvider.setUserState).toHaveBeenCalledWith( + ENCRYPTED_MIGRATION_DISMISSED, + expect.any(Date), + mockUserId, + ); + }); + + it("should handle errors during migration prompt and show toast", async () => { + const mockUserState = createMockUserState(null); + mockStateProvider.getUser.mockReturnValue(mockUserState); + + const mockError = new Error("Migration failed"); + mockEncryptedMigrator.runMigrations.mockRejectedValue(mockError); + + await service.runMigrationsIfNeeded(mockUserId); + + expect(PromptMigrationPasswordComponent.open).toHaveBeenCalledWith(mockDialogService); + expect(mockEncryptedMigrator.runMigrations).toHaveBeenCalledWith( + mockUserId, + mockMasterPassword, + ); + expect(mockLogService.error).toHaveBeenCalledWith( + "[EncryptedMigrationsScheduler] Error during migration prompt", + mockError, + ); + expect(mockToastService.showToast).toHaveBeenCalledWith({ + variant: "error", + message: "translated_migrationsFailed", + }); + }); + }); +}); diff --git a/libs/angular/src/key-management/encrypted-migration/encrypted-migrations-scheduler.service.ts b/libs/angular/src/key-management/encrypted-migration/encrypted-migrations-scheduler.service.ts new file mode 100644 index 00000000000..1c50919d1cb --- /dev/null +++ b/libs/angular/src/key-management/encrypted-migration/encrypted-migrations-scheduler.service.ts @@ -0,0 +1,188 @@ +import { NavigationEnd, Router } from "@angular/router"; +import { + combineLatest, + switchMap, + of, + firstValueFrom, + filter, + concatMap, + Observable, + map, +} from "rxjs"; + +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; +import { EncryptedMigrator } from "@bitwarden/common/key-management/encrypted-migrator/encrypted-migrator.abstraction"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { Utils } from "@bitwarden/common/platform/misc/utils"; +import { + UserKeyDefinition, + ENCRYPTED_MIGRATION_DISK, + StateProvider, +} from "@bitwarden/common/platform/state"; +import { SyncService } from "@bitwarden/common/platform/sync"; +import { UserId } from "@bitwarden/common/types/guid"; +import { DialogService, ToastService } from "@bitwarden/components"; +import { LogService } from "@bitwarden/logging"; + +import { EncryptedMigrationsSchedulerService } from "./encrypted-migrations-scheduler.service.abstraction"; +import { PromptMigrationPasswordComponent } from "./prompt-migration-password.component"; + +export const ENCRYPTED_MIGRATION_DISMISSED = new UserKeyDefinition( + ENCRYPTED_MIGRATION_DISK, + "encryptedMigrationDismissed", + { + deserializer: (obj: string) => (obj != null ? new Date(obj) : null), + clearOn: [], + }, +); +const DISMISS_TIME_HOURS = 24; +const VAULT_ROUTE = "/vault"; + +/** + * This services schedules encrypted migrations for users on clients that are interactive (non-cli), and handles manual interaction, + * if it is required by showing a UI prompt. It is only one means of triggering migrations, in case the user stays unlocked for a while, + * or regularly logs in without a master-password, when the migrations do require a master-password to run. + */ +export class DefaultEncryptedMigrationsSchedulerService + implements EncryptedMigrationsSchedulerService +{ + isMigrating = false; + url$: Observable; + + constructor( + private syncService: SyncService, + private accountService: AccountService, + private stateProvider: StateProvider, + private encryptedMigrator: EncryptedMigrator, + private authService: AuthService, + private logService: LogService, + private dialogService: DialogService, + private toastService: ToastService, + private i18nService: I18nService, + private router: Router, + ) { + this.url$ = this.router.events.pipe( + filter((event: any) => event instanceof NavigationEnd), + map((event: NavigationEnd) => event.url), + ); + + // For all accounts, if the auth status changes to unlocked or a sync happens, prompt for migration + this.accountService.accounts$ + .pipe( + switchMap((accounts) => { + const userIds = Object.keys(accounts) as UserId[]; + + if (userIds.length === 0) { + return of([]); + } + + return combineLatest( + userIds.map((userId) => + combineLatest([ + this.authService.authStatusFor$(userId), + this.syncService.lastSync$(userId).pipe(filter((lastSync) => lastSync != null)), + this.url$, + ]).pipe( + filter( + ([authStatus, _date, url]) => + authStatus === AuthenticationStatus.Unlocked && url === VAULT_ROUTE, + ), + concatMap(() => this.runMigrationsIfNeeded(userId)), + ), + ), + ); + }), + ) + .subscribe(); + } + + async runMigrationsIfNeeded(userId: UserId): Promise { + const authStatus = await firstValueFrom(this.authService.authStatusFor$(userId)); + if (authStatus !== AuthenticationStatus.Unlocked) { + return; + } + + if (this.isMigrating || this.encryptedMigrator.isRunningMigrations()) { + this.logService.info( + `[EncryptedMigrationsScheduler] Skipping migration check for user ${userId} because migrations are already in progress`, + ); + return; + } + + this.isMigrating = true; + switch (await this.encryptedMigrator.needsMigrations(userId)) { + case "noMigrationNeeded": + this.logService.info( + `[EncryptedMigrationsScheduler] No migrations needed for user ${userId}`, + ); + break; + case "needsMigrationWithMasterPassword": + this.logService.info( + `[EncryptedMigrationsScheduler] User ${userId} needs migrations with master password`, + ); + // If the user is unlocked, we can run migrations with the master password + await this.runMigrationsWithInteraction(userId); + break; + case "needsMigration": + this.logService.info( + `[EncryptedMigrationsScheduler] User ${userId} needs migrations with master password`, + ); + // If the user is unlocked, we can prompt for the master password + await this.runMigrationsWithoutInteraction(userId); + break; + } + this.isMigrating = false; + } + + private async runMigrationsWithoutInteraction(userId: UserId): Promise { + try { + await this.encryptedMigrator.runMigrations(userId, null); + } catch (error) { + this.logService.error( + "[EncryptedMigrationsScheduler] Error during migration without interaction", + error, + ); + } + } + + private async runMigrationsWithInteraction(userId: UserId): Promise { + // A dialog can be dismissed for a certain amount of time + const dismissedDate = await firstValueFrom( + this.stateProvider.getUser(userId, ENCRYPTED_MIGRATION_DISMISSED).state$, + ); + if (dismissedDate != null) { + const now = new Date(); + const timeDiff = now.getTime() - (dismissedDate as Date).getTime(); + const hoursDiff = timeDiff / (1000 * 60 * 60); + + if (hoursDiff < DISMISS_TIME_HOURS) { + this.logService.info( + "[EncryptedMigrationsScheduler] Migration prompt dismissed recently, skipping for now.", + ); + return; + } + } + + try { + const dialog = PromptMigrationPasswordComponent.open(this.dialogService); + const masterPassword = await firstValueFrom(dialog.closed); + if (Utils.isNullOrWhitespace(masterPassword)) { + await this.stateProvider.setUserState(ENCRYPTED_MIGRATION_DISMISSED, new Date(), userId); + } else { + await this.encryptedMigrator.runMigrations( + userId, + masterPassword === undefined ? null : masterPassword, + ); + } + } catch (error) { + this.logService.error("[EncryptedMigrationsScheduler] Error during migration prompt", error); + // If migrations failed when the user actively was prompted, show a toast + this.toastService.showToast({ + variant: "error", + message: this.i18nService.t("migrationsFailed"), + }); + } + } +} diff --git a/libs/angular/src/key-management/encrypted-migration/prompt-migration-password.component.html b/libs/angular/src/key-management/encrypted-migration/prompt-migration-password.component.html new file mode 100644 index 00000000000..6df08342885 --- /dev/null +++ b/libs/angular/src/key-management/encrypted-migration/prompt-migration-password.component.html @@ -0,0 +1,55 @@ +
+ +
+ {{ "updateEncryptionSettingsTitle" | i18n }} +
+
+

+ {{ "updateEncryptionSettingsDesc" | i18n }} + + {{ "learnMore" | i18n }} + + +

+ + {{ "masterPass" | i18n }} + {{ "confirmIdentityToContinue" | i18n }} + + + +
+ + + + + +
diff --git a/libs/angular/src/key-management/encrypted-migration/prompt-migration-password.component.ts b/libs/angular/src/key-management/encrypted-migration/prompt-migration-password.component.ts new file mode 100644 index 00000000000..060901d68fb --- /dev/null +++ b/libs/angular/src/key-management/encrypted-migration/prompt-migration-password.component.ts @@ -0,0 +1,85 @@ +import { CommonModule } from "@angular/common"; +import { Component, inject, ChangeDetectionStrategy } from "@angular/core"; +import { FormBuilder, ReactiveFormsModule, Validators } from "@angular/forms"; +import { filter, firstValueFrom, map } from "rxjs"; + +import { JslibModule } from "@bitwarden/angular/jslib.module"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; +import { VerificationType } from "@bitwarden/common/auth/enums/verification-type"; +import { + LinkModule, + AsyncActionsModule, + ButtonModule, + DialogModule, + DialogRef, + DialogService, + FormFieldModule, + IconButtonModule, +} from "@bitwarden/components"; + +/** + * This is a generic prompt to run encryption migrations that require the master password. + */ +@Component({ + changeDetection: ChangeDetectionStrategy.OnPush, + templateUrl: "prompt-migration-password.component.html", + imports: [ + DialogModule, + LinkModule, + CommonModule, + JslibModule, + ButtonModule, + IconButtonModule, + ReactiveFormsModule, + AsyncActionsModule, + FormFieldModule, + ], +}) +export class PromptMigrationPasswordComponent { + private dialogRef = inject(DialogRef); + private formBuilder = inject(FormBuilder); + private uvService = inject(UserVerificationService); + private accountService = inject(AccountService); + + migrationPasswordForm = this.formBuilder.group({ + masterPassword: ["", [Validators.required]], + }); + + static open(dialogService: DialogService) { + return dialogService.open(PromptMigrationPasswordComponent); + } + + submit = async () => { + const masterPasswordControl = this.migrationPasswordForm.controls.masterPassword; + + if (!masterPasswordControl.value || masterPasswordControl.invalid) { + return; + } + + const { userId, email } = await firstValueFrom( + this.accountService.activeAccount$.pipe( + filter((account) => account != null), + map((account) => { + return { + userId: account!.id, + email: account!.email, + }; + }), + ), + ); + + if ( + !(await this.uvService.verifyUserByMasterPassword( + { type: VerificationType.MasterPassword, secret: masterPasswordControl.value }, + userId, + email, + )) + ) { + return; + } + + // Return the master password to the caller + this.dialogRef.close(masterPasswordControl.value); + }; +} diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 1589f5c5f30..13935beab19 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -1,6 +1,7 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore -import { ErrorHandler, LOCALE_ID, NgModule } from "@angular/core"; +import { APP_INITIALIZER, ErrorHandler, LOCALE_ID, NgModule } from "@angular/core"; +import { Router } from "@angular/router"; import { Subject } from "rxjs"; // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. @@ -177,10 +178,12 @@ import { EncryptServiceImplementation } from "@bitwarden/common/key-management/c import { WebCryptoFunctionService } from "@bitwarden/common/key-management/crypto/services/web-crypto-function.service"; import { DeviceTrustServiceAbstraction } from "@bitwarden/common/key-management/device-trust/abstractions/device-trust.service.abstraction"; import { DeviceTrustService } from "@bitwarden/common/key-management/device-trust/services/device-trust.service.implementation"; +import { DefaultEncryptedMigrator } from "@bitwarden/common/key-management/encrypted-migrator/default-encrypted-migrator"; +import { EncryptedMigrator } from "@bitwarden/common/key-management/encrypted-migrator/encrypted-migrator.abstraction"; import { DefaultChangeKdfApiService } from "@bitwarden/common/key-management/kdf/change-kdf-api.service"; import { ChangeKdfApiService } from "@bitwarden/common/key-management/kdf/change-kdf-api.service.abstraction"; -import { DefaultChangeKdfService } from "@bitwarden/common/key-management/kdf/change-kdf-service"; -import { ChangeKdfService } from "@bitwarden/common/key-management/kdf/change-kdf-service.abstraction"; +import { DefaultChangeKdfService } from "@bitwarden/common/key-management/kdf/change-kdf.service"; +import { ChangeKdfService } from "@bitwarden/common/key-management/kdf/change-kdf.service.abstraction"; import { KeyConnectorService as KeyConnectorServiceAbstraction } from "@bitwarden/common/key-management/key-connector/abstractions/key-connector.service"; import { KeyConnectorService } from "@bitwarden/common/key-management/key-connector/services/key-connector.service"; import { KeyApiService } from "@bitwarden/common/key-management/keys/services/abstractions/key-api-service.abstraction"; @@ -328,6 +331,7 @@ import { DefaultTaskService, TaskService } from "@bitwarden/common/vault/tasks"; import { AnonLayoutWrapperDataService, DefaultAnonLayoutWrapperDataService, + DialogService, ToastService, } from "@bitwarden/components"; import { @@ -396,6 +400,8 @@ import { DeviceTrustToastService as DeviceTrustToastServiceAbstraction } from ". import { DeviceTrustToastService } from "../auth/services/device-trust-toast.service.implementation"; import { NoopPremiumInterestStateService } from "../billing/services/premium-interest/noop-premium-interest-state.service"; import { PremiumInterestStateService } from "../billing/services/premium-interest/premium-interest-state.service.abstraction"; +import { DefaultEncryptedMigrationsSchedulerService } from "../key-management/encrypted-migration/encrypted-migrations-scheduler.service"; +import { EncryptedMigrationsSchedulerService } from "../key-management/encrypted-migration/encrypted-migrations-scheduler.service.abstraction"; import { FormValidationErrorsService as FormValidationErrorsServiceAbstraction } from "../platform/abstractions/form-validation-errors.service"; import { DocumentLangSetter } from "../platform/i18n"; import { FormValidationErrorsService } from "../platform/services/form-validation-errors.service"; @@ -516,6 +522,23 @@ const safeProviders: SafeProvider[] = [ TokenServiceAbstraction, ], }), + safeProvider({ + provide: ChangeKdfService, + useClass: DefaultChangeKdfService, + deps: [ChangeKdfApiService, SdkService], + }), + safeProvider({ + provide: EncryptedMigrator, + useClass: DefaultEncryptedMigrator, + deps: [ + KdfConfigService, + ChangeKdfService, + LogService, + ConfigService, + MasterPasswordServiceAbstraction, + SyncService, + ], + }), safeProvider({ provide: LoginStrategyServiceAbstraction, useClass: LoginStrategyService, @@ -1665,6 +1688,7 @@ const safeProviders: SafeProvider[] = [ SsoLoginServiceAbstraction, SyncService, UserAsymmetricKeysRegenerationService, + EncryptedMigrator, LogService, ], }), @@ -1735,6 +1759,28 @@ const safeProviders: SafeProvider[] = [ InternalMasterPasswordServiceAbstraction, ], }), + safeProvider({ + provide: EncryptedMigrationsSchedulerService, + useClass: DefaultEncryptedMigrationsSchedulerService, + deps: [ + SyncService, + AccountService, + StateProvider, + EncryptedMigrator, + AuthServiceAbstraction, + LogService, + DialogService, + ToastService, + I18nServiceAbstraction, + Router, + ], + }), + safeProvider({ + provide: APP_INITIALIZER as SafeInjectionToken<() => Promise>, + useFactory: (encryptedMigrationsScheduler: EncryptedMigrationsSchedulerService) => () => {}, + deps: [EncryptedMigrationsSchedulerService], + multi: true, + }), safeProvider({ provide: LockService, useClass: DefaultLockService, diff --git a/libs/auth/src/angular/login-via-auth-request/login-via-auth-request.component.ts b/libs/auth/src/angular/login-via-auth-request/login-via-auth-request.component.ts index 2436593dfda..040d4d3c121 100644 --- a/libs/auth/src/angular/login-via-auth-request/login-via-auth-request.component.ts +++ b/libs/auth/src/angular/login-via-auth-request/login-via-auth-request.component.ts @@ -822,7 +822,7 @@ export class LoginViaAuthRequestComponent implements OnInit, OnDestroy { } private async handleSuccessfulLoginNavigation(userId: UserId) { - await this.loginSuccessHandlerService.run(userId); + await this.loginSuccessHandlerService.run(userId, null); await this.router.navigate(["vault"]); } } diff --git a/libs/auth/src/angular/login/login.component.ts b/libs/auth/src/angular/login/login.component.ts index 0b011b5641f..91ca2b614d1 100644 --- a/libs/auth/src/angular/login/login.component.ts +++ b/libs/auth/src/angular/login/login.component.ts @@ -382,7 +382,7 @@ export class LoginComponent implements OnInit, OnDestroy { } // User logged in successfully so execute side effects - await this.loginSuccessHandlerService.run(authResult.userId); + await this.loginSuccessHandlerService.run(authResult.userId, authResult.masterPassword); // Determine where to send the user next // The AuthGuard will handle routing to change-password based on state diff --git a/libs/auth/src/angular/new-device-verification/new-device-verification.component.ts b/libs/auth/src/angular/new-device-verification/new-device-verification.component.ts index c3d6ff5d1fe..726cfd7b3b5 100644 --- a/libs/auth/src/angular/new-device-verification/new-device-verification.component.ts +++ b/libs/auth/src/angular/new-device-verification/new-device-verification.component.ts @@ -152,9 +152,7 @@ export class NewDeviceVerificationComponent implements OnInit, OnDestroy { return; } - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.loginSuccessHandlerService.run(authResult.userId); + await this.loginSuccessHandlerService.run(authResult.userId, authResult.masterPassword); // TODO: PM-22663 use the new service to handle routing. const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); diff --git a/libs/auth/src/angular/registration/registration-finish/registration-finish.component.ts b/libs/auth/src/angular/registration/registration-finish/registration-finish.component.ts index 19e7c1feabd..99eaa2404d9 100644 --- a/libs/auth/src/angular/registration/registration-finish/registration-finish.component.ts +++ b/libs/auth/src/angular/registration/registration-finish/registration-finish.component.ts @@ -206,7 +206,10 @@ export class RegistrationFinishComponent implements OnInit, OnDestroy { return; } - await this.loginSuccessHandlerService.run(authenticationResult.userId); + await this.loginSuccessHandlerService.run( + authenticationResult.userId, + authenticationResult.masterPassword ?? null, + ); if (this.premiumInterest) { await this.premiumInterestStateService.setPremiumInterest( diff --git a/libs/auth/src/angular/sso/sso.component.ts b/libs/auth/src/angular/sso/sso.component.ts index bf618ba39f4..d0cc2bd83e5 100644 --- a/libs/auth/src/angular/sso/sso.component.ts +++ b/libs/auth/src/angular/sso/sso.component.ts @@ -437,7 +437,7 @@ export class SsoComponent implements OnInit { // Everything after the 2FA check is considered a successful login // Just have to figure out where to send the user - await this.loginSuccessHandlerService.run(authResult.userId); + await this.loginSuccessHandlerService.run(authResult.userId, null); // Save off the OrgSsoIdentifier for use in the TDE flows (or elsewhere) // - TDE login decryption options component diff --git a/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.ts b/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.ts index ca19d3652bb..4c143cc59f9 100644 --- a/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.ts +++ b/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.ts @@ -450,7 +450,7 @@ export class TwoFactorAuthComponent implements OnInit, OnDestroy { } // User is fully logged in so handle any post login logic before executing navigation - await this.loginSuccessHandlerService.run(authResult.userId); + await this.loginSuccessHandlerService.run(authResult.userId, authResult.masterPassword); // Save off the OrgSsoIdentifier for use in the TDE flows // - TDE login decryption options component diff --git a/libs/auth/src/common/abstractions/login-success-handler.service.ts b/libs/auth/src/common/abstractions/login-success-handler.service.ts index 8dee1dd32b9..e6d75d661f5 100644 --- a/libs/auth/src/common/abstractions/login-success-handler.service.ts +++ b/libs/auth/src/common/abstractions/login-success-handler.service.ts @@ -5,6 +5,7 @@ export abstract class LoginSuccessHandlerService { * Runs any service calls required after a successful login. * Service calls that should be included in this method are only those required to be awaited after successful login. * @param userId The user id. + * @param masterPassword The master password, if available. Null when logging in with SSO or other non-master-password methods. */ - abstract run(userId: UserId): Promise; + abstract run(userId: UserId, masterPassword: string | null): Promise; } diff --git a/libs/auth/src/common/login-strategies/login.strategy.spec.ts b/libs/auth/src/common/login-strategies/login.strategy.spec.ts index 38d62cfdd83..ceb36a44633 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.spec.ts @@ -308,6 +308,7 @@ describe("LoginStrategy", () => { const result = await passwordLoginStrategy.logIn(credentials); const expected = new AuthResult(); + expected.masterPassword = "password"; expected.userId = userId; expected.resetMasterPassword = true; expected.twoFactorProviders = null; @@ -323,6 +324,7 @@ describe("LoginStrategy", () => { const result = await passwordLoginStrategy.logIn(credentials); const expected = new AuthResult(); + expected.masterPassword = "password"; expected.userId = userId; expected.resetMasterPassword = false; expected.twoFactorProviders = null; diff --git a/libs/auth/src/common/login-strategies/login.strategy.ts b/libs/auth/src/common/login-strategies/login.strategy.ts index b8e4ee9e822..2e3c41da900 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.ts @@ -108,6 +108,8 @@ export abstract class LoginStrategy { data.tokenRequest.setTwoFactor(twoFactor); this.cache.next(data); const [authResult] = await this.startLogIn(); + // There is an import cycle between PasswordLoginStrategyData and LoginStrategy, which means this cast is necessary, which is solved by extracting the data classes. + authResult.masterPassword = (this.cache.value as any)["masterPassword"] ?? null; return authResult; } @@ -264,6 +266,9 @@ export abstract class LoginStrategy { await this.processForceSetPasswordReason(response.forcePasswordReset, userId); this.messagingService.send("loggedIn"); + // There is an import cycle between PasswordLoginStrategyData and LoginStrategy, which means this cast is necessary, which is solved by extracting the data classes. + // TODO: https://bitwarden.atlassian.net/browse/PM-27573 + result.masterPassword = (this.cache.value as any)["masterPassword"] ?? null; return result; } diff --git a/libs/auth/src/common/login-strategies/password-login.strategy.ts b/libs/auth/src/common/login-strategies/password-login.strategy.ts index 829351cc88f..ad49567b2ff 100644 --- a/libs/auth/src/common/login-strategies/password-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/password-login.strategy.ts @@ -33,6 +33,8 @@ export class PasswordLoginStrategyData implements LoginStrategyData { localMasterKeyHash: string; /** The user's master key */ masterKey: MasterKey; + /** The user's master password */ + masterPassword: string; /** * Tracks if the user needs to update their password due to * a password that does not meet an organization's master password policy. @@ -83,6 +85,7 @@ export class PasswordLoginStrategy extends LoginStrategy { masterPassword, email, ); + data.masterPassword = masterPassword; data.userEnteredEmail = email; // Hash the password early (before authentication) so we don't persist it in memory in plaintext @@ -251,6 +254,7 @@ export class PasswordLoginStrategy extends LoginStrategy { this.cache.next(data); const [authResult] = await this.startLogIn(); + authResult.masterPassword = this.cache.value["masterPassword"] ?? null; return authResult; } diff --git a/libs/auth/src/common/services/login-success-handler/default-login-success-handler.service.spec.ts b/libs/auth/src/common/services/login-success-handler/default-login-success-handler.service.spec.ts index 6fb355a8a1b..caa5d8b3290 100644 --- a/libs/auth/src/common/services/login-success-handler/default-login-success-handler.service.spec.ts +++ b/libs/auth/src/common/services/login-success-handler/default-login-success-handler.service.spec.ts @@ -1,6 +1,7 @@ import { MockProxy, mock } from "jest-mock-extended"; import { SsoLoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/sso-login.service.abstraction"; +import { EncryptedMigrator } from "@bitwarden/common/key-management/encrypted-migrator/encrypted-migrator.abstraction"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { SyncService } from "@bitwarden/common/platform/sync"; import { UserId } from "@bitwarden/common/types/guid"; @@ -19,6 +20,7 @@ describe("DefaultLoginSuccessHandlerService", () => { let ssoLoginService: MockProxy; let syncService: MockProxy; let userAsymmetricKeysRegenerationService: MockProxy; + let encryptedMigrator: MockProxy; let logService: MockProxy; const userId = "USER_ID" as UserId; @@ -30,6 +32,7 @@ describe("DefaultLoginSuccessHandlerService", () => { ssoLoginService = mock(); syncService = mock(); userAsymmetricKeysRegenerationService = mock(); + encryptedMigrator = mock(); logService = mock(); service = new DefaultLoginSuccessHandlerService( @@ -38,6 +41,7 @@ describe("DefaultLoginSuccessHandlerService", () => { ssoLoginService, syncService, userAsymmetricKeysRegenerationService, + encryptedMigrator, logService, ); @@ -50,7 +54,7 @@ describe("DefaultLoginSuccessHandlerService", () => { describe("run", () => { it("should call required services on successful login", async () => { - await service.run(userId); + await service.run(userId, null); expect(syncService.fullSync).toHaveBeenCalledWith(true, { skipTokenRefresh: true }); expect(userAsymmetricKeysRegenerationService.regenerateIfNeeded).toHaveBeenCalledWith(userId); @@ -58,7 +62,7 @@ describe("DefaultLoginSuccessHandlerService", () => { }); it("should get SSO email", async () => { - await service.run(userId); + await service.run(userId, null); expect(ssoLoginService.getSsoEmail).toHaveBeenCalled(); }); @@ -68,8 +72,8 @@ describe("DefaultLoginSuccessHandlerService", () => { ssoLoginService.getSsoEmail.mockResolvedValue(null); }); - it("should log error and return early", async () => { - await service.run(userId); + it("should not check SSO requirements", async () => { + await service.run(userId, null); expect(logService.debug).toHaveBeenCalledWith("SSO login email not found."); expect(ssoLoginService.updateSsoRequiredCache).not.toHaveBeenCalled(); @@ -82,7 +86,7 @@ describe("DefaultLoginSuccessHandlerService", () => { }); it("should call updateSsoRequiredCache() and clearSsoEmail()", async () => { - await service.run(userId); + await service.run(userId, null); expect(ssoLoginService.updateSsoRequiredCache).toHaveBeenCalledWith(testEmail, userId); expect(ssoLoginService.clearSsoEmail).toHaveBeenCalled(); diff --git a/libs/auth/src/common/services/login-success-handler/default-login-success-handler.service.ts b/libs/auth/src/common/services/login-success-handler/default-login-success-handler.service.ts index 2b9672f1c0b..9d4311868d7 100644 --- a/libs/auth/src/common/services/login-success-handler/default-login-success-handler.service.ts +++ b/libs/auth/src/common/services/login-success-handler/default-login-success-handler.service.ts @@ -1,4 +1,5 @@ import { SsoLoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/sso-login.service.abstraction"; +import { EncryptedMigrator } from "@bitwarden/common/key-management/encrypted-migrator/encrypted-migrator.abstraction"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { SyncService } from "@bitwarden/common/platform/sync"; import { UserId } from "@bitwarden/common/types/guid"; @@ -15,12 +16,19 @@ export class DefaultLoginSuccessHandlerService implements LoginSuccessHandlerSer private ssoLoginService: SsoLoginServiceAbstraction, private syncService: SyncService, private userAsymmetricKeysRegenerationService: UserAsymmetricKeysRegenerationService, + private encryptedMigrator: EncryptedMigrator, private logService: LogService, ) {} - async run(userId: UserId): Promise { + + async run(userId: UserId, masterPassword: string | null): Promise { await this.syncService.fullSync(true, { skipTokenRefresh: true }); await this.userAsymmetricKeysRegenerationService.regenerateIfNeeded(userId); await this.loginEmailService.clearLoginEmail(); + try { + await this.encryptedMigrator.runMigrations(userId, masterPassword); + } catch { + // Don't block login success on migration failure + } const ssoLoginEmail = await this.ssoLoginService.getSsoEmail(); diff --git a/libs/common/src/auth/models/domain/auth-result.ts b/libs/common/src/auth/models/domain/auth-result.ts index a61a35eeb1d..ae3e9bdeda6 100644 --- a/libs/common/src/auth/models/domain/auth-result.ts +++ b/libs/common/src/auth/models/domain/auth-result.ts @@ -18,6 +18,8 @@ export class AuthResult { email: string; requiresEncryptionKeyMigration: boolean; requiresDeviceVerification: boolean; + // The master-password used in the authentication process + masterPassword: string | null; get requiresTwoFactor() { return this.twoFactorProviders != null; diff --git a/libs/common/src/key-management/encrypted-migrator/default-encrypted-migrator.spec.ts b/libs/common/src/key-management/encrypted-migrator/default-encrypted-migrator.spec.ts new file mode 100644 index 00000000000..5a681ec2913 --- /dev/null +++ b/libs/common/src/key-management/encrypted-migrator/default-encrypted-migrator.spec.ts @@ -0,0 +1,194 @@ +import { mock } from "jest-mock-extended"; + +// eslint-disable-next-line no-restricted-imports +import { KdfConfigService } from "@bitwarden/key-management"; +import { LogService } from "@bitwarden/logging"; + +import { ConfigService } from "../../platform/abstractions/config/config.service"; +import { SyncService } from "../../platform/sync"; +import { UserId } from "../../types/guid"; +import { ChangeKdfService } from "../kdf/change-kdf.service.abstraction"; +import { MasterPasswordServiceAbstraction } from "../master-password/abstractions/master-password.service.abstraction"; + +import { DefaultEncryptedMigrator } from "./default-encrypted-migrator"; +import { EncryptedMigration } from "./migrations/encrypted-migration"; +import { MinimumKdfMigration } from "./migrations/minimum-kdf-migration"; + +jest.mock("./migrations/minimum-kdf-migration"); + +describe("EncryptedMigrator", () => { + const mockKdfConfigService = mock(); + const mockChangeKdfService = mock(); + const mockLogService = mock(); + const configService = mock(); + const masterPasswordService = mock(); + const syncService = mock(); + + let sut: DefaultEncryptedMigrator; + const mockMigration = mock(); + + const mockUserId = "00000000-0000-0000-0000-000000000000" as UserId; + const mockMasterPassword = "masterPassword123"; + + beforeEach(() => { + jest.clearAllMocks(); + + // Mock the MinimumKdfMigration constructor to return our mock + (MinimumKdfMigration as jest.MockedClass).mockImplementation( + () => mockMigration, + ); + + sut = new DefaultEncryptedMigrator( + mockKdfConfigService, + mockChangeKdfService, + mockLogService, + configService, + masterPasswordService, + syncService, + ); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + describe("runMigrations", () => { + it("should throw error when userId is null", async () => { + await expect(sut.runMigrations(null as any, null)).rejects.toThrow("userId"); + }); + + it("should throw error when userId is undefined", async () => { + await expect(sut.runMigrations(undefined as any, null)).rejects.toThrow("userId"); + }); + + it("should not run migration when needsMigration returns 'noMigrationNeeded'", async () => { + mockMigration.needsMigration.mockResolvedValue("noMigrationNeeded"); + + await sut.runMigrations(mockUserId, null); + + expect(mockMigration.needsMigration).toHaveBeenCalledWith(mockUserId); + expect(mockMigration.runMigrations).not.toHaveBeenCalled(); + }); + + it("should run migration when needsMigration returns 'needsMigration'", async () => { + mockMigration.needsMigration.mockResolvedValue("needsMigration"); + + await sut.runMigrations(mockUserId, mockMasterPassword); + + expect(mockMigration.needsMigration).toHaveBeenCalledWith(mockUserId); + expect(mockMigration.runMigrations).toHaveBeenCalledWith(mockUserId, mockMasterPassword); + }); + + it("should run migration when needsMigration returns 'needsMigrationWithMasterPassword'", async () => { + mockMigration.needsMigration.mockResolvedValue("needsMigrationWithMasterPassword"); + + await sut.runMigrations(mockUserId, mockMasterPassword); + + expect(mockMigration.needsMigration).toHaveBeenCalledWith(mockUserId); + expect(mockMigration.runMigrations).toHaveBeenCalledWith(mockUserId, mockMasterPassword); + }); + + it("should throw error when migration needs master password but null is provided", async () => { + mockMigration.needsMigration.mockResolvedValue("needsMigrationWithMasterPassword"); + + await sut.runMigrations(mockUserId, null); + expect(mockMigration.needsMigration).toHaveBeenCalledWith(mockUserId); + expect(mockMigration.runMigrations).not.toHaveBeenCalled(); + }); + + it("should run multiple migrations", async () => { + const mockSecondMigration = mock(); + mockSecondMigration.needsMigration.mockResolvedValue("needsMigration"); + + (sut as any).migrations.push({ + name: "Test Second Migration", + migration: mockSecondMigration, + }); + + mockMigration.needsMigration.mockResolvedValue("needsMigration"); + + await sut.runMigrations(mockUserId, mockMasterPassword); + + expect(mockMigration.needsMigration).toHaveBeenCalledWith(mockUserId); + expect(mockSecondMigration.needsMigration).toHaveBeenCalledWith(mockUserId); + expect(mockMigration.runMigrations).toHaveBeenCalledWith(mockUserId, mockMasterPassword); + expect(mockSecondMigration.runMigrations).toHaveBeenCalledWith( + mockUserId, + mockMasterPassword, + ); + }); + }); + + describe("needsMigrations", () => { + it("should return 'noMigrationNeeded' when no migrations are needed", async () => { + mockMigration.needsMigration.mockResolvedValue("noMigrationNeeded"); + + const result = await sut.needsMigrations(mockUserId); + + expect(result).toBe("noMigrationNeeded"); + expect(mockMigration.needsMigration).toHaveBeenCalledWith(mockUserId); + }); + + it("should return 'needsMigration' when at least one migration needs to run", async () => { + mockMigration.needsMigration.mockResolvedValue("needsMigration"); + + const result = await sut.needsMigrations(mockUserId); + + expect(result).toBe("needsMigration"); + expect(mockMigration.needsMigration).toHaveBeenCalledWith(mockUserId); + }); + + it("should return 'needsMigrationWithMasterPassword' when at least one migration needs master password", async () => { + mockMigration.needsMigration.mockResolvedValue("needsMigrationWithMasterPassword"); + + const result = await sut.needsMigrations(mockUserId); + + expect(result).toBe("needsMigrationWithMasterPassword"); + expect(mockMigration.needsMigration).toHaveBeenCalledWith(mockUserId); + }); + + it("should prioritize 'needsMigrationWithMasterPassword' over 'needsMigration'", async () => { + const mockSecondMigration = mock(); + mockSecondMigration.needsMigration.mockResolvedValue("needsMigration"); + + (sut as any).migrations.push({ + name: "Test Second Migration", + migration: mockSecondMigration, + }); + + mockMigration.needsMigration.mockResolvedValue("needsMigrationWithMasterPassword"); + + const result = await sut.needsMigrations(mockUserId); + + expect(result).toBe("needsMigrationWithMasterPassword"); + expect(mockMigration.needsMigration).toHaveBeenCalledWith(mockUserId); + expect(mockSecondMigration.needsMigration).toHaveBeenCalledWith(mockUserId); + }); + + it("should return 'needsMigration' when some migrations need running but none need master password", async () => { + const mockSecondMigration = mock(); + mockSecondMigration.needsMigration.mockResolvedValue("noMigrationNeeded"); + + (sut as any).migrations.push({ + name: "Test Second Migration", + migration: mockSecondMigration, + }); + + mockMigration.needsMigration.mockResolvedValue("needsMigration"); + + const result = await sut.needsMigrations(mockUserId); + + expect(result).toBe("needsMigration"); + expect(mockMigration.needsMigration).toHaveBeenCalledWith(mockUserId); + expect(mockSecondMigration.needsMigration).toHaveBeenCalledWith(mockUserId); + }); + + it("should throw error when userId is null", async () => { + await expect(sut.needsMigrations(null as any)).rejects.toThrow("userId"); + }); + + it("should throw error when userId is undefined", async () => { + await expect(sut.needsMigrations(undefined as any)).rejects.toThrow("userId"); + }); + }); +}); diff --git a/libs/common/src/key-management/encrypted-migrator/default-encrypted-migrator.ts b/libs/common/src/key-management/encrypted-migrator/default-encrypted-migrator.ts new file mode 100644 index 00000000000..bc91e24070a --- /dev/null +++ b/libs/common/src/key-management/encrypted-migrator/default-encrypted-migrator.ts @@ -0,0 +1,113 @@ +// eslint-disable-next-line no-restricted-imports +import { KdfConfigService } from "@bitwarden/key-management"; +import { LogService } from "@bitwarden/logging"; + +import { assertNonNullish } from "../../auth/utils"; +import { ConfigService } from "../../platform/abstractions/config/config.service"; +import { SyncService } from "../../platform/sync"; +import { UserId } from "../../types/guid"; +import { ChangeKdfService } from "../kdf/change-kdf.service.abstraction"; +import { MasterPasswordServiceAbstraction } from "../master-password/abstractions/master-password.service.abstraction"; + +import { EncryptedMigrator } from "./encrypted-migrator.abstraction"; +import { EncryptedMigration, MigrationRequirement } from "./migrations/encrypted-migration"; +import { MinimumKdfMigration } from "./migrations/minimum-kdf-migration"; + +export class DefaultEncryptedMigrator implements EncryptedMigrator { + private migrations: { name: string; migration: EncryptedMigration }[] = []; + private isRunningMigration = false; + + constructor( + readonly kdfConfigService: KdfConfigService, + readonly changeKdfService: ChangeKdfService, + private readonly logService: LogService, + readonly configService: ConfigService, + readonly masterPasswordService: MasterPasswordServiceAbstraction, + readonly syncService: SyncService, + ) { + // Register migrations here + this.migrations.push({ + name: "Minimum PBKDF2 Iteration Count Migration", + migration: new MinimumKdfMigration( + kdfConfigService, + changeKdfService, + logService, + configService, + masterPasswordService, + ), + }); + } + + async runMigrations(userId: UserId, masterPassword: string | null): Promise { + assertNonNullish(userId, "userId"); + + // Ensure that the requirements for running all migrations are met + const needsMigration = await this.needsMigrations(userId); + if (needsMigration === "noMigrationNeeded") { + return; + } else if (needsMigration === "needsMigrationWithMasterPassword" && masterPassword == null) { + // If a migration needs a password, but none is provided, the migrations are skipped. If a manual caller + // during a login / unlock flow calls without a master password in a login / unlock strategy that has no + // password, such as biometric unlock, the migrations are skipped. + // + // The fallback to this, the encrypted migrations scheduler, will first check if a migration needs a password + // and then prompt the user. If the user enters their password, runMigrations is called again with the password. + return; + } + + try { + // No concurrent migrations allowed, so acquire a service-wide lock + if (this.isRunningMigration) { + return; + } + this.isRunningMigration = true; + + // Run all migrations sequentially in the order they were registered + this.logService.mark("[Encrypted Migrator] Start"); + this.logService.info(`[Encrypted Migrator] Starting migrations for user: ${userId}`); + let ranMigration = false; + for (const { name, migration } of this.migrations) { + if ((await migration.needsMigration(userId)) !== "noMigrationNeeded") { + this.logService.info(`[Encrypted Migrator] Running migration: ${name}`); + const start = performance.now(); + await migration.runMigrations(userId, masterPassword); + this.logService.measure(start, "[Encrypted Migrator]", name, "ExecutionTime"); + ranMigration = true; + } + } + this.logService.mark("[Encrypted Migrator] Finish"); + this.logService.info(`[Encrypted Migrator] Completed migrations for user: ${userId}`); + if (ranMigration) { + await this.syncService.fullSync(true); + } + } catch (error) { + this.logService.error( + `[Encrypted Migrator] Error running migrations for user: ${userId}`, + error, + ); + throw error; // Re-throw the error to be handled by the caller + } finally { + this.isRunningMigration = false; + } + } + + async needsMigrations(userId: UserId): Promise { + assertNonNullish(userId, "userId"); + + const migrationRequirements = await Promise.all( + this.migrations.map(async ({ migration }) => migration.needsMigration(userId)), + ); + + if (migrationRequirements.includes("needsMigrationWithMasterPassword")) { + return "needsMigrationWithMasterPassword"; + } else if (migrationRequirements.includes("needsMigration")) { + return "needsMigration"; + } else { + return "noMigrationNeeded"; + } + } + + isRunningMigrations(): boolean { + return this.isRunningMigration; + } +} diff --git a/libs/common/src/key-management/encrypted-migrator/encrypted-migrator.abstraction.ts b/libs/common/src/key-management/encrypted-migrator/encrypted-migrator.abstraction.ts new file mode 100644 index 00000000000..7e408374f7e --- /dev/null +++ b/libs/common/src/key-management/encrypted-migrator/encrypted-migrator.abstraction.ts @@ -0,0 +1,32 @@ +import { UserId } from "../../types/guid"; + +import { MigrationRequirement } from "./migrations/encrypted-migration"; + +export abstract class EncryptedMigrator { + /** + * Runs migrations on a decrypted user, with the cryptographic state initialized. + * This only runs the migrations that are needed for the user. + * This needs to be run after the decrypted user key has been set to state. + * + * If the master password is required but not provided, the migrations will not run, and the function will return early. + * If migrations are already running, the migrations will not run again, and the function will return early. + * + * @param userId The ID of the user to run migrations for. + * @param masterPassword The user's current master password. + * @throws If the user does not exist + * @throws If the user is locked or logged out + * @throws If a migration fails + */ + abstract runMigrations(userId: UserId, masterPassword: string | null): Promise; + /** + * Checks if the user needs to run any migrations. + * This is used to determine if the user should be prompted to run migrations. + * @param userId The ID of the user to check migrations for. + */ + abstract needsMigrations(userId: UserId): Promise; + + /** + * Indicates whether migrations are currently running. + */ + abstract isRunningMigrations(): boolean; +} diff --git a/libs/common/src/key-management/encrypted-migrator/migrations/encrypted-migration.ts b/libs/common/src/key-management/encrypted-migrator/migrations/encrypted-migration.ts new file mode 100644 index 00000000000..e6b1dcc45f6 --- /dev/null +++ b/libs/common/src/key-management/encrypted-migrator/migrations/encrypted-migration.ts @@ -0,0 +1,36 @@ +import { UserId } from "../../../types/guid"; + +/** + * @internal + * IMPORTANT: Please read this when implementing new migrations. + * + * An encrypted migration defines an online migration that mutates the persistent state of the user on the server, or locally. + * It should only be run once per user (or for local migrations, once per device). Migrations get scheduled automatically, + * during actions such as login and unlock, or during sync. + * + * Migrations can require the master-password, which is provided by the user if required. + * Migrations are run as soon as possible non-lazily, and MAY block unlock / login, if they have to run. + * + * Most importantly, implementing a migration should be done such that concurrent migrations may fail, but must never + * leave the user in a broken state. Locally, these are scheduled with an application-global lock. However, no such guarantees + * are made for the server, and other devices may run the migration concurrently. + * + * When adding a migration, it *MUST* be feature-flagged for the initial roll-out. + */ +export interface EncryptedMigration { + /** + * Runs the migration. + * @throws If the migration fails, such as when no network is available. + * @throws If the requirements for migration are not met (e.g. the user is locked) + */ + runMigrations(userId: UserId, masterPassword: string | null): Promise; + /** + * Returns whether the migration needs to be run for the user, and if it does, whether the master password is required. + */ + needsMigration(userId: UserId): Promise; +} + +export type MigrationRequirement = + | "needsMigration" + | "needsMigrationWithMasterPassword" + | "noMigrationNeeded"; diff --git a/libs/common/src/key-management/encrypted-migrator/migrations/minimum-kdf-migration.spec.ts b/libs/common/src/key-management/encrypted-migrator/migrations/minimum-kdf-migration.spec.ts new file mode 100644 index 00000000000..cf2bd307b6c --- /dev/null +++ b/libs/common/src/key-management/encrypted-migrator/migrations/minimum-kdf-migration.spec.ts @@ -0,0 +1,184 @@ +import { mock } from "jest-mock-extended"; + +// eslint-disable-next-line no-restricted-imports +import { + Argon2KdfConfig, + KdfConfigService, + KdfType, + PBKDF2KdfConfig, +} from "@bitwarden/key-management"; +import { LogService } from "@bitwarden/logging"; + +import { FeatureFlag } from "../../../enums/feature-flag.enum"; +import { ConfigService } from "../../../platform/abstractions/config/config.service"; +import { UserId } from "../../../types/guid"; +import { ChangeKdfService } from "../../kdf/change-kdf.service.abstraction"; +import { MasterPasswordServiceAbstraction } from "../../master-password/abstractions/master-password.service.abstraction"; + +import { MinimumKdfMigration } from "./minimum-kdf-migration"; + +describe("MinimumKdfMigration", () => { + const mockKdfConfigService = mock(); + const mockChangeKdfService = mock(); + const mockLogService = mock(); + const mockConfigService = mock(); + const mockMasterPasswordService = mock(); + + let sut: MinimumKdfMigration; + + const mockUserId = "00000000-0000-0000-0000-000000000000" as UserId; + const mockMasterPassword = "masterPassword"; + + beforeEach(() => { + jest.clearAllMocks(); + + sut = new MinimumKdfMigration( + mockKdfConfigService, + mockChangeKdfService, + mockLogService, + mockConfigService, + mockMasterPasswordService, + ); + }); + + describe("needsMigration", () => { + it("should return 'noMigrationNeeded' when user does not have a master password`", async () => { + mockMasterPasswordService.userHasMasterPassword.mockResolvedValue(false); + const result = await sut.needsMigration(mockUserId); + expect(result).toBe("noMigrationNeeded"); + }); + + it("should return 'noMigrationNeeded' when user uses argon2id`", async () => { + mockMasterPasswordService.userHasMasterPassword.mockResolvedValue(true); + mockKdfConfigService.getKdfConfig.mockResolvedValue(new Argon2KdfConfig(3, 64, 4)); + const result = await sut.needsMigration(mockUserId); + expect(result).toBe("noMigrationNeeded"); + }); + + it("should return 'noMigrationNeeded' when PBKDF2 iterations are already above minimum", async () => { + const mockKdfConfig = { + kdfType: KdfType.PBKDF2_SHA256, + iterations: PBKDF2KdfConfig.ITERATIONS.min + 1000, + }; + mockKdfConfigService.getKdfConfig.mockResolvedValue(mockKdfConfig as any); + + const result = await sut.needsMigration(mockUserId); + + expect(result).toBe("noMigrationNeeded"); + expect(mockKdfConfigService.getKdfConfig).toHaveBeenCalledWith(mockUserId); + }); + + it("should return 'noMigrationNeeded' when PBKDF2 iterations equal minimum", async () => { + const mockKdfConfig = { + kdfType: KdfType.PBKDF2_SHA256, + iterations: PBKDF2KdfConfig.ITERATIONS.min, + }; + mockKdfConfigService.getKdfConfig.mockResolvedValue(mockKdfConfig as any); + mockConfigService.getFeatureFlag.mockResolvedValue(true); + + const result = await sut.needsMigration(mockUserId); + + expect(result).toBe("noMigrationNeeded"); + expect(mockKdfConfigService.getKdfConfig).toHaveBeenCalledWith(mockUserId); + }); + + it("should return 'noMigrationNeeded' when feature flag is disabled", async () => { + const mockKdfConfig = { + kdfType: KdfType.PBKDF2_SHA256, + iterations: PBKDF2KdfConfig.ITERATIONS.min - 1000, + }; + mockKdfConfigService.getKdfConfig.mockResolvedValue(mockKdfConfig as any); + mockConfigService.getFeatureFlag.mockResolvedValue(false); + + const result = await sut.needsMigration(mockUserId); + + expect(result).toBe("noMigrationNeeded"); + expect(mockKdfConfigService.getKdfConfig).toHaveBeenCalledWith(mockUserId); + expect(mockConfigService.getFeatureFlag).toHaveBeenCalledWith( + FeatureFlag.ForceUpdateKDFSettings, + ); + }); + + it("should return 'needsMigrationWithMasterPassword' when PBKDF2 iterations are below minimum and feature flag is enabled", async () => { + const mockKdfConfig = { + kdfType: KdfType.PBKDF2_SHA256, + iterations: PBKDF2KdfConfig.ITERATIONS.min - 1000, + }; + mockKdfConfigService.getKdfConfig.mockResolvedValue(mockKdfConfig as any); + mockConfigService.getFeatureFlag.mockResolvedValue(true); + + const result = await sut.needsMigration(mockUserId); + + expect(result).toBe("needsMigrationWithMasterPassword"); + expect(mockKdfConfigService.getKdfConfig).toHaveBeenCalledWith(mockUserId); + expect(mockConfigService.getFeatureFlag).toHaveBeenCalledWith( + FeatureFlag.ForceUpdateKDFSettings, + ); + }); + + it("should throw error when userId is null", async () => { + await expect(sut.needsMigration(null as any)).rejects.toThrow("userId"); + }); + + it("should throw error when userId is undefined", async () => { + await expect(sut.needsMigration(undefined as any)).rejects.toThrow("userId"); + }); + }); + + describe("runMigrations", () => { + it("should update KDF parameters with minimum PBKDF2 iterations", async () => { + await sut.runMigrations(mockUserId, mockMasterPassword); + + expect(mockLogService.info).toHaveBeenCalledWith( + `[MinimumKdfMigration] Updating user ${mockUserId} to minimum PBKDF2 iteration count ${PBKDF2KdfConfig.ITERATIONS.min}`, + ); + expect(mockChangeKdfService.updateUserKdfParams).toHaveBeenCalledWith( + mockMasterPassword, + expect.any(PBKDF2KdfConfig), + mockUserId, + ); + + // Verify the PBKDF2KdfConfig has the correct iteration count + const kdfConfigArg = (mockChangeKdfService.updateUserKdfParams as jest.Mock).mock.calls[0][1]; + expect(kdfConfigArg.iterations).toBe(PBKDF2KdfConfig.ITERATIONS.defaultValue); + }); + + it("should throw error when userId is null", async () => { + await expect(sut.runMigrations(null as any, mockMasterPassword)).rejects.toThrow("userId"); + }); + + it("should throw error when userId is undefined", async () => { + await expect(sut.runMigrations(undefined as any, mockMasterPassword)).rejects.toThrow( + "userId", + ); + }); + + it("should throw error when masterPassword is null", async () => { + await expect(sut.runMigrations(mockUserId, null as any)).rejects.toThrow("masterPassword"); + }); + + it("should throw error when masterPassword is undefined", async () => { + await expect(sut.runMigrations(mockUserId, undefined as any)).rejects.toThrow( + "masterPassword", + ); + }); + + it("should handle errors from changeKdfService", async () => { + const mockError = new Error("KDF update failed"); + mockChangeKdfService.updateUserKdfParams.mockRejectedValue(mockError); + + await expect(sut.runMigrations(mockUserId, mockMasterPassword)).rejects.toThrow( + "KDF update failed", + ); + + expect(mockLogService.info).toHaveBeenCalledWith( + `[MinimumKdfMigration] Updating user ${mockUserId} to minimum PBKDF2 iteration count ${PBKDF2KdfConfig.ITERATIONS.min}`, + ); + expect(mockChangeKdfService.updateUserKdfParams).toHaveBeenCalledWith( + mockMasterPassword, + expect.any(PBKDF2KdfConfig), + mockUserId, + ); + }); + }); +}); diff --git a/libs/common/src/key-management/encrypted-migrator/migrations/minimum-kdf-migration.ts b/libs/common/src/key-management/encrypted-migrator/migrations/minimum-kdf-migration.ts new file mode 100644 index 00000000000..0666064b36e --- /dev/null +++ b/libs/common/src/key-management/encrypted-migrator/migrations/minimum-kdf-migration.ts @@ -0,0 +1,68 @@ +import { UserId } from "@bitwarden/common/types/guid"; +// eslint-disable-next-line no-restricted-imports +import { KdfConfigService, KdfType, PBKDF2KdfConfig } from "@bitwarden/key-management"; +import { LogService } from "@bitwarden/logging"; + +import { assertNonNullish } from "../../../auth/utils"; +import { FeatureFlag } from "../../../enums/feature-flag.enum"; +import { ConfigService } from "../../../platform/abstractions/config/config.service"; +import { ChangeKdfService } from "../../kdf/change-kdf.service.abstraction"; +import { MasterPasswordServiceAbstraction } from "../../master-password/abstractions/master-password.service.abstraction"; + +import { EncryptedMigration, MigrationRequirement } from "./encrypted-migration"; + +/** + * @internal + * This migrator ensures the user's account has a minimum PBKDF2 iteration count. + * It will update the entire account, logging out old clients if necessary. + */ +export class MinimumKdfMigration implements EncryptedMigration { + constructor( + private readonly kdfConfigService: KdfConfigService, + private readonly changeKdfService: ChangeKdfService, + private readonly logService: LogService, + private readonly configService: ConfigService, + private readonly masterPasswordService: MasterPasswordServiceAbstraction, + ) {} + + async runMigrations(userId: UserId, masterPassword: string | null): Promise { + assertNonNullish(userId, "userId"); + assertNonNullish(masterPassword, "masterPassword"); + + this.logService.info( + `[MinimumKdfMigration] Updating user ${userId} to minimum PBKDF2 iteration count ${PBKDF2KdfConfig.ITERATIONS.defaultValue}`, + ); + await this.changeKdfService.updateUserKdfParams( + masterPassword!, + new PBKDF2KdfConfig(PBKDF2KdfConfig.ITERATIONS.defaultValue), + userId, + ); + await this.kdfConfigService.setKdfConfig( + userId, + new PBKDF2KdfConfig(PBKDF2KdfConfig.ITERATIONS.defaultValue), + ); + } + + async needsMigration(userId: UserId): Promise { + assertNonNullish(userId, "userId"); + + if (!(await this.masterPasswordService.userHasMasterPassword(userId))) { + return "noMigrationNeeded"; + } + + // Only PBKDF2 users below the minimum iteration count need migration + const kdfConfig = await this.kdfConfigService.getKdfConfig(userId); + if ( + kdfConfig.kdfType !== KdfType.PBKDF2_SHA256 || + kdfConfig.iterations >= PBKDF2KdfConfig.ITERATIONS.min + ) { + return "noMigrationNeeded"; + } + + if (!(await this.configService.getFeatureFlag(FeatureFlag.ForceUpdateKDFSettings))) { + return "noMigrationNeeded"; + } + + return "needsMigrationWithMasterPassword"; + } +} diff --git a/libs/common/src/key-management/kdf/change-kdf-service.abstraction.ts b/libs/common/src/key-management/kdf/change-kdf.service.abstraction.ts similarity index 100% rename from libs/common/src/key-management/kdf/change-kdf-service.abstraction.ts rename to libs/common/src/key-management/kdf/change-kdf.service.abstraction.ts diff --git a/libs/common/src/key-management/kdf/change-kdf-service.spec.ts b/libs/common/src/key-management/kdf/change-kdf.service.spec.ts similarity index 99% rename from libs/common/src/key-management/kdf/change-kdf-service.spec.ts rename to libs/common/src/key-management/kdf/change-kdf.service.spec.ts index c7df90f4790..12096155641 100644 --- a/libs/common/src/key-management/kdf/change-kdf-service.spec.ts +++ b/libs/common/src/key-management/kdf/change-kdf.service.spec.ts @@ -17,7 +17,7 @@ import { } from "../master-password/types/master-password.types"; import { ChangeKdfApiService } from "./change-kdf-api.service.abstraction"; -import { DefaultChangeKdfService } from "./change-kdf-service"; +import { DefaultChangeKdfService } from "./change-kdf.service"; describe("ChangeKdfService", () => { const changeKdfApiService = mock(); diff --git a/libs/common/src/key-management/kdf/change-kdf-service.ts b/libs/common/src/key-management/kdf/change-kdf.service.ts similarity index 97% rename from libs/common/src/key-management/kdf/change-kdf-service.ts rename to libs/common/src/key-management/kdf/change-kdf.service.ts index 64fbd1fce05..89d97e6704f 100644 --- a/libs/common/src/key-management/kdf/change-kdf-service.ts +++ b/libs/common/src/key-management/kdf/change-kdf.service.ts @@ -14,7 +14,7 @@ import { } from "../master-password/types/master-password.types"; import { ChangeKdfApiService } from "./change-kdf-api.service.abstraction"; -import { ChangeKdfService } from "./change-kdf-service.abstraction"; +import { ChangeKdfService } from "./change-kdf.service.abstraction"; export class DefaultChangeKdfService implements ChangeKdfService { constructor( diff --git a/libs/common/src/key-management/master-password/abstractions/master-password.service.abstraction.ts b/libs/common/src/key-management/master-password/abstractions/master-password.service.abstraction.ts index f982c2c5ce8..0e86761685f 100644 --- a/libs/common/src/key-management/master-password/abstractions/master-password.service.abstraction.ts +++ b/libs/common/src/key-management/master-password/abstractions/master-password.service.abstraction.ts @@ -106,6 +106,13 @@ export abstract class MasterPasswordServiceAbstraction { password: string, masterPasswordUnlockData: MasterPasswordUnlockData, ) => Promise; + + /** + * Returns whether the user has a master password set. + * @param userId The user ID. + * @throws If the user ID is missing. + */ + abstract userHasMasterPassword(userId: UserId): Promise; } export abstract class InternalMasterPasswordServiceAbstraction extends MasterPasswordServiceAbstraction { diff --git a/libs/common/src/key-management/master-password/services/fake-master-password.service.ts b/libs/common/src/key-management/master-password/services/fake-master-password.service.ts index 5db7f178b18..90fcaddb1a5 100644 --- a/libs/common/src/key-management/master-password/services/fake-master-password.service.ts +++ b/libs/common/src/key-management/master-password/services/fake-master-password.service.ts @@ -33,6 +33,10 @@ export class FakeMasterPasswordService implements InternalMasterPasswordServiceA this.masterKeyHashSubject.next(initialMasterKeyHash); } + userHasMasterPassword(userId: UserId): Promise { + return this.mock.userHasMasterPassword(userId); + } + emailToSalt(email: string): MasterPasswordSalt { return this.mock.emailToSalt(email); } diff --git a/libs/common/src/key-management/master-password/services/master-password.service.ts b/libs/common/src/key-management/master-password/services/master-password.service.ts index 8012a9230e7..c2947b2263d 100644 --- a/libs/common/src/key-management/master-password/services/master-password.service.ts +++ b/libs/common/src/key-management/master-password/services/master-password.service.ts @@ -25,6 +25,7 @@ import { MasterKey, UserKey } from "../../../types/key"; import { KeyGenerationService } from "../../crypto"; import { CryptoFunctionService } from "../../crypto/abstractions/crypto-function.service"; import { EncryptedString, EncString } from "../../crypto/models/enc-string"; +import { USES_KEY_CONNECTOR } from "../../key-connector/services/key-connector.service"; import { InternalMasterPasswordServiceAbstraction } from "../abstractions/master-password.service.abstraction"; import { MasterKeyWrappedUserKey, @@ -85,6 +86,19 @@ export class MasterPasswordService implements InternalMasterPasswordServiceAbstr private accountService: AccountService, ) {} + async userHasMasterPassword(userId: UserId): Promise { + assertNonNullish(userId, "userId"); + // A user has a master-password if they have a master-key encrypted user key *but* are not a key connector user + // Note: We can't use the key connector service as an abstraction here because it causes a run-time dependency injection cycle between KC service and MP service. + const usesKeyConnector = await firstValueFrom( + this.stateProvider.getUser(userId, USES_KEY_CONNECTOR).state$, + ); + const usesMasterKey = await firstValueFrom( + this.stateProvider.getUser(userId, MASTER_KEY_ENCRYPTED_USER_KEY).state$, + ); + return usesMasterKey && !usesKeyConnector; + } + saltForUser$(userId: UserId): Observable { assertNonNullish(userId, "userId"); return this.accountService.accounts$.pipe( @@ -307,6 +321,7 @@ export class MasterPasswordService implements InternalMasterPasswordServiceAbstr masterPasswordUnlockData.kdf.toSdkConfig(), ), ); + return userKey as UserKey; } diff --git a/libs/key-management-ui/src/lock/components/lock.component.spec.ts b/libs/key-management-ui/src/lock/components/lock.component.spec.ts index b51d4c13490..b708d101f82 100644 --- a/libs/key-management-ui/src/lock/components/lock.component.spec.ts +++ b/libs/key-management-ui/src/lock/components/lock.component.spec.ts @@ -22,6 +22,7 @@ import { } from "@bitwarden/common/auth/types/verification"; import { ClientType, DeviceType } from "@bitwarden/common/enums"; import { DeviceTrustServiceAbstraction } from "@bitwarden/common/key-management/device-trust/abstractions/device-trust.service.abstraction"; +import { EncryptedMigrator } from "@bitwarden/common/key-management/encrypted-migrator/encrypted-migrator.abstraction"; import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; import { PinServiceAbstraction } from "@bitwarden/common/key-management/pin/pin.service.abstraction"; import { BroadcasterService } from "@bitwarden/common/platform/abstractions/broadcaster.service"; @@ -91,6 +92,7 @@ describe("LockComponent", () => { const mockLockComponentService = mock(); const mockAnonLayoutWrapperDataService = mock(); const mockBroadcasterService = mock(); + const mockEncryptedMigrator = mock(); const mockConfigService = mock(); beforeEach(async () => { @@ -149,6 +151,7 @@ describe("LockComponent", () => { { provide: LockComponentService, useValue: mockLockComponentService }, { provide: AnonLayoutWrapperDataService, useValue: mockAnonLayoutWrapperDataService }, { provide: BroadcasterService, useValue: mockBroadcasterService }, + { provide: EncryptedMigrator, useValue: mockEncryptedMigrator }, { provide: ConfigService, useValue: mockConfigService }, ], }) diff --git a/libs/key-management-ui/src/lock/components/lock.component.ts b/libs/key-management-ui/src/lock/components/lock.component.ts index 801a9d191f5..7f715d2215d 100644 --- a/libs/key-management-ui/src/lock/components/lock.component.ts +++ b/libs/key-management-ui/src/lock/components/lock.component.ts @@ -31,6 +31,7 @@ import { import { ClientType, DeviceType } from "@bitwarden/common/enums"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { DeviceTrustServiceAbstraction } from "@bitwarden/common/key-management/device-trust/abstractions/device-trust.service.abstraction"; +import { EncryptedMigrator } from "@bitwarden/common/key-management/encrypted-migrator/encrypted-migrator.abstraction"; import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; import { PinServiceAbstraction } from "@bitwarden/common/key-management/pin/pin.service.abstraction"; import { BroadcasterService } from "@bitwarden/common/platform/abstractions/broadcaster.service"; @@ -177,6 +178,8 @@ export class LockComponent implements OnInit, OnDestroy { private logoutService: LogoutService, private lockComponentService: LockComponentService, private anonLayoutWrapperDataService: AnonLayoutWrapperDataService, + private encryptedMigrator: EncryptedMigrator, + private configService: ConfigService, // desktop deps private broadcasterService: BroadcasterService, @@ -639,6 +642,16 @@ export class LockComponent implements OnInit, OnDestroy { } await this.biometricStateService.resetUserPromptCancelled(); + + try { + await this.encryptedMigrator.runMigrations( + this.activeAccount.id, + afterUnlockActions.passwordEvaluation?.masterPassword ?? null, + ); + } catch { + // Don't block login success on migration failure + } + this.messagingService.send("unlocked"); if (afterUnlockActions.passwordEvaluation) { diff --git a/libs/state/src/core/state-definitions.ts b/libs/state/src/core/state-definitions.ts index 9d404f14dd7..156c03620b7 100644 --- a/libs/state/src/core/state-definitions.ts +++ b/libs/state/src/core/state-definitions.ts @@ -54,8 +54,6 @@ export const DEVICE_TRUST_DISK_LOCAL = new StateDefinition("deviceTrust", "disk" web: "disk-local", browser: "disk-backup-local-storage", }); -export const KDF_CONFIG_DISK = new StateDefinition("kdfConfig", "disk"); -export const KEY_CONNECTOR_DISK = new StateDefinition("keyConnector", "disk"); export const LOGIN_EMAIL_DISK = new StateDefinition("loginEmail", "disk", { web: "disk-local", }); @@ -64,8 +62,6 @@ export const LOGIN_STRATEGY_MEMORY = new StateDefinition("loginStrategy", "memor export const MASTER_PASSWORD_DISK = new StateDefinition("masterPassword", "disk"); export const MASTER_PASSWORD_MEMORY = new StateDefinition("masterPassword", "memory"); export const MASTER_PASSWORD_UNLOCK_DISK = new StateDefinition("masterPasswordUnlock", "disk"); -export const PIN_DISK = new StateDefinition("pinUnlock", "disk"); -export const PIN_MEMORY = new StateDefinition("pinUnlock", "memory"); export const ROUTER_DISK = new StateDefinition("router", "disk"); export const SSO_DISK = new StateDefinition("ssoLogin", "disk"); export const SSO_DISK_LOCAL = new StateDefinition("ssoLoginLocal", "disk", { web: "disk-local" }); @@ -117,13 +113,10 @@ export const PHISHING_DETECTION_DISK = new StateDefinition("phishingDetection", export const APPLICATION_ID_DISK = new StateDefinition("applicationId", "disk", { web: "disk-local", }); -export const BIOMETRIC_SETTINGS_DISK = new StateDefinition("biometricSettings", "disk"); export const CLEAR_EVENT_DISK = new StateDefinition("clearEvent", "disk"); export const CONFIG_DISK = new StateDefinition("config", "disk", { web: "disk-local", }); -export const CRYPTO_DISK = new StateDefinition("crypto", "disk"); -export const CRYPTO_MEMORY = new StateDefinition("crypto", "memory"); export const DESKTOP_SETTINGS_DISK = new StateDefinition("desktopSettings", "disk"); export const ENVIRONMENT_DISK = new StateDefinition("environment", "disk"); export const ENVIRONMENT_MEMORY = new StateDefinition("environment", "memory"); @@ -225,3 +218,14 @@ export const VAULT_BROWSER_INTRO_CAROUSEL = new StateDefinition( "disk", ); export const VAULT_AT_RISK_PASSWORDS_MEMORY = new StateDefinition("vaultAtRiskPasswords", "memory"); + +// KM + +export const BIOMETRIC_SETTINGS_DISK = new StateDefinition("biometricSettings", "disk"); +export const ENCRYPTED_MIGRATION_DISK = new StateDefinition("encryptedMigration", "disk"); +export const PIN_DISK = new StateDefinition("pinUnlock", "disk"); +export const PIN_MEMORY = new StateDefinition("pinUnlock", "memory"); +export const CRYPTO_DISK = new StateDefinition("crypto", "disk"); +export const CRYPTO_MEMORY = new StateDefinition("crypto", "memory"); +export const KDF_CONFIG_DISK = new StateDefinition("kdfConfig", "disk"); +export const KEY_CONNECTOR_DISK = new StateDefinition("keyConnector", "disk"); From 1bfff49ef5d6fc9489d0d6422fd4887388f8bd30 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Wed, 3 Dec 2025 20:10:10 +0100 Subject: [PATCH 08/12] [PM-29122] Fix debug build causing slow unlock (#17798) * Fix debug build causing slow unlock * Cleanup * Fix release mode build actually building debug --- .github/workflows/build-desktop.yml | 8 ++++++-- apps/desktop/desktop_native/napi/scripts/build.js | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/.github/workflows/build-desktop.yml b/.github/workflows/build-desktop.yml index c973796207c..ab5a1a50c17 100644 --- a/.github/workflows/build-desktop.yml +++ b/.github/workflows/build-desktop.yml @@ -263,9 +263,11 @@ jobs: PKG_CONFIG_ALLOW_CROSS: true PKG_CONFIG_ALL_STATIC: true TARGET: musl + # Note: It is important that we use the release build because some compute heavy + # operations such as key derivation for oo7 on linux are too slow in debug mode run: | rustup target add x86_64-unknown-linux-musl - node build.js --target=x86_64-unknown-linux-musl + node build.js --target=x86_64-unknown-linux-musl --release - name: Build application run: npm run dist:lin @@ -426,9 +428,11 @@ jobs: PKG_CONFIG_ALLOW_CROSS: true PKG_CONFIG_ALL_STATIC: true TARGET: musl + # Note: It is important that we use the release build because some compute heavy + # operations such as key derivation for oo7 on linux are too slow in debug mode run: | rustup target add aarch64-unknown-linux-musl - node build.js --target=aarch64-unknown-linux-musl + node build.js --target=aarch64-unknown-linux-musl --release - name: Check index.d.ts generated if: github.event_name == 'pull_request' && steps.cache.outputs.cache-hit != 'true' diff --git a/apps/desktop/desktop_native/napi/scripts/build.js b/apps/desktop/desktop_native/napi/scripts/build.js index a6680f5d311..7b3dccf81e4 100644 --- a/apps/desktop/desktop_native/napi/scripts/build.js +++ b/apps/desktop/desktop_native/napi/scripts/build.js @@ -11,4 +11,4 @@ if (isRelease) { process.env.RUST_LOG = 'debug'; } -execSync(`napi build --platform --js false`, { stdio: 'inherit', env: process.env }); +execSync(`napi build --platform --js false ${isRelease ? '--release' : ''}`, { stdio: 'inherit', env: process.env }); From d64da69fa7488421c3063e3784a5ec5fd35768bf Mon Sep 17 00:00:00 2001 From: Alex <55413326+AlexRubik@users.noreply.github.com> Date: Wed, 3 Dec 2025 14:21:58 -0500 Subject: [PATCH 09/12] [PM-6979] Remove HIBP 404 handling (#17769) --- .../dirt/services/hibp-api.service.spec.ts | 21 +++++++++++++++++++ .../common/src/services/audit.service.spec.ts | 13 ++++++------ libs/common/src/services/audit.service.ts | 11 +--------- 3 files changed, 29 insertions(+), 16 deletions(-) diff --git a/libs/common/src/dirt/services/hibp-api.service.spec.ts b/libs/common/src/dirt/services/hibp-api.service.spec.ts index fd2a54bdd10..9e08b4d0623 100644 --- a/libs/common/src/dirt/services/hibp-api.service.spec.ts +++ b/libs/common/src/dirt/services/hibp-api.service.spec.ts @@ -35,5 +35,26 @@ describe("HibpApiService", () => { expect(result).toHaveLength(1); expect(result[0]).toBeInstanceOf(BreachAccountResponse); }); + + it("should return empty array when no breaches found (REST semantics)", async () => { + // Server now returns 200 OK with empty array [] instead of 404 + const mockResponse: any[] = []; + const username = "safe@example.com"; + + apiService.send.mockResolvedValue(mockResponse); + + const result = await sut.getHibpBreach(username); + + expect(apiService.send).toHaveBeenCalledWith( + "GET", + "/hibp/breach?username=" + encodeURIComponent(username), + null, + true, + true, + ); + expect(result).toEqual([]); + expect(result).toBeInstanceOf(Array); + expect(result).toHaveLength(0); + }); }); }); diff --git a/libs/common/src/services/audit.service.spec.ts b/libs/common/src/services/audit.service.spec.ts index b0e96eb5c5c..e653b026735 100644 --- a/libs/common/src/services/audit.service.spec.ts +++ b/libs/common/src/services/audit.service.spec.ts @@ -1,7 +1,6 @@ import { ApiService } from "../abstractions/api.service"; import { HibpApiService } from "../dirt/services/hibp-api.service"; import { CryptoFunctionService } from "../key-management/crypto/abstractions/crypto-function.service"; -import { ErrorResponse } from "../models/response/error.response"; import { AuditService } from "./audit.service"; @@ -73,14 +72,16 @@ describe("AuditService", () => { expect(mockApi.nativeFetch).toHaveBeenCalledTimes(4); }); - it("should return empty array for breachedAccounts on 404", async () => { - mockHibpApi.getHibpBreach.mockRejectedValueOnce({ statusCode: 404 } as ErrorResponse); + it("should return empty array for breachedAccounts when no breaches found", async () => { + // Server returns 200 with empty array (correct REST semantics) + mockHibpApi.getHibpBreach.mockResolvedValueOnce([]); const result = await auditService.breachedAccounts("user@example.com"); expect(result).toEqual([]); }); - it("should throw error for breachedAccounts on non-404 error", async () => { - mockHibpApi.getHibpBreach.mockRejectedValueOnce({ statusCode: 500 } as ErrorResponse); - await expect(auditService.breachedAccounts("user@example.com")).rejects.toThrow(); + it("should propagate errors from breachedAccounts", async () => { + const error = new Error("API error"); + mockHibpApi.getHibpBreach.mockRejectedValueOnce(error); + await expect(auditService.breachedAccounts("user@example.com")).rejects.toBe(error); }); }); diff --git a/libs/common/src/services/audit.service.ts b/libs/common/src/services/audit.service.ts index 0bdf45917de..7762c2cbd93 100644 --- a/libs/common/src/services/audit.service.ts +++ b/libs/common/src/services/audit.service.ts @@ -6,7 +6,6 @@ import { AuditService as AuditServiceAbstraction } from "../abstractions/audit.s import { BreachAccountResponse } from "../dirt/models/response/breach-account.response"; import { HibpApiService } from "../dirt/services/hibp-api.service"; import { CryptoFunctionService } from "../key-management/crypto/abstractions/crypto-function.service"; -import { ErrorResponse } from "../models/response/error.response"; import { Utils } from "../platform/misc/utils"; const PwnedPasswordsApi = "https://api.pwnedpasswords.com/range/"; @@ -70,14 +69,6 @@ export class AuditService implements AuditServiceAbstraction { } async breachedAccounts(username: string): Promise { - try { - return await this.hibpApiService.getHibpBreach(username); - } catch (e) { - const error = e as ErrorResponse; - if (error.statusCode === 404) { - return []; - } - throw new Error(); - } + return this.hibpApiService.getHibpBreach(username); } } From 28fbddb63f80653ea7f6bc3efed8545b3d2ae5ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20=C3=85berg?= Date: Wed, 3 Dec 2025 20:40:55 +0100 Subject: [PATCH 10/12] fix(passkeys): [PM-28324] Add a guard that conditionally forces a popout depending on platform * Add a guard that conditionally forces a popout depending on platform * Test the routeguard * Use mockImplementation instead. * autoclose popout --- .../guards/platform-popout.guard.spec.ts | 193 ++++++++++++++++++ .../popup/guards/platform-popout.guard.ts | 46 +++++ apps/browser/src/popup/app-routing.module.ts | 3 +- .../login-via-webauthn.component.ts | 21 +- 4 files changed, 261 insertions(+), 2 deletions(-) create mode 100644 apps/browser/src/auth/popup/guards/platform-popout.guard.spec.ts create mode 100644 apps/browser/src/auth/popup/guards/platform-popout.guard.ts diff --git a/apps/browser/src/auth/popup/guards/platform-popout.guard.spec.ts b/apps/browser/src/auth/popup/guards/platform-popout.guard.spec.ts new file mode 100644 index 00000000000..d39012fd88a --- /dev/null +++ b/apps/browser/src/auth/popup/guards/platform-popout.guard.spec.ts @@ -0,0 +1,193 @@ +import { TestBed } from "@angular/core/testing"; +import { ActivatedRouteSnapshot, RouterStateSnapshot } from "@angular/router"; + +import { BrowserApi } from "../../../platform/browser/browser-api"; +import BrowserPopupUtils from "../../../platform/browser/browser-popup-utils"; + +import { platformPopoutGuard } from "./platform-popout.guard"; + +describe("platformPopoutGuard", () => { + let getPlatformInfoSpy: jest.SpyInstance; + let inPopoutSpy: jest.SpyInstance; + let inSidebarSpy: jest.SpyInstance; + let openPopoutSpy: jest.SpyInstance; + let closePopupSpy: jest.SpyInstance; + + const mockRoute = {} as ActivatedRouteSnapshot; + const mockState: RouterStateSnapshot = { + url: "/login-with-passkey?param=value", + } as RouterStateSnapshot; + + beforeEach(() => { + getPlatformInfoSpy = jest.spyOn(BrowserApi, "getPlatformInfo"); + inPopoutSpy = jest.spyOn(BrowserPopupUtils, "inPopout"); + inSidebarSpy = jest.spyOn(BrowserPopupUtils, "inSidebar"); + openPopoutSpy = jest.spyOn(BrowserPopupUtils, "openPopout").mockImplementation(); + closePopupSpy = jest.spyOn(BrowserApi, "closePopup").mockImplementation(); + + TestBed.configureTestingModule({}); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + describe("when platform matches", () => { + beforeEach(() => { + getPlatformInfoSpy.mockResolvedValue({ os: "linux" }); + inPopoutSpy.mockReturnValue(false); + inSidebarSpy.mockReturnValue(false); + }); + + it("should open popout and block navigation when not already in popout or sidebar", async () => { + const guard = platformPopoutGuard(["linux"]); + const result = await TestBed.runInInjectionContext(() => guard(mockRoute, mockState)); + + expect(getPlatformInfoSpy).toHaveBeenCalled(); + expect(inPopoutSpy).toHaveBeenCalledWith(window); + expect(inSidebarSpy).toHaveBeenCalledWith(window); + expect(openPopoutSpy).toHaveBeenCalledWith( + "popup/index.html#/login-with-passkey?param=value&autoClosePopout=true", + ); + expect(closePopupSpy).toHaveBeenCalledWith(window); + expect(result).toBe(false); + }); + + it("should allow navigation when already in popout", async () => { + inPopoutSpy.mockReturnValue(true); + + const guard = platformPopoutGuard(["linux"]); + const result = await TestBed.runInInjectionContext(() => guard(mockRoute, mockState)); + + expect(openPopoutSpy).not.toHaveBeenCalled(); + expect(closePopupSpy).not.toHaveBeenCalled(); + expect(result).toBe(true); + }); + + it("should allow navigation when already in sidebar", async () => { + inSidebarSpy.mockReturnValue(true); + + const guard = platformPopoutGuard(["linux"]); + const result = await TestBed.runInInjectionContext(() => guard(mockRoute, mockState)); + + expect(openPopoutSpy).not.toHaveBeenCalled(); + expect(closePopupSpy).not.toHaveBeenCalled(); + expect(result).toBe(true); + }); + }); + + describe("when platform does not match", () => { + beforeEach(() => { + getPlatformInfoSpy.mockResolvedValue({ os: "win" }); + inPopoutSpy.mockReturnValue(false); + inSidebarSpy.mockReturnValue(false); + }); + + it("should allow navigation without opening popout", async () => { + const guard = platformPopoutGuard(["linux"]); + const result = await TestBed.runInInjectionContext(() => guard(mockRoute, mockState)); + + expect(getPlatformInfoSpy).toHaveBeenCalled(); + expect(openPopoutSpy).not.toHaveBeenCalled(); + expect(result).toBe(true); + }); + }); + + describe("when forcePopout is true", () => { + beforeEach(() => { + getPlatformInfoSpy.mockResolvedValue({ os: "win" }); + inPopoutSpy.mockReturnValue(false); + inSidebarSpy.mockReturnValue(false); + }); + + it("should open popout regardless of platform", async () => { + const guard = platformPopoutGuard(["linux"], true); + const result = await TestBed.runInInjectionContext(() => guard(mockRoute, mockState)); + + expect(openPopoutSpy).toHaveBeenCalledWith( + "popup/index.html#/login-with-passkey?param=value&autoClosePopout=true", + ); + expect(closePopupSpy).toHaveBeenCalledWith(window); + expect(result).toBe(false); + }); + + it("should not open popout when already in popout", async () => { + inPopoutSpy.mockReturnValue(true); + + const guard = platformPopoutGuard(["linux"], true); + const result = await TestBed.runInInjectionContext(() => guard(mockRoute, mockState)); + + expect(openPopoutSpy).not.toHaveBeenCalled(); + expect(result).toBe(true); + }); + }); + + describe("with multiple platforms", () => { + beforeEach(() => { + inPopoutSpy.mockReturnValue(false); + inSidebarSpy.mockReturnValue(false); + }); + + it.each(["linux", "mac", "win"])( + "should open popout when platform is %s and included in platforms array", + async (platform) => { + getPlatformInfoSpy.mockResolvedValue({ os: platform }); + + const guard = platformPopoutGuard(["linux", "mac", "win"]); + const result = await TestBed.runInInjectionContext(() => guard(mockRoute, mockState)); + + expect(openPopoutSpy).toHaveBeenCalledWith( + "popup/index.html#/login-with-passkey?param=value&autoClosePopout=true", + ); + expect(closePopupSpy).toHaveBeenCalledWith(window); + expect(result).toBe(false); + }, + ); + + it("should not open popout when platform is not in the array", async () => { + getPlatformInfoSpy.mockResolvedValue({ os: "android" }); + + const guard = platformPopoutGuard(["linux", "mac"]); + const result = await TestBed.runInInjectionContext(() => guard(mockRoute, mockState)); + + expect(openPopoutSpy).not.toHaveBeenCalled(); + expect(result).toBe(true); + }); + }); + + describe("url handling", () => { + beforeEach(() => { + getPlatformInfoSpy.mockResolvedValue({ os: "linux" }); + inPopoutSpy.mockReturnValue(false); + inSidebarSpy.mockReturnValue(false); + }); + + it("should preserve query parameters in the popout url", async () => { + const stateWithQuery: RouterStateSnapshot = { + url: "/path?foo=bar&baz=qux", + } as RouterStateSnapshot; + + const guard = platformPopoutGuard(["linux"]); + await TestBed.runInInjectionContext(() => guard(mockRoute, stateWithQuery)); + + expect(openPopoutSpy).toHaveBeenCalledWith( + "popup/index.html#/path?foo=bar&baz=qux&autoClosePopout=true", + ); + expect(closePopupSpy).toHaveBeenCalledWith(window); + }); + + it("should handle urls without query parameters", async () => { + const stateWithoutQuery: RouterStateSnapshot = { + url: "/simple-path", + } as RouterStateSnapshot; + + const guard = platformPopoutGuard(["linux"]); + await TestBed.runInInjectionContext(() => guard(mockRoute, stateWithoutQuery)); + + expect(openPopoutSpy).toHaveBeenCalledWith( + "popup/index.html#/simple-path?autoClosePopout=true", + ); + expect(closePopupSpy).toHaveBeenCalledWith(window); + }); + }); +}); diff --git a/apps/browser/src/auth/popup/guards/platform-popout.guard.ts b/apps/browser/src/auth/popup/guards/platform-popout.guard.ts new file mode 100644 index 00000000000..aad005e141b --- /dev/null +++ b/apps/browser/src/auth/popup/guards/platform-popout.guard.ts @@ -0,0 +1,46 @@ +import { ActivatedRouteSnapshot, CanActivateFn, RouterStateSnapshot } from "@angular/router"; + +import { BrowserApi } from "../../../platform/browser/browser-api"; +import BrowserPopupUtils from "../../../platform/browser/browser-popup-utils"; + +/** + * Guard that forces a popout window for specific platforms. + * Useful when popup context would close during operations (e.g., WebAuthn on Linux). + * + * @param platforms - Array of platform OS strings (e.g., ["linux", "mac", "win"]) + * @param forcePopout - If true, always force popout regardless of platform (useful for testing) + * @returns CanActivateFn that opens popout and blocks navigation if conditions met + */ +export function platformPopoutGuard( + platforms: string[], + forcePopout: boolean = false, +): CanActivateFn { + return async (route: ActivatedRouteSnapshot, state: RouterStateSnapshot) => { + // Check if current platform matches + const platformInfo = await BrowserApi.getPlatformInfo(); + const isPlatformMatch = platforms.includes(platformInfo.os); + + // Check if already in popout/sidebar + const inPopout = BrowserPopupUtils.inPopout(window); + const inSidebar = BrowserPopupUtils.inSidebar(window); + + // Open popout if conditions met + if ((isPlatformMatch || forcePopout) && !inPopout && !inSidebar) { + // Add autoClosePopout query param to signal the popout should close after completion + const [path, existingQuery] = state.url.split("?"); + const params = new URLSearchParams(existingQuery || ""); + params.set("autoClosePopout", "true"); + const urlWithAutoClose = `${path}?${params.toString()}`; + + // Open the popout window + await BrowserPopupUtils.openPopout(`popup/index.html#${urlWithAutoClose}`); + + // Close the original popup window + BrowserApi.closePopup(window); + + return false; // Block navigation - popout will reload + } + + return true; // Allow navigation + }; +} diff --git a/apps/browser/src/popup/app-routing.module.ts b/apps/browser/src/popup/app-routing.module.ts index a36396afa1a..48f06147cdf 100644 --- a/apps/browser/src/popup/app-routing.module.ts +++ b/apps/browser/src/popup/app-routing.module.ts @@ -48,6 +48,7 @@ import { LockComponent, ConfirmKeyConnectorDomainComponent } from "@bitwarden/ke import { AccountSwitcherComponent } from "../auth/popup/account-switching/account-switcher.component"; import { AuthExtensionRoute } from "../auth/popup/constants/auth-extension-route.constant"; import { fido2AuthGuard } from "../auth/popup/guards/fido2-auth.guard"; +import { platformPopoutGuard } from "../auth/popup/guards/platform-popout.guard"; import { AccountSecurityComponent } from "../auth/popup/settings/account-security.component"; import { ExtensionDeviceManagementComponent } from "../auth/popup/settings/extension-device-management.component"; import { Fido2Component } from "../autofill/popup/fido2/fido2.component"; @@ -414,7 +415,7 @@ const routes: Routes = [ }, { path: AuthRoute.LoginWithPasskey, - canActivate: [unauthGuardFn(unauthRouteOverrides)], + canActivate: [unauthGuardFn(unauthRouteOverrides), platformPopoutGuard(["linux"])], data: { pageIcon: TwoFactorAuthSecurityKeyIcon, pageTitle: { diff --git a/libs/angular/src/auth/login-via-webauthn/login-via-webauthn.component.ts b/libs/angular/src/auth/login-via-webauthn/login-via-webauthn.component.ts index 764d9fe7733..b4d856309ed 100644 --- a/libs/angular/src/auth/login-via-webauthn/login-via-webauthn.component.ts +++ b/libs/angular/src/auth/login-via-webauthn/login-via-webauthn.component.ts @@ -2,7 +2,7 @@ // @ts-strict-ignore import { CommonModule } from "@angular/common"; import { Component, OnInit } from "@angular/core"; -import { Router, RouterModule } from "@angular/router"; +import { ActivatedRoute, Router, RouterModule } from "@angular/router"; import { firstValueFrom } from "rxjs"; import { JslibModule } from "@bitwarden/angular/jslib.module"; @@ -19,6 +19,7 @@ import { ClientType } from "@bitwarden/common/enums"; import { ErrorResponse } from "@bitwarden/common/models/response/error.response"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; +import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service"; import { @@ -49,6 +50,7 @@ export type State = "assert" | "assertFailed"; }) export class LoginViaWebAuthnComponent implements OnInit { protected currentState: State = "assert"; + private shouldAutoClosePopout = false; protected readonly Icons = { TwoFactorAuthSecurityKeyIcon, @@ -70,6 +72,7 @@ export class LoginViaWebAuthnComponent implements OnInit { constructor( private webAuthnLoginService: WebAuthnLoginServiceAbstraction, private router: Router, + private route: ActivatedRoute, private logService: LogService, private validationService: ValidationService, private i18nService: I18nService, @@ -77,9 +80,14 @@ export class LoginViaWebAuthnComponent implements OnInit { private keyService: KeyService, private platformUtilsService: PlatformUtilsService, private anonLayoutWrapperDataService: AnonLayoutWrapperDataService, + private messagingService: MessagingService, ) {} ngOnInit(): void { + // Check if we should auto-close the popout after successful authentication + this.shouldAutoClosePopout = + this.route.snapshot.queryParamMap.get("autoClosePopout") === "true"; + // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. // eslint-disable-next-line @typescript-eslint/no-floating-promises this.authenticate(); @@ -123,6 +131,17 @@ export class LoginViaWebAuthnComponent implements OnInit { await this.loginSuccessHandlerService.run(authResult.userId, null); } + // If autoClosePopout is enabled and we're in a browser extension, + // re-open the regular popup and close this popout window + if ( + this.shouldAutoClosePopout && + this.platformUtilsService.getClientType() === ClientType.Browser + ) { + this.messagingService.send("openPopup"); + window.close(); + return; + } + await this.router.navigate([this.successRoute]); } catch (error) { if (error instanceof ErrorResponse) { From 04d7744747e64e06feff308c2483152abe709238 Mon Sep 17 00:00:00 2001 From: Jonathan Prusik Date: Wed, 3 Dec 2025 15:12:12 -0500 Subject: [PATCH 11/12] normalize lowercasing for cipher compared against lowercased input value (#17803) --- apps/browser/src/autofill/background/notification.background.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/browser/src/autofill/background/notification.background.ts b/apps/browser/src/autofill/background/notification.background.ts index 547c5ba1575..17e3ec159c3 100644 --- a/apps/browser/src/autofill/background/notification.background.ts +++ b/apps/browser/src/autofill/background/notification.background.ts @@ -658,7 +658,7 @@ export default class NotificationBackground { if ( username !== null && newPassword === null && - cipher.login.username === normalizedUsername && + cipher.login.username.toLowerCase() === normalizedUsername && cipher.login.password === currentPassword ) { // Assumed to be a login From dab1a37bfe4b3a51aa7426533fa52795b2371474 Mon Sep 17 00:00:00 2001 From: Nick Krantz <125900171+nick-livefront@users.noreply.github.com> Date: Wed, 3 Dec 2025 14:19:26 -0600 Subject: [PATCH 12/12] PM-24535 Web premium upgrade path for archive (#16854) * add premium badge to web filter when the user does not have access to premium * remove feature flag pass through in favor of showing/hiding archive vault observable * refactor archive observable to be more generic * add archive premium badge for the web * show premium badge inline for archive filter * show premium subscription ended message when user has archived ciphers * fix missing refactor * remove unneeded can archive check * reference observable directly * reduce the number of firstValueFroms by combining observables into a single stream * fix failing tests * add import to storybook * update variable naming for premium filters * pass event to `promptForPremium` * remove check for organization * fix footer variable reference * refactor back to `hasArchiveFlagEnabled$` - more straight forward to the underlying logic * update archive service test with new feature flag format --- .../item-more-options.component.spec.ts | 2 +- .../item-more-options.component.ts | 2 +- .../settings/vault-settings-v2.component.ts | 2 +- .../vault/app/vault/item-footer.component.ts | 2 +- .../vault-filter/vault-filter.component.ts | 3 + .../vault-cipher-row.component.html | 20 +++- .../vault-cipher-row.component.spec.ts | 1 + .../vault-items/vault-cipher-row.component.ts | 18 +++- .../vault-items/vault-items.component.html | 1 + .../vault-items/vault-items.component.spec.ts | 7 ++ .../vault-items/vault-items.component.ts | 4 + .../vault-items/vault-items.module.ts | 2 + .../vault-items/vault-items.stories.ts | 7 ++ .../components/vault-filter.component.ts | 34 ++++++- .../vault-filter-section.component.html | 3 + .../vault-filter-section.component.ts | 9 ++ .../models/vault-filter-section.type.ts | 10 ++ .../shared/vault-filter-shared.module.ts | 3 +- .../individual-vault/vault.component.html | 10 ++ .../vault/individual-vault/vault.component.ts | 36 ++++--- apps/web/src/locales/en/messages.json | 9 ++ .../components/vault-filter.component.ts | 2 +- .../abstractions/cipher-archive.service.ts | 5 +- .../default-cipher-archive.service.spec.ts | 95 ++++++++++++++++++- .../default-cipher-archive.service.ts | 37 ++++---- 25 files changed, 265 insertions(+), 59 deletions(-) diff --git a/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.spec.ts b/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.spec.ts index 577b7d96771..b9f48b7407b 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.spec.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.spec.ts @@ -108,7 +108,7 @@ describe("ItemMoreOptionsComponent", () => { { provide: RestrictedItemTypesService, useValue: { restricted$: of([]) } }, { provide: CipherArchiveService, - useValue: { userCanArchive$: () => of(true), hasArchiveFlagEnabled$: () => of(true) }, + useValue: { userCanArchive$: () => of(true), hasArchiveFlagEnabled$: of(true) }, }, { provide: ToastService, useValue: { showToast: () => {} } }, { provide: Router, useValue: { navigate: () => Promise.resolve(true) } }, 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 4dfaf7bc66f..b65acc6ca8e 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 @@ -141,7 +141,7 @@ export class ItemMoreOptionsComponent { }), ); - protected showArchive$: Observable = this.cipherArchiveService.hasArchiveFlagEnabled$(); + protected showArchive$: Observable = this.cipherArchiveService.hasArchiveFlagEnabled$; protected canArchive$: Observable = this.accountService.activeAccount$.pipe( getUserId, diff --git a/apps/browser/src/vault/popup/settings/vault-settings-v2.component.ts b/apps/browser/src/vault/popup/settings/vault-settings-v2.component.ts index c6db820c232..e085cb21c2d 100644 --- a/apps/browser/src/vault/popup/settings/vault-settings-v2.component.ts +++ b/apps/browser/src/vault/popup/settings/vault-settings-v2.component.ts @@ -49,7 +49,7 @@ export class VaultSettingsV2Component implements OnInit, OnDestroy { this.userId$.pipe(switchMap((userId) => this.cipherArchiveService.userCanArchive$(userId))), ); - protected readonly showArchiveItem = toSignal(this.cipherArchiveService.hasArchiveFlagEnabled$()); + protected readonly showArchiveItem = toSignal(this.cipherArchiveService.hasArchiveFlagEnabled$); protected readonly userHasArchivedItems = toSignal( this.userId$.pipe( diff --git a/apps/desktop/src/vault/app/vault/item-footer.component.ts b/apps/desktop/src/vault/app/vault/item-footer.component.ts index 0034bd9a43c..0ac12c928f2 100644 --- a/apps/desktop/src/vault/app/vault/item-footer.component.ts +++ b/apps/desktop/src/vault/app/vault/item-footer.component.ts @@ -225,7 +225,7 @@ export class ItemFooterComponent implements OnInit, OnChanges { switchMap((id) => combineLatest([ this.cipherArchiveService.userCanArchive$(id), - this.cipherArchiveService.hasArchiveFlagEnabled$(), + this.cipherArchiveService.hasArchiveFlagEnabled$, ]), ), ), diff --git a/apps/web/src/app/admin-console/organizations/collections/vault-filter/vault-filter.component.ts b/apps/web/src/app/admin-console/organizations/collections/vault-filter/vault-filter.component.ts index 01e61f0ab28..a253bb87c50 100644 --- a/apps/web/src/app/admin-console/organizations/collections/vault-filter/vault-filter.component.ts +++ b/apps/web/src/app/admin-console/organizations/collections/vault-filter/vault-filter.component.ts @@ -11,6 +11,7 @@ import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.servic import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { CipherArchiveService } from "@bitwarden/common/vault/abstractions/cipher-archive.service"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { PremiumUpgradePromptService } from "@bitwarden/common/vault/abstractions/premium-upgrade-prompt.service"; import { TreeNode } from "@bitwarden/common/vault/models/domain/tree-node"; import { RestrictedItemTypesService } from "@bitwarden/common/vault/services/restricted-item-types.service"; import { DialogService, ToastService } from "@bitwarden/components"; @@ -59,6 +60,7 @@ export class VaultFilterComponent protected restrictedItemTypesService: RestrictedItemTypesService, protected cipherService: CipherService, protected cipherArchiveService: CipherArchiveService, + premiumUpgradePromptService: PremiumUpgradePromptService, ) { super( vaultFilterService, @@ -72,6 +74,7 @@ export class VaultFilterComponent restrictedItemTypesService, cipherService, cipherArchiveService, + premiumUpgradePromptService, ); } diff --git a/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.html b/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.html index c8732154ef4..d56c9d15cff 100644 --- a/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.html +++ b/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.html @@ -203,10 +203,22 @@ {{ "eventLogs" | i18n }} @if (showArchiveButton) { - + @if (userCanArchive) { + + } + @if (!userCanArchive) { + + } } @if (showUnArchiveButton) { diff --git a/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.spec.ts b/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.spec.ts index d5f7b54f37a..9378ee54e51 100644 --- a/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.spec.ts +++ b/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.spec.ts @@ -72,6 +72,7 @@ describe("VaultCipherRowComponent", () => { fixture = TestBed.createComponent(VaultCipherRowComponent); component = fixture.componentInstance; + fixture.componentRef.setInput("archiveEnabled", false); overlayContainer = TestBed.inject(OverlayContainer); }); diff --git a/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.ts b/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.ts index 4ea062db8d1..92c49ac218a 100644 --- a/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.ts +++ b/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.ts @@ -8,6 +8,7 @@ import { OnInit, Output, ViewChild, + input, } from "@angular/core"; import { CollectionView } from "@bitwarden/admin-console/common"; @@ -101,8 +102,10 @@ export class VaultCipherRowComponent implements OnInit // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals // eslint-disable-next-line @angular-eslint/prefer-signals @Input() userCanArchive: boolean; + /** Archive feature is enabled */ + readonly archiveEnabled = input.required(); /** - * Enforge Org Data Ownership Policy Status + * Enforce Org Data Ownership Policy Status */ // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals // eslint-disable-next-line @angular-eslint/prefer-signals @@ -142,16 +145,21 @@ export class VaultCipherRowComponent implements OnInit } protected get showArchiveButton() { + if (!this.archiveEnabled()) { + return false; + } + return ( - this.userCanArchive && - !CipherViewLikeUtils.isArchived(this.cipher) && - !CipherViewLikeUtils.isDeleted(this.cipher) && - !this.cipher.organizationId + !CipherViewLikeUtils.isArchived(this.cipher) && !CipherViewLikeUtils.isDeleted(this.cipher) ); } // If item is archived always show unarchive button, even if user is not premium protected get showUnArchiveButton() { + if (!this.archiveEnabled()) { + return false; + } + return CipherViewLikeUtils.isArchived(this.cipher); } diff --git a/apps/web/src/app/vault/components/vault-items/vault-items.component.html b/apps/web/src/app/vault/components/vault-items/vault-items.component.html index cb2af9a64e5..70c44e80a39 100644 --- a/apps/web/src/app/vault/components/vault-items/vault-items.component.html +++ b/apps/web/src/app/vault/components/vault-items/vault-items.component.html @@ -179,6 +179,7 @@ (onEvent)="event($event)" [userCanArchive]="userCanArchive" [enforceOrgDataOwnershipPolicy]="enforceOrgDataOwnershipPolicy" + [archiveEnabled]="archiveFeatureEnabled$ | async" > diff --git a/apps/web/src/app/vault/components/vault-items/vault-items.component.spec.ts b/apps/web/src/app/vault/components/vault-items/vault-items.component.spec.ts index 902fc2eb5a2..1eccb4c49ce 100644 --- a/apps/web/src/app/vault/components/vault-items/vault-items.component.spec.ts +++ b/apps/web/src/app/vault/components/vault-items/vault-items.component.spec.ts @@ -4,6 +4,7 @@ import { of } from "rxjs"; import { CollectionView } from "@bitwarden/admin-console/common"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { CipherArchiveService } from "@bitwarden/common/vault/abstractions/cipher-archive.service"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { CipherAuthorizationService } from "@bitwarden/common/vault/services/cipher-authorization.service"; import { RestrictedItemTypesService } from "@bitwarden/common/vault/services/restricted-item-types.service"; @@ -54,6 +55,12 @@ describe("VaultItemsComponent", () => { t: (key: string) => key, }, }, + { + provide: CipherArchiveService, + useValue: { + hasArchiveFlagEnabled$: of(true), + }, + }, ], }); diff --git a/apps/web/src/app/vault/components/vault-items/vault-items.component.ts b/apps/web/src/app/vault/components/vault-items/vault-items.component.ts index 3ab643927f1..a935314eb3a 100644 --- a/apps/web/src/app/vault/components/vault-items/vault-items.component.ts +++ b/apps/web/src/app/vault/components/vault-items/vault-items.component.ts @@ -7,6 +7,7 @@ import { Observable, combineLatest, map, of, startWith, switchMap } from "rxjs"; import { CollectionView, Unassigned, CollectionAdminView } from "@bitwarden/admin-console/common"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { CipherArchiveService } from "@bitwarden/common/vault/abstractions/cipher-archive.service"; import { CipherAuthorizationService } from "@bitwarden/common/vault/services/cipher-authorization.service"; import { RestrictedCipherType, @@ -145,9 +146,12 @@ export class VaultItemsComponent { protected disableMenu$: Observable; private restrictedTypes: RestrictedCipherType[] = []; + protected archiveFeatureEnabled$ = this.cipherArchiveService.hasArchiveFlagEnabled$; + constructor( protected cipherAuthorizationService: CipherAuthorizationService, protected restrictedItemTypesService: RestrictedItemTypesService, + protected cipherArchiveService: CipherArchiveService, ) { this.canDeleteSelected$ = this.selection.changed.pipe( startWith(null), diff --git a/apps/web/src/app/vault/components/vault-items/vault-items.module.ts b/apps/web/src/app/vault/components/vault-items/vault-items.module.ts index a3a92559878..a7c264114b9 100644 --- a/apps/web/src/app/vault/components/vault-items/vault-items.module.ts +++ b/apps/web/src/app/vault/components/vault-items/vault-items.module.ts @@ -3,6 +3,7 @@ import { CommonModule } from "@angular/common"; import { NgModule } from "@angular/core"; import { RouterModule } from "@angular/router"; +import { PremiumBadgeComponent } from "@bitwarden/angular/billing/components/premium-badge"; import { ScrollLayoutDirective, TableModule } from "@bitwarden/components"; import { CopyCipherFieldDirective } from "@bitwarden/vault"; @@ -29,6 +30,7 @@ import { VaultItemsComponent } from "./vault-items.component"; PipesModule, CopyCipherFieldDirective, ScrollLayoutDirective, + PremiumBadgeComponent, ], declarations: [VaultItemsComponent, VaultCipherRowComponent, VaultCollectionRowComponent], exports: [VaultItemsComponent], diff --git a/apps/web/src/app/vault/components/vault-items/vault-items.stories.ts b/apps/web/src/app/vault/components/vault-items/vault-items.stories.ts index 043ae900b40..d973fbcbbc7 100644 --- a/apps/web/src/app/vault/components/vault-items/vault-items.stories.ts +++ b/apps/web/src/app/vault/components/vault-items/vault-items.stories.ts @@ -30,6 +30,7 @@ import { import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { CollectionId, OrganizationId } from "@bitwarden/common/types/guid"; +import { CipherArchiveService } from "@bitwarden/common/vault/abstractions/cipher-archive.service"; import { CipherType } from "@bitwarden/common/vault/enums"; import { AttachmentView } from "@bitwarden/common/vault/models/view/attachment.view"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; @@ -143,6 +144,12 @@ export default { isCipherRestricted: () => false, // No restrictions for this story }, }, + { + provide: CipherArchiveService, + useValue: { + hasArchiveFlagEnabled$: of(true), + }, + }, ], }), applicationConfig({ diff --git a/apps/web/src/app/vault/individual-vault/vault-filter/components/vault-filter.component.ts b/apps/web/src/app/vault/individual-vault/vault-filter/components/vault-filter.component.ts index e40a32dc8b9..8839fa5039d 100644 --- a/apps/web/src/app/vault/individual-vault/vault-filter/components/vault-filter.component.ts +++ b/apps/web/src/app/vault/individual-vault/vault-filter/components/vault-filter.component.ts @@ -19,8 +19,10 @@ import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { BillingApiServiceAbstraction } from "@bitwarden/common/billing/abstractions/billing-api.service.abstraction"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { UserId } from "@bitwarden/common/types/guid"; import { CipherArchiveService } from "@bitwarden/common/vault/abstractions/cipher-archive.service"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { PremiumUpgradePromptService } from "@bitwarden/common/vault/abstractions/premium-upgrade-prompt.service"; import { CipherType } from "@bitwarden/common/vault/enums"; import { TreeNode } from "@bitwarden/common/vault/models/domain/tree-node"; import { RestrictedItemTypesService } from "@bitwarden/common/vault/services/restricted-item-types.service"; @@ -170,6 +172,7 @@ export class VaultFilterComponent implements OnInit, OnDestroy { protected restrictedItemTypesService: RestrictedItemTypesService, protected cipherService: CipherService, protected cipherArchiveService: CipherArchiveService, + private premiumUpgradePromptService: PremiumUpgradePromptService, ) {} async ngOnInit(): Promise { @@ -252,14 +255,20 @@ export class VaultFilterComponent implements OnInit, OnDestroy { }; async buildAllFilters(): Promise { - const hasArchiveFlag = await firstValueFrom(this.cipherArchiveService.hasArchiveFlagEnabled$()); + const [userId, showArchive] = await firstValueFrom( + combineLatest([ + this.accountService.activeAccount$.pipe(getUserId), + this.cipherArchiveService.hasArchiveFlagEnabled$, + ]), + ); + const builderFilter = {} as VaultFilterList; builderFilter.organizationFilter = await this.addOrganizationFilter(); builderFilter.typeFilter = await this.addTypeFilter(); builderFilter.folderFilter = await this.addFolderFilter(); builderFilter.collectionFilter = await this.addCollectionFilter(); - if (hasArchiveFlag) { - builderFilter.archiveFilter = await this.addArchiveFilter(); + if (showArchive) { + builderFilter.archiveFilter = await this.addArchiveFilter(userId); } builderFilter.trashFilter = await this.addTrashFilter(); return builderFilter; @@ -419,7 +428,18 @@ export class VaultFilterComponent implements OnInit, OnDestroy { return trashFilterSection; } - protected async addArchiveFilter(): Promise { + protected async addArchiveFilter(userId: UserId): Promise { + const [hasArchivedCiphers, userHasPremium] = await firstValueFrom( + combineLatest([ + this.cipherArchiveService + .archivedCiphers$(userId) + .pipe(map((archivedCiphers) => archivedCiphers.length > 0)), + this.cipherArchiveService.userHasPremium$(userId), + ]), + ); + + const promptForPremiumOnFilter = !userHasPremium && !hasArchivedCiphers; + const archiveFilterSection: VaultFilterSection = { data$: this.vaultFilterService.buildTypeTree( { @@ -442,6 +462,12 @@ export class VaultFilterComponent implements OnInit, OnDestroy { isSelectable: true, }, action: this.applyTypeFilter as (filterNode: TreeNode) => Promise, + premiumOptions: { + showBadgeForNonPremium: true, + blockFilterAction: promptForPremiumOnFilter + ? async () => await this.premiumUpgradePromptService.promptForPremium() + : undefined, + }, }; return archiveFilterSection; } diff --git a/apps/web/src/app/vault/individual-vault/vault-filter/shared/components/vault-filter-section.component.html b/apps/web/src/app/vault/individual-vault/vault-filter/shared/components/vault-filter-section.component.html index f7078d2a67a..66f14dcf2f6 100644 --- a/apps/web/src/app/vault/individual-vault/vault-filter/shared/components/vault-filter-section.component.html +++ b/apps/web/src/app/vault/individual-vault/vault-filter/shared/components/vault-filter-section.component.html @@ -105,6 +105,9 @@ *ngComponentOutlet="optionsInfo.component; injector: createInjector(f.node)" > + + +
    ) { + if (this.section?.premiumOptions?.blockFilterAction) { + await this.section.premiumOptions.blockFilterAction(); + return; + } + await this.section?.action(filterNode); } @@ -123,6 +128,10 @@ export class VaultFilterSectionComponent implements OnInit, OnDestroy { return this.section?.options; } + get premiumFeature() { + return this.section?.premiumOptions?.showBadgeForNonPremium; + } + get divider() { return this.section?.divider; } diff --git a/apps/web/src/app/vault/individual-vault/vault-filter/shared/models/vault-filter-section.type.ts b/apps/web/src/app/vault/individual-vault/vault-filter/shared/models/vault-filter-section.type.ts index f1e6222b57a..d275b1251e9 100644 --- a/apps/web/src/app/vault/individual-vault/vault-filter/shared/models/vault-filter-section.type.ts +++ b/apps/web/src/app/vault/individual-vault/vault-filter/shared/models/vault-filter-section.type.ts @@ -47,6 +47,16 @@ export type VaultFilterSection = { component: any; }; divider?: boolean; + premiumOptions?: { + /** When true, the premium badge will show on the filter for non-premium users. */ + showBadgeForNonPremium?: true; + /** + * Action to be called instead of applying the filter. + * Useful when the user does not have access to a filter (e.g., premium feature) + * and custom behavior is needed when invoking the filter. + */ + blockFilterAction?: () => Promise; + }; }; export type VaultFilterList = { diff --git a/apps/web/src/app/vault/individual-vault/vault-filter/shared/vault-filter-shared.module.ts b/apps/web/src/app/vault/individual-vault/vault-filter/shared/vault-filter-shared.module.ts index c8becac8ef5..190ace6db63 100644 --- a/apps/web/src/app/vault/individual-vault/vault-filter/shared/vault-filter-shared.module.ts +++ b/apps/web/src/app/vault/individual-vault/vault-filter/shared/vault-filter-shared.module.ts @@ -1,5 +1,6 @@ import { NgModule } from "@angular/core"; +import { PremiumBadgeComponent } from "@bitwarden/angular/billing/components/premium-badge"; import { SearchModule } from "@bitwarden/components"; import { SharedModule } from "../../../../shared"; @@ -7,7 +8,7 @@ import { SharedModule } from "../../../../shared"; import { VaultFilterSectionComponent } from "./components/vault-filter-section.component"; @NgModule({ - imports: [SharedModule, SearchModule], + imports: [SharedModule, SearchModule, PremiumBadgeComponent], declarations: [VaultFilterSectionComponent], exports: [SharedModule, VaultFilterSectionComponent, SearchModule], }) diff --git a/apps/web/src/app/vault/individual-vault/vault.component.html b/apps/web/src/app/vault/individual-vault/vault.component.html index 711a34413b5..522b63c21fd 100644 --- a/apps/web/src/app/vault/individual-vault/vault.component.html +++ b/apps/web/src/app/vault/individual-vault/vault.component.html @@ -34,6 +34,16 @@ {{ trashCleanupWarning }} + +

    {{ "premiumSubscriptionEndedDesc" | i18n }}

    + {{ + "restartPremium" | i18n + }} +     ; VaultItemsModule, SharedModule, OrganizationWarningsModule, + BannerComponent, ], providers: [ RoutedVaultFilterService, @@ -230,13 +231,6 @@ export class VaultComponent implements OnInit, OnDestr .pipe(map((a) => a?.id)) .pipe(switchMap((id) => this.organizationService.organizations$(id))); - protected userCanArchive$ = this.accountService.activeAccount$.pipe( - getUserId, - switchMap((userId) => { - return this.cipherArchiveService.userCanArchive$(userId); - }), - ); - emptyState$ = combineLatest([ this.currentSearchText$, this.routedVaultFilterService.filter$, @@ -295,14 +289,28 @@ export class VaultComponent implements OnInit, OnDestr }), ); - protected enforceOrgDataOwnershipPolicy$ = this.accountService.activeAccount$.pipe( - getUserId, + private userId$ = this.accountService.activeAccount$.pipe(getUserId); + + protected enforceOrgDataOwnershipPolicy$ = this.userId$.pipe( switchMap((userId) => this.policyService.policyAppliesToUser$(PolicyType.OrganizationDataOwnership, userId), ), ); - private userId$ = this.accountService.activeAccount$.pipe(getUserId); + protected userCanArchive$ = this.userId$.pipe( + switchMap((userId) => { + return this.cipherArchiveService.userCanArchive$(userId); + }), + ); + + protected showSubscriptionEndedMessaging$ = this.userId$.pipe( + switchMap((userId) => + combineLatest([ + this.routedVaultFilterBridgeService.activeFilter$, + this.cipherArchiveService.showSubscriptionEndedMessaging$(userId), + ]).pipe(map(([activeFilter, showMessaging]) => activeFilter.isArchived && showMessaging)), + ), + ); constructor( private syncService: SyncService, @@ -438,13 +446,13 @@ export class VaultComponent implements OnInit, OnDestr allowedCiphers$, filter$, this.currentSearchText$, - this.cipherArchiveService.hasArchiveFlagEnabled$(), + this.cipherArchiveService.hasArchiveFlagEnabled$, ]).pipe( filter(([ciphers, filter]) => ciphers != undefined && filter != undefined), - concatMap(async ([ciphers, filter, searchText, archiveEnabled]) => { + concatMap(async ([ciphers, filter, searchText, showArchiveVault]) => { const failedCiphers = (await firstValueFrom(this.cipherService.failedToDecryptCiphers$(activeUserId))) ?? []; - const filterFunction = createFilterFunction(filter, archiveEnabled); + const filterFunction = createFilterFunction(filter, showArchiveVault); // Append any failed to decrypt ciphers to the top of the cipher list const allCiphers = [...failedCiphers, ...ciphers]; diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index 19eec245885..4ed0ac639b0 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -3133,6 +3133,15 @@ } } }, + "premiumSubscriptionEnded": { + "message": "Your Premium subscription ended" + }, + "premiumSubscriptionEndedDesc": { + "message": "To regain access to your archive, restart your Premium subscription. If you edit details for an archived item before restarting, it'll be moved back into your vault." + }, + "restartPremium": { + "message": "Restart Premium" + }, "additionalStorageGb": { "message": "Additional storage (GB)" }, diff --git a/libs/angular/src/vault/vault-filter/components/vault-filter.component.ts b/libs/angular/src/vault/vault-filter/components/vault-filter.component.ts index 659db1bb925..f664cff2e8d 100644 --- a/libs/angular/src/vault/vault-filter/components/vault-filter.component.ts +++ b/libs/angular/src/vault/vault-filter/components/vault-filter.component.ts @@ -89,7 +89,7 @@ export class VaultFilterComponent implements OnInit { this.collections = await this.initCollections(); this.showArchiveVaultFilter = await firstValueFrom( - this.cipherArchiveService.hasArchiveFlagEnabled$(), + this.cipherArchiveService.hasArchiveFlagEnabled$, ); this.isLoaded = true; diff --git a/libs/common/src/vault/abstractions/cipher-archive.service.ts b/libs/common/src/vault/abstractions/cipher-archive.service.ts index d33fc5e7cc7..0969b7de1ac 100644 --- a/libs/common/src/vault/abstractions/cipher-archive.service.ts +++ b/libs/common/src/vault/abstractions/cipher-archive.service.ts @@ -4,10 +4,11 @@ import { CipherId, UserId } from "@bitwarden/common/types/guid"; import { CipherViewLike } from "@bitwarden/common/vault/utils/cipher-view-like-utils"; export abstract class CipherArchiveService { - abstract hasArchiveFlagEnabled$(): Observable; + abstract hasArchiveFlagEnabled$: Observable; abstract archivedCiphers$(userId: UserId): Observable; abstract userCanArchive$(userId: UserId): Observable; - abstract showArchiveVault$(userId: UserId): Observable; + abstract userHasPremium$(userId: UserId): Observable; abstract archiveWithServer(ids: CipherId | CipherId[], userId: UserId): Promise; abstract unarchiveWithServer(ids: CipherId | CipherId[], userId: UserId): Promise; + abstract showSubscriptionEndedMessaging$(userId: UserId): Observable; } diff --git a/libs/common/src/vault/services/default-cipher-archive.service.spec.ts b/libs/common/src/vault/services/default-cipher-archive.service.spec.ts index 972b04d2c4e..807311ca851 100644 --- a/libs/common/src/vault/services/default-cipher-archive.service.spec.ts +++ b/libs/common/src/vault/services/default-cipher-archive.service.spec.ts @@ -1,5 +1,5 @@ import { mock } from "jest-mock-extended"; -import { of, firstValueFrom } from "rxjs"; +import { of, firstValueFrom, BehaviorSubject } from "rxjs"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions"; @@ -24,12 +24,14 @@ describe("DefaultCipherArchiveService", () => { const userId = "user-id" as UserId; const cipherId = "123" as CipherId; + const featureFlag = new BehaviorSubject(true); beforeEach(() => { mockCipherService = mock(); mockApiService = mock(); mockBillingAccountProfileStateService = mock(); mockConfigService = mock(); + mockConfigService.getFeatureFlag$.mockReturnValue(featureFlag.asObservable()); service = new DefaultCipherArchiveService( mockCipherService, @@ -86,7 +88,7 @@ describe("DefaultCipherArchiveService", () => { describe("userCanArchive$", () => { it("should return true when user has premium and feature flag is enabled", async () => { mockBillingAccountProfileStateService.hasPremiumFromAnySource$.mockReturnValue(of(true)); - mockConfigService.getFeatureFlag$.mockReturnValue(of(true)); + featureFlag.next(true); const result = await firstValueFrom(service.userCanArchive$(userId)); @@ -101,7 +103,7 @@ describe("DefaultCipherArchiveService", () => { it("should return false when feature flag is disabled", async () => { mockBillingAccountProfileStateService.hasPremiumFromAnySource$.mockReturnValue(of(false)); - mockConfigService.getFeatureFlag$.mockReturnValue(of(false)); + featureFlag.next(false); const result = await firstValueFrom(service.userCanArchive$(userId)); @@ -109,6 +111,93 @@ describe("DefaultCipherArchiveService", () => { }); }); + describe("hasArchiveFlagEnabled$", () => { + it("returns true when feature flag is enabled", async () => { + featureFlag.next(true); + + const result = await firstValueFrom(service.hasArchiveFlagEnabled$); + + expect(result).toBe(true); + expect(mockConfigService.getFeatureFlag$).toHaveBeenCalledWith( + FeatureFlag.PM19148_InnovationArchive, + ); + }); + + it("returns false when feature flag is disabled", async () => { + featureFlag.next(false); + + const result = await firstValueFrom(service.hasArchiveFlagEnabled$); + + expect(result).toBe(false); + }); + }); + + describe("userHasPremium$", () => { + it("returns true when user has premium", async () => { + mockBillingAccountProfileStateService.hasPremiumFromAnySource$.mockReturnValue(of(true)); + + const result = await firstValueFrom(service.userHasPremium$(userId)); + + expect(result).toBe(true); + expect(mockBillingAccountProfileStateService.hasPremiumFromAnySource$).toHaveBeenCalledWith( + userId, + ); + }); + + it("returns false when user does not have premium", async () => { + mockBillingAccountProfileStateService.hasPremiumFromAnySource$.mockReturnValue(of(false)); + + const result = await firstValueFrom(service.userHasPremium$(userId)); + + expect(result).toBe(false); + }); + }); + + describe("showSubscriptionEndedMessaging$", () => { + it("returns true when user has archived ciphers but no premium", async () => { + const mockCiphers: CipherListView[] = [ + { + id: "1", + archivedDate: "2024-01-15T10:30:00.000Z", + type: "identity", + } as unknown as CipherListView, + ]; + + mockCipherService.cipherListViews$.mockReturnValue(of(mockCiphers)); + mockBillingAccountProfileStateService.hasPremiumFromAnySource$.mockReturnValue(of(false)); + + const result = await firstValueFrom(service.showSubscriptionEndedMessaging$(userId)); + + expect(result).toBe(true); + }); + + it("returns false when user has archived ciphers and has premium", async () => { + const mockCiphers: CipherListView[] = [ + { + id: "1", + archivedDate: "2024-01-15T10:30:00.000Z", + type: "identity", + } as unknown as CipherListView, + ]; + + mockCipherService.cipherListViews$.mockReturnValue(of(mockCiphers)); + mockBillingAccountProfileStateService.hasPremiumFromAnySource$.mockReturnValue(of(true)); + + const result = await firstValueFrom(service.showSubscriptionEndedMessaging$(userId)); + + expect(result).toBe(false); + }); + + it("returns false when user has no archived ciphers and no premium", async () => { + mockCipherService.cipherListViews$.mockReturnValue(of([])); + mockBillingAccountProfileStateService.hasPremiumFromAnySource$.mockReturnValue(of(false)); + + const result = await firstValueFrom(service.showSubscriptionEndedMessaging$(userId)); + + expect(result).toBe(false); + }); + }); + describe("archiveWithServer", () => { const mockResponse = { data: [ diff --git a/libs/common/src/vault/services/default-cipher-archive.service.ts b/libs/common/src/vault/services/default-cipher-archive.service.ts index a56a22474a3..8076735c9e2 100644 --- a/libs/common/src/vault/services/default-cipher-archive.service.ts +++ b/libs/common/src/vault/services/default-cipher-archive.service.ts @@ -27,10 +27,6 @@ export class DefaultCipherArchiveService implements CipherArchiveService { private configService: ConfigService, ) {} - hasArchiveFlagEnabled$(): Observable { - return this.configService.getFeatureFlag$(FeatureFlag.PM19148_InnovationArchive); - } - /** * Observable that contains the list of ciphers that have been archived. */ @@ -61,23 +57,22 @@ export class DefaultCipherArchiveService implements CipherArchiveService { ); } - /** - * User can access the archive vault if: - * Feature Flag is enabled - * There is at least one archived item - * ///////////// NOTE ///////////// - * This is separated from userCanArchive because a user that loses premium status, but has archived items, - * should still be able to access their archive vault. The items will be read-only, and can be restored. - */ - showArchiveVault$(userId: UserId): Observable { - return combineLatest([ - this.configService.getFeatureFlag$(FeatureFlag.PM19148_InnovationArchive), - this.archivedCiphers$(userId), - ]).pipe( - map( - ([archiveFlagEnabled, hasArchivedItems]) => - archiveFlagEnabled && hasArchivedItems.length > 0, - ), + /** Returns true when the archive features should be shown. */ + hasArchiveFlagEnabled$: Observable = this.configService + .getFeatureFlag$(FeatureFlag.PM19148_InnovationArchive) + .pipe(shareReplay({ refCount: true, bufferSize: 1 })); + + /** Returns true when the user has premium from any means. */ + userHasPremium$(userId: UserId): Observable { + return this.billingAccountProfileStateService + .hasPremiumFromAnySource$(userId) + .pipe(shareReplay({ refCount: true, bufferSize: 1 })); + } + + /** Returns true when the user has previously archived ciphers but lost their premium membership. */ + showSubscriptionEndedMessaging$(userId: UserId): Observable { + return combineLatest([this.archivedCiphers$(userId), this.userHasPremium$(userId)]).pipe( + map(([archivedCiphers, hasPremium]) => archivedCiphers.length > 0 && !hasPremium), shareReplay({ refCount: true, bufferSize: 1 }), ); }