diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index cae554c872c..eec07b5b1ed 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -655,9 +655,7 @@ export default class MainBackground { this.kdfConfigService, this.keyGenerationService, this.logService, - this.masterPasswordService, this.stateProvider, - this.stateService, ); this.keyService = new DefaultKeyService( diff --git a/apps/cli/src/service-container/service-container.ts b/apps/cli/src/service-container/service-container.ts index 5bc07f63c32..6a4651bcd5a 100644 --- a/apps/cli/src/service-container/service-container.ts +++ b/apps/cli/src/service-container/service-container.ts @@ -436,9 +436,7 @@ export class ServiceContainer { this.kdfConfigService, this.keyGenerationService, this.logService, - this.masterPasswordService, this.stateProvider, - this.stateService, ); this.keyService = new KeyService( diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 37220b5195d..6334e5815d6 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -1173,9 +1173,7 @@ const safeProviders: SafeProvider[] = [ KdfConfigService, KeyGenerationServiceAbstraction, LogService, - MasterPasswordServiceAbstraction, StateProvider, - StateServiceAbstraction, ], }), safeProvider({ diff --git a/libs/auth/src/common/abstractions/pin.service.abstraction.ts b/libs/auth/src/common/abstractions/pin.service.abstraction.ts index 0d0f29dff40..16550888b94 100644 --- a/libs/auth/src/common/abstractions/pin.service.abstraction.ts +++ b/libs/auth/src/common/abstractions/pin.service.abstraction.ts @@ -1,4 +1,4 @@ -import { EncString, EncryptedString } from "@bitwarden/common/platform/models/domain/enc-string"; +import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; import { UserId } from "@bitwarden/common/types/guid"; import { PinKey, UserKey } from "@bitwarden/common/types/key"; import { KdfConfig } from "@bitwarden/key-management"; @@ -90,17 +90,6 @@ export abstract class PinServiceAbstraction { */ abstract clearUserKeyEncryptedPin(userId: UserId): Promise; - /** - * Gets the old MasterKey, encrypted by the PinKey (formerly called `pinProtected`). - * Deprecated and used for migration purposes only. - */ - abstract getOldPinKeyEncryptedMasterKey: (userId: UserId) => Promise; - - /** - * Clears the old MasterKey, encrypted by the PinKey. - */ - abstract clearOldPinKeyEncryptedMasterKey: (userId: UserId) => Promise; - /** * Makes a PinKey from the provided PIN. */ diff --git a/libs/auth/src/common/services/pin/pin.service.implementation.ts b/libs/auth/src/common/services/pin/pin.service.implementation.ts index 0f6ac05f381..99fb725c295 100644 --- a/libs/auth/src/common/services/pin/pin.service.implementation.ts +++ b/libs/auth/src/common/services/pin/pin.service.implementation.ts @@ -4,11 +4,9 @@ import { firstValueFrom, map } from "rxjs"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; -import { MasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; import { CryptoFunctionService } from "@bitwarden/common/platform/abstractions/crypto-function.service"; import { KeyGenerationService } from "@bitwarden/common/platform/abstractions/key-generation.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; -import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { EncString, EncryptedString } from "@bitwarden/common/platform/models/domain/enc-string"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { @@ -18,7 +16,7 @@ import { UserKeyDefinition, } from "@bitwarden/common/platform/state"; import { UserId } from "@bitwarden/common/types/guid"; -import { MasterKey, PinKey, UserKey } from "@bitwarden/common/types/key"; +import { PinKey, UserKey } from "@bitwarden/common/types/key"; import { KdfConfig, KdfConfigService } from "@bitwarden/key-management"; import { PinServiceAbstraction } from "../../abstractions/pin.service.abstraction"; @@ -73,19 +71,6 @@ export const USER_KEY_ENCRYPTED_PIN = new UserKeyDefinition( }, ); -/** - * The old MasterKey, encrypted by the PinKey (formerly called `pinProtected`). - * Deprecated and used for migration purposes only. - */ -export const OLD_PIN_KEY_ENCRYPTED_MASTER_KEY = new UserKeyDefinition( - PIN_DISK, - "oldPinKeyEncryptedMasterKey", - { - deserializer: (jsonValue) => jsonValue, - clearOn: ["logout"], - }, -); - export class PinService implements PinServiceAbstraction { constructor( private accountService: AccountService, @@ -94,9 +79,7 @@ export class PinService implements PinServiceAbstraction { private kdfConfigService: KdfConfigService, private keyGenerationService: KeyGenerationService, private logService: LogService, - private masterPasswordService: MasterPasswordServiceAbstraction, private stateProvider: StateProvider, - private stateService: StateService, ) {} async getPinKeyEncryptedUserKeyPersistent(userId: UserId): Promise { @@ -190,9 +173,7 @@ export class PinService implements PinServiceAbstraction { this.accountService.accounts$.pipe(map((accounts) => accounts[userId].email)), ); const kdfConfig = await this.kdfConfigService.getKdfConfig(); - const pinKey = await this.makePinKey(pin, email, kdfConfig); - return await this.encryptService.encrypt(userKey.key, pinKey); } @@ -242,20 +223,6 @@ export class PinService implements PinServiceAbstraction { return await this.encryptService.encrypt(pin, userKey); } - async getOldPinKeyEncryptedMasterKey(userId: UserId): Promise { - this.validateUserId(userId, "Cannot get oldPinKeyEncryptedMasterKey."); - - return await firstValueFrom( - this.stateProvider.getUserState$(OLD_PIN_KEY_ENCRYPTED_MASTER_KEY, userId), - ); - } - - async clearOldPinKeyEncryptedMasterKey(userId: UserId): Promise { - this.validateUserId(userId, "Cannot clear oldPinKeyEncryptedMasterKey."); - - await this.stateProvider.setUserState(OLD_PIN_KEY_ENCRYPTED_MASTER_KEY, null, userId); - } - async makePinKey(pin: string, salt: string, kdfConfig: KdfConfig): Promise { const pinKey = await this.keyGenerationService.deriveKeyFromPassword(pin, salt, kdfConfig); return (await this.keyGenerationService.stretchKey(pinKey)) as PinKey; @@ -264,23 +231,13 @@ export class PinService implements PinServiceAbstraction { async getPinLockType(userId: UserId): Promise { this.validateUserId(userId, "Cannot get PinLockType."); - /** - * We can't check the `userKeyEncryptedPin` (formerly called `protectedPin`) for both because old - * accounts only used it for MP on Restart - */ const aUserKeyEncryptedPinIsSet = !!(await this.getUserKeyEncryptedPin(userId)); const aPinKeyEncryptedUserKeyPersistentIsSet = !!(await this.getPinKeyEncryptedUserKeyPersistent(userId)); - const anOldPinKeyEncryptedMasterKeyIsSet = - !!(await this.getOldPinKeyEncryptedMasterKey(userId)); - if (aPinKeyEncryptedUserKeyPersistentIsSet || anOldPinKeyEncryptedMasterKeyIsSet) { + if (aPinKeyEncryptedUserKeyPersistentIsSet) { return "PERSISTENT"; - } else if ( - aUserKeyEncryptedPinIsSet && - !aPinKeyEncryptedUserKeyPersistentIsSet && - !anOldPinKeyEncryptedMasterKeyIsSet - ) { + } else if (aUserKeyEncryptedPinIsSet && !aPinKeyEncryptedUserKeyPersistentIsSet) { return "EPHEMERAL"; } else { return "DISABLED"; @@ -302,7 +259,7 @@ export class PinService implements PinServiceAbstraction { case "DISABLED": return false; case "PERSISTENT": - // The above getPinLockType call ensures that we have either a PinKeyEncryptedUserKey or OldPinKeyEncryptedMasterKey set. + // The above getPinLockType call ensures that we have either a PinKeyEncryptedUserKey set. return true; case "EPHEMERAL": { // The above getPinLockType call ensures that we have a UserKeyEncryptedPin set. @@ -326,31 +283,21 @@ export class PinService implements PinServiceAbstraction { try { const pinLockType = await this.getPinLockType(userId); - const requireMasterPasswordOnClientRestart = pinLockType === "EPHEMERAL"; - const { pinKeyEncryptedUserKey, oldPinKeyEncryptedMasterKey } = - await this.getPinKeyEncryptedKeys(pinLockType, userId); + const pinKeyEncryptedUserKey = await this.getPinKeyEncryptedKeys(pinLockType, userId); const email = await firstValueFrom( this.accountService.accounts$.pipe(map((accounts) => accounts[userId].email)), ); const kdfConfig = await this.kdfConfigService.getKdfConfig(); - let userKey: UserKey; - - if (oldPinKeyEncryptedMasterKey) { - userKey = await this.decryptAndMigrateOldPinKeyEncryptedMasterKey( - userId, - pin, - email, - kdfConfig, - requireMasterPasswordOnClientRestart, - oldPinKeyEncryptedMasterKey, - ); - } else { - userKey = await this.decryptUserKey(userId, pin, email, kdfConfig, pinKeyEncryptedUserKey); - } - + const userKey: UserKey = await this.decryptUserKey( + userId, + pin, + email, + kdfConfig, + pinKeyEncryptedUserKey, + ); if (!userKey) { this.logService.warning(`User key null after pin key decryption.`); return null; @@ -394,109 +341,23 @@ export class PinService implements PinServiceAbstraction { } /** - * Creates a new `pinKeyEncryptedUserKey` and clears the `oldPinKeyEncryptedMasterKey`. - * @returns UserKey - */ - private async decryptAndMigrateOldPinKeyEncryptedMasterKey( - userId: UserId, - pin: string, - email: string, - kdfConfig: KdfConfig, - requireMasterPasswordOnClientRestart: boolean, - oldPinKeyEncryptedMasterKey: EncString, - ): Promise { - this.validateUserId(userId, "Cannot decrypt and migrate oldPinKeyEncryptedMasterKey."); - - const masterKey = await this.decryptMasterKeyWithPin( - userId, - pin, - email, - kdfConfig, - oldPinKeyEncryptedMasterKey, - ); - - const encUserKey = await this.stateService.getEncryptedCryptoSymmetricKey({ userId: userId }); - - const userKey = await this.masterPasswordService.decryptUserKeyWithMasterKey( - masterKey, - userId, - encUserKey ? new EncString(encUserKey) : undefined, - ); - - const pinKeyEncryptedUserKey = await this.createPinKeyEncryptedUserKey(pin, userKey, userId); - await this.storePinKeyEncryptedUserKey( - pinKeyEncryptedUserKey, - requireMasterPasswordOnClientRestart, - userId, - ); - - const userKeyEncryptedPin = await this.createUserKeyEncryptedPin(pin, userKey); - await this.setUserKeyEncryptedPin(userKeyEncryptedPin, userId); - - await this.clearOldPinKeyEncryptedMasterKey(userId); - - return userKey; - } - - // Only for migration purposes - private async decryptMasterKeyWithPin( - userId: UserId, - pin: string, - salt: string, - kdfConfig: KdfConfig, - oldPinKeyEncryptedMasterKey?: EncString, - ): Promise { - this.validateUserId(userId, "Cannot decrypt master key with PIN."); - - if (!oldPinKeyEncryptedMasterKey) { - const oldPinKeyEncryptedMasterKeyString = await this.getOldPinKeyEncryptedMasterKey(userId); - - if (oldPinKeyEncryptedMasterKeyString == null) { - throw new Error("No oldPinKeyEncrytedMasterKey found."); - } - - oldPinKeyEncryptedMasterKey = new EncString(oldPinKeyEncryptedMasterKeyString); - } - - const pinKey = await this.makePinKey(pin, salt, kdfConfig); - const masterKey = await this.encryptService.decryptToBytes(oldPinKeyEncryptedMasterKey, pinKey); - - return new SymmetricCryptoKey(masterKey) as MasterKey; - } - - /** - * Gets the user's `pinKeyEncryptedUserKey` (persistent or ephemeral) and `oldPinKeyEncryptedMasterKey` + * Gets the user's `pinKeyEncryptedUserKey` (persistent or ephemeral) * (if one exists) based on the user's PinLockType. * - * @remarks The `oldPinKeyEncryptedMasterKey` (formerly `pinProtected`) is only used for migration and - * will be null for all migrated accounts. * @throws If PinLockType is 'DISABLED' or if userId is not provided */ private async getPinKeyEncryptedKeys( pinLockType: PinLockType, userId: UserId, - ): Promise<{ pinKeyEncryptedUserKey: EncString; oldPinKeyEncryptedMasterKey?: EncString }> { + ): Promise { this.validateUserId(userId, "Cannot get PinKey encrypted keys."); switch (pinLockType) { case "PERSISTENT": { - const pinKeyEncryptedUserKey = await this.getPinKeyEncryptedUserKeyPersistent(userId); - const oldPinKeyEncryptedMasterKey = await this.getOldPinKeyEncryptedMasterKey(userId); - - return { - pinKeyEncryptedUserKey, - oldPinKeyEncryptedMasterKey: oldPinKeyEncryptedMasterKey - ? new EncString(oldPinKeyEncryptedMasterKey) - : undefined, - }; + return await this.getPinKeyEncryptedUserKeyPersistent(userId); } case "EPHEMERAL": { - const pinKeyEncryptedUserKey = await this.getPinKeyEncryptedUserKeyEphemeral(userId); - - return { - pinKeyEncryptedUserKey, - oldPinKeyEncryptedMasterKey: undefined, // Going forward, we only migrate non-ephemeral version - }; + return await this.getPinKeyEncryptedUserKeyEphemeral(userId); } case "DISABLED": throw new Error("Pin is disabled"); diff --git a/libs/auth/src/common/services/pin/pin.service.spec.ts b/libs/auth/src/common/services/pin/pin.service.spec.ts index 794d08b63b2..ebdc36219ef 100644 --- a/libs/auth/src/common/services/pin/pin.service.spec.ts +++ b/libs/auth/src/common/services/pin/pin.service.spec.ts @@ -1,11 +1,9 @@ import { mock } from "jest-mock-extended"; 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 { CryptoFunctionService } from "@bitwarden/common/platform/abstractions/crypto-function.service"; import { KeyGenerationService } from "@bitwarden/common/platform/abstractions/key-generation.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; -import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { EncString } from "@bitwarden/common/platform/models/domain/enc-string"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; @@ -15,14 +13,13 @@ import { mockAccountServiceWith, } from "@bitwarden/common/spec"; import { UserId } from "@bitwarden/common/types/guid"; -import { MasterKey, PinKey, UserKey } from "@bitwarden/common/types/key"; +import { PinKey, UserKey } from "@bitwarden/common/types/key"; import { DEFAULT_KDF_CONFIG, KdfConfigService } from "@bitwarden/key-management"; import { PinService, PIN_KEY_ENCRYPTED_USER_KEY_PERSISTENT, PIN_KEY_ENCRYPTED_USER_KEY_EPHEMERAL, - OLD_PIN_KEY_ENCRYPTED_MASTER_KEY, USER_KEY_ENCRYPTED_PIN, PinLockType, } from "./pin.service.implementation"; @@ -31,7 +28,6 @@ describe("PinService", () => { let sut: PinService; let accountService: FakeAccountService; - let masterPasswordService: FakeMasterPasswordService; let stateProvider: FakeStateProvider; const cryptoFunctionService = mock(); @@ -39,11 +35,9 @@ describe("PinService", () => { const kdfConfigService = mock(); const keyGenerationService = mock(); const logService = mock(); - const stateService = mock(); const mockUserId = Utils.newGuid() as UserId; const mockUserKey = new SymmetricCryptoKey(randomBytes(64)) as UserKey; - const mockMasterKey = new SymmetricCryptoKey(randomBytes(32)) as MasterKey; const mockPinKey = new SymmetricCryptoKey(randomBytes(32)) as PinKey; const mockUserEmail = "user@example.com"; const mockPin = "1234"; @@ -57,15 +51,10 @@ describe("PinService", () => { "2.fb5kOEZvh9zPABbP8WRmSQ==|Yi6ZAJY+UtqCKMUSqp1ahY9Kf8QuneKXs6BMkpNsakLVOzTYkHHlilyGABMF7GzUO8QHyZi7V/Ovjjg+Naf3Sm8qNhxtDhibITv4k8rDnM0=|TFkq3h2VNTT1z5BFbebm37WYuxyEHXuRo0DZJI7TQnw=", ); - const oldPinKeyEncryptedMasterKeyPostMigration: any = null; - const oldPinKeyEncryptedMasterKeyPreMigrationPersistent = - "2.fb5kOEZvh9zPABbP8WRmSQ==|Yi6ZAJY+UtqCKMUSqp1ahY9Kf8QuneKXs6BMkpNsakLVOzTYkHHlilyGABMF7GzUO8QHyZi7V/Ovjjg+Naf3Sm8qNhxtDhibITv4k8rDnM0=|TFkq3h2VNTT1z5BFbebm37WYuxyEHXuRo0DZJI7TQnw="; - beforeEach(() => { jest.clearAllMocks(); accountService = mockAccountServiceWith(mockUserId, { email: mockUserEmail }); - masterPasswordService = new FakeMasterPasswordService(); stateProvider = new FakeStateProvider(accountService); sut = new PinService( @@ -75,9 +64,7 @@ describe("PinService", () => { kdfConfigService, keyGenerationService, logService, - masterPasswordService, stateProvider, - stateService, ); }); @@ -111,12 +98,6 @@ describe("PinService", () => { await expect(sut.clearUserKeyEncryptedPin(undefined)).rejects.toThrow( "User ID is required. Cannot clear userKeyEncryptedPin.", ); - await expect(sut.getOldPinKeyEncryptedMasterKey(undefined)).rejects.toThrow( - "User ID is required. Cannot get oldPinKeyEncryptedMasterKey.", - ); - await expect(sut.clearOldPinKeyEncryptedMasterKey(undefined)).rejects.toThrow( - "User ID is required. Cannot clear oldPinKeyEncryptedMasterKey.", - ); await expect( sut.createPinKeyEncryptedUserKey(mockPin, mockUserKey, undefined), ).rejects.toThrow("User ID is required. Cannot create pinKeyEncryptedUserKey."); @@ -288,31 +269,6 @@ describe("PinService", () => { }); }); - describe("oldPinKeyEncryptedMasterKey methods", () => { - describe("getOldPinKeyEncryptedMasterKey()", () => { - it("should get the oldPinKeyEncryptedMasterKey of the specified userId", async () => { - await sut.getOldPinKeyEncryptedMasterKey(mockUserId); - - expect(stateProvider.mock.getUserState$).toHaveBeenCalledWith( - OLD_PIN_KEY_ENCRYPTED_MASTER_KEY, - mockUserId, - ); - }); - }); - - describe("clearOldPinKeyEncryptedMasterKey()", () => { - it("should clear the oldPinKeyEncryptedMasterKey of the specified userId", async () => { - await sut.clearOldPinKeyEncryptedMasterKey(mockUserId); - - expect(stateProvider.mock.setUserState).toHaveBeenCalledWith( - OLD_PIN_KEY_ENCRYPTED_MASTER_KEY, - null, - mockUserId, - ); - }); - }); - }); - describe("makePinKey()", () => { it("should make a PinKey", async () => { // Arrange @@ -346,26 +302,10 @@ describe("PinService", () => { expect(result).toBe("PERSISTENT"); }); - it("should return 'PERSISTENT' if an old oldPinKeyEncryptedMasterKey is found", async () => { - // Arrange - sut.getUserKeyEncryptedPin = jest.fn().mockResolvedValue(null); - sut.getPinKeyEncryptedUserKeyPersistent = jest.fn().mockResolvedValue(null); - sut.getOldPinKeyEncryptedMasterKey = jest - .fn() - .mockResolvedValue(oldPinKeyEncryptedMasterKeyPreMigrationPersistent); - - // Act - const result = await sut.getPinLockType(mockUserId); - - // Assert - expect(result).toBe("PERSISTENT"); - }); - - it("should return 'EPHEMERAL' if neither a pinKeyEncryptedUserKey (persistent version) nor an old oldPinKeyEncryptedMasterKey are found, but a userKeyEncryptedPin is found", async () => { + 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); - sut.getOldPinKeyEncryptedMasterKey = jest.fn().mockResolvedValue(null); // Act const result = await sut.getPinLockType(mockUserId); @@ -374,11 +314,10 @@ describe("PinService", () => { expect(result).toBe("EPHEMERAL"); }); - it("should return 'DISABLED' if ALL three of these are NOT found: userKeyEncryptedPin, pinKeyEncryptedUserKey (persistent version), oldPinKeyEncryptedMasterKey", async () => { + 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); - sut.getOldPinKeyEncryptedMasterKey = jest.fn().mockResolvedValue(null); // Act const result = await sut.getPinLockType(mockUserId); @@ -476,46 +415,20 @@ describe("PinService", () => { }); describe("decryptUserKeyWithPin()", () => { - async function setupDecryptUserKeyWithPinMocks( - pinLockType: PinLockType, - migrationStatus: "PRE" | "POST" = "POST", - ) { + async function setupDecryptUserKeyWithPinMocks(pinLockType: PinLockType) { sut.getPinLockType = jest.fn().mockResolvedValue(pinLockType); - mockPinEncryptedKeyDataByPinLockType(pinLockType, migrationStatus); + mockPinEncryptedKeyDataByPinLockType(pinLockType); kdfConfigService.getKdfConfig.mockResolvedValue(DEFAULT_KDF_CONFIG); - if (pinLockType === "PERSISTENT" && migrationStatus === "PRE") { - await mockDecryptAndMigrateOldPinKeyEncryptedMasterKeyFn(); - } else { - mockDecryptUserKeyFn(); - } + mockDecryptUserKeyFn(); sut.getUserKeyEncryptedPin = jest.fn().mockResolvedValue(mockUserKeyEncryptedPin); encryptService.decryptToUtf8.mockResolvedValue(mockPin); cryptoFunctionService.compareFast.calledWith(mockPin, "1234").mockResolvedValue(true); } - async function mockDecryptAndMigrateOldPinKeyEncryptedMasterKeyFn() { - sut.makePinKey = jest.fn().mockResolvedValue(mockPinKey); - encryptService.decryptToBytes.mockResolvedValue(mockMasterKey.key); - - stateService.getEncryptedCryptoSymmetricKey.mockResolvedValue(mockUserKey.keyB64); - masterPasswordService.mock.decryptUserKeyWithMasterKey.mockResolvedValue(mockUserKey); - - sut.createPinKeyEncryptedUserKey = jest - .fn() - .mockResolvedValue(pinKeyEncryptedUserKeyPersistant); - - await sut.storePinKeyEncryptedUserKey(pinKeyEncryptedUserKeyPersistant, false, mockUserId); - - sut.createUserKeyEncryptedPin = jest.fn().mockResolvedValue(mockUserKeyEncryptedPin); - await sut.setUserKeyEncryptedPin(mockUserKeyEncryptedPin, mockUserId); - - await sut.clearOldPinKeyEncryptedMasterKey(mockUserId); - } - function mockDecryptUserKeyFn() { sut.getPinKeyEncryptedUserKeyPersistent = jest .fn() @@ -524,26 +437,12 @@ describe("PinService", () => { encryptService.decryptToBytes.mockResolvedValue(mockUserKey.key); } - function mockPinEncryptedKeyDataByPinLockType( - pinLockType: PinLockType, - migrationStatus: "PRE" | "POST" = "POST", - ) { + function mockPinEncryptedKeyDataByPinLockType(pinLockType: PinLockType) { switch (pinLockType) { case "PERSISTENT": sut.getPinKeyEncryptedUserKeyPersistent = jest .fn() .mockResolvedValue(pinKeyEncryptedUserKeyPersistant); - - if (migrationStatus === "PRE") { - sut.getOldPinKeyEncryptedMasterKey = jest - .fn() - .mockResolvedValue(oldPinKeyEncryptedMasterKeyPreMigrationPersistent); - } else { - sut.getOldPinKeyEncryptedMasterKey = jest - .fn() - .mockResolvedValue(oldPinKeyEncryptedMasterKeyPostMigration); // null - } - break; case "EPHEMERAL": sut.getPinKeyEncryptedUserKeyEphemeral = jest @@ -557,49 +456,16 @@ describe("PinService", () => { } } - const testCases: { pinLockType: PinLockType; migrationStatus: "PRE" | "POST" }[] = [ - { pinLockType: "PERSISTENT", migrationStatus: "PRE" }, - { pinLockType: "PERSISTENT", migrationStatus: "POST" }, - { pinLockType: "EPHEMERAL", migrationStatus: "POST" }, + const testCases: { pinLockType: PinLockType }[] = [ + { pinLockType: "PERSISTENT" }, + { pinLockType: "EPHEMERAL" }, ]; - testCases.forEach(({ pinLockType, migrationStatus }) => { - describe(`given a ${pinLockType} PIN (${migrationStatus} migration)`, () => { - if (pinLockType === "PERSISTENT" && migrationStatus === "PRE") { - it("should clear the oldPinKeyEncryptedMasterKey from state", async () => { - // Arrange - await setupDecryptUserKeyWithPinMocks(pinLockType, migrationStatus); - - // Act - await sut.decryptUserKeyWithPin(mockPin, mockUserId); - - // Assert - expect(stateProvider.mock.setUserState).toHaveBeenCalledWith( - OLD_PIN_KEY_ENCRYPTED_MASTER_KEY, - null, - mockUserId, - ); - }); - - it("should set the new pinKeyEncrypterUserKeyPersistent to state", async () => { - // Arrange - await setupDecryptUserKeyWithPinMocks(pinLockType, migrationStatus); - - // Act - await sut.decryptUserKeyWithPin(mockPin, mockUserId); - - // Assert - expect(stateProvider.mock.setUserState).toHaveBeenCalledWith( - PIN_KEY_ENCRYPTED_USER_KEY_PERSISTENT, - pinKeyEncryptedUserKeyPersistant.encryptedString, - mockUserId, - ); - }); - } - + 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, migrationStatus); + await setupDecryptUserKeyWithPinMocks(pinLockType); // Act const result = await sut.decryptUserKeyWithPin(mockPin, mockUserId); @@ -610,7 +476,7 @@ describe("PinService", () => { it(`should return null when PIN is incorrect and user key cannot be decrypted`, async () => { // Arrange - await setupDecryptUserKeyWithPinMocks(pinLockType, migrationStatus); + await setupDecryptUserKeyWithPinMocks(pinLockType); sut.decryptUserKeyWithPin = jest.fn().mockResolvedValue(null); // Act @@ -623,7 +489,7 @@ describe("PinService", () => { // 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, migrationStatus); + await setupDecryptUserKeyWithPinMocks(pinLockType); encryptService.decryptToUtf8.mockResolvedValue("9999"); // non matching PIN // Act diff --git a/libs/common/src/key-management/master-password/services/master-password.service.ts b/libs/common/src/key-management/master-password/services/master-password.service.ts index 72987b13827..9ed01cf0c83 100644 --- a/libs/common/src/key-management/master-password/services/master-password.service.ts +++ b/libs/common/src/key-management/master-password/services/master-password.service.ts @@ -163,19 +163,6 @@ export class MasterPasswordService implements InternalMasterPasswordServiceAbstr throw new Error("No master key found."); } - // Try one more way to get the user key if it still wasn't found. - if (userKey == null) { - const deprecatedKey = await this.stateService.getEncryptedCryptoSymmetricKey({ - userId: userId, - }); - - if (deprecatedKey == null) { - throw new Error("No encrypted user key found."); - } - - userKey = new EncString(deprecatedKey); - } - let decUserKey: Uint8Array; if (userKey.encryptionType === EncryptionType.AesCbc256_B64) { diff --git a/libs/common/src/key-management/vault-timeout/services/vault-timeout.service.ts b/libs/common/src/key-management/vault-timeout/services/vault-timeout.service.ts index 1762b9156db..d71b8972727 100644 --- a/libs/common/src/key-management/vault-timeout/services/vault-timeout.service.ts +++ b/libs/common/src/key-management/vault-timeout/services/vault-timeout.service.ts @@ -147,7 +147,6 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction { await this.masterPasswordService.clearMasterKey(lockingUserId); await this.stateService.setUserKeyAutoUnlock(null, { userId: lockingUserId }); - await this.stateService.setCryptoMasterKeyAuto(null, { userId: lockingUserId }); await this.cipherService.clearCache(lockingUserId); diff --git a/libs/common/src/platform/abstractions/state.service.ts b/libs/common/src/platform/abstractions/state.service.ts index 3b0ce07623b..e4dbe76d7e4 100644 --- a/libs/common/src/platform/abstractions/state.service.ts +++ b/libs/common/src/platform/abstractions/state.service.ts @@ -50,14 +50,6 @@ export abstract class StateService { value: boolean, options?: StorageOptions, ) => Promise; - /** - * @deprecated For migration purposes only, use getUserKeyMasterKey instead - */ - getEncryptedCryptoSymmetricKey: (options?: StorageOptions) => Promise; - /** - * @deprecated For migration purposes only, use setUserKeyAuto instead - */ - setCryptoMasterKeyAuto: (value: string | null, options?: StorageOptions) => Promise; getDuckDuckGoSharedKey: (options?: StorageOptions) => Promise; setDuckDuckGoSharedKey: (value: string, options?: StorageOptions) => Promise; diff --git a/libs/common/src/platform/models/domain/account.ts b/libs/common/src/platform/models/domain/account.ts index 9873e5c8574..b9d10f47e97 100644 --- a/libs/common/src/platform/models/domain/account.ts +++ b/libs/common/src/platform/models/domain/account.ts @@ -48,8 +48,6 @@ export class EncryptionPair { export class AccountKeys { publicKey?: Uint8Array; - /** @deprecated July 2023, left for migration purposes*/ - cryptoMasterKeyAuto?: string; /** @deprecated July 2023, left for migration purposes*/ cryptoSymmetricKey?: EncryptionPair = new EncryptionPair< string, diff --git a/libs/common/src/platform/services/state.service.ts b/libs/common/src/platform/services/state.service.ts index a78a9b37a8c..284c8a7f2dc 100644 --- a/libs/common/src/platform/services/state.service.ts +++ b/libs/common/src/platform/services/state.service.ts @@ -222,45 +222,6 @@ export class StateService< await this.saveSecureStorageKey(partialKeys.userBiometricKey, value, options); } - /** - * @deprecated Use UserKeyAuto instead - */ - async setCryptoMasterKeyAuto(value: string | null, options?: StorageOptions): Promise { - options = this.reconcileOptions( - this.reconcileOptions(options, { keySuffix: "auto" }), - await this.defaultSecureStorageOptions(), - ); - if (options?.userId == null) { - return; - } - await this.saveSecureStorageKey(partialKeys.autoKey, value, options); - } - - /** - * @deprecated I don't see where this is even used - */ - async getCryptoMasterKeyB64(options?: StorageOptions): Promise { - options = this.reconcileOptions(options, await this.defaultSecureStorageOptions()); - if (options?.userId == null) { - return null; - } - return await this.secureStorageService.get( - `${options?.userId}${partialKeys.masterKey}`, - options, - ); - } - - /** - * @deprecated I don't see where this is even used - */ - async setCryptoMasterKeyB64(value: string, options?: StorageOptions): Promise { - options = this.reconcileOptions(options, await this.defaultSecureStorageOptions()); - if (options?.userId == null) { - return; - } - await this.saveSecureStorageKey(partialKeys.masterKey, value, options); - } - async getDuckDuckGoSharedKey(options?: StorageOptions): Promise { options = this.reconcileOptions(options, await this.defaultSecureStorageOptions()); if (options?.userId == null) { @@ -619,8 +580,6 @@ export class StateService< await this.setUserKeyAutoUnlock(null, { userId: userId }); await this.setUserKeyBiometric(null, { userId: userId }); - await this.setCryptoMasterKeyAuto(null, { userId: userId }); - await this.setCryptoMasterKeyB64(null, { userId: userId }); } protected async removeAccountFromMemory(userId: string = null): Promise { diff --git a/libs/key-management/src/abstractions/key.service.ts b/libs/key-management/src/abstractions/key.service.ts index 659dd1bbb29..4b257fbbebd 100644 --- a/libs/key-management/src/abstractions/key.service.ts +++ b/libs/key-management/src/abstractions/key.service.ts @@ -390,14 +390,6 @@ export abstract class KeyService { publicKey: string; privateKey: EncString; }>; - /** - * Previously, the master key was used for any additional key like the biometrics or pin key. - * We have switched to using the user key for these purposes. This method is for clearing the state - * of the older keys on logout or post migration. - * @param keySuffix The desired type of key to clear - * @param userId The desired user - */ - abstract clearDeprecatedKeys(keySuffix: KeySuffixOptions, userId?: string): Promise; /** * Retrieves all the keys needed for decrypting Ciphers diff --git a/libs/key-management/src/key.service.spec.ts b/libs/key-management/src/key.service.spec.ts index 2eab9de7487..a0afc160794 100644 --- a/libs/key-management/src/key.service.spec.ts +++ b/libs/key-management/src/key.service.spec.ts @@ -252,14 +252,6 @@ describe("keyService", () => { userId: mockUserId, }); }); - - it("clears the old deprecated Auto key whenever a User Key is set", async () => { - await keyService.setUserKey(mockUserKey, mockUserId); - - expect(stateService.setCryptoMasterKeyAuto).toHaveBeenCalledWith(null, { - userId: mockUserId, - }); - }); }); it("throws if key is null", async () => { diff --git a/libs/key-management/src/key.service.ts b/libs/key-management/src/key.service.ts index b18405a4200..870513f3913 100644 --- a/libs/key-management/src/key.service.ts +++ b/libs/key-management/src/key.service.ts @@ -254,16 +254,10 @@ export class DefaultKeyService implements KeyServiceAbstraction { // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. // eslint-disable-next-line @typescript-eslint/no-floating-promises this.stateService.setUserKeyAutoUnlock(null, { userId: userId }); - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.clearDeprecatedKeys(KeySuffixOptions.Auto, userId); } if (keySuffix === KeySuffixOptions.Pin && userId != null) { // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. this.pinService.clearPinKeyEncryptedUserKeyEphemeral(userId); - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.clearDeprecatedKeys(KeySuffixOptions.Pin, userId); } } @@ -565,7 +559,6 @@ export class DefaultKeyService implements KeyServiceAbstraction { await this.pinService.clearPinKeyEncryptedUserKeyPersistent(userId); await this.pinService.clearPinKeyEncryptedUserKeyEphemeral(userId); await this.pinService.clearUserKeyEncryptedPin(userId); - await this.clearDeprecatedKeys(KeySuffixOptions.Pin, userId); } async makeSendKey(keyMaterial: CsprngArray): Promise { @@ -726,7 +719,6 @@ export class DefaultKeyService implements KeyServiceAbstraction { } else { await this.stateService.setUserKeyAutoUnlock(null, { userId: userId }); } - await this.clearDeprecatedKeys(KeySuffixOptions.Auto, userId); const storePin = await this.shouldStoreKey(KeySuffixOptions.Pin, userId); if (storePin) { @@ -749,9 +741,6 @@ export class DefaultKeyService implements KeyServiceAbstraction { noPreExistingPersistentKey, userId, ); - // We can't always clear deprecated keys because the pin is only - // migrated once used to unlock - await this.clearDeprecatedKeys(KeySuffixOptions.Pin, userId); } else { await this.pinService.clearPinKeyEncryptedUserKeyPersistent(userId); await this.pinService.clearPinKeyEncryptedUserKeyEphemeral(userId); @@ -835,19 +824,6 @@ export class DefaultKeyService implements KeyServiceAbstraction { return [new SymmetricCryptoKey(newSymKey) as T, protectedSymKey]; } - // --LEGACY METHODS-- - // We previously used the master key for additional keys, but now we use the user key. - // These methods support migrating the old keys to the new ones. - // TODO: Remove after 2023.10 release (https://bitwarden.atlassian.net/browse/PM-3475) - - async clearDeprecatedKeys(keySuffix: KeySuffixOptions, userId?: UserId) { - if (keySuffix === KeySuffixOptions.Auto) { - await this.stateService.setCryptoMasterKeyAuto(null, { userId: userId }); - } else if (keySuffix === KeySuffixOptions.Pin && userId != null) { - await this.pinService.clearOldPinKeyEncryptedMasterKey(userId); - } - } - userKey$(userId: UserId): Observable { return this.stateProvider.getUser(userId, USER_KEY).state$; }