cyu / rack-cors

Rack Middleware for handling Cross-Origin Resource Sharing (CORS), which makes cross-origin AJAX possible.
MIT License
3.26k stars 263 forks source link

Response code for unauthorized preflight request #190

Closed KevinBongart closed 4 years ago

KevinBongart commented 4 years ago

Hi, my Rails application has a test that ensures preflight requests from unauthorized hosts are rejected. Something vaguely like:

RSpec.describe 'CORS' do
  it 'rejects preflight from unauthorized hosts' do
    expect do
      integration_session.send :process, :options, '/',
        headers: {
          'Origin'                        => 'https://unauthorized.com',
          'Access-Control-Request-Method' => 'GET'
        }
    end.to raise_error(ActionController::RoutingError)
  end
end

When upgrading from rack-cors 0.4.3 to 1.0.0 (and later versions), I noticed that this test is failing because it now returns a 200 response with no headers.

What is the expected behavior of CORS when receiving a preflight request from an unauthorized host: respond with a 200 and no headers or reject the preflight request and respond with a 401 or 403 code?

KevinBongart commented 4 years ago

Doing a little more research, I couldn't find a CORS spec that authoritatively defines this scenario. This Stack Overflow answer seems somewhat opinionated, but agrees with the rack-cors behavior that preflight requests from not allowed origins should still return a 200 code with an empty list of options.

@cyu Can you confirm this is is correct? If so, I'll change my app spec, but otherwise I'm happy to contribute a PR for rack-cors.

cyu commented 4 years ago

@KevinBongart I think the absence of Access-Control-Allow-Origin is enough for browsers to determine that the request fail. A 200 status code makes sense to me. Having said that, I'll be happy to make any changes if the specs is clear in what the exact response should look like.

Your original test was actually testing a bug in the Rack::Cors where unauthorized requests were being propagated into the Rails stack, which would generate the routing error since there wasn't a route to service an OPTIONS request. That was changed to return a result immediately from Rack::Cors instead of letting it propagate.

KevinBongart commented 4 years ago

Thanks for the explanation!

According to this spec, it sounds like the correct behavior for this scenario is a success response code with empty headers (though this part of the spec is pretty difficult to read). Looks like rack-cors is behaving correctly, so I fixed the test in my app to confirm I receive a 200 with an empty header.