From 0c9a476f38066dc9c63327bec5d00ad3efc0379b Mon Sep 17 00:00:00 2001 From: Aryan Jassal Date: Tue, 10 Sep 2024 16:08:21 +1000 Subject: [PATCH] feat: performed requested changes --- src/client/handlers/VaultsSecretsRemove.ts | 8 ++-- src/client/types.ts | 15 ++---- src/vaults/VaultOps.ts | 54 +++++++++++----------- tests/client/handlers/vaults.test.ts | 33 +++++++++++-- tests/vaults/VaultOps.test.ts | 18 ++++---- 5 files changed, 75 insertions(+), 53 deletions(-) diff --git a/src/client/handlers/VaultsSecretsRemove.ts b/src/client/handlers/VaultsSecretsRemove.ts index 705f22ac3..6dc6ca070 100644 --- a/src/client/handlers/VaultsSecretsRemove.ts +++ b/src/client/handlers/VaultsSecretsRemove.ts @@ -34,11 +34,9 @@ class VaultsSecretsRemove extends UnaryHandler< await vaultManager.withVaults( [vaultId], async (vault) => { - for (const secretName of input.secretNames) { - await vaultOps.deleteSecret(vault, secretName, { - recursive: input.options?.recursive, - }); - } + await vaultOps.deleteSecret(vault, input.secretNames, { + recursive: input.options?.recursive, + }); }, tran, ); diff --git a/src/client/types.ts b/src/client/types.ts index 2b3c913ec..32b2cd55d 100644 --- a/src/client/types.ts +++ b/src/client/types.ts @@ -304,18 +304,14 @@ type SecretPathMessage = { secretName: string; }; -type SecretPathsMessage = { - secretNames: Array; -}; - type SecretIdentifierMessage = VaultIdentifierMessage & SecretPathMessage; -type SecretRemoveMessage = VaultIdentifierMessage & - SecretPathsMessage & { - options?: { - recursive?: boolean; - }; +type SecretRemoveMessage = VaultIdentifierMessage & { + secretNames: Array; + options?: { + recursive?: boolean; }; +}; // Contains binary content as a binary string 'toString('binary')' type ContentMessage = { @@ -426,7 +422,6 @@ export type { VaultsVersionMessage, VaultsLatestVersionMessage, SecretPathMessage, - SecretPathsMessage, SecretIdentifierMessage, SecretRemoveMessage, ContentMessage, diff --git a/src/vaults/VaultOps.ts b/src/vaults/VaultOps.ts index abd931429..05cdaca8a 100644 --- a/src/vaults/VaultOps.ts +++ b/src/vaults/VaultOps.ts @@ -129,37 +129,39 @@ async function statSecret(vault: Vault, secretName: string): Promise { */ async function deleteSecret( vault: Vault, - secretName: string, + secretNames: Array, fileOptions?: FileOptions, logger?: Logger, ): Promise { - try { - await vault.writeF(async (efs) => { - const stat = await efs.stat(secretName); - if (stat.isDirectory()) { - await efs.rmdir(secretName, fileOptions); - logger?.info(`Deleted directory at '${secretName}'`); - } else { - // Remove the specified file - await efs.unlink(secretName); - logger?.info(`Deleted secret at '${secretName}'`); + await vault.writeF(async (efs) => { + for (const secretName of secretNames) { + try { + const stat = await efs.stat(secretName); + if (stat.isDirectory()) { + await efs.rmdir(secretName, fileOptions); + logger?.info(`Deleted directory at '${secretName}'`); + } else { + // Remove the specified file + await efs.unlink(secretName); + logger?.info(`Deleted secret at '${secretName}'`); + } + } catch (e) { + if (e.code === 'ENOENT') { + throw new vaultsErrors.ErrorSecretsSecretUndefined( + `Secret with name: ${secretName} does not exist`, + { cause: e }, + ); + } + if (e.code === 'ENOTEMPTY') { + throw new vaultsErrors.ErrorVaultsRecursive( + `Could not delete directory '${secretName}' without recursive option`, + { cause: e }, + ); + } + throw e; } - }); - } catch (e) { - if (e.code === 'ENOENT') { - throw new vaultsErrors.ErrorSecretsSecretUndefined( - `Secret with name: ${secretName} does not exist`, - { cause: e }, - ); - } - if (e.code === 'ENOTEMPTY') { - throw new vaultsErrors.ErrorVaultsRecursive( - `Could not delete directory '${secretName}' without recursive option`, - { cause: e }, - ); } - throw e; - } + }); } /** diff --git a/tests/client/handlers/vaults.test.ts b/tests/client/handlers/vaults.test.ts index e2bb47e3a..4b9b7609d 100644 --- a/tests/client/handlers/vaults.test.ts +++ b/tests/client/handlers/vaults.test.ts @@ -1355,6 +1355,7 @@ describe('vaultsSecretsNew and vaultsSecretsDelete, vaultsSecretsGet', () => { vaultsSecretsRemove: typeof vaultsSecretsRemove; vaultsSecretsGet: typeof vaultsSecretsGet; vaultsSecretsNewDir: typeof vaultsSecretsNewDir; + vaultsSecretsStat: typeof vaultsSecretsStat; }>; let vaultManager: VaultManager; beforeEach(async () => { @@ -1410,6 +1411,10 @@ describe('vaultsSecretsNew and vaultsSecretsDelete, vaultsSecretsGet', () => { fs, vaultManager, }), + vaultsSecretsStat: new VaultsSecretsStat({ + db, + vaultManager, + }), }, host: localhost, }); @@ -1427,6 +1432,7 @@ describe('vaultsSecretsNew and vaultsSecretsDelete, vaultsSecretsGet', () => { vaultsSecretsRemove, vaultsSecretsGet, vaultsSecretsNewDir, + vaultsSecretsStat, }, streamFactory: () => webSocketClient.connection.newStream(), toError: networkUtils.toError, @@ -1527,10 +1533,12 @@ describe('vaultsSecretsNew and vaultsSecretsDelete, vaultsSecretsGet', () => { ); }); test('deletes directory recursively', async () => { - // Create secret + // Create secrets const vaultName = 'test-vault'; + const secretDirName = 'secretDir'; const secretList = ['test-secret1', 'test-secret2', 'test-secret3']; - const secretDir = path.join(dataDir, 'secretDir'); + const secretDir = path.join(dataDir, secretDirName); + const secretNames = secretList.map((v) => path.join(secretDirName, v)); await fs.promises.mkdir(secretDir); for (const secret of secretList) { const secretFile = path.join(secretDir, secret); @@ -1548,18 +1556,35 @@ describe('vaultsSecretsNew and vaultsSecretsDelete, vaultsSecretsGet', () => { await testsUtils.expectRemoteError( rpcClient.methods.vaultsSecretsRemove({ nameOrId: vaultsIdEncoded, - secretNames: ['secretDir'], + secretNames: [secretDirName], }), vaultsErrors.ErrorVaultsRecursive, ); const deleteResponse = await rpcClient.methods.vaultsSecretsRemove({ nameOrId: vaultsIdEncoded, - secretNames: ['secretDir'], + secretNames: [secretDirName], options: { recursive: true, }, }); expect(deleteResponse.success).toBeTruthy(); + // Check secret was deleted + for (const secretName of secretNames) { + await testsUtils.expectRemoteError( + rpcClient.methods.vaultsSecretsGet({ + nameOrId: vaultsIdEncoded, + secretName: secretName, + }), + vaultsErrors.ErrorSecretsSecretUndefined, + ); + } + await testsUtils.expectRemoteError( + rpcClient.methods.vaultsSecretsStat({ + nameOrId: vaultsIdEncoded, + secretName: secretDirName, + }), + vaultsErrors.ErrorSecretsSecretUndefined, + ); }); }); describe('vaultsSecretsNewDir and vaultsSecretsList', () => { diff --git a/tests/vaults/VaultOps.test.ts b/tests/vaults/VaultOps.test.ts index 0d04713e8..fe22ca4f5 100644 --- a/tests/vaults/VaultOps.test.ts +++ b/tests/vaults/VaultOps.test.ts @@ -265,36 +265,36 @@ describe('VaultOps', () => { describe('deleteSecret', () => { test('deleting a secret', async () => { await writeSecret(secretName, secretContent); - await vaultOps.deleteSecret(vault, secretName); + await vaultOps.deleteSecret(vault, [secretName]); await expectSecretNot(secretName); }); test('deleting a secret in a directory', async () => { const secretPath = path.join(dirName, secretName); await writeSecret(secretPath, secretContent); - await vaultOps.deleteSecret(vault, secretPath); + await vaultOps.deleteSecret(vault, [secretPath]); await expectSecretNot(secretPath); await expectDirExists(dirName); }); test('deleting a directory', async () => { await mkdir(dirName); - await vaultOps.deleteSecret(vault, dirName); + await vaultOps.deleteSecret(vault, [dirName]); await expectDirExistsNot(dirName); }); test('deleting a directory with a file should fail', async () => { const secretPath = path.join(dirName, secretName); await writeSecret(secretPath, secretContent); - await expect(vaultOps.deleteSecret(vault, dirName)).rejects.toThrow( + await expect(vaultOps.deleteSecret(vault, [dirName])).rejects.toThrow( vaultsErrors.ErrorVaultsRecursive, ); }); test('deleting a directory with force', async () => { const secretPath = path.join(dirName, secretName); await writeSecret(secretPath, secretContent); - await vaultOps.deleteSecret(vault, dirName, { recursive: true }); + await vaultOps.deleteSecret(vault, [dirName], { recursive: true }); await expectDirExistsNot(dirName); }); test('deleting a secret that does not exist should fail', async () => { - await expect(vaultOps.deleteSecret(vault, secretName)).rejects.toThrow( + await expect(vaultOps.deleteSecret(vault, [secretName])).rejects.toThrow( vaultsErrors.ErrorSecretsSecretUndefined, ); }); @@ -521,8 +521,10 @@ describe('VaultOps', () => { await vaultOps.getSecret(vault, '.hidingDir/.hiddenInSecret') ).toString(), ).toStrictEqual('change_inside'); - await vaultOps.deleteSecret(vault, '.hidingSecret', { recursive: true }); - await vaultOps.deleteSecret(vault, '.hidingDir', { recursive: true }); + await vaultOps.deleteSecret(vault, ['.hidingSecret'], { + recursive: true, + }); + await vaultOps.deleteSecret(vault, ['.hidingDir'], { recursive: true }); list = await vaultOps.listSecrets(vault); expect(list.sort()).toStrictEqual([].sort()); },