1
0
mirror of https://github.com/bitwarden/browser.git synced 2024-11-11 10:10:25 +01:00

Auth/PM-10601 - Tech Debt Cleanup - Refactor Lock Component and User Verification to use PinService (#10408)

* PM-10601 - PinSvc new unlock check first draft

* PM-10601 - PinSvc - add new method for determining if pin decryption is available.

* PM-10601 - Add more docs on PinSvc

* PM-10601 - Update Lock Comp & User Verification service + tests to use new isPinDecryptionAvailable method
This commit is contained in:
Jared Snider 2024-08-13 15:19:13 -04:00 committed by GitHub
parent a6176aaf2c
commit a13e99ebe9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 115 additions and 12 deletions

View File

@ -323,10 +323,7 @@ export class LockComponent implements OnInit, OnDestroy {
private async load(userId: UserId) { private async load(userId: UserId) {
this.pinLockType = await this.pinService.getPinLockType(userId); this.pinLockType = await this.pinService.getPinLockType(userId);
const ephemeralPinSet = await this.pinService.getPinKeyEncryptedUserKeyEphemeral(userId); this.pinEnabled = await this.pinService.isPinDecryptionAvailable(userId);
this.pinEnabled =
(this.pinLockType === "EPHEMERAL" && !!ephemeralPinSet) || this.pinLockType === "PERSISTENT";
this.masterPasswordEnabled = await this.userVerificationService.hasMasterPassword(); this.masterPasswordEnabled = await this.userVerificationService.hasMasterPassword();

View File

@ -113,9 +113,17 @@ export abstract class PinServiceAbstraction {
/** /**
* Declares whether or not the user has a PIN set (either persistent or ephemeral). * 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<boolean>; abstract isPinSet: (userId: UserId) => Promise<boolean>;
/**
* 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<boolean>;
/** /**
* Decrypts the UserKey with the provided PIN. * Decrypts the UserKey with the provided PIN.
* *

View File

@ -292,6 +292,34 @@ export class PinService implements PinServiceAbstraction {
return (await this.getPinLockType(userId)) !== "DISABLED"; return (await this.getPinLockType(userId)) !== "DISABLED";
} }
async isPinDecryptionAvailable(userId: UserId): Promise<boolean> {
this.validateUserId(userId, "Cannot determine if decryption of user key via PIN is available.");
const pinLockType = await this.getPinLockType(userId);
switch (pinLockType) {
case "DISABLED":
return false;
case "PERSISTENT":
// The above getPinLockType call ensures that we have either a PinKeyEncryptedUserKey or OldPinKeyEncryptedMasterKey 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);
}
default: {
// Compile-time check for exhaustive switch
const _exhaustiveCheck: never = pinLockType;
throw new Error(`Unexpected pinLockType: ${_exhaustiveCheck}`);
}
}
}
async decryptUserKeyWithPin(pin: string, userId: UserId): Promise<UserKey | null> { async decryptUserKeyWithPin(pin: string, userId: UserId): Promise<UserKey | null> {
this.validateUserId(userId, "Cannot decrypt user key with PIN."); this.validateUserId(userId, "Cannot decrypt user key with PIN.");

View File

@ -416,6 +416,66 @@ describe("PinService", () => {
}); });
}); });
describe("isPinDecryptionAvailable()", () => {
it("should return false if pinLockType is DISABLED", async () => {
// Arrange
sut.getPinLockType = jest.fn().mockResolvedValue("DISABLED");
// Act
const result = await sut.isPinDecryptionAvailable(mockUserId);
// Assert
expect(result).toBe(false);
});
it("should return true if pinLockType is PERSISTENT", async () => {
// Arrange
sut.getPinLockType = jest.fn().mockResolvedValue("PERSISTENT");
// Act
const result = await sut.isPinDecryptionAvailable(mockUserId);
// Assert
expect(result).toBe(true);
});
it("should return true if pinLockType is EPHEMERAL and we have an ephemeral PIN key encrypted user key", async () => {
// Arrange
sut.getPinLockType = jest.fn().mockResolvedValue("EPHEMERAL");
sut.getPinKeyEncryptedUserKeyEphemeral = jest
.fn()
.mockResolvedValue(pinKeyEncryptedUserKeyEphemeral);
// Act
const result = await sut.isPinDecryptionAvailable(mockUserId);
// Assert
expect(result).toBe(true);
});
it("should return false if pinLockType is EPHEMERAL and we do not have an ephemeral PIN key encrypted user key", async () => {
// Arrange
sut.getPinLockType = jest.fn().mockResolvedValue("EPHEMERAL");
sut.getPinKeyEncryptedUserKeyEphemeral = jest.fn().mockResolvedValue(null);
// Act
const result = await sut.isPinDecryptionAvailable(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",
);
});
});
describe("decryptUserKeyWithPin()", () => { describe("decryptUserKeyWithPin()", () => {
async function setupDecryptUserKeyWithPinMocks( async function setupDecryptUserKeyWithPinMocks(
pinLockType: PinLockType, pinLockType: PinLockType,

View File

@ -410,6 +410,12 @@ describe("UserVerificationService", () => {
function setPinAvailability(type: PinLockType) { function setPinAvailability(type: PinLockType) {
pinService.getPinLockType.mockResolvedValue(type); pinService.getPinLockType.mockResolvedValue(type);
if (type === "EPHEMERAL" || type === "PERSISTENT") {
pinService.isPinDecryptionAvailable.mockResolvedValue(true);
} else if (type === "DISABLED") {
pinService.isPinDecryptionAvailable.mockResolvedValue(false);
}
} }
function disableBiometricsAvailability() { function disableBiometricsAvailability() {

View File

@ -57,13 +57,17 @@ export class UserVerificationService implements UserVerificationServiceAbstracti
): Promise<UserVerificationOptions> { ): Promise<UserVerificationOptions> {
const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id;
if (verificationType === "client") { if (verificationType === "client") {
const [userHasMasterPassword, pinLockType, biometricsLockSet, biometricsUserKeyStored] = const [
await Promise.all([ userHasMasterPassword,
this.hasMasterPasswordAndMasterKeyHash(userId), isPinDecryptionAvailable,
this.pinService.getPinLockType(userId), biometricsLockSet,
this.vaultTimeoutSettingsService.isBiometricLockSet(userId), biometricsUserKeyStored,
this.cryptoService.hasUserKeyStored(KeySuffixOptions.Biometric, userId), ] = await Promise.all([
]); this.hasMasterPasswordAndMasterKeyHash(userId),
this.pinService.isPinDecryptionAvailable(userId),
this.vaultTimeoutSettingsService.isBiometricLockSet(userId),
this.cryptoService.hasUserKeyStored(KeySuffixOptions.Biometric, userId),
]);
// note: we do not need to check this.platformUtilsService.supportsBiometric() because // note: we do not need to check this.platformUtilsService.supportsBiometric() because
// we can just use the logic below which works for both desktop & the browser extension. // we can just use the logic below which works for both desktop & the browser extension.
@ -71,7 +75,7 @@ export class UserVerificationService implements UserVerificationServiceAbstracti
return { return {
client: { client: {
masterPassword: userHasMasterPassword, masterPassword: userHasMasterPassword,
pin: pinLockType !== "DISABLED", pin: isPinDecryptionAvailable,
biometrics: biometrics:
biometricsLockSet && biometricsLockSet &&
(biometricsUserKeyStored || !this.platformUtilsService.supportsSecureStorage()), (biometricsUserKeyStored || !this.platformUtilsService.supportsSecureStorage()),