From c1ca69abf00af6aac8c1842b68cda9e0914c66ca Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Tue, 28 Nov 2023 13:29:21 +0100 Subject: [PATCH 1/2] chore: update browser-passworder --- package.json | 2 +- src/KeyringController.test.ts | 19 ++++--------------- src/KeyringController.ts | 5 ++--- src/test/encryptor.mock.ts | 4 ++++ src/types.ts | 21 ++++++++++++++++++++- yarn.lock | 4 ++-- 6 files changed, 33 insertions(+), 22 deletions(-) diff --git a/package.json b/package.json index 832b7dca..fa1b2ec9 100644 --- a/package.json +++ b/package.json @@ -44,7 +44,7 @@ }, "dependencies": { "@ethereumjs/tx": "^4.2.0", - "@metamask/browser-passworder": "^4.2.0", + "@metamask/browser-passworder": "^4.3.0", "@metamask/eth-hd-keyring": "^7.0.1", "@metamask/eth-sig-util": "^7.0.0", "@metamask/eth-simple-keyring": "^6.0.1", diff --git a/src/KeyringController.test.ts b/src/KeyringController.test.ts index 667a5f30..fca3a39c 100644 --- a/src/KeyringController.test.ts +++ b/src/KeyringController.test.ts @@ -863,13 +863,11 @@ describe('KeyringController', () => { }); deleteEncryptionKeyAndSalt(keyringController); const initialVault = keyringController.store.getState().vault; - const updatedVaultMock = - '{"vault": "updated_vault_detail", "salt": "salt"}'; const mockEncryptionResult = { data: '0x1234', iv: 'an iv', }; - sinon.stub(mockEncryptor, 'updateVault').resolves(updatedVaultMock); + sinon.stub(mockEncryptor, 'isVaultUpdated').returns(false); sinon .stub(mockEncryptor, 'encryptWithKey') .resolves(mockEncryptionResult); @@ -898,21 +896,12 @@ describe('KeyringController', () => { }, }); const initialVault = keyringController.store.getState().vault; - const updatedVaultMock = - '{"vault": "updated_vault_detail", "salt": "salt"}'; - const mockEncryptionResult = { - data: '0x1234', - iv: 'an iv', - }; - sinon.stub(mockEncryptor, 'updateVault').resolves(updatedVaultMock); - sinon - .stub(mockEncryptor, 'encryptWithKey') - .resolves(mockEncryptionResult); + sinon.stub(mockEncryptor, 'isVaultUpdated').returns(false); await keyringController.unlockKeyrings(PASSWORD); const updatedVault = keyringController.store.getState().vault; - expect(initialVault).not.toBe(updatedVault); + expect(initialVault).toBe(updatedVault); }); }); @@ -929,7 +918,7 @@ describe('KeyringController', () => { const initialVault = keyringController.store.getState().vault; const updatedVaultMock = '{"vault": "updated_vault_detail", "salt": "salt"}'; - sinon.stub(mockEncryptor, 'updateVault').resolves(updatedVaultMock); + sinon.stub(mockEncryptor, 'isVaultUpdated').returns(false); sinon.stub(mockEncryptor, 'encrypt').resolves(updatedVaultMock); await keyringController.unlockKeyrings(PASSWORD); diff --git a/src/KeyringController.ts b/src/KeyringController.ts index 412a67a6..4978feec 100644 --- a/src/KeyringController.ts +++ b/src/KeyringController.ts @@ -931,9 +931,8 @@ class KeyringController extends EventEmitter { if ( this.password && (!this.#cacheEncryptionKey || !encryptionKey) && - this.#encryptor.updateVault && - (await this.#encryptor.updateVault(encryptedVault, this.password)) !== - encryptedVault + this.#encryptor.isVaultUpdated && + !this.#encryptor.isVaultUpdated(encryptedVault) ) { // Re-encrypt the vault with safer method if one is available await this.persistAllKeyrings(); diff --git a/src/test/encryptor.mock.ts b/src/test/encryptor.mock.ts index 0d3a4155..2c97d5ff 100644 --- a/src/test/encryptor.mock.ts +++ b/src/test/encryptor.mock.ts @@ -81,6 +81,10 @@ export class MockEncryptor implements ExportableKeyEncryptor { return _vault; } + isVaultUpdated(_vault: string) { + return true; + } + generateSalt() { return MOCK_ENCRYPTION_SALT; } diff --git a/src/types.ts b/src/types.ts index 67b2385f..d8ed3457 100644 --- a/src/types.ts +++ b/src/types.ts @@ -2,6 +2,7 @@ import type { DetailedDecryptResult, DetailedEncryptionResult, EncryptionResult, + KeyDerivationOptions, } from '@metamask/browser-passworder'; import type { Json, Keyring } from '@metamask/utils'; @@ -68,9 +69,27 @@ export type GenericEncryptor = { * * @param vault - The encrypted string to update. * @param password - The password to decrypt the vault with. + * @param targetDerivationParams - The optional target derivation params to + * use for re-encrypting the vault. * @returns The updated encrypted string. */ - updateVault?: (vault: string, password: string) => Promise; + updateVault?: ( + vault: string, + password: string, + targetDerivationParams?: KeyDerivationOptions, + ) => Promise; + /** + * Optional vault migration helper. Checks if the provided vault is up to date + * with the desired encryption algorithm. + * + * @param vault - The encrypted string to check. + * @param targetDerivationParams - The desired target derivation params. + * @returns The updated encrypted string. + */ + isVaultUpdated?: ( + vault: string, + targetDerivationParams?: KeyDerivationOptions, + ) => boolean; }; /** diff --git a/yarn.lock b/yarn.lock index 3f67aa4a..4f5484bc 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1127,7 +1127,7 @@ __metadata: languageName: node linkType: hard -"@metamask/browser-passworder@npm:^4.2.0": +"@metamask/browser-passworder@npm:^4.3.0": version: 4.3.0 resolution: "@metamask/browser-passworder@npm:4.3.0" dependencies: @@ -1208,7 +1208,7 @@ __metadata: "@lavamoat/allow-scripts": ^2.3.1 "@lavamoat/preinstall-always-fail": ^1.0.0 "@metamask/auto-changelog": ^3.0.0 - "@metamask/browser-passworder": ^4.2.0 + "@metamask/browser-passworder": ^4.3.0 "@metamask/eslint-config": ^12.2.0 "@metamask/eslint-config-jest": ^12.1.0 "@metamask/eslint-config-nodejs": ^12.1.0 From f962ad431f72f8a99d10cadba8dc2ceab4170930 Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Tue, 28 Nov 2023 13:40:32 +0100 Subject: [PATCH 2/2] refactor: remove `updateVault` from `GenericEncryptor` --- src/types.ts | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/src/types.ts b/src/types.ts index d8ed3457..4d8feda1 100644 --- a/src/types.ts +++ b/src/types.ts @@ -63,21 +63,6 @@ export type GenericEncryptor = { * @returns The decrypted object. */ decrypt: (password: string, encryptedString: string) => Promise; - /** - * Optional vault migration helper. Updates the provided vault, re-encrypting - * data with a safer algorithm if one is available. - * - * @param vault - The encrypted string to update. - * @param password - The password to decrypt the vault with. - * @param targetDerivationParams - The optional target derivation params to - * use for re-encrypting the vault. - * @returns The updated encrypted string. - */ - updateVault?: ( - vault: string, - password: string, - targetDerivationParams?: KeyDerivationOptions, - ) => Promise; /** * Optional vault migration helper. Checks if the provided vault is up to date * with the desired encryption algorithm.