From ec5522e246ae3b18ed6b305d4644ddef885ad1fe Mon Sep 17 00:00:00 2001 From: Calvin Yu Date: Tue, 13 Jun 2017 07:39:55 -0400 Subject: [PATCH 1/5] Move Rails 5 example ahead of Rails 3/4 example And small updates to Rails configuration --- README.md | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index f1ca9ca..83844e8 100644 --- a/README.md +++ b/README.md @@ -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). From 1cb59ce7864f65e94f806dd196e92adec498cf20 Mon Sep 17 00:00:00 2001 From: Peter Retzlaff Date: Thu, 13 Jul 2017 17:32:21 +0200 Subject: [PATCH 2/5] Prevent insecure configuration. * Don't mirror origin when Access-Control-Allow-Credentials is true. * Prevent users from enabling wildcard origins with credentials. --- lib/rack/cors.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/rack/cors.rb b/lib/rack/cors.rb index 914ebec..f8f7c96 100644 --- a/lib/rack/cors.rb +++ b/lib/rack/cors.rb @@ -298,11 +298,14 @@ def resource_for_path(path) end class Resource + class CorsMisconfiguartionError < StandardError; end + attr_accessor :path, :methods, :headers, :expose, :max_age, :credentials, :pattern, :if_proc, :vary_headers def initialize(public_resource, path, opts={}) + raise CorsMisconfiguartionError 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] @@ -354,7 +357,7 @@ def public_resource? end def origin_for_response_header(origin) - return '*' if public_resource? && !credentials + return '*' if public_resource? origin end From ece6302494a574b6714d437a7f11599ce8850ecf Mon Sep 17 00:00:00 2001 From: Peter Retzlaff Date: Thu, 13 Jul 2017 17:54:15 +0200 Subject: [PATCH 3/5] Add message to the CorsMisconfigurationError. --- lib/rack/cors.rb | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/rack/cors.rb b/lib/rack/cors.rb index f8f7c96..9cd9c6b 100644 --- a/lib/rack/cors.rb +++ b/lib/rack/cors.rb @@ -298,12 +298,18 @@ def resource_for_path(path) end class Resource - class CorsMisconfiguartionError < StandardError; end + 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 CorsMisconfiguartionError if public_resource && opts[:credentials] + raise CorsMisconfigurationError if public_resource && opts[:credentials] + self.path = path self.credentials = opts[:credentials].nil? ? !public_resource : opts[:credentials] self.max_age = opts[:max_age] || 1728000 From aa1599b2b392caca543507f0b7c0516e3f6e0418 Mon Sep 17 00:00:00 2001 From: Peter Retzlaff Date: Thu, 13 Jul 2017 18:32:09 +0200 Subject: [PATCH 4/5] Modify tests to expect wildcard origin header. --- test/unit/cors_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/cors_test.rb b/test/unit/cors_test.rb index feb10bb..e73c759 100644 --- a/test/unit/cors_test.rb +++ b/test/unit/cors_test.rb @@ -247,7 +247,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 @@ -314,7 +314,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 From 95d09d4b21f321e77cc58aa5c9ccf23e8bae7594 Mon Sep 17 00:00:00 2001 From: Peter Retzlaff Date: Fri, 14 Jul 2017 13:54:09 +0200 Subject: [PATCH 5/5] Add two regression tests for insecure CORS configurations. --- test/unit/cors_test.rb | 13 +++++++++++++ test/unit/insecure.ru | 8 ++++++++ 2 files changed, 21 insertions(+) create mode 100644 test/unit/insecure.ru diff --git a/test/unit/cors_test.rb b/test/unit/cors_test.rb index e73c759..13f1d5d 100644 --- a/test/unit/cors_test.rb +++ b/test/unit/cors_test.rb @@ -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 @@ -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") } diff --git a/test/unit/insecure.ru b/test/unit/insecure.ru new file mode 100644 index 0000000..0fb3e97 --- /dev/null +++ b/test/unit/insecure.ru @@ -0,0 +1,8 @@ +require 'rack/cors' + +use Rack::Cors do + allow do + origins '*' + resource '/public', credentials: true + end +end