fernando-mc / serverless-finch

A Serverless Framework Plugin for Static Site Deployment
MIT License
457 stars 71 forks source link

Change the default CORS to allow http://*.amazonaws.com instead of HTTPS #105

Open shinglyu opened 4 years ago

shinglyu commented 4 years ago

This is a:

For feature requests or changes:

Current behavior (if any)

The default CORS setting is allowing PUT, POST and DELETE from https://*amazonaws.com

Proposed behavior

Change it from https://*amazonaws.com to http://*.amazonaws.com (no SSL)

Proposed implementation deatils (optional)

Justification

As far as I know, only EC2 exposes https://*.amazonaws.com URLs. But since we are using serverless, it's quite weird to host part of the website on EC2 and let it make CORS call to the S3 bucket, which hosts the other part of the static website. I do have a valid use case that needs HTTP, and I believe it's more common than the EC2 case: I have a static website hosted on S3. Then I want to upload files to the same S3 using a pre-signed PUT URL. So I build an API Gateway => Lambda REST endpoint that generates the pre-signed URL, then my frontend can use that URL to directly upload to S3. But since S3 only supports HTTP (no S) for static web hosting, the frontend page that makes the PUT request is most likely served under the URL http://mybucket.s3-website.eu-central-1.amazonaws.com/. So using HTTP make the whole flow complete.

fernando-mc commented 3 years ago

Sorry for the delay and thank you for the suggestion @shinglyu!

This use case makes sense to me simplifies the workflow for a lot of folks currently manually adding CORS config to a custom bucket that they're POSTing/PUTing files into with presigned URLs. Appreciate you brining this up.

As we would be changing the old default CORS policy I'd be open to this as part of a new breaking change version. If we do want to make an update to this I'd like to ask @linusmarco or some others folks to evaluate the default CORS policy to something more respectable. (If we're going to be making a breaking change here I'd rather do it once)

@shinglyu would you be open to taking this on as a PR?

fernando-mc commented 3 years ago

I think there's actually a related PR that I'd want to merge at the same time that could help address this too:

https://github.com/fernando-mc/serverless-finch/pull/103/files