diff --git a/packages/keyring-controller/CHANGELOG.md b/packages/keyring-controller/CHANGELOG.md index ad72a80f8be..62e88f97ee9 100644 --- a/packages/keyring-controller/CHANGELOG.md +++ b/packages/keyring-controller/CHANGELOG.md @@ -7,6 +7,26 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Added optional `EncryptionKey`, `SupportedKeyDerivationOptions` and `EncryptionResult` type parameters to the `KeyringController`, `ExportableKeyEncryptor` and `KeyringControllerOptions` types ([#7127](https://github.com/MetaMask/core/pull/7127)) + - This type parameter allows specifying the encryption key, key derivation options and encryption result types supported by the injected encryptor, defaulting to `@metamask/browser-passworder` types. + +### Changed + +- **BREAKING:** The `KeyringController` constructor options now require an encryptor ([#7127](https://github.com/MetaMask/core/pull/7127)) + - The `encryptor` constructor option was previously optional and defaulted to an instance of `@metamask/browser-passworder`. +- **BREAKING:** The `GenericEncryptor` and `ExportableKeyEncryptor` types have been merged into a single `Encryptor` type ([#7127](https://github.com/MetaMask/core/pull/7127)) + +### Removed + +- **BREAKING:** The `cacheEncryptionKey` parameter has been removed from the `KeyringController` constructor options ([#7127](https://github.com/MetaMask/core/pull/7127)) + - 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. + +### Fixed + +- Fixed incorrect type for `decryptWithKey` method of `ExportableKeyEncryptor` (now `Encryptor`) ([#7127](https://github.com/MetaMask/core/pull/7127)) + ## [24.0.0] ### Changed diff --git a/packages/keyring-controller/jest.config.js b/packages/keyring-controller/jest.config.js index 9ad7de73d4f..79cab1c92de 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: 95.78, + branches: 94.03, functions: 100, - lines: 98.68, - statements: 98.69, + lines: 98.31, + statements: 98.33, }, }, diff --git a/packages/keyring-controller/src/KeyringController.test.ts b/packages/keyring-controller/src/KeyringController.test.ts index 0eddeaa4777..dcc35b1e5f5 100644 --- a/packages/keyring-controller/src/KeyringController.test.ts +++ b/packages/keyring-controller/src/KeyringController.test.ts @@ -33,6 +33,7 @@ import type { KeyringControllerOptions, KeyringControllerActions, KeyringMetadata, + SerializedKeyring, } from './KeyringController'; import { AccountImportStrategy, @@ -84,11 +85,49 @@ const uint8ArraySeed = new Uint8Array( const privateKey = '1e4e6a4c0c077f4ae8ddfbf372918e61dd0fb4a4cfa592cb16e7546d505e68fc'; const password = 'password123'; -const freshVault = - '{"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 commonConfig = { chain: Chain.Goerli, hardfork: Hardfork.Berlin }; +const defaultKeyrings: SerializedKeyring[] = [ + { + 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: '' }, + }, +]; + +const defaultCredentials = { password, salt: 'salt' }; + +/** + * Build a vault string with the given keyrings. + * This vault can be used with the MockEncryptor to test KeyringController + * with controlled keyrings. + * + * @param keyrings - The keyrings to include in the vault. + * @returns The vault string. + */ +function createVault(keyrings: SerializedKeyring[] = defaultKeyrings): string { + return JSON.stringify({ + data: JSON.stringify({ + tag: { key: defaultCredentials, iv: 'iv' }, + value: keyrings, + }), + iv: 'iv', + salt: 'salt', + }); +} + describe('KeyringController', () => { afterEach(() => { sinon.restore(); @@ -96,28 +135,6 @@ describe('KeyringController', () => { }); describe('constructor', () => { - it('should use the default encryptor if none is provided', async () => { - expect( - () => - new KeyringController({ - messenger: buildKeyringControllerMessenger(), - cacheEncryptionKey: true, - }), - ).not.toThrow(); - }); - - 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, - encryptor: { encrypt: jest.fn(), decrypt: jest.fn() }, - }), - ).toThrow(KeyringControllerError.UnsupportedEncryptionKeyExport); - }); - it('allows overwriting the built-in Simple keyring builder', async () => { const mockSimpleKeyringBuilder = // todo: keyring types are mismatched, this should be fixed in they keyrings themselves @@ -150,28 +167,26 @@ describe('KeyringController', () => { { skipVaultCreation: true, state: { - vault: 'my vault', + vault: createVault([ + { + type: KeyringTypes.hd, + data: '', + metadata: { id: 'hd', name: '' }, + }, + { + type: 'Unsupported', + data: '', + metadata: { id: 'unsupported', name: '' }, + }, + { + type: KeyringTypes.hd, + data: '', + metadata: { id: 'hd2', name: '' }, + }, + ]), }, }, - async ({ controller, encryptor }) => { - jest.spyOn(encryptor, 'decrypt').mockResolvedValueOnce([ - { - type: KeyringTypes.hd, - data: '', - metadata: { id: 'hd', name: '' }, - }, - { - type: 'Unsupported', - data: '', - metadata: { id: 'unsupported', name: '' }, - }, - { - type: KeyringTypes.hd, - data: '', - metadata: { id: 'hd2', name: '' }, - }, - ]); - + async ({ controller }) => { await controller.submitPassword(password); expect(controller.state.keyrings).toHaveLength(2); @@ -194,29 +209,27 @@ describe('KeyringController', () => { { skipVaultCreation: true, state: { - vault: 'my vault', + vault: createVault([ + { + type: 'HD Key Tree', + data: '', + metadata: { id: 'hd', name: '' }, + }, + { + type: 'HD Key Tree', + data: '', + metadata: { id: 'hd2', name: '' }, + }, + // This keyring was already unsupported + // (no metadata, and is at the end of the array) + { + type: MockKeyring.type, + data: 'unsupported', + }, + ]), }, }, - async ({ controller, encryptor }) => { - jest.spyOn(encryptor, 'decrypt').mockResolvedValueOnce([ - { - type: 'HD Key Tree', - data: '', - metadata: { id: 'hd', name: '' }, - }, - { - type: 'HD Key Tree', - data: '', - metadata: { id: 'hd2', name: '' }, - }, - // This keyring was already unsupported - // (no metadata, and is at the end of the array) - { - type: MockKeyring.type, - data: 'unsupported', - }, - ]); - + async ({ controller }) => { await controller.submitPassword(password); expect(controller.state.keyrings).toHaveLength(2); @@ -300,12 +313,12 @@ describe('KeyringController', () => { it('should throw an error if there is no primary keyring', async () => { await withController( - { skipVaultCreation: true, state: { vault: 'my vault' } }, - async ({ controller, encryptor }) => { - jest - .spyOn(encryptor, 'decrypt') - .mockResolvedValueOnce([{ type: 'Unsupported', data: '' }]); - await controller.submitPassword('123'); + { + skipVaultCreation: true, + state: { vault: createVault([{ type: 'Unsupported', data: '' }]) }, + }, + async ({ controller }) => { + await controller.submitPassword(password); await expect(controller.addNewAccount()).rejects.toThrow( 'No HD keyring found', @@ -564,255 +577,179 @@ 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 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( - { 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 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') - .mockResolvedValue([]); - 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').mockResolvedValue([]); + 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); + }); + }); + + 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(); - !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'); - }, - ); - }); + expect(controller.state.encryptionKey).toBeDefined(); + expect(controller.state.encryptionSalt).toBeDefined(); }); - }), - ); + }); + }); }); describe('setLocked', () => { @@ -1210,12 +1147,12 @@ describe('KeyringController', () => { it('should throw an error if there is no keyring', async () => { await withController( - { skipVaultCreation: true, state: { vault: 'my vault' } }, - async ({ controller, encryptor }) => { - jest - .spyOn(encryptor, 'decrypt') - .mockResolvedValueOnce([{ type: 'Unsupported', data: '' }]); - await controller.submitPassword('123'); + { + skipVaultCreation: true, + state: { vault: createVault([{ type: 'Unsupported', data: '' }]) }, + }, + async ({ controller }) => { + await controller.submitPassword(password); await expect( controller.getKeyringForAccount( @@ -2615,600 +2552,453 @@ 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 unlock also with unsupported keyrings', async () => { - await withController( - { - cacheEncryptionKey, - skipVaultCreation: true, - state: { vault: freshVault }, - }, - async ({ controller, encryptor }) => { - jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ - { - type: 'UnsupportedKeyring', - data: '0x1234', - }, - ]); - - await controller.submitPassword(password); + 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); + }); + }); - expect(controller.state.isUnlocked).toBe(true); - }, - ); - }); + it('should unlock also with unsupported keyrings', async () => { + await withController( + { + skipVaultCreation: true, + state: { + vault: createVault([ + { + type: 'UnsupportedKeyring', + data: '0x1234', + }, + ]), + }, + }, + async ({ controller }) => { + await controller.submitPassword(password); - it('should throw error if vault unlocked has an unexpected shape', async () => { - await withController( - { - cacheEncryptionKey, - skipVaultCreation: true, - state: { vault: freshVault }, - }, - async ({ controller, encryptor }) => { - jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ - { - foo: 'bar', - }, - ]); + expect(controller.state.isUnlocked).toBe(true); + }, + ); + }); - await expect(controller.submitPassword(password)).rejects.toThrow( - KeyringControllerError.VaultDataError, - ); - }, + it('should throw error if vault unlocked has an unexpected shape', async () => { + await withController( + { + skipVaultCreation: true, + state: { + vault: createVault([ + { + // @ts-expect-error testing invalid vault shape + foo: 'bar', + }, + ]), + }, + }, + async ({ controller }) => { + 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 () => { - stubKeyringClassWithAccount(HdKeyring, '0x123'); - await withController( - { - cacheEncryptionKey, - state: { vault: freshVault }, - skipVaultCreation: true, - }, - async ({ controller, encryptor }) => { - jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ - { - type: KeyringTypes.hd, - data: { - accounts: ['0x123'], - }, - metadata: { - id: '123', - name: '', - }, - }, - ]); - - await controller.submitPassword(password); + }, + ); + }); - expect(controller.state.keyrings).toStrictEqual([ - { - type: KeyringTypes.hd, + it('should unlock succesfully when the controller is instantiated with an existing `keyringsMetadata`', async () => { + stubKeyringClassWithAccount(HdKeyring, '0x123'); + await withController( + { + state: { + vault: createVault([ + { + type: KeyringTypes.hd, + data: { 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, + metadata: { + id: '123', + name: '', }, - 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'], - }, - }, - ]); - hdKeyringSerializeSpy.mockResolvedValue({ - // @ts-expect-error we are assigning a mock value - accounts: ['0x123'], - }); + ]), + }, + skipVaultCreation: true, + }, + async ({ controller }) => { + 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: ['0x123'], + metadata: { + id: '123', + 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( + 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: createVault([ { - cacheEncryptionKey: false, - 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'], - }, - }, - ]); - hdKeyringSerializeSpy.mockResolvedValue({ - // @ts-expect-error we are assigning a mock value + type: KeyringTypes.hd, + data: { 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: '', - }, - }, - ]); + }, }, - ); + ]), + }, + skipVaultCreation: true, + }, + async ({ controller, encryptor }) => { + const encryptWithKeySpy = jest.spyOn(encryptor, 'encryptWithKey'); + jest.spyOn(encryptor, 'importKey').mockResolvedValue('imported key'); + hdKeyringSerializeSpy.mockResolvedValue({ + // @ts-expect-error we are assigning a mock value + accounts: ['0x123'], }); - it('should unlock the wallet if the state has a duplicate account and the encryption parameters are outdated', async () => { - stubKeyringClassWithAccount(MockKeyring, '0x123'); - stubKeyringClassWithAccount(HdKeyring, '0x123'); - await withController( - { - skipVaultCreation: true, - cacheEncryptionKey, - 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([ - { - type: KeyringTypes.hd, - 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.keyrings).toStrictEqual([ + { + type: KeyringTypes.hd, + accounts: expect.any(Array), + metadata: { + id: expect.any(String), + name: '', + }, }, - ); - }); - - it('should unlock the wallet also if encryption parameters are outdated and the vault upgrade fails', async () => { - await withController( + ]); + expect(encryptWithKeySpy).toHaveBeenCalledWith('imported key', [ { - skipVaultCreation: true, - cacheEncryptionKey, - state: { vault: freshVault }, + type: KeyringTypes.hd, + data: { + accounts: ['0x123'], + }, + metadata: { + id: expect.any(String), + name: '', + }, }, - 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); + it('should unlock the wallet if the state has a duplicate account and the encryption parameters are outdated', async () => { + stubKeyringClassWithAccount(MockKeyring, '0x123'); + stubKeyringClassWithAccount(HdKeyring, '0x123'); + await withController( + { + skipVaultCreation: true, + state: { + vault: createVault([ + { + type: KeyringTypes.hd, + data: {}, + }, + { + type: MockKeyring.type, + data: {}, + }, + ]), + }, + keyringBuilders: [keyringBuilderFactory(MockKeyring)], + }, + async ({ controller, encryptor, messenger }) => { + const unlockListener = jest.fn(); + messenger.subscribe('KeyringController:unlock', unlockListener); + jest.spyOn(encryptor, 'isVaultUpdated').mockReturnValue(false); - expect(controller.state.isUnlocked).toBe(true); - }, - ); - }); + await controller.submitPassword(password); - it('should unlock the wallet discarding existing duplicate accounts', async () => { - stubKeyringClassWithAccount(MockKeyring, '0x123'); - stubKeyringClassWithAccount(HdKeyring, '0x123'); - await withController( - { - skipVaultCreation: true, - cacheEncryptionKey, - state: { vault: freshVault }, - keyringBuilders: [keyringBuilderFactory(MockKeyring)], - }, - 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: {}, + 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( + { + skipVaultCreation: true, + state: { + vault: createVault([ + { + type: KeyringTypes.hd, + data: { + accounts: ['0x123'], }, - ]); + }, + ]), + }, + }, + async ({ controller, encryptor }) => { + jest.spyOn(encryptor, 'isVaultUpdated').mockReturnValue(false); + jest.spyOn(encryptor, 'encrypt').mockRejectedValue(new Error()); - 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.isUnlocked).toBe(true); + }, + ); + }); - cacheEncryptionKey && - it('should upgrade the vault encryption if the key encryptor has different parameters', async () => { - await withController( + it('should unlock the wallet discarding existing duplicate accounts', async () => { + stubKeyringClassWithAccount(MockKeyring, '0x123'); + stubKeyringClassWithAccount(HdKeyring, '0x123'); + await withController( + { + skipVaultCreation: true, + state: { + vault: createVault([ { - skipVaultCreation: true, - cacheEncryptionKey, - 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'], - }, - }, - ]); - - await controller.submitPassword(password); - - expect(encryptSpy).toHaveBeenCalledTimes(1); + type: KeyringTypes.hd, + data: {}, }, - ); - }); - - 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 }, + type: MockKeyring.type, + data: {}, }, - 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); - - expect(encryptSpy).not.toHaveBeenCalled(); - }, - ); - }); + ]), + }, + keyringBuilders: [keyringBuilderFactory(MockKeyring)], + }, + async ({ controller, messenger }) => { + const unlockListener = jest.fn(); + messenger.subscribe('KeyringController:unlock', unlockListener); + + await controller.submitPassword(password); - !cacheEncryptionKey && - it('should upgrade the vault encryption if the generic encryptor has different parameters', async () => { - await withController( + expect(controller.state.keyrings).toHaveLength(1); // Second keyring will be skipped as "unsupported". + expect(unlockListener).toHaveBeenCalledTimes(1); + }, + ); + }); + + it('should upgrade the vault encryption if the key encryptor has different parameters', async () => { + await withController( + { + skipVaultCreation: true, + state: { + vault: createVault([ { - skipVaultCreation: true, - cacheEncryptionKey, - 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'], - }, - }, - ]); - - await controller.submitPassword(password); - - expect(encryptSpy).toHaveBeenCalledTimes(1); + type: KeyringTypes.hd, + data: { + accounts: ['0x123'], + }, }, - ); - }); + ]), + }, + }, + async ({ controller, encryptor }) => { + jest.spyOn(encryptor, 'isVaultUpdated').mockReturnValue(false); + const encryptSpy = jest.spyOn(encryptor, 'encryptWithKey'); - it('should not upgrade the vault encryption if the encryptor has the same parameters and the keyring has metadata', async () => { - await withController( - { - skipVaultCreation: true, - cacheEncryptionKey, - state: { vault: freshVault }, - }, - 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); + + expect(encryptSpy).toHaveBeenCalledTimes(1); + }, + ); + }); + + it('should not upgrade the vault encryption if the key encryptor has the same parameters', async () => { + await withController( + { + skipVaultCreation: true, + state: { + vault: createVault([ + { + type: KeyringTypes.hd, + data: { + accounts: ['0x123'], }, - ]); + }, + ]), + }, + }, + async ({ controller, encryptor }) => { + jest.spyOn(encryptor, 'isVaultUpdated').mockReturnValue(true); + const encryptSpy = jest.spyOn(encryptor, 'encrypt'); - 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 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 not upgrade the vault encryption if the encryptor has the same parameters and the keyring has metadata', async () => { + await withController( + { + skipVaultCreation: true, + state: { + vault: createVault([ + { + 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'); - 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(); - }); - }); + await controller.submitPassword(password); - 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); - }, - ); - }); - }), - ); - }); + expect(encryptSpy).not.toHaveBeenCalled(); + }, + ); + }); - describe('submitEncryptionKey', () => { - it('should submit encryption key and decrypt', async () => { + 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( - { cacheEncryptionKey: true }, - async ({ controller, initialState }) => { - await controller.submitEncryptionKey( - MOCK_ENCRYPTION_KEY, - initialState.encryptionSalt as string, - ); - expect(controller.state).toStrictEqual(initialState); + { + skipVaultCreation: true, + state: { + vault: createVault(), + // @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(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, + vault: createVault([ + { + type: 'UnsupportedKeyring', + data: '0x1234', + }, + ]), // @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, initialState, encryptor }) => { - jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ - { - type: 'UnsupportedKeyring', - data: '0x1234', - }, - ]); - + async ({ controller, initialState }) => { await controller.submitEncryptionKey( MOCK_ENCRYPTION_KEY, initialState.encryptionSalt as string, @@ -3226,10 +3016,14 @@ describe('KeyringController', () => { }); await withController( { - cacheEncryptionKey: true, skipVaultCreation: true, state: { - vault: freshVault, + vault: createVault([ + { + type: KeyringTypes.hd, + data: '0x123', + }, + ]), // @ts-expect-error we want to force the controller to have an // encryption salt equal to the one in the vault encryptionSalt: SALT, @@ -3237,12 +3031,6 @@ describe('KeyringController', () => { }, async ({ controller, initialState, encryptor }) => { const encryptWithKeySpy = jest.spyOn(encryptor, 'encryptWithKey'); - jest.spyOn(encryptor, 'decryptWithKey').mockResolvedValueOnce([ - { - type: KeyringTypes.hd, - data: '0x123', - }, - ]); await controller.submitEncryptionKey( MOCK_ENCRYPTION_KEY, @@ -3270,110 +3058,81 @@ 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, - ), - ).rejects.toThrow(KeyringControllerError.VaultDataError); - }, - ); + await expect( + controller.submitEncryptionKey( + MOCK_ENCRYPTION_KEY, + initialState.encryptionSalt as string, + ), + ).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); + }); }); }); describe('exportEncryptionKey', () => { it('should export encryption key and unlock', async () => { - await withController( - { cacheEncryptionKey: true }, - async ({ controller }) => { - const encryptionKey = await controller.exportEncryptionKey(); - expect(encryptionKey).toBeDefined(); + await withController(async ({ controller }) => { + const encryptionKey = await controller.exportEncryptionKey(); + expect(encryptionKey).toBeDefined(); - await controller.setLocked(); + await controller.setLocked(); - await controller.submitEncryptionKey(encryptionKey); + await controller.submitEncryptionKey(encryptionKey); - expect(controller.isUnlocked()).toBe(true); - }, - ); + expect(controller.isUnlocked()).toBe(true); + }); }); it('should throw error if controller is locked', async () => { - await withController( - { cacheEncryptionKey: true }, - async ({ controller }) => { - await controller.setLocked(); - await expect(controller.exportEncryptionKey()).rejects.toThrow( - KeyringControllerError.ControllerLocked, - ); - }, - ); - }); - - it('should throw error if encryptionKey is not set', async () => { await withController(async ({ controller }) => { + await controller.setLocked(); await expect(controller.exportEncryptionKey()).rejects.toThrow( - KeyringControllerError.EncryptionKeyNotSet, + KeyringControllerError.ControllerLocked, ); }); }); it('should export key after password change', async () => { - await withController( - { cacheEncryptionKey: true }, - async ({ controller }) => { - await controller.changePassword('new password'); - const encryptionKey = await controller.exportEncryptionKey(); - expect(encryptionKey).toBeDefined(); - }, - ); + await withController(async ({ controller }) => { + await controller.changePassword('new password'); + const encryptionKey = await controller.exportEncryptionKey(); + expect(encryptionKey).toBeDefined(); + }); }); it('should export key after password change to the same password', async () => { - await withController( - { cacheEncryptionKey: true }, - async ({ controller }) => { - await controller.changePassword(password); - const encryptionKey = await controller.exportEncryptionKey(); - expect(encryptionKey).toBeDefined(); - }, - ); + await withController(async ({ controller }) => { + await controller.changePassword(password); + const encryptionKey = await controller.exportEncryptionKey(); + expect(encryptionKey).toBeDefined(); + }); }); }); @@ -3439,12 +3198,12 @@ describe('KeyringController', () => { it('should throw an error if there is no primary keyring', async () => { await withController( - { skipVaultCreation: true, state: { vault: 'my vault' } }, - async ({ controller, encryptor }) => { - jest - .spyOn(encryptor, 'decrypt') - .mockResolvedValueOnce([{ type: 'Unsupported', data: '' }]); - await controller.submitPassword('123'); + { + skipVaultCreation: true, + state: { vault: createVault([{ type: 'Unsupported', data: '' }]) }, + }, + async ({ controller }) => { + await controller.submitPassword(password); await expect(controller.verifySeedPhrase()).rejects.toThrow( KeyringControllerError.KeyringNotFound, @@ -4283,7 +4042,7 @@ describe('KeyringController', () => { it('includes expected state in debug snapshots', async () => { await withController( // Skip vault creation and use static vault to get deterministic state snapshot - { skipVaultCreation: true, state: { vault: freshVault } }, + { skipVaultCreation: true, state: { vault: createVault() } }, ({ controller }) => { expect( deriveStateFromMetadata( @@ -4303,7 +4062,7 @@ describe('KeyringController', () => { it('includes expected state in state logs', async () => { await withController( // Skip vault creation and use static vault to get deterministic state snapshot - { skipVaultCreation: true, state: { vault: freshVault } }, + { skipVaultCreation: true, state: { vault: createVault() } }, ({ controller }) => { expect( deriveStateFromMetadata( @@ -4324,7 +4083,7 @@ describe('KeyringController', () => { it('persists expected state', async () => { await withController( // Skip vault creation and use static vault to get deterministic state snapshot - { skipVaultCreation: true, state: { vault: freshVault } }, + { skipVaultCreation: true, state: { vault: createVault() } }, ({ controller }) => { expect( deriveStateFromMetadata( @@ -4344,7 +4103,7 @@ describe('KeyringController', () => { it('exposes expected state to UI', async () => { await withController( // Skip vault creation and use static vault to get deterministic state snapshot - { skipVaultCreation: true, state: { vault: freshVault } }, + { skipVaultCreation: true, state: { vault: createVault() } }, ({ controller }) => { expect( deriveStateFromMetadata( diff --git a/packages/keyring-controller/src/KeyringController.ts b/packages/keyring-controller/src/KeyringController.ts index 9b801696d44..a8eb0c702bd 100644 --- a/packages/keyring-controller/src/KeyringController.ts +++ b/packages/keyring-controller/src/KeyringController.ts @@ -1,7 +1,7 @@ import type { TypedTransaction, TypedTxData } from '@ethereumjs/tx'; import { isValidPrivate, getBinarySize } from '@ethereumjs/util'; import { BaseController } from '@metamask/base-controller'; -import * as encryptorUtils from '@metamask/browser-passworder'; +import type * as encryptorUtils from '@metamask/browser-passworder'; import { HdKeyring } from '@metamask/eth-hd-keyring'; import { normalize as ethNormalize } from '@metamask/eth-sig-util'; import SimpleKeyring from '@metamask/eth-simple-keyring'; @@ -252,20 +252,21 @@ export type KeyringControllerMessenger = Messenger< KeyringControllerEvents >; -export type KeyringControllerOptions = { +export type KeyringControllerOptions< + EncryptionKey = encryptorUtils.EncryptionKey | CryptoKey, + SupportedKeyDerivationOptions = encryptorUtils.KeyDerivationOptions, + EncryptionResult extends + EncryptionResultConstraint = DefaultEncryptionResult, +> = { keyringBuilders?: { (): EthKeyring; type: string }[]; messenger: KeyringControllerMessenger; state?: { vault?: string; keyringsMetadata?: KeyringMetadata[] }; -} & ( - | { - cacheEncryptionKey: true; - encryptor?: ExportableKeyEncryptor; - } - | { - cacheEncryptionKey?: false; - encryptor?: GenericEncryptor | ExportableKeyEncryptor; - } -); + encryptor: Encryptor< + EncryptionKey, + SupportedKeyDerivationOptions, + EncryptionResult + >; +}; /** * A keyring object representation. @@ -335,11 +336,28 @@ type SessionState = { password?: string; }; +export type EncryptionResultConstraint = { + salt?: string; + keyMetadata?: SupportedKeyMetadata; +}; + +export type DefaultEncryptionResult = { + data: string; + iv: string; + salt?: string; + keyMetadata?: SupportedKeyMetadata; +}; + /** - * A generic encryptor interface that supports encrypting and decrypting - * serializable data with a password. + * An encryptor interface that supports encrypting and decrypting + * serializable data with a password, and exporting and importing keys. */ -export type GenericEncryptor = { +export type Encryptor< + EncryptionKey = encryptorUtils.EncryptionKey | CryptoKey, + SupportedKeyDerivationParams = encryptorUtils.KeyDerivationOptions, + EncryptionResult extends + EncryptionResultConstraint = DefaultEncryptionResult, +> = { /** * Encrypts the given object with the given password. * @@ -368,73 +386,65 @@ export type GenericEncryptor = { vault: string, targetDerivationParams?: encryptorUtils.KeyDerivationOptions, ) => boolean; + /** + * 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 serialized 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 serialized 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 encryptedObject - The encrypted string to decrypt. + * @returns The decrypted object. + */ + decryptWithKey: ( + key: EncryptionKey, + encryptedObject: EncryptionResult, + ) => Promise; + /** + * Decrypts the given encrypted string with the given password, and returns + * the decrypted object and the salt and serialized 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 serialized key string used for + * encryption. + */ + decryptWithDetail: ( + password: string, + encryptedString: string, + ) => Promise; + /** + * Generates an encryption key from a serialized key. + * + * @param key - The serialized key string. + * @returns The encryption key. + */ + importKey: (key: string) => Promise; }; -/** - * 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 KeyringSelector = | { type: string; @@ -508,30 +518,6 @@ function assertHasUint8ArrayMnemonic( } } -/** - * Assert that the provided encryptor supports - * encryption and encryption key export. - * - * @param encryptor - The encryptor to check. - * @throws If the encryptor does not support key encryption. - */ -function assertIsExportableKeyEncryptor( - encryptor: GenericEncryptor | ExportableKeyEncryptor, -): asserts encryptor is ExportableKeyEncryptor { - if ( - !( - 'importKey' in encryptor && - typeof encryptor.importKey === 'function' && - 'decryptWithKey' in encryptor && - typeof encryptor.decryptWithKey === 'function' && - 'encryptWithKey' in encryptor && - typeof encryptor.encryptWithKey === 'function' - ) - ) { - throw new Error(KeyringControllerError.UnsupportedEncryptionKeyExport); - } -} - /** * Assert that the provided password is a valid non-empty string. * @@ -647,7 +633,12 @@ function normalize(address: string): string | undefined { * with the internal keyring controller and handling certain complex operations that involve the * keyrings. */ -export class KeyringController extends BaseController< +export class KeyringController< + EncryptionKey = encryptorUtils.EncryptionKey | CryptoKey, + SupportedKeyDerivationOptions = encryptorUtils.KeyDerivationOptions, + EncryptionResult extends + EncryptionResultConstraint = DefaultEncryptionResult, +> extends BaseController< typeof name, KeyringControllerState, KeyringControllerMessenger @@ -658,9 +649,11 @@ export class KeyringController extends BaseController< readonly #keyringBuilders: { (): EthKeyring; type: string }[]; - readonly #encryptor: GenericEncryptor | ExportableKeyEncryptor; - - readonly #cacheEncryptionKey: boolean; + readonly #encryptor: Encryptor< + EncryptionKey, + SupportedKeyDerivationOptions, + EncryptionResult + >; #keyrings: { keyring: EthKeyring; metadata: KeyringMetadata }[]; @@ -678,13 +671,14 @@ export class KeyringController extends BaseController< * @param options.messenger - A restricted messenger. * @param options.state - Initial state to set on this controller. */ - constructor(options: KeyringControllerOptions) { - const { - encryptor = encryptorUtils, - keyringBuilders, - messenger, - state, - } = options; + constructor( + options: KeyringControllerOptions< + EncryptionKey, + SupportedKeyDerivationOptions, + EncryptionResult + >, + ) { + const { encryptor, keyringBuilders, messenger, state } = options; super({ name, @@ -735,13 +729,6 @@ export class KeyringController extends BaseController< 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(); } @@ -1463,12 +1450,10 @@ export class KeyringController extends BaseController< // 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; + }); }); } @@ -2061,50 +2046,36 @@ export class KeyringController extends BaseController< 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; - 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); - updatedState.encryptionKey = result.exportedKeyString; - updatedState.encryptionSalt = result.salt; + if (encryptionSalt && encryptionSalt !== parsedEncryptedVault.salt) { + throw new Error(KeyringControllerError.ExpiredCredentials); } else { - const parsedEncryptedVault = JSON.parse(encryptedVault); - - if (encryptionSalt && encryptionSalt !== parsedEncryptedVault.salt) { - throw new Error(KeyringControllerError.ExpiredCredentials); - } else { - encryptionSalt = parsedEncryptedVault.salt as string; - } - - 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; - updatedState.encryptionSalt = encryptionSalt; + encryptionSalt = parsedEncryptedVault.salt as string; } - } else { - if (typeof password !== 'string') { + + if (typeof encryptionKey !== 'string') { throw new TypeError(KeyringControllerError.WrongPasswordType); } - vault = await this.#encryptor.decrypt(password, encryptedVault); - this.#password = password; + 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; + updatedState.encryptionSalt = encryptionSalt; } if (!isSerializedKeyringsArray(vault)) { @@ -2162,33 +2133,23 @@ export class KeyringController extends BaseController< 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, + 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; } if (!updatedState.vault) { diff --git a/packages/keyring-controller/tests/mocks/mockEncryptor.ts b/packages/keyring-controller/tests/mocks/mockEncryptor.ts index e8aaf09d81a..86d7c932690 100644 --- a/packages/keyring-controller/tests/mocks/mockEncryptor.ts +++ b/packages/keyring-controller/tests/mocks/mockEncryptor.ts @@ -9,7 +9,7 @@ import type { import type { Json } from '@metamask/utils'; import { isEqual } from 'lodash'; -import type { ExportableKeyEncryptor } from '../../src/KeyringController'; +import type { Encryptor } from '../../src/KeyringController'; export const PASSWORD = 'password123'; export const SALT = 'salt'; @@ -27,9 +27,9 @@ function deriveKey(password: string, salt: string) { }; } -export default class MockEncryptor implements ExportableKeyEncryptor { +export default class MockEncryptor implements Encryptor { async encrypt(password: string, dataObj: Json): Promise { - const salt = generateSalt(); + const salt = this.generateSalt(); const key = deriveKey(password, salt); const result = await this.encryptWithKey(key, dataObj); return JSON.stringify({ @@ -39,9 +39,9 @@ export default class MockEncryptor implements ExportableKeyEncryptor { } async decrypt(password: string, text: string): Promise { - const { salt } = JSON.parse(text); - const key = deriveKey(password, salt); - return await this.decryptWithKey(key, text); + const payload = JSON.parse(text); + const key = deriveKey(password, payload.salt); + return await this.decryptWithKey(key, payload); } async encryptWithDetail( @@ -49,7 +49,7 @@ export default class MockEncryptor implements ExportableKeyEncryptor { dataObj: Json, salt?: string, ): Promise { - const _salt = salt ?? generateSalt(); + const _salt = salt ?? this.generateSalt(); const key = deriveKey(password, _salt); const result = await this.encryptWithKey(key, dataObj); return { @@ -65,11 +65,11 @@ export default class MockEncryptor implements ExportableKeyEncryptor { password: string, text: string, ): Promise { - const { salt } = JSON.parse(text); - const key = deriveKey(password, salt); + const payload = JSON.parse(text); + const key = deriveKey(password, payload.salt); return { - vault: await this.decryptWithKey(key, text), - salt, + vault: await this.decryptWithKey(key, payload), + salt: payload.salt, exportedKeyString: JSON.stringify(key), }; } @@ -85,7 +85,10 @@ export default class MockEncryptor implements ExportableKeyEncryptor { }; } - async decryptWithKey(key: unknown, ciphertext: string): Promise { + async decryptWithKey( + key: unknown, + ciphertext: EncryptionResult, + ): Promise { // This conditional assignment is required because sometimes the keyring // controller passes in the parsed object instead of the string. const ciphertextObj = @@ -105,18 +108,15 @@ export default class MockEncryptor implements ExportableKeyEncryptor { return _vault; } + generateSalt() { + return SALT; + } + isVaultUpdated(_vault: string) { return true; } } -function generateSalt() { - // Generate random salt. - - // return crypto.randomUUID(); - return SALT; // TODO some tests rely on fixed salt, but wouldn't it be better to generate random value here? -} - function generateIV() { // Generate random salt. diff --git a/packages/seedless-onboarding-controller/CHANGELOG.md b/packages/seedless-onboarding-controller/CHANGELOG.md index 9957ee77f14..44d5e0536b3 100644 --- a/packages/seedless-onboarding-controller/CHANGELOG.md +++ b/packages/seedless-onboarding-controller/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- `SeedlessOnboardingController` now accepts an optional `SupportedKeyDerivationOptions` type parameter ([#7127](https://github.com/MetaMask/core/pull/7127)) + - The type parameter can be used to specify which key derivation algorithms are supported by the controller instance. + ## [6.1.0] ### Changed diff --git a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.test.ts index fe1d10682e4..885116e6678 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 { deriveStateFromMetadata } from '@metamask/base-controller'; -import type { EncryptionKey } from '@metamask/browser-passworder'; +import type { + EncryptionKey, + KeyDerivationOptions, +} from '@metamask/browser-passworder'; import { encrypt, decrypt, @@ -109,31 +112,38 @@ 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: RootMessenger; - keyringControllerMessenger: MockKeyringControllerMessenger; - toprfClient: ToprfSecureBackup; - mockRefreshJWTToken: jest.Mock; - mockRevokeRefreshToken: jest.Mock; - mockRenewRefreshToken: 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: RootMessenger; + keyringControllerMessenger: MockKeyringControllerMessenger; + toprfClient: ToprfSecureBackup; + mockRefreshJWTToken: jest.Mock; + mockRevokeRefreshToken: jest.Mock; + mockRenewRefreshToken: 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. @@ -176,7 +186,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(); @@ -444,9 +458,12 @@ async function mockChangePassword( * @param seedPhrase - The mock seed phrase. * @param keyringId - The mock keyring id. */ -async function mockCreateToprfKeyAndBackupSeedPhrase( +async function mockCreateToprfKeyAndBackupSeedPhrase< + EKey, + SupportedKeyDerivationParams, +>( toprfClient: ToprfSecureBackup, - controller: SeedlessOnboardingController, + controller: SeedlessOnboardingController, password: string, seedPhrase: Uint8Array, keyringId: string, diff --git a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts index 5811e04a27f..0baa8e2cae2 100644 --- a/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts +++ b/packages/seedless-onboarding-controller/src/SeedlessOnboardingController.ts @@ -1,5 +1,6 @@ import { keccak256AndHexify } from '@metamask/auth-network-utils'; import { BaseController, type StateMetadata } from '@metamask/base-controller'; +import type * as encryptionUtils from '@metamask/browser-passworder'; import type { KeyPair, RecoverEncryptionKeyResult, @@ -223,12 +224,18 @@ const seedlessOnboardingMetadata: StateMetadata extends BaseController< +export class SeedlessOnboardingController< + EncryptionKey, + SupportedKeyDerivationOptions = encryptionUtils.KeyDerivationOptions, +> extends BaseController< typeof controllerName, SeedlessOnboardingControllerState, SeedlessOnboardingControllerMessenger > { - readonly #vaultEncryptor: VaultEncryptor; + readonly #vaultEncryptor: VaultEncryptor< + EncryptionKey, + SupportedKeyDerivationOptions + >; readonly #controllerOperationMutex = new Mutex(); @@ -285,7 +292,10 @@ export class SeedlessOnboardingController extends BaseController< revokeRefreshToken, renewRefreshToken, passwordOutdatedCacheTTL = PASSWORD_OUTDATED_CACHE_TTL_MS, - }: 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 4788aa10d21..cf471a4344d 100644 --- a/packages/seedless-onboarding-controller/src/types.ts +++ b/packages/seedless-onboarding-controller/src/types.ts @@ -2,7 +2,7 @@ import type { ControllerGetStateAction, ControllerStateChangeEvent, } from '@metamask/base-controller'; -import type { ExportableKeyEncryptor } from '@metamask/keyring-controller'; +import type { Encryptor } from '@metamask/keyring-controller'; import type { Messenger } from '@metamask/messenger'; import type { KeyPair, NodeAuthTokens } from '@metamask/toprf-secure-backup'; import type { MutexInterface } from 'async-mutex'; @@ -216,8 +216,8 @@ export type SeedlessOnboardingControllerMessenger = Messenger< /** * Encryptor interface for encrypting and decrypting seedless onboarding vault. */ -export type VaultEncryptor = Omit< - ExportableKeyEncryptor, +export type VaultEncryptor = Omit< + Encryptor, 'encryptWithKey' >; @@ -270,7 +270,10 @@ export type RenewRefreshToken = (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, + SupportedKeyDerivationParams, +> = { messenger: SeedlessOnboardingControllerMessenger; /** @@ -283,7 +286,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..e97c0b49a0a 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', @@ -151,7 +152,7 @@ export default class MockVaultEncryptor async decryptWithKey( encryptionKey: EncryptionKey | webcrypto.CryptoKey, - payload: string, + payload: EncryptionResult, ) { let encData: EncryptionResult; if (typeof payload === 'string') {