From 64c3c6b782e43d82f5c94d8ee2cf80572d4688db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=F0=9F=91=A8=F0=9F=8F=BC=E2=80=8D=F0=9F=92=BB=20Romain=20M?= =?UTF-8?q?arcadier-Muller?= Date: Thu, 16 May 2019 00:03:22 +0200 Subject: [PATCH] fix(cloudfront): Use regional endpoint for S3 bucket origins The regional endpoint has to be used for S3 bucket origins, otherwise CloudFront will receive an HTTP 302 response (redirecting to the regional endpoint), which it will cache. This will lead to users seeing the actual bucket endpoint, instead of it being hidden behind the CloudFront distribution. --- .../aws-cloudfront/lib/web_distribution.ts | 4 +- ...nteg.cloudfront-alias-target.expected.json | 134 +++++++++--------- ...eg.cloudfront-bucket-logging.expected.json | 4 +- ...teg.cloudfront-ipv6-disabled.expected.json | 2 +- .../test/integ.cloudfront-s3.expected.json | 133 +++++++++++++++++ .../test/integ.cloudfront-s3.ts | 30 ++++ .../test/integ.cloudfront.expected.json | 4 +- .../aws-cloudfront/test/test.basic.ts | 6 +- 8 files changed, 240 insertions(+), 77 deletions(-) create mode 100644 packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-s3.expected.json create mode 100644 packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-s3.ts diff --git a/packages/@aws-cdk/aws-cloudfront/lib/web_distribution.ts b/packages/@aws-cdk/aws-cloudfront/lib/web_distribution.ts index 051c81f7e229f..ee59dd2f44c40 100644 --- a/packages/@aws-cdk/aws-cloudfront/lib/web_distribution.ts +++ b/packages/@aws-cdk/aws-cloudfront/lib/web_distribution.ts @@ -566,7 +566,7 @@ export class CloudFrontWebDistribution extends cdk.Construct implements route53. const originProperty: CfnDistribution.OriginProperty = { id: originId, domainName: originConfig.s3OriginSource - ? originConfig.s3OriginSource.s3BucketSource.bucketDomainName + ? originConfig.s3OriginSource.s3BucketSource.bucketRegionalDomainName : originConfig.customOriginSource!.domainName, originPath: originConfig.originPath, originCustomHeaders: originHeaders.length > 0 ? originHeaders : undefined, @@ -660,7 +660,7 @@ export class CloudFrontWebDistribution extends cdk.Construct implements route53. distributionConfig = { ...distributionConfig, logging: { - bucket: this.loggingBucket.bucketDomainName, + bucket: this.loggingBucket.bucketRegionalDomainName, includeCookies: props.loggingConfig.includeCookies || false, prefix: props.loggingConfig.prefix } diff --git a/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-alias-target.expected.json b/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-alias-target.expected.json index cc30641153dc3..28d030f8977ae 100644 --- a/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-alias-target.expected.json +++ b/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-alias-target.expected.json @@ -1,78 +1,78 @@ { - "Resources": { - "HostedZoneDB99F866": { - "Type": "AWS::Route53::HostedZone", - "Properties": { - "Name": "test.public." - } - }, - "HostedZoneAlias40D2E006": { - "Type": "AWS::Route53::RecordSet", - "Properties": { - "Name": "_foo.test.public.", - "Type": "A", - "AliasTarget": { - "DNSName": { - "Fn::GetAtt": [ - "MyDistributionCFDistributionDE147309", - "DomainName" - ] - }, - "HostedZoneId": "Z2FDTNDATAQYW2" + "Resources": { + "HostedZoneDB99F866": { + "Type": "AWS::Route53::HostedZone", + "Properties": { + "Name": "test.public." + } + }, + "HostedZoneAlias40D2E006": { + "Type": "AWS::Route53::RecordSet", + "Properties": { + "Name": "_foo.test.public.", + "Type": "A", + "AliasTarget": { + "DNSName": { + "Fn::GetAtt": [ + "MyDistributionCFDistributionDE147309", + "DomainName" + ] }, - "HostedZoneId": { - "Ref": "HostedZoneDB99F866" - } + "HostedZoneId": "Z2FDTNDATAQYW2" + }, + "HostedZoneId": { + "Ref": "HostedZoneDB99F866" } - }, - "Bucket83908E77": { - "Type": "AWS::S3::Bucket" - }, - "MyDistributionCFDistributionDE147309": { - "Type": "AWS::CloudFront::Distribution", - "Properties": { - "DistributionConfig": { - "CacheBehaviors": [], - "DefaultCacheBehavior": { - "AllowedMethods": [ - "GET", - "HEAD" - ], - "CachedMethods": [ - "GET", - "HEAD" - ], - "ForwardedValues": { - "Cookies": { - "Forward": "none" - }, - "QueryString": false + } + }, + "Bucket83908E77": { + "Type": "AWS::S3::Bucket" + }, + "MyDistributionCFDistributionDE147309": { + "Type": "AWS::CloudFront::Distribution", + "Properties": { + "DistributionConfig": { + "CacheBehaviors": [], + "DefaultCacheBehavior": { + "AllowedMethods": [ + "GET", + "HEAD" + ], + "CachedMethods": [ + "GET", + "HEAD" + ], + "ForwardedValues": { + "Cookies": { + "Forward": "none" }, - "TargetOriginId": "origin1", - "ViewerProtocolPolicy": "redirect-to-https" + "QueryString": false }, - "DefaultRootObject": "index.html", - "Enabled": true, - "HttpVersion": "http2", - "IPV6Enabled": true, - "Origins": [ - { - "DomainName": { - "Fn::GetAtt": [ - "Bucket83908E77", - "DomainName" - ] - }, - "Id": "origin1", - "S3OriginConfig": {} - } - ], - "PriceClass": "PriceClass_100", - "ViewerCertificate": { - "CloudFrontDefaultCertificate": true + "TargetOriginId": "origin1", + "ViewerProtocolPolicy": "redirect-to-https" + }, + "DefaultRootObject": "index.html", + "Enabled": true, + "HttpVersion": "http2", + "IPV6Enabled": true, + "Origins": [ + { + "DomainName": { + "Fn::GetAtt": [ + "Bucket83908E77", + "RegionalDomainName" + ] + }, + "Id": "origin1", + "S3OriginConfig": {} } + ], + "PriceClass": "PriceClass_100", + "ViewerCertificate": { + "CloudFrontDefaultCertificate": true } } } } } +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-bucket-logging.expected.json b/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-bucket-logging.expected.json index 7511e32c73122..aa89c33d039c6 100644 --- a/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-bucket-logging.expected.json +++ b/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-bucket-logging.expected.json @@ -34,7 +34,7 @@ "Bucket": { "Fn::GetAtt": [ "Bucket83908E77", - "DomainName" + "RegionalDomainName" ] }, "IncludeCookies": true, @@ -104,7 +104,7 @@ "Bucket": { "Fn::GetAtt": [ "AnAmazingWebsiteProbably2LoggingBucket222F7CE9", - "DomainName" + "RegionalDomainName" ] }, "IncludeCookies": false diff --git a/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-ipv6-disabled.expected.json b/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-ipv6-disabled.expected.json index 0bf449d269685..2ac63ff9337c8 100644 --- a/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-ipv6-disabled.expected.json +++ b/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-ipv6-disabled.expected.json @@ -35,7 +35,7 @@ "DomainName": { "Fn::GetAtt": [ "Bucket83908E77", - "DomainName" + "RegionalDomainName" ] }, "Id": "origin1", diff --git a/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-s3.expected.json b/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-s3.expected.json new file mode 100644 index 0000000000000..042b831d8cc76 --- /dev/null +++ b/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-s3.expected.json @@ -0,0 +1,133 @@ +{ + "Resources": { + "Bucket83908E77": { + "Type": "AWS::S3::Bucket" + }, + "BucketPolicyE9A3008A": { + "Type": "AWS::S3::BucketPolicy", + "Properties": { + "Bucket": { + "Ref": "Bucket83908E77" + }, + "PolicyDocument": { + "Statement": [ + { + "Action": [ + "s3:Get*", + "s3:List*" + ], + "Effect": "Allow", + "Principal": { + "CanonicalUser": { + "Fn::GetAtt": [ + "OAI", + "S3CanonicalUserId" + ] + } + }, + "Resource": [ + { + "Fn::GetAtt": [ + "Bucket83908E77", + "Arn" + ] + }, + { + "Fn::Join": [ + "", + [ + { + "Fn::GetAtt": [ + "Bucket83908E77", + "Arn" + ] + }, + "/*" + ] + ] + } + ] + } + ], + "Version": "2012-10-17" + } + } + }, + "OAI": { + "Type": "AWS::CloudFront::CloudFrontOriginAccessIdentity", + "Properties": { + "CloudFrontOriginAccessIdentityConfig": { + "Comment": "Allows CloudFront to reach to the bucket!" + } + } + }, + "DistributionCFDistribution882A7313": { + "Type": "AWS::CloudFront::Distribution", + "Properties": { + "DistributionConfig": { + "CacheBehaviors": [], + "DefaultCacheBehavior": { + "AllowedMethods": [ + "GET", + "HEAD" + ], + "CachedMethods": [ + "GET", + "HEAD" + ], + "ForwardedValues": { + "Cookies": { + "Forward": "none" + }, + "QueryString": false + }, + "TargetOriginId": "origin1", + "ViewerProtocolPolicy": "redirect-to-https" + }, + "DefaultRootObject": "index.html", + "Enabled": true, + "HttpVersion": "http2", + "IPV6Enabled": true, + "Origins": [ + { + "DomainName": { + "Fn::GetAtt": [ + "Bucket83908E77", + "RegionalDomainName" + ] + }, + "Id": "origin1", + "S3OriginConfig": { + "OriginAccessIdentity": { + "Fn::Join": [ + "", + [ + "origin-access-identity/cloudfront/", + { + "Ref": "OAI" + } + ] + ] + } + } + } + ], + "PriceClass": "PriceClass_100", + "ViewerCertificate": { + "CloudFrontDefaultCertificate": true + } + } + } + } + }, + "Outputs": { + "DistributionDomainName": { + "Value": { + "Fn::GetAtt": [ + "DistributionCFDistribution882A7313", + "DomainName" + ] + } + } + } +} diff --git a/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-s3.ts b/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-s3.ts new file mode 100644 index 0000000000000..194b021bf75a7 --- /dev/null +++ b/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront-s3.ts @@ -0,0 +1,30 @@ +import iam = require('@aws-cdk/aws-iam'); +import s3 = require('@aws-cdk/aws-s3'); +import cdk = require('@aws-cdk/cdk'); +import cloudfront = require('../lib'); + +const app = new cdk.App(); +const stack = new cdk.Stack(app, 'integ-cloudfront-s3'); + +const bucket = new s3.Bucket(stack, 'Bucket', { removalPolicy: cdk.RemovalPolicy.Destroy }); +const oai = new cloudfront.CfnCloudFrontOriginAccessIdentity(stack, 'OAI', { + cloudFrontOriginAccessIdentityConfig: { + comment: 'Allows CloudFront to reach to the bucket!', + } +}); +const dist = new cloudfront.CloudFrontWebDistribution(stack, 'Distribution', { + originConfigs: [{ + behaviors: [{ isDefaultBehavior: true }], + s3OriginSource: { + s3BucketSource: bucket, + originAccessIdentityId: oai.cloudFrontOriginAccessIdentityId, + }, + }] +}); +bucket.addToResourcePolicy(new iam.PolicyStatement() + .allow() + .addActions('s3:Get*', 's3:List*') + .addResources(bucket.bucketArn, bucket.arnForObjects('*')) + .addCanonicalUserPrincipal(oai.cloudFrontOriginAccessIdentityS3CanonicalUserId)); + +new cdk.CfnOutput(stack, 'DistributionDomainName', { value: dist.domainName }); diff --git a/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront.expected.json b/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront.expected.json index e4a24e472126b..afaf9f12a2178 100644 --- a/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront.expected.json +++ b/packages/@aws-cdk/aws-cloudfront/test/integ.cloudfront.expected.json @@ -35,7 +35,7 @@ "DomainName": { "Fn::GetAtt": [ "Bucket83908E77", - "DomainName" + "RegionalDomainName" ] }, "Id": "origin1", @@ -50,4 +50,4 @@ } } } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-cloudfront/test/test.basic.ts b/packages/@aws-cdk/aws-cloudfront/test/test.basic.ts index acb6d39b441b9..37fa731d04e03 100644 --- a/packages/@aws-cdk/aws-cloudfront/test/test.basic.ts +++ b/packages/@aws-cdk/aws-cloudfront/test/test.basic.ts @@ -130,7 +130,7 @@ export = { "DomainName": { "Fn::GetAtt": [ "Bucket83908E77", - "DomainName" + "RegionalDomainName" ] }, "Id": "origin1", @@ -205,7 +205,7 @@ export = { "DomainName": { "Fn::GetAtt": [ "Bucket83908E77", - "DomainName" + "RegionalDomainName" ] }, "Id": "origin1", @@ -283,7 +283,7 @@ export = { "DomainName": { "Fn::GetAtt": [ "Bucket83908E77", - "DomainName" + "RegionalDomainName" ] }, "Id": "origin1",