aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.61k stars 3.9k forks source link

S3BucketOrigin.withOriginAccessControl: No Option to add ListBucket permission #31689

Open andyfase opened 1 week ago

andyfase commented 1 week ago

Describe the bug

The withOriginAccessControl method only has functionality to add GetObject, PutObject or DeleteObject permissions to the provided bucket resource policy. When using CloudFront to host a SPA app (Single Page App) its common to require to put a custom error response to translate HTTP 404 (page not found) to HTTP 200 responses, this is support deep linking within the SPA app.

To allow for this the S3 bucket must provide ListBucket permission to CloudFront, allowing CloudFront to identify the file doesnt exist and actually omit a HTTP 404. Currently this is not exposed via withOriginAccessControl and a user has no understand this and then add the permission manally to the bucket policy

Given the code for withOriginAccessControl is already modifiing the bucket resource policy it should be expected that it also handles this use case

Regression Issue

Last Known Working CDK Version

N/A

Expected Behavior

Bucket Policy has the ability to have ListBucket permissions granted to CloudFront

Current Behavior

Only GetObject permissions added to the /* resource ARN - ListBucket needs to be to the bucket resource not a Key resource

Reproduction Steps

use withOriginAccessControl and see that ListBucket permission cannot be added

Possible Solution

Expose functionality (extra prop) to withOriginAccessControl to allow for ListBucket permission adding

Additional Information/Context

N/A

CDK CLI Version

2.160.0

Framework Version

No response

Node.js Version

v20.14.0

OS

osx

Language

TypeScript

Language Version

No response

Other information

No response

hetvi20 commented 1 week ago

### This permission allows CloudFront to handle 404 errors and deliver custom error responses, which is essential for deep linking within the SPA.

public withOriginAccessControl(props?: { allowListBucket?: boolean }) { const bucketPolicy = { // already have permissions GetObject: true, PutObject: true, DeleteObject: true, };

if (props?.allowListBucket) { // Add ListBucket permission to the bucket resource policy bucketPolicy.ListBucket = true; }

this.updateBucketPolicy(bucketPolicy); } Once this adjustment is made, users will able to add the ListBucket permission directly through with OriginAccessControl, simplifying the process and avoiding need for manual modifications to the bucket policy.

khushail commented 1 week ago

@andyfase , thanks for reaching out.

Looks like this past issue is similar to the one you have created- https://github.com/aws/aws-cdk/issues/13983 . Could you please take a look and confirm the same?

Thanks.

andyfase commented 1 week ago

Hi @khushail

Yes that past issue is similar - but that is to for OAI access pattern vs the newer OAC.

I agree with the comments made in that issue that adding ListBucket should be "opt-in" to prevent the exposure of the contents of the bucket if a index.html isnt present (i.e. the developer should have to enable this).

khushail commented 1 week ago

thanks @andyfase for replying back. Keeping all the discussion in mind, I think it would be appropriate to convert this bug to Feature request and mark it as P2 , to be available for community as well as team contribution. Please feel free to comment/share feedback if you think otherwise.

@hetvi20 , thanks for commenting. If you would like to contribute a PR, please feel free to follow our Contribution guide and team would be happy to review your submission.

Thanks.

hetvi20 commented 1 week ago

@khushail I have go through it thank for information. Can you merge my pull request in this respo.

khushail commented 6 days ago

@hetvi20 , I don't see any pull request linked with this issue. Also did not find any PR in CDK Repo. Could you please share your PR and link with this issue?

Once a PR has been submitted, community-reviewers review it and provide their feedback in form of comments. Then its reviewed by cdk maintainers who give the final stamp of approval and then PR is merged. If you need any guidance/help from the community, please feel free to reach out at CDK.DEV community slack channel.

Let me know if you need any other information/help.