CSRF protection

Author: AlexanderPavlenko

lib/omniauth/strategies/oauth2.rb

@@ -47,7 +47,12 @@ def request_phase
       end
 
       def authorize_params
-        options.authorize_params.merge(options.authorize_options.inject({}){|h,k| h[k.to_sym] = options[k] if options[k]; h})
+        if options.authorize_params[:state].to_s.empty?
+          options.authorize_params[:state] = 3.times.map{ rand.to_s[2..-1] }.reduce(&:concat)
+        end
+        params = options.authorize_params.merge(options.authorize_options.inject({}){|h,k| h[k.to_sym] = options[k] if options[k]; h})
+        session['omniauth.state'] = params[:state]
+        params
       end
 
       def token_params
@@ -58,6 +63,9 @@ def callback_phase
         if request.params['error'] || request.params['error_reason']
           raise CallbackError.new(request.params['error'], request.params['error_description'] || request.params['error_reason'], request.params['error_uri'])
         end
+        if request.params['state'] != session.delete('omniauth.state')
+          raise CallbackError.new(nil, :csrf_detected)
+        end
 
         self.access_token = build_access_token
         self.access_token = access_token.refresh! if access_token.expired?

spec/omniauth/strategies/oauth2_spec.rb

@@ -22,13 +22,20 @@ def app; lambda{|env| [200, {}, ["Hello."]]} end
     subject { fresh_strategy }
 
     it 'should include any authorize params passed in the :authorize_params option' do
-      instance = subject.new('abc', 'def', :authorize_params => {:foo => 'bar', :baz => 'zip'})
-      instance.authorize_params.should == {'foo' => 'bar', 'baz' => 'zip'}
+      instance = subject.new('abc', 'def', :authorize_params => {:foo => 'bar', :baz => 'zip', :state => '123'})
+      instance.authorize_params.should == {'foo' => 'bar', 'baz' => 'zip', 'state' => '123'}
     end
 
     it 'should include top-level options that are marked as :authorize_options' do
-      instance = subject.new('abc', 'def', :authorize_options => [:scope, :foo], :scope => 'bar', :foo => 'baz')
-      instance.authorize_params.should == {'scope' => 'bar', 'foo' => 'baz'}
+      instance = subject.new('abc', 'def', :authorize_options => [:scope, :foo], :scope => 'bar', :foo => 'baz', :authorize_params => {:state => '123'})
+      instance.authorize_params.should == {'scope' => 'bar', 'foo' => 'baz', 'state' => '123'}
+    end
+
+    it 'should include random state in the authorize params' do
+      instance = subject.new('abc', 'def')
+      instance.authorize_params.keys.should == ['state']
+      instance.session['omniauth.state'].should_not be_empty
+      instance.session['omniauth.state'].should == instance.authorize_params['state']
     end
   end