fix CSRF vulnerability. prepare 1.5.0 release

README.md

@@ -52,8 +52,6 @@ end
 
 If you want to set the `display` format or `scope` on a per-request basis, you can just pass it to the OmniAuth request phase URL, for example: `/auth/facebook?display=popup` or `/auth/facebook?scope=email`.
 
-You can also pass through a `state` param which will be passed along to the callback url.
-
 ### Custom Callback URL/Path
 
 You can set a custom `callback_url` or `callback_path` option to override the default value. See [OmniAuth::Strategy#callback_url](https://github.com/intridea/omniauth/blob/master/lib/omniauth/strategy.rb#L411) for more details on the default.

lib/omniauth/facebook/version.rb

@@ -1,5 +1,5 @@
 module OmniAuth
   module Facebook
-    VERSION = "1.4.1"
+    VERSION = "1.5.0"
   end
 end

lib/omniauth/strategies/facebook.rb

@@ -112,20 +112,17 @@ def access_token_options
       end
 
       ##
-      # You can pass +display+, +state+ or +scope+ params to the auth request, if
+      # You can pass +display+ or +scope+ params to the auth request, if
       # you need to set them dynamically. You can also set these options
       # in the OmniAuth config :authorize_params option.
       #
       # /auth/facebook?display=popup&state=ABC
       #
       def authorize_params
         super.tap do |params|
-          %w[display state scope].each do |v|
+          %w[display scope].each do |v|
             if request.params[v]
               params[v.to_sym] = request.params[v]
-
-              # to support omniauth-oauth2's auto csrf protection
-              session['omniauth.state'] = params[:state] if v == 'state'
             end
           end
 

test/support/shared_examples.rb

@@ -50,20 +50,20 @@ module CSRFAuthorizeParamsTests
       assert_equal strategy.authorize_params['state'], strategy.session['omniauth.state']
     end
 
-    test 'should store state in the session when present in authorize params vs. a random one' do
+    test 'should not store state in the session when present in authorize params vs. a random one' do
       @options = { :authorize_params => { :state => 'bar' } }
       refute_empty strategy.authorize_params['state']
-      assert_equal 'bar', strategy.authorize_params[:state]
+      refute_equal 'bar', strategy.authorize_params[:state]
       refute_empty strategy.session['omniauth.state']
-      assert_equal 'bar', strategy.session['omniauth.state']
+      refute_equal 'bar', strategy.session['omniauth.state']
     end
 
-    test 'should store state in the session when present in request params vs. a random one' do
+    test 'should not store state in the session when present in request params vs. a random one' do
       @request.stubs(:params).returns({ 'state' => 'foo' })
       refute_empty strategy.authorize_params['state']
-      assert_equal 'foo', strategy.authorize_params[:state]
+      refute_equal 'foo', strategy.authorize_params[:state]
       refute_empty strategy.session['omniauth.state']
-      assert_equal 'foo', strategy.session['omniauth.state']
+      refute_equal 'foo', strategy.session['omniauth.state']
     end
   end
 

test/test.rb

@@ -56,12 +56,6 @@ class AuthorizeParamsTest < StrategyTestCase
     assert_equal 'touch', strategy.authorize_params[:display]
   end
 
-  test 'includes state parameter from request when present' do
-    @request.stubs(:params).returns({ 'state' => 'some_state' })
-    assert strategy.authorize_params.is_a?(Hash)
-    assert_equal 'some_state', strategy.authorize_params[:state]
-  end
-
   test 'overrides default scope with parameter passed from request' do
     @request.stubs(:params).returns({ 'scope' => 'email' })
     assert strategy.authorize_params.is_a?(Hash)