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

CORS response headers missing from OPTIONS request in rack app #214

Closed johnknapp closed 3 years ago

johnknapp commented 3 years ago

I've verified my cors setup and read through other issues in this repo. Not sure what else to investigate and welcome your advice.

My environment:

Problem statement:

Etcetera:

cyu commented 3 years ago

@johnknapp bear with me, I've never worked with Roda before.

Can you show me how you're setting up that middleware along side the Roda app?

johnknapp commented 3 years ago

Yes Calvin, I'll provide more detail. I've been in touch with roda author on this and he agrees it is an odd one. My aim is an example to reproduce the problem. -JK

johnknapp commented 3 years ago

Alright @cyu here is a public repo with a minimal rack-cors / roda test app which replicates the problem reported. The readme in that repo explains the testing methodology.

I hope you can help resolve this issue!

With a cc to @jeremyevans, the author of roda.

jeremyevans commented 3 years ago

Looking at the test Roda application, the pizza stuff is ignored (hash_branch is called with a symbol, not a string). Additionally, the json plugin doesn't seem to be used, so even if the pizza branch was taken, it would probably result in an exception being raised.

Does it reproduce with the following config.ru? (this is the rough equivalent of what the example Roda app does, just with fewer headers)

require 'rack/cors'

  use Rack::Cors do
    allowed_methods = %i[get post put delete options head]
    allow do
      origins 'https://rack-cors-roda.herokuapp.com/'
      resource '*', headers: :any, methods: allowed_methods
    end
  end

s = 'hello rack-cors'
run(proc do |env|
  if env['PATH_INFO'] == '/'
    [200, {'Content-Length'=>s.length}, [s.dup]]
  else
    [404, {'Content-Length'=>0}, ['']]
  end
end)
johnknapp commented 3 years ago

Thank you @jeremyevans for this pure rack test.

A preflight curl test as documented in the readme still exhibits this problem! (Pure rack with puma, roda gem has been removed.)

You can see what I tested on heroku in this branch of my rack-cors test repo. Note the change in Gemfile, Gemfile.lock, config.ru, and README.md.

Let's see what @cyu has to say about this pure rack test.

johnknapp commented 3 years ago

UPDATE:

  1. I did a heroku test of the pure rack example. This curl preflight response had cors headers.
  2. Noticing the example used webrick vs puma and rack-cors1.1.0 vs 1.1.1, I made added puma and rolled rack-cors back to 1.1.0 in my config.ru test from @jeremyevans. This curl preflight response did not have cors headers.

Perhaps @cyu can point out the misconfiguration in our config.ru as seen in our repository which, running on heroku and testing with curl, returns no response headers to a cors preflight test as documented in the readme of that branch of that repo.

johnknapp commented 3 years ago

I added debug: true and logged results on heroku:

Dec 18 13:41:04 rack-cors-roda heroku/router at=info method=OPTIONS path="/" host=rack-cors-roda.herokuapp.com request_id=699b502a-2707-4ac9-80be-a051ed456495 fwd="<ip addy snipped>" dyno=web.1 connect=1ms service=16ms status=200 bytes=71 protocol=https
Dec 18 13:41:04 rack-cors-roda app/web.1 D, [2020-12-18T21:41:04.430276 #4] DEBUG -- : Incoming Headers:
Dec 18 13:41:04 rack-cors-roda app/web.1   Origin: https://random-sample.herokuapp.com
Dec 18 13:41:04 rack-cors-roda app/web.1   Path-Info: /
Dec 18 13:41:04 rack-cors-roda app/web.1   Access-Control-Request-Method: POST
Dec 18 13:41:04 rack-cors-roda app/web.1   Access-Control-Request-Headers: X-Requested-With
Dec 18 13:41:04 rack-cors-roda app/web.1 D, [2020-12-18T21:41:04.430440 #4] DEBUG -- : Preflight Headers:

To be clear, there were no further log entries after this!

johnknapp commented 3 years ago

I switched to a wildcard cors origin and got preflight response headers:

Dec 18 13:50:41 rack-cors-roda heroku/router at=info method=OPTIONS path="/" host=rack-cors-roda.herokuapp.com request_id=ba854b6e-1bbe-42a2-afc4-27b0dcc88157 fwd="73.223.174.252" dyno=web.1 connect=0ms service=7ms status=200 bytes=280 protocol=https
Dec 18 13:50:41 rack-cors-roda app/web.1 D, [2020-12-18T21:50:40.900613 #4] DEBUG -- : Incoming Headers:
Dec 18 13:50:41 rack-cors-roda app/web.1   Origin: https://random-sample.herokuapp.com
Dec 18 13:50:41 rack-cors-roda app/web.1   Path-Info: /
Dec 18 13:50:41 rack-cors-roda app/web.1   Access-Control-Request-Method: POST
Dec 18 13:50:41 rack-cors-roda app/web.1   Access-Control-Request-Headers: X-Requested-With
Dec 18 13:50:41 rack-cors-roda app/web.1 D, [2020-12-18T21:50:40.900801 #4] DEBUG -- : Preflight Headers:
Dec 18 13:50:41 rack-cors-roda app/web.1   Access-Control-Allow-Origin: *
Dec 18 13:50:41 rack-cors-roda app/web.1   Access-Control-Allow-Methods: GET, POST, PUT, DELETE, OPTIONS, HEAD
Dec 18 13:50:41 rack-cors-roda app/web.1   Access-Control-Expose-Headers: 
Dec 18 13:50:41 rack-cors-roda app/web.1   Access-Control-Max-Age: 0
Dec 18 13:50:41 rack-cors-roda app/web.1   Access-Control-Allow-Headers: X-Requested-With

Inspired, I went back to my production roda app and set a wildcard origin. A POST request from my javascript front end (svelte app using fetch) received expected response headers from the preflight request. Which unsurprisingly failed thus:

Response to preflight request doesn't pass access control check: The value of the 'Access-Control-Allow-Origin' header in the response must not be the wildcard '*' when the request's credentials mode is 'include'.

Then I removed `credentials: 'include, mode: 'cors' from my JS fetch POST and achieved a successful cross platform POST to my back end. Naturally, my back-end sent cookies back to the browser.

But of course with a wildcard cors origin, the browser will not accept cookies. That is a show stopper for us.

Summary of the current situation:

The solution we need is simple:

cyu commented 3 years ago

@johnknapp try removing the trailing '/' from the origin.

johnknapp commented 3 years ago

To @cyu:

What else?

cyu commented 3 years ago

@johnknapp I just tested and that works. I think your test is invalid. Using your example test:

johnknapp commented 3 years ago

@cyu Thank you Calvin! I would like to leave this open for the moment until I do a bit more testing tomorrow. I confirmed my config.ru works without slash but the typical roda approach of configuring rack middlewares within the app subclass of Roda is failing still. It will probably turn out rack-cors needs to install within config.ru instead of the common roda approach.

To help other roda users, I'll confirm this and comment here before closing.

cyu commented 3 years ago

@johnknapp when I tested I did test with both the use command in config.ru and also inside the Roda subclass, and in both cases it worked.

johnknapp commented 3 years ago

Confirmed, thank you again @cyu. I updated my rack-cors + roda repository to reflect reality and added notes which may help others in the future. (With a cc to @jeremyevans, thanks to you too!)