fix(askar): throw error if imported wallet exists (#1593)

Signed-off-by: Ariel Gentile <gentilester@gmail.com>

packages/askar/src/wallet/AskarWallet.ts

@@ -12,6 +12,7 @@ import {
   FileSystem,
   WalletNotFoundError,
   KeyDerivationMethod,
+  WalletImportPathExistsError,
 } from '@aries-framework/core'
 // eslint-disable-next-line import/order
 import { Store } from '@hyperledger/aries-askar-shared'
@@ -111,6 +112,11 @@ export class AskarWallet extends AskarBaseWallet {
       })
     }
     try {
+      // Make sure path exists before creating the wallet
+      if (filePath) {
+        await this.fileSystem.createDirectory(filePath)
+      }
+
       this._store = await Store.provision({
         recreate: false,
         uri: askarWalletConfig.uri,
@@ -280,7 +286,12 @@ export class AskarWallet extends AskarBaseWallet {
     }
 
     try {
-      // This method ensures that destination directory is created
+      // Export path already exists
+      if (await this.fileSystem.exists(destinationPath)) {
+        throw new WalletExportPathExistsError(
+          `Unable to create export, wallet export at path '${exportConfig.path}' already exists`
+        )
+      }
       const exportedWalletConfig = await this.getAskarWalletConfig({
         ...this.walletConfig,
         storage: { type: 'sqlite', path: destinationPath },
@@ -289,12 +300,8 @@ export class AskarWallet extends AskarBaseWallet {
       // Close this wallet before copying
       await this.close()
 
-      // Export path already exists
-      if (await this.fileSystem.exists(destinationPath)) {
-        throw new WalletExportPathExistsError(
-          `Unable to create export, wallet export at path '${exportConfig.path}' already exists`
-        )
-      }
+      // Make sure destination path exists
+      await this.fileSystem.createDirectory(destinationPath)
 
       // Copy wallet to the destination path
       await this.fileSystem.copyFile(sourcePath, destinationPath)
@@ -311,14 +318,14 @@ export class AskarWallet extends AskarBaseWallet {
 
       await this._open(this.walletConfig)
     } catch (error) {
-      if (error instanceof WalletExportPathExistsError) throw error
-
       const errorMessage = `Error exporting wallet '${this.walletConfig.id}': ${error.message}`
       this.logger.error(errorMessage, {
         error,
         errorMessage: error.message,
       })
 
+      if (error instanceof WalletExportPathExistsError) throw error
+
       throw new WalletError(errorMessage, { cause: error })
     }
   }
@@ -332,9 +339,16 @@ export class AskarWallet extends AskarBaseWallet {
     }
 
     try {
-      // This method ensures that destination directory is created
       const importWalletConfig = await this.getAskarWalletConfig(walletConfig)
 
+      // Import path already exists
+      if (await this.fileSystem.exists(destinationPath)) {
+        throw new WalletExportPathExistsError(`Unable to import wallet. Path '${importConfig.path}' already exists`)
+      }
+
+      // Make sure destination path exists
+      await this.fileSystem.createDirectory(destinationPath)
+
       // Copy wallet to the destination path
       await this.fileSystem.copyFile(sourcePath, destinationPath)
 
@@ -355,6 +369,13 @@ export class AskarWallet extends AskarBaseWallet {
         errorMessage: error.message,
       })
 
+      if (error instanceof WalletImportPathExistsError) throw error
+
+      // Cleanup any wallet file we could have created
+      if (await this.fileSystem.exists(destinationPath)) {
+        await this.fileSystem.delete(destinationPath)
+      }
+
       throw new WalletError(errorMessage, { cause: error })
     }
   }
@@ -387,13 +408,9 @@ export class AskarWallet extends AskarBaseWallet {
   private async getAskarWalletConfig(walletConfig: WalletConfig) {
     const { uri, path } = uriFromWalletConfig(walletConfig, this.fileSystem.dataPath)
 
-    // Make sure path exists before creating the wallet
-    if (path) {
-      await this.fileSystem.createDirectory(path)
-    }
-
     return {
       uri,
+      path,
       profile: walletConfig.id,
       // FIXME: Default derivation method should be set somewhere in either agent config or some constants
       keyMethod: keyDerivationMethodToStoreKeyMethod(

packages/askar/tests/askar-sqlite.e2e.test.ts

@@ -147,6 +147,71 @@ describeRunInNodeVersion([18], 'Askar SQLite agents', () => {
     })
   })
 
+  test('throws error when attempting to export and import to existing paths', async () => {
+    await bobAgent.initialize()
+
+    if (!bobAgent.config.walletConfig) {
+      throw new Error('No wallet config on bobAgent')
+    }
+
+    const backupKey = 'someBackupKey'
+    const backupWalletName = `backup-${utils.uuid()}`
+    const backupPath = path.join(tmpdir(), backupWalletName)
+
+    // Create backup and try to export it again to the same path
+    await bobAgent.wallet.export({ path: backupPath, key: backupKey })
+    await expect(async () => await bobAgent.wallet.export({ path: backupPath, key: backupKey })).rejects.toThrowError(
+      /Unable to create export/
+    )
+
+    await bobAgent.wallet.delete()
+
+    // Import backup with different wallet id and initialize
+    await bobAgent.wallet.import({ id: backupWalletName, key: backupWalletName }, { path: backupPath, key: backupKey })
+    await bobAgent.wallet.initialize({ id: backupWalletName, key: backupWalletName })
+    await bobAgent.wallet.close()
+
+    // Try to import again an existing wallet
+    await expect(
+      async () =>
+        await bobAgent.wallet.import(
+          { id: backupWalletName, key: backupWalletName },
+          { path: backupPath, key: backupKey }
+        )
+    ).rejects.toThrowError(/Unable to import wallet/)
+  })
+
+  test('throws error when attempting to import using wrong key', async () => {
+    await bobAgent.initialize()
+
+    if (!bobAgent.config.walletConfig) {
+      throw new Error('No wallet config on bobAgent')
+    }
+
+    const backupKey = 'someBackupKey'
+    const wrongBackupKey = 'wrongBackupKey'
+    const backupWalletName = `backup-${utils.uuid()}`
+    const backupPath = path.join(tmpdir(), backupWalletName)
+
+    // Create backup and try to export it again to the same path
+    await bobAgent.wallet.export({ path: backupPath, key: backupKey })
+    await bobAgent.wallet.delete()
+
+    // Try to import backup with wrong key
+    await expect(
+      async () =>
+        await bobAgent.wallet.import(
+          { id: backupWalletName, key: backupWalletName },
+          { path: backupPath, key: wrongBackupKey }
+        )
+    ).rejects.toThrow()
+
+    // Try to import again using the correct key
+    await bobAgent.wallet.import({ id: backupWalletName, key: backupWalletName }, { path: backupPath, key: backupKey })
+    await bobAgent.wallet.initialize({ id: backupWalletName, key: backupWalletName })
+    await bobAgent.wallet.close()
+  })
+
   test('changing wallet key', async () => {
     const walletConfig = {
       id: 'mywallet',

packages/core/src/wallet/error/WalletImportPathExistsError.ts

@@ -0,0 +1,7 @@
+import { WalletError } from './WalletError'
+
+export class WalletImportPathExistsError extends WalletError {
+  public constructor(message: string, { cause }: { cause?: Error } = {}) {
+    super(message, { cause })
+  }
+}

packages/core/src/wallet/error/index.ts

@@ -3,4 +3,5 @@ export { WalletNotFoundError } from './WalletNotFoundError'
 export { WalletInvalidKeyError } from './WalletInvalidKeyError'
 export { WalletError } from './WalletError'
 export { WalletKeyExistsError } from './WalletKeyExistsError'
+export { WalletImportPathExistsError } from './WalletImportPathExistsError'
 export { WalletExportPathExistsError } from './WalletExportPathExistsError'