Skip to content

Commit

Permalink
Merge branch 'improvement/ARSN-362-implicitDeny' into tmp/octopus/w/7…
Browse files Browse the repository at this point in the history 
….70/improvement/ARSN-362-implicitDeny
  • Loading branch information
@bert-e
bert-e committed Oct 30, 2023
2 parents 79b83a9 + df5ff0f commit c34ad0d
Show file tree
Hide file tree
Showing 2 changed files with 123 additions and 26 deletions.
22 changes: 19 additions & 3 deletions lib/policyEvaluator/evaluator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,7 @@ export function evaluatePolicy(
}

/**
* @deprecated Upgrade to standardEvaluateAllPolicies
* Evaluate whether a request is permitted under a policy.
* @param requestContext - Info necessary to
* evaluate permission
Expand All @@ -325,6 +326,16 @@ export function evaluateAllPolicies(
allPolicies: any[],
log: Logger,
): string {
return standardEvaluateAllPolicies(requestContext, allPolicies, log).verdict;
}
export function standardEvaluateAllPolicies(
requestContext: RequestContext,
allPolicies: any[],
log: Logger,
): {
verdict: string;
isImplicit: boolean;
} {
log.trace('evaluating all policies');
let allow = false;
let allowWithTagCondition = false;
Expand All @@ -333,7 +344,10 @@ export function evaluateAllPolicies(
const singlePolicyVerdict = evaluatePolicy(requestContext, allPolicies[i], log);
// If there is any Deny, just return Deny
if (singlePolicyVerdict === 'Deny') {
return 'Deny';
return {
verdict: 'Deny',
isImplicit: false,
};
}
if (singlePolicyVerdict === 'Allow') {
allow = true;
Expand All @@ -344,6 +358,7 @@ export function evaluateAllPolicies(
} // else 'Neutral'
}
let verdict;
let isImplicit = false;
if (allow) {
if (denyWithTagCondition) {
verdict = 'NeedTagConditionEval';
Expand All @@ -355,8 +370,9 @@ export function evaluateAllPolicies(
verdict = 'NeedTagConditionEval';
} else {
verdict = 'Deny';
isImplicit = true;
}
}
log.trace('result of evaluating all policies', { verdict });
return verdict;
log.trace('result of evaluating all policies', { verdict, isImplicit });
return { verdict, isImplicit };
}
127 changes: 104 additions & 23 deletions tests/unit/policyEvaluator.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const fakeTimers = require('@sinonjs/fake-timers');
const evaluator = require('../../lib/policyEvaluator/evaluator');
const evaluatePolicy = evaluator.evaluatePolicy;
const evaluateAllPolicies = evaluator.evaluateAllPolicies;
const standardEvaluateAllPolicies = evaluator.standardEvaluateAllPolicies;
const handleWildcards =
require('../../lib/policyEvaluator/utils/wildcards').handleWildcards;
const substituteVariables =
Expand Down Expand Up @@ -1422,33 +1423,42 @@ describe('policyEvaluator', () => {
'my_favorite_bucket', undefined,
undefined, undefined, 'bucketDelete', 's3');
requestContext.setRequesterInfo({});
const result = evaluateAllPolicies(requestContext,
const result = standardEvaluateAllPolicies(requestContext,
[samples['arn:aws:iam::aws:policy/AmazonS3FullAccess'],
samples['Deny Bucket Policy']], log);
assert.strictEqual(result, 'Deny');
assert.deepStrictEqual(result, {
verdict: 'Deny',
isImplicit: false,
});
});

it('should deny access if request action is not in any policy', () => {
requestContext = new RequestContext({}, {},
'notVeryPrivate', undefined,
undefined, undefined, 'bucketDelete', 's3');
requestContext.setRequesterInfo({});
const result = evaluateAllPolicies(requestContext,
const result = standardEvaluateAllPolicies(requestContext,
[samples['Multi-Statement Policy'],
samples['Variable Bucket Policy']], log);
assert.strictEqual(result, 'Deny');
assert.deepStrictEqual(result, {
verdict: 'Deny',
isImplicit: true,
});
});

it('should deny access if request resource is not in any policy', () => {
requestContext = new RequestContext({}, {},
'notbucket', undefined,
undefined, undefined, 'objectGet', 's3');
requestContext.setRequesterInfo({});
const result = evaluateAllPolicies(requestContext, [
const result = standardEvaluateAllPolicies(requestContext, [
samples['Multi-Statement Policy'],
samples['Variable Bucket Policy'],
], log);
assert.strictEqual(result, 'Deny');
assert.deepStrictEqual(result, {
verdict: 'Deny',
isImplicit: true,
});
});

const TestMatrixPolicies = {
Expand Down Expand Up @@ -1507,70 +1517,141 @@ describe('policyEvaluator', () => {
const TestMatrix = [
{
policiesToEvaluate: [],
expectedPolicyEvaluation: 'Deny',
expectedPolicyEvaluation: {
verdict: 'Deny',
isImplicit: true,
},
},
{
policiesToEvaluate: ['Allow'],
expectedPolicyEvaluation: 'Allow',
expectedPolicyEvaluation: {
verdict: 'Allow',
isImplicit: false,
},
},
{
policiesToEvaluate: ['Neutral'],
expectedPolicyEvaluation: 'Deny',
expectedPolicyEvaluation: {
verdict: 'Deny',
isImplicit: true,
},
},
{
policiesToEvaluate: ['Deny'],
expectedPolicyEvaluation: 'Deny',
expectedPolicyEvaluation: {
verdict: 'Deny',
isImplicit: false,
},
},
{
policiesToEvaluate: ['Allow', 'Allow'],
expectedPolicyEvaluation: 'Allow',
expectedPolicyEvaluation: {
verdict: 'Allow',
isImplicit: false,
},
},
{
policiesToEvaluate: ['Allow', 'Neutral'],
expectedPolicyEvaluation: 'Allow',
expectedPolicyEvaluation: {
verdict: 'Allow',
isImplicit: false,
},
},
{
policiesToEvaluate: ['Neutral', 'Allow'],
expectedPolicyEvaluation: 'Allow',
expectedPolicyEvaluation: {
verdict: 'Allow',
isImplicit: false,
},
},
{
policiesToEvaluate: ['Neutral', 'Neutral'],
expectedPolicyEvaluation: 'Deny',
expectedPolicyEvaluation: {
verdict: 'Deny',
isImplicit: true,
},
},
{
policiesToEvaluate: ['Neutral', 'Deny'],
expectedPolicyEvaluation: {
verdict: 'Deny',
isImplicit: false,
},
},
{
policiesToEvaluate: ['Allow', 'Deny'],
expectedPolicyEvaluation: 'Deny',
expectedPolicyEvaluation: {
verdict: 'Deny',
isImplicit: false,
},
},
{
policiesToEvaluate: ['AllowWithTagCondition'],
expectedPolicyEvaluation: 'NeedTagConditionEval',
expectedPolicyEvaluation: {
verdict: 'NeedTagConditionEval',
isImplicit: false,
},
},
{
policiesToEvaluate: ['Allow', 'AllowWithTagCondition'],
expectedPolicyEvaluation: 'Allow',
expectedPolicyEvaluation: {
verdict: 'Allow',
isImplicit: false,
},
},
{
policiesToEvaluate: ['DenyWithTagCondition'],
expectedPolicyEvaluation: 'Deny',
expectedPolicyEvaluation: {
verdict: 'Deny',
isImplicit: true,
},
},
{
policiesToEvaluate: ['Allow', 'DenyWithTagCondition'],
expectedPolicyEvaluation: 'NeedTagConditionEval',
expectedPolicyEvaluation: {
verdict: 'NeedTagConditionEval',
isImplicit: false,
},
},
{
policiesToEvaluate: ['AllowWithTagCondition', 'DenyWithTagCondition'],
expectedPolicyEvaluation: 'NeedTagConditionEval',
expectedPolicyEvaluation: {
verdict: 'NeedTagConditionEval',
isImplicit: false,
},
},
{
policiesToEvaluate: ['AllowWithTagCondition', 'DenyWithTagCondition', 'Deny'],
expectedPolicyEvaluation: 'Deny',
expectedPolicyEvaluation: {
verdict: 'Deny',
isImplicit: false,
},
},
{
policiesToEvaluate: ['DenyWithTagCondition', 'AllowWithTagCondition', 'Allow'],
expectedPolicyEvaluation: 'NeedTagConditionEval',
expectedPolicyEvaluation: {
verdict: 'NeedTagConditionEval',
isImplicit: false,
},
},
];

TestMatrix.forEach(testCase => {
it(`policies evaluating individually to [${testCase.policiesToEvaluate.join(', ')}] `
+ `should return ${testCase.expectedPolicyEvaluation}`, () => {
requestContext = new RequestContext({}, {},
'my_favorite_bucket', undefined,
undefined, undefined, 'objectGet', 's3');
requestContext.setRequesterInfo({});
const result = standardEvaluateAllPolicies(
requestContext,
testCase.policiesToEvaluate.map(policyName => TestMatrixPolicies[policyName]),
log);
assert.deepStrictEqual(result, testCase.expectedPolicyEvaluation);
});
});


TestMatrix.forEach(testCase => {
it(`policies evaluating individually to [${testCase.policiesToEvaluate.join(', ')}] `
+ `should return ${testCase.expectedPolicyEvaluation}`, () => {
Expand All @@ -1582,7 +1663,7 @@ describe('policyEvaluator', () => {
requestContext,
testCase.policiesToEvaluate.map(policyName => TestMatrixPolicies[policyName]),
log);
assert.strictEqual(result, testCase.expectedPolicyEvaluation);
assert.strictEqual(result, testCase.expectedPolicyEvaluation.verdict);
});
});
});
Expand Down

0 comments on commit c34ad0d

Please sign in to comment.