From ae522895d51c5761723db3788224cacd4c8df88b Mon Sep 17 00:00:00 2001 From: Aryan Jassal Date: Tue, 10 Sep 2024 13:14:44 +1000 Subject: [PATCH] feat: adding secrets remove handler feat: deleted old secrets delete handlers feat: added tests feat: performed requested changes chore: added more tests for vaultOps.deleteSecret --- src/client/callers/index.ts | 6 +- src/client/callers/vaultsSecretsDelete.ts | 12 -- src/client/callers/vaultsSecretsRemove.ts | 12 ++ ...ecretsDelete.ts => VaultsSecretsRemove.ts} | 23 ++-- src/client/handlers/index.ts | 6 +- src/client/types.ts | 8 ++ src/vaults/VaultOps.ts | 54 ++++---- tests/client/handlers/vaults.test.ts | 130 +++++++++++++++++- tests/vaults/VaultOps.test.ts | 40 ++++-- 9 files changed, 220 insertions(+), 71 deletions(-) delete mode 100644 src/client/callers/vaultsSecretsDelete.ts create mode 100644 src/client/callers/vaultsSecretsRemove.ts rename src/client/handlers/{VaultsSecretsDelete.ts => VaultsSecretsRemove.ts} (69%) diff --git a/src/client/callers/index.ts b/src/client/callers/index.ts index d89931efd..c38418c2c 100644 --- a/src/client/callers/index.ts +++ b/src/client/callers/index.ts @@ -63,7 +63,6 @@ import vaultsPermissionUnset from './vaultsPermissionUnset'; import vaultsPull from './vaultsPull'; import vaultsRename from './vaultsRename'; import vaultsScan from './vaultsScan'; -import vaultsSecretsDelete from './vaultsSecretsDelete'; import vaultsSecretsEdit from './vaultsSecretsEdit'; import vaultsSecretsEnv from './vaultsSecretsEnv'; import vaultsSecretsGet from './vaultsSecretsGet'; @@ -72,6 +71,7 @@ import vaultsSecretsMkdir from './vaultsSecretsMkdir'; import vaultsSecretsNew from './vaultsSecretsNew'; import vaultsSecretsNewDir from './vaultsSecretsNewDir'; import vaultsSecretsRename from './vaultsSecretsRename'; +import vaultsSecretsRemove from './vaultsSecretsRemove'; import vaultsSecretsStat from './vaultsSecretsStat'; import vaultsVersion from './vaultsVersion'; @@ -144,7 +144,6 @@ const clientManifest = { vaultsPull, vaultsRename, vaultsScan, - vaultsSecretsDelete, vaultsSecretsEdit, vaultsSecretsEnv, vaultsSecretsGet, @@ -153,6 +152,7 @@ const clientManifest = { vaultsSecretsNew, vaultsSecretsNewDir, vaultsSecretsRename, + vaultsSecretsRemove, vaultsSecretsStat, vaultsVersion, }; @@ -224,7 +224,6 @@ export { vaultsPull, vaultsRename, vaultsScan, - vaultsSecretsDelete, vaultsSecretsEdit, vaultsSecretsEnv, vaultsSecretsGet, @@ -233,6 +232,7 @@ export { vaultsSecretsNew, vaultsSecretsNewDir, vaultsSecretsRename, + vaultsSecretsRemove, vaultsSecretsStat, vaultsVersion, }; diff --git a/src/client/callers/vaultsSecretsDelete.ts b/src/client/callers/vaultsSecretsDelete.ts deleted file mode 100644 index e6c48f188..000000000 --- a/src/client/callers/vaultsSecretsDelete.ts +++ /dev/null @@ -1,12 +0,0 @@ -import type { HandlerTypes } from '@matrixai/rpc'; -import type VaultsSecretsDelete from '../handlers/VaultsSecretsDelete'; -import { UnaryCaller } from '@matrixai/rpc'; - -type CallerTypes = HandlerTypes; - -const vaultsSecretsDelete = new UnaryCaller< - CallerTypes['input'], - CallerTypes['output'] ->(); - -export default vaultsSecretsDelete; diff --git a/src/client/callers/vaultsSecretsRemove.ts b/src/client/callers/vaultsSecretsRemove.ts new file mode 100644 index 000000000..e546b43ac --- /dev/null +++ b/src/client/callers/vaultsSecretsRemove.ts @@ -0,0 +1,12 @@ +import type { HandlerTypes } from '@matrixai/rpc'; +import type VaultsSecretsRemove from '../handlers/VaultsSecretsRemove'; +import { UnaryCaller } from '@matrixai/rpc'; + +type CallerTypes = HandlerTypes; + +const vaultsSecretsRemove = new UnaryCaller< + CallerTypes['input'], + CallerTypes['output'] +>(); + +export default vaultsSecretsRemove; diff --git a/src/client/handlers/VaultsSecretsDelete.ts b/src/client/handlers/VaultsSecretsRemove.ts similarity index 69% rename from src/client/handlers/VaultsSecretsDelete.ts rename to src/client/handlers/VaultsSecretsRemove.ts index f42872a79..6dc6ca070 100644 --- a/src/client/handlers/VaultsSecretsDelete.ts +++ b/src/client/handlers/VaultsSecretsRemove.ts @@ -2,8 +2,8 @@ import type { DB } from '@matrixai/db'; import type { ClientRPCRequestParams, ClientRPCResponseResult, - SecretIdentifierMessage, SuccessMessage, + SecretRemoveMessage, } from '../types'; import type VaultManager from '../../vaults/VaultManager'; import { UnaryHandler } from '@matrixai/rpc'; @@ -11,16 +11,16 @@ import * as vaultsUtils from '../../vaults/utils'; import * as vaultsErrors from '../../vaults/errors'; import * as vaultOps from '../../vaults/VaultOps'; -class VaultsSecretsDelete extends UnaryHandler< +class VaultsSecretsRemove extends UnaryHandler< { vaultManager: VaultManager; db: DB; }, - ClientRPCRequestParams, + ClientRPCRequestParams, ClientRPCResponseResult > { public handle = async ( - input: ClientRPCRequestParams, + input: ClientRPCRequestParams, ): Promise> => { const { vaultManager, db } = this.container; await db.withTransactionF(async (tran) => { @@ -30,21 +30,20 @@ class VaultsSecretsDelete extends UnaryHandler< ); const vaultId = vaultIdFromName ?? vaultsUtils.decodeVaultId(input.nameOrId); - if (vaultId == null) { - throw new vaultsErrors.ErrorVaultsVaultUndefined(); - } + if (vaultId == null) throw new vaultsErrors.ErrorVaultsVaultUndefined(); await vaultManager.withVaults( [vaultId], async (vault) => { - await vaultOps.deleteSecret(vault, input.secretName); + await vaultOps.deleteSecret(vault, input.secretNames, { + recursive: input.options?.recursive, + }); }, tran, ); }); - return { - success: true, - }; + + return { success: true }; }; } -export default VaultsSecretsDelete; +export default VaultsSecretsRemove; diff --git a/src/client/handlers/index.ts b/src/client/handlers/index.ts index a6aebaceb..43c613a7f 100644 --- a/src/client/handlers/index.ts +++ b/src/client/handlers/index.ts @@ -80,7 +80,6 @@ import VaultsPermissionUnset from './VaultsPermissionUnset'; import VaultsPull from './VaultsPull'; import VaultsRename from './VaultsRename'; import VaultsScan from './VaultsScan'; -import VaultsSecretsDelete from './VaultsSecretsDelete'; import VaultsSecretsEdit from './VaultsSecretsEdit'; import VaultsSecretsEnv from './VaultsSecretsEnv'; import VaultsSecretsGet from './VaultsSecretsGet'; @@ -89,6 +88,7 @@ import VaultsSecretsMkdir from './VaultsSecretsMkdir'; import VaultsSecretsNew from './VaultsSecretsNew'; import VaultsSecretsNewDir from './VaultsSecretsNewDir'; import VaultsSecretsRename from './VaultsSecretsRename'; +import VaultsSecretsRemove from './VaultsSecretsRemove'; import VaultsSecretsStat from './VaultsSecretsStat'; import VaultsVersion from './VaultsVersion'; @@ -184,7 +184,6 @@ const serverManifest = (container: { vaultsPull: new VaultsPull(container), vaultsRename: new VaultsRename(container), vaultsScan: new VaultsScan(container), - vaultsSecretsDelete: new VaultsSecretsDelete(container), vaultsSecretsEdit: new VaultsSecretsEdit(container), vaultsSecretsEnv: new VaultsSecretsEnv(container), vaultsSecretsGet: new VaultsSecretsGet(container), @@ -193,6 +192,7 @@ const serverManifest = (container: { vaultsSecretsNew: new VaultsSecretsNew(container), vaultsSecretsNewDir: new VaultsSecretsNewDir(container), vaultsSecretsRename: new VaultsSecretsRename(container), + VaultsSecretsRemove: new VaultsSecretsRemove(container), vaultsSecretsStat: new VaultsSecretsStat(container), vaultsVersion: new VaultsVersion(container), }; @@ -266,7 +266,6 @@ export { VaultsPull, VaultsRename, VaultsScan, - VaultsSecretsDelete, VaultsSecretsEdit, VaultsSecretsEnv, VaultsSecretsGet, @@ -275,6 +274,7 @@ export { VaultsSecretsNew, VaultsSecretsNewDir, VaultsSecretsRename, + VaultsSecretsRemove, VaultsSecretsStat, VaultsVersion, }; diff --git a/src/client/types.ts b/src/client/types.ts index 0b125b18d..32b2cd55d 100644 --- a/src/client/types.ts +++ b/src/client/types.ts @@ -306,6 +306,13 @@ type SecretPathMessage = { type SecretIdentifierMessage = VaultIdentifierMessage & SecretPathMessage; +type SecretRemoveMessage = VaultIdentifierMessage & { + secretNames: Array; + options?: { + recursive?: boolean; + }; +}; + // Contains binary content as a binary string 'toString('binary')' type ContentMessage = { secretContent: string; @@ -416,6 +423,7 @@ export type { VaultsLatestVersionMessage, SecretPathMessage, SecretIdentifierMessage, + SecretRemoveMessage, ContentMessage, SecretContentMessage, SecretMkdirMessage, 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 b847ee018..4b9b7609d 100644 --- a/tests/client/handlers/vaults.test.ts +++ b/tests/client/handlers/vaults.test.ts @@ -31,7 +31,7 @@ import { VaultsPermissionSet, VaultsPermissionUnset, VaultsRename, - VaultsSecretsDelete, + VaultsSecretsRemove, VaultsSecretsEdit, VaultsSecretsEnv, VaultsSecretsGet, @@ -52,7 +52,7 @@ import { vaultsPermissionSet, vaultsPermissionUnset, vaultsRename, - vaultsSecretsDelete, + vaultsSecretsRemove, vaultsSecretsEdit, vaultsSecretsEnv, vaultsSecretsGet, @@ -1352,8 +1352,10 @@ describe('vaultsSecretsNew and vaultsSecretsDelete, vaultsSecretsGet', () => { let webSocketClient: WebSocketClient; let rpcClient: RPCClient<{ vaultsSecretsNew: typeof vaultsSecretsNew; - vaultsSecretsDelete: typeof vaultsSecretsDelete; + vaultsSecretsRemove: typeof vaultsSecretsRemove; vaultsSecretsGet: typeof vaultsSecretsGet; + vaultsSecretsNewDir: typeof vaultsSecretsNewDir; + vaultsSecretsStat: typeof vaultsSecretsStat; }>; let vaultManager: VaultManager; beforeEach(async () => { @@ -1396,7 +1398,7 @@ describe('vaultsSecretsNew and vaultsSecretsDelete, vaultsSecretsGet', () => { db, vaultManager, }), - vaultsSecretsDelete: new VaultsSecretsDelete({ + vaultsSecretsRemove: new VaultsSecretsRemove({ db, vaultManager, }), @@ -1404,6 +1406,15 @@ describe('vaultsSecretsNew and vaultsSecretsDelete, vaultsSecretsGet', () => { db, vaultManager, }), + vaultsSecretsNewDir: new VaultsSecretsNewDir({ + db, + fs, + vaultManager, + }), + vaultsSecretsStat: new VaultsSecretsStat({ + db, + vaultManager, + }), }, host: localhost, }); @@ -1418,8 +1429,10 @@ describe('vaultsSecretsNew and vaultsSecretsDelete, vaultsSecretsGet', () => { rpcClient = new RPCClient({ manifest: { vaultsSecretsNew, - vaultsSecretsDelete, + vaultsSecretsRemove, vaultsSecretsGet, + vaultsSecretsNewDir, + vaultsSecretsStat, }, streamFactory: () => webSocketClient.connection.newStream(), toError: networkUtils.toError, @@ -1456,9 +1469,9 @@ describe('vaultsSecretsNew and vaultsSecretsDelete, vaultsSecretsGet', () => { const secretContent = getResponse1.secretContent; expect(secretContent).toStrictEqual(secret); // Delete secret - const deleteResponse = await rpcClient.methods.vaultsSecretsDelete({ + const deleteResponse = await rpcClient.methods.vaultsSecretsRemove({ nameOrId: vaultIdEncoded, - secretName: secret, + secretNames: [secret], }); expect(deleteResponse.success).toBeTruthy(); // Check secret was deleted @@ -1470,6 +1483,109 @@ describe('vaultsSecretsNew and vaultsSecretsDelete, vaultsSecretsGet', () => { vaultsErrors.ErrorSecretsSecretUndefined, ); }); + test('deletes multiple secrets', async () => { + // Create secret + const secret1 = 'test-secret1'; + const secret2 = 'test-secret2'; + const vaultId = await vaultManager.createVault('test-vault'); + const vaultIdEncoded = vaultsUtils.encodeVaultId(vaultId); + await rpcClient.methods.vaultsSecretsNew({ + nameOrId: vaultIdEncoded, + secretName: secret1, + secretContent: Buffer.from(secret1).toString('binary'), + }); + await rpcClient.methods.vaultsSecretsNew({ + nameOrId: vaultIdEncoded, + secretName: secret2, + secretContent: Buffer.from(secret2).toString('binary'), + }); + // Get secret + const getResponse1 = await rpcClient.methods.vaultsSecretsGet({ + nameOrId: vaultIdEncoded, + secretName: secret1, + }); + const getResponse2 = await rpcClient.methods.vaultsSecretsGet({ + nameOrId: vaultIdEncoded, + secretName: secret2, + }); + expect(getResponse1.secretContent).toStrictEqual(secret1); + expect(getResponse2.secretContent).toStrictEqual(secret2); + // Delete secret + const deleteResponse = await rpcClient.methods.vaultsSecretsRemove({ + nameOrId: vaultIdEncoded, + secretNames: [secret1, secret2], + }); + expect(deleteResponse.success).toBeTruthy(); + // Check secret was deleted + await testsUtils.expectRemoteError( + rpcClient.methods.vaultsSecretsGet({ + nameOrId: vaultIdEncoded, + secretName: secret1, + }), + vaultsErrors.ErrorSecretsSecretUndefined, + ); + await testsUtils.expectRemoteError( + rpcClient.methods.vaultsSecretsGet({ + nameOrId: vaultIdEncoded, + secretName: secret2, + }), + vaultsErrors.ErrorSecretsSecretUndefined, + ); + }); + test('deletes directory recursively', async () => { + // Create secrets + const vaultName = 'test-vault'; + const secretDirName = 'secretDir'; + const secretList = ['test-secret1', 'test-secret2', 'test-secret3']; + 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); + // Write secret to file + await fs.promises.writeFile(secretFile, secret); + } + const vaultId = await vaultManager.createVault(vaultName); + const vaultsIdEncoded = vaultsUtils.encodeVaultId(vaultId); + const addResponse = await rpcClient.methods.vaultsSecretsNewDir({ + nameOrId: vaultsIdEncoded, + dirName: secretDir, + }); + expect(addResponse.success).toBeTruthy(); + // Delete secret + await testsUtils.expectRemoteError( + rpcClient.methods.vaultsSecretsRemove({ + nameOrId: vaultsIdEncoded, + secretNames: [secretDirName], + }), + vaultsErrors.ErrorVaultsRecursive, + ); + const deleteResponse = await rpcClient.methods.vaultsSecretsRemove({ + nameOrId: vaultsIdEncoded, + 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', () => { const logger = new Logger('vaultsSecretsNewDirList test', LogLevel.WARN, [ diff --git a/tests/vaults/VaultOps.test.ts b/tests/vaults/VaultOps.test.ts index 0d04713e8..8c1f5da32 100644 --- a/tests/vaults/VaultOps.test.ts +++ b/tests/vaults/VaultOps.test.ts @@ -265,39 +265,61 @@ 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, ); }); + test('deleting multiple secrets', async () => { + const secretNames = ['secret1', 'secret2', 'secret3']; + for (const secretName of secretNames) { + await writeSecret(secretName, secretName); + } + await vaultOps.deleteSecret(vault, secretNames); + for (const secretName of secretNames) { + await expectSecretNot(secretName); + } + }); + test('deleting multiple secrets should add only one new log message', async () => { + const secretNames = ['secret1', 'secret2', 'secret3']; + for (const secretName of secretNames) { + await writeSecret(secretName, secretName); + } + const logLength = (await vault.log()).length; + await vaultOps.deleteSecret(vault, secretNames); + for (const secretName of secretNames) { + await expectSecretNot(secretName); + } + expect((await vault.log()).length).toBe(logLength + 1); + }); }); describe('mkdir', () => { test('can create directory', async () => { @@ -521,8 +543,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()); },