Use encryptor isVaultUpdated (#310)

* chore: update browser-passworder * refactor: remove `updateVault` from `GenericEncryptor`
MetaMask · Nov 29, 2023 · 9388ab6 · 9388ab6
1 parent 546af14
commit 9388ab6
Show file tree

Hide file tree

Showing 6 changed files with 25 additions and 29 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -2,6 +2,7 @@ import type {
   DetailedDecryptResult,
   DetailedEncryptionResult,
   EncryptionResult,
+  KeyDerivationOptions,
 } from '@metamask/browser-passworder';
 import type { Json, Keyring } from '@metamask/utils';
 
@@ -63,14 +64,17 @@ export type GenericEncryptor = {
    */
   decrypt: (password: string, encryptedString: string) => Promise<unknown>;
   /**
-   * Optional vault migration helper. Updates the provided vault, re-encrypting
-   * data with a safer algorithm if one is available.
+   * Optional vault migration helper. Checks if the provided vault is up to date
+   * with the desired encryption algorithm.
    *
-   * @param vault - The encrypted string to update.
-   * @param password - The password to decrypt the vault with.
+   * @param vault - The encrypted string to check.
+   * @param targetDerivationParams - The desired target derivation params.
    * @returns The updated encrypted string.
    */
-  updateVault?: (vault: string, password: string) => Promise<string>;
+  isVaultUpdated?: (
+    vault: string,
+    targetDerivationParams?: KeyDerivationOptions,
+  ) => boolean;
 };
 
 /**

diff --git a/yarn.lock b/yarn.lock
@@ -1127,12 +1127,12 @@ __metadata:
   languageName: node
   linkType: hard
 
-"@metamask/browser-passworder@npm:^4.2.0":
-  version: 4.2.0
-  resolution: "@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:
     "@metamask/utils": ^8.2.0
-  checksum: 03b76357942d25a6316d6a03a8bc839cb18e53d9f95fc2787e0fbbcf13eeb2485ece47a2758e928d04635f8dbaa598794f2ecd0313e7c91f989bf11f2a0adec5
+  checksum: 7992553a0cd91902aa316a931d36c2628cb5a73fcbc28a31dce4177704036d739214c580ed833079003b2c7dfd60c5648a20734badf91e2c665cfe2f56012a8c
   languageName: node
   linkType: hard
 
@@ -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