fix(aws-iam): prevent adding duplicate resources and actions (aws#14712)

fixes aws#13611 cc @rix0rrr ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
sshver · Jun 15, 2021 · a8298cb · a8298cb
1 parent 0c50ec9
commit a8298cb
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 12 deletions.
diff --git a/packages/@aws-cdk/aws-glue/test/integ.table.expected.json b/packages/@aws-cdk/aws-glue/test/integ.table.expected.json
@@ -442,7 +442,6 @@
                 "glue:GetTableVersion",
                 "glue:GetTableVersions",
                 "glue:BatchCreatePartition",
-                "glue:BatchDeletePartition",
                 "glue:CreatePartition",
                 "glue:DeletePartition",
                 "glue:UpdatePartition"
@@ -520,7 +519,6 @@
                 "glue:GetTableVersion",
                 "glue:GetTableVersions",
                 "glue:BatchCreatePartition",
-                "glue:BatchDeletePartition",
                 "glue:CreatePartition",
                 "glue:DeletePartition",
                 "glue:UpdatePartition"
@@ -633,7 +631,6 @@
                 "glue:GetTableVersion",
                 "glue:GetTableVersions",
                 "glue:BatchCreatePartition",
-                "glue:BatchDeletePartition",
                 "glue:CreatePartition",
                 "glue:DeletePartition",
                 "glue:UpdatePartition"
@@ -711,7 +708,6 @@
                 "glue:GetTableVersion",
                 "glue:GetTableVersions",
                 "glue:BatchCreatePartition",
-                "glue:BatchDeletePartition",
                 "glue:CreatePartition",
                 "glue:DeletePartition",
                 "glue:UpdatePartition"
@@ -756,7 +752,6 @@
                 "glue:GetTableVersion",
                 "glue:GetTableVersions",
                 "glue:BatchCreatePartition",
-                "glue:BatchDeletePartition",
                 "glue:CreatePartition",
                 "glue:DeletePartition",
                 "glue:UpdatePartition"

diff --git a/packages/@aws-cdk/aws-glue/test/table.test.ts b/packages/@aws-cdk/aws-glue/test/table.test.ts
@@ -1189,7 +1189,6 @@ testFutureBehavior('grants: read and write', s3GrantWriteCtx, cdk.App, (app) =>
             'glue:GetTableVersion',
             'glue:GetTableVersions',
             'glue:BatchCreatePartition',
-            'glue:BatchDeletePartition',
             'glue:CreatePartition',
             'glue:DeletePartition',
             'glue:UpdatePartition',

diff --git a/packages/@aws-cdk/aws-iam/lib/policy-statement.ts b/packages/@aws-cdk/aws-iam/lib/policy-statement.ts
@@ -325,18 +325,18 @@ export class PolicyStatement {
    */
   public toStatementJson(): any {
     return noUndef({
-      Action: _norm(this.action),
-      NotAction: _norm(this.notAction),
+      Action: _norm(this.action, { unique: true }),
+      NotAction: _norm(this.notAction, { unique: true }),
       Condition: _norm(this.condition),
       Effect: _norm(this.effect),
       Principal: _normPrincipal(this.principal),
       NotPrincipal: _normPrincipal(this.notPrincipal),
-      Resource: _norm(this.resource),
-      NotResource: _norm(this.notResource),
+      Resource: _norm(this.resource, { unique: true }),
+      NotResource: _norm(this.notResource, { unique: true }),
       Sid: _norm(this.sid),
     });
 
-    function _norm(values: any) {
+    function _norm(values: any, { unique }: { unique: boolean } = { unique: false }) {
 
       if (typeof(values) === 'undefined') {
         return undefined;
@@ -355,7 +355,7 @@ export class PolicyStatement {
           return values[0];
         }
 
-        return values;
+        return unique ? [...new Set(values)] : values;
       }
 
       if (typeof(values) === 'object') {

diff --git a/packages/@aws-cdk/aws-iam/test/policy-document.test.ts b/packages/@aws-cdk/aws-iam/test/policy-document.test.ts
@@ -334,6 +334,40 @@ describe('IAM policy document', () => {
     });
   });
 
+  test('addResources()/addActions() will not add duplicates', () => {
+    const stack = new Stack();
+
+    const statement = new PolicyStatement();
+    statement.addActions('a');
+    statement.addActions('a');
+
+    statement.addResources('x');
+    statement.addResources('x');
+
+    expect(stack.resolve(statement.toStatementJson())).toEqual({
+      Effect: 'Allow',
+      Action: ['a'],
+      Resource: ['x'],
+    });
+  });
+
+  test('addNotResources()/addNotActions() will not add duplicates', () => {
+    const stack = new Stack();
+
+    const statement = new PolicyStatement();
+    statement.addNotActions('a');
+    statement.addNotActions('a');
+
+    statement.addNotResources('x');
+    statement.addNotResources('x');
+
+    expect(stack.resolve(statement.toStatementJson())).toEqual({
+      Effect: 'Allow',
+      NotAction: ['a'],
+      NotResource: ['x'],
+    });
+  });
+
   test('addCanonicalUserPrincipal can be used to add cannonical user principals', () => {
     const stack = new Stack();
     const p = new PolicyDocument();