Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

aws-cloudfront: easily support Origin Access Identity for S3 buckets #941

Closed
leepa opened this issue Oct 16, 2018 · 12 comments · Fixed by #4491
Closed

aws-cloudfront: easily support Origin Access Identity for S3 buckets #941

leepa opened this issue Oct 16, 2018 · 12 comments · Fixed by #4491
Assignees
Labels
@aws-cdk/aws-cloudfront Related to Amazon CloudFront feature-request A feature should be added or improved.

Comments

@leepa
Copy link
Contributor

leepa commented Oct 16, 2018

Currently it doesn't seem possible to do this without creating a CloudFrontOriginAccessIdentityResource and then creating a 'Canonical User' policy fragment.

Here's the Java version:

        Bucket bucket = new Bucket(this, "Bucket", BucketProps.builder()
                .build());

        CloudFrontOriginAccessIdentityResource identityResource = new CloudFrontOriginAccessIdentityResource(this, "OAI", CloudFrontOriginAccessIdentityResourceProps.builder()
                .withCloudFrontOriginAccessIdentityConfig(new CloudFrontOriginAccessIdentityResource.CloudFrontOriginAccessIdentityConfigProperty.Builder()
                        .withComment("A comment")
                        .build()
                )
                .build());

        CloudFrontWebDistribution webDistribution = new CloudFrontWebDistribution(this, "CloudFront", CloudFrontWebDistributionProps.builder()
                .withViewerProtocolPolicy(ViewerProtocolPolicy.RedirectToHTTPS)
                .withPriceClass(PriceClass.PriceClass100)
                .withHttpVersion(HttpVersion.HTTP2)
                .withDefaultRootObject("")
                .withOriginConfigs(Collections.singletonList(
                        SourceConfiguration.builder()
                                .withBehaviors(Collections.singletonList(
                                        Behavior.builder()
                                                .withAllowedMethods(CloudFrontAllowedMethods.ALL)
                                                .withDefaultTtlSeconds(60)
                                                .withIsDefaultBehavior(true)
                                                .withForwardedValues(DistributionResource.ForwardedValuesProperty.builder()
                                                        .withCookies(DistributionResource.CookiesProperty.builder()
                                                                .withWhitelistedNames(Arrays.asList(
                                                                        "csrftoken",
                                                                        "sessionid",
                                                                        "messages"
                                                                ))
                                                                .withForward("whitelist")
                                                                .build())
                                                        .withQueryString(true)
                                                        .build())
                                                .build()
                                ))
                                .withS3OriginSource(S3OriginConfig.builder()
                                        .withS3BucketSource(bucket)
                                        .withOriginAccessIdentity(identityResource)
                                        .build())
                                .withOriginHeaders(new HashMap<String, String>() {{
                                    put("X-CloudFront-Forwarded-Proto", "https");
                                }})
                                .build()
                ))
                .build());

        class CanonicalUserPrincipal extends AccountPrincipal {
            private String canonicalUserId;

            private CanonicalUserPrincipal(String canonicalUserId) {
                this.canonicalUserId = canonicalUserId;
            }

            @Override
            public PrincipalPolicyFragment policyFragment() {
                return new PrincipalPolicyFragment(new HashMap<String,String>() {{
                    put("CanonicalUser", canonicalUserId);
                }});
            }
        }

        PolicyDocument document = new PolicyDocument();
        document.addStatement(new PolicyStatement()
                .addPrincipal(new CanonicalUserPrincipal(identityResource.getCloudFrontOriginAccessIdentityS3CanonicalUserId()))
                .addAction("s3:GetObject")
                .addResource("arn:aws:s3:::" + bucket.getBucketName() + "/*")
        );

        new BucketPolicyResource(this, "BucketPolicy", BucketPolicyResourceProps.builder()
                .withBucket(bucket.getBucketName())
                .withPolicyDocument(document)
                .build());
@rix0rrr
Copy link
Contributor

rix0rrr commented Oct 16, 2018

What does it do? Can you link us to some docs?

@mindstorms6
Copy link
Contributor

mindstorms6 commented Oct 16, 2018

Short version: the OAI is an IAM Principal that allows you to lock down the "source bucket" such that only CloudFront requests can hit your bucket (or anything else that policy allows)

