mirror of
https://github.com/bitwarden/browser
synced 2025-12-16 08:13:42 +00:00
Auth/PM-4596 - Extract PIN and Biometrics unlock method logic into re-useable services for user verification (#7107)
* PM-4596 - PinCryptoService first draft * PM-4596 - PinCryptoService - Refactor pinKeyEncryptedKey retrievals out into own method getPinKeyEncryptedKeys * PM-4596 - npm ci + npm run prettier to fix lint issues * PM-4596 - PinCryptoService - Add kdf types * PM-4596 - PinCryptoService - Refactor pin validation into own helper method. * PM-4596 - Rename pin-crypto.service.ts to pin-crypto.service.implementation.ts * PM-4596 - PinCryptoService - add additional logging for error states. * PM-4596 - JslibServicesModule - register new PinCryptoService and PinCryptoServiceAbstraction * PM-4596 - PinCryptoService - modify decryptUserKeyWithPin signature to not require email to match MP verification process in user verification service. * PM-4596 - Lock components - use new PinCryptoService.decryptUserKeyWithPin(...) to get user key + refactor base comp unlock with pin method to improve * PM-4596 - Lock component - if too many invalid attempts, added toast explaining that we were logging the user out due to excess PIN entry attempts * PM-4596 - UserVerificationService - (1) Refactor verifyUser(...) to use switch + separate methods for a cleaner parent method + better extensibility for PIN & biometrics which are TBD (2) Add PIN support to validateInput(...) * PM-4596 - UserVerificationService - add PIN and biometrics functions to verifyUser(...) * PM-4596 - PinCryptoService Spec - start test file - instantiates properly * PM-4596 - PinCryptoService tests - WIP * PM-4596 - PinCryptoService tests - WIP - got success cases working * PM-4596 - pin-crypto.service.implementation.spec.ts renamed to pin-crypto.service.spec.ts * PM-4596 - PinCryptoService.getPinKeyEncryptedKeys(...) - add comment + var name change for clarity * PM-4596 - PinCryptoService tests - test invalid, null return scenarios * PM-4596 - CLI - bw.ts - update UserVerificationService instantiation to include new pinCryptoService * PM-4596 - PinCryptoService - import VaultTimeoutSettingsServiceAbstraction instead of implementation for factory creation to get browser building * PM-4596 - (1) Create pinCryptoServiceFactory for browser background (2) Add it to the existing userVerificationServiceFactory * PM-4596 - Browser - Main.background.ts - Add pinCryptoService and add to userVerificationService dependencies * PM-4596 - UserVerificationService - per PR feedback simplify returns of verifyUserByPIN(...) and verifyUserByBiometrics(...) * PM-4596 - Messages.json on desktop & browser - per PR feedback, adjust tooManyInvalidPinEntryAttemptsLoggingOut translation text to remove "you" * PM-4596 - VerificationType enum - fix line copy mistake and give BIOMETRICS own, unique value. * PM-4596 - VerificationType - rename BIOMETRICS to Biometrics to match existing MasterPassword value case. * PM-4596 - Update verification type to consider whether or not a secret exists as we have added a new verification which doesn't have a type. Add new server and client side verification types. Update all relevant code to pass compilation checks. * PM-4596 - More verification type tweaking * PM-4596 - Verification - verificationHasSecret - tweak logic to be more dynamic and flexible for future verification types * PM-4596 - UpdateTempPasswordComp - use new MasterPasswordVerification * PM-4596 - Desktop - DeleteAcctComp - use VerificationWithSecret to solve compile error w/ accessing secret * PM-4596 - Per discussions with Andreas & Will, move new Pin Crypto services into libs/auth + added @bitwarden/auth path to CLI tsconfig + added new, required index.ts files for exporting service abstractions & implementations * PM-4596 - Fixed missed import fixes for lock components across clients for pin crypto service after moving into @bitwarden/auth * PM-4596 - More PinCryptoService import fixes to get browser & desktop building * PM-4596 - Update desktop lock comp tests to pass by providing new pin crypto service. * PM-4596 - User verification service -update todo * PM-4596 - PinCryptoService - per PR feedback, fix auto import wrong paths. * PM-4596 - PinCryptoService tests - fix imports per PR feedback * PM-4596 - UserVerificationSvc - rename method to validateSecretInput per PR feedback * Fix imports * PM-4596 - PinCryptoService - Refactor naming for clarity and move test cases into describes per PR feedback * reorg libs/auth; expose only libs/auth/core to cli app * PM-4596 - UserVerification - Resolve import issue with importing from libs/auth. Can't use @bitwarden/auth for whatever reason. * PM-4596 - Fix desktop build by fixing import * PM-4596 - Provide PinCryptoService to UserVerificationService * PM-4596 - PinCryptoServiceFactory - you cannot import services from @bitwarden/auth in the background b/c it brings along the libs/auth/components and introduces angular into the background context which doesn't have access to angular which causes random test failures. So, we must separate out the core services just like the CLI to only bring along the angular agnostic services from core. * PM-4596 - Refactor libs/auth to have angular / common + update all imports per discussion with Matt & Will. Introduced circular dep between PinCryptoService + VaultTimeoutSettingsService + UserVerificationService * PM-4596 - VaultTimeoutSettingsService - Refactor UserVerificationService out of the service and update all service instantiations and tests. The use of the UserVerificationService.hasMasterPassword method no longer needs to be used for backwards compatibility. This resolves the circular dependency between the PinCryptoService, the UserVerificationService, and the VaultTimeoutSettingsService. We will likely refactor the hasMasterPassword method out of the UserVerificationService in the future. * PM-4596 - Update CL tsconfig.libs.json to add new auth/common and auth/angular paths for jslib-services.module imports of pin crypto service to work and for test code coverage to run successfully. * PM-4596 - Address PR feedback * PM-4596 - Update root tsconfig (only used by storybook) to add new libs/auth paths to fix chromatic build pipeline. * PM-4596 - Actually update tsconfig with proper routes to fix storybook * PM-4596 - UserVerificationService - verifyUserByBiometrics - add error handling logic to convert failed or cancelled biometrics verification to a usable boolean * PM-4596 - Add missing await * PM-4596 - (1) Add log service and log to user verification service biometric flow to ensure errors are at least revealed to the console (2) Fix factory missing PinCryptoServiceInitOptions * PM-4596 - Use the correct log service abstraction * PM-4596 - Remove unused types per PR review --------- Co-authored-by: William Martin <contact@willmartian.com>
This commit is contained in:
Jared Snider
GitHub
@@ -1,12 +1,24 @@
|
||||
import { PinCryptoServiceAbstraction } from "../../../../../auth/src/common/abstractions/pin-crypto.service.abstraction";
|
||||
import { CryptoService } from "../../../platform/abstractions/crypto.service";
|
||||
import { I18nService } from "../../../platform/abstractions/i18n.service";
|
||||
import { LogService } from "../../../platform/abstractions/log.service";
|
||||
import { StateService } from "../../../platform/abstractions/state.service";
|
||||
import { KeySuffixOptions } from "../../../platform/enums/key-suffix-options.enum";
|
||||
import { UserKey } from "../../../platform/models/domain/symmetric-crypto-key";
|
||||
import { UserVerificationApiServiceAbstraction } from "../../abstractions/user-verification/user-verification-api.service.abstraction";
|
||||
import { UserVerificationService as UserVerificationServiceAbstraction } from "../../abstractions/user-verification/user-verification.service.abstraction";
|
||||
import { VerificationType } from "../../enums/verification-type";
|
||||
import { SecretVerificationRequest } from "../../models/request/secret-verification.request";
|
||||
import { VerifyOTPRequest } from "../../models/request/verify-otp.request";
|
||||
import { Verification } from "../../types/verification";
|
||||
import {
|
||||
MasterPasswordVerification,
|
||||
OtpVerification,
|
||||
PinVerification,
|
||||
ServerSideVerification,
|
||||
Verification,
|
||||
VerificationWithSecret,
|
||||
verificationHasSecret,
|
||||
} from "../../types/verification";
|
||||
|
||||
/**
|
||||
* Used for general-purpose user verification throughout the app.
|
||||
@@ -18,6 +30,8 @@ export class UserVerificationService implements UserVerificationServiceAbstracti
|
||||
private cryptoService: CryptoService,
|
||||
private i18nService: I18nService,
|
||||
private userVerificationApiService: UserVerificationApiServiceAbstraction,
|
||||
private pinCryptoService: PinCryptoServiceAbstraction,
|
||||
private logService: LogService,
|
||||
) {}
|
||||
|
||||
/**
|
||||
@@ -27,11 +41,11 @@ export class UserVerificationService implements UserVerificationServiceAbstracti
|
||||
* @param alreadyHashed Whether the master password is already hashed
|
||||
*/
|
||||
async buildRequest<T extends SecretVerificationRequest>(
|
||||
verification: Verification,
|
||||
verification: ServerSideVerification,
|
||||
requestClass?: new () => T,
|
||||
alreadyHashed?: boolean,
|
||||
) {
|
||||
this.validateInput(verification);
|
||||
this.validateSecretInput(verification);
|
||||
|
||||
const request =
|
||||
requestClass != null ? new requestClass() : (new SecretVerificationRequest() as T);
|
||||
@@ -57,42 +71,87 @@ export class UserVerificationService implements UserVerificationServiceAbstracti
|
||||
}
|
||||
|
||||
/**
|
||||
* Used to verify the Master Password client-side, or send the OTP to the server for verification (with no other data)
|
||||
* Used to verify Master Password, PIN, or biometrics client-side, or send the OTP to the server for verification (with no other data)
|
||||
* Generally used for client-side verification only.
|
||||
* @param verification User-supplied verification data (Master Password or OTP)
|
||||
* @param verification User-supplied verification data (OTP, MP, PIN, or biometrics)
|
||||
*/
|
||||
async verifyUser(verification: Verification): Promise<boolean> {
|
||||
this.validateInput(verification);
|
||||
if (verificationHasSecret(verification)) {
|
||||
this.validateSecretInput(verification);
|
||||
}
|
||||
|
||||
if (verification.type === VerificationType.OTP) {
|
||||
const request = new VerifyOTPRequest(verification.secret);
|
||||
try {
|
||||
await this.userVerificationApiService.postAccountVerifyOTP(request);
|
||||
} catch (e) {
|
||||
throw new Error(this.i18nService.t("invalidVerificationCode"));
|
||||
switch (verification.type) {
|
||||
case VerificationType.OTP:
|
||||
return this.verifyUserByOTP(verification);
|
||||
case VerificationType.MasterPassword:
|
||||
return this.verifyUserByMasterPassword(verification);
|
||||
case VerificationType.PIN:
|
||||
return this.verifyUserByPIN(verification);
|
||||
break;
|
||||
case VerificationType.Biometrics:
|
||||
return this.verifyUserByBiometrics();
|
||||
default: {
|
||||
// Compile-time check for exhaustive switch
|
||||
const _exhaustiveCheck: never = verification;
|
||||
return _exhaustiveCheck;
|
||||
}
|
||||
} else {
|
||||
let masterKey = await this.cryptoService.getMasterKey();
|
||||
if (!masterKey) {
|
||||
masterKey = await this.cryptoService.makeMasterKey(
|
||||
verification.secret,
|
||||
await this.stateService.getEmail(),
|
||||
await this.stateService.getKdfType(),
|
||||
await this.stateService.getKdfConfig(),
|
||||
);
|
||||
}
|
||||
const passwordValid = await this.cryptoService.compareAndUpdateKeyHash(
|
||||
verification.secret,
|
||||
masterKey,
|
||||
);
|
||||
if (!passwordValid) {
|
||||
throw new Error(this.i18nService.t("invalidMasterPassword"));
|
||||
}
|
||||
this.cryptoService.setMasterKey(masterKey);
|
||||
}
|
||||
}
|
||||
|
||||
private async verifyUserByOTP(verification: OtpVerification): Promise<boolean> {
|
||||
const request = new VerifyOTPRequest(verification.secret);
|
||||
try {
|
||||
await this.userVerificationApiService.postAccountVerifyOTP(request);
|
||||
} catch (e) {
|
||||
throw new Error(this.i18nService.t("invalidVerificationCode"));
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
private async verifyUserByMasterPassword(
|
||||
verification: MasterPasswordVerification,
|
||||
): Promise<boolean> {
|
||||
let masterKey = await this.cryptoService.getMasterKey();
|
||||
if (!masterKey) {
|
||||
masterKey = await this.cryptoService.makeMasterKey(
|
||||
verification.secret,
|
||||
await this.stateService.getEmail(),
|
||||
await this.stateService.getKdfType(),
|
||||
await this.stateService.getKdfConfig(),
|
||||
);
|
||||
}
|
||||
const passwordValid = await this.cryptoService.compareAndUpdateKeyHash(
|
||||
verification.secret,
|
||||
masterKey,
|
||||
);
|
||||
if (!passwordValid) {
|
||||
throw new Error(this.i18nService.t("invalidMasterPassword"));
|
||||
}
|
||||
// TODO: we should re-evaluate later on if user verification should have the side effect of modifying state. Probably not.
|
||||
await this.cryptoService.setMasterKey(masterKey);
|
||||
return true;
|
||||
}
|
||||
|
||||
private async verifyUserByPIN(verification: PinVerification): Promise<boolean> {
|
||||
const userKey = await this.pinCryptoService.decryptUserKeyWithPin(verification.secret);
|
||||
|
||||
return userKey != null;
|
||||
}
|
||||
|
||||
private async verifyUserByBiometrics(): Promise<boolean> {
|
||||
let userKey: UserKey;
|
||||
// Biometrics crashes and doesn't return a value if the user cancels the prompt
|
||||
try {
|
||||
userKey = await this.cryptoService.getUserKeyFromStorage(KeySuffixOptions.Biometric);
|
||||
} catch (e) {
|
||||
this.logService.error(`Biometrics User Verification failed: ${e.message}`);
|
||||
// So, any failures should be treated as a failed verification
|
||||
return false;
|
||||
}
|
||||
|
||||
return userKey != null;
|
||||
}
|
||||
|
||||
async requestOTP() {
|
||||
await this.userVerificationApiService.postAccountRequestOTP();
|
||||
}
|
||||
@@ -121,12 +180,15 @@ export class UserVerificationService implements UserVerificationServiceAbstracti
|
||||
);
|
||||
}
|
||||
|
||||
private validateInput(verification: Verification) {
|
||||
private validateSecretInput(verification: VerificationWithSecret) {
|
||||
if (verification?.secret == null || verification.secret === "") {
|
||||
if (verification.type === VerificationType.OTP) {
|
||||
throw new Error(this.i18nService.t("verificationCodeRequired"));
|
||||
} else {
|
||||
throw new Error(this.i18nService.t("masterPasswordRequired"));
|
||||
switch (verification.type) {
|
||||
case VerificationType.OTP:
|
||||
throw new Error(this.i18nService.t("verificationCodeRequired"));
|
||||
case VerificationType.MasterPassword:
|
||||
throw new Error(this.i18nService.t("masterPasswordRequired"));
|
||||
case VerificationType.PIN:
|
||||
throw new Error(this.i18nService.t("pinRequired"));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user