actix / actix-extras

A collection of additional crates supporting the actix and actix-web frameworks.
https://actix.rs
Apache License 2.0
762 stars 192 forks source link

Cors::default() prevents even same-origin requests. #378

Closed ramnivas closed 8 months ago

ramnivas commented 8 months ago

Expected Behavior

I use Cors::default() in some instances. I expected this to be sufficient to make same-origin requests work, but I see the "Origin is not allowed to make this request" failures. I noticed that the browser doesn't make any pre-flight requests, and I see the failure when the browser makes a POST request.

CORS Issue

Current Behavior

The POST request fails with the "Origin is not allowed to make this request" message.

Possible Solution

I am unsure, but should requests with Sec-Fetch-Site: same-origin be exempt from any CORS checking?

Steps to Reproduce (for bugs)

The easiest reproduction will be with Exograph. Please let me know if you prefer a standalone example, which will be slightly more involved to set up.

Context

I am a developer for Exograph, where the same Actix server serves the API and playground. By default, it uses Cors::default() to configure CORS.

Your Environment

robjtede commented 8 months ago

~CORS is only supposed to be used on cross-origin requests. You should avoid adding the Origin header, either manually, or through using mode: "cors" on your Fetch object.~

It might be enough to switch to Cors::default().block_on_origin_mismatch(false), but I think the most likely outcome of that is different CORS errors in your browser console relating to the lack of CORS response headers.

ramnivas commented 8 months ago

I am not adding the Origin header (the only headers I add are accept: "application/json, multipart/mixed", content-type: "application/json".

Per https://fetch.spec.whatwg.org/#http-requests, the browser will add the Origin header even when making a same-origin fetch unless it is a GET or HEAD request. In my case, it is a POST request, so per the spec, Origin is added by the browser. That same spec states: "It cannot be reliably identified as participating in the CORS protocol as the Origin header is also included for all requests whose method is neither GET nor HEAD".

robjtede commented 8 months ago

It cannot be reliably identified as participating in the CORS protocol as the Origin header

You're right. I'd forgotten that detail.

Can you confirm whether Cors::default().block_on_origin_mismatch(false) does or does not solve your issue?

There's a plan to make false the default on the next release anyway. If it solves this I'll get a release cut today.

ramnivas commented 8 months ago

That partially works. I tested these scenarios:

  1. same-origin, no allowed CORS domains list (my original issue): Works (no CORS errors)
  2. non-same-origin, no allowed CORS domain list: Works (I get the expected CORS error)
  3. non-same-origin, with the web apps's domain as allowed CORS domain list: Works (no CORS errors)

However, when I combine 1 and 3, it doesn't.

As the context: here is how I set up CORS. For this experiment, I changed the None => Cors::default() part to add .block_on_origin_mismatch(false).

ramnivas commented 8 months ago

I did more experiments, and here is what I observed:

This experiment suggests that CORS checking should not apply to methods other than GET and HEAD. Other methods will have a preflight request and checking that against CORS config should suffice.

I am less sure about POST, however, because:

robjtede commented 8 months ago

I've done some thorough testing of the scenarios presented and found that the behavior using Cors::default().block_on_origin_mismatch(false) seems to be expected.

I used 2 local domains, foo.local & bar.local, both over HTTPS to eliminate browser quirks. On page load, 6 fetch requests were sent out:

All the POST requests had JSON body to remove possibility of request being considered simple.

All the same-origin requests responded 200 and were readable by the browser despite the lack of CORS response headers. All the cross-origin requests responded 200 but shows as rejected in the network tools since they also lacked the necessary headers.

All requests were sent with mode: 'cors' and observed with Sec-Fetch-Mode: cors request headers. Only the same-site GET requests were observed with missing Origin headers.

Conclusion: same-origin requests work as expected with Cors::default().block_on_origin_mismatch(false).

Reproduction repo can be produced on request.

ramnivas commented 8 months ago

Thank you so much for fixing this issue and doing the hard work of testing all possible scenarios. I will upgrade to the new version as soon as it is available.

robjtede commented 8 months ago

It's out: https://crates.io/crates/actix-cors/0.7.0

ramnivas commented 8 months ago

On it (currently, upgrading to Rust 1.75, since that is a pre-requisite).

ramnivas commented 8 months ago

Upgraded and happy to report that everything is working as expected. Thank you!