INTEGRATION [PR#2181 > development/8.1] Improvement/arsn 362 implicit deny

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 };
 }

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": {

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,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 = {
@@ -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);
             });
         });
     });