Skip to content

Conversation

@markcarroll
Copy link
Contributor

@markcarroll markcarroll commented Apr 24, 2019

Issue #255

When CustomDomainName set, the origin set on the API gateway CORS headers is the CloudFront
URL not the custom one that the caller will actually be coming from. This fix changes the CORS header
to be the custom URL

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

When CustomDomainName set, the origin set on the API gateway CORS headers is the CloudFront URL not the custom
one that the caller will actually be coming from. This fix changes the CORS header to be the custom URL
@markcarroll markcarroll changed the title #255 Fix bug where wrong origin set on API CORS policy Fix bug where wrong origin set on API CORS policy Apr 24, 2019
@echo-bravo-yahoo
Copy link
Contributor

Hey! Thanks for the pull request!

This solves most of the problem, but there's one additional wrinkle. When you specify a custom domain (e.g., 'foo.example.com'), the CFN stack creates both an A record for 'foo.example.com' and an A record for the subdomain 'www.foo.example.com'. See this part of the template.

The change in this PR would cause CORS to work in the case of the bare domain ('foo.example.com'), but fail on the 'www.foo.example.com' subdomain. I think a good compromise is to remove the part of the template that creates the 'www.foo.example.com' record by default, since customers can already specify the www subdomain at stack creation if they want 'www.foo.example.com'. Curious what @markcarroll and @a-tan think about this.

@a-tan
Copy link
Contributor

a-tan commented Apr 24, 2019

I think that’s fine and is the behavior that CORS enforces. The solution in the traditional world is that there’s a htaccess file that redirects root to www (or vice versa). In the static s3 website space, the equivalent solution would be a S3 bucket with a redirect rule from one to the other. We should document the solution.

@markcarroll
Copy link
Contributor Author

I actually removed the 'www.' line as well but was going to submit that separately. It actually solves another issue I was having where the CloufFront certificate was not working because it was trying to add a www. to the domain name and since this was already a three level domain, it didn't need it. I can submit that too if that helps.

@echo-bravo-yahoo
Copy link
Contributor

If you push that change to this branch (markcarrol:staging), it'll add it to this pull request, and I'll approve/merge both changes as a bundle :)

@markcarroll
Copy link
Contributor Author

I removed the www. in both places.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants