search

aws-samples / cloudfront-authorization-at-edge

Protect downloads of your content hosted on CloudFront with Cognito authentication using cookies and Lambda@Edge
https://aws.amazon.com/blogs/networking-and-content-delivery/authorizationedge-using-cookies-protect-your-amazon-cloudfront-content-from-being-downloaded-by-unauthenticated-users/
MIT No Attribution
461 stars 157 forks source link

Update template.yaml to use BucketPolicy #258

Closed sambler closed 4 months ago

sambler commented 4 months ago

Replace legacy S3 AccessControl with a bucket policy.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

sambler commented 4 months ago

ignore the noise, there is already a bucket policy in place that works, so bring this down to removing the legacy AccessControl

ottokruse commented 4 months ago

Sure, thanks. What's the benefit of not specifying private? Just cleaner (fine reason) as ACLs are deprecated, or will there be an actual effect too?

sambler commented 4 months ago

At the time I got a fail to create bucket, needed to remove it to create the stack.

A quick test of only a bucket works with sam and cfn, so it may have been a bucket policy I added that conflicted with the Accesscontrol.

ottokruse commented 4 months ago

Okay sure. And what's the benefit of removing AccessControl=Private now from the template? What will that improve?

I think it's a good idea because that property is deprecated but just wondering if it has a tangible benefit besides that.

sambler commented 4 months ago

Turns out it would only make it easier to add other bucket policies, so not directly effecting this stack now.

I only submitted because I thought having the AccessControl was preventing the stack create, which I found to be a conflict with my modifications.

ottokruse commented 4 months ago

Aight, well let's get rid of a deprecated property 🤘🏻