Skip to content

Commit

Permalink
Add support for lazily fetching vault access status (#1478)
Browse files Browse the repository at this point in the history 
* feat: Refactor vault access handling in event command

* modified logic and test cases

* change _allowScriptAccess in tests to be function

* Add support for lazily fetching vault access status

---------

Co-authored-by: Appurva Murawat <[email protected]>
  • Loading branch information
@saksham-postman @appurva21
saksham-postman and appurva21 authored Oct 28, 2024
1 parent 9efdc9d commit ec6c130
Show file tree
Hide file tree
Showing 4 changed files with 199 additions and 16 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.yaml
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
unreleased:
new features:
- GH-1478 Added support for lazily fetching vault access status

7.42.0:
date: 2024-09-04
new features:
Expand Down
31 changes: 21 additions & 10 deletions lib/runner/extensions/event.command.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,16 +243,12 @@ module.exports = {
packageResolver = _.get(this, 'options.script.packageResolver'),

vaultSecrets = payload.context.vaultSecrets,
allowVaultAccess = _.get(vaultSecrets, '_.allowScriptAccess'),
// Do not assign any initial value here as it will be used
// to determine if the vault access check was done or not
hasVaultAccess,

events;

// Explicitly enable tracking for vault secrets here as this will
// not be sent to sandbox who otherwise takes care of mutation tracking
if (allowVaultAccess) {
vaultSecrets.enableTracking({ autoCompact: true });
}

// @todo: find a better place to code this so that event is not aware of such options
if (abortOnFailure) {
abortOnError = true;
Expand Down Expand Up @@ -398,19 +394,34 @@ module.exports = {
}
}.bind(this));

this.host.on(EXECUTION_VAULT_BASE + executionId, function (id, cmd, ...args) {
this.host.on(EXECUTION_VAULT_BASE + executionId, async function (id, cmd, ...args) {
if (hasVaultAccess === undefined) {
try {
// eslint-disable-next-line require-atomic-updates
hasVaultAccess = Boolean(await vaultSecrets?._?.allowScriptAccess(item.id));
}
catch (_) {
// eslint-disable-next-line require-atomic-updates
hasVaultAccess = false;
}
}

// Ensure error is string
// TODO identify why error objects are not being serialized correctly
const dispatch = (e, r) => { this.host.dispatch(EXECUTION_VAULT_BASE + executionId, id, e, r); };

if (!allowVaultAccess) {
if (!hasVaultAccess) {
return dispatch('Vault access denied');
}

if (!['get', 'set', 'unset'].includes(cmd)) {
return dispatch(`Invalid vault command: ${cmd}`);
}

// Explicitly enable tracking for vault secrets here as this will
// not be sent to sandbox who otherwise takes care of mutation tracking
vaultSecrets.enableTracking({ autoCompact: true });

dispatch(null, vaultSecrets[cmd](...args));
}.bind(this));

Expand Down Expand Up @@ -556,7 +567,7 @@ module.exports = {
result && result.request && (result.request = new sdk.Request(result.request));

// vault secrets are not sent to sandbox, thus using the scope from run context.
if (allowVaultAccess && vaultSecrets) {
if (hasVaultAccess && vaultSecrets) {
result.vaultSecrets = vaultSecrets;

// Prevent mutations from being carry-forwarded to subsequent events
Expand Down
2 changes: 1 addition & 1 deletion test/integration/sanity/variable-changes.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ describe('variable changes', function () {
requester: { followRedirects: false },
vaultSecrets: {
id: 'vault',
_allowScriptAccess: true,
_allowScriptAccess: function () { return true; },
values: [
{ key: 'vault:key5', value: 'vault-value-5', enabled: true },
{ key: 'vault:key6', value: 'vault-value-6', enabled: true }
Expand Down
178 changes: 173 additions & 5 deletions test/integration/sanity/vaultSecrets.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,7 @@ describe('vaultSecrets', function () {
vaultSecrets: {
id: 'vault',
prefix: 'vault:',
_allowScriptAccess: true,
_allowScriptAccess: function () { return true; },
values: [
{
key: 'vault:var1',
Expand Down Expand Up @@ -795,7 +795,7 @@ describe('vaultSecrets', function () {
vaultSecrets: {
id: 'vault',
prefix: 'vault:',
_allowScriptAccess: true,
_allowScriptAccess: function () { return true; },
values: [
{
key: 'vault:var1',
Expand Down Expand Up @@ -847,7 +847,7 @@ describe('vaultSecrets', function () {
vaultSecrets: {
id: 'vault',
prefix: 'vault:',
_allowScriptAccess: true,
_allowScriptAccess: function () { return true; },
values: [
{
key: 'vault:var1',
Expand Down Expand Up @@ -900,7 +900,7 @@ describe('vaultSecrets', function () {
vaultSecrets: {
id: 'vault',
prefix: 'vault:',
_allowScriptAccess: true,
_allowScriptAccess: function () { return true; },
values: [
{
key: 'vault:var1',
Expand Down Expand Up @@ -957,7 +957,7 @@ describe('vaultSecrets', function () {
vaultSecrets: {
id: 'vault',
prefix: 'vault:',
_allowScriptAccess: true,
_allowScriptAccess: function () { return true; },
values: [
{
key: 'vault:var1',
Expand Down Expand Up @@ -1073,5 +1073,173 @@ describe('vaultSecrets', function () {
expect(prerequest.vaultSecrets).to.deep.equal(undefined);
});
});

describe('should handle _allowScriptAccess as a function', function () {
var testrun;

before(function (done) {
this.run({
vaultSecrets: {
id: 'vault',
prefix: 'vault:',
_allowScriptAccess: function (itemId) {
return itemId === 'item1';
},
values: [
{
key: 'vault:var1',
value: 'value1'
}
]
},
collection: {
item: [{
id: 'item1',
event: [{
listen: 'prerequest',
script: {
exec: `
const v = await pm.vault.get('var1');
console.log(v);
`
}
}],
request: 'https://postman-echo.com/get'
}, {
id: 'item2',
event: [{
listen: 'prerequest',
script: {
exec: `
try {
const v = await pm.vault.get('var1');
console.log(v);
} catch (error) {
console.error(error.message);
}
`
}
}],
request: 'https://postman-echo.com/get'
}]
}
}, function (err, results) {
testrun = results;
done(err);
});
});

it('should allow vault access for item1', function () {
var prConsoleArgs = testrun.console.getCall(0).args.slice(2);

expect(prConsoleArgs).to.deep.equal(['value1']);
});

it('should deny vault access for item2', function () {
var prConsoleArgs = testrun.console.getCall(1).args.slice(2);

expect(prConsoleArgs).to.deep.equal(['Vault access denied']);
});
});

describe('should fail if _allowScriptAccess is not a function', function () {
var testrun;

before(function (done) {
this.run({
vaultSecrets: {
id: 'vault',
prefix: 'vault:',
_allowScriptAccess: true,
values: [
{
key: 'vault:var1',
value: 'value1'
}
]
},
collection: {
item: {
event: [{
listen: 'prerequest',
script: {
exec: `
try {
const v = await pm.vault.get('var1');
console.log('Vault value:', v);
} catch (error) {
console.error('Vault error:', error.message);
}
`
}
}],
request: 'https://postman-echo.com/get'
}
}
}, function (err, results) {
testrun = results;
done(err);
});
});

it('should deny vault access when _allowScriptAccess is not a function', function () {
expect(testrun.console.called).to.be.true;

var consoleArgs = testrun.console.getCall(0).args.slice(2);

expect(consoleArgs[0]).to.equal('Vault error:');
expect(consoleArgs[1]).to.equal('Vault access denied');
});
});

describe('should handle when _allowScriptAccess function throws', function () {
var testrun;

before(function (done) {
this.run({
vaultSecrets: {
id: 'vault',
prefix: 'vault:',
_allowScriptAccess: function () { throw new Error('Custom error'); },
values: [
{
key: 'vault:var1',
value: 'value1'
}
]
},
collection: {
item: {
event: [{
listen: 'prerequest',
script: {
exec: `
try {
const v = await pm.vault.get('var1');
console.log('Vault value:', v);
} catch (error) {
console.error('Vault error:', error.message);
}
`
}
}],
request: 'https://postman-echo.com/get'
}
}
}, function (err, results) {
testrun = results;
done(err);
});
});

it('should deny vault access when _allowScriptAccess function throws', function () {
expect(testrun.console.called).to.be.true;

var consoleArgs = testrun.console.getCall(0).args.slice(2);

expect(consoleArgs[0]).to.equal('Vault error:');
expect(consoleArgs[1]).to.equal('Vault access denied');
});
});
});
});

0 comments on commit ec6c130

Please sign in to comment.