From f8a30c64f669a39ae408718b66781f39ae66cd7d Mon Sep 17 00:00:00 2001 From: Ariel Gentile Date: Thu, 28 Sep 2023 13:14:17 -0300 Subject: [PATCH] fix(askar): throw error if imported wallet exists Signed-off-by: Ariel Gentile --- packages/askar/src/wallet/AskarWallet.ts | 47 +++++++++----- packages/askar/tests/askar-sqlite.e2e.test.ts | 65 +++++++++++++++++++ .../error/WalletImportPathExistsError.ts | 7 ++ packages/core/src/wallet/error/index.ts | 1 + 4 files changed, 105 insertions(+), 15 deletions(-) create mode 100644 packages/core/src/wallet/error/WalletImportPathExistsError.ts diff --git a/packages/askar/src/wallet/AskarWallet.ts b/packages/askar/src/wallet/AskarWallet.ts index 7a46016027..e69a318fd0 100644 --- a/packages/askar/src/wallet/AskarWallet.ts +++ b/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( diff --git a/packages/askar/tests/askar-sqlite.e2e.test.ts b/packages/askar/tests/askar-sqlite.e2e.test.ts index 41280f0684..e52b07fcce 100644 --- a/packages/askar/tests/askar-sqlite.e2e.test.ts +++ b/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', diff --git a/packages/core/src/wallet/error/WalletImportPathExistsError.ts b/packages/core/src/wallet/error/WalletImportPathExistsError.ts new file mode 100644 index 0000000000..32d9b46d67 --- /dev/null +++ b/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 }) + } +} diff --git a/packages/core/src/wallet/error/index.ts b/packages/core/src/wallet/error/index.ts index 92040216f4..0f9c04b4dd 100644 --- a/packages/core/src/wallet/error/index.ts +++ b/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'