diff --git a/lib/policyEvaluator/evaluator.ts b/lib/policyEvaluator/evaluator.ts index 01626d7e6..200816fa3 100644 --- a/lib/policyEvaluator/evaluator.ts +++ b/lib/policyEvaluator/evaluator.ts @@ -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 @@ -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; @@ -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; @@ -344,6 +358,7 @@ export function evaluateAllPolicies( } // else 'Neutral' } let verdict; + let isImplicit = false; if (allow) { if (denyWithTagCondition) { verdict = 'NeedTagConditionEval'; @@ -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 }; } diff --git a/package.json b/package.json index ce46184df..39c36c4e9 100644 --- a/package.json +++ b/package.json @@ -3,7 +3,7 @@ "engines": { "node": ">=16" }, - "version": "8.1.113", + "version": "8.1.114", "description": "Common utilities for the S3 project components", "main": "build/index.js", "repository": { diff --git a/tests/unit/policyEvaluator.spec.js b/tests/unit/policyEvaluator.spec.js index e534ebdcf..91fa9ab2d 100644 --- a/tests/unit/policyEvaluator.spec.js +++ b/tests/unit/policyEvaluator.spec.js @@ -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 = @@ -1422,10 +1423,13 @@ 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', () => { @@ -1433,10 +1437,13 @@ describe('policyEvaluator', () => { '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', () => { @@ -1444,11 +1451,14 @@ describe('policyEvaluator', () => { '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 = { @@ -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}`, () => { @@ -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); }); }); });