It's somewhat common to want this - we should make it easier.

However, @leepa, we do already have a CanonicalUserPrincipal object:

https://github.com/awslabs/aws-cdk/blob/59964427939e0a5b5aebef2359bc418a4bd29236/packages/%40aws-cdk/aws-iam/lib/policy-document.ts#L115

You should be able to drop yours :)

I think we should make it a bit easier though to handle static websites :P

@leepa
Copy link
Contributor Author

leepa commented Oct 16, 2018

@mindstorms6 doh - I missed that one! No matter and easy to flip to it. I agree though, we should make it easier.

@rix0rrr
Copy link
Contributor

rix0rrr commented Oct 16, 2018

Oh I see. We generate an "identity" and register it with the distribution, and then we say that this identity can access the bucket.

Seems like create the IdentityResource and obtaining the CanonicalUser from it should be a one-step operation on the distribution. Agreed?

@leepa
Copy link
Contributor Author

leepa commented Oct 16, 2018

Can't the very act of creating an S3 Origin with an Identity 'do the right thing'? The identity just needs a comment so... .withOriginAccessIdentity(true) might be nicer? That is unless there's a specific reference, do the legwork? Just feels like a lot of boilerplate for best practice.

@rix0rrr
Copy link
Contributor

rix0rrr commented Oct 17, 2018

Agreed. Passing the S3 bucket should do all this work.

@debora-ito debora-ito added the @aws-cdk/aws-cloudfront Related to Amazon CloudFront label Nov 7, 2018
@srchase srchase added feature-request A feature should be added or improved. and removed enhancement labels Jan 3, 2019
@rix0rrr rix0rrr added the gap label Jan 4, 2019
@workeitel
Copy link
Contributor

I started a early version of L2 OAI construct and would appreciate some feedback: master...workeitel:cloudfront-oai

You would use it like:

const bucket = new s3.Bucket(stack, 'Bucket', { removalPolicy: cdk.RemovalPolicy.Destroy });
const oai = new cloudfront.CloudFrontOriginAccessIdentity(stack, 'OAI', {
  comment: 'Allows CloudFront to reach to the bucket!',
});

bucket.grantRead(oai);

const dist = new cloudfront.CloudFrontWebDistribution(stack, 'Distribution', {
  originConfigs: [{
    behaviors: [{ isDefaultBehavior: true }],
    s3OriginSource: {
      s3BucketSource: bucket,
      originAccessIdentity: oai,
    },
  }]
});

or import it:

const oai = cloudfront.CloudFrontOriginAccessIdentity.fromCloudFrontOriginAccessIdentityName(
  stack,
  'OAIImported',
  'AEFASDF234'
);

@leepa
Copy link
Contributor Author

leepa commented Jun 23, 2019

In an ideal world I had imagined something like:

const dist = new cloudfront.CloudFrontWebDistribution(stack, 'Distribution', {
  originConfigs: [{
    behaviors: [{ isDefaultBehavior: true }],
    s3OriginSource: {
      s3BucketSource: bucket,
      useOriginAccessIdentity: true,
    },
  }]
});

The comment could be auto-generated. Never really seen the comment used for anything - unless people use it to browse the console maybe? If so, I personally think it's better not to have another L2 for this - people use OAI by default if fronting S3 to avoid public buckets anyway.

@RomainMuller
Copy link
Contributor

@leepa - that's a very valid point, however ability to import an existing OAI might be required in certain environments however (places where application developers are not entitled to create new identities/permissions).

@dsandor
Copy link

dsandor commented Jul 26, 2019

@leepa @workeitel

Hi folks. I ran into this issue evaluating the CDK for use with deploying a basic static website to S3 and then creating the corresponding CloudFront Distribution. What I found is that I can get everything working and even create the OIA however the last option of granting the OIA read access to the bucket is where I fall flat. Here is the code I worked out. The last part where I am adding the bucket policy blows up:

