Refactor Argon migration function

- return `success` boolean - make sure decrypted vaults match - send event on migration fail
tahowallet · Jun 30, 2023 · a569168 · a569168
1 parent ac79ba4
commit a569168
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 10 deletions.
diff --git a/background/lib/posthog.ts b/background/lib/posthog.ts
@@ -18,7 +18,8 @@ export enum OneTimeAnalyticsEvent {
   ONBOARDING_STARTED = "Onboarding Started",
   ONBOARDING_FINISHED = "Onboarding Finished",
   CHAIN_ADDED = "Chain Added",
-  ARGON_MIGRATION = "Vaults migrated to Argon2",
+  ARGON_MIGRATION = "Migrate to Argon2",
+  ARGON_MIGRATION_FAILED = "Migrate to Argon2 failed",
 }
 
 export const isOneTimeAnalyticsEvent = (

diff --git a/background/services/internal-signer/index.ts b/background/services/internal-signer/index.ts
@@ -277,13 +277,20 @@ export default class InternalSignerService extends BaseService<Events> {
       return true
     }
 
-    const { vaults, version } = await migrateVaultsToArgon(password)
+    const {
+      encryptedData: { vaults, version },
+      success,
+    } = await migrateVaultsToArgon(password)
     this.#cachedVaultVersion = version
 
-    if (version === VaultVersion.Argon2) {
+    if (success) {
       this.analyticsService.sendOneTimeAnalyticsEvent(
         OneTimeAnalyticsEvent.ARGON_MIGRATION
       )
+    } else {
+      this.analyticsService.sendOneTimeAnalyticsEvent(
+        OneTimeAnalyticsEvent.ARGON_MIGRATION_FAILED
+      )
     }
 
     if (!ignoreExistingVaults) {

diff --git a/background/services/internal-signer/storage.ts b/background/services/internal-signer/storage.ts
@@ -98,10 +98,10 @@ export async function writeLatestEncryptedVault(
 
 export async function migrateVaultsToArgon(
   password: string
-): Promise<SerializedEncryptedVaults> {
+): Promise<{ encryptedData: SerializedEncryptedVaults; success: boolean }> {
   const serializedVaults = await getEncryptedVaults()
   if (serializedVaults.version === VaultVersion.Argon2) {
-    return serializedVaults
+    return { encryptedData: serializedVaults, success: true }
   }
 
   const { vaults } = serializedVaults
@@ -128,6 +128,22 @@ export async function migrateVaultsToArgon(
           passwordOrSaltedKey: newSaltedKey,
         })
 
+        // try to decrypt the new vault to make sure it's valid
+        const newDecryptedVault = await decryptVault({
+          version: VaultVersion.Argon2,
+          vault: newEncryptedVault,
+          passwordOrSaltedKey: newSaltedKey,
+        })
+
+        if (
+          JSON.stringify(newDecryptedVault) !==
+          JSON.stringify(deprecatedDecryptedVault)
+        ) {
+          throw new Error(
+            "Failed to migrate vaults to Argon2. Decrypted vaults do not match."
+          )
+        }
+
         return {
           timeSaved,
           vault: newEncryptedVault,
@@ -144,9 +160,9 @@ export async function migrateVaultsToArgon(
       tallyVaults: newSerializedVaults,
     })
 
-    return newSerializedVaults
+    return { encryptedData: newSerializedVaults, success: true }
   } catch (error) {
     logger.error("Failed to migrate vaults to Argon2")
-    return serializedVaults
+    return { encryptedData: serializedVaults, success: false }
   }
 }
diff --git a/background/services/internal-signer/tests/storage.unit.test.ts b/background/services/internal-signer/tests/storage.unit.test.ts
@@ -63,7 +63,7 @@ describe("Storage utils", () => {
     expect(vaults[0].vault).toEqual(vaultEncryptedWithArgon2)
   })
 
-  it("shoould migrate existing vaults to Argon2", async () => {
+  it("should migrate existing vaults to Argon2", async () => {
     await browser.storage.local.set({
       tallyVaults: {
         version: VaultVersion.PBKDF2,
@@ -72,8 +72,12 @@ describe("Storage utils", () => {
     })
     await writeLatestEncryptedVault(vaultEncryptedWithPBKDF2)
 
-    const { vaults, version } = await migrateVaultsToArgon(mockedPassword)
+    const {
+      encryptedData: { vaults, version },
+      success,
+    } = await migrateVaultsToArgon(mockedPassword)
 
+    expect(success).toEqual(true)
     expect(version).toEqual(VaultVersion.Argon2)
     expect(vaults.length).toEqual(1)
 
@@ -94,8 +98,13 @@ describe("Storage utils", () => {
     })
     await writeLatestEncryptedVault(vaultEncryptedWithArgon2)
 
-    const { vaults } = await migrateVaultsToArgon(mockedPassword)
+    const {
+      encryptedData: { vaults, version },
+      success,
+    } = await migrateVaultsToArgon(mockedPassword)
 
+    expect(success).toEqual(true)
+    expect(version).toEqual(VaultVersion.Argon2)
     expect(vaults[0].vault).toEqual(vaultEncryptedWithArgon2)
   })
 })