feat(aws-s3): additional grant* methods for buckets (#591)

Add: - `grantPut` => s3:PutObject*, s3:Abort* (on objects) - `grantDelete` => s3:DeleteObject* (on objects) - `grantWrite` => put + delete (on bucket + objects) OTHER CHANGES - Fix 'npm run integ' in the 'aws-cdk-codepipeline' package, which would fail because a Topic with an email subscription could not be deleted.
aws · Aug 20, 2018 · a9114c5 · a9114c5
1 parent 2a5240c
commit a9114c5
Show file tree

Hide file tree

Showing 13 changed files with 118 additions and 49 deletions.
diff --git a/packages/@aws-cdk/aws-codepipeline/test/integ.cfn-template-from-repo.lit.expected.json b/packages/@aws-cdk/aws-codepipeline/test/integ.cfn-template-from-repo.lit.expected.json
@@ -31,8 +31,8 @@
                 "s3:GetObject*",
                 "s3:GetBucket*",
                 "s3:List*",
-                "s3:PutObject*",
                 "s3:DeleteObject*",
+                "s3:PutObject*",
                 "s3:Abort*"
               ],
               "Effect": "Allow",
@@ -144,6 +144,7 @@
                   "Version": "1"
                 },
                 "Configuration": {
+                  "StackName": "OurStack",
                   "ActionMode": "CHANGE_SET_REPLACE",
                   "ChangeSetName": "StagedChangeSet",
                   "TemplatePath": "SourceArtifact::template.yaml",
@@ -153,8 +154,7 @@
                       "PipelineDeployPrepareChangesRoleD28C853C",
                       "Arn"
                     ]
-                  },
-                  "StackName": "OurStack"
+                  }
                 },
                 "InputArtifacts": [
                   {
@@ -185,9 +185,9 @@
                   "Version": "1"
                 },
                 "Configuration": {
+                  "StackName": "OurStack",
                   "ActionMode": "CHANGE_SET_EXECUTE",
-                  "ChangeSetName": "StagedChangeSet",
-                  "StackName": "OurStack"
+                  "ChangeSetName": "StagedChangeSet"
                 },
                 "InputArtifacts": [],
                 "Name": "ExecuteChanges",

diff --git a/packages/@aws-cdk/aws-codepipeline/test/integ.lambda-pipeline.expected.json b/packages/@aws-cdk/aws-codepipeline/test/integ.lambda-pipeline.expected.json
@@ -31,8 +31,8 @@
                 "s3:GetObject*",
                 "s3:GetBucket*",
                 "s3:List*",
-                "s3:PutObject*",
                 "s3:DeleteObject*",
+                "s3:PutObject*",
                 "s3:Abort*"
               ],
               "Effect": "Allow",

diff --git a/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-cfn.expected.json b/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-cfn.expected.json
@@ -31,8 +31,8 @@
                 "s3:GetObject*",
                 "s3:GetBucket*",
                 "s3:List*",
-                "s3:PutObject*",
                 "s3:DeleteObject*",
+                "s3:PutObject*",
                 "s3:Abort*"
               ],
               "Effect": "Allow",
@@ -156,6 +156,7 @@
                   "Version": "1"
                 },
                 "Configuration": {
+                  "StackName": "IntegTest-TestActionStack",
                   "ActionMode": "CHANGE_SET_REPLACE",
                   "ChangeSetName": "ChangeSetIntegTest",
                   "TemplatePath": "SourceArtifact::test.yaml",
@@ -164,8 +165,7 @@
                       "CfnChangeSetRole6F05F6FC",
                       "Arn"
                     ]
-                  },
-                  "StackName": "IntegTest-TestActionStack"
+                  }
                 },
                 "InputArtifacts": [
                   {

diff --git a/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-code-commit-build.expected.json b/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-code-commit-build.expected.json
@@ -38,8 +38,8 @@
                 "s3:GetObject*",
                 "s3:GetBucket*",
                 "s3:List*",
-                "s3:PutObject*",
                 "s3:DeleteObject*",
+                "s3:PutObject*",
                 "s3:Abort*"
               ],
               "Effect": "Allow",
@@ -291,8 +291,8 @@
                 "s3:GetObject*",
                 "s3:GetBucket*",
                 "s3:List*",
-                "s3:PutObject*",
                 "s3:DeleteObject*",
+                "s3:PutObject*",
                 "s3:Abort*"
               ],
               "Effect": "Allow",

diff --git a/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-code-commit.expected.json b/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-code-commit.expected.json
@@ -38,8 +38,8 @@
                 "s3:GetObject*",
                 "s3:GetBucket*",
                 "s3:List*",
-                "s3:PutObject*",
                 "s3:DeleteObject*",
+                "s3:PutObject*",
                 "s3:Abort*"
               ],
               "Effect": "Allow",

diff --git a/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-events.expected.json b/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-events.expected.json
@@ -31,8 +31,8 @@
                 "s3:GetObject*",
                 "s3:GetBucket*",
                 "s3:List*",
-                "s3:PutObject*",
                 "s3:DeleteObject*",
+                "s3:PutObject*",
                 "s3:Abort*"
               ],
               "Effect": "Allow",
@@ -464,8 +464,8 @@
                 "s3:GetObject*",
                 "s3:GetBucket*",
                 "s3:List*",
-                "s3:PutObject*",
                 "s3:DeleteObject*",
+                "s3:PutObject*",
                 "s3:Abort*"
               ],
               "Effect": "Allow",
@@ -530,16 +530,6 @@
     "MyTopic86869434": {
       "Type": "AWS::SNS::Topic"
     },
-    "MyTopicbenisraeBF3B2D80": {
-      "Type": "AWS::SNS::Subscription",
-      "Properties": {
-        "Endpoint": "benisrae@amazon.com",
-        "Protocol": "email",
-        "TopicArn": {
-          "Ref": "MyTopic86869434"
-        }
-      }
-    },
     "MyTopicPolicy12A5EC17": {
       "Type": "AWS::SNS::TopicPolicy",
       "Properties": {

diff --git a/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-events.ts b/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-events.ts
@@ -31,7 +31,6 @@ new codebuild.PipelineBuildAction(stack, 'CodeBuildAction', {
 });
 
 const topic = new sns.Topic(stack, 'MyTopic');
-topic.subscribeEmail('benisrae', 'benisrae@amazon.com');
 
 pipeline.onStateChange('OnPipelineStateChange').addTarget(topic, {
     textTemplate: 'Pipeline <pipeline> changed state to <state>',

diff --git a/packages/@aws-cdk/aws-lambda/test/inline.expected.json b/packages/@aws-cdk/aws-lambda/test/inline.expected.json
@@ -34,8 +34,8 @@
                 "s3:GetObject*",
                 "s3:GetBucket*",
                 "s3:List*",
-                "s3:PutObject*",
                 "s3:DeleteObject*",
+                "s3:PutObject*",
                 "s3:Abort*"
               ],
               "Effect": "Allow",

diff --git a/packages/@aws-cdk/aws-lambda/test/integ.inline.expected.json b/packages/@aws-cdk/aws-lambda/test/integ.inline.expected.json
@@ -54,8 +54,8 @@
                 "s3:GetObject*",
                 "s3:GetBucket*",
                 "s3:List*",
-                "s3:PutObject*",
                 "s3:DeleteObject*",
+                "s3:PutObject*",
                 "s3:Abort*"
               ],
               "Effect": "Allow",

diff --git a/packages/@aws-cdk/aws-s3/lib/bucket.ts b/packages/@aws-cdk/aws-s3/lib/bucket.ts
@@ -161,17 +161,59 @@ export abstract class BucketRef extends cdk.Construct {
     }
 
     /**
-     * Temporary API for granting read permissions for this bucket and it's
-     * contents to an IAM principal (Role/Group/User).
+     * Grant read permissions for this bucket and it's contents to an IAM
+     * principal (Role/Group/User).
+     *
+     * If encryption is used, permission to use the key to decrypt the contents
+     * of the bucket will also be granted to the same principal.
      *
-     * If an encryption key is used, permission to ues the key to decrypt the
-     * contents of the bucket will also be granted.
+     * @param identity The principal
+     * @param objectsKeyPattern Restrict the permission to a certain key pattern (default '*')
      */
     public grantRead(identity?: iam.IPrincipal, objectsKeyPattern: any = '*') {
-        if (!identity) {
-            return;
-        }
-        this.grant(identity, objectsKeyPattern, perms.BUCKET_READ_ACTIONS, perms.KEY_READ_ACTIONS);
+        this.grant(identity, perms.BUCKET_READ_ACTIONS, perms.KEY_READ_ACTIONS,
+            this.bucketArn,
+            this.arnForObjects(objectsKeyPattern));
+    }
+
+    /**
+     * Grant write permissions to this bucket to an IAM principal.
+     *
+     * If encryption is used, permission to use the key to encrypt the contents
+     * of written files will also be granted to the same principal.
+     *
+     * @param identity The principal
+     * @param objectsKeyPattern Restrict the permission to a certain key pattern (default '*')
+     */
+    public grantWrite(identity?: iam.IPrincipal, objectsKeyPattern: any = '*') {
+        this.grant(identity, perms.BUCKET_WRITE_ACTIONS, perms.KEY_WRITE_ACTIONS,
+            this.bucketArn,
+            this.arnForObjects(objectsKeyPattern));
+    }
+
+    /**
+     * Grants s3:PutObject* and s3:Abort* permissions for this bucket to an IAM principal.
+     *
+     * If encryption is used, permission to use the key to encrypt the contents
+     * of written files will also be granted to the same principal.
+     * @param identity The principal
+     * @param objectsKeyPattern Restrict the permission to a certain key pattern (default '*')
+     */
+    public grantPut(identity?: iam.IPrincipal, objectsKeyPattern: any = '*') {
+        this.grant(identity, perms.BUCKET_PUT_ACTIONS, perms.KEY_WRITE_ACTIONS,
+            this.arnForObjects(objectsKeyPattern));
+    }
+
+    /**
+     * Grants s3:DeleteObject* permission to an IAM pricipal for objects
+     * in this bucket.
+     *
+     * @param identity The principal
+     * @param objectsKeyPattern Restrict the permission to a certain key pattern (default '*')
+     */
+    public grantDelete(identity?: iam.IPrincipal, objectsKeyPattern: any = '*') {
+        this.grant(identity, perms.BUCKET_DELETE_ACTIONS, [],
+            this.arnForObjects(objectsKeyPattern));
     }
 
     /**
@@ -180,21 +222,31 @@ export abstract class BucketRef extends cdk.Construct {
      *
      * If an encryption key is used, permission to use the key for
      * encrypt/decrypt will also be granted.
+     *
+     * @param identity The principal
+     * @param objectsKeyPattern Restrict the permission to a certain key pattern (default '*')
      */
     public grantReadWrite(identity?: iam.IPrincipal, objectsKeyPattern: any = '*') {
-        if (!identity) {
-            return;
-        }
         const bucketActions = perms.BUCKET_READ_ACTIONS.concat(perms.BUCKET_WRITE_ACTIONS);
         const keyActions = perms.KEY_READ_ACTIONS.concat(perms.KEY_WRITE_ACTIONS);
-        this.grant(identity, objectsKeyPattern, bucketActions, keyActions);
-    }
 
-    private grant(identity: iam.IPrincipal, objectsKeyPattern: any, bucketActions: string[], keyActions: string[]) {
-        const resources = [
+        this.grant(identity,
+            bucketActions,
+            keyActions,
             this.bucketArn,
-            this.arnForObjects(objectsKeyPattern)
-        ];
+            this.arnForObjects(objectsKeyPattern));
+    }
+
+    private grant(identity: iam.IPrincipal | undefined,
+                  bucketActions: string[],
+                  keyActions: string[],
+                  resource: cdk.Arn, ...otherResources: cdk.Arn[]) {
+
+        if (!identity) {
+            return;
+        }
+
+        const resources = [ resource, ...otherResources ];
 
         identity.addToPolicy(new cdk.PolicyStatement()
             .addResources(...resources)

diff --git a/packages/@aws-cdk/aws-s3/lib/perms.ts b/packages/@aws-cdk/aws-s3/lib/perms.ts
@@ -4,12 +4,20 @@ export const BUCKET_READ_ACTIONS = [
     's3:List*',
 ];
 
-export const BUCKET_WRITE_ACTIONS = [
+export const BUCKET_PUT_ACTIONS = [
     's3:PutObject*',
-    's3:DeleteObject*',
     's3:Abort*'
 ];
 
+export const BUCKET_DELETE_ACTIONS = [
+    's3:DeleteObject*'
+];
+
+export const BUCKET_WRITE_ACTIONS = [
+    ...BUCKET_DELETE_ACTIONS,
+    ...BUCKET_PUT_ACTIONS
+];
+
 export const KEY_READ_ACTIONS = [
     'kms:Decrypt',
     'kms:DescribeKey',

diff --git a/packages/@aws-cdk/aws-s3/test/integ.bucket.expected.json b/packages/@aws-cdk/aws-s3/test/integ.bucket.expected.json
@@ -114,8 +114,8 @@
                 "s3:GetObject*",
                 "s3:GetBucket*",
                 "s3:List*",
-                "s3:PutObject*",
                 "s3:DeleteObject*",
+                "s3:PutObject*",
                 "s3:Abort*"
               ],
               "Effect": "Allow",

diff --git a/packages/@aws-cdk/aws-s3/test/test.bucket.ts b/packages/@aws-cdk/aws-s3/test/test.bucket.ts
@@ -609,8 +609,8 @@ export = {
                               "s3:GetObject*",
                               "s3:GetBucket*",
                               "s3:List*",
-                              "s3:PutObject*",
                               "s3:DeleteObject*",
+                              "s3:PutObject*",
                               "s3:Abort*"
                             ],
                             "Effect": "Allow",
@@ -763,8 +763,8 @@ export = {
                               "s3:GetObject*",
                               "s3:GetBucket*",
                               "s3:List*",
-                              "s3:PutObject*",
                               "s3:DeleteObject*",
+                              "s3:PutObject*",
                               "s3:Abort*"
                             ],
                             "Effect": "Allow",
@@ -826,6 +826,26 @@ export = {
         },
     },
 
+    'more grants'(test: Test) {
+        const stack = new cdk.Stack();
+        const bucket = new s3.Bucket(stack, 'MyBucket', { encryption: s3.BucketEncryption.Kms });
+        const putter = new iam.User(stack, 'Putter');
+        const writer = new iam.User(stack, 'Writer');
+        const deleter = new iam.User(stack, 'Deleter');
+
+        bucket.grantPut(putter);
+        bucket.grantWrite(writer);
+        bucket.grantDelete(deleter);
+
+        const resources = stack.toCloudFormation().Resources;
+        const actions = (id: string) => resources[id].Properties.PolicyDocument.Statement[0].Action;
+
+        test.deepEqual(actions('WriterDefaultPolicyDC585BCE'), [ 's3:DeleteObject*', 's3:PutObject*', 's3:Abort*' ]);
+        test.deepEqual(actions('PutterDefaultPolicyAB138DD3'), [ 's3:PutObject*', 's3:Abort*' ]);
+        test.deepEqual(actions('DeleterDefaultPolicyCD33B8A0'), 's3:DeleteObject*');
+        test.done();
+    },
+
     'cross-stack permissions'(test: Test) {
         const stackA = new cdk.Stack();
         const bucketFromStackA = new s3.Bucket(stackA, 'MyBucket');