Feature/cors allow credentials #464
Conversation
progress commit to support Access-Control-Allow-Credentials and Access-Control-Expose-Headers
Set allow credentials output to "'true'" Added additional tests for the add_cors method
|
Still needs translator input/output tests |
…s and Expose-Headers CORS implementation.
| "application/json": "{}\n" | ||
| }, | ||
| "responseParameters": { | ||
| "method.response.header.Access-Control-Allow-Origin": "'*'", |
There was a problem hiding this comment.
This is invalid output. A-C-A-O isn't allowed to have the wildcard value when A-C-A-C is set.
There was a problem hiding this comment.
Good catch! @cast could you add some logic to throw InvalidDocument when a user tries to specify Allow-Credentials: true and Allow-Origin: *?
|
This is great! Could you add validation for the Credentials+Wildcard scenario? |
…dcard value ("'*'") now throws InvalidDocument
|
As I mentioned in the issue for this PR, I think that adding
Unless something has changed, SAM can't modify the headers of responses that make it through API Gateway and into Lambda. Is that still the case? |
|
Thanks @brettstack ! Will the error message still fit in the cloudformation UI/api before being truncated? The declarative part is at the end so if it is truncated people might not know what's going on. The documentation mentions that AllowedOrigin is a required field, while it is in fact implicitly set to to "'*'" when no explicit configuration is used. |
|
Thanks for raising this @chrisoverzero. After reviewing the spec and other documentation more thoroughly it does appear |
|
@cast if you can provide a repro/use-case for including this in preflight I can take another look. Otherwise the |
|
Thanks @chrisoverzero for the spec review. Apart from a now fixed side effect leading me to think the allowed exposed headers was an issue in our internal development I added it mainly because other api management solutions often allow or force to take a liberal approach to preflight CORS (ApiMan, Axway,...). I'm not aware of specific browsers and browser versions that require the exposed header in the preflight response. I'm assuming if there is an issue it would be in old versions due to the current living standard. I'll remove the exposed headers parts. |
cast
changed the title
|
Any updates? This is really helpful for our project : ) |
Relates to issue #463
Added support for CORS headers Access-Control-Expose-Headers and Access-Control-Allow-Credentials in the CORS Configuration block at the Serverless::Api resource.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.