make changePassword a no-op for same password

Author: matthiasgeihs

packages/keyring-controller/src/KeyringController.test.ts

@@ -3332,6 +3332,14 @@ describe('KeyringController', () => {
       );
     });
 
+    it('should throw error if encryptionKey is not set', async () => {
+      await withController(async ({ controller }) => {
+        await expect(controller.exportEncryptionKey()).rejects.toThrow(
+          KeyringControllerError.EncryptionKeyNotSet,
+        );
+      });
+    });
+
     it('should export key after password change', async () => {
       await withController(
         { cacheEncryptionKey: true },

packages/keyring-controller/src/KeyringController.ts

@@ -543,6 +543,20 @@ function assertIsValidPassword(password: unknown): asserts password is string {
   }
 }
 
+/**
+ * Assert that the provided encryption key is a valid non-empty string.
+ *
+ * @param encryptionKey - The encryption key to check.
+ * @throws If the encryption key is not a valid string.
+ */
+function assertIsEncryptionKeySet(
+  encryptionKey: string | undefined,
+): asserts encryptionKey is string {
+  if (!encryptionKey) {
+    throw new Error(KeyringControllerError.EncryptionKeyNotSet);
+  }
+}
+
 /**
  * Checks if the provided value is a serialized keyrings array.
  *
@@ -1417,6 +1431,11 @@ export class KeyringController extends BaseController<
   changePassword(password: string): Promise<void> {
     this.#assertIsUnlocked();
 
+    // If the password is the same, do nothing.
+    if (this.#password === password) {
+      return Promise.resolve();
+    }
+
     return this.#persistOrRollback(async () => {
       assertIsValidPassword(password);
 
@@ -1480,32 +1499,10 @@ export class KeyringController extends BaseController<
     this.#assertIsUnlocked();
 
     return await this.#withControllerLock(async () => {
-      // There is a case where the controller is unlocked but the encryption key
-      // is not set, even when #cacheEncryptionKey is true. This happens when
-      // calling changePassword with the existing password. In this case, the
-      // encryption key is deleted, but the state is not recreated, because the
-      // session state does not change in this case, and #updateVault is not
-      // called in #persistOrRollback.
-      if (!this.state.encryptionKey) {
-        return await this.#withVaultLock(async () => {
-          assertIsExportableKeyEncryptor(this.#encryptor);
-          assertIsValidPassword(this.#password);
-          const result = await this.#encryptor.decryptWithDetail(
-            this.#password,
-            // Ignoring undefined. Assuming vault is set when unlocked.
-            this.state.vault as string,
-          );
-          if (this.#cacheEncryptionKey) {
-            this.update((state) => {
-              state.encryptionKey = result.exportedKeyString;
-              state.encryptionSalt = result.salt;
-            });
-          }
-          return result.exportedKeyString;
-        });
-      }
+      const { encryptionKey } = this.state;
+      assertIsEncryptionKeySet(encryptionKey);
 
-      return this.state.encryptionKey;
+      return encryptionKey;
     });
   }
 

packages/keyring-controller/src/constants.ts

@@ -36,4 +36,5 @@ export enum KeyringControllerError {
   NoHdKeyring = 'KeyringController - No HD Keyring found',
   ControllerLockRequired = 'KeyringController - attempt to update vault during a non mutually exclusive operation',
   LastAccountInPrimaryKeyring = 'KeyringController - Last account in primary keyring cannot be removed',
+  EncryptionKeyNotSet = 'KeyringController - Encryption key not set',
 }