Add cross site protection via rack-protection

lib/omniauth.rb

@@ -15,6 +15,7 @@ module OmniAuth
   autoload :Form,     'omniauth/form'
   autoload :AuthHash, 'omniauth/auth_hash'
   autoload :FailureEndpoint, 'omniauth/failure_endpoint'
+  autoload :AuthenticityTokenProtection, 'omniauth/authenticity_token_protection'
 
   def self.strategies
     @strategies ||= []
@@ -29,20 +30,22 @@ module OmniAuth
       logger
     end
 
-    def self.defaults
+    def self.defaults # rubocop:disable MethodLength
       @defaults ||= {
         :camelizations => {},
         :path_prefix => '/auth',
         :on_failure => OmniAuth::FailureEndpoint,
         :failure_raise_out_environments => ['development'],
+        :request_validation_phase => OmniAuth::AuthenticityTokenProtection,
         :before_request_phase   => nil,
         :before_callback_phase  => nil,
         :before_options_phase   => nil,
         :form_css => Form::DEFAULT_CSS,
         :test_mode => false,
         :logger => default_logger,
-        :allowed_request_methods => %i[get post],
-        :mock_auth => {:default => AuthHash.new('provider' => 'default', 'uid' => '1234', 'info' => {'name' => 'Example User'})}
+        :allowed_request_methods => %i[post],
+        :mock_auth => {:default => AuthHash.new('provider' => 'default', 'uid' => '1234', 'info' => {'name' => 'Example User'})},
+        :silence_get_warning => false
       }
     end
 
@@ -74,6 +77,14 @@ module OmniAuth
       end
     end
 
+    def request_validation_phase(&block)
+      if block_given?
+        @request_validation_phase = block
+      else
+        @request_validation_phase
+      end
+    end
+
     def before_request_phase(&block)
       if block_given?
         @before_request_phase = block
@@ -111,8 +122,9 @@ module OmniAuth
       camelizations[name.to_s] = camelized.to_s
     end
 
-    attr_writer :on_failure, :before_callback_phase, :before_options_phase, :before_request_phase
-    attr_accessor :failure_raise_out_environments, :path_prefix, :allowed_request_methods, :form_css, :test_mode, :mock_auth, :full_host, :camelizations, :logger
+    attr_writer :on_failure, :before_callback_phase, :before_options_phase, :before_request_phase, :request_validation_phase
+    attr_accessor :failure_raise_out_environments, :path_prefix, :allowed_request_methods, :form_css,
+                  :test_mode, :mock_auth, :full_host, :camelizations, :logger, :silence_get_warning
   end
 
   def self.config

lib/omniauth/authenticity_token_protection.rb

@@ -0,0 +1,30 @@
+require 'rack-protection'
+
+module OmniAuth
+  class AuthenticityError < StandardError; end
+  class AuthenticityTokenProtection < Rack::Protection::AuthenticityToken
+    def initialize(options = {})
+      @options = default_options.merge(options)
+    end
+
+    def self.call(env)
+      new.call!(env)
+    end
+
+    def call!(env)
+      return if accepts?(env)
+
+      instrument env
+      react env
+    end
+
+  private
+
+    def deny(_env)
+      OmniAuth.logger.send(:warn, "Attack prevented by #{self.class}")
+      raise AuthenticityError.new(options[:message])
+    end
+
+    alias default_reaction deny
+  end
+end

lib/omniauth/strategy.rb

@@ -180,6 +180,8 @@ module OmniAuth
         raise(error)
       end
 
+      warn_if_using_get
+
       @env = env
       @env['omniauth.strategy'] = self if on_auth_path?
 
@@ -192,6 +194,25 @@ module OmniAuth
       @app.call(env)
     end
 
