fix(cloudtrail): include s3KeyPrefix in bucket policy resource (#7053)

Fixes: #6741
aws · Mar 31, 2020 · b49881f · b49881f
1 parent 497f63e
commit b49881f
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 20 deletions.
diff --git a/packages/@aws-cdk/aws-cloudtrail/lib/index.ts b/packages/@aws-cdk/aws-cloudtrail/lib/index.ts
@@ -142,22 +142,24 @@ export class Trail extends Resource {
 
     const cloudTrailPrincipal = new iam.ServicePrincipal("cloudtrail.amazonaws.com");
 
-    this.s3bucket = props.bucket || new s3.Bucket(this, 'S3', {encryption: s3.BucketEncryption.UNENCRYPTED});
+    this.s3bucket = props.bucket || new s3.Bucket(this, 'S3', { encryption: s3.BucketEncryption.UNENCRYPTED });
 
     this.s3bucket.addToResourcePolicy(new iam.PolicyStatement({
-        resources: [this.s3bucket.bucketArn],
-        actions: ['s3:GetBucketAcl'],
-        principals: [cloudTrailPrincipal],
-      }));
+      resources: [this.s3bucket.bucketArn],
+      actions: ['s3:GetBucketAcl'],
+      principals: [cloudTrailPrincipal],
+    }));
 
     this.s3bucket.addToResourcePolicy(new iam.PolicyStatement({
-        resources: [this.s3bucket.arnForObjects(`AWSLogs/${Stack.of(this).account}/*`)],
-        actions: ["s3:PutObject"],
-        principals: [cloudTrailPrincipal],
-        conditions:  {
-          StringEquals: {'s3:x-amz-acl': "bucket-owner-full-control"}
-        }
-      }));
+      resources: [this.s3bucket.arnForObjects(
+        `${props.s3KeyPrefix ? `${props.s3KeyPrefix}/` : ''}AWSLogs/${Stack.of(this).account}/*`
+      )],
+      actions: ["s3:PutObject"],
+      principals: [cloudTrailPrincipal],
+      conditions: {
+        StringEquals: { 's3:x-amz-acl': "bucket-owner-full-control" }
+      }
+    }));
 
     let logGroup: logs.CfnLogGroup | undefined;
     let logsRole: iam.IRole | undefined;
@@ -176,7 +178,7 @@ export class Trail extends Resource {
     }
 
     if (props.managementEvents) {
-      const managementEvent =  {
+      const managementEvent = {
         includeManagementEvents: true,
         readWriteType: props.managementEvents
       };
@@ -190,7 +192,7 @@ export class Trail extends Resource {
       isMultiRegionTrail: props.isMultiRegionTrail == null ? true : props.isMultiRegionTrail,
       includeGlobalServiceEvents: props.includeGlobalServiceEvents == null ? true : props.includeGlobalServiceEvents,
       trailName: this.physicalName,
-      kmsKeyId:  props.kmsKey && props.kmsKey.keyArn,
+      kmsKeyId: props.kmsKey && props.kmsKey.keyArn,
       s3BucketName: this.s3bucket.bucketName,
       s3KeyPrefix: props.s3KeyPrefix,
       cloudWatchLogsLogGroupArn: logGroup && logGroup.attrArn,

diff --git a/packages/@aws-cdk/aws-cloudtrail/test/test.cloudtrail.ts b/packages/@aws-cdk/aws-cloudtrail/test/test.cloudtrail.ts
@@ -75,20 +75,20 @@ export = {
       const cloudTrailPrincipal = new iam.ServicePrincipal("cloudtrail.amazonaws.com");
       Trailbucket.addToResourcePolicy(new iam.PolicyStatement({
         resources: [Trailbucket.bucketArn],
-         actions: ['s3:GetBucketAcl'],
-         principals: [cloudTrailPrincipal],
-       }));
+        actions: ['s3:GetBucketAcl'],
+        principals: [cloudTrailPrincipal],
+      }));
 
       Trailbucket.addToResourcePolicy(new iam.PolicyStatement({
         resources: [Trailbucket.arnForObjects(`AWSLogs/${Stack.of(stack).account}/*`)],
         actions: ["s3:PutObject"],
         principals: [cloudTrailPrincipal],
-        conditions:  {
-          StringEquals: {'s3:x-amz-acl': "bucket-owner-full-control"}
+        conditions: {
+          StringEquals: { 's3:x-amz-acl': "bucket-owner-full-control" }
         }
       }));
 
-      new Trail(stack, 'Trail', {bucket: Trailbucket});
+      new Trail(stack, 'Trail', { bucket: Trailbucket });
 
       expect(stack).to(haveResource("AWS::CloudTrail::Trail"));
       expect(stack).to(haveResource("AWS::S3::Bucket"));
@@ -112,6 +112,50 @@ export = {
       test.done();
     },
 
+    'with s3KeyPrefix'(test: Test) {
+      // GIVEN
+      const stack = getTestStack();
+
+      // WHEN
+      new Trail(stack, 'Trail', { s3KeyPrefix: 'someprefix' });
+
+      expect(stack).to(haveResource("AWS::CloudTrail::Trail"));
+      expect(stack).to(haveResource("AWS::S3::Bucket"));
+      expect(stack).to(haveResource('AWS::S3::BucketPolicy', {
+        Bucket: { Ref: 'TrailS30071F172' },
+        PolicyDocument: {
+          Statement: [
+            {
+              Action: 's3:GetBucketAcl',
+              Effect: 'Allow',
+              Principal: { Service: 'cloudtrail.amazonaws.com' },
+              Resource: { 'Fn::GetAtt': ['TrailS30071F172', 'Arn'] }
+            },
+            {
+              Action: 's3:PutObject',
+              Condition: {
+                StringEquals: { 's3:x-amz-acl': 'bucket-owner-full-control' }
+              },
+              Effect: 'Allow',
+              Principal: { Service: 'cloudtrail.amazonaws.com' },
+              Resource: {
+                'Fn::Join': [
+                  '',
+                  [
+                    { 'Fn::GetAtt': ['TrailS30071F172', 'Arn'] },
+                    '/someprefix/AWSLogs/123456789012/*'
+                  ]
+                ]
+              }
+            }
+          ],
+          Version: '2012-10-17'
+        }
+      }));
+
+      test.done();
+    },
+
     'with cloud watch logs': {
       'enabled'(test: Test) {
         const stack = getTestStack();