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

simple CORS #29

Open
2mol opened this issue Oct 13, 2019 · 4 comments
Open

simple CORS #29

2mol opened this issue Oct 13, 2019 · 4 comments

Comments

@2mol
Copy link

2mol commented Oct 13, 2019

Hi! Thanks for maintaining this package, it's very useful to me. I want to discuss the proposed change in #26.

The documentation for CorsResourcePolicy states that all simpleHeaders except for content-type are passed implicitly. If I use simpleCors, then my intention is to have Access-Control-Allow-Origin=*.

However, without content-type the preflight request (at least from Chrome and Firefox) isn't valid, and results in a 400 error.

From the documentation it's not clear to me what the intention is. Is this coming from the standard? We are referred to the w3c simple header list, which includes content-type.

However, for simpleCorsResourcePolicy it is explicitely mentioned that simple requests are not preceded by a preflight request.

So I might be confused, but I feel like adding simpleHeaders to simpleCorsResourcePolicy is the only thing that makes sense. Alternatively, what is the reason for content-type not being implicitely included in corsRequestHeaders?

@larskuhtz
Copy link
Owner

Unlike other simple headers, Content-Type is a simple header only for certain values:

A header is said to be a simple header if the header field name is an ASCII case-insensitive match for Accept, Accept-Language, or Content-Language or if it is an ASCII case-insensitive match for Content-Type and the header field value media type (excluding parameters) is an ASCII case-insensitive match for application/x-www-form-urlencoded, multipart/form-data, or text/plain.

This has consequences for the resource processing model. I am concerned that by explicitly adding the simple headers may lift this restriction. But I am not sure. Anybody who wants to take a deeper dive into the standard with regard to this is welcome.

@theobat
Copy link

theobat commented Jun 11, 2020

How about providing another value that would fit the very frequent use-case of supporting:

  • OPTIONS method (used in the preflight request in many cases)
  • ["Content-Type", "Authorization"] headers
-- Somehting like that maybe ?
-- | A preset version of cors with added OPTIONS method and Content-Type, Authorization headers
apiCors = simpleCorsResourcePolicy{
      corsOrigins = Nothing,
      corsMethods = simpleMethods <> ["OPTIONS"],
      corsRequestHeaders = simpleHeaders <> ["Content-Type", "Authorization"]
      }

Maybe "Authorization" is not frequent enough to justify this, but OPTIONS and Content-Type clearly are. As far as security is concerned, it can simply be stated that this is not a simpleCors policy anymore in the doc ?

@kamek-pf
Copy link

Just got bitten by this, providing a more permissive value out of the box would definitely help !

dhess added a commit to hackworthltd/primer that referenced this issue Jul 22, 2022
Notes:

* This policy was created using the following references:

https://stackoverflow.com/questions/63876281/cors-problem-while-making-a-fetch-call-from-ui-app-to-servant-server
https://stackoverflow.com/questions/41399055/haskell-yesod-cors-problems-with-browsers-options-requests-when-doing-post-req/56256513#56256513
larskuhtz/wai-cors#29 (comment)

We've also added `PUT` to the list of CORS methods recommended in the
last reference, as our API uses it.

* This policy does not specify any origins, which means its origins
are equivalent to "*". Therefore, this policy will not work with
credentialed requests. This is fine for now.

* This policy does not specify an `Access-Control-Max-Age` (via
wai-cors's `corsMaxAge` field), and therefore responses to browser
preflight requests will only be cached for 5 seconds. This is helpful
for testing, but is obviously not ideal for production use, so this
will need to be addressed if we decide to use CORS in production.
dhess added a commit to hackworthltd/primer that referenced this issue Jul 22, 2022
Notes:

* This policy was created using the following references:

https://stackoverflow.com/questions/63876281/cors-problem-while-making-a-fetch-call-from-ui-app-to-servant-server
https://stackoverflow.com/questions/41399055/haskell-yesod-cors-problems-with-browsers-options-requests-when-doing-post-req/56256513#56256513
larskuhtz/wai-cors#29 (comment)

We've also added `PUT` to the list of CORS methods recommended in the
last reference, as our API uses it.

* This policy does not specify any origins, which means its origins
are equivalent to "*". Therefore, this policy will not work with
credentialed requests. This is fine for now.

* This policy does not specify an `Access-Control-Max-Age` (via
wai-cors's `corsMaxAge` field), and therefore responses to browser
preflight requests will only be cached for 5 seconds. This is helpful
for testing, but is obviously not ideal for production use, so this
will need to be addressed if we decide to use CORS in production.
dhess added a commit to hackworthltd/primer that referenced this issue Jul 22, 2022
Notes:

* This policy was created using the following references:

https://stackoverflow.com/questions/63876281/cors-problem-while-making-a-fetch-call-from-ui-app-to-servant-server
https://stackoverflow.com/questions/41399055/haskell-yesod-cors-problems-with-browsers-options-requests-when-doing-post-req/56256513#56256513
larskuhtz/wai-cors#29 (comment)

We've also added `PUT` to the list of CORS methods recommended in the
last reference, as our API uses it.

* This policy does not specify any origins, which means its origins
are equivalent to "*". Therefore, this policy will not work with
credentialed requests. This is fine for now.

* This policy does not specify an `Access-Control-Max-Age` (via
wai-cors's `corsMaxAge` field), and therefore responses to browser
preflight requests will only be cached for 5 seconds. This is helpful
for testing, but is obviously not ideal for production use, so this
will need to be addressed if we decide to use CORS in production.
@goertzenator
Copy link

A documentation issue I think: The docs for simpleCorsResourcePolicy suggest that content-type is included, but it is clearly not.

I struggled to find the problem because Chrome was not showing me the body of the 400 response. I had to use Wireshark to pick up the body, and then the error message made things obvious.

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

No branches or pull requests

5 participants