+    def warn_if_using_get
+      return unless OmniAuth.config.allowed_request_methods.include?(:get)
+      return if OmniAuth.config.silence_get_warning
+
+      log :warn, <<-WARN
+  You are using GET as an allowed request method for OmniAuth. This may leave
+  you open to CSRF attacks. As of v2.0.0, OmniAuth by default allows only POST
+  to its own routes. You should review the following resources to guide your
+  mitigation:
+  https://github.com/omniauth/omniauth/wiki/Resolving-CVE-2015-9284
+  https://github.com/omniauth/omniauth/issues/960
+  https://nvd.nist.gov/vuln/detail/CVE-2015-9284
+  https://github.com/omniauth/omniauth/pull/809
+
+  You can ignore this warning by setting:
+  OmniAuth.config.silence_get_warning = true
+      WARN
+    end
+
     # Responds to an OPTIONS request.
     def options_call
       OmniAuth.config.before_options_phase.call(env) if OmniAuth.config.before_options_phase
@@ -206,6 +227,8 @@ module OmniAuth
 
       # store query params from the request url, extracted in the callback_phase
       session['omniauth.params'] = request.GET
+
+      OmniAuth.config.request_validation_phase.call(env) if OmniAuth.config.request_validation_phase
       OmniAuth.config.before_request_phase.call(env) if OmniAuth.config.before_request_phase
 
       if options.form.respond_to?(:call)
@@ -225,6 +248,8 @@ module OmniAuth
 
         request_phase
       end
+    rescue OmniAuth::AuthenticityError => e
+      fail!(:authenticity_error, e)
     end
 
     # Performs the steps necessary to run the callback phase of a strategy.

omniauth.gemspec

@@ -8,6 +8,7 @@ Gem::Specification.new do |spec|
   spec.add_dependency 'hashie', ['>= 3.4.6']
   spec.add_dependency 'rack', ['>= 1.6.2', '< 3']
   spec.add_development_dependency 'bundler', '~> 2.0'
+  spec.add_dependency 'rack-protection'
   spec.add_development_dependency 'rake', '~> 12.0'
   spec.authors       = ['Michael Bleigh', 'Erik Michaels-Ober', 'Tom Milewski']
   spec.description   = 'A generalized Rack framework for multiple-provider authentication.'

spec/helper.rb

@@ -24,6 +24,7 @@ require 'omniauth'
 require 'omniauth/test'
 
 OmniAuth.config.logger = Logger.new('/dev/null')
+OmniAuth.config.request_validation_phase = nil
 
 RSpec.configure do |config|
   config.include Rack::Test::Methods

spec/omniauth/strategies/developer_spec.rb

@@ -10,7 +10,7 @@ describe OmniAuth::Strategies::Developer do
   end
 
   context 'request phase' do
-    before(:each) { get '/auth/developer' }
+    before(:each) { post '/auth/developer' }
 
     it 'displays a form' do
       expect(last_response.status).to eq(200)

spec/omniauth/strategy_spec.rb

