diff --git a/eslint-warning-thresholds.json b/eslint-warning-thresholds.json index cca3ed4748e..7a3da2ba051 100644 --- a/eslint-warning-thresholds.json +++ b/eslint-warning-thresholds.json @@ -213,10 +213,10 @@ }, "packages/keyring-controller/src/KeyringController.test.ts": { "import-x/namespace": 14, - "jest/no-conditional-in-test": 8 + "jest/no-conditional-in-test": 7 }, "packages/keyring-controller/src/KeyringController.ts": { - "@typescript-eslint/no-unsafe-enum-comparison": 4, + "@typescript-eslint/no-unsafe-enum-comparison": 3, "@typescript-eslint/no-unused-vars": 1 }, "packages/keyring-controller/tests/mocks/mockKeyring.ts": { diff --git a/packages/keyring-controller/CHANGELOG.md b/packages/keyring-controller/CHANGELOG.md index 38f4b66ca6b..e7a1e7a6b4a 100644 --- a/packages/keyring-controller/CHANGELOG.md +++ b/packages/keyring-controller/CHANGELOG.md @@ -7,6 +7,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Added optional `SupportedKeyDerivationOptions` type parameter to the `ExportableKeyEncryptor` type ([#5963](https://github.com/MetaMask/core/pull/5963)) + - This type parameter allows specifying the key derivation options supported by the injected encryptor. + +### Changed + +- **BREAKING:** The `KeyringController` constructor now requires an encryptor supporting the `keyFromPassword`, `exportKey` and `generateSalt` methods ([#5963](https://github.com/MetaMask/core/pull/5963)) + +### Removed + +- **BREAKING:** The `cacheEncryptionKey` parameter has been removed from the `KeyringController` constructor options ([#5963](https://github.com/MetaMask/core/pull/5963)) + - This parameter was previously used to enable encryption key in-memory caching, but it is no longer needed as the controller now always uses the latest encryption key. +- **BREAKING:** The `submitEncryptionKey` method does not accept an `encryptionSalt` argument anymore ([#5963](https://github.com/MetaMask/core/pull/5963)) + - The encryption salt is now always taken from the vault. + ## [22.0.2] ### Fixed diff --git a/packages/keyring-controller/jest.config.js b/packages/keyring-controller/jest.config.js index 568a60b2b46..b419b2a2e9a 100644 --- a/packages/keyring-controller/jest.config.js +++ b/packages/keyring-controller/jest.config.js @@ -17,10 +17,10 @@ module.exports = merge(baseConfig, { // An object that configures minimum threshold enforcement for coverage results coverageThreshold: { global: { - branches: 94.31, + branches: 94.01, functions: 100, - lines: 98.79, - statements: 98.8, + lines: 98.76, + statements: 98.77, }, }, diff --git a/packages/keyring-controller/src/KeyringController.test.ts b/packages/keyring-controller/src/KeyringController.test.ts index 08e726c34fb..c37f59e7440 100644 --- a/packages/keyring-controller/src/KeyringController.test.ts +++ b/packages/keyring-controller/src/KeyringController.test.ts @@ -38,6 +38,7 @@ import { } from './KeyringController'; import MockEncryptor, { MOCK_ENCRYPTION_KEY, + MOCK_KEY, } from '../tests/mocks/mockEncryptor'; import { MockErc4337Keyring } from '../tests/mocks/mockErc4337Keyring'; import { MockKeyring } from '../tests/mocks/mockKeyring'; @@ -64,6 +65,8 @@ const uint8ArraySeed = new Uint8Array( seedWords.split(' ').map((word) => wordlist.indexOf(word)), ).buffer, ); +const mockVault = + '{"data":"{\\"tag\\":{\\"key\\":{\\"password\\":\\"password123\\",\\"salt\\":\\"salt\\"},\\"iv\\":\\"iv\\"},\\"value\\":[{\\"type\\":\\"HD Key Tree\\",\\"data\\":{\\"mnemonic\\":[119,97,114,114,105,111,114,32,108,97,110,103,117,97,103,101,32,106,111,107,101,32,98,111,110,117,115,32,117,110,102,97,105,114,32,97,114,116,105,115,116,32,107,97,110,103,97,114,111,111,32,99,105,114,99,108,101,32,101,120,112,97,110,100,32,104,111,112,101,32,109,105,100,100,108,101,32,103,97,117,103,101],\\"numberOfAccounts\\":1,\\"hdPath\\":\\"m/44\'/60\'/0\'/0\\"},\\"metadata\\":{\\"id\\":\\"01JXEFM7DAX2VJ0YFR4ESNY3GQ\\",\\"name\\":\\"\\"}}]}","iv":"iv","salt":"salt"}'; const privateKey = '1e4e6a4c0c077f4ae8ddfbf372918e61dd0fb4a4cfa592cb16e7546d505e68fc'; const password = 'password123'; @@ -82,7 +85,6 @@ describe('KeyringController', () => { () => new KeyringController({ messenger: buildKeyringControllerMessenger(), - cacheEncryptionKey: true, }), ).not.toThrow(); }); @@ -90,10 +92,9 @@ describe('KeyringController', () => { it('should throw error if cacheEncryptionKey is true and 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); @@ -133,11 +134,11 @@ describe('KeyringController', () => { { skipVaultCreation: true, state: { - vault: 'my vault', + vault: mockVault, }, }, async ({ controller, encryptor }) => { - jest.spyOn(encryptor, 'decrypt').mockResolvedValueOnce([ + jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ { type: KeyringTypes.hd, data: '', @@ -177,11 +178,11 @@ describe('KeyringController', () => { { skipVaultCreation: true, state: { - vault: 'my vault', + vault: mockVault, }, }, async ({ controller, encryptor }) => { - jest.spyOn(encryptor, 'decrypt').mockResolvedValueOnce([ + jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ { type: 'HD Key Tree', data: '', @@ -283,10 +284,10 @@ describe('KeyringController', () => { it('should throw an error if there is no primary keyring', async () => { await withController( - { skipVaultCreation: true, state: { vault: 'my vault' } }, + { skipVaultCreation: true, state: { vault: mockVault } }, async ({ controller, encryptor }) => { jest - .spyOn(encryptor, 'decrypt') + .spyOn(encryptor, 'decryptWithKey') .mockResolvedValueOnce([{ type: 'Unsupported', data: '' }]); await controller.submitPassword('123'); @@ -547,254 +548,181 @@ 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 initialVault = controller.state.vault; - const initialKeyrings = controller.state.keyrings; - await controller.createNewVaultAndRestore( - password, - uint8ArraySeed, - ); - expect(controller.state).not.toBe(initialState); - expect(controller.state.vault).toBeDefined(); - expect(controller.state.vault).toStrictEqual(initialVault); - 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, 'encrypt'); - 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 initialVault = controller.state.vault; + const initialKeyrings = controller.state.keyrings; + await controller.createNewVaultAndRestore(password, uint8ArraySeed); + expect(controller.state).not.toBe(initialState); + expect(controller.state.vault).toBeDefined(); + expect(controller.state.vault).toStrictEqual(initialVault); + expect(controller.state.keyrings).toHaveLength(initialKeyrings.length); + // new keyring metadata should be generated + expect(controller.state.keyrings).not.toStrictEqual(initialKeyrings); + }); + }); + + it('should call encryptor.encryptWithKey with the same keyrings if old seedWord is used', async () => { + await withController(async ({ controller, encryptor }) => { + const encryptSpy = jest.spyOn(encryptor, 'encryptWithKey'); + jest.spyOn(encryptor, 'importKey').mockResolvedValueOnce(MOCK_KEY); + 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); - expect(encryptSpy).toHaveBeenCalledWith(password, [ - { - data: serializedKeyring, - type: 'HD Key Tree', - metadata: { - id: expect.any(String), - name: '', - }, - }, - ]); + expect(encryptSpy).toHaveBeenCalledWith(MOCK_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(); + 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; - await controller.createNewVaultAndKeychain(password); + await controller.createNewVaultAndKeychain(password); - expect(controller.state.encryptionKey).toBeDefined(); - expect(controller.state.encryptionSalt).toBeDefined(); - }, - ); - }); + const currentSeedWord = await controller.exportSeedPhrase(password); + expect(initialState).toStrictEqual(controller.state); + expect(initialSeedWord).toBe(currentSeedWord); + expect(initialVault).toStrictEqual(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); + 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(); - expect(controller.state).not.toHaveProperty('encryptionKey'); - expect(controller.state).not.toHaveProperty('encryptionSalt'); - }, - ); - }); + await controller.createNewVaultAndKeychain(password); + + expect(controller.state.encryptionKey).toBeDefined(); + expect(controller.state.encryptionSalt).toBeDefined(); }); - }), - ); + }); + }); }); describe('setLocked', () => { @@ -1174,10 +1102,10 @@ describe('KeyringController', () => { it('should throw an error if there is no keyring', async () => { await withController( - { skipVaultCreation: true, state: { vault: 'my vault' } }, + { skipVaultCreation: true, state: { vault: mockVault } }, async ({ controller, encryptor }) => { jest - .spyOn(encryptor, 'decrypt') + .spyOn(encryptor, 'decryptWithKey') .mockResolvedValueOnce([{ type: 'Unsupported', data: '' }]); await controller.submitPassword('123'); @@ -2579,579 +2507,451 @@ 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 keyFromPasswordSpy = jest.spyOn(encryptor, 'keyFromPassword'); - 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); - }, - ); - }); + expect(keyFromPasswordSpy).toHaveBeenCalledWith( + newPassword, + controller.state.encryptionSalt, + true, + undefined, + ); + }); + }); - 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: mockVault }, + }, + async ({ controller, encryptor }) => { + jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ { - cacheEncryptionKey, - skipVaultCreation: true, - state: { vault: 'my vault' }, + type: 'UnsupportedKeyring', + data: '0x1234', }, - async ({ controller, encryptor }) => { - jest.spyOn(encryptor, 'decrypt').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: mockVault }, + }, + async ({ controller, encryptor }) => { + jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ { - cacheEncryptionKey, - skipVaultCreation: true, - state: { vault: 'my vault' }, + foo: 'bar', }, - async ({ controller, encryptor }) => { - jest.spyOn(encryptor, 'decrypt').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: mockVault }, + skipVaultCreation: true, + }, + async ({ controller, encryptor }) => { + jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ { - cacheEncryptionKey, - state: { vault: 'my vault' }, - skipVaultCreation: true, - }, - async ({ controller, encryptor }) => { - jest.spyOn(encryptor, 'decrypt').mockResolvedValueOnce([ - { - type: KeyringTypes.hd, - data: { - accounts: ['0x123'], - }, - metadata: { - id: '123', - name: '', - }, - }, - ]); + 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: 'my vault', - }, - skipVaultCreation: true, + it('should generate new metadata when there is no metadata in the vault', async () => { + const hdKeyringSerializeSpy = jest.spyOn( + HdKeyring.prototype, + 'serialize', + ); + await withController( + { + state: { + vault: mockVault, + }, + 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') - // @ts-expect-error we are assigning a mock value - .mockResolvedValue('imported key'); - 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(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: '', }, - ); - }); - - !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: 'my vault', - }, - skipVaultCreation: true, + }, + ]); + expect(encryptWithKeySpy).toHaveBeenCalledWith('imported key', [ + { + 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'], - }); - - 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: '', - }, - }, - ]); + 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: mockVault }, + 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: 'my vault' }, - 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, 'decrypt').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: mockVault }, + }, + async ({ controller, encryptor }) => { + jest.spyOn(encryptor, 'isVaultUpdated').mockReturnValue(false); + jest + .spyOn(encryptor, 'encryptWithKey') + .mockRejectedValue(new Error()); + jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ { - skipVaultCreation: true, - cacheEncryptionKey, - state: { vault: 'my vault' }, + 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, 'decrypt').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: mockVault }, + 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: 'my vault' }, - keyringBuilders: [keyringBuilderFactory(MockKeyring)], + type: KeyringTypes.hd, + data: {}, }, - async ({ controller, encryptor, messenger }) => { - const unlockListener = jest.fn(); - messenger.subscribe('KeyringController:unlock', unlockListener); - jest.spyOn(encryptor, 'decrypt').mockResolvedValueOnce([ - { - type: KeyringTypes.hd, - data: {}, - }, - { - type: MockKeyring.type, - data: {}, - }, - ]); - - await controller.submitPassword(password); - - expect(controller.state.keyrings).toHaveLength(1); // Second keyring will be skipped as "unsupported". - expect(unlockListener).toHaveBeenCalledTimes(1); + { + type: MockKeyring.type, + data: {}, }, - ); - }); - - cacheEncryptionKey && - it('should upgrade the vault encryption if the key encryptor has different parameters', async () => { - await withController( - { - skipVaultCreation: true, - cacheEncryptionKey, - state: { vault: 'my vault' }, - }, - 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(controller.state.keyrings).toHaveLength(1); // Second keyring will be skipped as "unsupported". + expect(unlockListener).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: 'my vault' }, + it('should upgrade the vault encryption if the key encryptor has different parameters', async () => { + await withController( + { + skipVaultCreation: true, + state: { vault: mockVault }, + }, + async ({ controller, encryptor }) => { + jest.spyOn(encryptor, 'isVaultUpdated').mockReturnValue(false); + jest.spyOn(encryptor, 'generateSalt').mockReturnValue('new salt'); + const keyFromPasswordSpy = jest.spyOn(encryptor, 'keyFromPassword'); + 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(true); - 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).not.toHaveBeenCalled(); - }, - ); - }); + expect(keyFromPasswordSpy).toHaveBeenCalledWith( + password, + 'new salt', + true, + undefined, + ); + expect(encryptSpy).toHaveBeenCalledTimes(1); + }, + ); + }); - !cacheEncryptionKey && - it('should upgrade the vault encryption if the generic encryptor has different parameters', async () => { - await withController( - { - skipVaultCreation: true, - cacheEncryptionKey, - state: { vault: 'my vault' }, + it('should not upgrade the vault encryption if the key encryptor has the same parameters', async () => { + await withController( + { + skipVaultCreation: true, + state: { vault: mockVault }, + }, + 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(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).not.toHaveBeenCalled(); + }, + ); + }); - 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: mockVault }, + }, + async ({ controller, encryptor }) => { + jest.spyOn(encryptor, 'isVaultUpdated').mockReturnValue(true); + const encryptSpy = jest.spyOn(encryptor, 'encrypt'); + jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ { - skipVaultCreation: true, - cacheEncryptionKey, - state: { vault: 'my vault' }, + 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 () => { + // TODO: Either fix this lint violation or explain why it's necessary to ignore. + // eslint-disable-next-line @typescript-eslint/no-floating-promises + 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( - { - skipVaultCreation: true, - cacheEncryptionKey, - state: { vault: 'my vault' }, - }, - async ({ controller }) => { - await expect( - controller.submitPassword('wrong password'), - ).rejects.toThrow('Incorrect password.'); - }, - ); - }); - }), - ); + it('should throw error when using the wrong password', async () => { + await withController( + { + skipVaultCreation: true, + state: { vault: mockVault }, + }, + async ({ controller, encryptor }) => { + jest + .spyOn(encryptor, 'decryptWithKey') + .mockRejectedValue(new Error('Incorrect password.')); + await expect( + controller.submitPassword('wrong password'), + ).rejects.toThrow('Incorrect password.'); + }, + ); + }); }); 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); + expect(controller.state).toStrictEqual(initialState); + }); }); it('should unlock also with unsupported keyrings', async () => { await withController( { - cacheEncryptionKey: true, skipVaultCreation: true, state: { vault: JSON.stringify({ data: '0x123', salt: 'my salt' }), @@ -3160,18 +2960,15 @@ describe('KeyringController', () => { encryptionSalt: 'my salt', }, }, - async ({ controller, initialState, encryptor }) => { - jest.spyOn(encryptor, 'decrypt').mockResolvedValueOnce([ + async ({ controller, encryptor }) => { + jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ { type: 'UnsupportedKeyring', data: '0x1234', }, ]); - await controller.submitEncryptionKey( - MOCK_ENCRYPTION_KEY, - initialState.encryptionSalt as string, - ); + await controller.submitEncryptionKey(MOCK_ENCRYPTION_KEY); expect(controller.state.isUnlocked).toBe(true); }, @@ -3185,7 +2982,6 @@ describe('KeyringController', () => { }); await withController( { - cacheEncryptionKey: true, skipVaultCreation: true, state: { vault: JSON.stringify({ data: '0x123', salt: 'my salt' }), @@ -3194,23 +2990,17 @@ describe('KeyringController', () => { encryptionSalt: 'my salt', }, }, - async ({ controller, initialState, encryptor }) => { + async ({ controller, encryptor }) => { const encryptWithKeySpy = jest.spyOn(encryptor, 'encryptWithKey'); - jest - .spyOn(encryptor, 'importKey') - // @ts-expect-error we are assigning a mock value - .mockResolvedValue('imported key'); - jest.spyOn(encryptor, 'decrypt').mockResolvedValueOnce([ + jest.spyOn(encryptor, 'importKey').mockResolvedValue('imported key'); + jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ { type: KeyringTypes.hd, data: '0x123', }, ]); - await controller.submitEncryptionKey( - MOCK_ENCRYPTION_KEY, - initialState.encryptionSalt as string, - ); + await controller.submitEncryptionKey(MOCK_ENCRYPTION_KEY); expect(controller.state.isUnlocked).toBe(true); expect(encryptWithKeySpy).toHaveBeenCalledWith('imported key', [ @@ -3230,50 +3020,29 @@ 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, 'decrypt').mockResolvedValueOnce([ - { - foo: 'bar', - }, - ]); - - await expect( - controller.submitEncryptionKey( - MOCK_ENCRYPTION_KEY, - initialState.encryptionSalt as string, - ), - ).rejects.toThrow(KeyringControllerError.VaultDataError); - }, - ); - }); + await withController(async ({ controller, encryptor }) => { + jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ + { + foo: 'bar', + }, + ]); - 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 expect( + controller.submitEncryptionKey(MOCK_ENCRYPTION_KEY), + ).rejects.toThrow(KeyringControllerError.VaultDataError); + }); }); 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 }) => { + await expect( + controller.submitEncryptionKey( + // @ts-expect-error we are testing the case of a user using + // the wrong encryptionKey type + 12341234, + ), + ).rejects.toThrow(KeyringControllerError.WrongEncryptionKeyType); + }); }); }); @@ -3339,10 +3108,10 @@ describe('KeyringController', () => { it('should throw an error if there is no primary keyring', async () => { await withController( - { skipVaultCreation: true, state: { vault: 'my vault' } }, + { skipVaultCreation: true, state: { vault: mockVault } }, async ({ controller, encryptor }) => { jest - .spyOn(encryptor, 'decrypt') + .spyOn(encryptor, 'decryptWithKey') .mockResolvedValueOnce([{ type: 'Unsupported', data: '' }]); await controller.submitPassword('123'); @@ -3744,7 +3513,6 @@ describe('KeyringController', () => { { // @ts-expect-error QRKeyring is not yet compatible with Keyring type. keyringBuilders: [keyringBuilderFactory(QRKeyring)], - cacheEncryptionKey: true, }, (args) => args, ); @@ -4361,8 +4129,6 @@ describe('KeyringController', () => { 'KeyringController:qrKeyringStateChange', listener, ); - const salt = signProcessKeyringController.state - .encryptionSalt as string; // We ensure there is a QRKeyring before locking await signProcessKeyringController.getOrAddQRKeyring(); // Locking the keyring will dereference the QRKeyring @@ -4370,7 +4136,6 @@ describe('KeyringController', () => { // ..and unlocking it should add a new instance of QRKeyring await signProcessKeyringController.submitEncryptionKey( MOCK_ENCRYPTION_KEY, - salt, ); // We call `getQRKeyring` instead of `getOrAddQRKeyring` so that // we are able to test if the subscription to the internal QR keyring diff --git a/packages/keyring-controller/src/KeyringController.ts b/packages/keyring-controller/src/KeyringController.ts index 65b3954f873..63df81bfe50 100644 --- a/packages/keyring-controller/src/KeyringController.ts +++ b/packages/keyring-controller/src/KeyringController.ts @@ -250,16 +250,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. @@ -321,12 +313,26 @@ export type SerializedKeyring = { metadata?: KeyringMetadata; }; +/** + * Cached encryption key used to encrypt/decrypt the vault. + */ +type CachedEncryptionKey = { + /** + * The exported encryption key string. + */ + exported: string; + /** + * The salt used to derive the encryption key. + */ + salt: string; +}; + /** * State/data that can be updated during a `withKeyring` operation. */ type SessionState = { keyrings: SerializedKeyring[]; - password?: string; + encryptionKey?: CachedEncryptionKey; }; /** @@ -368,66 +374,96 @@ export type GenericEncryptor = { * An encryptor interface that supports encrypting and decrypting * serializable data with a password, and exporting and importing keys. */ -export type ExportableKeyEncryptor = - GenericEncryptor & { - /** - * Encrypts the given object with the given encryption key. - * - * @param key - The encryption key to encrypt with. - * @param object - The object to encrypt. - * @returns The encryption result. - */ - encryptWithKey: ( - key: EncryptionKey, - object: Json, - ) => Promise; - /** - * Encrypts the given object with the given password, and returns the - * encryption result and the exported key string. - * - * @param password - The password to encrypt with. - * @param object - The object to encrypt. - * @param salt - The optional salt to use for encryption. - * @returns The encrypted string and the exported key string. - */ - encryptWithDetail: ( - password: string, - object: Json, - salt?: string, - ) => Promise; - /** - * Decrypts the given encrypted string with the given encryption key. - * - * @param key - The encryption key to decrypt with. - * @param encryptedString - The encrypted string to decrypt. - * @returns The decrypted object. - */ - decryptWithKey: ( - key: EncryptionKey, - encryptedString: string, - ) => Promise; - /** - * Decrypts the given encrypted string with the given password, and returns - * the decrypted object and the salt and exported key string used for - * encryption. - * - * @param password - The password to decrypt with. - * @param encryptedString - The encrypted string to decrypt. - * @returns The decrypted object and the salt and exported key string used for - * encryption. - */ - decryptWithDetail: ( - password: string, - encryptedString: string, - ) => Promise; - /** - * Generates an encryption key from exported key string. - * - * @param key - The exported key string. - * @returns The encryption key. - */ - importKey: (key: string) => Promise; - }; +export type ExportableKeyEncryptor< + EncryptionKey = unknown, + SupportedKeyDerivationParams = unknown, +> = GenericEncryptor & { + /** + * Encrypts the given object with the given encryption key. + * + * @param key - The encryption key to encrypt with. + * @param object - The object to encrypt. + * @returns The encryption result. + */ + encryptWithKey: ( + key: EncryptionKey, + object: Json, + ) => Promise; + /** + * Encrypts the given object with the given password, and returns the + * encryption result and the exported key string. + * + * @param password - The password to encrypt with. + * @param object - The object to encrypt. + * @param salt - The optional salt to use for encryption. + * @returns The encrypted string and the exported key string. + */ + encryptWithDetail: ( + password: string, + object: Json, + salt?: string, + ) => Promise; + /** + * Decrypts the given encrypted string with the given encryption key. + * + * @param key - The encryption key to decrypt with. + * @param encryptedString - The encrypted string to decrypt. + * @returns The decrypted object. + */ + decryptWithKey: ( + key: EncryptionKey, + encryptedString: string, + ) => Promise; + /** + * Decrypts the given encrypted string with the given password, and returns + * the decrypted object and the salt and exported key string used for + * encryption. + * + * @param password - The password to decrypt with. + * @param encryptedString - The encrypted string to decrypt. + * @returns The decrypted object and the salt and exported key string used for + * encryption. + */ + decryptWithDetail: ( + password: string, + encryptedString: string, + ) => Promise; + /** + * Generates an encryption key from exported key string. + * + * @param key - The exported key string. + * @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, + // setting this to unknown as currently each client has different + // key derivation options + keyDerivationOptions?: SupportedKeyDerivationParams, + ) => Promise; + /** + * Generates a random salt for key derivation. + */ + generateSalt: typeof encryptorUtils.generateSalt; +}; export type KeyringSelector = | { @@ -639,15 +675,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?: CachedEncryptionKey; #qrKeyringStateListener?: ( state: ReturnType, @@ -691,17 +725,11 @@ export class KeyringController extends BaseController< ? keyringBuilders.concat(defaultKeyringBuilders) : defaultKeyringBuilders; + assertIsExportableKeyEncryptor(encryptor); this.#encryptor = encryptor; this.#keyrings = []; this.#unsupportedKeyrings = []; - // This option 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); - } - this.#registerMessageHandlers(); } @@ -1167,7 +1195,7 @@ export class KeyringController extends BaseController< return this.#withRollback(async () => { this.#unsubscribeFromQRKeyringsEvents(); - this.#password = undefined; + this.#encryptionKey = undefined; await this.#clearKeyrings(); this.update((state) => { @@ -1419,17 +1447,7 @@ export class KeyringController extends BaseController< return this.#persistOrRollback(async () => { assertIsValidPassword(password); - - this.#password = password; - // 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; - }); - } + await this.#deriveEncryptionKey(password); }); } @@ -1438,19 +1456,13 @@ export class KeyringController extends BaseController< * using the given encryption key and salt. * * @param encryptionKey - Key to unlock the keychain. - * @param encryptionSalt - Salt to unlock the keychain. * @returns Promise resolving when the operation completes. */ - async submitEncryptionKey( - encryptionKey: string, - encryptionSalt: string, - ): Promise { + async submitEncryptionKey(encryptionKey: string): Promise { const { newMetadata } = await this.#withRollback(async () => { - const result = await this.#unlockKeyrings( - undefined, - encryptionKey, - encryptionSalt, - ); + const result = await this.#unlockKeyrings({ + exportedEncryptionKey: encryptionKey, + }); this.#setUnlocked(); return result; }); @@ -1479,7 +1491,7 @@ export class KeyringController extends BaseController< */ async submitPassword(password: string): Promise { const { newMetadata } = await this.#withRollback(async () => { - const result = await this.#unlockKeyrings(password); + const result = await this.#unlockKeyrings({ password }); this.#setUnlocked(); return result; }); @@ -1490,6 +1502,12 @@ export class KeyringController extends BaseController< // can attempt to upgrade the vault. await this.#withRollback(async () => { if (newMetadata || this.#isNewEncryptionAvailable()) { + await this.#deriveEncryptionKey(password, { + // If the vault is being upgraded, we want to ignore the metadata + // that is already in the vault, so we can effectively + // re-encrypt the vault with the new encryption config. + ignoreVaultKeyMetadata: true, + }); await this.#updateVault(); } }); @@ -2091,13 +2109,83 @@ export class KeyringController extends BaseController< delete state.encryptionSalt; }); - this.#password = password; + await this.#deriveEncryptionKey(password); await this.#clearKeyrings(); await this.#createKeyringWithFirstAccount(keyring.type, keyring.opts); this.#setUnlocked(); } + /** + * Derive the vault encryption key from the provided password, and + * assign it to the instance variable for later use with cryptographic + * functions. + * + * When the controller has a vault in its state, the key is derived + * using the salt from the vault. If the vault is empty, a new salt + * is generated and used to derive the key. + * + * @param password - The password to use for decryption or derivation. + * @param options - Options for the key derivation. + * @param options.ignoreVaultKeyMetadata - Whether to ignore the vault key metadata + */ + async #deriveEncryptionKey( + password: string, + options: { ignoreVaultKeyMetadata: boolean } = { + ignoreVaultKeyMetadata: false, + }, + ): Promise { + this.#assertControllerMutexIsLocked(); + const { vault } = this.state; + + if (typeof password !== 'string') { + throw new TypeError(KeyringControllerError.WrongPasswordType); + } + + let salt: string, keyMetadata: unknown; + if (vault && !options.ignoreVaultKeyMetadata) { + const parsedVault = JSON.parse(vault); + salt = parsedVault.salt; + keyMetadata = parsedVault.keyMetadata; + } else { + salt = this.#encryptor.generateSalt(); + } + + const exportedEncryptionKey = await this.#encryptor.exportKey( + await this.#encryptor.keyFromPassword(password, salt, true, keyMetadata), + ); + + this.#encryptionKey = { + salt, + exported: exportedEncryptionKey, + }; + } + + /** + * Use the provided encryption key and salt to set the + * encryptionKey instance variable. This method is used + * when the user provides an encryption key and salt + * to unlock the keychain, instead of using a password. + * + * @param encryptionKey - The encryption key to use. + * @param encryptionSalt - The salt to use for the encryption key. + */ + #useEncryptionKey(encryptionKey: string, encryptionSalt: string): void { + this.#assertControllerMutexIsLocked(); + + if ( + typeof encryptionKey !== 'string' || + typeof encryptionSalt !== 'string' + ) { + throw new TypeError(KeyringControllerError.WrongEncryptionKeyType); + } + + this.#encryptionKey = { + salt: encryptionSalt, + exported: encryptionKey, + }; + } + /** * Internal non-exclusive method to verify the seed phrase. * @@ -2195,7 +2283,7 @@ export class KeyringController extends BaseController< } /** - * Get a snapshot of session data held by class variables. + * Get a snapshot of session data held by instance variables. * * @returns An object with serialized keyrings, keyrings metadata, * and the user password. @@ -2203,7 +2291,7 @@ export class KeyringController extends BaseController< async #getSessionState(): Promise { return { keyrings: await this.#getSerializedKeyrings(), - password: this.#password, + encryptionKey: this.#encryptionKey, }; } @@ -2241,75 +2329,48 @@ 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 credentials - The credentials to unlock the keyrings. * @returns A promise resolving to the deserialized keyrings array. */ async #unlockKeyrings( - password: string | undefined, - encryptionKey?: string, - encryptionSalt?: string, + credentials: + | { + password: string; + } + | { + exportedEncryptionKey: string; + }, ): Promise<{ keyrings: { keyring: EthKeyring; metadata: KeyringMetadata }[]; newMetadata: boolean; }> { return this.#withVaultLock(async () => { - const encryptedVault = this.state.vault; - if (!encryptedVault) { + if (!this.state.vault) { throw new Error(KeyringControllerError.VaultError); } + const parsedEncryptedVault = JSON.parse(this.state.vault); - let vault; - const updatedState: Partial = {}; - - if (this.#cacheEncryptionKey) { - assertIsExportableKeyEncryptor(this.#encryptor); - - if (password) { - 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!; - } + if ('password' in credentials) { + await this.#deriveEncryptionKey(credentials.password); } else { - if (typeof password !== 'string') { - throw new TypeError(KeyringControllerError.WrongPasswordType); - } + const { exportedEncryptionKey } = credentials; + this.#useEncryptionKey( + exportedEncryptionKey, + parsedEncryptedVault.salt, + ); + } - vault = await this.#encryptor.decrypt(password, encryptedVault); - this.#password = password; + const encryptionKey = this.#encryptionKey?.exported; + if (!encryptionKey) { + throw new Error(KeyringControllerError.MissingCredentials); } + const key = await this.#encryptor.importKey(encryptionKey); + const vault = await this.#encryptor.decryptWithKey( + key, + parsedEncryptedVault, + ); + if (!isSerializedKeyringsArray(vault)) { throw new Error(KeyringControllerError.VaultDataError); } @@ -2321,10 +2382,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 = encryptionKey; + state.encryptionSalt = parsedEncryptedVault.salt; }); return { keyrings, newMetadata }; @@ -2341,72 +2400,44 @@ 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); } const serializedKeyrings = await this.#getSerializedKeyrings(); if ( - !serializedKeyrings.some((keyring) => keyring.type === KeyringTypes.hd) + !serializedKeyrings.some( + (keyring) => keyring.type === (KeyringTypes.hd as string), + ) ) { 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) { - 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 (!updatedState.vault) { - throw new Error(KeyringControllerError.MissingVaultData); + const key = await this.#encryptor.importKey(this.#encryptionKey.exported); + const encryptedVault = await this.#encryptor.encryptWithKey( + key, + serializedKeyrings, + ); + if (this.#encryptionKey.salt) { + // We need to include the salt used to derive + // the encryption key, to be able to derive it + // from password again. + encryptedVault.salt = this.#encryptionKey.salt; } + const updatedState: Partial = { + vault: JSON.stringify(encryptedVault), + encryptionKey: this.#encryptionKey.exported, + encryptionSalt: this.#encryptionKey.salt, + }; const updatedKeyrings = await this.#getUpdatedKeyrings(); 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; - } + state.encryptionKey = updatedState.encryptionKey; + state.encryptionSalt = updatedState.encryptionSalt; }); return true; @@ -2421,7 +2452,7 @@ 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; } @@ -2730,13 +2761,16 @@ export class KeyringController extends BaseController< ): Promise { return this.#withControllerLock(async ({ releaseLock }) => { const currentSerializedKeyrings = await this.#getSerializedKeyrings(); - const currentPassword = this.#password; + const currentEncryptionKey = this.#encryptionKey?.exported; + const currentEncryptionSalt = this.#encryptionKey?.salt; try { return await callback({ releaseLock }); } catch (e) { - // Keyrings and password are restored to their previous state - this.#password = currentPassword; + // Keyrings and encryption credentials are restored to their previous state + if (currentEncryptionKey && currentEncryptionSalt) { + this.#useEncryptionKey(currentEncryptionKey, currentEncryptionSalt); + } await this.#restoreSerializedKeyrings(currentSerializedKeyrings); throw e; diff --git a/packages/keyring-controller/src/constants.ts b/packages/keyring-controller/src/constants.ts index d914a3d6f74..eab9969fec9 100644 --- a/packages/keyring-controller/src/constants.ts +++ b/packages/keyring-controller/src/constants.ts @@ -3,6 +3,7 @@ export enum KeyringControllerError { KeyringNotFound = 'KeyringController - Keyring not found.', UnsafeDirectKeyringAccess = 'KeyringController - Returning keyring instances is unsafe', WrongPasswordType = 'KeyringController - Password must be of type string.', + WrongEncryptionKeyType = 'KeyringController - Encryption key must be of type string.', InvalidEmptyPassword = 'KeyringController - Password cannot be empty.', NoFirstAccount = 'KeyringController - First Account not found.', DuplicatedAccount = 'KeyringController - The account you are trying to import is a duplicate', diff --git a/packages/keyring-controller/tests/mocks/mockEncryptor.ts b/packages/keyring-controller/tests/mocks/mockEncryptor.ts index 034d9e32d1d..3edcd539f58 100644 --- a/packages/keyring-controller/tests/mocks/mockEncryptor.ts +++ b/packages/keyring-controller/tests/mocks/mockEncryptor.ts @@ -14,7 +14,7 @@ export const MOCK_ENCRYPTION_SALT = export const MOCK_HARDCODED_KEY = 'key'; export const MOCK_HEX = '0xabcdef0123456789'; // eslint-disable-next-line no-restricted-globals -const MOCK_KEY = Buffer.alloc(32); +export const MOCK_KEY = Buffer.alloc(32); const INVALID_PASSWORD_ERROR = 'Incorrect password.'; export default class MockEncryptor implements ExportableKeyEncryptor { @@ -64,21 +64,20 @@ export default class MockEncryptor implements ExportableKeyEncryptor { }; } - async decryptWithKey(key: unknown, text: string) { - return this.decrypt(key as string, text); + async decryptWithKey(_key: unknown, _text: string) { + return JSON.parse(this.cacheVal || '') ?? {}; } - async keyFromPassword(_password: string) { - return MOCK_KEY; + async keyFromPassword(_password: string, _salt?: string) { + return JSON.parse(MOCK_ENCRYPTION_KEY); } async importKey(key: string) { - if (key === '{}') { - throw new TypeError( - `Failed to execute 'importKey' on 'SubtleCrypto': The provided value is not of type '(ArrayBuffer or ArrayBufferView or JsonWebKey)'.`, - ); - } - return null; + return JSON.parse(key); + } + + async exportKey(key: unknown) { + return JSON.stringify(key); } async updateVault(_vault: string, _password: string) { diff --git a/packages/seedless-onboarding-controller/CHANGELOG.md b/packages/seedless-onboarding-controller/CHANGELOG.md index 12eac9eb3e9..fdb8871c247 100644 --- a/packages/seedless-onboarding-controller/CHANGELOG.md +++ b/packages/seedless-onboarding-controller/CHANGELOG.md @@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **BREAKING:** `SeedlessOnboardingController` now requires an additional `SupportedKeyDerivationOptions` type parameter ([#5963](https://github.com/MetaMask/core/pull/5963)) - Refresh and revoke token handling ([#5917](https://github.com/MetaMask/core/pull/5917)) - **BREAKING:** `authenticate` need extra `refreshToken` and `revokeToken` params, persist refresh token in state and store revoke token temporarily for user in next step - `createToprfKeyAndBackupSeedPhrase`, `fetchAllSecretData` store revoke token in vault diff --git a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts index e233f40c12f..7e23d71f63e 100644 --- a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts +++ b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts @@ -1,6 +1,9 @@ import { keccak256AndHexify } from '@metamask/auth-network-utils'; import type { Messenger } from '@metamask/base-controller'; -import type { EncryptionKey } from '@metamask/browser-passworder'; +import type { + EncryptionKey, + KeyDerivationOptions, +} from '@metamask/browser-passworder'; import { encrypt, decrypt, @@ -8,6 +11,9 @@ import { encryptWithDetail, decryptWithKey as decryptWithKeyBrowserPassworder, importKey as importKeyBrowserPassworder, + exportKey as exportKeyBrowserPassworder, + generateSalt as generateSaltBrowserPassworder, + keyFromPassword as keyFromPasswordBrowserPassworder, } from '@metamask/browser-passworder'; import { TOPRFError, @@ -100,29 +106,36 @@ const MOCK_AUTH_PUB_KEY = 'A09CwPHdl/qo2AjBOHen5d4QORaLedxOrSdgReq8IhzQ'; const MOCK_AUTH_PUB_KEY_OUTDATED = 'Ao2sa8imX7SD4KE4fJLoJ/iBufmaBxSFygG1qUhW2qAb'; -type WithControllerCallback = ({ - controller, - initialState, - encryptor, - messenger, -}: { - controller: SeedlessOnboardingController; - encryptor: VaultEncryptor; - initialState: SeedlessOnboardingControllerState; - messenger: SeedlessOnboardingControllerMessenger; - baseMessenger: Messenger; - toprfClient: ToprfSecureBackup; - mockRefreshJWTToken: jest.Mock; - mockRevokeRefreshToken: jest.Mock; -}) => Promise | ReturnValue; - -type WithControllerOptions = Partial< - SeedlessOnboardingControllerOptions +type WithControllerCallback = + ({ + controller, + initialState, + encryptor, + messenger, + }: { + controller: SeedlessOnboardingController< + EKey, + SupportedKeyDerivationOptions + >; + encryptor: VaultEncryptor; + initialState: SeedlessOnboardingControllerState; + messenger: SeedlessOnboardingControllerMessenger; + baseMessenger: Messenger; + toprfClient: ToprfSecureBackup; + mockRefreshJWTToken: jest.Mock; + mockRevokeRefreshToken: jest.Mock; + }) => Promise | ReturnValue; + +type WithControllerOptions = Partial< + SeedlessOnboardingControllerOptions >; -type WithControllerArgs = - | [WithControllerCallback] - | [WithControllerOptions, WithControllerCallback]; +type WithControllerArgs = + | [WithControllerCallback] + | [ + WithControllerOptions, + WithControllerCallback, + ]; /** * Get the default vault encryptor for the Seedless Onboarding Controller. @@ -142,6 +155,9 @@ function getDefaultSeedlessOnboardingVaultEncryptor() { payload: unknown, ) => Promise, importKey: importKeyBrowserPassworder, + exportKey: exportKeyBrowserPassworder, + generateSalt: generateSaltBrowserPassworder, + keyFromPassword: keyFromPasswordBrowserPassworder, }; } @@ -165,7 +181,11 @@ function createMockVaultEncryptor() { * @returns Whatever the callback returns. */ async function withController( - ...args: WithControllerArgs + ...args: WithControllerArgs< + ReturnValue, + EncryptionKey | webcrypto.CryptoKey, + KeyDerivationOptions + > ) { const [{ ...rest }, fn] = args.length === 2 ? args : [{}, args[0]]; const encryptor = new MockVaultEncryptor(); @@ -179,7 +199,10 @@ async function withController( newRefreshToken: 'newRefreshToken', }); - const controller = new SeedlessOnboardingController({ + const controller = new SeedlessOnboardingController< + EncryptionKey | webcrypto.CryptoKey, + KeyDerivationOptions + >({ encryptor, messenger, network: Web3AuthNetwork.Devnet, @@ -343,9 +366,12 @@ function mockChangeEncKey( * @param seedPhrase - The mock seed phrase. * @param keyringId - The mock keyring id. */ -async function mockCreateToprfKeyAndBackupSeedPhrase( +async function mockCreateToprfKeyAndBackupSeedPhrase< + EKey, + SupportedKeyDerivationOptions, +>( toprfClient: ToprfSecureBackup, - controller: SeedlessOnboardingController, + controller: SeedlessOnboardingController, password: string, seedPhrase: Uint8Array, keyringId: string, @@ -500,7 +526,10 @@ describe('SeedlessOnboardingController', () => { newRefreshToken: 'newRefreshToken', }); const { messenger } = mockSeedlessOnboardingMessenger(); - const controller = new SeedlessOnboardingController({ + const controller = new SeedlessOnboardingController< + EncryptionKey | webcrypto.CryptoKey, + KeyDerivationOptions + >({ messenger, encryptor: getDefaultSeedlessOnboardingVaultEncryptor(), refreshJWTToken: mockRefreshJWTToken, diff --git a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts index cd4bb2799e1..cfd6ccc0c70 100644 --- a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts +++ b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts @@ -121,12 +121,18 @@ const seedlessOnboardingMetadata: StateMetadata extends BaseController< +export class SeedlessOnboardingController< + EncryptionKey, + SupportedKeyDerivationOptions, +> extends BaseController< typeof controllerName, SeedlessOnboardingControllerState, SeedlessOnboardingControllerMessenger > { - readonly #vaultEncryptor: VaultEncryptor; + readonly #vaultEncryptor: VaultEncryptor< + EncryptionKey, + SupportedKeyDerivationOptions + >; readonly #controllerOperationMutex = new Mutex(); @@ -165,7 +171,10 @@ export class SeedlessOnboardingController extends BaseController< network = Web3AuthNetwork.Mainnet, refreshJWTToken, revokeRefreshToken, - }: SeedlessOnboardingControllerOptions) { + }: SeedlessOnboardingControllerOptions< + EncryptionKey, + SupportedKeyDerivationOptions + >) { super({ name: controllerName, metadata: seedlessOnboardingMetadata, diff --git a/packages/seedless-onboarding-controller/src/types.ts b/packages/seedless-onboarding-controller/src/types.ts index b10502ed74c..c55dd093b16 100644 --- a/packages/seedless-onboarding-controller/src/types.ts +++ b/packages/seedless-onboarding-controller/src/types.ts @@ -186,8 +186,8 @@ export type SeedlessOnboardingControllerMessenger = RestrictedMessenger< /** * Encryptor interface for encrypting and decrypting seedless onboarding vault. */ -export type VaultEncryptor = Omit< - ExportableKeyEncryptor, +export type VaultEncryptor = Omit< + ExportableKeyEncryptor, 'encryptWithKey' >; @@ -228,7 +228,10 @@ export type RevokeRefreshToken = (params: { * @param state - The initial state to set on this controller. * @param encryptor - The encryptor to use for encrypting and decrypting seedless onboarding vault. */ -export type SeedlessOnboardingControllerOptions = { +export type SeedlessOnboardingControllerOptions< + EncryptionKey, + SupportedKeyDerivationOptions, +> = { messenger: SeedlessOnboardingControllerMessenger; /** @@ -241,7 +244,7 @@ export type SeedlessOnboardingControllerOptions = { * * @default browser-passworder @link https://github.com/MetaMask/browser-passworder */ - encryptor: VaultEncryptor; + encryptor: VaultEncryptor; /** * A function to get a new jwt token using refresh token. diff --git a/packages/seedless-onboarding-controller/tests/mocks/vaultEncryptor.ts b/packages/seedless-onboarding-controller/tests/mocks/vaultEncryptor.ts index e3568755c45..6e1a3157a0c 100644 --- a/packages/seedless-onboarding-controller/tests/mocks/vaultEncryptor.ts +++ b/packages/seedless-onboarding-controller/tests/mocks/vaultEncryptor.ts @@ -9,7 +9,8 @@ import { webcrypto } from 'node:crypto'; import type { VaultEncryptor } from '../../src/types'; export default class MockVaultEncryptor - implements VaultEncryptor + implements + VaultEncryptor { DEFAULT_DERIVATION_PARAMS: KeyDerivationOptions = { algorithm: 'PBKDF2', @@ -85,7 +86,7 @@ export default class MockVaultEncryptor async keyFromPassword( password: string, - salt: string = this.DEFAULT_SALT, + salt: string, exportable: boolean = true, opts: KeyDerivationOptions = this.DEFAULT_DERIVATION_PARAMS, ) { @@ -217,4 +218,8 @@ export default class MockVaultEncryptor const result = await this.decryptWithKey(key, payload); return result; } + + generateSalt(): string { + return this.DEFAULT_SALT; + } }