Invalid principal in policy (Service: Amazon S3; Status Code: 400; Error Code: MalformedPolicy;

If I take out the policy statement at the bottom I get the CloudFront Dist with an OIA but the OIA cannot access the bucket. I clearly am picking the wrong attribute to pass as the ARN for the principal on the bucket policy. Any thoughts? If I get this working it is a nice work around until you have the CDK supporting the OIA out of the box.

export class StaticWebsiteStack extends cdk.Stack {
  constructor(scope: cdk.App, id: string, staticWebsiteConfig: IStaticWebsiteProps) {
    super(scope, id, undefined);

    const resourcePrefix = staticWebsiteConfig.resourcePrefix;
    const deploymentVersion = semver.inc(staticWebsiteConfig.deploymentVersion, 'patch') || '1.0.0';
    const originPath = deploymentVersion.replace(/\./g, '_');

    const sourceBucket = new Bucket(this, `S3BucketForWebsite`, {
      websiteIndexDocument: staticWebsiteConfig.indexDocument || 'index.html',
      bucketName: `${resourcePrefix}-website`,
    });

    new BucketDeployment(this, 'DeployWebsite', {
      source: Source.asset(staticWebsiteConfig.websiteDistPath),
      destinationBucket: sourceBucket,
      destinationKeyPrefix: originPath,
    });

    // See AWS-CDK Issue: https://github.com/aws/aws-cdk/issues/941
    const cloudFrontOia = new CfnCloudFrontOriginAccessIdentity(this, 'OIA', {
      cloudFrontOriginAccessIdentityConfig: {
        comment: `OIA for ${resourcePrefix} website.`
      }
    });

    let cloudFrontDistProps: CloudFrontWebDistributionProps = {
        originConfigs: [
          {
            s3OriginSource: {
              s3BucketSource: sourceBucket,
              originAccessIdentityId: cloudFrontOia.ref
            },
            behaviors: [ {isDefaultBehavior: true}],
            originPath: `/${originPath}`,
          }
        ],
        aliasConfiguration: {
          acmCertRef: staticWebsiteConfig.certificateArn,
          names: staticWebsiteConfig.domainNames || []
        }
      };

    new CloudFrontWebDistribution(this, `${resourcePrefix}-cloudfront`, cloudFrontDistProps);
    const policyStatement = new iam.PolicyStatement();
    policyStatement.addActions('s3:GetBucket*');
    policyStatement.addActions('s3:GetObject*');
    policyStatement.addActions('s3:List*');
    policyStatement.addAllResources();
    policyStatement.addArnPrincipal(cloudFrontOia.attrS3CanonicalUserId);

    sourceBucket.addToResourcePolicy(policyStatement);
  }
}

@dsandor
Copy link

dsandor commented Jul 26, 2019

FYI, I got this working by changing the policy statement I was creating to this:

    const policyStatement = new iam.PolicyStatement();
    policyStatement.addActions('s3:GetBucket*');
    policyStatement.addActions('s3:GetObject*');
    policyStatement.addActions('s3:List*');
    policyStatement.addResources(sourceBucket.bucketArn);
    policyStatement.addResources(`${sourceBucket.bucketArn}/*`);
    policyStatement.addCanonicalUserPrincipal(cloudFrontOia.attrS3CanonicalUserId);

    sourceBucket.addToResourcePolicy(policyStatement);

To match what the CDK was doing in other parts I added the bucketArn and the bucketArn with /*. I also changed the addArnPrincipal to addCanonicalUserPrincipal. This now properly creates a CloudFront distribution that has access to the bucket.

I created a class to make this easier called StaticWebsiteStack that lets you pass in some variables and it does all the magic needed to create the bucket, the Cloud Front distro and the bucket policy for a static website. It also supports versioning. The code is here with an example website (look in the /aws directory):
https://github.com/dsandor/cdk-static-website

@eladb eladb self-assigned this Aug 12, 2019
workeitel added a commit to workeitel/aws-cdk that referenced this issue Oct 13, 2019
Example usage:

```
const bucket = new s3.Bucket(stack, 'Bucket');
const oai = new cloudfront.CloudFrontOriginAccessIdentity(stack, 'OAI');

bucket.grantRead(oai); // must be granted explicitly

const dist = new cloudfront.CloudFrontWebDistribution(stack, 'Distribution', {
  originConfigs: [{
    behaviors: [{ isDefaultBehavior: true }],
    s3OriginSource: {
      s3BucketSource: bucket,
      originAccessIdentity: oai,
    },
  }]
});
```

It supports the old `originAccessIdentityId` as well:

```
const dist = new cloudfront.CloudFrontWebDistribution(stack, 'Distribution', {
  originConfigs: [{
    behaviors: [{ isDefaultBehavior: true }],
    s3OriginSource: {
      s3BucketSource: bucket,
      originAccessIdentityId: "...",  // or oai.name
    },
  }]
});
```

And importing Origin Access Identities:

```
const oai = cloudfront.CloudFrontOriginAccessIdentity.fromName(
  stack,
  'OAIImported',
  'AEFASDF234'
);
```

NOTE: while typically the S3CanonicalUserId is used in the policy this
value is not available after import so instead the CloudFront Arn is
constructed manually.

aws#941
workeitel added a commit to workeitel/aws-cdk that referenced this issue Oct 17, 2019
Example usage:

```
const bucket = new s3.Bucket(stack, 'Bucket');
const oai = new cloudfront.CloudFrontOriginAccessIdentity(stack, 'OAI');

bucket.grantRead(oai); // must be granted explicitly

const dist = new cloudfront.CloudFrontWebDistribution(stack, 'Distribution', {
  originConfigs: [{
    behaviors: [{ isDefaultBehavior: true }],
    s3OriginSource: {
      s3BucketSource: bucket,
      originAccessIdentity: oai,
    },
  }]
});
```

It supports the old `originAccessIdentityId` as well:

```
const dist = new cloudfront.CloudFrontWebDistribution(stack, 'Distribution', {
  originConfigs: [{
    behaviors: [{ isDefaultBehavior: true }],
    s3OriginSource: {
      s3BucketSource: bucket,
      originAccessIdentityId: "...",  // or oai.name
    },
  }]
});
```

And importing Origin Access Identities:

```
const oai = cloudfront.CloudFrontOriginAccessIdentity.fromName(
  stack,
  'OAIImported',
  'AEFASDF234'
);
```

NOTE: while typically the S3CanonicalUserId is used in the policy this
value is not available after import so instead the CloudFront Arn is
constructed manually.

aws#941
workeitel added a commit to workeitel/aws-cdk that referenced this issue Nov 3, 2019
Example usage:

```
const bucket = new s3.Bucket(stack, 'Bucket');
const oai = new cloudfront.CloudFrontOriginAccessIdentity(stack, 'OAI');

bucket.grantRead(oai); // must be granted explicitly

const dist = new cloudfront.CloudFrontWebDistribution(stack, 'Distribution', {
  originConfigs: [{
    behaviors: [{ isDefaultBehavior: true }],
    s3OriginSource: {
      s3BucketSource: bucket,
      originAccessIdentity: oai,
    },
  }]
});
```

It supports the old `originAccessIdentityId` as well:

```
const dist = new cloudfront.CloudFrontWebDistribution(stack, 'Distribution', {
  originConfigs: [{
    behaviors: [{ isDefaultBehavior: true }],
    s3OriginSource: {
      s3BucketSource: bucket,
      originAccessIdentityId: "...",  // or oai.name
    },
  }]
});
```

And importing Origin Access Identities:

```
const oai = cloudfront.CloudFrontOriginAccessIdentity.fromName(
  stack,
  'OAIImported',
  'AEFASDF234'
);
```

NOTE: while typically the S3CanonicalUserId is used in the policy this
value is not available after import so instead the CloudFront Arn is
constructed manually.

aws#941
workeitel added a commit to workeitel/aws-cdk that referenced this issue Dec 7, 2019
Example usage:

```
const bucket = new s3.Bucket(stack, 'Bucket');
const oai = new cloudfront.CloudFrontOriginAccessIdentity(stack, 'OAI');

const dist = new cloudfront.CloudFrontWebDistribution(stack, 'Distribution', {
  originConfigs: [{
    behaviors: [{ isDefaultBehavior: true }],
    s3OriginSource: {
      s3BucketSource: bucket,
      originAccessIdentity: oai,
    },
  }]
});
```

And importing Origin Access Identities:

```
const oaiImported = cloudfront.OriginAccessIdentity.fromOriginAccessIdentityName(
  stack,
  'OAIImported',
  'AEFASDF234'
);
```

NOTE: while typically the S3CanonicalUserId is used in the policy this
value is not available after import so instead the CloudFront Arn is
constructed manually.

aws#941
workeitel added a commit to workeitel/aws-cdk that referenced this issue Dec 7, 2019
Example usage:

```
const bucket = new s3.Bucket(stack, 'Bucket');
const oai = new cloudfront.CloudFrontOriginAccessIdentity(stack, 'OAI');

const dist = new cloudfront.CloudFrontWebDistribution(stack, 'Distribution', {
  originConfigs: [{
    behaviors: [{ isDefaultBehavior: true }],
    s3OriginSource: {
      s3BucketSource: bucket,
      originAccessIdentity: oai,
    },
  }]
});
```

And importing Origin Access Identities:

```
const oaiImported = cloudfront.OriginAccessIdentity.fromOriginAccessIdentityName(
  stack,
  'OAIImported',
  'AEFASDF234'
);
```

NOTE: while typically the S3CanonicalUserId is used in the policy this
value is not available after import so instead the CloudFront Arn is
constructed manually.

aws#941
@mergify mergify bot closed this as completed in #4491 Dec 12, 2019
mergify bot pushed a commit that referenced this issue Dec 12, 2019
Example usage:

```
const bucket = new s3.Bucket(stack, 'Bucket');
const oai = new cloudfront.CloudFrontOriginAccessIdentity(stack, 'OAI');

const dist = new cloudfront.CloudFrontWebDistribution(stack, 'Distribution', {
  originConfigs: [{
    behaviors: [{ isDefaultBehavior: true }],
    s3OriginSource: {
      s3BucketSource: bucket,
      originAccessIdentity: oai,
    },
  }]
});
```

And importing Origin Access Identities:

```
const oaiImported = cloudfront.OriginAccessIdentity.fromOriginAccessIdentityName(
  stack,
  'OAIImported',
  'AEFASDF234'
);
```

NOTE: while typically the S3CanonicalUserId is used in the policy this
value is not available after import so instead the CloudFront Arn is
constructed manually.

#941
ed-at-work pushed a commit to ed-at-work/aws-cdk that referenced this issue Dec 17, 2019
Example usage:

```
const bucket = new s3.Bucket(stack, 'Bucket');
const oai = new cloudfront.CloudFrontOriginAccessIdentity(stack, 'OAI');

const dist = new cloudfront.CloudFrontWebDistribution(stack, 'Distribution', {
  originConfigs: [{
    behaviors: [{ isDefaultBehavior: true }],
    s3OriginSource: {
      s3BucketSource: bucket,
      originAccessIdentity: oai,
    },
  }]
});
```

And importing Origin Access Identities:

```
const oaiImported = cloudfront.OriginAccessIdentity.fromOriginAccessIdentityName(
  stack,
  'OAIImported',
  'AEFASDF234'
);
```

NOTE: while typically the S3CanonicalUserId is used in the policy this
value is not available after import so instead the CloudFront Arn is
constructed manually.

aws#941
@wz2b
Copy link

wz2b commented Aug 6, 2023

I think the above answers are good. I made two small changes.

First, I added one additional policy at the top to disable unencrypted transport. To be honest, I'm not entirely sure it's necessary.

                {
                    "Effect": "Deny",
                    "Principal": {
                        "AWS": "*"
                    },
                    "Action": "s3:*",
                    "Resource": [
                        `${bucket.bucketArn}`,
                        `${bucket.bucketArn}/*`
                    ],
                    "Condition": {
                        "Bool": {
                            "aws:SecureTransport": "false"
                        }
                    }
                },

Second, when I did this i'm referring to the OIA as a principal:

"Principal": {
    "AWS": `arn:aws:iam::cloudfront:user/CloudFront Origin Access Identity ${oia.originAccessIdentityId}`
}

rather than using the "canonical user id." This works fine, but I'm wondering if one way is better than the other or if it's just a matter of style.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-cloudfront Related to Amazon CloudFront feature-request A feature should be added or improved.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants