From 414ee2563f250974f25aab8fad3446d1d3e12f0a Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Mon, 5 Feb 2024 13:02:28 -0500 Subject: [PATCH] [PM-5537] Biometric State Service (#7761) * Create state for biometric client key halves * Move enc string util to central utils * Provide biometric state through service * Use biometric state to track client key half * Create migration for client key half * Ensure client key half is removed on logout * Remove account data for client key half * Remove unnecessary key definition likes * Remove moved state from account * Fix null-conditional operator failure * Simplify migration * Remove lame test * Fix test type * Add migrator * Prefer userKey when legacy not needed * Fix tests --- apps/desktop/jest.config.js | 9 +- .../src/app/accounts/settings.component.ts | 18 +-- .../src/app/services/services.module.ts | 13 +- apps/desktop/src/auth/lock.component.spec.ts | 12 +- apps/desktop/src/auth/lock.component.ts | 6 +- apps/desktop/src/main.ts | 4 + apps/desktop/src/models/account.ts | 10 -- .../main/biometric/biometrics.service.spec.ts | 23 +++- .../main/biometric/biometrics.service.ts | 17 ++- .../services/electron-crypto.service.spec.ts | 57 ++++++-- .../services/electron-crypto.service.ts | 76 ++++++----- .../electron-state.service.abstraction.ts | 8 -- .../services/electron-state.service.ts | 44 ------- .../src/services/jslib-services.module.ts | 9 ++ libs/common/spec/utils.ts | 8 ++ .../biometric-state.service.spec.ts | 61 +++++++++ .../biometrics/biometric-state.service.ts | 75 +++++++++++ .../biometrics/biometric.state.spec.ts | 13 ++ .../platform/biometrics/biometric.state.ts | 17 +++ .../services/key-state/org-keys.state.spec.ts | 12 +- .../key-state/provider-keys.state.spec.ts | 11 +- .../src/platform/state/state-definitions.ts | 4 +- libs/common/src/state-migrations/migrate.ts | 7 +- ...client-key-half-state-to-providers.spec.ts | 123 ++++++++++++++++++ ...tric-client-key-half-state-to-providers.ts | 65 +++++++++ 25 files changed, 547 insertions(+), 155 deletions(-) create mode 100644 libs/common/src/platform/biometrics/biometric-state.service.spec.ts create mode 100644 libs/common/src/platform/biometrics/biometric-state.service.ts create mode 100644 libs/common/src/platform/biometrics/biometric.state.spec.ts create mode 100644 libs/common/src/platform/biometrics/biometric.state.ts create mode 100644 libs/common/src/state-migrations/migrations/14-move-biometric-client-key-half-state-to-providers.spec.ts create mode 100644 libs/common/src/state-migrations/migrations/14-move-biometric-client-key-half-state-to-providers.ts diff --git a/apps/desktop/jest.config.js b/apps/desktop/jest.config.js index cde02cd995..73f5ada287 100644 --- a/apps/desktop/jest.config.js +++ b/apps/desktop/jest.config.js @@ -9,7 +9,10 @@ module.exports = { ...sharedConfig, preset: "jest-preset-angular", setupFilesAfterEnv: ["/test.setup.ts"], - moduleNameMapper: pathsToModuleNameMapper(compilerOptions?.paths || {}, { - prefix: "/", - }), + moduleNameMapper: pathsToModuleNameMapper( + { "@bitwarden/common/spec": ["../../libs/common/spec"], ...(compilerOptions?.paths ?? {}) }, + { + prefix: "/", + }, + ), }; diff --git a/apps/desktop/src/app/accounts/settings.component.ts b/apps/desktop/src/app/accounts/settings.component.ts index 34c7f8d065..3b4bc82405 100644 --- a/apps/desktop/src/app/accounts/settings.component.ts +++ b/apps/desktop/src/app/accounts/settings.component.ts @@ -12,16 +12,17 @@ import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { UserVerificationService as UserVerificationServiceAbstraction } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; import { DeviceType } from "@bitwarden/common/enums"; import { VaultTimeoutAction } from "@bitwarden/common/enums/vault-timeout-action.enum"; -import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service"; import { ThemeType, KeySuffixOptions } from "@bitwarden/common/platform/enums"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { DialogService } from "@bitwarden/components"; import { SetPinComponent } from "../../auth/components/set-pin.component"; import { flagEnabled } from "../../platform/flags"; +import { ElectronCryptoService } from "../../platform/services/electron-crypto.service"; import { ElectronStateService } from "../../platform/services/electron-state.service.abstraction"; @Component({ selector: "app-settings", @@ -112,12 +113,13 @@ export class SettingsComponent implements OnInit { private vaultTimeoutSettingsService: VaultTimeoutSettingsService, private stateService: ElectronStateService, private messagingService: MessagingService, - private cryptoService: CryptoService, + private cryptoService: ElectronCryptoService, private modalService: ModalService, private themingService: AbstractThemingService, private settingsService: SettingsService, private dialogService: DialogService, private userVerificationService: UserVerificationServiceAbstraction, + private biometricStateService: BiometricStateService, ) { const isMac = this.platformUtilsService.getDevice() === DeviceType.MacOsDesktop; @@ -242,8 +244,9 @@ export class SettingsComponent implements OnInit { pin: this.userHasPinSet, biometric: await this.vaultTimeoutSettingsService.isBiometricLockSet(), autoPromptBiometrics: !(await this.stateService.getDisableAutoBiometricsPrompt()), - requirePasswordOnStart: - (await this.stateService.getBiometricRequirePasswordOnStart()) ?? false, + requirePasswordOnStart: await firstValueFrom( + this.biometricStateService.requirePasswordOnStart$, + ), approveLoginRequests: (await this.stateService.getApproveLoginRequests()) ?? false, clearClipboard: await this.stateService.getClearClipboard(), minimizeOnCopyToClipboard: await this.stateService.getMinimizeOnCopyToClipboard(), @@ -454,7 +457,7 @@ export class SettingsComponent implements OnInit { this.form.controls.requirePasswordOnStart.setValue(true); this.form.controls.autoPromptBiometrics.setValue(false); await this.stateService.setDisableAutoBiometricsPrompt(true); - await this.stateService.setBiometricRequirePasswordOnStart(true); + await this.cryptoService.setBiometricClientKeyHalf(); await this.stateService.setDismissedBiometricRequirePasswordOnStart(); } await this.cryptoService.refreshAdditionalKeys(); @@ -488,10 +491,9 @@ export class SettingsComponent implements OnInit { this.form.controls.autoPromptBiometrics.setValue(false); await this.updateAutoPromptBiometrics(); - await this.stateService.setBiometricRequirePasswordOnStart(true); + await this.cryptoService.setBiometricClientKeyHalf(); } else { - await this.stateService.setBiometricRequirePasswordOnStart(false); - await this.stateService.setBiometricEncryptionClientKeyHalf(null); + await this.cryptoService.removeBiometricClientKeyHalf(); } await this.stateService.setDismissedBiometricRequirePasswordOnStart(); await this.cryptoService.refreshAdditionalKeys(); diff --git a/apps/desktop/src/app/services/services.module.ts b/apps/desktop/src/app/services/services.module.ts index 175b6e5c8a..71c936ca59 100644 --- a/apps/desktop/src/app/services/services.module.ts +++ b/apps/desktop/src/app/services/services.module.ts @@ -34,6 +34,7 @@ import { PlatformUtilsService as PlatformUtilsServiceAbstraction } from "@bitwar import { StateService as StateServiceAbstraction } from "@bitwarden/common/platform/abstractions/state.service"; import { AbstractStorageService } from "@bitwarden/common/platform/abstractions/storage.service"; import { SystemService as SystemServiceAbstraction } from "@bitwarden/common/platform/abstractions/system.service"; +import { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service"; import { StateFactory } from "@bitwarden/common/platform/factories/state-factory"; import { GlobalState } from "@bitwarden/common/platform/models/domain/global-state"; import { MemoryStorageService } from "@bitwarden/common/platform/services/memory-storage.service"; @@ -47,7 +48,10 @@ import { DialogService } from "@bitwarden/components"; import { LoginGuard } from "../../auth/guards/login.guard"; import { Account } from "../../models/account"; -import { ElectronCryptoService } from "../../platform/services/electron-crypto.service"; +import { + DefaultElectronCryptoService, + ElectronCryptoService, +} from "../../platform/services/electron-crypto.service"; import { ElectronLogService } from "../../platform/services/electron-log.service"; import { ElectronPlatformUtilsService } from "../../platform/services/electron-platform-utils.service"; import { ElectronRendererMessagingService } from "../../platform/services/electron-renderer-messaging.service"; @@ -178,7 +182,11 @@ const RELOAD_CALLBACK = new InjectionToken<() => any>("RELOAD_CALLBACK"); }, { provide: CryptoServiceAbstraction, - useClass: ElectronCryptoService, + useExisting: ElectronCryptoService, + }, + { + provide: ElectronCryptoService, + useClass: DefaultElectronCryptoService, deps: [ CryptoFunctionServiceAbstraction, EncryptService, @@ -187,6 +195,7 @@ const RELOAD_CALLBACK = new InjectionToken<() => any>("RELOAD_CALLBACK"); StateServiceAbstraction, AccountServiceAbstraction, StateProvider, + BiometricStateService, ], }, ], diff --git a/apps/desktop/src/auth/lock.component.spec.ts b/apps/desktop/src/auth/lock.component.spec.ts index c36e75a378..6b2d6c01f6 100644 --- a/apps/desktop/src/auth/lock.component.spec.ts +++ b/apps/desktop/src/auth/lock.component.spec.ts @@ -21,9 +21,11 @@ import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.servic 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 { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service"; import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength"; import { DialogService } from "@bitwarden/components"; +import { ElectronCryptoService } from "../platform/services/electron-crypto.service"; import { ElectronStateService } from "../platform/services/electron-state.service.abstraction"; import { LockComponent } from "./lock.component"; @@ -78,7 +80,11 @@ describe("LockComponent", () => { }, { provide: CryptoService, - useValue: mock(), + useExisting: ElectronCryptoService, + }, + { + provide: ElectronCryptoService, + useValue: mock(), }, { provide: VaultTimeoutService, @@ -140,6 +146,10 @@ describe("LockComponent", () => { provide: PinCryptoServiceAbstraction, useValue: mock(), }, + { + provide: BiometricStateService, + useValue: mock(), + }, ], schemas: [NO_ERRORS_SCHEMA], }).compileComponents(); diff --git a/apps/desktop/src/auth/lock.component.ts b/apps/desktop/src/auth/lock.component.ts index 23485f143b..2c0e093f6a 100644 --- a/apps/desktop/src/auth/lock.component.ts +++ b/apps/desktop/src/auth/lock.component.ts @@ -13,7 +13,6 @@ import { DeviceTrustCryptoServiceAbstraction } from "@bitwarden/common/auth/abst import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; import { DeviceType } from "@bitwarden/common/enums"; import { BroadcasterService } from "@bitwarden/common/platform/abstractions/broadcaster.service"; -import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; @@ -22,6 +21,7 @@ import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/pl import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength"; import { DialogService } from "@bitwarden/components"; +import { ElectronCryptoService } from "../platform/services/electron-crypto.service"; import { ElectronStateService } from "../platform/services/electron-state.service.abstraction"; const BroadcasterSubscriptionId = "LockComponent"; @@ -41,7 +41,7 @@ export class LockComponent extends BaseLockComponent { i18nService: I18nService, platformUtilsService: PlatformUtilsService, messagingService: MessagingService, - cryptoService: CryptoService, + protected override cryptoService: ElectronCryptoService, vaultTimeoutService: VaultTimeoutService, vaultTimeoutSettingsService: VaultTimeoutSettingsService, environmentService: EnvironmentService, @@ -175,8 +175,8 @@ export class LockComponent extends BaseLockComponent { type: "warning", }); - await this.stateService.setBiometricRequirePasswordOnStart(response); if (response) { + await this.cryptoService.setBiometricClientKeyHalf(); await this.stateService.setDisableAutoBiometricsPrompt(true); } this.supportsBiometric = await this.canUseBiometric(); diff --git a/apps/desktop/src/main.ts b/apps/desktop/src/main.ts index af03f5ba98..8c5304c987 100644 --- a/apps/desktop/src/main.ts +++ b/apps/desktop/src/main.ts @@ -3,6 +3,7 @@ import * as path from "path"; import { app } from "electron"; import { AccountServiceImplementation } from "@bitwarden/common/auth/services/account.service"; +import { DefaultBiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service"; import { StateFactory } from "@bitwarden/common/platform/factories/state-factory"; import { GlobalState } from "@bitwarden/common/platform/models/domain/global-state"; import { EnvironmentService } from "@bitwarden/common/platform/services/environment.service"; @@ -159,6 +160,8 @@ export class Main { this.updaterMain, ); + const biometricStateService = new DefaultBiometricStateService(stateProvider); + this.biometricsService = new BiometricsService( this.i18nService, this.windowMain, @@ -166,6 +169,7 @@ export class Main { this.logService, this.messagingService, process.platform, + biometricStateService, ); this.desktopCredentialStorageListener = new DesktopCredentialStorageListener( diff --git a/apps/desktop/src/models/account.ts b/apps/desktop/src/models/account.ts index 65140975af..0291fdeb28 100644 --- a/apps/desktop/src/models/account.ts +++ b/apps/desktop/src/models/account.ts @@ -1,25 +1,15 @@ -import { Jsonify } from "type-fest"; - import { Account as BaseAccount, AccountSettings as BaseAccountSettings, - AccountKeys as BaseAccountKeys, } from "@bitwarden/common/platform/models/domain/account"; -import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; export class AccountSettings extends BaseAccountSettings { vaultTimeout = -1; // On Restart - requirePasswordOnStart?: boolean; dismissedBiometricRequirePasswordOnStartCallout?: boolean; } -export class AccountKeys extends BaseAccountKeys { - biometricEncryptionClientKeyHalf?: Jsonify; -} - export class Account extends BaseAccount { settings?: AccountSettings = new AccountSettings(); - keys?: AccountKeys = new AccountKeys(); constructor(init: Partial) { super(init); diff --git a/apps/desktop/src/platform/main/biometric/biometrics.service.spec.ts b/apps/desktop/src/platform/main/biometric/biometrics.service.spec.ts index 81dfd5a1b2..5b81ff9986 100644 --- a/apps/desktop/src/platform/main/biometric/biometrics.service.spec.ts +++ b/apps/desktop/src/platform/main/biometric/biometrics.service.spec.ts @@ -3,6 +3,8 @@ import { mock, MockProxy } from "jest-mock-extended"; 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 { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service"; +import { UserId } from "@bitwarden/common/types/guid"; import { WindowMain } from "../../../main/window.main"; import { ElectronStateService } from "../../services/electron-state.service.abstraction"; @@ -25,8 +27,10 @@ describe("biometrics tests", function () { const stateService = mock(); const logService = mock(); const messagingService = mock(); + const biometricStateService = mock(); it("Should call the platformspecific methods", async () => { + const userId = "userId-1" as UserId; const sut = new BiometricsService( i18nService, windowMain, @@ -34,6 +38,7 @@ describe("biometrics tests", function () { logService, messagingService, process.platform, + biometricStateService, ); const mockService = mock(); @@ -46,7 +51,7 @@ describe("biometrics tests", function () { sut.setEncryptionKeyHalf({ service: "test", key: "test", value: "test" }); expect(mockService.init).toBeCalled(); - await sut.canAuthBiometric({ service: "test", key: "test", userId: "test" }); + await sut.canAuthBiometric({ service: "test", key: "test", userId }); expect(mockService.osSupportsBiometric).toBeCalled(); // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. @@ -64,6 +69,7 @@ describe("biometrics tests", function () { logService, messagingService, "win32", + biometricStateService, ); const internalService = (sut as any).platformSpecificService; @@ -79,6 +85,7 @@ describe("biometrics tests", function () { logService, messagingService, "darwin", + biometricStateService, ); const internalService = (sut as any).platformSpecificService; expect(internalService).not.toBeNull(); @@ -89,6 +96,7 @@ describe("biometrics tests", function () { describe("can auth biometric", () => { let sut: BiometricsService; let innerService: MockProxy; + const userId = "userId-1" as UserId; beforeEach(() => { sut = new BiometricsService( @@ -98,6 +106,7 @@ describe("biometrics tests", function () { logService, messagingService, process.platform, + biometricStateService, ); innerService = mock(); @@ -108,9 +117,9 @@ describe("biometrics tests", function () { }); it("should return false if client key half is required and not provided", async () => { - stateService.getBiometricRequirePasswordOnStart.mockResolvedValue(true); + biometricStateService.getRequirePasswordOnStart.mockResolvedValue(true); - const result = await sut.canAuthBiometric({ service: "test", key: "test", userId: "test" }); + const result = await sut.canAuthBiometric({ service: "test", key: "test", userId }); expect(result).toBe(false); }); @@ -121,18 +130,18 @@ describe("biometrics tests", function () { sut.setEncryptionKeyHalf({ service: "test", key: "test", value: "test" }); expect(innerService.init).toBeCalled(); - await sut.canAuthBiometric({ service: "test", key: "test", userId: "test" }); + await sut.canAuthBiometric({ service: "test", key: "test", userId }); expect(innerService.osSupportsBiometric).toBeCalled(); }); it("should call osSupportBiometric if client key half is not required", async () => { - stateService.getBiometricRequirePasswordOnStart.mockResolvedValue(false); + biometricStateService.getRequirePasswordOnStart.mockResolvedValue(false); innerService.osSupportsBiometric.mockResolvedValue(true); - const result = await sut.canAuthBiometric({ service: "test", key: "test", userId: "test" }); + const result = await sut.canAuthBiometric({ service: "test", key: "test", userId }); expect(result).toBe(true); - expect(innerService.osSupportsBiometric).toBeCalled(); + expect(innerService.osSupportsBiometric).toHaveBeenCalled(); }); }); }); diff --git a/apps/desktop/src/platform/main/biometric/biometrics.service.ts b/apps/desktop/src/platform/main/biometric/biometrics.service.ts index 32d4707f59..dadd4713dc 100644 --- a/apps/desktop/src/platform/main/biometric/biometrics.service.ts +++ b/apps/desktop/src/platform/main/biometric/biometrics.service.ts @@ -1,6 +1,8 @@ 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 { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service"; +import { UserId } from "@bitwarden/common/types/guid"; import { WindowMain } from "../../../main/window.main"; import { ElectronStateService } from "../../services/electron-state.service.abstraction"; @@ -18,6 +20,7 @@ export class BiometricsService implements BiometricsServiceAbstraction { private logService: LogService, private messagingService: MessagingService, private platform: NodeJS.Platform, + private biometricStateService: BiometricStateService, ) { this.loadPlatformSpecificService(this.platform); } @@ -70,11 +73,9 @@ export class BiometricsService implements BiometricsServiceAbstraction { }: { service: string; key: string; - userId: string; + userId: UserId; }): Promise { - const requireClientKeyHalf = await this.stateService.getBiometricRequirePasswordOnStart({ - userId, - }); + const requireClientKeyHalf = await this.biometricStateService.getRequirePasswordOnStart(userId); const clientKeyHalfB64 = this.getClientKeyHalf(service, key); const clientKeyHalfSatisfied = !requireClientKeyHalf || !!clientKeyHalfB64; return clientKeyHalfSatisfied && (await this.osSupportsBiometric()); @@ -171,7 +172,13 @@ export class BiometricsService implements BiometricsServiceAbstraction { } private async enforceClientKeyHalf(service: string, storageKey: string): Promise { - const requireClientKeyHalf = await this.stateService.getBiometricRequirePasswordOnStart(); + // The first half of the storageKey is the userId, separated by `_` + // We need to extract from the service because the active user isn't properly synced to the main process, + // So we can't use the observables on `biometricStateService` + const [userId] = storageKey.split("_"); + const requireClientKeyHalf = await this.biometricStateService.getRequirePasswordOnStart( + userId as UserId, + ); const clientKeyHalfB64 = this.getClientKeyHalf(service, storageKey); if (requireClientKeyHalf && !clientKeyHalfB64) { diff --git a/apps/desktop/src/platform/services/electron-crypto.service.spec.ts b/apps/desktop/src/platform/services/electron-crypto.service.spec.ts index 5398fa31b4..50c1f2101a 100644 --- a/apps/desktop/src/platform/services/electron-crypto.service.spec.ts +++ b/apps/desktop/src/platform/services/electron-crypto.service.spec.ts @@ -5,7 +5,10 @@ import { CryptoFunctionService } from "@bitwarden/common/platform/abstractions/c import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service"; +import { Utils } from "@bitwarden/common/platform/misc/utils"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; +import { makeEncString, makeStaticByteArray } from "@bitwarden/common/spec"; import { CsprngArray } from "@bitwarden/common/types/csprng"; import { UserId } from "@bitwarden/common/types/guid"; import { UserKey } from "@bitwarden/common/types/key"; @@ -15,11 +18,11 @@ import { mockAccountServiceWith, } from "../../../../../libs/common/spec/fake-account-service"; -import { ElectronCryptoService } from "./electron-crypto.service"; +import { DefaultElectronCryptoService } from "./electron-crypto.service"; import { ElectronStateService } from "./electron-state.service.abstraction"; describe("electronCryptoService", () => { - let electronCryptoService: ElectronCryptoService; + let sut: DefaultElectronCryptoService; const cryptoFunctionService = mock(); const encryptService = mock(); @@ -28,6 +31,7 @@ describe("electronCryptoService", () => { const stateService = mock(); let accountService: FakeAccountService; let stateProvider: FakeStateProvider; + const biometricStateService = mock(); const mockUserId = "mock user id" as UserId; @@ -35,7 +39,7 @@ describe("electronCryptoService", () => { accountService = mockAccountServiceWith("userId" as UserId); stateProvider = new FakeStateProvider(accountService); - electronCryptoService = new ElectronCryptoService( + sut = new DefaultElectronCryptoService( cryptoFunctionService, encryptService, platformUtilService, @@ -43,6 +47,7 @@ describe("electronCryptoService", () => { stateService, accountService, stateProvider, + biometricStateService, ); }); @@ -50,8 +55,42 @@ describe("electronCryptoService", () => { jest.resetAllMocks(); }); - it("instantiates", () => { - expect(electronCryptoService).not.toBeFalsy(); + describe("setBiometricClientKeyHalf", () => { + const userKey = new SymmetricCryptoKey(makeStaticByteArray(64, 1)) as UserKey; + const keyBytes = makeStaticByteArray(32, 2) as CsprngArray; + const encKeyHalf = makeEncString(Utils.fromBufferToUtf8(keyBytes)); + + beforeEach(() => { + sut.getUserKey = jest.fn().mockResolvedValue(userKey); + cryptoFunctionService.randomBytes.mockResolvedValue(keyBytes); + encryptService.encrypt.mockResolvedValue(encKeyHalf); + }); + + it("sets a biometric client key half for the currently active user", async () => { + await sut.setBiometricClientKeyHalf(); + + expect(biometricStateService.setEncryptedClientKeyHalf).toHaveBeenCalledWith(encKeyHalf); + }); + + it("should create the key from csprng bytes", async () => { + await sut.setBiometricClientKeyHalf(); + + expect(cryptoFunctionService.randomBytes).toHaveBeenCalledWith(32); + }); + + it("should encrypt the key half with the user key", async () => { + await sut.setBiometricClientKeyHalf(); + + expect(encryptService.encrypt).toHaveBeenCalledWith(expect.any(String), userKey); + }); + }); + + describe("removeBiometricClientKeyHalf", () => { + it("removes the biometric client key half for the currently active user", async () => { + await sut.removeBiometricClientKeyHalf(); + + expect(biometricStateService.setEncryptedClientKeyHalf).toHaveBeenCalledWith(null); + }); }); describe("setUserKey", () => { @@ -66,9 +105,9 @@ describe("electronCryptoService", () => { it("sets an Biometric key if getBiometricUnlock is true and the platform supports secure storage", async () => { stateService.getBiometricUnlock.mockResolvedValue(true); platformUtilService.supportsSecureStorage.mockReturnValue(true); - stateService.getBiometricRequirePasswordOnStart.mockResolvedValue(false); + biometricStateService.getRequirePasswordOnStart.mockResolvedValue(true); - await electronCryptoService.setUserKey(mockUserKey, mockUserId); + await sut.setUserKey(mockUserKey, mockUserId); expect(stateService.setUserKeyBiometric).toHaveBeenCalledWith( expect.objectContaining({ key: expect.any(String), clientEncKeyHalf: null }), @@ -82,7 +121,7 @@ describe("electronCryptoService", () => { stateService.getBiometricUnlock.mockResolvedValue(true); platformUtilService.supportsSecureStorage.mockReturnValue(false); - await electronCryptoService.setUserKey(mockUserKey, mockUserId); + await sut.setUserKey(mockUserKey, mockUserId); expect(stateService.setUserKeyBiometric).toHaveBeenCalledWith(null, { userId: mockUserId, @@ -90,7 +129,7 @@ describe("electronCryptoService", () => { }); it("clears the old deprecated Biometric key whenever a User Key is set", async () => { - await electronCryptoService.setUserKey(mockUserKey, mockUserId); + await sut.setUserKey(mockUserKey, mockUserId); expect(stateService.setCryptoMasterKeyBiometric).toHaveBeenCalledWith(null, { userId: mockUserId, diff --git a/apps/desktop/src/platform/services/electron-crypto.service.ts b/apps/desktop/src/platform/services/electron-crypto.service.ts index f5d503828a..9fa64e231b 100644 --- a/apps/desktop/src/platform/services/electron-crypto.service.ts +++ b/apps/desktop/src/platform/services/electron-crypto.service.ts @@ -1,8 +1,11 @@ +import { firstValueFrom } from "rxjs"; + import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { CryptoFunctionService } from "@bitwarden/common/platform/abstractions/crypto-function.service"; import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service"; import { KeySuffixOptions } from "@bitwarden/common/platform/enums"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; @@ -15,7 +18,18 @@ import { UserKey, MasterKey } from "@bitwarden/common/types/key"; import { ElectronStateService } from "./electron-state.service.abstraction"; -export class ElectronCryptoService extends CryptoService { +export abstract class ElectronCryptoService extends CryptoService { + /** + * Creates and sets a new biometric client key half for the currently active user. + */ + abstract setBiometricClientKeyHalf(): Promise; + /** + * Removes the biometric client key half for the currently active user. + */ + abstract removeBiometricClientKeyHalf(): Promise; +} + +export class DefaultElectronCryptoService extends ElectronCryptoService { constructor( cryptoFunctionService: CryptoFunctionService, encryptService: EncryptService, @@ -24,6 +38,7 @@ export class ElectronCryptoService extends CryptoService { protected override stateService: ElectronStateService, accountService: AccountService, stateProvider: StateProvider, + private biometricStateService: BiometricStateService, ) { super( cryptoFunctionService, @@ -47,17 +62,27 @@ export class ElectronCryptoService extends CryptoService { override async clearStoredUserKey(keySuffix: KeySuffixOptions, userId?: UserId): Promise { if (keySuffix === KeySuffixOptions.Biometric) { - // 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.stateService.setUserKeyBiometric(null, { userId: userId }); - // 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.clearDeprecatedKeys(KeySuffixOptions.Biometric, userId); + await this.stateService.setUserKeyBiometric(null, { userId: userId }); + await this.biometricStateService.removeEncryptedClientKeyHalf(userId); + await this.clearDeprecatedKeys(KeySuffixOptions.Biometric, userId); 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 - super.clearStoredUserKey(keySuffix, userId); + await super.clearStoredUserKey(keySuffix, userId); + } + + async setBiometricClientKeyHalf(): Promise { + const userKey = await this.getUserKey(); + const keyBytes = await this.cryptoFunctionService.randomBytes(32); + const biometricKey = Utils.fromBufferToUtf8(keyBytes) as CsprngString; + const encKey = await this.encryptService.encrypt(biometricKey, userKey); + + await this.biometricStateService.setEncryptedClientKeyHalf(encKey); + } + + async removeBiometricClientKeyHalf(): Promise { + await this.biometricStateService.setEncryptedClientKeyHalf(null); } protected override async storeAdditionalKeys(key: UserKey, userId?: UserId) { @@ -86,10 +111,8 @@ export class ElectronCryptoService extends CryptoService { } protected async storeBiometricKey(key: UserKey, userId?: UserId): Promise { - let clientEncKeyHalf: CsprngString = null; - if (await this.stateService.getBiometricRequirePasswordOnStart({ userId })) { - clientEncKeyHalf = await this.getBiometricEncryptionClientKeyHalf(userId); - } + // May resolve to null, in which case no client key have is required + const clientEncKeyHalf = await this.getBiometricEncryptionClientKeyHalf(userId); await this.stateService.setUserKeyBiometric( { key: key.keyB64, clientEncKeyHalf }, { userId: userId }, @@ -105,30 +128,21 @@ export class ElectronCryptoService extends CryptoService { } protected override async clearAllStoredUserKeys(userId?: UserId): Promise { - await this.stateService.setUserKeyBiometric(null, { userId: userId }); - // 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 - super.clearAllStoredUserKeys(userId); + await this.clearStoredUserKey(KeySuffixOptions.Biometric, userId); + await super.clearAllStoredUserKeys(userId); } private async getBiometricEncryptionClientKeyHalf(userId?: UserId): Promise { - try { - let biometricKey = await this.stateService - .getBiometricEncryptionClientKeyHalf({ userId }) - .then((result) => result?.decrypt(null /* user encrypted */)) - .then((result) => result as CsprngString); - const userKey = await this.getUserKeyWithLegacySupport(); - if (biometricKey == null && userKey != null) { - const keyBytes = await this.cryptoFunctionService.randomBytes(32); - biometricKey = Utils.fromBufferToUtf8(keyBytes) as CsprngString; - const encKey = await this.encryptService.encrypt(biometricKey, userKey); - await this.stateService.setBiometricEncryptionClientKeyHalf(encKey); - } - - return biometricKey; - } catch { + const encryptedKeyHalfPromise = + userId == null + ? firstValueFrom(this.biometricStateService.encryptedClientKeyHalf$) + : this.biometricStateService.getEncryptedClientKeyHalf(userId); + const encryptedKeyHalf = await encryptedKeyHalfPromise; + if (encryptedKeyHalf == null) { return null; } + const userKey = await this.getUserKey(); + return (await this.encryptService.decryptToUtf8(encryptedKeyHalf, userKey)) as CsprngString; } // --LEGACY METHODS-- diff --git a/apps/desktop/src/platform/services/electron-state.service.abstraction.ts b/apps/desktop/src/platform/services/electron-state.service.abstraction.ts index 502fa441d8..26b1a25f57 100644 --- a/apps/desktop/src/platform/services/electron-state.service.abstraction.ts +++ b/apps/desktop/src/platform/services/electron-state.service.abstraction.ts @@ -1,17 +1,9 @@ import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; -import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; import { StorageOptions } from "@bitwarden/common/platform/models/domain/storage-options"; import { Account } from "../../models/account"; export abstract class ElectronStateService extends StateService { - getBiometricEncryptionClientKeyHalf: (options?: StorageOptions) => Promise; - setBiometricEncryptionClientKeyHalf: ( - value: EncString, - options?: StorageOptions, - ) => Promise; getDismissedBiometricRequirePasswordOnStart: (options?: StorageOptions) => Promise; setDismissedBiometricRequirePasswordOnStart: (options?: StorageOptions) => Promise; - getBiometricRequirePasswordOnStart: (options?: StorageOptions) => Promise; - setBiometricRequirePasswordOnStart: (value: boolean, options?: StorageOptions) => Promise; } diff --git a/apps/desktop/src/platform/services/electron-state.service.ts b/apps/desktop/src/platform/services/electron-state.service.ts index 3c19be1b1e..85182c02da 100644 --- a/apps/desktop/src/platform/services/electron-state.service.ts +++ b/apps/desktop/src/platform/services/electron-state.service.ts @@ -1,5 +1,4 @@ import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; import { GlobalState } from "@bitwarden/common/platform/models/domain/global-state"; import { StorageOptions } from "@bitwarden/common/platform/models/domain/storage-options"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; @@ -24,49 +23,6 @@ export class ElectronStateService await super.addAccount(account); } - async getBiometricEncryptionClientKeyHalf(options?: StorageOptions): Promise { - const account = await this.getAccount( - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - const key = account?.keys?.biometricEncryptionClientKeyHalf; - return key == null ? null : new EncString(key); - } - - async setBiometricEncryptionClientKeyHalf( - value: EncString, - options?: StorageOptions, - ): Promise { - const account = await this.getAccount( - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - account.keys.biometricEncryptionClientKeyHalf = value?.encryptedString; - await this.saveAccount( - account, - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - } - - async getBiometricRequirePasswordOnStart(options?: StorageOptions): Promise { - const account = await this.getAccount( - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - return account?.settings?.requirePasswordOnStart; - } - - async setBiometricRequirePasswordOnStart( - value: boolean, - options?: StorageOptions, - ): Promise { - const account = await this.getAccount( - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - account.settings.requirePasswordOnStart = value; - await this.saveAccount( - account, - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - } - async getDismissedBiometricRequirePasswordOnStart(options?: StorageOptions): Promise { const account = await this.getAccount( this.reconcileOptions(options, await this.defaultOnDiskOptions()), diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 75a39a8aae..b05bc95635 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -95,6 +95,10 @@ import { PlatformUtilsService as PlatformUtilsServiceAbstraction } from "@bitwar import { StateService as StateServiceAbstraction } from "@bitwarden/common/platform/abstractions/state.service"; import { AbstractStorageService } from "@bitwarden/common/platform/abstractions/storage.service"; import { ValidationService as ValidationServiceAbstraction } from "@bitwarden/common/platform/abstractions/validation.service"; +import { + BiometricStateService, + DefaultBiometricStateService, +} from "@bitwarden/common/platform/biometrics/biometric-state.service"; import { StateFactory } from "@bitwarden/common/platform/factories/state-factory"; import { devFlagEnabled, flagEnabled } from "@bitwarden/common/platform/misc/flags"; import { Account } from "@bitwarden/common/platform/models/domain/account"; @@ -876,6 +880,11 @@ import { ModalService } from "./modal.service"; OrganizationApiServiceAbstraction, ], }, + { + provide: BiometricStateService, + useClass: DefaultBiometricStateService, + deps: [StateProvider], + }, ], }) export class JslibServicesModule {} diff --git a/libs/common/spec/utils.ts b/libs/common/spec/utils.ts index ad5907f61d..93142d2571 100644 --- a/libs/common/spec/utils.ts +++ b/libs/common/spec/utils.ts @@ -3,6 +3,9 @@ import { Observable } from "rxjs"; import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; +import { EncryptionType } from "../src/platform/enums"; +import { Utils } from "../src/platform/misc/utils"; + function newGuid() { return "xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx".replace(/[xy]/g, (c) => { const r = (Math.random() * 16) | 0; @@ -29,6 +32,11 @@ export function mockEnc(s: string): MockProxy { return mocked; } +export function makeEncString(data?: string) { + data ??= Utils.newGuid(); + return new EncString(EncryptionType.AesCbc256_HmacSha256_B64, data, "test", "test"); +} + export function makeStaticByteArray(length: number, start = 0) { const arr = new Uint8Array(length); for (let i = 0; i < length; i++) { diff --git a/libs/common/src/platform/biometrics/biometric-state.service.spec.ts b/libs/common/src/platform/biometrics/biometric-state.service.spec.ts new file mode 100644 index 0000000000..6c33d1be33 --- /dev/null +++ b/libs/common/src/platform/biometrics/biometric-state.service.spec.ts @@ -0,0 +1,61 @@ +import { firstValueFrom } from "rxjs"; + +import { makeEncString } from "../../../spec"; +import { mockAccountServiceWith } from "../../../spec/fake-account-service"; +import { FakeStateProvider } from "../../../spec/fake-state-provider"; +import { UserId } from "../../types/guid"; + +import { BiometricStateService, DefaultBiometricStateService } from "./biometric-state.service"; +import { ENCRYPTED_CLIENT_KEY_HALF } from "./biometric.state"; + +describe("BiometricStateService", () => { + let sut: BiometricStateService; + const userId = "userId" as UserId; + const encClientKeyHalf = makeEncString(); + const encryptedClientKeyHalf = encClientKeyHalf.encryptedString; + const accountService = mockAccountServiceWith(userId); + let stateProvider: FakeStateProvider; + + beforeEach(() => { + stateProvider = new FakeStateProvider(accountService); + + sut = new DefaultBiometricStateService(stateProvider); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + describe("requirePasswordOnStart$", () => { + it("should be false when encryptedClientKeyHalf is undefined", async () => { + stateProvider.activeUser.getFake(ENCRYPTED_CLIENT_KEY_HALF).nextState(undefined); + expect(await firstValueFrom(sut.requirePasswordOnStart$)).toBe(false); + }); + + it("should be true when encryptedClientKeyHalf is defined", async () => { + stateProvider.activeUser.getFake(ENCRYPTED_CLIENT_KEY_HALF).nextState(encryptedClientKeyHalf); + expect(await firstValueFrom(sut.requirePasswordOnStart$)).toBe(true); + }); + }); + + describe("encryptedClientKeyHalf$", () => { + it("should track the encryptedClientKeyHalf state", async () => { + const state = stateProvider.activeUser.getFake(ENCRYPTED_CLIENT_KEY_HALF); + state.nextState(undefined); + + expect(await firstValueFrom(sut.encryptedClientKeyHalf$)).toBe(null); + + state.nextState(encryptedClientKeyHalf); + + expect(await firstValueFrom(sut.encryptedClientKeyHalf$)).toEqual(encClientKeyHalf); + }); + }); + + describe("setEncryptedClientKeyHalf", () => { + it("should update the encryptedClientKeyHalf$", async () => { + await sut.setEncryptedClientKeyHalf(encClientKeyHalf); + + expect(await firstValueFrom(sut.encryptedClientKeyHalf$)).toEqual(encClientKeyHalf); + }); + }); +}); diff --git a/libs/common/src/platform/biometrics/biometric-state.service.ts b/libs/common/src/platform/biometrics/biometric-state.service.ts new file mode 100644 index 0000000000..011e5cdc59 --- /dev/null +++ b/libs/common/src/platform/biometrics/biometric-state.service.ts @@ -0,0 +1,75 @@ +import { Observable, firstValueFrom, map } from "rxjs"; + +import { UserId } from "../../types/guid"; +import { EncryptedString, EncString } from "../models/domain/enc-string"; +import { ActiveUserState, StateProvider } from "../state"; + +import { ENCRYPTED_CLIENT_KEY_HALF } from "./biometric.state"; + +export abstract class BiometricStateService { + /** + * If the user has elected to require a password on first unlock of an application instance, this key will store the + * encrypted client key half used to unlock the vault. + * + * Tracks the currently active user + */ + encryptedClientKeyHalf$: Observable; + /** + * whether or not a password is required on first unlock after opening the application + * + * tracks the currently active user + */ + requirePasswordOnStart$: Observable; + + abstract setEncryptedClientKeyHalf(encryptedKeyHalf: EncString): Promise; + abstract getEncryptedClientKeyHalf(userId: UserId): Promise; + abstract getRequirePasswordOnStart(userId: UserId): Promise; + abstract removeEncryptedClientKeyHalf(userId: UserId): Promise; +} + +export class DefaultBiometricStateService implements BiometricStateService { + private encryptedClientKeyHalfState: ActiveUserState; + encryptedClientKeyHalf$: Observable; + requirePasswordOnStart$: Observable; + + constructor(private stateProvider: StateProvider) { + this.encryptedClientKeyHalfState = this.stateProvider.getActive(ENCRYPTED_CLIENT_KEY_HALF); + this.encryptedClientKeyHalf$ = this.encryptedClientKeyHalfState.state$.pipe( + map(encryptedClientKeyHalfToEncString), + ); + this.requirePasswordOnStart$ = this.encryptedClientKeyHalf$.pipe(map((keyHalf) => !!keyHalf)); + } + + async setEncryptedClientKeyHalf(encryptedKeyHalf: EncString): Promise { + await this.encryptedClientKeyHalfState.update(() => encryptedKeyHalf?.encryptedString ?? null); + } + + async removeEncryptedClientKeyHalf(userId: UserId): Promise { + await this.stateProvider.getUser(userId, ENCRYPTED_CLIENT_KEY_HALF).update(() => null); + } + + async getRequirePasswordOnStart(userId: UserId): Promise { + if (userId == null) { + return false; + } + return !!(await this.getEncryptedClientKeyHalf(userId)); + } + + async getEncryptedClientKeyHalf(userId: UserId): Promise { + return await firstValueFrom( + this.stateProvider + .getUser(userId, ENCRYPTED_CLIENT_KEY_HALF) + .state$.pipe(map(encryptedClientKeyHalfToEncString)), + ); + } + + async logout(userId: UserId): Promise { + await this.stateProvider.getUser(userId, ENCRYPTED_CLIENT_KEY_HALF).update(() => null); + } +} + +function encryptedClientKeyHalfToEncString( + encryptedKeyHalf: EncryptedString | undefined, +): EncString { + return encryptedKeyHalf == null ? null : new EncString(encryptedKeyHalf); +} diff --git a/libs/common/src/platform/biometrics/biometric.state.spec.ts b/libs/common/src/platform/biometrics/biometric.state.spec.ts new file mode 100644 index 0000000000..09c8875814 --- /dev/null +++ b/libs/common/src/platform/biometrics/biometric.state.spec.ts @@ -0,0 +1,13 @@ +import { ENCRYPTED_CLIENT_KEY_HALF } from "./biometric.state"; + +describe("encrypted client key half", () => { + const sut = ENCRYPTED_CLIENT_KEY_HALF; + + it("should deserialize encrypted client key half state", () => { + const encryptedClientKeyHalf = "encryptedClientKeyHalf"; + + const result = sut.deserializer(JSON.parse(JSON.stringify(encryptedClientKeyHalf))); + + expect(result).toEqual(encryptedClientKeyHalf); + }); +}); diff --git a/libs/common/src/platform/biometrics/biometric.state.ts b/libs/common/src/platform/biometrics/biometric.state.ts new file mode 100644 index 0000000000..90268d2110 --- /dev/null +++ b/libs/common/src/platform/biometrics/biometric.state.ts @@ -0,0 +1,17 @@ +import { EncryptedString } from "../models/domain/enc-string"; +import { KeyDefinition, BIOMETRIC_SETTINGS_DISK } from "../state"; + +/** + * If the user has elected to require a password on first unlock of an application instance, this key will store the + * encrypted client key half used to unlock the vault. + * + * For operating systems without application-level key storage, this key half is concatenated with a signature + * provided by the OS and used to encrypt the biometric key prior to storage. + */ +export const ENCRYPTED_CLIENT_KEY_HALF = new KeyDefinition( + BIOMETRIC_SETTINGS_DISK, + "clientKeyHalf", + { + deserializer: (obj) => obj, + }, +); diff --git a/libs/common/src/platform/services/key-state/org-keys.state.spec.ts b/libs/common/src/platform/services/key-state/org-keys.state.spec.ts index 62c5d8db1a..dd1e4a685e 100644 --- a/libs/common/src/platform/services/key-state/org-keys.state.spec.ts +++ b/libs/common/src/platform/services/key-state/org-keys.state.spec.ts @@ -1,22 +1,12 @@ import { mock } from "jest-mock-extended"; -import { makeStaticByteArray } from "../../../../spec"; -import { ProviderEncryptedOrganizationKey } from "../../../admin-console/models/domain/encrypted-organization-key"; +import { makeEncString, makeStaticByteArray } from "../../../../spec"; import { OrgKey } from "../../../types/key"; import { CryptoService } from "../../abstractions/crypto.service"; -import { EncryptionType } from "../../enums"; -import { Utils } from "../../misc/utils"; -import { EncString } from "../../models/domain/enc-string"; import { SymmetricCryptoKey } from "../../models/domain/symmetric-crypto-key"; import { USER_ENCRYPTED_ORGANIZATION_KEYS, USER_ORGANIZATION_KEYS } from "./org-keys.state"; -function makeEncString(data?: string) { - data ??= Utils.newGuid(); - return new EncString(EncryptionType.AesCbc256_HmacSha256_B64, data, "test", "test"); -} -ProviderEncryptedOrganizationKey; - describe("encrypted org keys", () => { const sut = USER_ENCRYPTED_ORGANIZATION_KEYS; diff --git a/libs/common/src/platform/services/key-state/provider-keys.state.spec.ts b/libs/common/src/platform/services/key-state/provider-keys.state.spec.ts index d4514a2838..78e61e0391 100644 --- a/libs/common/src/platform/services/key-state/provider-keys.state.spec.ts +++ b/libs/common/src/platform/services/key-state/provider-keys.state.spec.ts @@ -1,22 +1,15 @@ import { mock } from "jest-mock-extended"; -import { makeStaticByteArray } from "../../../../spec"; +import { makeEncString, makeStaticByteArray } from "../../../../spec"; import { ProviderId } from "../../../types/guid"; import { ProviderKey, UserPrivateKey } from "../../../types/key"; import { EncryptService } from "../../abstractions/encrypt.service"; -import { EncryptionType } from "../../enums"; -import { Utils } from "../../misc/utils"; -import { EncString, EncryptedString } from "../../models/domain/enc-string"; +import { EncryptedString } from "../../models/domain/enc-string"; import { SymmetricCryptoKey } from "../../models/domain/symmetric-crypto-key"; import { CryptoService } from "../crypto.service"; import { USER_ENCRYPTED_PROVIDER_KEYS, USER_PROVIDER_KEYS } from "./provider-keys.state"; -function makeEncString(data?: string) { - data ??= Utils.newGuid(); - return new EncString(EncryptionType.AesCbc256_HmacSha256_B64, data, "test", "test"); -} - describe("encrypted provider keys", () => { const sut = USER_ENCRYPTED_PROVIDER_KEYS; diff --git a/libs/common/src/platform/state/state-definitions.ts b/libs/common/src/platform/state/state-definitions.ts index dcee36727d..b9c6eb7458 100644 --- a/libs/common/src/platform/state/state-definitions.ts +++ b/libs/common/src/platform/state/state-definitions.ts @@ -22,14 +22,16 @@ export const ACCOUNT_MEMORY = new StateDefinition("account", "memory"); export const BILLING_BANNERS_DISK = new StateDefinition("billingBanners", "disk"); export const CRYPTO_DISK = new StateDefinition("crypto", "disk"); + export const ENVIRONMENT_DISK = new StateDefinition("environment", "disk"); export const GENERATOR_DISK = new StateDefinition("generator", "disk"); export const GENERATOR_MEMORY = new StateDefinition("generator", "memory"); +export const BIOMETRIC_SETTINGS_DISK = new StateDefinition("biometricSettings", "disk"); + // Admin Console export const ORGANIZATIONS_DISK = new StateDefinition("organizations", "disk"); export const POLICIES_DISK = new StateDefinition("policies", "disk"); export const POLICIES_MEMORY = new StateDefinition("policies", "memory"); export const PROVIDERS_DISK = new StateDefinition("providers", "disk"); -// diff --git a/libs/common/src/state-migrations/migrate.ts b/libs/common/src/state-migrations/migrate.ts index 8ea1a3c95b..170317a665 100644 --- a/libs/common/src/state-migrations/migrate.ts +++ b/libs/common/src/state-migrations/migrate.ts @@ -9,6 +9,7 @@ import { EverHadUserKeyMigrator } from "./migrations/10-move-ever-had-user-key-t import { OrganizationKeyMigrator } from "./migrations/11-move-org-keys-to-state-providers"; import { MoveEnvironmentStateToProviders } from "./migrations/12-move-environment-state-to-providers"; import { ProviderKeyMigrator } from "./migrations/13-move-provider-keys-to-state-providers"; +import { MoveBiometricClientKeyHalfToStateProviders } from "./migrations/14-move-biometric-client-key-half-state-to-providers"; import { FixPremiumMigrator } from "./migrations/3-fix-premium"; import { RemoveEverBeenUnlockedMigrator } from "./migrations/4-remove-ever-been-unlocked"; import { AddKeyTypeToOrgKeysMigrator } from "./migrations/5-add-key-type-to-org-keys"; @@ -19,7 +20,7 @@ import { MoveBrowserSettingsToGlobal } from "./migrations/9-move-browser-setting import { MinVersionMigrator } from "./migrations/min-version"; export const MIN_VERSION = 2; -export const CURRENT_VERSION = 13; +export const CURRENT_VERSION = 14; export type MinVersion = typeof MIN_VERSION; export async function migrate( @@ -36,7 +37,6 @@ export async function migrate( await storageService.save("stateVersion", CURRENT_VERSION); return; } - await MigrationBuilder.create() .with(MinVersionMigrator) .with(FixPremiumMigrator, 2, 3) @@ -49,7 +49,8 @@ export async function migrate( .with(EverHadUserKeyMigrator, 9, 10) .with(OrganizationKeyMigrator, 10, 11) .with(MoveEnvironmentStateToProviders, 11, 12) - .with(ProviderKeyMigrator, 12, CURRENT_VERSION) + .with(ProviderKeyMigrator, 12, 13) + .with(MoveBiometricClientKeyHalfToStateProviders, 13, CURRENT_VERSION) .migrate(migrationHelper); } diff --git a/libs/common/src/state-migrations/migrations/14-move-biometric-client-key-half-state-to-providers.spec.ts b/libs/common/src/state-migrations/migrations/14-move-biometric-client-key-half-state-to-providers.spec.ts new file mode 100644 index 0000000000..d3967fbe2f --- /dev/null +++ b/libs/common/src/state-migrations/migrations/14-move-biometric-client-key-half-state-to-providers.spec.ts @@ -0,0 +1,123 @@ +import { MockProxy, any } from "jest-mock-extended"; + +import { MigrationHelper } from "../migration-helper"; +import { mockMigrationHelper } from "../migration-helper.spec"; + +import { + MoveBiometricClientKeyHalfToStateProviders, + CLIENT_KEY_HALF, +} from "./14-move-biometric-client-key-half-state-to-providers"; + +function exampleJSON() { + return { + global: { + otherStuff: "otherStuff1", + }, + authenticatedAccounts: ["user-1", "user-2", "user-3"], + "user-1": { + keys: { + biometricEncryptionClientKeyHalf: "user1-key-half", + otherStuff: "overStuff2", + }, + otherStuff: "otherStuff3", + }, + "user-2": { + keys: { + otherStuff: "otherStuff4", + }, + otherStuff: "otherStuff5", + }, + }; +} + +function rollbackJSON() { + return { + "user_user-1_biometricSettings_clientKeyHalf": "user1-key-half", + global: { + otherStuff: "otherStuff1", + }, + authenticatedAccounts: ["user-1", "user-2", "user-3"], + "user-1": { + keys: { + otherStuff: "overStuff2", + }, + otherStuff: "otherStuff3", + }, + "user-2": { + keys: { + otherStuff: "otherStuff4", + }, + otherStuff: "otherStuff5", + }, + }; +} + +describe("DesktopBiometricState migrator", () => { + let helper: MockProxy; + let sut: MoveBiometricClientKeyHalfToStateProviders; + + describe("migrate", () => { + beforeEach(() => { + helper = mockMigrationHelper(exampleJSON(), 13); + sut = new MoveBiometricClientKeyHalfToStateProviders(13, 14); + }); + + it("should remove biometricEncryptionClientKeyHalf from all accounts", async () => { + await sut.migrate(helper); + expect(helper.set).toHaveBeenCalledTimes(1); + expect(helper.set).toHaveBeenCalledWith("user-1", { + keys: { + otherStuff: "overStuff2", + }, + otherStuff: "otherStuff3", + }); + }); + + it("should set biometricEncryptionClientKeyHalf value for account that have it", async () => { + await sut.migrate(helper); + + expect(helper.setToUser).toHaveBeenCalledWith("user-1", CLIENT_KEY_HALF, "user1-key-half"); + }); + + it("should not call extra setToUser", async () => { + await sut.migrate(helper); + + expect(helper.setToUser).toHaveBeenCalledTimes(1); + }); + }); + + describe("rollback", () => { + beforeEach(() => { + helper = mockMigrationHelper(rollbackJSON(), 14); + sut = new MoveBiometricClientKeyHalfToStateProviders(13, 14); + }); + + it("should null out new values", async () => { + await sut.rollback(helper); + + expect(helper.setToUser).toHaveBeenCalledWith("user-1", CLIENT_KEY_HALF, null); + }); + + it("should add explicit value back to accounts", async () => { + await sut.rollback(helper); + + expect(helper.set).toHaveBeenCalledTimes(1); + expect(helper.set).toHaveBeenCalledWith("user-1", { + keys: { + biometricEncryptionClientKeyHalf: "user1-key-half", + otherStuff: "overStuff2", + }, + otherStuff: "otherStuff3", + }); + }); + + it.each(["user-2", "user-3"])( + "should not try to restore values to missing accounts", + async (userId) => { + await sut.rollback(helper); + + expect(helper.set).not.toHaveBeenCalledWith(userId, any()); + }, + ); + }); +}); diff --git a/libs/common/src/state-migrations/migrations/14-move-biometric-client-key-half-state-to-providers.ts b/libs/common/src/state-migrations/migrations/14-move-biometric-client-key-half-state-to-providers.ts new file mode 100644 index 0000000000..a1ddd75398 --- /dev/null +++ b/libs/common/src/state-migrations/migrations/14-move-biometric-client-key-half-state-to-providers.ts @@ -0,0 +1,65 @@ +import { KeyDefinitionLike, MigrationHelper } from "../migration-helper"; +import { Migrator } from "../migrator"; + +type ExpectedAccountType = { + settings?: { + disableAutoBiometricsPrompt?: boolean; + biometricUnlock?: boolean; + dismissedBiometricRequirePasswordOnStartCallout?: boolean; + }; + keys?: { biometricEncryptionClientKeyHalf?: string }; +}; + +// Biometric text, no auto prompt text, fingerprint validated, and prompt cancelled are refreshed on every app start, so we don't need to migrate them +export const CLIENT_KEY_HALF: KeyDefinitionLike = { + key: "clientKeyHalf", + stateDefinition: { name: "biometricSettings" }, +}; + +export class MoveBiometricClientKeyHalfToStateProviders extends Migrator<13, 14> { + async migrate(helper: MigrationHelper): Promise { + const legacyAccounts = await helper.getAccounts(); + + await Promise.all( + legacyAccounts.map(async ({ userId, account }) => { + // Move account data + if (account?.keys?.biometricEncryptionClientKeyHalf != null) { + await helper.setToUser( + userId, + CLIENT_KEY_HALF, + account.keys.biometricEncryptionClientKeyHalf, + ); + + // Delete old account data + delete account?.keys?.biometricEncryptionClientKeyHalf; + await helper.set(userId, account); + } + }), + ); + } + + async rollback(helper: MigrationHelper): Promise { + async function rollbackUser(userId: string, account: ExpectedAccountType) { + let updatedAccount = false; + + const userKeyHalf = await helper.getFromUser(userId, CLIENT_KEY_HALF); + + if (userKeyHalf) { + account ??= {}; + account.keys ??= {}; + + updatedAccount = true; + account.keys.biometricEncryptionClientKeyHalf = userKeyHalf; + await helper.setToUser(userId, CLIENT_KEY_HALF, null); + } + + if (updatedAccount) { + await helper.set(userId, account); + } + } + + const accounts = await helper.getAccounts(); + + await Promise.all(accounts.map(({ userId, account }) => rollbackUser(userId, account))); + } +}