Skip to content

Commit

Permalink
chore: updated secrets remove to properly operate on multiple paths
Browse files Browse the repository at this point in the history
  • Loading branch information
@aryanjassal
aryanjassal committed Nov 1, 2024
1 parent 1031db4 commit cadc962
Show file tree
Hide file tree
Showing 6 changed files with 178 additions and 121 deletions.
4 changes: 2 additions & 2 deletions src/client/callers/vaultsSecretsRemove.ts
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import type { HandlerTypes } from '@matrixai/rpc';
import type VaultsSecretsRemove from '../handlers/VaultsSecretsRemove';
import { ClientCaller } from '@matrixai/rpc';
import { DuplexCaller } from '@matrixai/rpc';

type CallerTypes = HandlerTypes<VaultsSecretsRemove>;

const vaultsSecretsRemove = new ClientCaller<
const vaultsSecretsRemove = new DuplexCaller<
CallerTypes['input'],
CallerTypes['output']
>();
Expand Down
80 changes: 54 additions & 26 deletions src/client/handlers/VaultsSecretsRemove.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,25 @@ import type { DB } from '@matrixai/db';
import type {
ClientRPCRequestParams,
ClientRPCResponseResult,
SuccessMessage,
SecretIdentifierMessage,
SuccessOrErrorMessage,
} from '../types';
import type VaultManager from '../../vaults/VaultManager';
import { ClientHandler } from '@matrixai/rpc';
import { DuplexHandler } from '@matrixai/rpc';
import * as vaultsUtils from '../../vaults/utils';
import * as vaultsErrors from '../../vaults/errors';
import * as vaultOps from '../../vaults/VaultOps';

class VaultsSecretsRemove extends ClientHandler<
class VaultsSecretsRemove extends DuplexHandler<
{
db: DB;
vaultManager: VaultManager;
},
ClientRPCRequestParams<SecretIdentifierMessage>,
ClientRPCResponseResult<SuccessMessage>
ClientRPCResponseResult<SuccessOrErrorMessage>
> {
public handle = async (
public handle = async function* (
input: AsyncIterable<ClientRPCRequestParams<SecretIdentifierMessage>>,
): Promise<ClientRPCResponseResult<SuccessMessage>> => {
): AsyncGenerator<ClientRPCResponseResult<SuccessOrErrorMessage>> {
const { db, vaultManager }: { db: DB; vaultManager: VaultManager } =
this.container;
// Create a record of secrets to be removed, grouped by vault names
Expand All @@ -41,25 +40,54 @@ class VaultsSecretsRemove extends ClientHandler<
}
vaultGroups[vaultName].push(secretName);
});

await db.withTransactionF(async (tran) => {
for (const [vaultName, secretNames] of Object.entries(vaultGroups)) {
const vaultIdFromName = await vaultManager.getVaultId(vaultName, tran);
const vaultId = vaultIdFromName ?? vaultsUtils.decodeVaultId(vaultName);
if (vaultId == null) throw new vaultsErrors.ErrorVaultsVaultUndefined();
await vaultManager.withVaults(
[vaultId],
async (vault) => {
await vaultOps.deleteSecret(vault, secretNames, {
recursive: metadata?.options?.recursive,
});
},
tran,
);
}
});

return { type: 'success', success: true };
// Now, all the paths will be removed for a vault within a single commit
yield* db.withTransactionG(
async function* (tran): AsyncGenerator<SuccessOrErrorMessage> {
for (const [vaultName, secretNames] of Object.entries(vaultGroups)) {
const vaultIdFromName = await vaultManager.getVaultId(
vaultName,
tran,
);
const vaultId =
vaultIdFromName ?? vaultsUtils.decodeVaultId(vaultName);
if (vaultId == null) {
throw new vaultsErrors.ErrorVaultsVaultUndefined();
}
yield* vaultManager.withVaultsG(
[vaultId],
async function* (vault): AsyncGenerator<SuccessOrErrorMessage> {
yield* vault.writeG(
async function* (efs): AsyncGenerator<SuccessOrErrorMessage> {
for (const secretName of secretNames) {
try {
const stat = await efs.stat(secretName);
if (stat.isDirectory()) {
await efs.rmdir(secretName, {
recursive: metadata?.options?.recursive,
});
} else {
await efs.unlink(secretName);
}
yield {
type: 'success',
success: true,
};
} catch (e) {
yield {
type: 'error',
code: e.code,
reason: secretName,
};
}
}
},
);
},
tran,
);
}
},
);
};
}

