Fail with oauth errors instead of masking them

lib/omniauth/strategies/facebook.rb

@@ -7,8 +7,6 @@
 module OmniAuth
   module Strategies
     class Facebook < OmniAuth::Strategies::OAuth2
-      class NoAuthorizationCodeError < StandardError; end
-
       DEFAULT_SCOPE = 'email'
 
       option :client_options, {
@@ -63,11 +61,9 @@ def info_options
       end
 
       def callback_phase
-        with_authorization_code! do
+        with_authorization_code do
           super
         end
-      rescue NoAuthorizationCodeError => e
-        fail!(:no_authorization_code, e)
       rescue OmniAuth::Facebook::SignedRequest::UnknownSignatureAlgorithmError => e
         fail!(:unknown_signature_algorithm, e)
       end
@@ -126,7 +122,10 @@ def raw_signed_request_from_cookie
       #
       # 1. The request 'code' param (manual callback from standard server-side flow)
       # 2. A signed request from cookie (passed from the client during the client-side flow)
-      def with_authorization_code!
+      #
+      # Does not guarantee the presence of a code. This is used for
+      # all request types, including those that don't include codes.
+      def with_authorization_code
         if request.params.key?('code')
           yield
         elsif code_from_signed_request = signed_request_from_cookie && signed_request_from_cookie['code']
@@ -144,7 +143,7 @@ def with_authorization_code!
             options.provider_ignores_state = original_provider_ignores_state
           end
         else
-          raise NoAuthorizationCodeError, 'must pass either a `code` (via URL or by an `fbsr_XXX` signed request cookie)'
+          yield
         end
       end
 

test/strategy_test.rb

@@ -416,9 +416,17 @@ class CookieAndParamNotPresentTest < TestCase
     test 'is nil' do
       assert_nil strategy.send(:signed_request_from_cookie)
     end
+  end
+
+  class RaisesOauthErrors < TestCase
+    def setup
+      super
+      @request.stubs(:params).returns({'error_reason' => 'user_denied'})
+    end
 
-    test 'throws an error on calling build_access_token' do
-      assert_raises(OmniAuth::Strategies::Facebook::NoAuthorizationCodeError) { strategy.send(:with_authorization_code!) {} }
+    test 'raises oauth errors on error requests' do
+      strategy.expects(:fail!).times(1).with("user_denied", kind_of(OmniAuth::Strategies::OAuth2::CallbackError))
+      strategy.callback_phase
     end
   end
 
@@ -456,37 +464,6 @@ def setup
     end
   end
 
-  class MissingCodeInParamsRequestTest < TestCase
-    def setup
-      super
-      @request.stubs(:params).returns({})
-    end
-
-    test 'calls fail! when a code is not included in the params' do
-      strategy.expects(:fail!).times(1).with(:no_authorization_code, kind_of(OmniAuth::Strategies::Facebook::NoAuthorizationCodeError))
-      strategy.callback_phase
-    end
-  end
-
-  class MissingCodeInCookieRequestTest < TestCase
-    def setup(algo = nil)
-      super()
-      @payload = {
-        'algorithm' => algo || 'HMAC-SHA256',
-        'code' => nil,
-        'issued_at' => Time.now.to_i,
-        'user_id' => '123456'
-      }
-
-      @request.stubs(:cookies).returns({"fbsr_#{@client_id}" => signed_request(@payload, @client_secret)})
-    end
-
-    test 'calls fail! when a code is not included in the cookie' do
-      strategy.expects(:fail!).times(1).with(:no_authorization_code, kind_of(OmniAuth::Strategies::Facebook::NoAuthorizationCodeError))
-      strategy.callback_phase
-    end
-  end
-
   class UnknownAlgorithmInCookieRequestTest < TestCase
     def setup
       super()