fix(cloudfront-origins): policy not added for custom OAI (aws#18192)

Closes [**aws#18185**](aws#18185): When creating an `S3Origin` without including an existing `Origin Access Identity`, an `OriginAccessIdentity` is created and added to the bucket's resource policy. However, since the adding to the resource policy is inside of the `if (!this.originAccessIdentity)`closure, custom OAI's are not added to the bucket policy by default. Since using `bucket.grantRead` creates an overly permissive policy (as noted in the source code comments), adding the OAI to the bucket policy by default for both cases would create a more consistent result. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
TikiTDO · Feb 21, 2022 · 660dc49 · 660dc49
1 parent 12f1b47
commit 660dc49
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 10 deletions.
diff --git a/packages/@aws-cdk/aws-cloudfront-origins/lib/s3-origin.ts b/packages/@aws-cdk/aws-cloudfront-origins/lib/s3-origin.ts
@@ -73,17 +73,16 @@ class S3BucketOrigin extends cloudfront.OriginBase {
       this.originAccessIdentity = new cloudfront.OriginAccessIdentity(oaiScope, oaiId, {
         comment: `Identity for ${options.originId}`,
       });
-
-      // Used rather than `grantRead` because `grantRead` will grant overly-permissive policies.
-      // Only GetObject is needed to retrieve objects for the distribution.
-      // This also excludes KMS permissions; currently, OAI only supports SSE-S3 for buckets.
-      // Source: https://aws.amazon.com/blogs/networking-and-content-delivery/serving-sse-kms-encrypted-content-from-s3-using-cloudfront/
-      this.bucket.addToResourcePolicy(new iam.PolicyStatement({
-        resources: [this.bucket.arnForObjects('*')],
-        actions: ['s3:GetObject'],
-        principals: [this.originAccessIdentity.grantPrincipal],
-      }));
     }
+    // Used rather than `grantRead` because `grantRead` will grant overly-permissive policies.
+    // Only GetObject is needed to retrieve objects for the distribution.
+    // This also excludes KMS permissions; currently, OAI only supports SSE-S3 for buckets.
+    // Source: https://aws.amazon.com/blogs/networking-and-content-delivery/serving-sse-kms-encrypted-content-from-s3-using-cloudfront/
+    this.bucket.addToResourcePolicy(new iam.PolicyStatement({
+      resources: [this.bucket.arnForObjects('*')],
+      actions: ['s3:GetObject'],
+      principals: [this.originAccessIdentity.grantPrincipal],
+    }));
     return super.bind(scope, options);
   }
 

diff --git a/packages/@aws-cdk/aws-cloudfront-origins/test/integ.s3-origin-oai.expected.json b/packages/@aws-cdk/aws-cloudfront-origins/test/integ.s3-origin-oai.expected.json
@@ -5,6 +5,45 @@
       "UpdateReplacePolicy": "Retain",
       "DeletionPolicy": "Retain"
     },
+    "BucketPolicyE9A3008A": {
+      "Type": "AWS::S3::BucketPolicy",
+      "Properties": {
+        "Bucket": {
+          "Ref": "Bucket83908E77"
+        },
+        "PolicyDocument": {
+          "Statement": [
+            {
+              "Action": "s3:GetObject",
+              "Effect": "Allow",
+              "Principal": {
+                "CanonicalUser": {
+                  "Fn::GetAtt": [
+                    "OriginAccessIdentityDF1E3CAC",
+                    "S3CanonicalUserId"
+                  ]
+                }
+              },
+              "Resource": {
+                "Fn::Join": [
+                  "",
+                  [
+                    {
+                      "Fn::GetAtt": [
+                        "Bucket83908E77",
+                        "Arn"
+                      ]
+                    },
+                    "/*"
+                  ]
+                ]
+              }
+            }
+          ],
+          "Version": "2012-10-17"
+        }
+      }
+    },
     "OriginAccessIdentityDF1E3CAC": {
       "Type": "AWS::CloudFront::CloudFrontOriginAccessIdentity",
       "Properties": {

diff --git a/packages/@aws-cdk/aws-cloudfront-origins/test/s3-origin.test.ts b/packages/@aws-cdk/aws-cloudfront-origins/test/s3-origin.test.ts
@@ -82,6 +82,20 @@ describe('With bucket', () => {
         Comment: 'Identity for bucket provided by test',
       },
     });
+
+    expect(stack).toHaveResourceLike('AWS::S3::BucketPolicy', {
+      PolicyDocument: {
+        Statement: [{
+          Action: 's3:GetObject',
+          Principal: {
+            CanonicalUser: { 'Fn::GetAtt': ['OriginAccessIdentityDF1E3CAC', 'S3CanonicalUserId'] },
+          },
+          Resource: {
+            'Fn::Join': ['', [{ 'Fn::GetAtt': ['Bucket83908E77', 'Arn'] }, '/*']],
+          },
+        }],
+      },
+    });
   });
 
   test('creates an OriginAccessIdentity and grants read permissions on the bucket', () => {