fix(dynamodb): add missing permission for read stream data (aws#5074)

* Incorporate review comments Simplify if condition * Refactor the code * Incorporate review comments * Incorporate review comments * Incorporate review comments * Incorporate review comment
luisantonioa · Nov 29, 2019 · 22688ce · 22688ce
1 parent d2496e0
commit 22688ce
Show file tree

Hide file tree

Showing 6 changed files with 118 additions and 19 deletions.
diff --git a/packages/@aws-cdk/aws-dynamodb/lib/table.ts b/packages/@aws-cdk/aws-dynamodb/lib/table.ts
@@ -188,6 +188,7 @@ export interface LocalSecondaryIndexProps extends SecondaryIndexProps {
 export class Table extends Resource {
   /**
    * Permits an IAM Principal to list all DynamoDB Streams.
+   * @deprecated Use {@link #grantTableListStreams} for more granular permission
    * @param grantee The principal (no-op if undefined)
    */
   public static grantListStreams(grantee: iam.IGrantable): iam.Grant {
@@ -451,7 +452,7 @@ export class Table extends Resource {
    * @param grantee The principal (no-op if undefined)
    * @param actions The set of actions to allow (i.e. "dynamodb:DescribeStream", "dynamodb:GetRecords", ...)
    */
-  public grantStream(grantee: iam.IGrantable, ...actions: string[]) {
+  public grantStream(grantee: iam.IGrantable, ...actions: string[]): iam.Grant {
     if (!this.tableStreamArn) {
       throw new Error(`DynamoDB Streams must be enabled on the table ${this.node.path}`);
     }
@@ -474,12 +475,31 @@ export class Table extends Resource {
   }
 
   /**
-   * Permis an IAM principal all stream data read operations for this
+   * Permits an IAM Principal to list streams attached to current dynamodb table.
+   *
+   * @param grantee The principal (no-op if undefined)
+   */
+  public grantTableListStreams(grantee: iam.IGrantable): iam.Grant {
+    if (!this.tableStreamArn) {
+      throw new Error(`DynamoDB Streams must be enabled on the table ${this.node.path}`);
+    }
+    return iam.Grant.addToPrincipal({
+      grantee,
+      actions: ['dynamodb:ListStreams'],
+      resourceArns: [
+        Lazy.stringValue({ produce: () => `${this.tableArn}/stream/*`})
+      ],
+    });
+  }
+
+  /**
+   * Permits an IAM principal all stream data read operations for this
    * table's stream:
    * DescribeStream, GetRecords, GetShardIterator, ListStreams.
    * @param grantee The principal to grant access to
    */
-  public grantStreamRead(grantee: iam.IGrantable) {
+  public grantStreamRead(grantee: iam.IGrantable): iam.Grant {
+    this.grantTableListStreams(grantee);
     return this.grantStream(grantee, ...READ_STREAM_DATA_ACTIONS);
   }
 

diff --git a/packages/@aws-cdk/aws-dynamodb/test/test.dynamodb.ts b/packages/@aws-cdk/aws-dynamodb/test/test.dynamodb.ts
@@ -1184,6 +1184,55 @@ export = {
       test.done();
     },
 
+    '"grantTableListStreams" should fail if streaming is not enabled on table"'(test: Test) {
+      // GIVEN
+      const stack = new Stack();
+      const table = new Table(stack, 'my-table', {
+        partitionKey: {
+          name: 'id',
+          type: AttributeType.STRING
+        }
+      });
+      const user = new iam.User(stack, 'user');
+
+      // WHEN
+      test.throws(() => table.grantTableListStreams(user), /DynamoDB Streams must be enabled on the table my-table/);
+
+      test.done();
+    },
+
+    '"grantTableListStreams" allows principal to list all streams for this table'(test: Test) {
+      // GIVEN
+      const stack = new Stack();
+      const table = new Table(stack, 'my-table', {
+        partitionKey: {
+          name: 'id',
+          type: AttributeType.STRING
+        },
+        stream: StreamViewType.NEW_IMAGE
+      });
+      const user = new iam.User(stack, 'user');
+
+      // WHEN
+      table.grantTableListStreams(user);
+
+      // THEN
+      expect(stack).to(haveResource('AWS::IAM::Policy', {
+        "PolicyDocument": {
+          "Statement": [
+            {
+              "Action": "dynamodb:ListStreams",
+              "Effect": "Allow",
+              "Resource": { "Fn::Join": [ "", [ { "Fn::GetAtt": [ "mytable0324D45C", "Arn" ] }, "/stream/*" ] ] }
+            }
+          ],
+          "Version": "2012-10-17"
+        },
+        "Users": [ { "Ref": "user2C2B57AE" } ]
+      }));
+      test.done();
+    },
+
     '"grantStreamRead" should fail if streaming is not enabled on table"'(test: Test) {
       // GIVEN
       const stack = new Stack();
@@ -1220,6 +1269,11 @@ export = {
       expect(stack).to(haveResource('AWS::IAM::Policy', {
         "PolicyDocument": {
           "Statement": [
+            {
+              "Action": "dynamodb:ListStreams",
+              "Effect": "Allow",
+              "Resource": { "Fn::Join": [ "", [ { "Fn::GetAtt": [ "mytable0324D45C", "Arn" ] }, "/stream/*" ] ] }
+            },
             {
               "Action": [
                 "dynamodb:DescribeStream",

diff --git a/packages/@aws-cdk/aws-lambda-event-sources/lib/dynamodb.ts b/packages/@aws-cdk/aws-lambda-event-sources/lib/dynamodb.ts
@@ -27,6 +27,5 @@ export class DynamoEventSource extends StreamEventSource {
     );
 
     this.table.grantStreamRead(target);
-    dynamodb.Table.grantListStreams(target);
   }
 }
diff --git a/packages/@aws-cdk/aws-lambda-event-sources/test/integ.dynamodb.expected.json b/packages/@aws-cdk/aws-lambda-event-sources/test/integ.dynamodb.expected.json
@@ -36,6 +36,24 @@
       "Properties": {
         "PolicyDocument": {
           "Statement": [
+            {
+              "Action": "dynamodb:ListStreams",
+              "Effect": "Allow",
+              "Resource": {
+                "Fn::Join": [
+                  "",
+                  [
+                    {
+                      "Fn::GetAtt": [
+                        "TD925BC7E",
+                        "Arn"
+                      ]
+                    },
+                    "/stream/*"
+                  ]
+                ]
+              }
+            },
             {
               "Action": [
                 "dynamodb:DescribeStream",
@@ -49,11 +67,6 @@
                   "StreamArn"
                 ]
               }
-            },
-            {
-              "Action": "dynamodb:ListStreams",
-              "Effect": "Allow",
-              "Resource": "*"
             }
           ],
           "Version": "2012-10-17"

diff --git a/packages/@aws-cdk/aws-lambda-event-sources/test/test.dynamo.ts b/packages/@aws-cdk/aws-lambda-event-sources/test/test.dynamo.ts
@@ -30,6 +30,11 @@ export = {
     expect(stack).to(haveResource('AWS::IAM::Policy', {
       "PolicyDocument": {
         "Statement": [
+          {
+            "Action": "dynamodb:ListStreams",
+            "Effect": "Allow",
+            "Resource": { "Fn::Join": [ "", [ { "Fn::GetAtt": [ "TD925BC7E", "Arn" ] }, "/stream/*" ] ] }
+          },
           {
             "Action": [
               "dynamodb:DescribeStream",
@@ -43,11 +48,6 @@ export = {
                 "StreamArn"
               ]
             }
-          },
-          {
-            "Action": "dynamodb:ListStreams",
-            "Effect": "Allow",
-            "Resource": "*"
           }
         ],
         "Version": "2012-10-17"

diff --git a/packages/decdk/test/__snapshots__/synth.test.js.snap b/packages/decdk/test/__snapshots__/synth.test.js.snap
@@ -1083,6 +1083,24 @@ Object {
       "Properties": Object {
         "PolicyDocument": Object {
           "Statement": Array [
+            Object {
+              "Action": "dynamodb:ListStreams",
+              "Effect": "Allow",
+              "Resource": Object {
+                "Fn::Join": Array [
+                  "",
+                  Array [
+                    Object {
+                      "Fn::GetAtt": Array [
+                        "TableCD117FA1",
+                        "Arn",
+                      ],
+                    },
+                    "/stream/*",
+                  ],
+                ],
+              },
+            },
             Object {
               "Action": Array [
                 "dynamodb:DescribeStream",
@@ -1097,11 +1115,6 @@ Object {
                 ],
               },
             },
-            Object {
-              "Action": "dynamodb:ListStreams",
-              "Effect": "Allow",
-              "Resource": "*",
-            },
           ],
           "Version": "2012-10-17",
         },
-Original file line number
+Diff line change
@@ Expand Up / @@ -27,6 +27,5 @@ export class DynamoEventSource extends StreamEventSource { @@
         );
         this.table.grantStreamRead(target);
-        dynamodb.Table.grantListStreams(target);
       }
     }