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

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

Open
1 task
andyfase opened this issue Oct 7, 2024 · 8 comments · May be fixed by #32059
Open
1 task

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

andyfase opened this issue Oct 7, 2024 · 8 comments · May be fixed by #32059
Labels
@aws-cdk/aws-s3 Related to Amazon S3 effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2

Comments

@andyfase
Copy link

andyfase commented Oct 7, 2024

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

  • Select this option if this issue appears to be a regression.

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

@andyfase andyfase added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Oct 7, 2024
@github-actions github-actions bot added the @aws-cdk/aws-s3 Related to Amazon S3 label Oct 7, 2024
@khushail khushail added the needs-reproduction This issue needs reproduction. label Oct 7, 2024
@khushail khushail self-assigned this Oct 7, 2024
@khushail khushail added p2 and removed needs-triage This issue or PR still needs to be triaged. labels Oct 7, 2024
@hetvi20
Copy link

hetvi20 commented Oct 8, 2024

### 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 khushail added investigating This issue is being investigated and/or work is in progress to resolve the issue. and removed needs-reproduction This issue needs reproduction. labels Oct 8, 2024
@khushail
Copy link
Contributor

khushail commented Oct 8, 2024

@andyfase , thanks for reaching out.

Looks like this past issue is similar to the one you have created- #13983 .
Could you please take a look and confirm the same?

Thanks.

@khushail khushail added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed investigating This issue is being investigated and/or work is in progress to resolve the issue. labels Oct 8, 2024
@andyfase
Copy link
Author

andyfase commented Oct 8, 2024

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
Copy link
Contributor

khushail commented Oct 9, 2024

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.

@khushail khushail added feature-request A feature should be added or improved. effort/medium Medium work item – several days of effort and removed bug This issue is a bug. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Oct 9, 2024
@khushail khushail removed their assignment Oct 9, 2024
@hetvi20
Copy link

hetvi20 commented Oct 13, 2024

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

@khushail
Copy link
Contributor

@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.

@hetvi20
Copy link

hetvi20 commented Oct 22, 2024 via email

@khushail
Copy link
Contributor

@hetvi20 , this is comment with code. Let me clarify the process little bit more. You would need to submit a pull request with changes and integration tests, and link to this issue. Here is a sample PR -#31822

Once a PR is submitted, it would go through community-review and then core team's review. If you need any further help with creation of PR, here are few tutorials which could prove helpful-

  1. Contributing to AWS CDK
  2. Youtube tutorial

You could also reach out to Community for help and guidance on CDK.dev.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-s3 Related to Amazon S3 effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants