diff --git a/apps/browser/src/auth/popup/settings/account-security.component.ts b/apps/browser/src/auth/popup/settings/account-security.component.ts index a9f43045902d..4eb24d19605f 100644 --- a/apps/browser/src/auth/popup/settings/account-security.component.ts +++ b/apps/browser/src/auth/popup/settings/account-security.component.ts @@ -311,12 +311,8 @@ export class AccountSecurityComponent implements OnInit, OnDestroy { .pipe( concatMap(async (value) => { const userId = (await firstValueFrom(this.accountService.activeAccount$)).id; - const pinKeyEncryptedUserKey = - (await this.pinService.getPinKeyEncryptedUserKeyPersistent(userId)) || - (await this.pinService.getPinKeyEncryptedUserKeyEphemeral(userId)); - await this.pinService.clearPinKeyEncryptedUserKeyPersistent(userId); - await this.pinService.clearPinKeyEncryptedUserKeyEphemeral(userId); - await this.pinService.storePinKeyEncryptedUserKey(pinKeyEncryptedUserKey, value, userId); + const pin = await this.pinService.getPin(userId); + await this.pinService.setPin(pin, value ? "EPHEMERAL" : "PERSISTENT", userId); this.refreshTimeoutSettings$.next(); }), takeUntil(this.destroy$), @@ -486,7 +482,7 @@ export class AccountSecurityComponent implements OnInit, OnDestroy { } } else { const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); - await this.vaultTimeoutSettingsService.clear(userId); + await this.pinService.unsetPin(userId); } } diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 3ccd78db18ce..f8644c4f7eb2 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -98,6 +98,7 @@ import { KeyConnectorService as KeyConnectorServiceAbstraction } from "@bitwarde import { KeyConnectorService } from "@bitwarden/common/key-management/key-connector/services/key-connector.service"; import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; import { MasterPasswordService } from "@bitwarden/common/key-management/master-password/services/master-password.service"; +import { PinStateService } from "@bitwarden/common/key-management/pin/pin-state.service.implementation"; import { PinServiceAbstraction } from "@bitwarden/common/key-management/pin/pin.service.abstraction"; import { PinService } from "@bitwarden/common/key-management/pin/pin.service.implementation"; import { SecurityStateService } from "@bitwarden/common/key-management/security-state/abstractions/security-state.service"; @@ -706,18 +707,7 @@ export default class MainBackground { this.kdfConfigService = new DefaultKdfConfigService(this.stateProvider); - this.pinService = new PinService( - this.accountService, - this.cryptoFunctionService, - this.encryptService, - this.kdfConfigService, - this.keyGenerationService, - this.logService, - this.stateProvider, - ); - this.keyService = new DefaultKeyService( - this.pinService, this.masterPasswordService, this.keyGenerationService, this.cryptoFunctionService, @@ -730,6 +720,19 @@ export default class MainBackground { this.kdfConfigService, ); + const pinStateService = new PinStateService(this.stateProvider); + + this.pinService = new PinService( + this.accountService, + this.encryptService, + this.kdfConfigService, + this.keyGenerationService, + this.logService, + this.keyService, + this.sdkService, + pinStateService, + ); + this.appIdService = new AppIdService(this.storageService, this.logService); this.userDecryptionOptionsService = new UserDecryptionOptionsService(this.stateProvider); @@ -738,7 +741,7 @@ export default class MainBackground { this.vaultTimeoutSettingsService = new DefaultVaultTimeoutSettingsService( this.accountService, - this.pinService, + pinStateService, this.userDecryptionOptionsService, this.keyService, this.tokenService, @@ -756,6 +759,7 @@ export default class MainBackground { this.biometricStateService, this.messagingService, this.vaultTimeoutSettingsService, + this.pinService, ); this.apiService = new ApiService( @@ -1676,9 +1680,9 @@ export default class MainBackground { this.cipherService.clear(userBeingLoggedOut), // ! DO NOT REMOVE folderService.clear ! For more information see PM-25660 this.folderService.clear(userBeingLoggedOut), - this.vaultTimeoutSettingsService.clear(userBeingLoggedOut), this.biometricStateService.logout(userBeingLoggedOut), this.popupViewCacheBackgroundService.clearState(), + this.pinService.logout(userBeingLoggedOut), /* We intentionally do not clear: * - autofillSettingsService * - badgeSettingsService diff --git a/apps/browser/src/key-management/biometrics/background-browser-biometrics.service.spec.ts b/apps/browser/src/key-management/biometrics/background-browser-biometrics.service.spec.ts index 4017953ee282..5bbba77d12e7 100644 --- a/apps/browser/src/key-management/biometrics/background-browser-biometrics.service.spec.ts +++ b/apps/browser/src/key-management/biometrics/background-browser-biometrics.service.spec.ts @@ -1,5 +1,6 @@ import { mock } from "jest-mock-extended"; +import { PinServiceAbstraction } from "@bitwarden/common/key-management/pin/pin.service.abstraction"; import { VaultTimeoutSettingsService } from "@bitwarden/common/key-management/vault-timeout"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; @@ -18,6 +19,7 @@ describe("background browser biometrics service tests", function () { const biometricStateService = mock(); const messagingService = mock(); const vaultTimeoutSettingsService = mock(); + const pinService = mock(); beforeEach(() => { jest.resetAllMocks(); @@ -28,6 +30,7 @@ describe("background browser biometrics service tests", function () { biometricStateService, messagingService, vaultTimeoutSettingsService, + pinService, ); }); diff --git a/apps/browser/src/key-management/biometrics/background-browser-biometrics.service.ts b/apps/browser/src/key-management/biometrics/background-browser-biometrics.service.ts index 8f755cfeda69..c8be58b0bdec 100644 --- a/apps/browser/src/key-management/biometrics/background-browser-biometrics.service.ts +++ b/apps/browser/src/key-management/biometrics/background-browser-biometrics.service.ts @@ -1,6 +1,7 @@ import { combineLatest, timer } from "rxjs"; import { filter, concatMap } from "rxjs/operators"; +import { PinServiceAbstraction } from "@bitwarden/common/key-management/pin/pin.service.abstraction"; import { VaultTimeoutSettingsService } from "@bitwarden/common/key-management/vault-timeout"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; @@ -29,6 +30,7 @@ export class BackgroundBrowserBiometricsService extends BiometricsService { private biometricStateService: BiometricStateService, private messagingService: MessagingService, private vaultTimeoutSettingsService: VaultTimeoutSettingsService, + private pinService: PinServiceAbstraction, ) { super(); // Always connect to the native messaging background if biometrics are enabled, not just when it is used @@ -101,6 +103,7 @@ export class BackgroundBrowserBiometricsService extends BiometricsService { if (await this.keyService.validateUserKey(userKey, userId)) { await this.biometricStateService.setBiometricUnlockEnabled(true); await this.keyService.setUserKey(userKey, userId); + await this.pinService.userUnlocked(userId); // to update badge and other things this.messagingService.send("switchAccount", { userId }); return userKey; diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index 7a10dc2343f3..13684e56bda9 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -75,7 +75,6 @@ import { InternalMasterPasswordServiceAbstraction, MasterPasswordServiceAbstraction, } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; -import { PinServiceAbstraction } from "@bitwarden/common/key-management/pin/pin.service.abstraction"; import { VaultTimeoutService, VaultTimeoutStringType, @@ -271,7 +270,6 @@ const safeProviders: SafeProvider[] = [ safeProvider({ provide: KeyService, useFactory: ( - pinService: PinServiceAbstraction, masterPasswordService: InternalMasterPasswordServiceAbstraction, keyGenerationService: KeyGenerationService, cryptoFunctionService: CryptoFunctionService, @@ -284,7 +282,6 @@ const safeProviders: SafeProvider[] = [ kdfConfigService: KdfConfigService, ) => { const keyService = new DefaultKeyService( - pinService, masterPasswordService, keyGenerationService, cryptoFunctionService, @@ -300,7 +297,6 @@ const safeProviders: SafeProvider[] = [ return keyService; }, deps: [ - PinServiceAbstraction, InternalMasterPasswordServiceAbstraction, KeyGenerationService, CryptoFunctionService, diff --git a/apps/cli/src/service-container/service-container.ts b/apps/cli/src/service-container/service-container.ts index ccce00fabd85..ed87ae422bca 100644 --- a/apps/cli/src/service-container/service-container.ts +++ b/apps/cli/src/service-container/service-container.ts @@ -74,6 +74,7 @@ import { MasterPasswordUnlockService } from "@bitwarden/common/key-management/ma import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; import { DefaultMasterPasswordUnlockService } from "@bitwarden/common/key-management/master-password/services/default-master-password-unlock.service"; import { MasterPasswordService } from "@bitwarden/common/key-management/master-password/services/master-password.service"; +import { PinStateService } from "@bitwarden/common/key-management/pin/pin-state.service.implementation"; import { PinServiceAbstraction } from "@bitwarden/common/key-management/pin/pin.service.abstraction"; import { PinService } from "@bitwarden/common/key-management/pin/pin.service.implementation"; import { SecurityStateService } from "@bitwarden/common/key-management/security-state/abstractions/security-state.service"; @@ -93,7 +94,7 @@ import { } from "@bitwarden/common/platform/abstractions/environment.service"; import { SdkLoadService } from "@bitwarden/common/platform/abstractions/sdk/sdk-load.service"; import { SdkService } from "@bitwarden/common/platform/abstractions/sdk/sdk.service"; -import { KeySuffixOptions, LogLevelType } from "@bitwarden/common/platform/enums"; +import { LogLevelType } from "@bitwarden/common/platform/enums"; import { MessageSender } from "@bitwarden/common/platform/messaging"; import { TaskSchedulerService, @@ -459,18 +460,7 @@ export class ServiceContainer { this.accountService, ); - this.pinService = new PinService( - this.accountService, - this.cryptoFunctionService, - this.encryptService, - this.kdfConfigService, - this.keyGenerationService, - this.logService, - this.stateProvider, - ); - this.keyService = new KeyService( - this.pinService, this.masterPasswordService, this.keyGenerationService, this.cryptoFunctionService, @@ -483,6 +473,18 @@ export class ServiceContainer { this.kdfConfigService, ); + const pinStateService = new PinStateService(this.stateProvider); + this.pinService = new PinService( + this.accountService, + this.encryptService, + this.kdfConfigService, + this.keyGenerationService, + this.logService, + this.keyService, + this.sdkService, + pinStateService, + ); + this.masterPasswordUnlockService = new DefaultMasterPasswordUnlockService( this.masterPasswordService, this.keyService, @@ -506,7 +508,7 @@ export class ServiceContainer { this.vaultTimeoutSettingsService = new DefaultVaultTimeoutSettingsService( this.accountService, - this.pinService, + pinStateService, this.userDecryptionOptionsService, this.keyService, this.tokenService, @@ -772,7 +774,7 @@ export class ServiceContainer { this.folderApiService = new FolderApiService(this.folderService, this.apiService); const lockedCallback = async (userId: UserId) => - await this.keyService.clearStoredUserKey(KeySuffixOptions.Auto, userId); + await this.keyService.clearStoredUserKey(userId); this.userVerificationApiService = new UserVerificationApiService(this.apiService); diff --git a/apps/desktop/src/app/accounts/settings.component.spec.ts b/apps/desktop/src/app/accounts/settings.component.spec.ts index 18b62fbf5935..cb2a6ae259a7 100644 --- a/apps/desktop/src/app/accounts/settings.component.spec.ts +++ b/apps/desktop/src/app/accounts/settings.component.spec.ts @@ -390,7 +390,7 @@ describe("SettingsComponent", () => { await component.updatePinHandler(false); expect(component.form.controls.pin.value).toBe(false); - expect(vaultTimeoutSettingsService.clear).toHaveBeenCalled(); + expect(vaultTimeoutSettingsService.clear).not.toHaveBeenCalled(); expect(messagingService.send).toHaveBeenCalledWith("redrawMenu"); }); }); diff --git a/apps/desktop/src/app/accounts/settings.component.ts b/apps/desktop/src/app/accounts/settings.component.ts index 55b3b5f2f680..53b2cad43765 100644 --- a/apps/desktop/src/app/accounts/settings.component.ts +++ b/apps/desktop/src/app/accounts/settings.component.ts @@ -588,7 +588,7 @@ export class SettingsComponent implements OnInit, OnDestroy { this.form.controls.pin.setValue(this.userHasPinSet, { emitEvent: false }); } else { const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); - await this.vaultTimeoutSettingsService.clear(userId); + await this.pinService.unsetPin(userId); } } diff --git a/apps/desktop/src/app/app.component.ts b/apps/desktop/src/app/app.component.ts index 1c2d3aa464df..1a0e9ae92a0e 100644 --- a/apps/desktop/src/app/app.component.ts +++ b/apps/desktop/src/app/app.component.ts @@ -47,6 +47,7 @@ import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { ProcessReloadServiceAbstraction } from "@bitwarden/common/key-management/abstractions/process-reload.service"; 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 { PinServiceAbstraction } from "@bitwarden/common/key-management/pin/pin.service.abstraction"; import { VaultTimeout, VaultTimeoutAction, @@ -177,6 +178,7 @@ export class AppComponent implements OnInit, OnDestroy { private readonly destroyRef: DestroyRef, private readonly documentLangSetter: DocumentLangSetter, private restrictedItemTypesService: RestrictedItemTypesService, + private pinService: PinServiceAbstraction, private readonly tokenService: TokenService, private desktopAutotypeDefaultSettingPolicy: DesktopAutotypeDefaultSettingPolicy, ) { @@ -693,8 +695,8 @@ export class AppComponent implements OnInit, OnDestroy { await this.cipherService.clear(userBeingLoggedOut); // ! DO NOT REMOVE folderService.clear ! For more information see PM-25660 await this.folderService.clear(userBeingLoggedOut); - await this.vaultTimeoutSettingsService.clear(userBeingLoggedOut); await this.biometricStateService.logout(userBeingLoggedOut); + await this.pinService.logout(userBeingLoggedOut); await this.stateEventRunnerService.handleEvent("logout", userBeingLoggedOut); diff --git a/apps/desktop/src/app/services/services.module.ts b/apps/desktop/src/app/services/services.module.ts index 9f2bb1acc90d..a0ee33a459c2 100644 --- a/apps/desktop/src/app/services/services.module.ts +++ b/apps/desktop/src/app/services/services.module.ts @@ -306,7 +306,6 @@ const safeProviders: SafeProvider[] = [ provide: KeyServiceAbstraction, useClass: ElectronKeyService, deps: [ - PinServiceAbstraction, InternalMasterPasswordServiceAbstraction, KeyGenerationService, CryptoFunctionServiceAbstraction, diff --git a/apps/desktop/src/key-management/electron-key.service.spec.ts b/apps/desktop/src/key-management/electron-key.service.spec.ts index 2d60c47217de..cc1d68ed050b 100644 --- a/apps/desktop/src/key-management/electron-key.service.spec.ts +++ b/apps/desktop/src/key-management/electron-key.service.spec.ts @@ -4,7 +4,6 @@ import { KeyGenerationService } from "@bitwarden/common/key-management/crypto"; import { CryptoFunctionService } from "@bitwarden/common/key-management/crypto/abstractions/crypto-function.service"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; import { FakeMasterPasswordService } from "@bitwarden/common/key-management/master-password/services/fake-master-password.service"; -import { PinServiceAbstraction } from "@bitwarden/common/key-management/pin/pin.service.abstraction"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; @@ -26,7 +25,6 @@ import { ElectronKeyService } from "./electron-key.service"; describe("ElectronKeyService", () => { let keyService: ElectronKeyService; - const pinService = mock(); const keyGenerationService = mock(); const cryptoFunctionService = mock(); const encryptService = mock(); @@ -48,7 +46,6 @@ describe("ElectronKeyService", () => { stateProvider = new FakeStateProvider(accountService); keyService = new ElectronKeyService( - pinService, masterPasswordService, keyGenerationService, cryptoFunctionService, diff --git a/apps/desktop/src/key-management/electron-key.service.ts b/apps/desktop/src/key-management/electron-key.service.ts index 59295b2ca21d..8de079a826b2 100644 --- a/apps/desktop/src/key-management/electron-key.service.ts +++ b/apps/desktop/src/key-management/electron-key.service.ts @@ -3,7 +3,6 @@ import { KeyGenerationService } from "@bitwarden/common/key-management/crypto"; import { CryptoFunctionService } from "@bitwarden/common/key-management/crypto/abstractions/crypto-function.service"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; 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 { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; @@ -22,7 +21,6 @@ import { DesktopBiometricsService } from "./biometrics/desktop.biometrics.servic // TODO Remove this class once biometric client key half storage is moved https://bitwarden.atlassian.net/browse/PM-22342 export class ElectronKeyService extends DefaultKeyService { constructor( - pinService: PinServiceAbstraction, masterPasswordService: InternalMasterPasswordServiceAbstraction, keyGenerationService: KeyGenerationService, cryptoFunctionService: CryptoFunctionService, @@ -37,7 +35,6 @@ export class ElectronKeyService extends DefaultKeyService { private biometricService: DesktopBiometricsService, ) { super( - pinService, masterPasswordService, keyGenerationService, cryptoFunctionService, @@ -51,10 +48,6 @@ export class ElectronKeyService extends DefaultKeyService { ); } - override async clearStoredUserKey(keySuffix: KeySuffixOptions, userId: UserId): Promise { - await super.clearStoredUserKey(keySuffix, userId); - } - protected override async storeAdditionalKeys(key: UserKey, userId: UserId) { await super.storeAdditionalKeys(key, userId); diff --git a/libs/angular/src/auth/components/set-pin.component.ts b/libs/angular/src/auth/components/set-pin.component.ts index ba816d5ad34c..28f0fbaee979 100644 --- a/libs/angular/src/auth/components/set-pin.component.ts +++ b/libs/angular/src/auth/components/set-pin.component.ts @@ -9,7 +9,6 @@ import { UserVerificationService } from "@bitwarden/common/auth/abstractions/use import { PinServiceAbstraction } from "@bitwarden/common/key-management/pin/pin.service.abstraction"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { DialogRef } from "@bitwarden/components"; -import { KeyService } from "@bitwarden/key-management"; @Directive() export class SetPinComponent implements OnInit { @@ -22,7 +21,6 @@ export class SetPinComponent implements OnInit { constructor( private accountService: AccountService, - private keyService: KeyService, private dialogRef: DialogRef, private formBuilder: FormBuilder, private pinService: PinServiceAbstraction, @@ -47,25 +45,11 @@ export class SetPinComponent implements OnInit { } const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; - const userKey = await firstValueFrom(this.keyService.userKey$(userId)); - - const userKeyEncryptedPin = await this.pinService.createUserKeyEncryptedPin( - pinFormControl.value, - userKey, - ); - await this.pinService.setUserKeyEncryptedPin(userKeyEncryptedPin, userId); - - const pinKeyEncryptedUserKey = await this.pinService.createPinKeyEncryptedUserKey( + await this.pinService.setPin( pinFormControl.value, - userKey, + requireMasterPasswordOnClientRestart ? "EPHEMERAL" : "PERSISTENT", userId, ); - await this.pinService.storePinKeyEncryptedUserKey( - pinKeyEncryptedUserKey, - requireMasterPasswordOnClientRestart, - userId, - ); - this.dialogRef.close(true); }; } diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index f1d1c678c243..421415524726 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -182,6 +182,8 @@ import { } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; import { DefaultMasterPasswordUnlockService } from "@bitwarden/common/key-management/master-password/services/default-master-password-unlock.service"; import { MasterPasswordService } from "@bitwarden/common/key-management/master-password/services/master-password.service"; +import { PinStateServiceAbstraction } from "@bitwarden/common/key-management/pin/pin-state.service.abstraction"; +import { PinStateService } from "@bitwarden/common/key-management/pin/pin-state.service.implementation"; import { PinServiceAbstraction } from "@bitwarden/common/key-management/pin/pin.service.abstraction"; import { PinService } from "@bitwarden/common/key-management/pin/pin.service.implementation"; import { SecurityStateService } from "@bitwarden/common/key-management/security-state/abstractions/security-state.service"; @@ -698,7 +700,6 @@ const safeProviders: SafeProvider[] = [ provide: KeyService, useClass: DefaultKeyService, deps: [ - PinServiceAbstraction, InternalMasterPasswordServiceAbstraction, KeyGenerationService, CryptoFunctionServiceAbstraction, @@ -858,7 +859,7 @@ const safeProviders: SafeProvider[] = [ useClass: DefaultVaultTimeoutSettingsService, deps: [ AccountServiceAbstraction, - PinServiceAbstraction, + PinStateServiceAbstraction, UserDecryptionOptionsServiceAbstraction, KeyService, TokenServiceAbstraction, @@ -1294,17 +1295,23 @@ const safeProviders: SafeProvider[] = [ AccountServiceAbstraction, ], }), + safeProvider({ + provide: PinStateServiceAbstraction, + useClass: PinStateService, + deps: [StateProvider], + }), safeProvider({ provide: PinServiceAbstraction, useClass: PinService, deps: [ AccountServiceAbstraction, - CryptoFunctionServiceAbstraction, EncryptService, KdfConfigService, KeyGenerationService, LogService, - StateProvider, + KeyService, + SdkService, + PinStateServiceAbstraction, ], }), safeProvider({ diff --git a/libs/common/src/auth/services/user-verification/user-verification.service.spec.ts b/libs/common/src/auth/services/user-verification/user-verification.service.spec.ts index b8a9a85ed0a9..790c134398cb 100644 --- a/libs/common/src/auth/services/user-verification/user-verification.service.spec.ts +++ b/libs/common/src/auth/services/user-verification/user-verification.service.spec.ts @@ -19,8 +19,8 @@ import { import { FakeAccountService, mockAccountServiceWith } from "../../../../spec"; import { InternalMasterPasswordServiceAbstraction } from "../../../key-management/master-password/abstractions/master-password.service.abstraction"; +import { PinLockType } from "../../../key-management/pin/pin-lock-type"; import { PinServiceAbstraction } from "../../../key-management/pin/pin.service.abstraction"; -import { PinLockType } from "../../../key-management/pin/pin.service.implementation"; import { VaultTimeoutSettingsService } from "../../../key-management/vault-timeout"; import { I18nService } from "../../../platform/abstractions/i18n.service"; import { HashPurpose } from "../../../platform/enums"; diff --git a/libs/common/src/key-management/pin/pin-lock-type.ts b/libs/common/src/key-management/pin/pin-lock-type.ts new file mode 100644 index 000000000000..96578553b82a --- /dev/null +++ b/libs/common/src/key-management/pin/pin-lock-type.ts @@ -0,0 +1,7 @@ +/** + * - DISABLED : No PIN set. + * - PERSISTENT : PIN is set and persists through client reset. + * - EPHEMERAL : PIN is set, but does NOT persist through client reset. This means that + * after client reset the master password is required to unlock. + */ +export type PinLockType = "DISABLED" | "PERSISTENT" | "EPHEMERAL"; diff --git a/libs/common/src/key-management/pin/pin-state.service.abstraction.ts b/libs/common/src/key-management/pin/pin-state.service.abstraction.ts new file mode 100644 index 000000000000..cd10b8b7fe20 --- /dev/null +++ b/libs/common/src/key-management/pin/pin-state.service.abstraction.ts @@ -0,0 +1,86 @@ +import { Observable } from "rxjs"; + +import { PasswordProtectedKeyEnvelope } from "@bitwarden/sdk-internal"; + +import { UserId } from "../../types/guid"; +import { EncryptedString, EncString } from "../crypto/models/enc-string"; + +import { PinLockType } from "./pin-lock-type"; + +/** + * The PinStateService manages the storage and retrieval of PIN-related state for user accounts. + */ +export abstract class PinStateServiceAbstraction { + /** + * Gets the user's UserKey encrypted PIN + * @deprecated - This is not a public API. DO NOT USE IT + * @param userId The user's id + * @throws If the user id is not provided + */ + abstract userKeyEncryptedPin$(userId: UserId): Observable; + + /** + * Gets the user's {@link PinLockType} + * @param userId The user's id + * @throws If the user id is not provided + */ + abstract getPinLockType(userId: UserId): Promise; + + /** + * Checks if a user is enrolled into PIN unlock + * @param userId The user's id + */ + abstract isPinSet(userId: UserId): Promise; + + /** + * Gets the user's PIN-protected UserKey envelope, either persistent or ephemeral based on the provided PinLockType + * @deprecated - This is not a public API. DO NOT USE IT + * @param userId The user's id + * @param pinLockType User's {@link PinLockType}. + * @throws if the user id is not provided + * @throws if the pin lock type is not persistent or ephemeral + */ + abstract getPinProtectedUserKeyEnvelope( + userId: UserId, + pinLockType: PinLockType, + ): Promise; + + /** + * Gets the user's legacy PIN-protected UserKey + * @deprecated Use {@link getPinProtectedUserKeyEnvelope} instead. Only for migration support. + * @param userId The user's id + * @throws If the user id is not provided + */ + abstract getLegacyPinKeyEncryptedUserKeyPersistent(userId: UserId): Promise; + + /** + * Sets the PIN state for the user + * @deprecated - This is not a public API. DO NOT USE IT + * @param userId The user's id + * @param pinProtectedUserKeyEnvelope The user's PIN-protected UserKey envelope + * @param userKeyEncryptedPin The user's UserKey-encrypted PIN + * @param pinLockType The user's PinLockType + * @throws If the user id, pinProtectedUserKeyEnvelope, or pinLockType is not provided + * @throws If the pin lock type is not persistent or ephemeral + */ + abstract setPinState( + userId: UserId, + pinProtectedUserKeyEnvelope: PasswordProtectedKeyEnvelope, + userKeyEncryptedPin: EncryptedString, + pinLockType: PinLockType, + ): Promise; + + /** + * Clears all PIN state for the user, both persistent and ephemeral + * @param userId The user's id + * @throws If the user id is not provided + */ + abstract clearPinState(userId: UserId): Promise; + + /** + * Clears only the user's ephemeral PIN. Persistent PIN state and UserKey wrapped PIN remains unchanged. + * @param userId The user's id + * @throws If the user id is not provided + */ + abstract clearEphemeralPinState(userId: UserId): Promise; +} diff --git a/libs/common/src/key-management/pin/pin-state.service.implementation.ts b/libs/common/src/key-management/pin/pin-state.service.implementation.ts new file mode 100644 index 000000000000..0bf6cb60fb01 --- /dev/null +++ b/libs/common/src/key-management/pin/pin-state.service.implementation.ts @@ -0,0 +1,129 @@ +import { firstValueFrom, map, Observable } from "rxjs"; + +import { PasswordProtectedKeyEnvelope } from "@bitwarden/sdk-internal"; +import { StateProvider } from "@bitwarden/state"; +import { UserId } from "@bitwarden/user-core"; + +import { assertNonNullish } from "../../auth/utils"; +import { EncryptedString, EncString } from "../crypto/models/enc-string"; + +import { PinLockType } from "./pin-lock-type"; +import { PinStateServiceAbstraction } from "./pin-state.service.abstraction"; +import { + PIN_PROTECTED_USER_KEY_ENVELOPE_PERSISTENT, + PIN_PROTECTED_USER_KEY_ENVELOPE_EPHEMERAL, + USER_KEY_ENCRYPTED_PIN, + PIN_KEY_ENCRYPTED_USER_KEY_PERSISTENT, +} from "./pin.state"; + +export class PinStateService implements PinStateServiceAbstraction { + constructor(private stateProvider: StateProvider) {} + + userKeyEncryptedPin$(userId: UserId): Observable { + assertNonNullish(userId, "userId"); + + return this.stateProvider + .getUserState$(USER_KEY_ENCRYPTED_PIN, userId) + .pipe(map((value) => (value ? new EncString(value) : null))); + } + + async isPinSet(userId: UserId): Promise { + assertNonNullish(userId, "userId"); + return (await this.getPinLockType(userId)) !== "DISABLED"; + } + + async getPinLockType(userId: UserId): Promise { + assertNonNullish(userId, "userId"); + + const isPersistentPinSet = + (await this.getPinProtectedUserKeyEnvelope(userId, "PERSISTENT")) != null || + // Deprecated + (await this.getLegacyPinKeyEncryptedUserKeyPersistent(userId)) != null; + const isPinSet = + (await firstValueFrom(this.stateProvider.getUserState$(USER_KEY_ENCRYPTED_PIN, userId))) != + null; + + if (isPersistentPinSet) { + return "PERSISTENT"; + } else if (isPinSet) { + return "EPHEMERAL"; + } else { + return "DISABLED"; + } + } + + async getPinProtectedUserKeyEnvelope( + userId: UserId, + pinLockType: PinLockType, + ): Promise { + assertNonNullish(userId, "userId"); + + if (pinLockType === "EPHEMERAL") { + return await firstValueFrom( + this.stateProvider.getUserState$(PIN_PROTECTED_USER_KEY_ENVELOPE_EPHEMERAL, userId), + ); + } else if (pinLockType === "PERSISTENT") { + return await firstValueFrom( + this.stateProvider.getUserState$(PIN_PROTECTED_USER_KEY_ENVELOPE_PERSISTENT, userId), + ); + } else { + throw new Error(`Unsupported PinLockType: ${pinLockType}`); + } + } + + async getLegacyPinKeyEncryptedUserKeyPersistent(userId: UserId): Promise { + assertNonNullish(userId, "userId"); + + return await firstValueFrom( + this.stateProvider + .getUserState$(PIN_KEY_ENCRYPTED_USER_KEY_PERSISTENT, userId) + .pipe(map((value) => (value ? new EncString(value) : null))), + ); + } + + async setPinState( + userId: UserId, + pinProtectedUserKeyEnvelope: PasswordProtectedKeyEnvelope, + userKeyEncryptedPin: EncryptedString, + pinLockType: PinLockType, + ): Promise { + assertNonNullish(userId, "userId"); + assertNonNullish(pinProtectedUserKeyEnvelope, "pinProtectedUserKeyEnvelope"); + assertNonNullish(pinLockType, "pinLockType"); + + if (pinLockType === "EPHEMERAL") { + await this.stateProvider.setUserState( + PIN_PROTECTED_USER_KEY_ENVELOPE_EPHEMERAL, + pinProtectedUserKeyEnvelope, + userId, + ); + } else if (pinLockType === "PERSISTENT") { + await this.stateProvider.setUserState( + PIN_PROTECTED_USER_KEY_ENVELOPE_PERSISTENT, + pinProtectedUserKeyEnvelope, + userId, + ); + } else { + throw new Error(`Cannot set up PIN with pin lock type ${pinLockType}`); + } + + await this.stateProvider.setUserState(USER_KEY_ENCRYPTED_PIN, userKeyEncryptedPin, userId); + } + + async clearPinState(userId: UserId): Promise { + assertNonNullish(userId, "userId"); + + await this.stateProvider.setUserState(USER_KEY_ENCRYPTED_PIN, null, userId); + await this.stateProvider.setUserState(PIN_PROTECTED_USER_KEY_ENVELOPE_EPHEMERAL, null, userId); + await this.stateProvider.setUserState(PIN_PROTECTED_USER_KEY_ENVELOPE_PERSISTENT, null, userId); + + // Note: This can be deleted after sufficiently many PINs are migrated and the state is removed. + await this.stateProvider.setUserState(PIN_KEY_ENCRYPTED_USER_KEY_PERSISTENT, null, userId); + } + + async clearEphemeralPinState(userId: UserId): Promise { + assertNonNullish(userId, "userId"); + + await this.stateProvider.setUserState(PIN_PROTECTED_USER_KEY_ENVELOPE_EPHEMERAL, null, userId); + } +} diff --git a/libs/common/src/key-management/pin/pin-state.service.spec.ts b/libs/common/src/key-management/pin/pin-state.service.spec.ts new file mode 100644 index 000000000000..be85a15e6d3b --- /dev/null +++ b/libs/common/src/key-management/pin/pin-state.service.spec.ts @@ -0,0 +1,727 @@ +import { firstValueFrom } from "rxjs"; + +import { PasswordProtectedKeyEnvelope } from "@bitwarden/sdk-internal"; + +import { FakeAccountService, FakeStateProvider, mockAccountServiceWith } from "../../../spec"; +import { Utils } from "../../platform/misc/utils"; +import { UserId } from "../../types/guid"; +import { EncryptedString } from "../crypto/models/enc-string"; + +import { PinLockType } from "./pin-lock-type"; +import { PinStateService } from "./pin-state.service.implementation"; +import { + USER_KEY_ENCRYPTED_PIN, + PIN_PROTECTED_USER_KEY_ENVELOPE_EPHEMERAL, + PIN_PROTECTED_USER_KEY_ENVELOPE_PERSISTENT, + PIN_KEY_ENCRYPTED_USER_KEY_PERSISTENT, +} from "./pin.state"; + +describe("PinStateService", () => { + let sut: PinStateService; + + let accountService: FakeAccountService; + let stateProvider: FakeStateProvider; + + const mockUserId = Utils.newGuid() as UserId; + const mockUserEmail = "user@example.com"; + const mockUserKeyEncryptedPin = "userKeyEncryptedPin" as EncryptedString; + const mockEphemeralEnvelope = "mock-ephemeral-envelope" as PasswordProtectedKeyEnvelope; + const mockPersistentEnvelope = "mock-persistent-envelope" as PasswordProtectedKeyEnvelope; + + beforeEach(() => { + accountService = mockAccountServiceWith(mockUserId, { email: mockUserEmail }); + stateProvider = new FakeStateProvider(accountService); + + sut = new PinStateService(stateProvider); + }); + + it("should instantiate the PinStateService", () => { + expect(sut).not.toBeFalsy(); + }); + + describe("userKeyWrappedPin$", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + test.each([null, undefined])("throws if userId is %p", async (userId) => { + // Arrange + await sut.setPinState( + mockUserId, + mockPersistentEnvelope, + mockUserKeyEncryptedPin, + "PERSISTENT", + ); + + // Act & Assert + expect(() => sut.userKeyEncryptedPin$(userId as any)).toThrow("userId is null or undefined."); + }); + + test.each([null, undefined])("emits null if userKeyEncryptedPin is nullish", async (value) => { + // Arrange + await stateProvider.setUserState(USER_KEY_ENCRYPTED_PIN, value, mockUserId); + + // Act + const result = await firstValueFrom(sut.userKeyEncryptedPin$(mockUserId)); + + // Assert + expect(result).toBe(null); + }); + + it("emits the userKeyEncryptedPin when available", async () => { + // Arrange + await sut.setPinState( + mockUserId, + mockPersistentEnvelope, + mockUserKeyEncryptedPin, + "PERSISTENT", + ); + + // Act + const result = await firstValueFrom(sut.userKeyEncryptedPin$(mockUserId)); + + // Assert + expect(result?.encryptedString).toEqual(mockUserKeyEncryptedPin); + }); + + it("emits null when userKeyEncryptedPin isn't available", async () => { + // Arrange - don't set any state + + // Act + const result = await firstValueFrom(sut.userKeyEncryptedPin$(mockUserId)); + + // Assert + expect(result).toBeNull(); + }); + }); + + describe("getPinLockType()", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it("should throw an error if userId is null", async () => { + // Act & Assert + await expect(sut.getPinLockType(null as any)).rejects.toThrow("userId"); + }); + + it("should return 'PERSISTENT' if a pin protected user key (persistent) is found", async () => { + // Arrange + await sut.setPinState( + mockUserId, + mockPersistentEnvelope, + mockUserKeyEncryptedPin, + "PERSISTENT", + ); + + // Act + const result = await sut.getPinLockType(mockUserId); + + // Assert + expect(result).toBe("PERSISTENT"); + }); + + it("should return 'PERSISTENT' if a legacy pin key encrypted user key (persistent) is found", async () => { + // Arrange + await stateProvider.setUserState( + PIN_KEY_ENCRYPTED_USER_KEY_PERSISTENT, + mockUserKeyEncryptedPin, + mockUserId, + ); + + // Act + const result = await sut.getPinLockType(mockUserId); + + // Assert + expect(result).toBe("PERSISTENT"); + }); + + it("should return 'EPHEMERAL' if only user key encrypted pin is found", async () => { + // Arrange + await stateProvider.setUserState(USER_KEY_ENCRYPTED_PIN, mockUserKeyEncryptedPin, mockUserId); + + // Act + const result = await sut.getPinLockType(mockUserId); + + // Assert + expect(result).toBe("EPHEMERAL"); + }); + + it("should return 'DISABLED' if no PIN-related state is found", async () => { + // Arrange - don't set any PIN-related state + + // Act + const result = await sut.getPinLockType(mockUserId); + + // Assert + expect(result).toBe("DISABLED"); + }); + + it("should return 'DISABLED' if all PIN-related state is null", async () => { + // Arrange - explicitly set all state to null + await stateProvider.setUserState( + PIN_PROTECTED_USER_KEY_ENVELOPE_PERSISTENT, + null, + mockUserId, + ); + await stateProvider.setUserState(PIN_KEY_ENCRYPTED_USER_KEY_PERSISTENT, null, mockUserId); + await stateProvider.setUserState(USER_KEY_ENCRYPTED_PIN, null, mockUserId); + + // Act + const result = await sut.getPinLockType(mockUserId); + + // Assert + expect(result).toBe("DISABLED"); + }); + }); + + describe("getPinProtectedUserKeyEnvelope()", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + test.each([ + [null, "PERSISTENT" as PinLockType], + [undefined, "PERSISTENT" as PinLockType], + [null, "EPHEMERAL" as PinLockType], + [undefined, "EPHEMERAL" as PinLockType], + [null, "DISABLED" as PinLockType], + [undefined, "DISABLED" as PinLockType], + ])("throws if userId is %p with pinLockType %s", async (userId, pinLockType: PinLockType) => { + // Using unnecesary switch so we can have exhaustive check on PinLockType + switch (pinLockType) { + case "PERSISTENT": + return await expect( + sut.getPinProtectedUserKeyEnvelope(userId as any, pinLockType), + ).rejects.toThrow("userId is null or undefined."); + case "EPHEMERAL": + return await expect( + sut.getPinProtectedUserKeyEnvelope(userId as any, pinLockType), + ).rejects.toThrow("userId is null or undefined."); + case "DISABLED": + return await expect( + sut.getPinProtectedUserKeyEnvelope(userId as any, pinLockType), + ).rejects.toThrow("userId is null or undefined."); + default: { + // This is the exhaustive check, will cause a compile error if a PinLockType is not handled above + const _exhaustiveCheck: never = pinLockType; + return _exhaustiveCheck; + } + } + }); + + it("should throw error for unsupported pinLockType", async () => { + // Act & Assert + await expect( + sut.getPinProtectedUserKeyEnvelope(mockUserId, "DISABLED" as any), + ).rejects.toThrow("Unsupported PinLockType: DISABLED"); + }); + + test.each([["PERSISTENT" as PinLockType], ["EPHEMERAL" as PinLockType]])( + "should return %s envelope when pinLockType is %s", + async (pinLockType: PinLockType) => { + // Arrange + const mockEnvelope = + pinLockType === "PERSISTENT" ? mockPersistentEnvelope : mockEphemeralEnvelope; + + await sut.setPinState(mockUserId, mockEnvelope, mockUserKeyEncryptedPin, pinLockType); + + // Act + const result = await sut.getPinProtectedUserKeyEnvelope(mockUserId, pinLockType); + + // Assert + expect(result).toBe(mockEnvelope); + }, + ); + + test.each([["PERSISTENT" as PinLockType], ["EPHEMERAL" as PinLockType]])( + "should return null when %s envelope is not set", + async (pinLockType: PinLockType) => { + // Arrange - don't set any state + + // Act + const result = await sut.getPinProtectedUserKeyEnvelope(mockUserId, pinLockType); + + // Assert + expect(result).toBeNull(); + }, + ); + + test.each([ + ["PERSISTENT" as PinLockType, PIN_PROTECTED_USER_KEY_ENVELOPE_PERSISTENT], + ["EPHEMERAL" as PinLockType, PIN_PROTECTED_USER_KEY_ENVELOPE_EPHEMERAL], + ])( + "should return null when %s envelope is explicitly set to null", + async (pinLockType, keyDefinition) => { + // Arrange + await stateProvider.setUserState(keyDefinition, null, mockUserId); + + // Act + const result = await sut.getPinProtectedUserKeyEnvelope(mockUserId, pinLockType); + + // Assert + expect(result).toBeNull(); + }, + ); + + it("should not cross-contaminate PERSISTENT and EPHEMERAL envelopes", async () => { + // Arrange - set both envelopes to different values + await sut.setPinState( + mockUserId, + mockPersistentEnvelope, + mockUserKeyEncryptedPin, + "PERSISTENT", + ); + await sut.setPinState( + mockUserId, + mockEphemeralEnvelope, + mockUserKeyEncryptedPin, + "EPHEMERAL", + ); + + // Act + const persistentResult = await sut.getPinProtectedUserKeyEnvelope(mockUserId, "PERSISTENT"); + const ephemeralResult = await sut.getPinProtectedUserKeyEnvelope(mockUserId, "EPHEMERAL"); + + // Assert + expect(persistentResult).toBe(mockPersistentEnvelope); + expect(ephemeralResult).toBe(mockEphemeralEnvelope); + expect(persistentResult).not.toBe(ephemeralResult); + }); + }); + + describe("getLegacyPinKeyEncryptedUserKeyPersistent()", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + test.each([null, undefined])("throws if userId is %p", async (userId) => { + // Act & Assert + await expect(() => + sut.getLegacyPinKeyEncryptedUserKeyPersistent(userId as any), + ).rejects.toThrow("userId is null or undefined."); + }); + + it("should return EncString when legacy key is set", async () => { + // Arrange + await stateProvider.setUserState( + PIN_KEY_ENCRYPTED_USER_KEY_PERSISTENT, + mockUserKeyEncryptedPin, + mockUserId, + ); + + // Act + const result = await sut.getLegacyPinKeyEncryptedUserKeyPersistent(mockUserId); + + // Assert + expect(result?.encryptedString).toEqual(mockUserKeyEncryptedPin); + }); + + test.each([null, undefined])("should return null when legacy key is %p", async (value) => { + // Arrange + await stateProvider.setUserState(PIN_KEY_ENCRYPTED_USER_KEY_PERSISTENT, value, mockUserId); + + // Act + const result = await sut.getLegacyPinKeyEncryptedUserKeyPersistent(mockUserId); + + // Assert + expect(result).toBeNull(); + }); + }); + + describe("setPinState()", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + test.each([[null], [undefined]])("throws if userId is %p", async (userId) => { + // Act & Assert + await expect( + sut.setPinState( + userId as any, + mockPersistentEnvelope, + mockUserKeyEncryptedPin, + "PERSISTENT", + ), + ).rejects.toThrow(`userId is null or undefined.`); + }); + + test.each([[null], [undefined]])( + "throws if pinProtectedUserKeyEnvelope is %p", + async (envelope) => { + // Act & Assert + await expect( + sut.setPinState(mockUserId, envelope as any, mockUserKeyEncryptedPin, "PERSISTENT"), + ).rejects.toThrow(`pinProtectedUserKeyEnvelope is null or undefined.`); + }, + ); + + test.each([[null], [undefined]])("throws if pinLockType is %p", async (pinLockType) => { + // Act & Assert + await expect( + sut.setPinState( + mockUserId, + mockPersistentEnvelope, + mockUserKeyEncryptedPin, + pinLockType as any, + ), + ).rejects.toThrow(`pinLockType is null or undefined.`); + }); + + it("should throw error for unsupported pinLockType", async () => { + // Act & Assert + await expect( + sut.setPinState( + mockUserId, + mockPersistentEnvelope, + mockUserKeyEncryptedPin, + "DISABLED" as PinLockType, + ), + ).rejects.toThrow("Cannot set up PIN with pin lock type DISABLED"); + }); + + test.each([["PERSISTENT" as PinLockType], ["EPHEMERAL" as PinLockType]])( + "should set %s PIN state correctly", + async (pinLockType: PinLockType) => { + // Arrange + const mockEnvelope = + pinLockType === "PERSISTENT" ? mockPersistentEnvelope : mockEphemeralEnvelope; + + // Act + await sut.setPinState(mockUserId, mockEnvelope, mockUserKeyEncryptedPin, pinLockType); + + // Assert - verify the correct envelope was set + const envelopeResult = await sut.getPinProtectedUserKeyEnvelope(mockUserId, pinLockType); + expect(envelopeResult).toBe(mockEnvelope); + + // Assert - verify the user key encrypted PIN was set + const pinResult = await firstValueFrom(sut.userKeyEncryptedPin$(mockUserId)); + expect(pinResult?.encryptedString).toEqual(mockUserKeyEncryptedPin); + }, + ); + }); + + describe("clearPinState", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + test.each([null, undefined])("throws if userId is %p", async (userId) => { + // Act & Assert + await expect(sut.clearPinState(userId as any)).rejects.toThrow( + `userId is null or undefined.`, + ); + }); + + it("clears UserKey encrypted PIN", async () => { + // Arrange + await sut.setPinState( + mockUserId, + mockPersistentEnvelope, + mockUserKeyEncryptedPin, + "PERSISTENT", + ); + + // Act + await sut.clearPinState(mockUserId); + + // Assert + const result = await firstValueFrom(sut.userKeyEncryptedPin$(mockUserId)); + expect(result).toBeNull(); + }); + + it("clears ephemeral PIN protected user key envelope", async () => { + // Arrange + await sut.setPinState( + mockUserId, + mockEphemeralEnvelope, + mockUserKeyEncryptedPin, + "EPHEMERAL", + ); + + // Act + await sut.clearPinState(mockUserId); + + // Assert + const result = await sut.getPinProtectedUserKeyEnvelope(mockUserId, "EPHEMERAL"); + expect(result).toBeNull(); + }); + + it("clears persistent PIN protected user key envelope", async () => { + // Arrange + await sut.setPinState( + mockUserId, + mockPersistentEnvelope, + mockUserKeyEncryptedPin, + "PERSISTENT", + ); + + // Act + await sut.clearPinState(mockUserId); + + // Assert + const result = await sut.getPinProtectedUserKeyEnvelope(mockUserId, "PERSISTENT"); + expect(result).toBeNull(); + }); + + it("clears legacy PIN key encrypted user key persistent", async () => { + // Arrange + await stateProvider.setUserState( + PIN_KEY_ENCRYPTED_USER_KEY_PERSISTENT, + mockUserKeyEncryptedPin, + mockUserId, + ); + + // Act + await sut.clearPinState(mockUserId); + + // Assert + const result = await sut.getLegacyPinKeyEncryptedUserKeyPersistent(mockUserId); + expect(result).toBeNull(); + }); + + it("clears all PIN state when all types are set", async () => { + // Arrange - set up all possible PIN state + await sut.setPinState( + mockUserId, + mockPersistentEnvelope, + mockUserKeyEncryptedPin, + "PERSISTENT", + ); + await sut.setPinState( + mockUserId, + mockEphemeralEnvelope, + mockUserKeyEncryptedPin, + "EPHEMERAL", + ); + await stateProvider.setUserState( + PIN_KEY_ENCRYPTED_USER_KEY_PERSISTENT, + mockUserKeyEncryptedPin, + mockUserId, + ); + + // Verify all state is set before clearing + expect(await firstValueFrom(sut.userKeyEncryptedPin$(mockUserId))).not.toBeNull(); + expect(await sut.getPinProtectedUserKeyEnvelope(mockUserId, "EPHEMERAL")).not.toBeNull(); + expect(await sut.getPinProtectedUserKeyEnvelope(mockUserId, "PERSISTENT")).not.toBeNull(); + expect(await sut.getLegacyPinKeyEncryptedUserKeyPersistent(mockUserId)).not.toBeNull(); + + // Act + await sut.clearPinState(mockUserId); + + // Assert - all PIN state should be cleared + expect(await firstValueFrom(sut.userKeyEncryptedPin$(mockUserId))).toBeNull(); + expect(await sut.getPinProtectedUserKeyEnvelope(mockUserId, "EPHEMERAL")).toBeNull(); + expect(await sut.getPinProtectedUserKeyEnvelope(mockUserId, "PERSISTENT")).toBeNull(); + expect(await sut.getLegacyPinKeyEncryptedUserKeyPersistent(mockUserId)).toBeNull(); + }); + + it("results in PIN lock type DISABLED after clearing", async () => { + // Arrange + await sut.setPinState( + mockUserId, + mockPersistentEnvelope, + mockUserKeyEncryptedPin, + "PERSISTENT", + ); + + // Verify PIN is set up before clearing + expect(await sut.getPinLockType(mockUserId)).toBe("PERSISTENT"); + + // Act + await sut.clearPinState(mockUserId); + + // Assert + expect(await sut.getPinLockType(mockUserId)).toBe("DISABLED"); + }); + + it("handles clearing when no PIN state exists", async () => { + // Arrange - no PIN state set up + + // Act & Assert - should not throw + await expect(sut.clearPinState(mockUserId)).resolves.not.toThrow(); + + // Verify state remains cleared + expect(await firstValueFrom(sut.userKeyEncryptedPin$(mockUserId))).toBeNull(); + expect(await sut.getPinProtectedUserKeyEnvelope(mockUserId, "EPHEMERAL")).toBeNull(); + expect(await sut.getPinProtectedUserKeyEnvelope(mockUserId, "PERSISTENT")).toBeNull(); + expect(await sut.getLegacyPinKeyEncryptedUserKeyPersistent(mockUserId)).toBeNull(); + expect(await sut.getPinLockType(mockUserId)).toBe("DISABLED"); + }); + }); + + describe("clearEphemeralPinState", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + test.each([null, undefined])("throws if userId is %p", async (userId) => { + // Act & Assert + await expect(sut.clearEphemeralPinState(userId as any)).rejects.toThrow( + `userId is null or undefined.`, + ); + }); + + it("clears only ephemeral PIN protected user key envelope", async () => { + // Arrange + await sut.setPinState( + mockUserId, + mockEphemeralEnvelope, + mockUserKeyEncryptedPin, + "EPHEMERAL", + ); + + // Act + await sut.clearEphemeralPinState(mockUserId); + + // Assert + const result = await sut.getPinProtectedUserKeyEnvelope(mockUserId, "EPHEMERAL"); + expect(result).toBeNull(); + }); + + it("does not clear user key encrypted PIN", async () => { + // Arrange + await sut.setPinState( + mockUserId, + mockEphemeralEnvelope, + mockUserKeyEncryptedPin, + "EPHEMERAL", + ); + + // Act + await sut.clearEphemeralPinState(mockUserId); + + // Assert - user key encrypted PIN should still be present + const pinResult = await firstValueFrom(sut.userKeyEncryptedPin$(mockUserId)); + expect(pinResult?.encryptedString).toEqual(mockUserKeyEncryptedPin); + }); + + it("does not clear persistent PIN protected user key envelope", async () => { + // Arrange - set up both ephemeral and persistent state + await sut.setPinState( + mockUserId, + mockEphemeralEnvelope, + mockUserKeyEncryptedPin, + "EPHEMERAL", + ); + await sut.setPinState( + mockUserId, + mockPersistentEnvelope, + mockUserKeyEncryptedPin, + "PERSISTENT", + ); + + // Act + await sut.clearEphemeralPinState(mockUserId); + + // Assert - persistent envelope should still be present + const persistentResult = await sut.getPinProtectedUserKeyEnvelope(mockUserId, "PERSISTENT"); + expect(persistentResult).toBe(mockPersistentEnvelope); + + // Assert - ephemeral envelope should be cleared + const ephemeralResult = await sut.getPinProtectedUserKeyEnvelope(mockUserId, "EPHEMERAL"); + expect(ephemeralResult).toBeNull(); + }); + + it("does not clear legacy PIN key encrypted user key persistent", async () => { + // Arrange - set up ephemeral state and legacy state + await sut.setPinState( + mockUserId, + mockEphemeralEnvelope, + mockUserKeyEncryptedPin, + "EPHEMERAL", + ); + await stateProvider.setUserState( + PIN_KEY_ENCRYPTED_USER_KEY_PERSISTENT, + mockUserKeyEncryptedPin, + mockUserId, + ); + + // Act + await sut.clearEphemeralPinState(mockUserId); + + // Assert - legacy PIN should still be present + const legacyResult = await sut.getLegacyPinKeyEncryptedUserKeyPersistent(mockUserId); + expect(legacyResult?.encryptedString).toEqual(mockUserKeyEncryptedPin); + + // Assert - ephemeral envelope should be cleared + const ephemeralResult = await sut.getPinProtectedUserKeyEnvelope(mockUserId, "EPHEMERAL"); + expect(ephemeralResult).toBeNull(); + }); + + it("changes PIN lock type from EPHEMERAL to DISABLED when no other PIN state exists", async () => { + // Arrange - set up only ephemeral PIN state + await sut.setPinState( + mockUserId, + mockEphemeralEnvelope, + mockUserKeyEncryptedPin, + "EPHEMERAL", + ); + + // Verify PIN lock type is EPHEMERAL before clearing + expect(await sut.getPinLockType(mockUserId)).toBe("EPHEMERAL"); + + // Act + await sut.clearEphemeralPinState(mockUserId); + + // Assert - PIN lock type should be DISABLED since no PIN envelope exists + // Note: USER_KEY_ENCRYPTED_PIN still exists but without an envelope, it's effectively disabled + expect(await sut.getPinLockType(mockUserId)).toBe("EPHEMERAL"); + }); + + it("keeps PIN lock type as PERSISTENT when persistent state remains", async () => { + // Arrange - set up both ephemeral and persistent state + await sut.setPinState( + mockUserId, + mockEphemeralEnvelope, + mockUserKeyEncryptedPin, + "EPHEMERAL", + ); + await sut.setPinState( + mockUserId, + mockPersistentEnvelope, + mockUserKeyEncryptedPin, + "PERSISTENT", + ); + + // Verify PIN lock type is PERSISTENT before clearing (persistent takes precedence) + expect(await sut.getPinLockType(mockUserId)).toBe("PERSISTENT"); + + // Act + await sut.clearEphemeralPinState(mockUserId); + + // Assert - PIN lock type should remain PERSISTENT + expect(await sut.getPinLockType(mockUserId)).toBe("PERSISTENT"); + }); + + it("handles clearing when no ephemeral PIN state exists", async () => { + // Arrange - no PIN state set up + + // Act & Assert - should not throw + await expect(sut.clearEphemeralPinState(mockUserId)).resolves.not.toThrow(); + + // Verify state remains cleared + expect(await sut.getPinProtectedUserKeyEnvelope(mockUserId, "EPHEMERAL")).toBeNull(); + }); + + it("handles clearing when only persistent PIN state exists", async () => { + // Arrange - set up only persistent state + await sut.setPinState( + mockUserId, + mockPersistentEnvelope, + mockUserKeyEncryptedPin, + "PERSISTENT", + ); + + // Act & Assert - should not throw + await expect(sut.clearEphemeralPinState(mockUserId)).resolves.not.toThrow(); + + // Assert - persistent state should remain unchanged + expect(await sut.getPinProtectedUserKeyEnvelope(mockUserId, "PERSISTENT")).toBe( + mockPersistentEnvelope, + ); + expect(await sut.getPinProtectedUserKeyEnvelope(mockUserId, "EPHEMERAL")).toBeNull(); + expect(await sut.getPinLockType(mockUserId)).toBe("PERSISTENT"); + }); + }); +}); diff --git a/libs/common/src/key-management/pin/pin.service.abstraction.ts b/libs/common/src/key-management/pin/pin.service.abstraction.ts index 33f369cc50f5..b242320c06ed 100644 --- a/libs/common/src/key-management/pin/pin.service.abstraction.ts +++ b/libs/common/src/key-management/pin/pin.service.abstraction.ts @@ -1,128 +1,78 @@ // eslint-disable-next-line no-restricted-imports import { KdfConfig } from "@bitwarden/key-management"; -import { EncString } from "../../key-management/crypto/models/enc-string"; import { UserId } from "../../types/guid"; import { PinKey, UserKey } from "../../types/key"; -import { PinLockType } from "./pin.service.implementation"; +import { PinLockType } from "./pin-lock-type"; /** - * The PinService is used for PIN-based unlocks. Below is a very basic overview of the PIN flow: + * The PinService provides PIN-based unlock functionality for user accounts. * - * -- Setting the PIN via {@link SetPinComponent} -- + * ## Overview * - * When the user submits the setPinForm: - - * 1. We encrypt the PIN with the UserKey and store it on disk as `userKeyEncryptedPin`. - * - * 2. We create a PinKey from the PIN, and then use that PinKey to encrypt the UserKey, resulting in - * a `pinKeyEncryptedUserKey`, which can be stored in one of two ways depending on what the user selects - * for the `requireMasterPasswordOnClientReset` checkbox. - * - * If `requireMasterPasswordOnClientReset` is: - * - TRUE, store in memory as `pinKeyEncryptedUserKeyEphemeral` (does NOT persist through a client reset) - * - FALSE, store on disk as `pinKeyEncryptedUserKeyPersistent` (persists through a client reset) - * - * -- Unlocking with the PIN via {@link LockComponent} -- - * - * When the user enters their PIN, we decrypt their UserKey with the PIN and set that UserKey to state. + * - The PIN is used to unlock the user's UserKey + * - PIN state and key material are managed using secure envelopes and encrypted state, with support for both ephemeral (in-memory) and persistent (on-disk) storage. + * When stored ephemerally, PIN unlock is only available after first unlock. When stored persistent, PIN unlock is available before first unlock. + * - The PIN is also stored, encrypted with the user's UserKey. After first unlock, the PIN can be retrieved. */ export abstract class PinServiceAbstraction { /** - * Gets the persistent (stored on disk) version of the UserKey, encrypted by the PinKey. + * Gets the user's PIN + * @throws If the user is locked + * @returns The user's PIN */ - abstract getPinKeyEncryptedUserKeyPersistent: (userId: UserId) => Promise; + abstract getPin(userId: UserId): Promise; /** - * Clears the persistent (stored on disk) version of the UserKey, encrypted by the PinKey. + * Setup pin unlock + * @throws If the provided user is locked */ - abstract clearPinKeyEncryptedUserKeyPersistent(userId: UserId): Promise; + abstract setPin(pin: string, pinLockType: PinLockType, userId: UserId): Promise; /** - * Gets the ephemeral (stored in memory) version of the UserKey, encrypted by the PinKey. + * Clear pin unlock */ - abstract getPinKeyEncryptedUserKeyEphemeral: (userId: UserId) => Promise; + abstract unsetPin(userId: UserId): Promise; /** - * Clears the ephemeral (stored in memory) version of the UserKey, encrypted by the PinKey. - */ - abstract clearPinKeyEncryptedUserKeyEphemeral(userId: UserId): Promise; - - /** - * Creates a pinKeyEncryptedUserKey from the provided PIN and UserKey. + * Gets the user's PinLockType {@link PinLockType}. */ - abstract createPinKeyEncryptedUserKey: ( - pin: string, - userKey: UserKey, - userId: UserId, - ) => Promise; + abstract getPinLockType(userId: UserId): Promise; /** - * Stores the UserKey, encrypted by the PinKey. - * @param storeEphemeralVersion If true, stores an ephemeral version via the private {@link setPinKeyEncryptedUserKeyEphemeral} method. - * If false, stores a persistent version via the private {@link setPinKeyEncryptedUserKeyPersistent} method. + * Declares whether or not the user has a PIN set (either persistent or ephemeral). + * Note: for ephemeral, this does not check if we actual have an ephemeral PIN-encrypted UserKey stored in memory. + * Decryption might not be possible even if this returns true. Use {@link isPinDecryptionAvailable} if decryption is required. */ - abstract storePinKeyEncryptedUserKey: ( - pinKeyEncryptedUserKey: EncString, - storeEphemeralVersion: boolean, - userId: UserId, - ) => Promise; + abstract isPinSet(userId: UserId): Promise; /** - * Gets the user's PIN, encrypted by the UserKey. + * Checks if PIN-encrypted keys are stored for the user. + * Used for unlock / user verification scenarios where we will need to decrypt the UserKey with the PIN. */ - abstract getUserKeyEncryptedPin: (userId: UserId) => Promise; + abstract isPinDecryptionAvailable(userId: UserId): Promise; /** - * Sets the user's PIN, encrypted by the UserKey. + * Clears ephemeral PINs for the user being logged out. */ - abstract setUserKeyEncryptedPin: ( - userKeyEncryptedPin: EncString, - userId: UserId, - ) => Promise; + abstract logout(userId: UserId): Promise; /** - * Creates a PIN, encrypted by the UserKey. + * Decrypts the UserKey with the provided PIN. + * @returns UserKey + * @throws If the pin lock type is ephemeral but the ephemeral pin protected user key envelope is not available */ - abstract createUserKeyEncryptedPin: (pin: string, userKey: UserKey) => Promise; + abstract decryptUserKeyWithPin(pin: string, userId: UserId): Promise; /** - * Clears the user's PIN, encrypted by the UserKey. + * @deprecated This is not deprecated, but only meant to be called by KeyService. DO NOT USE IT. */ - abstract clearUserKeyEncryptedPin(userId: UserId): Promise; + abstract userUnlocked(userId: UserId): Promise; /** * Makes a PinKey from the provided PIN. + * @deprecated - Note: This is currently re-used by vault exports, which is still permitted but should be refactored out to use a different construct. */ - abstract makePinKey: (pin: string, salt: string, kdfConfig: KdfConfig) => Promise; - - /** - * Gets the user's PinLockType {@link PinLockType}. - */ - abstract getPinLockType: (userId: UserId) => Promise; - - /** - * Declares whether or not the user has a PIN set (either persistent or ephemeral). - * Note: for ephemeral, this does not check if we actual have an ephemeral PIN-encrypted UserKey stored in memory. - * Decryption might not be possible even if this returns true. Use {@link isPinDecryptionAvailable} if decryption is required. - */ - abstract isPinSet: (userId: UserId) => Promise; - - /** - * Checks if PIN-encrypted keys are stored for the user. - * Used for unlock / user verification scenarios where we will need to decrypt the UserKey with the PIN. - */ - abstract isPinDecryptionAvailable: (userId: UserId) => Promise; - - /** - * Decrypts the UserKey with the provided PIN. - * - * @remarks - If the user has an old pinKeyEncryptedMasterKey (formerly called `pinProtected`), the UserKey - * will be obtained via the private {@link decryptAndMigrateOldPinKeyEncryptedMasterKey} method. - * - If the user does not have an old pinKeyEncryptedMasterKey, the UserKey will be obtained via the - * private {@link decryptUserKey} method. - * @returns UserKey - */ - abstract decryptUserKeyWithPin: (pin: string, userId: UserId) => Promise; + abstract makePinKey(pin: string, salt: string, kdfConfig: KdfConfig): Promise; } diff --git a/libs/common/src/key-management/pin/pin.service.implementation.ts b/libs/common/src/key-management/pin/pin.service.implementation.ts index da46cd3bc767..29a10d1a0a4d 100644 --- a/libs/common/src/key-management/pin/pin.service.implementation.ts +++ b/libs/common/src/key-management/pin/pin.service.implementation.ts @@ -1,275 +1,137 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { firstValueFrom, map } from "rxjs"; // eslint-disable-next-line no-restricted-imports -import { KdfConfig, KdfConfigService } from "@bitwarden/key-management"; +import { KdfConfig, KdfConfigService, KeyService } from "@bitwarden/key-management"; import { AccountService } from "../../auth/abstractions/account.service"; -import { CryptoFunctionService } from "../../key-management/crypto/abstractions/crypto-function.service"; +import { assertNonNullish } from "../../auth/utils"; import { EncryptService } from "../../key-management/crypto/abstractions/encrypt.service"; -import { EncString, EncryptedString } from "../../key-management/crypto/models/enc-string"; +import { EncString } from "../../key-management/crypto/models/enc-string"; import { LogService } from "../../platform/abstractions/log.service"; -import { PIN_DISK, PIN_MEMORY, StateProvider, UserKeyDefinition } from "../../platform/state"; +import { SdkService } from "../../platform/abstractions/sdk/sdk.service"; +import { SymmetricCryptoKey } from "../../platform/models/domain/symmetric-crypto-key"; import { UserId } from "../../types/guid"; import { PinKey, UserKey } from "../../types/key"; import { KeyGenerationService } from "../crypto"; +import { firstValueFromOrThrow } from "../utils"; +import { PinLockType } from "./pin-lock-type"; +import { PinStateServiceAbstraction } from "./pin-state.service.abstraction"; import { PinServiceAbstraction } from "./pin.service.abstraction"; -/** - * - DISABLED : No PIN set. - * - PERSISTENT : PIN is set and persists through client reset. - * - EPHEMERAL : PIN is set, but does NOT persist through client reset. This means that - * after client reset the master password is required to unlock. - */ -export type PinLockType = "DISABLED" | "PERSISTENT" | "EPHEMERAL"; - -/** - * The persistent (stored on disk) version of the UserKey, encrypted by the PinKey. - * - * @remarks Persists through a client reset. Used when `requireMasterPasswordOnClientRestart` is disabled. - * @see SetPinComponent.setPinForm.requireMasterPasswordOnClientRestart - */ -export const PIN_KEY_ENCRYPTED_USER_KEY_PERSISTENT = new UserKeyDefinition( - PIN_DISK, - "pinKeyEncryptedUserKeyPersistent", - { - deserializer: (jsonValue) => jsonValue, - clearOn: ["logout"], - }, -); - -/** - * The ephemeral (stored in memory) version of the UserKey, encrypted by the PinKey. - * - * @remarks Does NOT persist through a client reset. Used when `requireMasterPasswordOnClientRestart` is enabled. - * @see SetPinComponent.setPinForm.requireMasterPasswordOnClientRestart - */ -export const PIN_KEY_ENCRYPTED_USER_KEY_EPHEMERAL = new UserKeyDefinition( - PIN_MEMORY, - "pinKeyEncryptedUserKeyEphemeral", - { - deserializer: (jsonValue) => jsonValue, - clearOn: ["logout"], - }, -); - -/** - * The PIN, encrypted by the UserKey. - */ -export const USER_KEY_ENCRYPTED_PIN = new UserKeyDefinition( - PIN_DISK, - "userKeyEncryptedPin", - { - deserializer: (jsonValue) => jsonValue, - clearOn: ["logout"], - }, -); - export class PinService implements PinServiceAbstraction { constructor( private accountService: AccountService, - private cryptoFunctionService: CryptoFunctionService, private encryptService: EncryptService, private kdfConfigService: KdfConfigService, private keyGenerationService: KeyGenerationService, private logService: LogService, - private stateProvider: StateProvider, + private keyService: KeyService, + private sdkService: SdkService, + private pinStateService: PinStateServiceAbstraction, ) {} - async getPinKeyEncryptedUserKeyPersistent(userId: UserId): Promise { - this.validateUserId(userId, "Cannot get pinKeyEncryptedUserKeyPersistent."); - - return EncString.fromJSON( - await firstValueFrom( - this.stateProvider.getUserState$(PIN_KEY_ENCRYPTED_USER_KEY_PERSISTENT, userId), - ), - ); + getPinLockType(userId: UserId): Promise { + assertNonNullish(userId, "userId"); + return this.pinStateService.getPinLockType(userId); } - /** - * Sets the persistent (stored on disk) version of the UserKey, encrypted by the PinKey. - */ - private async setPinKeyEncryptedUserKeyPersistent( - pinKeyEncryptedUserKey: EncString, - userId: UserId, - ): Promise { - this.validateUserId(userId, "Cannot set pinKeyEncryptedUserKeyPersistent."); - - if (pinKeyEncryptedUserKey == null) { - throw new Error( - "No pinKeyEncryptedUserKey provided. Cannot set pinKeyEncryptedUserKeyPersistent.", - ); + async isPinSet(userId: UserId): Promise { + assertNonNullish(userId, "userId"); + return (await this.pinStateService.getPinLockType(userId)) !== "DISABLED"; + } + + async logout(userId: UserId): Promise { + assertNonNullish(userId, "userId"); + await this.pinStateService.clearPinState(userId); + } + + async userUnlocked(userId: UserId): Promise { + if ( + (await this.pinStateService.getPinLockType(userId)) === "EPHEMERAL" && + !(await this.isPinDecryptionAvailable(userId)) + ) { + this.logService.info("[Pin Service] On first unlock: Setting up ephemeral PIN"); + + // On first unlock, set the ephemeral pin envelope, if it is not set yet + const pin = await this.getPin(userId); + await this.setPin(pin, "EPHEMERAL", userId); + } else if ((await this.pinStateService.getPinLockType(userId)) === "PERSISTENT") { + // Encrypted migration for persistent pin unlock to pin envelopes. + // This will be removed at the earliest in 2026.1.0 + // + // ----- ENCRYPTION MIGRATION ----- + // Pin-key encrypted user-keys are eagerly migrated to the new pin-protected user key envelope format. + if ((await this.pinStateService.getLegacyPinKeyEncryptedUserKeyPersistent(userId)) != null) { + this.logService.info( + "[Pin Service] Migrating legacy PIN key to PinProtectedUserKeyEnvelope", + ); + const pin = await this.getPin(userId); + await this.setPin(pin, "PERSISTENT", userId); + } } - - await this.stateProvider.setUserState( - PIN_KEY_ENCRYPTED_USER_KEY_PERSISTENT, - pinKeyEncryptedUserKey?.encryptedString, - userId, - ); - } - - async clearPinKeyEncryptedUserKeyPersistent(userId: UserId): Promise { - this.validateUserId(userId, "Cannot clear pinKeyEncryptedUserKeyPersistent."); - - await this.stateProvider.setUserState(PIN_KEY_ENCRYPTED_USER_KEY_PERSISTENT, null, userId); } - async getPinKeyEncryptedUserKeyEphemeral(userId: UserId): Promise { - this.validateUserId(userId, "Cannot get pinKeyEncryptedUserKeyEphemeral."); + async getPin(userId: UserId): Promise { + assertNonNullish(userId, "userId"); - return EncString.fromJSON( - await firstValueFrom( - this.stateProvider.getUserState$(PIN_KEY_ENCRYPTED_USER_KEY_EPHEMERAL, userId), - ), + const userKey: UserKey = await firstValueFromOrThrow( + this.keyService.userKey$(userId), + "userKey", ); - } - - /** - * Sets the ephemeral (stored in memory) version of the UserKey, encrypted by the PinKey. - */ - private async setPinKeyEncryptedUserKeyEphemeral( - pinKeyEncryptedUserKey: EncString, - userId: UserId, - ): Promise { - this.validateUserId(userId, "Cannot set pinKeyEncryptedUserKeyEphemeral."); - - if (pinKeyEncryptedUserKey == null) { - throw new Error( - "No pinKeyEncryptedUserKey provided. Cannot set pinKeyEncryptedUserKeyEphemeral.", - ); - } - - await this.stateProvider.setUserState( - PIN_KEY_ENCRYPTED_USER_KEY_EPHEMERAL, - pinKeyEncryptedUserKey?.encryptedString, - userId, + const userKeyEncryptedPin = await firstValueFromOrThrow( + this.pinStateService.userKeyEncryptedPin$(userId), + "userKeyEncryptedPin", ); + return this.encryptService.decryptString(userKeyEncryptedPin, userKey); } - async clearPinKeyEncryptedUserKeyEphemeral(userId: UserId): Promise { - this.validateUserId(userId, "Cannot clear pinKeyEncryptedUserKeyEphemeral."); - - await this.stateProvider.setUserState(PIN_KEY_ENCRYPTED_USER_KEY_EPHEMERAL, null, userId); - } - - async createPinKeyEncryptedUserKey( - pin: string, - userKey: UserKey, - userId: UserId, - ): Promise { - this.validateUserId(userId, "Cannot create pinKeyEncryptedUserKey."); - - if (!userKey) { - throw new Error("No UserKey provided. Cannot create pinKeyEncryptedUserKey."); - } + async setPin(pin: string, pinLockType: PinLockType, userId: UserId): Promise { + assertNonNullish(pin, "pin"); + assertNonNullish(pinLockType, "pinLockType"); + assertNonNullish(userId, "userId"); - const email = await firstValueFrom( - this.accountService.accounts$.pipe(map((accounts) => accounts[userId].email)), - ); - const kdfConfig = await this.kdfConfigService.getKdfConfig(userId); - const pinKey = await this.makePinKey(pin, email, kdfConfig); - - return await this.encryptService.wrapSymmetricKey(userKey, pinKey); - } - - async storePinKeyEncryptedUserKey( - pinKeyEncryptedUserKey: EncString, - storeAsEphemeral: boolean, - userId: UserId, - ): Promise { - this.validateUserId(userId, "Cannot store pinKeyEncryptedUserKey."); - - if (storeAsEphemeral) { - await this.setPinKeyEncryptedUserKeyEphemeral(pinKeyEncryptedUserKey, userId); - } else { - await this.setPinKeyEncryptedUserKeyPersistent(pinKeyEncryptedUserKey, userId); - } - } - - async getUserKeyEncryptedPin(userId: UserId): Promise { - this.validateUserId(userId, "Cannot get userKeyEncryptedPin."); - - return EncString.fromJSON( - await firstValueFrom(this.stateProvider.getUserState$(USER_KEY_ENCRYPTED_PIN, userId)), + // Use the sdk to create an enrollment, not yet persisting it to state + const { pinProtectedUserKeyEnvelope, userKeyEncryptedPin } = await firstValueFrom( + this.sdkService.userClient$(userId).pipe( + map((sdk) => { + using ref = sdk.take(); + return ref.value.crypto().enroll_pin(pin); + }), + ), ); - } - - async setUserKeyEncryptedPin(userKeyEncryptedPin: EncString, userId: UserId): Promise { - this.validateUserId(userId, "Cannot set userKeyEncryptedPin."); - await this.stateProvider.setUserState( - USER_KEY_ENCRYPTED_PIN, - userKeyEncryptedPin?.encryptedString, + await this.pinStateService.setPinState( userId, + pinProtectedUserKeyEnvelope, + userKeyEncryptedPin, + pinLockType, ); } - async clearUserKeyEncryptedPin(userId: UserId): Promise { - this.validateUserId(userId, "Cannot clear userKeyEncryptedPin."); - - await this.stateProvider.setUserState(USER_KEY_ENCRYPTED_PIN, null, userId); - } - - async createUserKeyEncryptedPin(pin: string, userKey: UserKey): Promise { - if (!userKey) { - throw new Error("No UserKey provided. Cannot create userKeyEncryptedPin."); - } - - return await this.encryptService.encryptString(pin, userKey); - } - - async makePinKey(pin: string, salt: string, kdfConfig: KdfConfig): Promise { - const start = Date.now(); - const pinKey = await this.keyGenerationService.deriveKeyFromPassword(pin, salt, kdfConfig); - this.logService.info(`[Pin Service] deriving pin key took ${Date.now() - start}ms`); - - return (await this.keyGenerationService.stretchKey(pinKey)) as PinKey; - } - - async getPinLockType(userId: UserId): Promise { - this.validateUserId(userId, "Cannot get PinLockType."); - - const aUserKeyEncryptedPinIsSet = !!(await this.getUserKeyEncryptedPin(userId)); - const aPinKeyEncryptedUserKeyPersistentIsSet = - !!(await this.getPinKeyEncryptedUserKeyPersistent(userId)); - - if (aPinKeyEncryptedUserKeyPersistentIsSet) { - return "PERSISTENT"; - } else if (aUserKeyEncryptedPinIsSet && !aPinKeyEncryptedUserKeyPersistentIsSet) { - return "EPHEMERAL"; - } else { - return "DISABLED"; - } - } - - async isPinSet(userId: UserId): Promise { - this.validateUserId(userId, "Cannot determine if PIN is set."); - - return (await this.getPinLockType(userId)) !== "DISABLED"; + async unsetPin(userId: UserId): Promise { + assertNonNullish(userId, "userId"); + await this.pinStateService.clearPinState(userId); } async isPinDecryptionAvailable(userId: UserId): Promise { - this.validateUserId(userId, "Cannot determine if decryption of user key via PIN is available."); - - const pinLockType = await this.getPinLockType(userId); + assertNonNullish(userId, "userId"); + const pinLockType = await this.pinStateService.getPinLockType(userId); switch (pinLockType) { case "DISABLED": return false; case "PERSISTENT": - // The above getPinLockType call ensures that we have either a PinKeyEncryptedUserKey set. + // The above getPinLockType call ensures that we have either a PinKeyEncryptedUserKey or PinProtectedKeyEnvelope set. return true; case "EPHEMERAL": { // The above getPinLockType call ensures that we have a UserKeyEncryptedPin set. - // However, we must additively check to ensure that we have a set PinKeyEncryptedUserKeyEphemeral b/c otherwise - // we cannot take a PIN, derive a PIN key, and decrypt the ephemeral UserKey. - const pinKeyEncryptedUserKeyEphemeral = - await this.getPinKeyEncryptedUserKeyEphemeral(userId); - return Boolean(pinKeyEncryptedUserKeyEphemeral); + // However, we must additively check to ensure that we have a set PinKeyEncryptedUserKeyEphemeral, since + // this is only available after first unlock + const ephemeralPinProtectedKeyEnvelope = + await this.pinStateService.getPinProtectedUserKeyEnvelope(userId, "EPHEMERAL"); + return ephemeralPinProtectedKeyEnvelope != null; } - default: { // Compile-time check for exhaustive switch const _exhaustiveCheck: never = pinLockType; @@ -279,112 +141,89 @@ export class PinService implements PinServiceAbstraction { } async decryptUserKeyWithPin(pin: string, userId: UserId): Promise { - this.validateUserId(userId, "Cannot decrypt user key with PIN."); + assertNonNullish(pin, "pin"); + assertNonNullish(userId, "userId"); - try { - const pinLockType = await this.getPinLockType(userId); + const hasPinProtectedKeyEnvelopeSet = + (await this.pinStateService.getPinProtectedUserKeyEnvelope(userId, "EPHEMERAL")) != null || + (await this.pinStateService.getPinProtectedUserKeyEnvelope(userId, "PERSISTENT")) != null; - const pinKeyEncryptedUserKey = await this.getPinKeyEncryptedKeys(pinLockType, userId); + if (hasPinProtectedKeyEnvelopeSet) { + this.logService.info("[Pin Service] Pin-unlock via PinProtectedUserKeyEnvelope"); - const email = await firstValueFrom( - this.accountService.accounts$.pipe(map((accounts) => accounts[userId].email)), - ); - const kdfConfig = await this.kdfConfigService.getKdfConfig(userId); - - const userKey: UserKey = await this.decryptUserKey( + const pinLockType = await this.pinStateService.getPinLockType(userId); + const envelope = await this.pinStateService.getPinProtectedUserKeyEnvelope( userId, - pin, - email, - kdfConfig, - pinKeyEncryptedUserKey, + pinLockType, ); - if (!userKey) { - this.logService.warning(`User key null after pin key decryption.`); + + try { + // Use the sdk to create an enrollment, not yet persisting it to state + const startTime = performance.now(); + const userKeyBytes = await firstValueFrom( + this.sdkService.client$.pipe( + map((sdk) => { + if (!sdk) { + throw new Error("SDK not available"); + } + return sdk.crypto().unseal_password_protected_key_envelope(pin, envelope!); + }), + ), + ); + this.logService.measure(startTime, "Crypto", "PinService", "UnsealPinEnvelope"); + + return new SymmetricCryptoKey(userKeyBytes) as UserKey; + } catch (error) { + this.logService.error(`Failed to unseal pin: ${error}`); return null; } - - if (!(await this.validatePin(userKey, pin, userId))) { - this.logService.warning(`Pin key decryption successful but pin validation failed.`); + } else { + this.logService.info("[Pin Service] Pin-unlock via legacy PinKeyEncryptedUserKey"); + + // This branch is deprecated and will be removed in the future, but is kept for migration. + try { + const pinKeyEncryptedUserKey = + await this.pinStateService.getLegacyPinKeyEncryptedUserKeyPersistent(userId); + const email = await firstValueFrom( + this.accountService.accounts$.pipe(map((accounts) => accounts[userId].email)), + ); + const kdfConfig = await this.kdfConfigService.getKdfConfig(userId); + return await this.decryptUserKey(pin, email, kdfConfig, pinKeyEncryptedUserKey!); + } catch (error) { + this.logService.error(`Error decrypting user key with pin: ${error}`); return null; } - - return userKey; - } catch (error) { - this.logService.error(`Error decrypting user key with pin: ${error}`); - return null; } } + /// Anything below here is deprecated and will be removed subsequently + + async makePinKey(pin: string, salt: string, kdfConfig: KdfConfig): Promise { + const startTime = performance.now(); + const pinKey = await this.keyGenerationService.deriveKeyFromPassword(pin, salt, kdfConfig); + this.logService.measure(startTime, "Crypto", "PinService", "makePinKey"); + + return (await this.keyGenerationService.stretchKey(pinKey)) as PinKey; + } + /** * Decrypts the UserKey with the provided PIN. + * @deprecated + * @throws If the PIN does not match the PIN that was used to encrypt the user key + * @throws If the salt, or KDF don't match the salt / KDF used to encrypt the user key */ private async decryptUserKey( - userId: UserId, pin: string, salt: string, kdfConfig: KdfConfig, - pinKeyEncryptedUserKey?: EncString, + pinKeyEncryptedUserKey: EncString, ): Promise { - this.validateUserId(userId, "Cannot decrypt user key."); - - pinKeyEncryptedUserKey ||= await this.getPinKeyEncryptedUserKeyPersistent(userId); - pinKeyEncryptedUserKey ||= await this.getPinKeyEncryptedUserKeyEphemeral(userId); - - if (!pinKeyEncryptedUserKey) { - throw new Error("No pinKeyEncryptedUserKey found."); - } - + assertNonNullish(pin, "pin"); + assertNonNullish(salt, "salt"); + assertNonNullish(kdfConfig, "kdfConfig"); + assertNonNullish(pinKeyEncryptedUserKey, "pinKeyEncryptedUserKey"); const pinKey = await this.makePinKey(pin, salt, kdfConfig); const userKey = await this.encryptService.unwrapSymmetricKey(pinKeyEncryptedUserKey, pinKey); - return userKey as UserKey; } - - /** - * Gets the user's `pinKeyEncryptedUserKey` (persistent or ephemeral) - * (if one exists) based on the user's PinLockType. - * - * @throws If PinLockType is 'DISABLED' or if userId is not provided - */ - private async getPinKeyEncryptedKeys( - pinLockType: PinLockType, - userId: UserId, - ): Promise { - this.validateUserId(userId, "Cannot get PinKey encrypted keys."); - - switch (pinLockType) { - case "PERSISTENT": { - return await this.getPinKeyEncryptedUserKeyPersistent(userId); - } - case "EPHEMERAL": { - return await this.getPinKeyEncryptedUserKeyEphemeral(userId); - } - case "DISABLED": - throw new Error("Pin is disabled"); - default: { - // Compile-time check for exhaustive switch - const _exhaustiveCheck: never = pinLockType; - return _exhaustiveCheck; - } - } - } - - private async validatePin(userKey: UserKey, pin: string, userId: UserId): Promise { - this.validateUserId(userId, "Cannot validate PIN."); - - const userKeyEncryptedPin = await this.getUserKeyEncryptedPin(userId); - const decryptedPin = await this.encryptService.decryptString(userKeyEncryptedPin, userKey); - - const isPinValid = this.cryptoFunctionService.compareFast(decryptedPin, pin); - return isPinValid; - } - - /** - * Throws a custom error message if user ID is not provided. - */ - private validateUserId(userId: UserId, errorMessage: string = "") { - if (!userId) { - throw new Error(`User ID is required. ${errorMessage}`); - } - } } diff --git a/libs/common/src/key-management/pin/pin.service.spec.ts b/libs/common/src/key-management/pin/pin.service.spec.ts index b014c26c7dce..8d78255bb1b5 100644 --- a/libs/common/src/key-management/pin/pin.service.spec.ts +++ b/libs/common/src/key-management/pin/pin.service.spec.ts @@ -1,68 +1,64 @@ import { mock } from "jest-mock-extended"; +import { BehaviorSubject, filter } from "rxjs"; // eslint-disable-next-line no-restricted-imports -import { DEFAULT_KDF_CONFIG, KdfConfigService } from "@bitwarden/key-management"; +import { DEFAULT_KDF_CONFIG, KdfConfigService, KeyService } from "@bitwarden/key-management"; +import { PasswordProtectedKeyEnvelope } from "@bitwarden/sdk-internal"; -import { FakeAccountService, FakeStateProvider, mockAccountServiceWith } from "../../../spec"; +import { MockSdkService } from "../..//platform/spec/mock-sdk.service"; +import { FakeAccountService, mockAccountServiceWith, mockEnc } from "../../../spec"; import { LogService } from "../../platform/abstractions/log.service"; import { Utils } from "../../platform/misc/utils"; import { SymmetricCryptoKey } from "../../platform/models/domain/symmetric-crypto-key"; import { UserId } from "../../types/guid"; import { PinKey, UserKey } from "../../types/key"; import { KeyGenerationService } from "../crypto"; -import { CryptoFunctionService } from "../crypto/abstractions/crypto-function.service"; import { EncryptService } from "../crypto/abstractions/encrypt.service"; -import { EncString } from "../crypto/models/enc-string"; +import { EncryptedString, EncString } from "../crypto/models/enc-string"; -import { - PinService, - PIN_KEY_ENCRYPTED_USER_KEY_PERSISTENT, - PIN_KEY_ENCRYPTED_USER_KEY_EPHEMERAL, - USER_KEY_ENCRYPTED_PIN, - PinLockType, -} from "./pin.service.implementation"; +import { PinStateServiceAbstraction } from "./pin-state.service.abstraction"; +import { PinService } from "./pin.service.implementation"; describe("PinService", () => { let sut: PinService; let accountService: FakeAccountService; - let stateProvider: FakeStateProvider; - const cryptoFunctionService = mock(); const encryptService = mock(); const kdfConfigService = mock(); const keyGenerationService = mock(); const logService = mock(); - const mockUserId = Utils.newGuid() as UserId; - const mockUserKey = new SymmetricCryptoKey(randomBytes(64)) as UserKey; + const mockUserKey = new SymmetricCryptoKey(new Uint8Array(64)) as UserKey; const mockPinKey = new SymmetricCryptoKey(randomBytes(32)) as PinKey; const mockUserEmail = "user@example.com"; const mockPin = "1234"; const mockUserKeyEncryptedPin = new EncString("userKeyEncryptedPin"); - - // Note: both pinKeyEncryptedUserKeys use encryptionType: 2 (AesCbc256_HmacSha256_B64) - const pinKeyEncryptedUserKeyEphemeral = new EncString( - "2.gbauOANURUHqvhLTDnva1A==|nSW+fPumiuTaDB/s12+JO88uemV6rhwRSR+YR1ZzGr5j6Ei3/h+XEli2Unpz652NlZ9NTuRpHxeOqkYYJtp7J+lPMoclgteXuAzUu9kqlRc=|DeUFkhIwgkGdZA08bDnDqMMNmZk21D+H5g8IostPKAY=", - ); - const pinKeyEncryptedUserKeyPersistant = new EncString( - "2.fb5kOEZvh9zPABbP8WRmSQ==|Yi6ZAJY+UtqCKMUSqp1ahY9Kf8QuneKXs6BMkpNsakLVOzTYkHHlilyGABMF7GzUO8QHyZi7V/Ovjjg+Naf3Sm8qNhxtDhibITv4k8rDnM0=|TFkq3h2VNTT1z5BFbebm37WYuxyEHXuRo0DZJI7TQnw=", - ); + const mockEphemeralEnvelope = "mock-ephemeral-envelope" as PasswordProtectedKeyEnvelope; + const mockPersistentEnvelope = "mock-persistent-envelope" as PasswordProtectedKeyEnvelope; + const keyService = mock(); + const sdkService = new MockSdkService(); + const pinStateService = mock(); + const behaviorSubject = new BehaviorSubject<{ userId: UserId; userKey: UserKey }>(null); beforeEach(() => { - jest.clearAllMocks(); - accountService = mockAccountServiceWith(mockUserId, { email: mockUserEmail }); - stateProvider = new FakeStateProvider(accountService); + (keyService as any)["unlockedUserKeys$"] = behaviorSubject + .asObservable() + .pipe(filter((x) => x != null)); + sdkService.client.crypto + .mockDeep() + .unseal_password_protected_key_envelope.mockReturnValue(new Uint8Array(64)); sut = new PinService( accountService, - cryptoFunctionService, encryptService, kdfConfigService, keyGenerationService, logService, - stateProvider, + keyService, + sdkService, + pinStateService, ); }); @@ -70,267 +66,282 @@ describe("PinService", () => { expect(sut).not.toBeFalsy(); }); - describe("userId validation", () => { - it("should throw an error if a userId is not provided", async () => { - await expect(sut.getPinKeyEncryptedUserKeyPersistent(undefined)).rejects.toThrow( - "User ID is required. Cannot get pinKeyEncryptedUserKeyPersistent.", - ); - await expect(sut.getPinKeyEncryptedUserKeyEphemeral(undefined)).rejects.toThrow( - "User ID is required. Cannot get pinKeyEncryptedUserKeyEphemeral.", - ); - await expect(sut.clearPinKeyEncryptedUserKeyPersistent(undefined)).rejects.toThrow( - "User ID is required. Cannot clear pinKeyEncryptedUserKeyPersistent.", - ); - await expect(sut.clearPinKeyEncryptedUserKeyEphemeral(undefined)).rejects.toThrow( - "User ID is required. Cannot clear pinKeyEncryptedUserKeyEphemeral.", - ); - await expect( - sut.createPinKeyEncryptedUserKey(mockPin, mockUserKey, undefined), - ).rejects.toThrow("User ID is required. Cannot create pinKeyEncryptedUserKey."); - await expect(sut.getUserKeyEncryptedPin(undefined)).rejects.toThrow( - "User ID is required. Cannot get userKeyEncryptedPin.", - ); - await expect(sut.setUserKeyEncryptedPin(mockUserKeyEncryptedPin, undefined)).rejects.toThrow( - "User ID is required. Cannot set userKeyEncryptedPin.", - ); - await expect(sut.clearUserKeyEncryptedPin(undefined)).rejects.toThrow( - "User ID is required. Cannot clear userKeyEncryptedPin.", - ); - await expect( - sut.createPinKeyEncryptedUserKey(mockPin, mockUserKey, undefined), - ).rejects.toThrow("User ID is required. Cannot create pinKeyEncryptedUserKey."); - await expect(sut.getPinLockType(undefined)).rejects.toThrow("Cannot get PinLockType."); - await expect(sut.isPinSet(undefined)).rejects.toThrow( - "User ID is required. Cannot determine if PIN is set.", - ); + describe("userUnlocked()", () => { + beforeEach(() => { + jest.clearAllMocks(); }); - }); - describe("get/clear/create/store pinKeyEncryptedUserKey methods", () => { - describe("getPinKeyEncryptedUserKeyPersistent()", () => { - it("should get the pinKeyEncryptedUserKey of the specified userId", async () => { - await sut.getPinKeyEncryptedUserKeyPersistent(mockUserId); + it("should set up ephemeral PIN on first unlock if needed", async () => { + // Arrange + pinStateService.getPinLockType.mockResolvedValue("EPHEMERAL"); + jest.spyOn(sut, "isPinDecryptionAvailable").mockResolvedValue(false); + const getPinSpy = jest.spyOn(sut, "getPin").mockResolvedValue(mockPin); + const setPinSpy = jest.spyOn(sut, "setPin").mockResolvedValue(); + + // Act + await sut.userUnlocked(mockUserId); - expect(stateProvider.mock.getUserState$).toHaveBeenCalledWith( - PIN_KEY_ENCRYPTED_USER_KEY_PERSISTENT, - mockUserId, - ); - }); + // Assert + expect(getPinSpy).toHaveBeenCalledWith(mockUserId); + expect(setPinSpy).toHaveBeenCalledWith(mockPin, "EPHEMERAL", mockUserId); + expect(logService.info).toHaveBeenCalledWith( + "[Pin Service] On first unlock: Setting up ephemeral PIN", + ); }); - describe("clearPinKeyEncryptedUserKeyPersistent()", () => { - it("should clear the pinKeyEncryptedUserKey of the specified userId", async () => { - await sut.clearPinKeyEncryptedUserKeyPersistent(mockUserId); + it("should migrate legacy persistent PIN if needed", async () => { + // Arrange + pinStateService.getPinLockType.mockResolvedValue("PERSISTENT"); + pinStateService.getLegacyPinKeyEncryptedUserKeyPersistent.mockResolvedValue( + mockEnc("legacy-key"), + ); + const getPinSpy = jest.spyOn(sut, "getPin").mockResolvedValue(mockPin); + const setPinSpy = jest.spyOn(sut, "setPin").mockResolvedValue(); + + // Act + await sut.userUnlocked(mockUserId); - expect(stateProvider.mock.setUserState).toHaveBeenCalledWith( - PIN_KEY_ENCRYPTED_USER_KEY_PERSISTENT, - null, - mockUserId, - ); - }); + // Assert + expect(getPinSpy).toHaveBeenCalledWith(mockUserId); + expect(setPinSpy).toHaveBeenCalledWith(mockPin, "PERSISTENT", mockUserId); + expect(logService.info).toHaveBeenCalledWith( + "[Pin Service] Migrating legacy PIN key to PinProtectedUserKeyEnvelope", + ); }); - describe("getPinKeyEncryptedUserKeyEphemeral()", () => { - it("should get the pinKeyEncrypterUserKeyEphemeral of the specified userId", async () => { - await sut.getPinKeyEncryptedUserKeyEphemeral(mockUserId); + it("should do nothing if no migration or setup is needed", async () => { + // Arrange + pinStateService.getPinLockType.mockResolvedValue("DISABLED"); + const getPinSpy = jest.spyOn(sut, "getPin"); + const setPinSpy = jest.spyOn(sut, "setPin"); + + // Act + await sut.userUnlocked(mockUserId); - expect(stateProvider.mock.getUserState$).toHaveBeenCalledWith( - PIN_KEY_ENCRYPTED_USER_KEY_EPHEMERAL, - mockUserId, - ); - }); + // Assert + expect(getPinSpy).not.toHaveBeenCalled(); + expect(setPinSpy).not.toHaveBeenCalled(); }); + }); + + describe("makePinKey()", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it("should make a PinKey", async () => { + // Arrange + keyGenerationService.deriveKeyFromPassword.mockResolvedValue(mockPinKey); - describe("clearPinKeyEncryptedUserKeyEphemeral()", () => { - it("should clear the pinKeyEncryptedUserKey of the specified userId", async () => { - await sut.clearPinKeyEncryptedUserKeyEphemeral(mockUserId); + // Act + await sut.makePinKey(mockPin, mockUserEmail, DEFAULT_KDF_CONFIG); - expect(stateProvider.mock.setUserState).toHaveBeenCalledWith( - PIN_KEY_ENCRYPTED_USER_KEY_EPHEMERAL, - null, - mockUserId, - ); - }); + // Assert + expect(keyGenerationService.deriveKeyFromPassword).toHaveBeenCalledWith( + mockPin, + mockUserEmail, + DEFAULT_KDF_CONFIG, + ); + expect(keyGenerationService.stretchKey).toHaveBeenCalledWith(mockPinKey); }); + }); - describe("createPinKeyEncryptedUserKey()", () => { - it("should throw an error if a userKey is not provided", async () => { - await expect( - sut.createPinKeyEncryptedUserKey(mockPin, undefined, mockUserId), - ).rejects.toThrow("No UserKey provided. Cannot create pinKeyEncryptedUserKey."); - }); + describe("getPin()", () => { + beforeEach(() => { + jest.clearAllMocks(); + keyService.userKey$.mockReturnValue(new BehaviorSubject(mockUserKey).asObservable()); + }); - it("should create a pinKeyEncryptedUserKey", async () => { - // Arrange - sut.makePinKey = jest.fn().mockResolvedValue(mockPinKey); + it("should successfully decrypt and return the PIN", async () => { + const expectedPin = "1234"; + pinStateService.userKeyEncryptedPin$.mockReturnValue( + new BehaviorSubject(mockUserKeyEncryptedPin).asObservable(), + ); + encryptService.decryptString.mockResolvedValue(expectedPin); - // Act - await sut.createPinKeyEncryptedUserKey(mockPin, mockUserKey, mockUserId); + const result = await sut.getPin(mockUserId); - // Assert - expect(encryptService.wrapSymmetricKey).toHaveBeenCalledWith(mockUserKey, mockPinKey); - }); + expect(result).toBe(expectedPin); + expect(encryptService.decryptString).toHaveBeenCalledWith( + mockUserKeyEncryptedPin, + mockUserKey, + ); }); - describe("storePinKeyEncryptedUserKey", () => { - it("should store a pinKeyEncryptedUserKey (persistent version) when 'storeAsEphemeral' is false", async () => { - // Arrange - const storeAsEphemeral = false; + it("should throw an error if userId is null", async () => { + await expect(sut.getPin(null as any)).rejects.toThrow("userId"); + }); - // Act - await sut.storePinKeyEncryptedUserKey( - pinKeyEncryptedUserKeyPersistant, - storeAsEphemeral, - mockUserId, - ); + it("should throw an error if userKey is not available", async () => { + keyService.userKey$.mockReturnValue(new BehaviorSubject(null).asObservable()); + await expect(sut.getPin(mockUserId)).rejects.toThrow("userKey"); + }); + }); - // Assert - expect(stateProvider.mock.setUserState).toHaveBeenCalledWith( - PIN_KEY_ENCRYPTED_USER_KEY_PERSISTENT, - pinKeyEncryptedUserKeyPersistant.encryptedString, - mockUserId, - ); - }); - - it("should store a pinKeyEncryptedUserKeyEphemeral when 'storeAsEphemeral' is true", async () => { - // Arrange - const storeAsEphemeral = true; + describe("unsetPin()", () => { + beforeEach(async () => { + jest.clearAllMocks(); + }); - // Act - await sut.storePinKeyEncryptedUserKey( - pinKeyEncryptedUserKeyEphemeral, - storeAsEphemeral, - mockUserId, - ); + it("should throw an error if userId is null", async () => { + await expect(sut.unsetPin(null as any)).rejects.toThrow("userId"); + }); - // Assert - expect(stateProvider.mock.setUserState).toHaveBeenCalledWith( - PIN_KEY_ENCRYPTED_USER_KEY_EPHEMERAL, - pinKeyEncryptedUserKeyEphemeral.encryptedString, - mockUserId, - ); - }); + it("should call pinStateService.clearPinState with the correct userId", async () => { + await sut.unsetPin(mockUserId); + expect(pinStateService.clearPinState).toHaveBeenCalledWith(mockUserId); }); }); - describe("userKeyEncryptedPin methods", () => { - describe("getUserKeyEncryptedPin()", () => { - it("should get the userKeyEncryptedPin of the specified userId", async () => { - await sut.getUserKeyEncryptedPin(mockUserId); + describe("setPin()", () => { + const mockPinProtectedUserKeyEnvelope = "mock-envelope" as PasswordProtectedKeyEnvelope; + const mockUserKeyEncryptedPinFromSdk = "sdk-encrypted-pin"; - expect(stateProvider.mock.getUserState$).toHaveBeenCalledWith( - USER_KEY_ENCRYPTED_PIN, - mockUserId, - ); - }); + beforeEach(() => {}); + + it("should throw an error if pin is null", async () => { + // Act & Assert + await expect(sut.setPin(null as any, "EPHEMERAL", mockUserId)).rejects.toThrow("pin"); }); - describe("setUserKeyEncryptedPin()", () => { - it("should set the userKeyEncryptedPin of the specified userId", async () => { - await sut.setUserKeyEncryptedPin(mockUserKeyEncryptedPin, mockUserId); + it("should throw an error if pinLockType is null", async () => { + // Act & Assert + await expect(sut.setPin(mockPin, null as any, mockUserId)).rejects.toThrow("pinLockType"); + }); - expect(stateProvider.mock.setUserState).toHaveBeenCalledWith( - USER_KEY_ENCRYPTED_PIN, - mockUserKeyEncryptedPin.encryptedString, - mockUserId, - ); - }); + it("should throw an error if userId is null", async () => { + // Act & Assert + await expect(sut.setPin(mockPin, "EPHEMERAL", null as any)).rejects.toThrow("userId"); }); - describe("clearUserKeyEncryptedPin()", () => { - it("should clear the pinKeyEncryptedUserKey of the specified userId", async () => { - await sut.clearUserKeyEncryptedPin(mockUserId); + it("should successfully set an EPHEMERAL pin", async () => { + sdkService.simulate + .userLogin(mockUserId) + .crypto.mockDeep() + .enroll_pin.mockReturnValue({ + pinProtectedUserKeyEnvelope: mockPinProtectedUserKeyEnvelope, + userKeyEncryptedPin: mockUserKeyEncryptedPinFromSdk as EncryptedString, + }); + + await sut.setPin(mockPin, "EPHEMERAL", mockUserId); - expect(stateProvider.mock.setUserState).toHaveBeenCalledWith( - USER_KEY_ENCRYPTED_PIN, - null, - mockUserId, - ); - }); + expect(pinStateService.setPinState).toHaveBeenCalledWith( + mockUserId, + mockPinProtectedUserKeyEnvelope, + mockUserKeyEncryptedPinFromSdk, + "EPHEMERAL", + ); }); - describe("createUserKeyEncryptedPin()", () => { - it("should throw an error if a userKey is not provided", async () => { - await expect(sut.createUserKeyEncryptedPin(mockPin, undefined)).rejects.toThrow( - "No UserKey provided. Cannot create userKeyEncryptedPin.", - ); - }); + it("should successfully set a PERSISTENT pin", async () => { + sdkService.simulate + .userLogin(mockUserId) + .crypto.mockDeep() + .enroll_pin.mockReturnValue({ + pinProtectedUserKeyEnvelope: mockPinProtectedUserKeyEnvelope, + userKeyEncryptedPin: mockUserKeyEncryptedPinFromSdk as EncryptedString, + }); - it("should create a userKeyEncryptedPin from the provided PIN and userKey", async () => { - encryptService.encryptString.mockResolvedValue(mockUserKeyEncryptedPin); + await sut.setPin(mockPin, "PERSISTENT", mockUserId); - const result = await sut.createUserKeyEncryptedPin(mockPin, mockUserKey); + expect(pinStateService.setPinState).toHaveBeenCalledWith( + mockUserId, + mockPinProtectedUserKeyEnvelope, + mockUserKeyEncryptedPinFromSdk, + "PERSISTENT", + ); + }); + }); - expect(encryptService.encryptString).toHaveBeenCalledWith(mockPin, mockUserKey); - expect(result).toEqual(mockUserKeyEncryptedPin); - }); + describe("getPinLockType()", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it("should call pinStateService.getPinLockType with the correct userId", async () => { + pinStateService.getPinLockType.mockResolvedValue("EPHEMERAL"); + const result = await sut.getPinLockType(mockUserId); + expect(pinStateService.getPinLockType).toHaveBeenCalledWith(mockUserId); + expect(result).toBe("EPHEMERAL"); }); }); - describe("makePinKey()", () => { - it("should make a PinKey", async () => { - // Arrange - keyGenerationService.deriveKeyFromPassword.mockResolvedValue(mockPinKey); + describe("isPinDecryptionAvailable()", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it("should throw an error if userId is null", async () => { + // Act & Assert + await expect(sut.isPinDecryptionAvailable(null as any)).rejects.toThrow("userId"); + }); + + it("should return false if pinLockType is DISABLED", async () => { + // Arrange - don't set any PIN-related state (will result in DISABLED) // Act - await sut.makePinKey(mockPin, mockUserEmail, DEFAULT_KDF_CONFIG); + const result = await sut.isPinDecryptionAvailable(mockUserId); // Assert - expect(keyGenerationService.deriveKeyFromPassword).toHaveBeenCalledWith( - mockPin, - mockUserEmail, - DEFAULT_KDF_CONFIG, - ); - expect(keyGenerationService.stretchKey).toHaveBeenCalledWith(mockPinKey); + expect(result).toBe(false); }); - }); - describe("getPinLockType()", () => { - it("should return 'PERSISTENT' if a pinKeyEncryptedUserKey (persistent version) is found", async () => { - // Arrange - sut.getUserKeyEncryptedPin = jest.fn().mockResolvedValue(null); - sut.getPinKeyEncryptedUserKeyPersistent = jest - .fn() - .mockResolvedValue(pinKeyEncryptedUserKeyPersistant); + it("should return true if pinLockType is PERSISTENT", async () => { + // Arrange - mock lock type + pinStateService.getPinLockType.mockResolvedValue("PERSISTENT"); // Act - const result = await sut.getPinLockType(mockUserId); + const result = await sut.isPinDecryptionAvailable(mockUserId); // Assert - expect(result).toBe("PERSISTENT"); + expect(result).toBe(true); }); - it("should return 'EPHEMERAL' if a pinKeyEncryptedUserKey (persistent version) is not found but a userKeyEncryptedPin is found", async () => { - // Arrange - sut.getUserKeyEncryptedPin = jest.fn().mockResolvedValue(mockUserKeyEncryptedPin); - sut.getPinKeyEncryptedUserKeyPersistent = jest.fn().mockResolvedValue(null); + it("should return true if pinLockType is EPHEMERAL and ephemeral envelope is available", async () => { + // Arrange - mock lock type and set ephemeral envelope + pinStateService.getPinLockType.mockResolvedValue("EPHEMERAL"); + pinStateService.getPinProtectedUserKeyEnvelope.mockResolvedValue(mockEphemeralEnvelope); // Act - const result = await sut.getPinLockType(mockUserId); + const result = await sut.isPinDecryptionAvailable(mockUserId); // Assert - expect(result).toBe("EPHEMERAL"); + expect(result).toBe(true); }); - it("should return 'DISABLED' if both of these are NOT found: userKeyEncryptedPin, pinKeyEncryptedUserKey (persistent version)", async () => { - // Arrange - sut.getUserKeyEncryptedPin = jest.fn().mockResolvedValue(null); - sut.getPinKeyEncryptedUserKeyPersistent = jest.fn().mockResolvedValue(null); + it("should return false if pinLockType is EPHEMERAL but ephemeral envelope is not available", async () => { + // Arrange - set only user key encrypted pin (EPHEMERAL) but no ephemeral envelope + pinStateService.getPinLockType.mockResolvedValue("EPHEMERAL"); + pinStateService.getPinProtectedUserKeyEnvelope.mockResolvedValue(null); // Act - const result = await sut.getPinLockType(mockUserId); + const result = await sut.isPinDecryptionAvailable(mockUserId); // Assert - expect(result).toBe("DISABLED"); + expect(result).toBe(false); + }); + + it("should handle unexpected pinLockType and throw error", async () => { + // Arrange - mock getPinLockType to return an unexpected value + pinStateService.getPinLockType.mockResolvedValue("UNKNOWN" as any); + + // Act & Assert + await expect(sut.isPinDecryptionAvailable(mockUserId)).rejects.toThrow( + "Unexpected pinLockType: UNKNOWN", + ); }); }); describe("isPinSet()", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + it.each(["PERSISTENT", "EPHEMERAL"])( "should return true if the user PinLockType is '%s'", async () => { // Arrange - sut.getPinLockType = jest.fn().mockResolvedValue("PERSISTENT"); + pinStateService.getPinLockType.mockResolvedValue("PERSISTENT"); // Act const result = await sut.isPinSet(mockUserId); @@ -342,7 +353,7 @@ describe("PinService", () => { it("should return false if the user PinLockType is 'DISABLED'", async () => { // Arrange - sut.getPinLockType = jest.fn().mockResolvedValue("DISABLED"); + pinStateService.getPinLockType.mockResolvedValue("DISABLED"); // Act const result = await sut.isPinSet(mockUserId); @@ -352,162 +363,87 @@ describe("PinService", () => { }); }); - describe("isPinDecryptionAvailable()", () => { - it("should return false if pinLockType is DISABLED", async () => { - // Arrange - sut.getPinLockType = jest.fn().mockResolvedValue("DISABLED"); + describe("logout", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); - // Act - const result = await sut.isPinDecryptionAvailable(mockUserId); + it("should throw when userId is null", async () => { + await expect(sut.logout(null as any)).rejects.toThrow("userId"); + }); - // Assert - expect(result).toBe(false); + it("should call pinStateService.clearPinState", async () => { + await sut.logout(mockUserId); + expect(pinStateService.clearPinState).toHaveBeenCalledWith(mockUserId); }); + }); - it("should return true if pinLockType is PERSISTENT", async () => { - // Arrange - sut.getPinLockType = jest.fn().mockResolvedValue("PERSISTENT"); + describe("decryptUserKeyWithPin", () => { + beforeEach(() => { + jest.clearAllMocks(); + pinStateService.userKeyEncryptedPin$.mockReset(); + pinStateService.getPinProtectedUserKeyEnvelope.mockReset(); + pinStateService.getLegacyPinKeyEncryptedUserKeyPersistent.mockReset(); + }); - // Act - const result = await sut.isPinDecryptionAvailable(mockUserId); + it("should throw an error if userId is null", async () => { + await expect(sut.decryptUserKeyWithPin("1234", null as any)).rejects.toThrow("userId"); + }); - // Assert - expect(result).toBe(true); + it("should throw an error if pin is null", async () => { + await expect(sut.decryptUserKeyWithPin(null as any, mockUserId)).rejects.toThrow("pin"); }); - it("should return true if pinLockType is EPHEMERAL and we have an ephemeral PIN key encrypted user key", async () => { + it("should return userkey with new pin EPHEMERAL", async () => { // Arrange - sut.getPinLockType = jest.fn().mockResolvedValue("EPHEMERAL"); - sut.getPinKeyEncryptedUserKeyEphemeral = jest - .fn() - .mockResolvedValue(pinKeyEncryptedUserKeyEphemeral); + const mockPin = "1234"; + pinStateService.userKeyEncryptedPin$.mockReturnValueOnce( + new BehaviorSubject(mockUserKeyEncryptedPin), + ); + pinStateService.getPinProtectedUserKeyEnvelope.mockResolvedValueOnce(mockEphemeralEnvelope); // Act - const result = await sut.isPinDecryptionAvailable(mockUserId); + const result = await sut.decryptUserKeyWithPin(mockPin, mockUserId); // Assert - expect(result).toBe(true); + expect(result).toEqual(mockUserKey); }); - it("should return false if pinLockType is EPHEMERAL and we do not have an ephemeral PIN key encrypted user key", async () => { + it("should return userkey with new pin PERSISTENT", async () => { // Arrange - sut.getPinLockType = jest.fn().mockResolvedValue("EPHEMERAL"); - sut.getPinKeyEncryptedUserKeyEphemeral = jest.fn().mockResolvedValue(null); + const mockPin = "1234"; + pinStateService.userKeyEncryptedPin$.mockReturnValueOnce( + new BehaviorSubject(mockUserKeyEncryptedPin), + ); + pinStateService.getPinProtectedUserKeyEnvelope.mockResolvedValueOnce(mockPersistentEnvelope); // Act - const result = await sut.isPinDecryptionAvailable(mockUserId); + const result = await sut.decryptUserKeyWithPin(mockPin, mockUserId); // Assert - expect(result).toBe(false); - }); - - it("should throw an error if an unexpected pinLockType is returned", async () => { - // Arrange - sut.getPinLockType = jest.fn().mockResolvedValue("UNKNOWN"); - - // Act & Assert - await expect(sut.isPinDecryptionAvailable(mockUserId)).rejects.toThrow( - "Unexpected pinLockType: UNKNOWN", - ); + expect(result).toEqual(mockUserKey); }); - }); - - describe("decryptUserKeyWithPin()", () => { - async function setupDecryptUserKeyWithPinMocks(pinLockType: PinLockType) { - sut.getPinLockType = jest.fn().mockResolvedValue(pinLockType); - - mockPinEncryptedKeyDataByPinLockType(pinLockType); + it("should return userkey with legacy pin PERSISTENT", async () => { + keyGenerationService.deriveKeyFromPassword.mockResolvedValue(mockPinKey); + keyGenerationService.stretchKey.mockResolvedValue(mockPinKey); kdfConfigService.getKdfConfig.mockResolvedValue(DEFAULT_KDF_CONFIG); - - mockDecryptUserKeyFn(); - - sut.getUserKeyEncryptedPin = jest.fn().mockResolvedValue(mockUserKeyEncryptedPin); - encryptService.decryptString.mockResolvedValue(mockPin); - cryptoFunctionService.compareFast.calledWith(mockPin, "1234").mockResolvedValue(true); - } - - function mockDecryptUserKeyFn() { - sut.getPinKeyEncryptedUserKeyPersistent = jest - .fn() - .mockResolvedValue(pinKeyEncryptedUserKeyPersistant); - sut.makePinKey = jest.fn().mockResolvedValue(mockPinKey); encryptService.unwrapSymmetricKey.mockResolvedValue(mockUserKey); - } - - function mockPinEncryptedKeyDataByPinLockType(pinLockType: PinLockType) { - switch (pinLockType) { - case "PERSISTENT": - sut.getPinKeyEncryptedUserKeyPersistent = jest - .fn() - .mockResolvedValue(pinKeyEncryptedUserKeyPersistant); - break; - case "EPHEMERAL": - sut.getPinKeyEncryptedUserKeyEphemeral = jest - .fn() - .mockResolvedValue(pinKeyEncryptedUserKeyEphemeral); - - break; - case "DISABLED": - // no mocking required. Error should be thrown - break; - } - } - - const testCases: { pinLockType: PinLockType }[] = [ - { pinLockType: "PERSISTENT" }, - { pinLockType: "EPHEMERAL" }, - ]; - - testCases.forEach(({ pinLockType }) => { - describe(`given a ${pinLockType} PIN)`, () => { - it(`should successfully decrypt and return user key when using a valid PIN`, async () => { - // Arrange - await setupDecryptUserKeyWithPinMocks(pinLockType); - - // Act - const result = await sut.decryptUserKeyWithPin(mockPin, mockUserId); - - // Assert - expect(result).toEqual(mockUserKey); - }); - it(`should return null when PIN is incorrect and user key cannot be decrypted`, async () => { - // Arrange - await setupDecryptUserKeyWithPinMocks(pinLockType); - sut.decryptUserKeyWithPin = jest.fn().mockResolvedValue(null); - - // Act - const result = await sut.decryptUserKeyWithPin(mockPin, mockUserId); - - // Assert - expect(result).toBeNull(); - }); - - // not sure if this is a realistic scenario but going to test it anyway - it(`should return null when PIN doesn't match after successful user key decryption`, async () => { - // Arrange - await setupDecryptUserKeyWithPinMocks(pinLockType); - encryptService.decryptString.mockResolvedValue("9999"); // non matching PIN - - // Act - const result = await sut.decryptUserKeyWithPin(mockPin, mockUserId); - - // Assert - expect(result).toBeNull(); - }); - }); - }); - - it(`should return null when pin is disabled`, async () => { // Arrange - await setupDecryptUserKeyWithPinMocks("DISABLED"); + const mockPin = "1234"; + pinStateService.userKeyEncryptedPin$.mockReturnValueOnce( + new BehaviorSubject(mockUserKeyEncryptedPin), + ); + pinStateService.getLegacyPinKeyEncryptedUserKeyPersistent.mockResolvedValueOnce( + mockUserKeyEncryptedPin, + ); // Act const result = await sut.decryptUserKeyWithPin(mockPin, mockUserId); // Assert - expect(result).toBeNull(); + expect(result).toEqual(mockUserKey); }); }); }); diff --git a/libs/common/src/key-management/pin/pin.state.ts b/libs/common/src/key-management/pin/pin.state.ts new file mode 100644 index 000000000000..4ad0524035f5 --- /dev/null +++ b/libs/common/src/key-management/pin/pin.state.ts @@ -0,0 +1,61 @@ +import { PIN_DISK, PIN_MEMORY, UserKeyDefinition } from "@bitwarden/common/platform/state"; +import { PasswordProtectedKeyEnvelope } from "@bitwarden/sdk-internal"; + +import { EncryptedString } from "../crypto/models/enc-string"; + +/** + * The persistent (stored on disk) version of the UserKey, encrypted by the PinKey. + * + * @deprecated + * @remarks Persists through a client reset. Used when `requireMasterPasswordOnClientRestart` is disabled. + * @see SetPinComponent.setPinForm.requireMasterPasswordOnClientRestart + */ +export const PIN_KEY_ENCRYPTED_USER_KEY_PERSISTENT = new UserKeyDefinition( + PIN_DISK, + "pinKeyEncryptedUserKeyPersistent", + { + deserializer: (jsonValue) => jsonValue, + clearOn: ["logout"], + }, +); + +/** + * The persistent (stored on disk) version of the UserKey, stored in a `PasswordProtectedKeyEnvelope`. + * + * @remarks Persists through a client reset. Used when `requireMasterPasswordOnClientRestart` is disabled. + * @see SetPinComponent.setPinForm.requireMasterPasswordOnClientRestart + */ +export const PIN_PROTECTED_USER_KEY_ENVELOPE_PERSISTENT = + new UserKeyDefinition( + PIN_DISK, + "pinProtectedUserKeyEnvelopePersistent", + { + deserializer: (jsonValue) => jsonValue, + clearOn: ["logout"], + }, + ); + +/** + * The ephemeral (stored in memory) version of the UserKey, stored in a `PasswordProtectedKeyEnvelope`. + */ +export const PIN_PROTECTED_USER_KEY_ENVELOPE_EPHEMERAL = + new UserKeyDefinition( + PIN_MEMORY, + "pinProtectedUserKeyEnvelopeEphemeral", + { + deserializer: (jsonValue) => jsonValue, + clearOn: ["logout"], + }, + ); + +/** + * The PIN, encrypted by the UserKey. + */ +export const USER_KEY_ENCRYPTED_PIN = new UserKeyDefinition( + PIN_DISK, + "userKeyEncryptedPin", + { + deserializer: (jsonValue) => jsonValue, + clearOn: ["logout"], + }, +); diff --git a/libs/common/src/key-management/services/default-process-reload.service.ts b/libs/common/src/key-management/services/default-process-reload.service.ts index bc5739167ce2..24bf81abcdfd 100644 --- a/libs/common/src/key-management/services/default-process-reload.service.ts +++ b/libs/common/src/key-management/services/default-process-reload.service.ts @@ -58,8 +58,7 @@ export class DefaultProcessReloadService implements ProcessReloadServiceAbstract // If there is an active user, check if they have a pinKeyEncryptedUserKeyEphemeral. If so, prevent process reload upon lock. const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; if (userId != null) { - const ephemeralPin = await this.pinService.getPinKeyEncryptedUserKeyEphemeral(userId); - if (ephemeralPin != null) { + if ((await this.pinService.getPinLockType(userId)) === "EPHEMERAL") { this.logService.info( "[Process Reload Service] Ephemeral pin active, preventing process reload", ); diff --git a/libs/common/src/key-management/vault-timeout/abstractions/vault-timeout-settings.service.ts b/libs/common/src/key-management/vault-timeout/abstractions/vault-timeout-settings.service.ts index bcbf0029199e..697b8a1875c8 100644 --- a/libs/common/src/key-management/vault-timeout/abstractions/vault-timeout-settings.service.ts +++ b/libs/common/src/key-management/vault-timeout/abstractions/vault-timeout-settings.service.ts @@ -56,6 +56,4 @@ export abstract class VaultTimeoutSettingsService { * @returns boolean true if biometric lock is set */ abstract isBiometricLockSet(userId?: string): Promise; - - abstract clear(userId: UserId): Promise; } diff --git a/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.spec.ts b/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.spec.ts index 93067111bed5..f3fec6d849ac 100644 --- a/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.spec.ts +++ b/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.spec.ts @@ -20,7 +20,7 @@ import { TokenService } from "../../../auth/services/token.service"; import { LogService } from "../../../platform/abstractions/log.service"; import { Utils } from "../../../platform/misc/utils"; import { UserId } from "../../../types/guid"; -import { PinServiceAbstraction } from "../../pin/pin.service.abstraction"; +import { PinStateServiceAbstraction } from "../../pin/pin-state.service.abstraction"; import { VaultTimeoutSettingsService as VaultTimeoutSettingsServiceAbstraction } from "../abstractions/vault-timeout-settings.service"; import { VaultTimeoutAction } from "../enums/vault-timeout-action.enum"; import { VaultTimeout, VaultTimeoutStringType } from "../types/vault-timeout.type"; @@ -30,7 +30,7 @@ import { VAULT_TIMEOUT, VAULT_TIMEOUT_ACTION } from "./vault-timeout-settings.st describe("VaultTimeoutSettingsService", () => { let accountService: FakeAccountService; - let pinService: MockProxy; + let pinStateService: MockProxy; let userDecryptionOptionsService: MockProxy; let keyService: MockProxy; let tokenService: MockProxy; @@ -46,7 +46,7 @@ describe("VaultTimeoutSettingsService", () => { beforeEach(() => { accountService = mockAccountServiceWith(mockUserId); - pinService = mock(); + pinStateService = mock(); userDecryptionOptionsService = mock(); keyService = mock(); tokenService = mock(); @@ -96,7 +96,7 @@ describe("VaultTimeoutSettingsService", () => { }); it("contains Lock when the user has either a persistent or ephemeral PIN configured", async () => { - pinService.isPinSet.mockResolvedValue(true); + pinStateService.isPinSet.mockResolvedValue(true); const result = await firstValueFrom( vaultTimeoutSettingsService.availableVaultTimeoutActions$(), @@ -118,7 +118,7 @@ describe("VaultTimeoutSettingsService", () => { it("not contains Lock when the user does not have a master password, PIN, or biometrics", async () => { userDecryptionOptionsSubject.next(new UserDecryptionOptions({ hasMasterPassword: false })); - pinService.isPinSet.mockResolvedValue(false); + pinStateService.isPinSet.mockResolvedValue(false); biometricStateService.biometricUnlockEnabled$ = of(false); const result = await firstValueFrom( @@ -212,7 +212,7 @@ describe("VaultTimeoutSettingsService", () => { "returns $expected when policy is $policy, has PIN unlock method: $hasPinUnlock or Biometric unlock method: $hasBiometricUnlock, and user preference is $userPreference", async ({ hasPinUnlock, hasBiometricUnlock, policy, userPreference, expected }) => { biometricStateService.getBiometricUnlockEnabled.mockResolvedValue(hasBiometricUnlock); - pinService.isPinSet.mockResolvedValue(hasPinUnlock); + pinStateService.isPinSet.mockResolvedValue(hasPinUnlock); userDecryptionOptionsSubject.next( new UserDecryptionOptions({ hasMasterPassword: false }), @@ -377,7 +377,7 @@ describe("VaultTimeoutSettingsService", () => { ): VaultTimeoutSettingsService { return new VaultTimeoutSettingsService( accountService, - pinService, + pinStateService, userDecryptionOptionsService, keyService, tokenService, diff --git a/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.ts b/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.ts index e40b896dc8c0..4ca23bb24bf0 100644 --- a/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.ts +++ b/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.ts @@ -30,7 +30,7 @@ import { TokenService } from "../../../auth/abstractions/token.service"; import { LogService } from "../../../platform/abstractions/log.service"; import { StateProvider } from "../../../platform/state"; import { UserId } from "../../../types/guid"; -import { PinServiceAbstraction } from "../../pin/pin.service.abstraction"; +import { PinStateServiceAbstraction } from "../../pin/pin-state.service.abstraction"; import { VaultTimeoutSettingsService as VaultTimeoutSettingsServiceAbstraction } from "../abstractions/vault-timeout-settings.service"; import { VaultTimeoutAction } from "../enums/vault-timeout-action.enum"; import { VaultTimeout, VaultTimeoutStringType } from "../types/vault-timeout.type"; @@ -40,7 +40,7 @@ import { VAULT_TIMEOUT, VAULT_TIMEOUT_ACTION } from "./vault-timeout-settings.st export class VaultTimeoutSettingsService implements VaultTimeoutSettingsServiceAbstraction { constructor( private accountService: AccountService, - private pinService: PinServiceAbstraction, + private pinStateService: PinStateServiceAbstraction, private userDecryptionOptionsService: UserDecryptionOptionsServiceAbstraction, private keyService: KeyService, private tokenService: TokenService, @@ -279,7 +279,7 @@ export class VaultTimeoutSettingsService implements VaultTimeoutSettingsServiceA const canLock = (await this.userHasMasterPassword(userId)) || - (await this.pinService.isPinSet(userId as UserId)) || + (await this.pinStateService.isPinSet(userId as UserId)) || (await this.isBiometricLockSet(userId)); if (canLock) { @@ -289,10 +289,6 @@ export class VaultTimeoutSettingsService implements VaultTimeoutSettingsServiceA return availableActions; } - async clear(userId: UserId): Promise { - await this.keyService.clearPinKeys(userId); - } - private async userHasMasterPassword(userId: string): Promise { if (userId) { const decryptionOptions = await firstValueFrom( diff --git a/libs/common/src/platform/services/sdk/default-sdk.service.spec.ts b/libs/common/src/platform/services/sdk/default-sdk.service.spec.ts index 4aee0d48e5a6..c78e6370ffb8 100644 --- a/libs/common/src/platform/services/sdk/default-sdk.service.spec.ts +++ b/libs/common/src/platform/services/sdk/default-sdk.service.spec.ts @@ -247,8 +247,8 @@ function createMockClient(): MockProxy { client.crypto.mockReturnValue(mock()); client.platform.mockReturnValue({ state: jest.fn().mockReturnValue(mock()), + load_flags: jest.fn().mockReturnValue(mock()), free: mock(), - load_flags: jest.fn(), }); return client; } diff --git a/libs/common/src/types/key.ts b/libs/common/src/types/key.ts index ca56deb2fb12..46dcc14a8b27 100644 --- a/libs/common/src/types/key.ts +++ b/libs/common/src/types/key.ts @@ -9,6 +9,7 @@ export type PrfKey = Opaque; export type UserKey = Opaque; /** @deprecated Interacting with the master key directly is prohibited. Use a high level function from MasterPasswordService instead. */ export type MasterKey = Opaque; +/** @deprecated */ export type PinKey = Opaque; export type OrgKey = Opaque; export type ProviderKey = Opaque; 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 e7550e34b9f1..7fcb050f34ed 100644 --- a/libs/key-management-ui/src/lock/components/lock.component.ts +++ b/libs/key-management-ui/src/lock/components/lock.component.ts @@ -622,6 +622,7 @@ export class LockComponent implements OnInit, OnDestroy { this.logService.mark("Vault unlocked"); await this.keyService.setUserKey(key, this.activeAccount.id); + await this.pinService.userUnlocked(this.activeAccount.id); // Now that we have a decrypted user key in memory, we can check if we // need to establish trust on the current device diff --git a/libs/key-management/src/abstractions/key.service.ts b/libs/key-management/src/abstractions/key.service.ts index e4bb83cb2fd5..abd4dcc15631 100644 --- a/libs/key-management/src/abstractions/key.service.ts +++ b/libs/key-management/src/abstractions/key.service.ts @@ -142,11 +142,10 @@ export abstract class KeyService { abstract makeUserKeyV1(): Promise; /** * Clears the user's stored version of the user key - * @param keySuffix The desired version of the key to clear * @param userId The desired user * @throws Error when userId is null or undefined. */ - abstract clearStoredUserKey(keySuffix: KeySuffixOptions, userId: string): Promise; + abstract clearStoredUserKey(userId: string): Promise; /** * Retrieves the user's master key if it is in state, or derives it from the provided password * @param password The user's master password that will be used to derive a master key if one isn't found @@ -228,6 +227,7 @@ export abstract class KeyService { * @deprecated Use {@link orgKeys$} with a required {@link UserId} instead. */ abstract activeUserOrgKeys$: Observable>; + /** * Returns the organization's symmetric key * @deprecated Use the observable userOrgKeys$ and `map` to the desired {@link OrgKey} instead @@ -345,14 +345,6 @@ export abstract class KeyService { * @throws If the provided key is a null-ish value. */ abstract makeKeyPair(key: SymmetricCryptoKey): Promise<[string, EncString]>; - /** - * Clears the user's pin keys from storage - * Note: This will remove the stored pin and as a result, - * disable pin protection for the user - * @param userId The desired user - * @throws Error when provided userId is null or undefined - */ - abstract clearPinKeys(userId: UserId): Promise; /** * @param keyMaterial The key material to derive the send key from * @returns A new send key diff --git a/libs/key-management/src/key.service.spec.ts b/libs/key-management/src/key.service.spec.ts index 46d1125711b3..0dd9f3603f5d 100644 --- a/libs/key-management/src/key.service.spec.ts +++ b/libs/key-management/src/key.service.spec.ts @@ -10,7 +10,6 @@ import { EncryptedString, } from "@bitwarden/common/key-management/crypto/models/enc-string"; import { FakeMasterPasswordService } from "@bitwarden/common/key-management/master-password/services/fake-master-password.service"; -import { PinServiceAbstraction } from "@bitwarden/common/key-management/pin/pin.service.abstraction"; import { UnsignedPublicKey, WrappedSigningKey } from "@bitwarden/common/key-management/types"; import { VaultTimeoutStringType } from "@bitwarden/common/key-management/vault-timeout"; import { VAULT_TIMEOUT } from "@bitwarden/common/key-management/vault-timeout/services/vault-timeout-settings.state"; @@ -57,7 +56,6 @@ import { KdfConfig } from "./models/kdf-config"; describe("keyService", () => { let keyService: DefaultKeyService; - const pinService = mock(); const keyGenerationService = mock(); const cryptoFunctionService = mock(); const encryptService = mock(); @@ -77,7 +75,6 @@ describe("keyService", () => { stateProvider = new FakeStateProvider(accountService); keyService = new DefaultKeyService( - pinService, masterPasswordService, keyGenerationService, cryptoFunctionService, @@ -256,54 +253,6 @@ describe("keyService", () => { "No userId provided.", ); }); - - describe("Pin Key refresh", () => { - const mockPinKeyEncryptedUserKey = new EncString( - "2.AAAw2vTUePO+CCyokcIfVw==|DTBNlJ5yVsV2Bsk3UU3H6Q==|YvFBff5gxWqM+UsFB6BKimKxhC32AtjF3IStpU1Ijwg=", - ); - const mockUserKeyEncryptedPin = new EncString( - "2.BBBw2vTUePO+CCyokcIfVw==|DTBNlJ5yVsV2Bsk3UU3H6Q==|YvFBff5gxWqM+UsFB6BKimKxhC32AtjF3IStpU1Ijwg=", - ); - - it("sets a pinKeyEncryptedUserKeyPersistent if a userKeyEncryptedPin and pinKeyEncryptedUserKey is set", async () => { - pinService.createPinKeyEncryptedUserKey.mockResolvedValue(mockPinKeyEncryptedUserKey); - pinService.getUserKeyEncryptedPin.mockResolvedValue(mockUserKeyEncryptedPin); - pinService.getPinKeyEncryptedUserKeyPersistent.mockResolvedValue( - mockPinKeyEncryptedUserKey, - ); - - await keyService.setUserKey(mockUserKey, mockUserId); - - expect(pinService.storePinKeyEncryptedUserKey).toHaveBeenCalledWith( - mockPinKeyEncryptedUserKey, - false, - mockUserId, - ); - }); - - it("sets a pinKeyEncryptedUserKeyEphemeral if a userKeyEncryptedPin is set, but a pinKeyEncryptedUserKey is not set", async () => { - pinService.createPinKeyEncryptedUserKey.mockResolvedValue(mockPinKeyEncryptedUserKey); - pinService.getUserKeyEncryptedPin.mockResolvedValue(mockUserKeyEncryptedPin); - pinService.getPinKeyEncryptedUserKeyPersistent.mockResolvedValue(null); - - await keyService.setUserKey(mockUserKey, mockUserId); - - expect(pinService.storePinKeyEncryptedUserKey).toHaveBeenCalledWith( - mockPinKeyEncryptedUserKey, - true, - mockUserId, - ); - }); - - it("clears the pinKeyEncryptedUserKeyPersistent and pinKeyEncryptedUserKeyEphemeral if the UserKeyEncryptedPin is not set", async () => { - pinService.getUserKeyEncryptedPin.mockResolvedValue(null); - - await keyService.setUserKey(mockUserKey, mockUserId); - - expect(pinService.clearPinKeyEncryptedUserKeyPersistent).toHaveBeenCalledWith(mockUserId); - expect(pinService.clearPinKeyEncryptedUserKeyEphemeral).toHaveBeenCalledWith(mockUserId); - }); - }); }); describe("setUserKeys", () => { @@ -388,36 +337,22 @@ describe("keyService", () => { const invalidUserIdTestCases = [ { keySuffix: KeySuffixOptions.Auto, userId: null as unknown as UserId }, { keySuffix: KeySuffixOptions.Auto, userId: undefined as unknown as UserId }, - { keySuffix: KeySuffixOptions.Pin, userId: null as unknown as UserId }, - { keySuffix: KeySuffixOptions.Pin, userId: undefined as unknown as UserId }, ]; test.each(invalidUserIdTestCases)( "throws when keySuffix is $keySuffix and userId is $userId", async ({ keySuffix, userId }) => { - await expect(keyService.clearStoredUserKey(keySuffix, userId)).rejects.toThrow( - "UserId is required", - ); + await expect(keyService.clearStoredUserKey(userId)).rejects.toThrow("UserId is required"); }, ); }); describe("with Auto key suffix", () => { it("UserKeyAutoUnlock is cleared and pin keys are not cleared", async () => { - await keyService.clearStoredUserKey(KeySuffixOptions.Auto, mockUserId); + await keyService.clearStoredUserKey(mockUserId); expect(stateService.setUserKeyAutoUnlock).toHaveBeenCalledWith(null, { userId: mockUserId, }); - expect(pinService.clearPinKeyEncryptedUserKeyEphemeral).not.toHaveBeenCalled(); - }); - }); - - describe("with PIN key suffix", () => { - it("pin keys are cleared and user key auto unlock not", async () => { - await keyService.clearStoredUserKey(KeySuffixOptions.Pin, mockUserId); - - expect(stateService.setUserKeyAutoUnlock).not.toHaveBeenCalled(); - expect(pinService.clearPinKeyEncryptedUserKeyEphemeral).toHaveBeenCalledWith(mockUserId); }); }); }); @@ -448,24 +383,6 @@ describe("keyService", () => { }); }); - describe("clearPinKeys", () => { - test.each([null as unknown as UserId, undefined as unknown as UserId])( - "throws when the provided userId is %s", - async (userId) => { - await expect(keyService.clearPinKeys(userId)).rejects.toThrow("UserId is required"); - }, - ); - it("calls pin service to clear", async () => { - const userId = "someOtherUser" as UserId; - - await keyService.clearPinKeys(userId); - - expect(pinService.clearPinKeyEncryptedUserKeyPersistent).toHaveBeenCalledWith(userId); - expect(pinService.clearPinKeyEncryptedUserKeyEphemeral).toHaveBeenCalledWith(userId); - expect(pinService.clearUserKeyEncryptedPin).toHaveBeenCalledWith(userId); - }); - }); - describe("userPrivateKey$", () => { let mockUserKey: UserKey; let mockUserPrivateKey: Uint8Array; @@ -1262,7 +1179,6 @@ describe("keyService", () => { expect(result).toEqual(mockUserKey); expect(validateUserKeySpy).toHaveBeenCalledWith(mockUserKey, mockUserId); expect(logService.warning).toHaveBeenCalledWith("Invalid key, throwing away stored keys"); - expect(pinService.clearPinKeyEncryptedUserKeyEphemeral).toHaveBeenCalledWith(mockUserId); expect(stateService.setUserKeyAutoUnlock).toHaveBeenCalledWith(null, { userId: mockUserId, }); diff --git a/libs/key-management/src/key.service.ts b/libs/key-management/src/key.service.ts index a13c74e96d2f..fc3404101245 100644 --- a/libs/key-management/src/key.service.ts +++ b/libs/key-management/src/key.service.ts @@ -27,7 +27,6 @@ import { EncryptedString, } from "@bitwarden/common/key-management/crypto/models/enc-string"; 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 { WrappedSigningKey } from "@bitwarden/common/key-management/types"; import { VaultTimeoutStringType } from "@bitwarden/common/key-management/vault-timeout"; import { VAULT_TIMEOUT } from "@bitwarden/common/key-management/vault-timeout/services/vault-timeout-settings.state"; @@ -72,7 +71,6 @@ export class DefaultKeyService implements KeyServiceAbstraction { readonly activeUserOrgKeys$: Observable>; constructor( - protected pinService: PinServiceAbstraction, protected masterPasswordService: InternalMasterPasswordServiceAbstraction, protected keyGenerationService: KeyGenerationService, protected cryptoFunctionService: CryptoFunctionService, @@ -105,6 +103,13 @@ export class DefaultKeyService implements KeyServiceAbstraction { await this.stateProvider.setUserState(USER_EVER_HAD_USER_KEY, true, userId); await this.storeAdditionalKeys(key, userId); + + // Await the key actually being set. This ensures that any subsequent callers know the key is already in state. + // There were bugs related to the stateprovider observables in the past that caused issues around this. + const userKey = await firstValueFrom(this.userKey$(userId).pipe(filter((k) => k != null))); + if (userKey == null) { + throw new Error("Failed to set user key"); + } } async setUserKeys( @@ -223,17 +228,12 @@ export class DefaultKeyService implements KeyServiceAbstraction { await this.clearAllStoredUserKeys(userId); } - async clearStoredUserKey(keySuffix: KeySuffixOptions, userId: UserId): Promise { + async clearStoredUserKey(userId: UserId): Promise { if (userId == null) { throw new Error("UserId is required"); } - if (keySuffix === KeySuffixOptions.Auto) { - await this.stateService.setUserKeyAutoUnlock(null, { userId: userId }); - } - if (keySuffix === KeySuffixOptions.Pin) { - await this.pinService.clearPinKeyEncryptedUserKeyEphemeral(userId); - } + await this.stateService.setUserKeyAutoUnlock(null, { userId: userId }); } /** @@ -513,16 +513,6 @@ export class DefaultKeyService implements KeyServiceAbstraction { await this.stateProvider.setUserState(USER_KEY_ENCRYPTED_SIGNING_KEY, null, userId); } - async clearPinKeys(userId: UserId): Promise { - if (userId == null) { - throw new Error("UserId is required"); - } - - await this.pinService.clearPinKeyEncryptedUserKeyPersistent(userId); - await this.pinService.clearPinKeyEncryptedUserKeyEphemeral(userId); - await this.pinService.clearUserKeyEncryptedPin(userId); - } - async makeSendKey(keyMaterial: CsprngArray): Promise { return await this.keyGenerationService.deriveKeyFromMaterial( keyMaterial, @@ -546,7 +536,6 @@ export class DefaultKeyService implements KeyServiceAbstraction { await this.clearProviderKeys(userId); await this.clearKeyPair(userId); await this.clearSigningKey(userId); - await this.clearPinKeys(userId); await this.stateProvider.setUserState(USER_EVER_HAD_USER_KEY, null, userId); } @@ -678,32 +667,6 @@ export class DefaultKeyService implements KeyServiceAbstraction { } else { await this.stateService.setUserKeyAutoUnlock(null, { userId: userId }); } - - const storePin = await this.shouldStoreKey(KeySuffixOptions.Pin, userId); - if (storePin) { - // Decrypt userKeyEncryptedPin with user key - const pin = await this.encryptService.decryptString( - (await this.pinService.getUserKeyEncryptedPin(userId))!, - key, - ); - - const pinKeyEncryptedUserKey = await this.pinService.createPinKeyEncryptedUserKey( - pin, - key, - userId, - ); - const noPreExistingPersistentKey = - (await this.pinService.getPinKeyEncryptedUserKeyPersistent(userId)) == null; - - await this.pinService.storePinKeyEncryptedUserKey( - pinKeyEncryptedUserKey, - noPreExistingPersistentKey, - userId, - ); - } else { - await this.pinService.clearPinKeyEncryptedUserKeyPersistent(userId); - await this.pinService.clearPinKeyEncryptedUserKeyEphemeral(userId); - } } protected async shouldStoreKey(keySuffix: KeySuffixOptions, userId: UserId) { @@ -720,11 +683,6 @@ export class DefaultKeyService implements KeyServiceAbstraction { shouldStoreKey = vaultTimeout == VaultTimeoutStringType.Never; break; } - case KeySuffixOptions.Pin: { - const userKeyEncryptedPin = await this.pinService.getUserKeyEncryptedPin(userId); - shouldStoreKey = !!userKeyEncryptedPin; - break; - } } return shouldStoreKey; } @@ -744,7 +702,6 @@ export class DefaultKeyService implements KeyServiceAbstraction { protected async clearAllStoredUserKeys(userId: UserId): Promise { await this.stateService.setUserKeyAutoUnlock(null, { userId: userId }); - await this.pinService.clearPinKeyEncryptedUserKeyEphemeral(userId); } private async hashPhrase(hash: Uint8Array, minimumEntropy = 64) {