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

CORS broken for browsers that do preflight checks #100

Closed
PollyP opened this issue Jan 25, 2017 · 5 comments
Closed

CORS broken for browsers that do preflight checks #100

PollyP opened this issue Jan 25, 2017 · 5 comments
Assignees

Comments

@PollyP
Copy link
Contributor

PollyP commented Jan 25, 2017

I'm having trouble with cross origin requests. On my host, I'm running a python server that serves my application on port 8000, plus a vagrant instance of phantom of the capitol on port 9292. Before running rackup I edited config/phantom-dc_config.rb and phantom-dc_config.rb.example to set this line:

CORS_ALLOWED_DOMAINS = 'http://localhost:8000'

Curl requests work fine, but if I run a browser that does a cross-origin preflight check (e.g. OPTIONS on Chrome) then it fails with a HTTP status code of 405, "Method not allowed." My outgoing request's Origin matches what I've set in the config file. I tried setting and exporting CORS_ALLOWED_DOMAINS in the environment, but I get the same result.

Does anyone have any thoughts about what I am doing wrong here?

@PollyP PollyP changed the title problem configuring CORS CORS broken for browsers that do preflight checks Jan 27, 2017
@PollyP
Copy link
Contributor Author

PollyP commented Jan 27, 2017

[Rewrote issue title for clarity]

I have a workaround for this, which I got from @padrino (https://github.com/padrino/padrino-framework/wiki/Cross-domain-requests,-Access-Control-Allow-Origin) and @cyu (https://github.com/cyu/rack-cors)

After cloning the phnatom repo, I edited the Gemfile to add this line:
gem 'rack-cors'

And edited config.ru to add this:
use Rack::Cors do allow do origins '*' resource '/retrieve-form-elements', :headers => :any, :methods => [:post, :options] end end

And now the preflight check from Chrome works and I can actually do the post.

Obviously you need to edit origins for your real allowed origins, and add the other apis as allowed resources. And a good solution would get the origins from the CORS_ALLOWED_DOMAINS variable.

I would be happy to put together a PR to really fix, if I could get a volunteer to guide me a bit, as I don't know a thing about Rails/Ruby/etc.

@wioux wioux self-assigned this Jan 27, 2017
@wioux
Copy link
Member

wioux commented Jan 27, 2017

@PollyP CORS_ALLOWED_DOMAINS should actually be an array, and its values can be given in the environment variable CORS_ALLOWED_DOMAINS as a space separated list.

You can include * in the list as well to allow all origins. See main.rb#L9-L15:

  before do
    if CORS_ALLOWED_DOMAINS.include? request.env['HTTP_ORIGIN'] or CORS_ALLOWED_DOMAINS.include? "*"
      response.headers['Access-Control-Allow-Origin'] = request.env['HTTP_ORIGIN']
      response.headers['Access-Control-Allow-Credentials'] = "true"
    end
    response.headers['X-Backend-Hostname'] = Socket.gethostname
  end

@PollyP
Copy link
Contributor Author

PollyP commented Jan 27, 2017

@wioux I stuck a log warning in there to track down the problem, and this code was never getting called. The server filtered out the http options method before it ever hit this code.

@wioux
Copy link
Member

wioux commented Jan 28, 2017

Ah! Ok, thanks for pointing that out. It seems appropriate to include the rack-cors gem as you suggest.

It looks like you can pattern match all resources with *, instead of having to specify /retrieve-form-elements. That would be useful since this issue should crop up with many of our routes (e.g. those under the debug/cwc controllers as well).

I would put the configuration in phantom-dc_config.rb.example. There, you may have to call Padrino.use instead of the global function.

PollyP added a commit to PollyP/phantom-of-the-capitol that referenced this issue Jan 28, 2017
@PollyP PollyP mentioned this issue Jan 28, 2017
@PollyP
Copy link
Contributor Author

PollyP commented Jan 28, 2017

Hi, My PR is failing the Travis build here: https://travis-ci.org/EFForg/phantom-of-the-capitol/builds/196075289. It is not liking the addition to the Gemfile. Is there something special I need to do to make Travis happy?

UPDATE: Ah, yes, the Gemfile.lock needs to be committed. The Travis build works now.

@wioux Thanks for your advice above.

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

2 participants