Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Prevent insecure CORS configurations #142

Merged
merged 5 commits into from
Jul 15, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,28 +28,29 @@ module YourApp

# ...

# Rails 3/4
# Rails 5

config.middleware.insert_before 0, "Rack::Cors" do
config.middleware.insert_before 0, Rack::Cors do
allow do
origins '*'
resource '*', :headers => :any, :methods => [:get, :post, :options]
end
end

# Rails 5

config.middleware.insert_before 0, Rack::Cors do
# Rails 3/4

config.middleware.insert_before 0, "Rack::Cors" do
allow do
origins '*'
resource '*', :headers => :any, :methods => [:get, :post, :options]
end
end

end
end
```
Refer to [rails 3 example](https://github.com/cyu/rack-cors/tree/master/examples/rails3) and [rails 4 example](https://github.com/cyu/rack-cors/tree/master/examples/rails4) for more details.

We use `insert_before` to make sure `Rack::Cors` runs at the beginning of the stack to make sure it isn't interfered with with other middleware (see `Rack::Cache` note in **Common Gotchas** section). Check out the [rails 4 example](https://github.com/cyu/rack-cors/tree/master/examples/rails4) and [rails 3 example](https://github.com/cyu/rack-cors/tree/master/examples/rails3).

See The [Rails Guide to Rack](http://guides.rubyonrails.org/rails_on_rack.html) for more details on rack middlewares or watch the [railscast](http://railscasts.com/episodes/151-rack-middleware).

Expand Down
13 changes: 11 additions & 2 deletions lib/rack/cors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -298,11 +298,20 @@ def resource_for_path(path)
end

class Resource
class CorsMisconfigurationError < StandardError
def message
"Allowing credentials for wildcard origins is insecure."\
" Please specify more restrictive origins or set 'credentials' to false in your CORS configuration."
end
end

attr_accessor :path, :methods, :headers, :expose, :max_age, :credentials, :pattern, :if_proc, :vary_headers

def initialize(public_resource, path, opts={})
raise CorsMisconfigurationError if public_resource && opts[:credentials]

self.path = path
self.credentials = opts[:credentials].nil? ? true : opts[:credentials]
self.credentials = opts[:credentials].nil? ? !public_resource : opts[:credentials]
self.max_age = opts[:max_age] || 1728000
self.pattern = compile(path)
self.if_proc = opts[:if]
Expand Down Expand Up @@ -354,7 +363,7 @@ def public_resource?
end

def origin_for_response_header(origin)
return '*' if public_resource? && !credentials
return '*' if public_resource?
origin
end

Expand Down
17 changes: 15 additions & 2 deletions test/unit/cors_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,11 @@ def load_app(name)
cors_request '/conditional', :origin => 'http://192.168.0.1:1234'
end

it "should not allow credentials for public resources" do
cors_request '/public'
last_response.headers['Access-Control-Allow-Credentials'].must_be_nil
end

describe 'logging' do
it 'should not log debug messages if debug option is false' do
app = mock
Expand Down Expand Up @@ -247,7 +252,7 @@ def load_app(name)
it 'should * origin should allow any origin' do
preflight_request('http://locohost:3000', '/public')
should_render_cors_success
last_response.headers['Access-Control-Allow-Origin'].must_equal 'http://locohost:3000'
last_response.headers['Access-Control-Allow-Origin'].must_equal '*'
end

it 'should * origin should allow any origin, and set * if no credentials required' do
Expand All @@ -269,6 +274,14 @@ def load_app(name)
end
end

describe "with insecure configuration" do
let(:app) { load_app('insecure') }

it "should raise an error" do
proc { cors_request '/public' }.must_raise Rack::Cors::Resource::CorsMisconfigurationError
end
end

describe "with non HTTP config" do
let(:app) { load_app("non_http") }

Expand Down Expand Up @@ -314,7 +327,7 @@ def app

it "should return original headers if in debug" do
cors_request origin: "http://example.net"
last_response.headers['X-Rack-CORS-Original-Access-Control-Allow-Origin'].must_equal "http://example.net"
last_response.headers['X-Rack-CORS-Original-Access-Control-Allow-Origin'].must_equal "*"
end
end

Expand Down
8 changes: 8 additions & 0 deletions test/unit/insecure.ru
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
require 'rack/cors'

use Rack::Cors do
allow do
origins '*'
resource '/public', credentials: true
end
end