@@ -2,7 +2,7 @@ require 'helper'
 
 def make_env(path = '/auth/test', props = {})
   {
-    'REQUEST_METHOD' => 'GET',
+    'REQUEST_METHOD' => 'POST',
     'PATH_INFO' => path,
     'rack.session' => {},
     'rack.input' => StringIO.new('test=true')
@@ -524,20 +524,20 @@ describe OmniAuth::Strategy do
     end
 
     context 'request method restriction' do
-      before do
-        OmniAuth.config.allowed_request_methods = [:post]
+      before(:context) do
+        OmniAuth.config.allowed_request_methods = %i[put post]
       end
 
       it 'does not allow a request method of the wrong type' do
-        expect { strategy.call(make_env) }.not_to raise_error
+        expect { strategy.call(make_env('/auth/test', 'REQUEST_METHOD' => 'GET')) }.not_to raise_error
       end
 
       it 'allows a request method of the correct type' do
-        expect { strategy.call(make_env('/auth/test', 'REQUEST_METHOD' => 'POST')) }.to raise_error('Request Phase')
+        expect { strategy.call(make_env('/auth/test')) }.to raise_error('Request Phase')
       end
 
-      after do
-        OmniAuth.config.allowed_request_methods = %i[get post]
+      after(:context) do
+        OmniAuth.config.allowed_request_methods = %i[post]
       end
     end
 
@@ -548,7 +548,7 @@ describe OmniAuth::Strategy do
         end
 
         it 'sets the Allow header properly' do
-          expect(response[1]['Allow']).to eq('GET, POST')
+          expect(response[1]['Allow']).to eq('POST')
         end
       end
 
@@ -809,6 +809,50 @@ describe OmniAuth::Strategy do
         OmniAuth.config.test_mode = false
       end
     end
+
+    context 'authenticity validation' do
+      let(:app) { lambda { |_env| [200, {}, ['reached our target']] } }
+      let(:strategy) { ExampleStrategy.new(app, :request_path => '/auth/test') }
+      before do
+        OmniAuth.config.request_validation_phase = OmniAuth::AuthenticityTokenProtection
+      end
+
+      context 'with default POST only request methods' do
+        let!(:csrf_token) { SecureRandom.base64(32) }
+        let(:escaped_token) { URI.encode_www_form_component(csrf_token, Encoding::UTF_8) }
+
+        it 'allows a request with matching authenticity_token' do
+          post_env = make_env('/auth/test', 'rack.session' => {:csrf => csrf_token}, 'rack.input' => StringIO.new("authenticity_token=#{escaped_token}"))
+          expect { strategy.call(post_env) }.to raise_error('Request Phase')
+        end
+
+        it 'does not allow a request without a matching authenticity token' do
+          post_env = make_env('/auth/test', 'rack.input' => StringIO.new("authenticity_token=#{escaped_token}"))
+          expect(strategy.call(post_env)[0]).to eq(302)
+          expect(strategy.call(post_env)[2]).to eq(['302 Moved'])
+        end
+      end
+
+      context 'with allowed GET' do
+        before(:context) do
+          @old_allowed_request_methods = OmniAuth.config.allowed_request_methods
+          OmniAuth.config.allowed_request_methods = %i[post get]
+        end
+
+        it 'allows a request without authenticity token' do
+          get_env = make_env('/auth/test', 'REQUEST_METHOD' => 'GET')
+          expect { strategy.call(get_env) }.to raise_error('Request Phase')
+        end
+
+        after(:context) do
+          OmniAuth.config.allowed_request_methods = @old_allowed_request_methods
+        end
+      end
+
+      after do
+        OmniAuth.config.request_validation_phase = nil
+      end
+    end
   end
 
   context 'setup phase' do

spec/omniauth_spec.rb

@@ -26,20 +26,22 @@ describe OmniAuth do
     end
 
     before do
-      @old_path_prefix           = OmniAuth.config.path_prefix
-      @old_on_failure            = OmniAuth.config.on_failure
-      @old_before_callback_phase = OmniAuth.config.before_callback_phase
-      @old_before_options_phase  = OmniAuth.config.before_options_phase
-      @old_before_request_phase  = OmniAuth.config.before_request_phase
+      @old_path_prefix              = OmniAuth.config.path_prefix
+      @old_on_failure               = OmniAuth.config.on_failure
+      @old_before_callback_phase    = OmniAuth.config.before_callback_phase
+      @old_before_options_phase     = OmniAuth.config.before_options_phase
+      @old_before_request_phase     = OmniAuth.config.before_request_phase
+      @old_request_validation_phase = OmniAuth.config.request_validation_phase
     end
 
     after do
       OmniAuth.configure do |config|
-        config.path_prefix           = @old_path_prefix
-        config.on_failure            = @old_on_failure
-        config.before_callback_phase = @old_before_callback_phase
-        config.before_options_phase  = @old_before_options_phase
-        config.before_request_phase  = @old_before_request_phase
+        config.path_prefix              = @old_path_prefix
+        config.on_failure               = @old_on_failure
+        config.before_callback_phase    = @old_before_callback_phase
+        config.before_options_phase     = @old_before_options_phase
+        config.before_request_phase     = @old_before_request_phase
+        config.request_validation_phase = @old_request_validation_phase
       end
     end
 
@@ -88,6 +90,15 @@ describe OmniAuth do
       expect(OmniAuth.config.before_callback_phase.call).to eq('heyhey')
     end
 
+    it 'is able to set request_validation_phase' do
+      OmniAuth.configure do |config|
+        config.request_validation_phase do
+          'validated'
+        end
+      end
+      expect(OmniAuth.config.request_validation_phase.call).to eq('validated')
+    end
+
     describe 'mock auth' do
       before do
         @auth_hash = {:uid => '12345', :info => {:name => 'Joe', :email => 'joe@example.com'}}