Expand Down
50 changes: 24 additions & 26 deletions src/vaults/VaultOps.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,37 +129,35 @@ async function statSecret(vault: Vault, secretName: string): Promise<Stat> {
*/
async function deleteSecret(
vault: Vault,
secretNames: Array<string>,
secretName: string,
fileOptions?: FileOptions,
logger?: Logger,
): Promise<void> {
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;
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;
}
});
}
Expand Down
124 changes: 89 additions & 35 deletions tests/client/handlers/vaults.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1008,7 +1008,7 @@ describe('vaultsSecretsWriteFile', () => {
// Make the directory
await vaultManager.withVaults([vaultId], async (vault) => {
await vault.writeF(async (efs) => {
efs.mkdir(dirName);
await efs.mkdir(dirName);
});
});
// Try writing to directory
Expand Down Expand Up @@ -1056,7 +1056,7 @@ describe('vaultsSecretsWriteFile', () => {
// Make the directory
await vaultManager.withVaults([vaultId], async (vault) => {
await vault.writeF(async (efs) => {
efs.mkdir(dirName);
await efs.mkdir(dirName);
});
});
// Try writing
Expand Down Expand Up @@ -1789,7 +1789,9 @@ describe('vaultsSecretsNew and vaultsSecretsDelete, vaultsSecretsGet', () => {
secretName: secretName,
});
await deleteWriter.close();
expect((await deleteStream.output).success).toBeTruthy();
for await (const data of deleteStream.readable) {
expect(data.type).toStrictEqual('success');
}
// Check secret was deleted
const deleteGetStream = await rpcClient.methods.vaultsSecretsGet();
const deleteGetWriter = deleteGetStream.writable.getWriter();
Expand Down Expand Up @@ -1839,7 +1841,7 @@ describe('vaultsSecretsNew and vaultsSecretsDelete, vaultsSecretsGet', () => {
`${secretName1}${secretName2}${secretName3}`,
);
});
test('should not fail to get secrets on error when continueOnError is set', async () => {
test('should continue on error if option is set', async () => {
// Create secrets
const secretName1 = 'test-secret1';
const secretName2 = 'test-secret2';
Expand Down Expand Up @@ -1894,7 +1896,52 @@ describe('vaultsSecretsNew and vaultsSecretsDelete, vaultsSecretsGet', () => {
secretName: secretName2,
});
await deleteWriter.close();
expect((await deleteStream.output).success).toBeTruthy();
for await (const data of deleteStream.readable) {
expect(data.type).toStrictEqual('success');
}
// Check each secret was deleted
await checkSecretIsDeleted(vaultId, secretName1);
await checkSecretIsDeleted(vaultId, secretName2);
});
test('continues deleting secrets if invalid secret is present', async () => {
// Create secrets
const secretName1 = 'test-secret1';
const secretName2 = 'test-secret2';
const invalidName = 'invalid-name';
const vaultId = await vaultManager.createVault('test-vault');
const vaultIdEncoded = vaultsUtils.encodeVaultId(vaultId);
await createVaultSecret(vaultId, secretName1, secretName1);
await createVaultSecret(vaultId, secretName2, secretName2);
// Delete secrets
const deleteStream = await rpcClient.methods.vaultsSecretsRemove();
const deleteWriter = deleteStream.writable.getWriter();
await deleteWriter.write({
nameOrId: vaultIdEncoded,
secretName: secretName1,
});
await deleteWriter.write({
nameOrId: vaultIdEncoded,
secretName: invalidName,
});
await deleteWriter.write({
nameOrId: vaultIdEncoded,
secretName: secretName2,
});
await deleteWriter.close();
let errorCount = 0;
for await (const data of deleteStream.readable) {
if (data.type === 'error') {
// TS cannot properly evaluate a type as nested as this, so we use the
// as keyword to help it. Inside this block, the type of data is 'error'.
const error = data as ErrorMessage;
// No other file name should raise this error
expect(error.reason).toStrictEqual(invalidName);
errorCount++;
continue;
}
expect(data.type).toStrictEqual('success');
}
expect(errorCount).toEqual(1);
// Check each secret was deleted
await checkSecretIsDeleted(vaultId, secretName1);
await checkSecretIsDeleted(vaultId, secretName2);
Expand Down Expand Up @@ -1973,7 +2020,9 @@ describe('vaultsSecretsNew and vaultsSecretsDelete, vaultsSecretsGet', () => {
secretName: secretName3,
});
await deleteWriter.close();
expect((await deleteStream.output).success).toBeTruthy();
for await (const data of deleteStream.readable) {
expect(data.type).toStrictEqual('success');
}
// Ensure single log message for deleting the secrets
await vaultManager.withVaults(
[vaultId1, vaultId2],
Expand All @@ -1988,13 +2037,16 @@ describe('vaultsSecretsNew and vaultsSecretsDelete, vaultsSecretsGet', () => {
const vaultId = await vaultManager.createVault('test-vault');
const vaultIdEncoded = vaultsUtils.encodeVaultId(vaultId);
const secretDir = 'secret-dir';
const secretName1 = `${secretDir}/test-secret1`;
const secretName2 = `${secretDir}/test-secret2`;
const secretName3 = `${secretDir}/test-secret3`;
const secretName1 = 'test-secret1';
const secretName2 = 'test-secret2';
const secretName3 = 'test-secret3';
const secretPath1 = path.join(secretDir, secretName1);
const secretPath2 = path.join(secretDir, secretName2);
const secretPath3 = path.join(secretDir, secretName3);
await createVaultDir(vaultId, secretDir);
await createVaultSecret(vaultId, secretName1, secretName1);
await createVaultSecret(vaultId, secretName2, secretName2);
await createVaultSecret(vaultId, secretName3, secretName3);
await createVaultSecret(vaultId, secretPath1, secretPath1);
await createVaultSecret(vaultId, secretPath2, secretPath2);
await createVaultSecret(vaultId, secretPath3, secretPath3);
// Deleting directory with recursive set should not fail
const deleteStream = await rpcClient.methods.vaultsSecretsRemove();
await (async () => {
Expand All @@ -2006,11 +2058,13 @@ describe('vaultsSecretsNew and vaultsSecretsDelete, vaultsSecretsGet', () => {
});
await writer.close();
})();
expect((await deleteStream.output).success).toBeTruthy();
for await (const data of deleteStream.readable) {
expect(data.type).toStrictEqual('success');
}
// Check each secret and the secret directory were deleted
await checkSecretIsDeleted(vaultId, secretName1);
await checkSecretIsDeleted(vaultId, secretName2);
await checkSecretIsDeleted(vaultId, secretName3);
await checkSecretIsDeleted(vaultId, secretPath1);
await checkSecretIsDeleted(vaultId, secretPath2);
await checkSecretIsDeleted(vaultId, secretPath3);
await testsUtils.expectRemoteError(
rpcClient.methods.vaultsSecretsStat({
nameOrId: vaultIdEncoded,
Expand All @@ -2024,28 +2078,28 @@ describe('vaultsSecretsNew and vaultsSecretsDelete, vaultsSecretsGet', () => {
const vaultId = await vaultManager.createVault('test-vault');
const vaultIdEncoded = vaultsUtils.encodeVaultId(vaultId);
const secretDir = 'secret-dir';
const secretName1 = `${secretDir}/test-secret1`;
const secretName2 = `${secretDir}/test-secret2`;
const secretName3 = `${secretDir}/test-secret3`;
const secretName1 = 'test-secret1';
const secretName2 = 'test-secret2';
const secretName3 = 'test-secret3';
const secretPath1 = path.join(secretDir, secretName1);
const secretPath2 = path.join(secretDir, secretName2);
const secretPath3 = path.join(secretDir, secretName3);
await createVaultDir(vaultId, secretDir);
await createVaultSecret(vaultId, secretName1, secretName1);
await createVaultSecret(vaultId, secretName2, secretName2);
await createVaultSecret(vaultId, secretName3, secretName3);
await createVaultSecret(vaultId, secretPath1, secretPath1);
await createVaultSecret(vaultId, secretPath2, secretPath2);
await createVaultSecret(vaultId, secretPath3, secretPath3);
// Deleting directory with recursive unset should fail
const failDeleteStream = await rpcClient.methods.vaultsSecretsRemove();
await (async () => {
const writer = failDeleteStream.writable.getWriter();
await writer.write({ nameOrId: vaultIdEncoded, secretName: secretDir });
await writer.close();
})();
await testsUtils.expectRemoteError(
failDeleteStream.output,
vaultsErrors.ErrorVaultsRecursive,
);
const response = await rpcClient.methods.vaultsSecretsRemove();
const writer = response.writable.getWriter();
await writer.write({ nameOrId: vaultIdEncoded, secretName: secretDir });
await writer.close();
for await (const data of response.readable) {
expect(data.type).toStrictEqual('error');
}
// Check each secret and the secret directory exist
await checkSecretExists(vaultId, secretName1);
await checkSecretExists(vaultId, secretName2);
await checkSecretExists(vaultId, secretName3);
await checkSecretExists(vaultId, secretPath1);
await checkSecretExists(vaultId, secretPath2);
await checkSecretExists(vaultId, secretPath3);
await checkSecretExists(vaultId, secretDir);
});
});
Expand Down
Loading

0 comments on commit cadc962

Please sign in to comment.