Skip to content

Commit

Permalink
Merge branch 'improvement/ARSN-362-impDeny' into tmp/octopus/w/7.70/i…
Browse files Browse the repository at this point in the history 
…mprovement/ARSN-362-impDeny
  • Loading branch information
@bert-e
bert-e committed Sep 15, 2023
2 parents b61d178 + 1d74f51 commit e0352bd
Show file tree
Hide file tree
Showing 2 changed files with 193 additions and 3 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 evaluateAllPoliciesNew
* 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 evaluateAllPoliciesNew(requestContext, allPolicies, log).verdict;
}
export function evaluateAllPoliciesNew(
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 };
}
174 changes: 174 additions & 0 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 evaluateAllPoliciesNew = evaluator.evaluateAllPoliciesNew;
const handleWildcards =
require('../../lib/policyEvaluator/utils/wildcards').handleWildcards;
const substituteVariables =
Expand Down Expand Up @@ -1451,6 +1452,49 @@ describe('policyEvaluator', () => {
assert.strictEqual(result, 'Deny');
});

it('should deny access if any policy results in a Deny', () => {
requestContext = new RequestContext({}, {},
'my_favorite_bucket', undefined,
undefined, undefined, 'bucketDelete', 's3');
requestContext.setRequesterInfo({});
const result = evaluateAllPoliciesNew(requestContext,
[samples['arn:aws:iam::aws:policy/AmazonS3FullAccess'],
samples['Deny Bucket Policy']], log);
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 = evaluateAllPoliciesNew(requestContext,
[samples['Multi-Statement Policy'],
samples['Variable Bucket Policy']], log);
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 = evaluateAllPoliciesNew(requestContext, [
samples['Multi-Statement Policy'],
samples['Variable Bucket Policy'],
], log);
assert.deepStrictEqual(result, {
verdict: 'Deny',
isImplicit: true,
});
});

const TestMatrixPolicies = {
Allow: {
Version: '2012-10-17',
Expand Down Expand Up @@ -1504,6 +1548,136 @@ describe('policyEvaluator', () => {
},
};

const TestMatrixV2 = [
{
policiesToEvaluate: [],
expectedPolicyEvaluation: {
verdict: 'Deny',
isImplicit: true,
},
},
{
policiesToEvaluate: ['Allow'],
expectedPolicyEvaluation: {
verdict: 'Allow',
isImplicit: false,
},
},
{
policiesToEvaluate: ['Neutral'],
expectedPolicyEvaluation: {
verdict: 'Deny',
isImplicit: true,
},
},
{
policiesToEvaluate: ['Deny'],
expectedPolicyEvaluation: {
verdict: 'Deny',
isImplicit: false,
},
},
{
policiesToEvaluate: ['Allow', 'Allow'],
expectedPolicyEvaluation: {
verdict: 'Allow',
isImplicit: false,
},
},
{
policiesToEvaluate: ['Allow', 'Neutral'],
expectedPolicyEvaluation: {
verdict: 'Allow',
isImplicit: false,
},
},
{
policiesToEvaluate: ['Neutral', 'Allow'],
expectedPolicyEvaluation: {
verdict: 'Allow',
isImplicit: false,
},
},
{
policiesToEvaluate: ['Neutral', 'Neutral'],
expectedPolicyEvaluation: {
verdict: 'Deny',
isImplicit: true,
},
},
{
policiesToEvaluate: ['Allow', 'Deny'],
expectedPolicyEvaluation: {
verdict: 'Deny',
isImplicit: false,
},
},
{
policiesToEvaluate: ['AllowWithTagCondition'],
expectedPolicyEvaluation: {
verdict: 'NeedTagConditionEval',
isImplicit: false,
},
},
{
policiesToEvaluate: ['Allow', 'AllowWithTagCondition'],
expectedPolicyEvaluation: {
verdict: 'Allow',
isImplicit: false,
},
},
{
policiesToEvaluate: ['DenyWithTagCondition'],
expectedPolicyEvaluation: {
verdict: 'Deny',
isImplicit: true,
},
},
{
policiesToEvaluate: ['Allow', 'DenyWithTagCondition'],
expectedPolicyEvaluation: {
verdict: 'NeedTagConditionEval',
isImplicit: false,
},
},
{
policiesToEvaluate: ['AllowWithTagCondition', 'DenyWithTagCondition'],
expectedPolicyEvaluation: {
verdict: 'NeedTagConditionEval',
isImplicit: false,
},
},
{
policiesToEvaluate: ['AllowWithTagCondition', 'DenyWithTagCondition', 'Deny'],
expectedPolicyEvaluation: {
verdict: 'Deny',
isImplicit: false,
},
},
{
policiesToEvaluate: ['DenyWithTagCondition', 'AllowWithTagCondition', 'Allow'],
expectedPolicyEvaluation: {
verdict: 'NeedTagConditionEval',
isImplicit: false,
},
},
];

TestMatrixV2.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 = evaluateAllPoliciesNew(
requestContext,
testCase.policiesToEvaluate.map(policyName => TestMatrixPolicies[policyName]),
log);
assert.deepStrictEqual(result, testCase.expectedPolicyEvaluation);
});
});

const TestMatrix = [
{
policiesToEvaluate: [],
Expand Down

0 comments on commit e0352bd

Please sign in to comment.