diff --git a/packages/keyring-controller/src/KeyringController.test.ts b/packages/keyring-controller/src/KeyringController.test.ts index f6f8a106108..a9ba9681712 100644 --- a/packages/keyring-controller/src/KeyringController.test.ts +++ b/packages/keyring-controller/src/KeyringController.test.ts @@ -91,18 +91,17 @@ describe('KeyringController', () => { () => new KeyringController({ messenger: buildKeyringControllerMessenger(), - cacheEncryptionKey: true, }), ).not.toThrow(); }); - it('should throw error if cacheEncryptionKey is true and encryptor does not support key export', () => { + it('should throw error if the encryptor does not support key export', () => { expect( () => - // @ts-expect-error testing an invalid encryptor new KeyringController({ messenger: buildKeyringControllerMessenger(), cacheEncryptionKey: true, + // @ts-expect-error testing an invalid encryptor encryptor: { encrypt: jest.fn(), decrypt: jest.fn() }, }), ).toThrow(KeyringControllerError.UnsupportedEncryptionKeyExport); @@ -556,259 +555,197 @@ describe('KeyringController', () => { }); describe('createNewVaultAndRestore', () => { - [false, true].map((cacheEncryptionKey) => - describe(`when cacheEncryptionKey is ${cacheEncryptionKey}`, () => { - it('should create new vault and restore', async () => { - await withController( - { cacheEncryptionKey }, - async ({ controller, initialState }) => { - const initialKeyrings = controller.state.keyrings; - await controller.createNewVaultAndRestore( - password, - uint8ArraySeed, - ); - expect(controller.state).not.toBe(initialState); - expect(controller.state.vault).toBeDefined(); - expect(controller.state.keyrings).toHaveLength( - initialKeyrings.length, - ); - // new keyring metadata should be generated - expect(controller.state.keyrings).not.toStrictEqual( - initialKeyrings, - ); - }, - ); - }); - - it('should call encryptor.encrypt with the same keyrings if old seedWord is used', async () => { - await withController( - { cacheEncryptionKey }, - async ({ controller, encryptor }) => { - const encryptSpy = jest.spyOn(encryptor, 'encryptWithKey'); - const serializedKeyring = await controller.withKeyring( - { type: 'HD Key Tree' }, - async ({ keyring }) => keyring.serialize(), - ); - const currentSeedWord = - await controller.exportSeedPhrase(password); + it('should create new vault and restore', async () => { + await withController(async ({ controller, initialState }) => { + const initialKeyrings = controller.state.keyrings; + await controller.createNewVaultAndRestore(password, uint8ArraySeed); + expect(controller.state).not.toBe(initialState); + expect(controller.state.vault).toBeDefined(); + expect(controller.state.keyrings).toHaveLength(initialKeyrings.length); + // new keyring metadata should be generated + expect(controller.state.keyrings).not.toStrictEqual(initialKeyrings); + }); + }); + + it('should call encryptor.encrypt with the same keyrings if old seedWord is used', async () => { + await withController(async ({ controller, encryptor }) => { + const encryptSpy = jest.spyOn(encryptor, 'encryptWithKey'); + const serializedKeyring = await controller.withKeyring( + { type: 'HD Key Tree' }, + async ({ keyring }) => keyring.serialize(), + ); + const currentSeedWord = await controller.exportSeedPhrase(password); - await controller.createNewVaultAndRestore( - password, - currentSeedWord, - ); + await controller.createNewVaultAndRestore(password, currentSeedWord); - const key = JSON.parse(MOCK_ENCRYPTION_KEY); - expect(encryptSpy).toHaveBeenCalledWith(key, [ - { - data: serializedKeyring, - type: 'HD Key Tree', - metadata: { - id: expect.any(String), - name: '', - }, - }, - ]); + const key = JSON.parse(MOCK_ENCRYPTION_KEY); + expect(encryptSpy).toHaveBeenCalledWith(key, [ + { + data: serializedKeyring, + type: 'HD Key Tree', + metadata: { + id: expect.any(String), + name: '', }, - ); - }); + }, + ]); + }); + }); - it('should throw error if creating new vault and restore without password', async () => { - await withController( - { cacheEncryptionKey }, - async ({ controller }) => { - await expect( - controller.createNewVaultAndRestore('', uint8ArraySeed), - ).rejects.toThrow(KeyringControllerError.InvalidEmptyPassword); - }, - ); - }); + it('should throw error if creating new vault and restore without password', async () => { + await withController(async ({ controller }) => { + await expect( + controller.createNewVaultAndRestore('', uint8ArraySeed), + ).rejects.toThrow(KeyringControllerError.InvalidEmptyPassword); + }); + }); - it('should throw error if creating new vault and restoring without seed phrase', async () => { - await withController( - { cacheEncryptionKey }, - async ({ controller }) => { - await expect( - controller.createNewVaultAndRestore( - password, - // @ts-expect-error invalid seed phrase - '', - ), - ).rejects.toThrow( - 'Eth-Hd-Keyring: Deserialize method cannot be called with an opts value for numberOfAccounts and no menmonic', - ); - }, - ); - }); + it('should throw error if creating new vault and restoring without seed phrase', async () => { + await withController(async ({ controller }) => { + await expect( + controller.createNewVaultAndRestore( + password, + // @ts-expect-error invalid seed phrase + '', + ), + ).rejects.toThrow( + 'Eth-Hd-Keyring: Deserialize method cannot be called with an opts value for numberOfAccounts and no menmonic', + ); + }); + }); - cacheEncryptionKey && - it('should set encryptionKey and encryptionSalt in state', async () => { - await withController( - { cacheEncryptionKey }, - async ({ controller }) => { - await controller.createNewVaultAndRestore( - password, - uint8ArraySeed, - ); - expect(controller.state.encryptionKey).toBeDefined(); - expect(controller.state.encryptionSalt).toBeDefined(); - }, - ); - }); - }), - ); + it('should set encryptionKey and encryptionSalt in state', async () => { + await withController(async ({ controller }) => { + await controller.createNewVaultAndRestore(password, uint8ArraySeed); + expect(controller.state.encryptionKey).toBeDefined(); + expect(controller.state.encryptionSalt).toBeDefined(); + }); + }); }); describe('createNewVaultAndKeychain', () => { - [false, true].map((cacheEncryptionKey) => - describe(`when cacheEncryptionKey is ${cacheEncryptionKey}`, () => { - describe('when there is no existing vault', () => { - it('should create new vault, mnemonic and keychain', async () => { - await withController( - { cacheEncryptionKey, skipVaultCreation: true }, - async ({ controller }) => { - await controller.createNewVaultAndKeychain(password); - - const currentSeedPhrase = - await controller.exportSeedPhrase(password); - - expect(currentSeedPhrase.length).toBeGreaterThan(0); - expect( - isValidHexAddress( - controller.state.keyrings[0].accounts[0] as Hex, - ), - ).toBe(true); - expect(controller.state.vault).toBeDefined(); - }, - ); - }); + describe('when there is no existing vault', () => { + it('should create new vault, mnemonic and keychain', async () => { + await withController( + { skipVaultCreation: true }, + async ({ controller }) => { + await controller.createNewVaultAndKeychain(password); - cacheEncryptionKey && - it('should set encryptionKey and encryptionSalt in state', async () => { - await withController( - { cacheEncryptionKey, skipVaultCreation: true }, - async ({ controller }) => { - await controller.createNewVaultAndKeychain(password); + const currentSeedPhrase = + await controller.exportSeedPhrase(password); - expect(controller.state.encryptionKey).toBeDefined(); - expect(controller.state.encryptionSalt).toBeDefined(); - }, - ); - }); + expect(currentSeedPhrase.length).toBeGreaterThan(0); + expect( + isValidHexAddress( + controller.state.keyrings[0].accounts[0] as Hex, + ), + ).toBe(true); + expect(controller.state.vault).toBeDefined(); + }, + ); + }); - it('should set default state', async () => { - await withController( - { cacheEncryptionKey, skipVaultCreation: true }, - async ({ controller }) => { - await controller.createNewVaultAndKeychain(password); - - expect(controller.state.keyrings).not.toStrictEqual([]); - const keyring = controller.state.keyrings[0]; - expect(keyring.accounts).not.toStrictEqual([]); - expect(keyring.type).toBe('HD Key Tree'); - expect(controller.state.vault).toBeDefined(); - }, - ); - }); + it('should set encryptionKey and encryptionSalt in state', async () => { + await withController( + { skipVaultCreation: true }, + async ({ controller }) => { + await controller.createNewVaultAndKeychain(password); - it('should throw error if password is of wrong type', async () => { - await withController( - { cacheEncryptionKey, skipVaultCreation: true }, - async ({ controller }) => { - await expect( - controller.createNewVaultAndKeychain( - // @ts-expect-error invalid password - 123, - ), - ).rejects.toThrow(KeyringControllerError.WrongPasswordType); - }, - ); - }); + expect(controller.state.encryptionKey).toBeDefined(); + expect(controller.state.encryptionSalt).toBeDefined(); + }, + ); + }); - it('should throw error if the first account is not found on the keyring', async () => { - jest.spyOn(HdKeyring.prototype, 'getAccounts').mockReturnValue([]); - await withController( - { cacheEncryptionKey, skipVaultCreation: true }, - async ({ controller }) => { - await expect( - controller.createNewVaultAndKeychain(password), - ).rejects.toThrow(KeyringControllerError.NoFirstAccount); - }, - ); - }); + it('should set default state', async () => { + await withController( + { skipVaultCreation: true }, + async ({ controller }) => { + await controller.createNewVaultAndKeychain(password); - !cacheEncryptionKey && - it('should not set encryptionKey and encryptionSalt in state', async () => { - await withController( - { skipVaultCreation: true }, - async ({ controller }) => { - await controller.createNewVaultAndKeychain(password); + expect(controller.state.keyrings).not.toStrictEqual([]); + const keyring = controller.state.keyrings[0]; + expect(keyring.accounts).not.toStrictEqual([]); + expect(keyring.type).toBe('HD Key Tree'); + expect(controller.state.vault).toBeDefined(); + }, + ); + }); - expect(controller.state).not.toHaveProperty('encryptionKey'); - expect(controller.state).not.toHaveProperty('encryptionSalt'); - }, - ); - }); - }); + it('should throw error if password is of wrong type', async () => { + await withController( + { skipVaultCreation: true }, + async ({ controller }) => { + await expect( + controller.createNewVaultAndKeychain( + // @ts-expect-error invalid password + 123, + ), + ).rejects.toThrow(KeyringControllerError.WrongPasswordType); + }, + ); + }); - describe('when there is an existing vault', () => { - it('should not create a new vault or keychain', async () => { - await withController( - { cacheEncryptionKey }, - async ({ controller, initialState }) => { - const initialSeedWord = - await controller.exportSeedPhrase(password); - expect(initialSeedWord).toBeDefined(); - const initialVault = controller.state.vault; - - await controller.createNewVaultAndKeychain(password); - - const currentSeedWord = - await controller.exportSeedPhrase(password); - expect(initialState).toStrictEqual(controller.state); - expect(initialSeedWord).toBe(currentSeedWord); - expect(initialVault).toStrictEqual(controller.state.vault); - }, - ); - }); + it('should throw error if the first account is not found on the keyring', async () => { + jest.spyOn(HdKeyring.prototype, 'getAccounts').mockReturnValue([]); + await withController( + { skipVaultCreation: true }, + async ({ controller }) => { + await expect( + controller.createNewVaultAndKeychain(password), + ).rejects.toThrow(KeyringControllerError.NoFirstAccount); + }, + ); + }); - cacheEncryptionKey && - it('should set encryptionKey and encryptionSalt in state', async () => { - await withController( - { cacheEncryptionKey }, - async ({ controller }) => { - await controller.setLocked(); - expect(controller.state.encryptionKey).toBeUndefined(); - expect(controller.state.encryptionSalt).toBeUndefined(); + it('should not set encryptionKey and encryptionSalt in state', async () => { + await withController( + { skipVaultCreation: true }, + async ({ controller }) => { + await controller.createNewVaultAndKeychain(password); - await controller.createNewVaultAndKeychain(password); + expect(controller.state).not.toHaveProperty('encryptionKey'); + expect(controller.state).not.toHaveProperty('encryptionSalt'); + }, + ); + }); + }); - expect(controller.state.encryptionKey).toBeDefined(); - expect(controller.state.encryptionSalt).toBeDefined(); - }, - ); - }); + describe('when there is an existing vault', () => { + it('should not create a new vault or keychain', async () => { + await withController(async ({ controller, initialState }) => { + const initialSeedWord = await controller.exportSeedPhrase(password); + expect(initialSeedWord).toBeDefined(); + const initialVault = controller.state.vault; - !cacheEncryptionKey && - it('should not set encryptionKey and encryptionSalt in state', async () => { - await withController( - { skipVaultCreation: false, cacheEncryptionKey }, - async ({ controller }) => { - await controller.createNewVaultAndKeychain(password); + await controller.createNewVaultAndKeychain(password); - expect(controller.state).not.toHaveProperty('encryptionKey'); - expect(controller.state).not.toHaveProperty('encryptionSalt'); - }, - ); - }); + const currentSeedWord = await controller.exportSeedPhrase(password); + expect(initialState).toStrictEqual(controller.state); + expect(initialSeedWord).toBe(currentSeedWord); + expect(initialVault).toStrictEqual(controller.state.vault); + }); + }); + + it('should set encryptionKey and encryptionSalt in state', async () => { + await withController(async ({ controller }) => { + await controller.setLocked(); + expect(controller.state.encryptionKey).toBeUndefined(); + expect(controller.state.encryptionSalt).toBeUndefined(); + + await controller.createNewVaultAndKeychain(password); + + expect(controller.state.encryptionKey).toBeDefined(); + expect(controller.state.encryptionSalt).toBeDefined(); }); - }), - ); + }); + }); }); describe('envelope encryption', () => { it('should create new vault with encryption key', async () => { await withController( - { cacheEncryptionKey: true, envelopeEncryption: true }, + { envelopeEncryption: true }, async ({ controller }) => { expect(controller.state.encryptionKey).toBeDefined(); expect(controller.state.encryptedEncryptionKey).toBeDefined(); @@ -816,20 +753,9 @@ describe('KeyringController', () => { ); }); - it('should throw error if creating new vault with encryption key and cacheEncryptionKey is false', async () => { - await withController( - { cacheEncryptionKey: false, skipVaultCreation: true }, - async ({ controller }) => { - await expect( - controller.createNewVaultAndKeychain(password, MOCK_ENCRYPTION_KEY), - ).rejects.toThrow(KeyringControllerError.CacheEncryptionKeyDisabled); - }, - ); - }); - it('should unlock with password', async () => { await withController( - { cacheEncryptionKey: true, envelopeEncryption: true }, + { envelopeEncryption: true }, async ({ controller }) => { expect(controller.isUnlocked()).toBe(true); expect(controller.state.isUnlocked).toBe(true); @@ -849,7 +775,7 @@ describe('KeyringController', () => { it('should lock and unlock after change password', async () => { await withController( - { cacheEncryptionKey: true, envelopeEncryption: true }, + { envelopeEncryption: true }, async ({ controller }) => { await controller.changePassword(password2); @@ -871,7 +797,7 @@ describe('KeyringController', () => { it('should lock and unlock after change password and key', async () => { await withController( - { cacheEncryptionKey: true, envelopeEncryption: true }, + { envelopeEncryption: true }, async ({ controller }) => { await controller.changePasswordAndEncryptionKey( password2, @@ -896,7 +822,7 @@ describe('KeyringController', () => { it('should verify password', async () => { await withController( - { cacheEncryptionKey: true, envelopeEncryption: true }, + { envelopeEncryption: true }, async ({ controller }) => { expect(await controller.verifyPassword(password)).toBeUndefined(); }, @@ -905,7 +831,7 @@ describe('KeyringController', () => { it('should throw error on verify wrong password', async () => { await withController( - { cacheEncryptionKey: true, envelopeEncryption: true }, + { envelopeEncryption: true }, async ({ controller }) => { await expect(controller.verifyPassword(password2)).rejects.toThrow( DECRYPTION_ERROR, @@ -916,7 +842,7 @@ describe('KeyringController', () => { it('should unlock with old key after change password failed', async () => { await withController( - { cacheEncryptionKey: true, envelopeEncryption: true }, + { envelopeEncryption: true }, async ({ controller, encryptor }) => { // Make change password fail. jest.spyOn(encryptor, 'encryptWithKey').mockImplementationOnce(() => { @@ -2731,587 +2657,524 @@ describe('KeyringController', () => { }); describe('changePassword', () => { - [false, true].map((cacheEncryptionKey) => - describe(`when cacheEncryptionKey is ${cacheEncryptionKey}`, () => { - it('should encrypt the vault with the new password', async () => { - await withController( - { cacheEncryptionKey }, - async ({ controller, encryptor }) => { - const newPassword = 'new-password'; - const spiedEncryptionFn = jest.spyOn( - encryptor, - cacheEncryptionKey ? 'encryptWithDetail' : 'encrypt', - ); + it('should encrypt the vault with the new password', async () => { + await withController(async ({ controller, encryptor }) => { + const newPassword = 'new-password'; + const spiedEncryptionFn = jest.spyOn(encryptor, 'encryptWithDetail'); - await controller.changePassword(newPassword); + await controller.changePassword(newPassword); - // we pick the first argument of the first call - expect(spiedEncryptionFn.mock.calls[0][0]).toBe(newPassword); - }, - ); - }); + // we pick the first argument of the first call + expect(spiedEncryptionFn.mock.calls[0][0]).toBe(newPassword); + }); + }); - it('should throw error if `isUnlocked` is false', async () => { - await withController( - { cacheEncryptionKey }, - async ({ controller }) => { - await controller.setLocked(); + it('should throw error if `isUnlocked` is false', async () => { + await withController(async ({ controller }) => { + await controller.setLocked(); - await expect(async () => - controller.changePassword(''), - ).rejects.toThrow(KeyringControllerError.ControllerLocked); - }, - ); - }); + await expect(async () => controller.changePassword('')).rejects.toThrow( + KeyringControllerError.ControllerLocked, + ); + }); + }); - it('should throw error if the new password is an empty string', async () => { - await withController( - { cacheEncryptionKey }, - async ({ controller }) => { - await expect(controller.changePassword('')).rejects.toThrow( - KeyringControllerError.InvalidEmptyPassword, - ); - }, - ); - }); + it('should throw error if the new password is an empty string', async () => { + await withController(async ({ controller }) => { + await expect(controller.changePassword('')).rejects.toThrow( + KeyringControllerError.InvalidEmptyPassword, + ); + }); + }); - it('should throw error if the new password is undefined', async () => { - await withController( - { cacheEncryptionKey }, - async ({ controller }) => { - await expect( - // @ts-expect-error we are testing wrong input - controller.changePassword(undefined), - ).rejects.toThrow(KeyringControllerError.WrongPasswordType); - }, - ); - }); + it('should throw error if the new password is undefined', async () => { + await withController(async ({ controller }) => { + await expect( + // @ts-expect-error we are testing wrong input + controller.changePassword(undefined), + ).rejects.toThrow(KeyringControllerError.WrongPasswordType); + }); + }); - it('should throw error when the controller is locked', async () => { - await withController(async ({ controller }) => { - await controller.setLocked(); + it('should throw error when the controller is locked', async () => { + await withController(async ({ controller }) => { + await controller.setLocked(); - await expect(async () => - controller.changePassword('whatever'), - ).rejects.toThrow(KeyringControllerError.ControllerLocked); - }); - }); - }), - ); + await expect(async () => + controller.changePassword('whatever'), + ).rejects.toThrow(KeyringControllerError.ControllerLocked); + }); + }); }); describe('submitPassword', () => { - [false, true].map((cacheEncryptionKey) => - describe(`when cacheEncryptionKey is ${cacheEncryptionKey}`, () => { - it('should submit password and decrypt', async () => { - await withController( - { cacheEncryptionKey }, - async ({ controller, initialState }) => { - await controller.submitPassword(password); - expect(controller.state).toStrictEqual(initialState); - }, - ); - }); + it('should submit password and decrypt', async () => { + await withController(async ({ controller, initialState }) => { + await controller.submitPassword(password); + expect(controller.state).toStrictEqual(initialState); + }); + }); - it('should emit KeyringController:unlock event', async () => { - await withController( - { cacheEncryptionKey }, - async ({ controller, messenger }) => { - const listener = sinon.spy(); - messenger.subscribe('KeyringController:unlock', listener); - await controller.submitPassword(password); - expect(listener.called).toBe(true); - }, - ); - }); + it('should emit KeyringController:unlock event', async () => { + await withController(async ({ controller, messenger }) => { + const listener = sinon.spy(); + messenger.subscribe('KeyringController:unlock', listener); + await controller.submitPassword(password); + expect(listener.called).toBe(true); + }); + }); - it('should unlock also with unsupported keyrings', async () => { - await withController( + it('should unlock also with unsupported keyrings', async () => { + await withController( + { + skipVaultCreation: true, + state: { vault: freshVault }, + }, + async ({ controller, encryptor }) => { + jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ { - cacheEncryptionKey, - skipVaultCreation: true, - state: { vault: freshVault }, + type: 'UnsupportedKeyring', + data: '0x1234', }, - async ({ controller, encryptor }) => { - jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ - { - type: 'UnsupportedKeyring', - data: '0x1234', - }, - ]); + ]); - await controller.submitPassword(password); + await controller.submitPassword(password); - expect(controller.state.isUnlocked).toBe(true); - }, - ); - }); + expect(controller.state.isUnlocked).toBe(true); + }, + ); + }); - it('should throw error if vault unlocked has an unexpected shape', async () => { - await withController( + it('should throw error if vault unlocked has an unexpected shape', async () => { + await withController( + { + skipVaultCreation: true, + state: { vault: freshVault }, + }, + async ({ controller, encryptor }) => { + jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ { - cacheEncryptionKey, - skipVaultCreation: true, - state: { vault: freshVault }, + foo: 'bar', }, - async ({ controller, encryptor }) => { - jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ - { - foo: 'bar', - }, - ]); + ]); - await expect(controller.submitPassword(password)).rejects.toThrow( - KeyringControllerError.VaultDataError, - ); - }, + await expect(controller.submitPassword(password)).rejects.toThrow( + KeyringControllerError.VaultDataError, ); - }); + }, + ); + }); - it('should throw error if vault is missing', async () => { - await withController( - { skipVaultCreation: true }, - async ({ controller }) => { - await expect(controller.submitPassword(password)).rejects.toThrow( - KeyringControllerError.VaultError, - ); - }, - ); - }); + it('should throw error if vault is missing', async () => { + await withController( + { skipVaultCreation: true }, + async ({ controller }) => { + await expect(controller.submitPassword(password)).rejects.toThrow( + KeyringControllerError.VaultError, + ); + }, + ); + }); - it('should unlock succesfully when the controller is instantiated with an existing `keyringsMetadata`', async () => { - // @ts-expect-error HdKeyring is not yet compatible with Keyring type. - stubKeyringClassWithAccount(HdKeyring, '0x123'); - await withController( + it('should unlock succesfully when the controller is instantiated with an existing `keyringsMetadata`', async () => { + // @ts-expect-error HdKeyring is not yet compatible with Keyring type. + stubKeyringClassWithAccount(HdKeyring, '0x123'); + await withController( + { + state: { vault: freshVault }, + skipVaultCreation: true, + }, + async ({ controller, encryptor }) => { + jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ { - cacheEncryptionKey, - state: { vault: freshVault }, - skipVaultCreation: true, + type: KeyringTypes.hd, + data: { + accounts: ['0x123'], + }, + metadata: { + id: '123', + name: '', + }, }, - async ({ controller, encryptor }) => { - jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ - { - type: KeyringTypes.hd, - data: { - accounts: ['0x123'], - }, - metadata: { - id: '123', - name: '', - }, - }, - ]); + ]); - await controller.submitPassword(password); + await controller.submitPassword(password); - expect(controller.state.keyrings).toStrictEqual([ - { - type: KeyringTypes.hd, - accounts: ['0x123'], - metadata: { - id: '123', - name: '', - }, - }, - ]); + expect(controller.state.keyrings).toStrictEqual([ + { + type: KeyringTypes.hd, + accounts: ['0x123'], + metadata: { + id: '123', + name: '', + }, }, - ); - }); + ]); + }, + ); + }); - cacheEncryptionKey && - it('should generate new metadata when there is no metadata in the vault and cacheEncryptionKey is enabled', async () => { - const hdKeyringSerializeSpy = jest.spyOn( - HdKeyring.prototype, - 'serialize', - ); - await withController( - { - cacheEncryptionKey: true, - state: { - vault: freshVault, - }, - skipVaultCreation: true, + it('should generate new metadata when there is no metadata in the vault and cacheEncryptionKey is enabled', async () => { + const hdKeyringSerializeSpy = jest.spyOn( + HdKeyring.prototype, + 'serialize', + ); + await withController( + { + state: { + vault: freshVault, + }, + skipVaultCreation: true, + }, + async ({ controller, encryptor }) => { + const encryptWithKeySpy = jest.spyOn(encryptor, 'encryptWithKey'); + jest.spyOn(encryptor, 'importKey').mockResolvedValue('imported key'); + jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ + { + type: KeyringTypes.hd, + data: { + accounts: ['0x123'], }, - async ({ controller, encryptor }) => { - const encryptWithKeySpy = jest.spyOn( - encryptor, - 'encryptWithKey', - ); - jest - .spyOn(encryptor, 'importKey') - .mockResolvedValue('imported key'); - jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ - { - type: KeyringTypes.hd, - data: { - accounts: ['0x123'], - }, - }, - ]); - hdKeyringSerializeSpy.mockResolvedValue({ - // @ts-expect-error we are assigning a mock value - accounts: ['0x123'], - }); + }, + ]); + hdKeyringSerializeSpy.mockResolvedValue({ + // @ts-expect-error we are assigning a mock value + accounts: ['0x123'], + }); - await controller.submitPassword(password); + await controller.submitPassword(password); - expect(controller.state.keyrings).toStrictEqual([ - { - type: KeyringTypes.hd, - accounts: expect.any(Array), - metadata: { - id: expect.any(String), - name: '', - }, - }, - ]); - expect(encryptWithKeySpy).toHaveBeenCalledWith('imported key', [ - { - type: KeyringTypes.hd, - data: { - accounts: ['0x123'], - }, - metadata: { - id: expect.any(String), - name: '', - }, - }, - ]); + expect(controller.state.keyrings).toStrictEqual([ + { + type: KeyringTypes.hd, + accounts: expect.any(Array), + metadata: { + id: expect.any(String), + name: '', }, - ); - }); + }, + ]); + expect(encryptWithKeySpy).toHaveBeenCalledWith('imported key', [ + { + type: KeyringTypes.hd, + data: { + accounts: ['0x123'], + }, + metadata: { + id: expect.any(String), + name: '', + }, + }, + ]); + }, + ); + }); - !cacheEncryptionKey && - it('should generate new metadata when there is no metadata in the vault and cacheEncryptionKey is disabled', async () => { - const hdKeyringSerializeSpy = jest.spyOn( - HdKeyring.prototype, - 'serialize', - ); - await withController( - { - cacheEncryptionKey: false, - state: { - vault: freshVault, - }, - skipVaultCreation: true, + it('should generate new metadata when there is no metadata in the vault and cacheEncryptionKey is disabled', async () => { + const hdKeyringSerializeSpy = jest.spyOn( + HdKeyring.prototype, + 'serialize', + ); + await withController( + { + state: { + vault: freshVault, + }, + skipVaultCreation: true, + }, + async ({ controller, encryptor }) => { + const encryptSpy = jest.spyOn(encryptor, 'encrypt'); + jest.spyOn(encryptor, 'decrypt').mockResolvedValueOnce([ + { + type: KeyringTypes.hd, + data: { + accounts: ['0x123'], }, - async ({ controller, encryptor }) => { - const encryptSpy = jest.spyOn(encryptor, 'encrypt'); - jest.spyOn(encryptor, 'decrypt').mockResolvedValueOnce([ - { - type: KeyringTypes.hd, - data: { - accounts: ['0x123'], - }, - }, - ]); - hdKeyringSerializeSpy.mockResolvedValue({ - // @ts-expect-error we are assigning a mock value - accounts: ['0x123'], - }); + }, + ]); + hdKeyringSerializeSpy.mockResolvedValue({ + // @ts-expect-error we are assigning a mock value + accounts: ['0x123'], + }); - await controller.submitPassword(password); + await controller.submitPassword(password); - expect(controller.state.keyrings).toStrictEqual([ - { - type: KeyringTypes.hd, - accounts: expect.any(Array), - metadata: { - id: expect.any(String), - name: '', - }, - }, - ]); - expect(encryptSpy).toHaveBeenCalledWith(password, [ - { - type: KeyringTypes.hd, - data: { - accounts: ['0x123'], - }, - metadata: { - id: expect.any(String), - name: '', - }, - }, - ]); + expect(controller.state.keyrings).toStrictEqual([ + { + type: KeyringTypes.hd, + accounts: expect.any(Array), + metadata: { + id: expect.any(String), + name: '', }, - ); - }); + }, + ]); + expect(encryptSpy).toHaveBeenCalledWith(password, [ + { + type: KeyringTypes.hd, + data: { + accounts: ['0x123'], + }, + metadata: { + id: expect.any(String), + name: '', + }, + }, + ]); + }, + ); + }); - it('should unlock the wallet if the state has a duplicate account and the encryption parameters are outdated', async () => { - stubKeyringClassWithAccount(MockKeyring, '0x123'); - // @ts-expect-error HdKeyring is not yet compatible with Keyring type. - stubKeyringClassWithAccount(HdKeyring, '0x123'); - await withController( + it('should unlock the wallet if the state has a duplicate account and the encryption parameters are outdated', async () => { + stubKeyringClassWithAccount(MockKeyring, '0x123'); + // @ts-expect-error HdKeyring is not yet compatible with Keyring type. + stubKeyringClassWithAccount(HdKeyring, '0x123'); + await withController( + { + skipVaultCreation: true, + state: { vault: freshVault }, + keyringBuilders: [keyringBuilderFactory(MockKeyring)], + }, + async ({ controller, encryptor, messenger }) => { + const unlockListener = jest.fn(); + messenger.subscribe('KeyringController:unlock', unlockListener); + jest.spyOn(encryptor, 'isVaultUpdated').mockReturnValue(false); + jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ { - skipVaultCreation: true, - cacheEncryptionKey, - state: { vault: freshVault }, - keyringBuilders: [keyringBuilderFactory(MockKeyring)], + type: KeyringTypes.hd, + data: {}, }, - async ({ controller, encryptor, messenger }) => { - const unlockListener = jest.fn(); - messenger.subscribe('KeyringController:unlock', unlockListener); - jest.spyOn(encryptor, 'isVaultUpdated').mockReturnValue(false); - jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ - { - type: KeyringTypes.hd, - data: {}, - }, - { - type: MockKeyring.type, - data: {}, - }, - ]); + { + type: MockKeyring.type, + data: {}, + }, + ]); - await controller.submitPassword(password); + await controller.submitPassword(password); - expect(controller.state.isUnlocked).toBe(true); - expect(unlockListener).toHaveBeenCalledTimes(1); - }, - ); - }); + expect(controller.state.isUnlocked).toBe(true); + expect(unlockListener).toHaveBeenCalledTimes(1); + }, + ); + }); - it('should unlock the wallet also if encryption parameters are outdated and the vault upgrade fails', async () => { - await withController( + it('should unlock the wallet also if encryption parameters are outdated and the vault upgrade fails', async () => { + await withController( + { + skipVaultCreation: true, + state: { vault: freshVault }, + }, + async ({ controller, encryptor }) => { + jest.spyOn(encryptor, 'isVaultUpdated').mockReturnValue(false); + jest.spyOn(encryptor, 'encrypt').mockRejectedValue(new Error()); + jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ { - skipVaultCreation: true, - cacheEncryptionKey, - state: { vault: freshVault }, + type: KeyringTypes.hd, + data: { + accounts: ['0x123'], + }, }, - async ({ controller, encryptor }) => { - jest.spyOn(encryptor, 'isVaultUpdated').mockReturnValue(false); - jest.spyOn(encryptor, 'encrypt').mockRejectedValue(new Error()); - jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ - { - type: KeyringTypes.hd, - data: { - accounts: ['0x123'], - }, - }, - ]); + ]); - await controller.submitPassword(password); + await controller.submitPassword(password); - expect(controller.state.isUnlocked).toBe(true); - }, - ); - }); + expect(controller.state.isUnlocked).toBe(true); + }, + ); + }); - it('should unlock the wallet discarding existing duplicate accounts', async () => { - stubKeyringClassWithAccount(MockKeyring, '0x123'); - // @ts-expect-error HdKeyring is not yet compatible with Keyring type. - stubKeyringClassWithAccount(HdKeyring, '0x123'); - await withController( + it('should unlock the wallet discarding existing duplicate accounts', async () => { + stubKeyringClassWithAccount(MockKeyring, '0x123'); + // @ts-expect-error HdKeyring is not yet compatible with Keyring type. + stubKeyringClassWithAccount(HdKeyring, '0x123'); + await withController( + { + skipVaultCreation: true, + state: { vault: freshVault }, + keyringBuilders: [keyringBuilderFactory(MockKeyring)], + }, + async ({ controller, encryptor, messenger }) => { + const unlockListener = jest.fn(); + messenger.subscribe('KeyringController:unlock', unlockListener); + jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ { - skipVaultCreation: true, - cacheEncryptionKey, - state: { vault: freshVault }, - keyringBuilders: [keyringBuilderFactory(MockKeyring)], + type: KeyringTypes.hd, + data: {}, }, - async ({ controller, encryptor, messenger }) => { - const unlockListener = jest.fn(); - messenger.subscribe('KeyringController:unlock', unlockListener); - jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ - { - type: KeyringTypes.hd, - data: {}, - }, - { - type: MockKeyring.type, - data: {}, - }, - ]); + { + type: MockKeyring.type, + data: {}, + }, + ]); - await controller.submitPassword(password); + await controller.submitPassword(password); - expect(controller.state.keyrings).toHaveLength(1); // Second keyring will be skipped as "unsupported". - expect(unlockListener).toHaveBeenCalledTimes(1); - }, - ); - }); + expect(controller.state.keyrings).toHaveLength(1); // Second keyring will be skipped as "unsupported". + expect(unlockListener).toHaveBeenCalledTimes(1); + }, + ); + }); - cacheEncryptionKey && - it('should upgrade the vault encryption if the key encryptor has different parameters', async () => { - await withController( - { - skipVaultCreation: true, - cacheEncryptionKey, - state: { vault: freshVault }, + it('should upgrade the vault encryption if the key encryptor has different parameters', async () => { + await withController( + { + skipVaultCreation: true, + state: { vault: freshVault }, + }, + async ({ controller, encryptor }) => { + jest.spyOn(encryptor, 'isVaultUpdated').mockReturnValue(false); + const encryptSpy = jest.spyOn(encryptor, 'encryptWithKey'); + jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ + { + type: KeyringTypes.hd, + data: { + accounts: ['0x123'], }, - async ({ controller, encryptor }) => { - jest.spyOn(encryptor, 'isVaultUpdated').mockReturnValue(false); - const encryptSpy = jest.spyOn(encryptor, 'encryptWithKey'); - jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ - { - type: KeyringTypes.hd, - data: { - accounts: ['0x123'], - }, - }, - ]); + }, + ]); - await controller.submitPassword(password); + await controller.submitPassword(password); - expect(encryptSpy).toHaveBeenCalledTimes(1); - }, - ); - }); + expect(encryptSpy).toHaveBeenCalledTimes(1); + }, + ); + }); - cacheEncryptionKey && - it('should not upgrade the vault encryption if the key encryptor has the same parameters', async () => { - await withController( - { - skipVaultCreation: true, - cacheEncryptionKey, - state: { vault: freshVault }, + it('should not upgrade the vault encryption if the key encryptor has the same parameters', async () => { + await withController( + { + skipVaultCreation: true, + state: { vault: freshVault }, + }, + async ({ controller, encryptor }) => { + jest.spyOn(encryptor, 'isVaultUpdated').mockReturnValue(true); + const encryptSpy = jest.spyOn(encryptor, 'encrypt'); + jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ + { + type: KeyringTypes.hd, + data: { + accounts: ['0x123'], }, - async ({ controller, encryptor }) => { - jest.spyOn(encryptor, 'isVaultUpdated').mockReturnValue(true); - const encryptSpy = jest.spyOn(encryptor, 'encrypt'); - jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ - { - type: KeyringTypes.hd, - data: { - accounts: ['0x123'], - }, - }, - ]); + }, + ]); - // TODO actually this does trigger re-encryption. The catch is - // that this test is run with cacheEncryptionKey enabled, so - // `encryptWithKey` is being used instead of `encrypt`. Hence, - // the spy on `encrypt` doesn't trigger. - await controller.submitPassword(password); + // TODO actually this does trigger re-encryption. The catch is + // that this test is run with cacheEncryptionKey enabled, so + // `encryptWithKey` is being used instead of `encrypt`. Hence, + // the spy on `encrypt` doesn't trigger. + await controller.submitPassword(password); - expect(encryptSpy).not.toHaveBeenCalled(); - }, - ); - }); + expect(encryptSpy).not.toHaveBeenCalled(); + }, + ); + }); - !cacheEncryptionKey && - it('should upgrade the vault encryption if the generic encryptor has different parameters', async () => { - await withController( - { - skipVaultCreation: true, - cacheEncryptionKey, - state: { vault: freshVault }, + it('should upgrade the vault encryption if the generic encryptor has different parameters', async () => { + await withController( + { + skipVaultCreation: true, + state: { vault: freshVault }, + }, + async ({ controller, encryptor }) => { + jest.spyOn(encryptor, 'isVaultUpdated').mockReturnValue(false); + const encryptSpy = jest.spyOn(encryptor, 'encrypt'); + jest.spyOn(encryptor, 'decrypt').mockResolvedValueOnce([ + { + type: KeyringTypes.hd, + data: { + accounts: ['0x123'], }, - async ({ controller, encryptor }) => { - jest.spyOn(encryptor, 'isVaultUpdated').mockReturnValue(false); - const encryptSpy = jest.spyOn(encryptor, 'encrypt'); - jest.spyOn(encryptor, 'decrypt').mockResolvedValueOnce([ - { - type: KeyringTypes.hd, - data: { - accounts: ['0x123'], - }, - }, - ]); + }, + ]); - await controller.submitPassword(password); + await controller.submitPassword(password); - expect(encryptSpy).toHaveBeenCalledTimes(1); - }, - ); - }); + expect(encryptSpy).toHaveBeenCalledTimes(1); + }, + ); + }); - it('should not upgrade the vault encryption if the encryptor has the same parameters and the keyring has metadata', async () => { - await withController( + it('should not upgrade the vault encryption if the encryptor has the same parameters and the keyring has metadata', async () => { + await withController( + { + skipVaultCreation: true, + state: { vault: freshVault }, + }, + async ({ controller, encryptor }) => { + jest.spyOn(encryptor, 'isVaultUpdated').mockReturnValue(true); + const encryptSpy = jest.spyOn(encryptor, 'encrypt'); + jest.spyOn(encryptor, 'decrypt').mockResolvedValueOnce([ { - skipVaultCreation: true, - cacheEncryptionKey, - state: { vault: freshVault }, + type: KeyringTypes.hd, + data: { + accounts: ['0x123'], + }, + metadata: { + id: '123', + name: '', + }, }, - async ({ controller, encryptor }) => { - jest.spyOn(encryptor, 'isVaultUpdated').mockReturnValue(true); - const encryptSpy = jest.spyOn(encryptor, 'encrypt'); - jest.spyOn(encryptor, 'decrypt').mockResolvedValueOnce([ - { - type: KeyringTypes.hd, - data: { - accounts: ['0x123'], - }, - metadata: { - id: '123', - name: '', - }, - }, - ]); + ]); - await controller.submitPassword(password); + await controller.submitPassword(password); - expect(encryptSpy).not.toHaveBeenCalled(); - }, - ); - }); + expect(encryptSpy).not.toHaveBeenCalled(); + }, + ); + }); - !cacheEncryptionKey && - it('should throw error if password is of wrong type', async () => { - await withController( - { cacheEncryptionKey }, - async ({ controller }) => { - await expect( - // @ts-expect-error we are testing the case of a user using - // the wrong password type - controller.submitPassword(12341234), - ).rejects.toThrow(KeyringControllerError.WrongPasswordType); - }, - ); - }); + it('should throw error if password is of wrong type', async () => { + await withController(async ({ controller }) => { + await expect( + // @ts-expect-error we are testing the case of a user using + // the wrong password type + controller.submitPassword(12341234), + ).rejects.toThrow(KeyringControllerError.WrongPasswordType); + }); + }); - cacheEncryptionKey && - it('should set encryptionKey and encryptionSalt in state', async () => { - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - withController({ cacheEncryptionKey }, async ({ controller }) => { - await controller.submitPassword(password); - expect(controller.state.encryptionKey).toBeDefined(); - expect(controller.state.encryptionSalt).toBeDefined(); - }); - }); + it('should set encryptionKey and encryptionSalt in state', async () => { + withController(async ({ controller }) => { + await controller.submitPassword(password); + expect(controller.state.encryptionKey).toBeDefined(); + expect(controller.state.encryptionSalt).toBeDefined(); + }); + }); - it('should throw error when using the wrong password', async () => { - await withController( - { - cacheEncryptionKey, - skipVaultCreation: true, - state: { - vault: freshVault, - // @ts-expect-error we want to force the controller to have an - // encryption salt equal to the one in the vault - encryptionSalt: SALT, - }, - }, - async ({ controller }) => { - await expect( - controller.submitPassword('wrong password'), - ).rejects.toThrow(DECRYPTION_ERROR); - }, - ); - }); - }), - ); + it('should throw error when using the wrong password', async () => { + await withController( + { + skipVaultCreation: true, + state: { + vault: freshVault, + // @ts-expect-error we want to force the controller to have an + // encryption salt equal to the one in the vault + encryptionSalt: SALT, + }, + }, + async ({ controller }) => { + await expect( + controller.submitPassword('wrong password'), + ).rejects.toThrow(DECRYPTION_ERROR); + }, + ); + }); }); describe('submitEncryptionKey', () => { it('should submit encryption key and decrypt', async () => { - await withController( - { cacheEncryptionKey: true }, - async ({ controller, initialState }) => { - await controller.submitEncryptionKey( - MOCK_ENCRYPTION_KEY, - initialState.encryptionSalt as string, - ); - expect(controller.state).toStrictEqual(initialState); - }, - ); + await withController(async ({ controller, initialState }) => { + await controller.submitEncryptionKey( + MOCK_ENCRYPTION_KEY, + initialState.encryptionSalt as string, + ); + expect(controller.state).toStrictEqual(initialState); + }); }); it('should unlock also with unsupported keyrings', async () => { await withController( { - cacheEncryptionKey: true, skipVaultCreation: true, state: { vault: freshVault, @@ -3345,7 +3208,6 @@ describe('KeyringController', () => { }); await withController( { - cacheEncryptionKey: true, skipVaultCreation: true, state: { vault: freshVault, @@ -3389,50 +3251,41 @@ describe('KeyringController', () => { }); it('should throw error if vault unlocked has an unexpected shape', async () => { - await withController( - { cacheEncryptionKey: true }, - async ({ controller, initialState, encryptor }) => { - jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ - { - foo: 'bar', - }, - ]); + await withController(async ({ controller, initialState, encryptor }) => { + jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ + { + foo: 'bar', + }, + ]); - await expect( - controller.submitEncryptionKey( - MOCK_ENCRYPTION_KEY, - initialState.encryptionSalt as string, // TODO Why need the salt here?? - ), - ).rejects.toThrow(KeyringControllerError.VaultDataError); - }, - ); + await expect( + controller.submitEncryptionKey( + MOCK_ENCRYPTION_KEY, + initialState.encryptionSalt as string, // TODO Why need the salt here?? + ), + ).rejects.toThrow(KeyringControllerError.VaultDataError); + }); }); it('should throw error if encryptionSalt is different from the one in the vault', async () => { - await withController( - { cacheEncryptionKey: true }, - async ({ controller }) => { - await expect( - controller.submitEncryptionKey(MOCK_ENCRYPTION_KEY, '0x1234'), - ).rejects.toThrow(KeyringControllerError.ExpiredCredentials); - }, - ); + await withController(async ({ controller }) => { + await expect( + controller.submitEncryptionKey(MOCK_ENCRYPTION_KEY, '0x1234'), + ).rejects.toThrow(KeyringControllerError.ExpiredCredentials); + }); }); it('should throw error if encryptionKey is of an unexpected type', async () => { - await withController( - { cacheEncryptionKey: true }, - async ({ controller, initialState }) => { - await expect( - controller.submitEncryptionKey( - // @ts-expect-error we are testing the case of a user using - // the wrong encryptionKey type - 12341234, - initialState.encryptionSalt as string, - ), - ).rejects.toThrow(KeyringControllerError.WrongPasswordType); - }, - ); + await withController(async ({ controller, initialState }) => { + await expect( + controller.submitEncryptionKey( + // @ts-expect-error we are testing the case of a user using + // the wrong encryptionKey type + 12341234, + initialState.encryptionSalt as string, + ), + ).rejects.toThrow(KeyringControllerError.WrongPasswordType); + }); }); }); @@ -3903,7 +3756,6 @@ describe('KeyringController', () => { { // @ts-expect-error QRKeyring is not yet compatible with Keyring type. keyringBuilders: [keyringBuilderFactory(QRKeyring)], - cacheEncryptionKey: true, }, (args) => args, ); diff --git a/packages/keyring-controller/src/KeyringController.ts b/packages/keyring-controller/src/KeyringController.ts index 3471eca0259..3ebfc296376 100644 --- a/packages/keyring-controller/src/KeyringController.ts +++ b/packages/keyring-controller/src/KeyringController.ts @@ -254,16 +254,8 @@ export type KeyringControllerOptions = { keyringBuilders?: { (): EthKeyring; type: string }[]; messenger: KeyringControllerMessenger; state?: { vault?: string; keyringsMetadata?: KeyringMetadata[] }; -} & ( - | { - cacheEncryptionKey: true; - encryptor?: ExportableKeyEncryptor; - } - | { - cacheEncryptionKey?: false; - encryptor?: GenericEncryptor | ExportableKeyEncryptor; - } -); + encryptor?: ExportableKeyEncryptor; +}; /** * A keyring object representation. @@ -325,13 +317,28 @@ export type SerializedKeyring = { metadata?: KeyringMetadata; }; +type CachedEncryptionKey = { + /** + * The exported encryption key string. + */ + exported: string; + /** + * The salt used to derive the encryption key. + */ + salt?: string; + /** + * The encrypted encryption key, if envelope encryption is used. + */ + encryptedEncryptionKey?: string; +}; + /** * State/data that can be updated during a `withKeyring` operation. */ type SessionState = { keyrings: SerializedKeyring[]; password?: string; - encryptionKey?: string; + encryptionKey?: CachedEncryptionKey; }; /** @@ -432,6 +439,32 @@ export type ExportableKeyEncryptor = * @returns The encryption key. */ importKey: (key: string) => Promise; + /** + * Exports the encryption key as a string. + * + * @param key - The encryption key to export. + * @returns The exported key string. + */ + exportKey: (key: EncryptionKey) => Promise; + /** + * Derives an encryption key from a password. + * + * @param password - The password to derive the key from. + * @param salt - The salt to use for key derivation. + * @param exportable - Whether the key should be exportable or not. + * @param options - Optional key derivation options. + * @returns The derived encryption key. + */ + keyFromPassword: ( + password: string, + salt: string, + exportable?: boolean, + opts?: encryptorUtils.KeyDerivationOptions, + ) => Promise; + /** + * Generates a random salt for key derivation. + */ + generateSalt: typeof encryptorUtils.generateSalt; }; export type KeyringSelector = @@ -548,20 +581,6 @@ function assertIsValidPassword(password: unknown): asserts password is string { } } -/** - * Assert that the provided cacheEncryptionKey is true. - * - * @param cacheEncryptionKey - The cacheEncryptionKey to check. - * @throws If the cacheEncryptionKey is not true. - */ -function assertIsCacheEncryptionKeyTrue( - cacheEncryptionKey: boolean, -): asserts cacheEncryptionKey is true { - if (!cacheEncryptionKey) { - throw new Error(KeyringControllerError.CacheEncryptionKeyDisabled); - } -} - /** * Checks if the provided value is a serialized keyrings array. * @@ -658,17 +677,13 @@ export class KeyringController extends BaseController< readonly #keyringBuilders: { (): EthKeyring; type: string }[]; - readonly #encryptor: GenericEncryptor | ExportableKeyEncryptor; - - readonly #cacheEncryptionKey: boolean; + readonly #encryptor: ExportableKeyEncryptor; #keyrings: { keyring: EthKeyring; metadata: KeyringMetadata }[]; #unsupportedKeyrings: SerializedKeyring[]; - #password?: string; - - #encryptionKey?: string; + #encryptionKey?: CachedEncryptionKey; #qrKeyringStateListener?: ( state: ReturnType, @@ -713,16 +728,13 @@ export class KeyringController extends BaseController< ? keyringBuilders.concat(defaultKeyringBuilders) : defaultKeyringBuilders; - this.#encryptor = encryptor; this.#keyrings = []; this.#unsupportedKeyrings = []; - // This option allows the controller to cache an exported key + // This type of encryptor allows the controller to cache an exported key // for use in decrypting and encrypting data without password - this.#cacheEncryptionKey = Boolean(options.cacheEncryptionKey); - if (this.#cacheEncryptionKey) { - assertIsExportableKeyEncryptor(encryptor); - } + assertIsExportableKeyEncryptor(encryptor); + this.#encryptor = encryptor; this.#registerMessageHandlers(); } @@ -1212,7 +1224,6 @@ export class KeyringController extends BaseController< return this.#withRollback(async () => { this.#unsubscribeFromQRKeyringsEvents(); - this.#password = undefined; this.#encryptionKey = undefined; await this.#clearKeyrings(); @@ -1481,21 +1492,21 @@ export class KeyringController extends BaseController< return this.#persistOrRollback(async () => { assertIsValidPassword(password); - // Update password. - this.#password = password; - // Update encryption key. - this.#updateCachedEncryptionKey(encryptionKey); + await this.#initEncryptionKey( + password, + encryptionKey + ? { useEnvelopeEncryption: true, encryptionKey } + : { useEnvelopeEncryption: false }, + ); // We need to clear encryption key and salt from state // to force the controller to re-encrypt the vault using // the new password. - if (this.#cacheEncryptionKey) { - this.update((state) => { - delete state.encryptionKey; - delete state.encryptionSalt; - }); - } + this.update((state) => { + delete state.encryptionKey; + delete state.encryptionSalt; + }); }); } @@ -1512,11 +1523,7 @@ export class KeyringController extends BaseController< encryptionSalt: string, ): Promise { const { newMetadata } = await this.#withRollback(async () => { - const result = await this.#unlockKeyrings( - undefined, - encryptionKey, - encryptionSalt, - ); + const result = await this.#unlockKeyrings(encryptionKey); this.#setUnlocked(); return result; }); @@ -1545,7 +1552,8 @@ export class KeyringController extends BaseController< */ async submitPassword(password: string): Promise { const { newMetadata } = await this.#withRollback(async () => { - const result = await this.#unlockKeyrings(password); + const key = await this.#unlockEncryptionKey(password); + const result = await this.#unlockKeyrings(key); this.#setUnlocked(); return result; }); @@ -2159,9 +2167,15 @@ export class KeyringController extends BaseController< delete state.encryptionSalt; }); - this.#password = password; - - this.#updateCachedEncryptionKey(encryptionKey); + await this.#initEncryptionKey( + password, + encryptionKey + ? { + useEnvelopeEncryption: true, + encryptionKey, + } + : { useEnvelopeEncryption: false }, + ); await this.#clearKeyrings(); await this.#createKeyringWithFirstAccount(keyring.type, keyring.opts); @@ -2173,12 +2187,77 @@ export class KeyringController extends BaseController< * * @param encryptionKey - Optional new vault encryption key. */ - #updateCachedEncryptionKey(encryptionKey?: string) { - if (!encryptionKey && this.state.encryptedEncryptionKey) { - this.#encryptionKey = this.state.encryptionKey; + async #initEncryptionKey( + password: string, + options: + | { + useEnvelopeEncryption: true; + encryptionKey: string; + } + | { useEnvelopeEncryption: false; encryptionKey?: never } = { + useEnvelopeEncryption: false, + }, + ): Promise { + if (options.useEnvelopeEncryption === true) { + this.#encryptionKey = { + exported: options.encryptionKey, + encryptedEncryptionKey: await this.#encryptor.encrypt( + password, + options.encryptionKey, + ), + }; + } else { + const salt = this.#encryptor.generateSalt(); + const key = await this.#encryptor.keyFromPassword(password, salt, true); + this.#encryptionKey = { + salt, + exported: await this.#encryptor.exportKey(key), + }; + } + } + + /** + * Decrypts or derives the vault encryption key from the encrypted + * encryption key or the password, depending on whether envelope + * encryption is enabled. + * + * @param password - The password to use for decryption or derivation. + * @returns A promise resolving to the decrypted or derived exported encryption key. + */ + async #unlockEncryptionKey(password: string): Promise { + this.#assertControllerMutexIsLocked(); + const { vault, encryptedEncryptionKey } = this.state; + + if (typeof password !== 'string') { + throw new TypeError(KeyringControllerError.WrongPasswordType); + } + + if (encryptedEncryptionKey) { + // In envelope encryption mode, the vault encryption key is stored + // encrypted in `encryptedEncryptionKey` and is decrypted using the password. + this.#encryptionKey = { + encryptedEncryptionKey, + exported: (await this.#encryptor.decrypt( + password, + encryptedEncryptionKey, + )) as string, + }; } else { - this.#encryptionKey = encryptionKey; + // In non-envelope encryption mode, the vault encryption key is derived + // from the password directly, using the salt included in the vault. + if (!vault) { + throw new Error(KeyringControllerError.VaultError); + } + const { salt } = JSON.parse(vault); + this.#encryptionKey = { + salt, + exported: await this.#encryptor.exportKey( + await this.#encryptor.keyFromPassword(password, salt, true), + ), + }; } + + return this.#encryptionKey.exported; } /** @@ -2286,7 +2365,6 @@ export class KeyringController extends BaseController< async #getSessionState(): Promise { return { keyrings: await this.#getSerializedKeyrings(), - password: this.#password, encryptionKey: this.#encryptionKey, }; } @@ -2325,90 +2403,28 @@ export class KeyringController extends BaseController< * Unlock Keyrings, decrypting the vault and deserializing all * keyrings contained in it, using a password or an encryption key with salt. * - * @param password - The keyring controller password. - * @param encryptionKey - An exported key string to unlock keyrings with. - * @param encryptionSalt - The salt used to encrypt the vault. + * @param exportedEncryptionKey - The exported encryption key to unlock the keyrings. * @returns A promise resolving to the deserialized keyrings array. */ - async #unlockKeyrings( - password: string | undefined, - encryptionKey?: string, - encryptionSalt?: string, - ): Promise<{ + async #unlockKeyrings(exportedEncryptionKey: string): Promise<{ keyrings: { keyring: EthKeyring; metadata: KeyringMetadata }[]; newMetadata: boolean; }> { return this.#withVaultLock(async () => { - const { vault: encryptedVault, encryptedEncryptionKey } = this.state; - if (!encryptedVault) { - throw new Error(KeyringControllerError.VaultError); + if (typeof exportedEncryptionKey !== 'string') { + throw new TypeError(KeyringControllerError.WrongPasswordType); } - let vault; - const updatedState: Partial = {}; - - if (this.#cacheEncryptionKey) { - assertIsExportableKeyEncryptor(this.#encryptor); - - if (password) { - if (encryptedEncryptionKey) { - const key = (await this.#encryptor.decrypt( - password, - encryptedEncryptionKey, - )) as string; - - const importedKey = await this.#encryptor.importKey(key); - - vault = await this.#encryptor.decryptWithKey( - importedKey, - encryptedVault, - ); - this.#password = password; - updatedState.encryptionKey = key; - } else { - const result = await this.#encryptor.decryptWithDetail( - password, - encryptedVault, - ); - vault = result.vault; - this.#password = password; - - updatedState.encryptionKey = result.exportedKeyString; - updatedState.encryptionSalt = result.salt; - } - } else { - const parsedEncryptedVault = JSON.parse(encryptedVault); - - if (encryptionSalt !== parsedEncryptedVault.salt) { - throw new Error(KeyringControllerError.ExpiredCredentials); - } - - if (typeof encryptionKey !== 'string') { - throw new TypeError(KeyringControllerError.WrongPasswordType); - } - - const key = await this.#encryptor.importKey(encryptionKey); - vault = await this.#encryptor.decryptWithKey( - key, - parsedEncryptedVault, - ); - - // This call is required on the first call because encryptionKey - // is not yet inside the memStore - updatedState.encryptionKey = encryptionKey; - // we can safely assume that encryptionSalt is defined here - // because we compare it with the salt from the vault - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - updatedState.encryptionSalt = encryptionSalt!; - } - } else { - if (typeof password !== 'string') { - throw new TypeError(KeyringControllerError.WrongPasswordType); - } - - vault = await this.#encryptor.decrypt(password, encryptedVault); - this.#password = password; + if (!this.state.vault) { + throw new Error(KeyringControllerError.VaultError); } + const parsedEncryptedVault = JSON.parse(this.state.vault); + + const key = await this.#encryptor.importKey(exportedEncryptionKey); + const vault = await this.#encryptor.decryptWithKey( + key, + parsedEncryptedVault, + ); if (!isSerializedKeyringsArray(vault)) { throw new Error(KeyringControllerError.VaultDataError); @@ -2421,10 +2437,8 @@ export class KeyringController extends BaseController< this.update((state) => { state.keyrings = updatedKeyrings; - if (updatedState.encryptionKey || updatedState.encryptionSalt) { - state.encryptionKey = updatedState.encryptionKey; - state.encryptionSalt = updatedState.encryptionSalt; - } + state.encryptionKey = exportedEncryptionKey; + state.encryptionSalt = parsedEncryptedVault.salt; }); return { keyrings, newMetadata }; @@ -2441,18 +2455,7 @@ export class KeyringController extends BaseController< // Ensure no duplicate accounts are persisted. await this.#assertNoDuplicateAccounts(); - const { encryptionKey, encryptionSalt, vault } = this.state; - - // READ THIS CAREFULLY: - // We do check if the vault is still considered up-to-date, if not, we would not re-use the - // cached key and we will re-generate a new one (based on the password). - // - // This helps doing seamless updates of the vault. Useful in case we change some cryptographic - // parameters to the KDF. - const useCachedKey = - encryptionKey && vault && this.#encryptor.isVaultUpdated?.(vault); - - if (!this.#password && !encryptionKey) { + if (!this.#encryptionKey) { throw new Error(KeyringControllerError.MissingCredentials); } @@ -2466,64 +2469,30 @@ export class KeyringController extends BaseController< throw new Error(KeyringControllerError.NoHdKeyring); } - const updatedState: Partial = {}; - - if (this.#cacheEncryptionKey) { - assertIsExportableKeyEncryptor(this.#encryptor); - - if (useCachedKey) { - const key = await this.#encryptor.importKey(encryptionKey); - const vaultJSON = await this.#encryptor.encryptWithKey( - key, - serializedKeyrings, - ); - vaultJSON.salt = encryptionSalt; - updatedState.vault = JSON.stringify(vaultJSON); - } else if (this.#password) { - if (this.#encryptionKey) { - // Update cached key. - updatedState.encryptionKey = this.#encryptionKey; - - // Encrypt key and update encrypted key. - const encryptedKey = await this.#encryptor.encrypt( - this.#password, - this.#encryptionKey, - ); - updatedState.encryptedEncryptionKey = encryptedKey; - - // Encrypt and update vault. - const importedKey = await this.#encryptor.importKey( - this.#encryptionKey, - ); - const vaultJSON = await this.#encryptor.encryptWithKey( - importedKey, - serializedKeyrings, - ); - updatedState.vault = JSON.stringify(vaultJSON); - } else { - const { vault: newVault, exportedKeyString } = - await this.#encryptor.encryptWithDetail( - this.#password, - serializedKeyrings, - ); - updatedState.vault = newVault; - updatedState.encryptionKey = exportedKeyString; - } - } - } else { - assertIsValidPassword(this.#password); - updatedState.vault = await this.#encryptor.encrypt( - this.#password, - serializedKeyrings, - ); - } - - if (this.#isEnvelopeEncryptionEnabled()) { - assertIsCacheEncryptionKeyTrue(this.#cacheEncryptionKey); + const key = await this.#encryptor.importKey(this.#encryptionKey.exported); + const encryptedVault = await this.#encryptor.encryptWithKey( + key, + serializedKeyrings, + ); + if (this.#encryptionKey.salt) { + // if the password is being used to encrypt the vault directly + // (i.e. no envelope encryption), we need to include the salt + // used to derive the encryption key. + encryptedVault.salt = this.#encryptionKey.salt; } + const updatedState: Partial = { + vault: JSON.stringify(encryptedVault), + encryptionKey: this.#encryptionKey.exported, + encryptionSalt: encryptedVault.salt, + }; - if (!updatedState.vault) { - throw new Error(KeyringControllerError.MissingVaultData); + if ( + this.#encryptionKey.encryptedEncryptionKey && + this.#encryptionKey.encryptedEncryptionKey !== + this.state.encryptedEncryptionKey + ) { + updatedState.encryptedEncryptionKey = + this.#encryptionKey.encryptedEncryptionKey; } const updatedKeyrings = await this.#getUpdatedKeyrings(); @@ -2531,11 +2500,12 @@ export class KeyringController extends BaseController< this.update((state) => { state.vault = updatedState.vault; state.keyrings = updatedKeyrings; - if (updatedState.encryptionKey) { - state.encryptionKey = updatedState.encryptionKey; - state.encryptionSalt = JSON.parse(updatedState.vault as string).salt; - } - if (updatedState.encryptedEncryptionKey) { + state.encryptionKey = updatedState.encryptionKey; + state.encryptionSalt = updatedState.encryptionSalt; + if ( + updatedState.encryptedEncryptionKey && + updatedState.encryptedEncryptionKey !== state.encryptedEncryptionKey + ) { state.encryptedEncryptionKey = updatedState.encryptedEncryptionKey; } }); @@ -2552,22 +2522,13 @@ export class KeyringController extends BaseController< #isNewEncryptionAvailable(): boolean { const { vault } = this.state; - if (!vault || !this.#password || !this.#encryptor.isVaultUpdated) { + if (!vault || !this.#encryptor.isVaultUpdated) { return false; } return !this.#encryptor.isVaultUpdated(vault); } - /** - * Checks if envelope encryption is enabled. - * - * @returns A boolean indicating whether envelope encryption is enabled. - */ - #isEnvelopeEncryptionEnabled() { - return this.state.encryptedEncryptionKey || this.#encryptionKey; - } - /** * Retrieves all the accounts from keyrings instances * that are currently in memory. @@ -2878,12 +2839,13 @@ export class KeyringController extends BaseController< ): Promise { return this.#withControllerLock(async ({ releaseLock }) => { const currentSerializedKeyrings = await this.#getSerializedKeyrings(); - const currentPassword = this.#password; + const currentEncryptionKey = this.#encryptionKey; + try { return await callback({ releaseLock }); } catch (e) { - // Keyrings and password are restored to their previous state - this.#password = currentPassword; + // Keyrings and encryption key are restored to their previous state + this.#encryptionKey = currentEncryptionKey; await this.#restoreSerializedKeyrings(currentSerializedKeyrings); throw e; diff --git a/packages/keyring-controller/tests/mocks/mockEncryptor.ts b/packages/keyring-controller/tests/mocks/mockEncryptor.ts index e8aaf09d81a..ab2c4d21bdc 100644 --- a/packages/keyring-controller/tests/mocks/mockEncryptor.ts +++ b/packages/keyring-controller/tests/mocks/mockEncryptor.ts @@ -92,6 +92,12 @@ export default class MockEncryptor implements ExportableKeyEncryptor { typeof ciphertext === 'string' ? JSON.parse(ciphertext) : ciphertext; const data = JSON.parse(ciphertextObj.data); if (!isEqual(data.tag, { key, iv: ciphertextObj.iv })) { + console.log('Decryption failed: key or iv does not match.', { + key, + iv: ciphertextObj.iv, + tag: data.tag, + }); + throw new Error(DECRYPTION_ERROR); } return data.value; @@ -101,6 +107,14 @@ export default class MockEncryptor implements ExportableKeyEncryptor { return JSON.parse(key); } + async exportKey(key: unknown) { + return JSON.stringify(key); + } + + async keyFromPassword(password: string, salt: string): Promise { + return deriveKey(password, salt); + } + async updateVault(_vault: string, _password: string) { return _vault; } @@ -108,6 +122,10 @@ export default class MockEncryptor implements ExportableKeyEncryptor { isVaultUpdated(_vault: string) { return true; } + + generateSalt() { + return generateSalt(); + } } function generateSalt() {