From 24a1704f057b2fda5992d21dc3a07c8764ca577f Mon Sep 17 00:00:00 2001
From: Brandur <brandur@mutelight.org>
Date: Thu, 27 Jul 2017 09:01:53 -0700
Subject: [PATCH] Improve error handling safety in the event of unrecognized
 OAuth error

It was brought up in #562 that in case we receive an OAuth error that we
don't know about, `specific_oauth_error` will fall through with a `nil`,
then picked up by `specific_api_error` which will always try to handle
the error as if it were a `Hash` (even if we know it's not!) and thus
lead to typing problems at runtime.

This patch throws a generic `OAuthError` in cases where a code comes
back that we don't recognize. I'm still crazy about the fact that we
don't have a better way of recognizing an OAuth error in particular, but
it should do the trick.
---
 lib/stripe/stripe_client.rb       |  7 ++++---
 test/stripe/stripe_client_test.rb | 18 ++++++++++++++++++
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/lib/stripe/stripe_client.rb b/lib/stripe/stripe_client.rb
index b8949bf0..213c64c8 100644
--- a/lib/stripe/stripe_client.rb
+++ b/lib/stripe/stripe_client.rb
@@ -249,8 +249,7 @@ module Stripe
 
       if error_data.is_a?(String)
         error = specific_oauth_error(resp, error_data)
-      end
-      if error.nil?
+      else
         error = specific_api_error(resp, error_data)
       end
 
@@ -319,7 +318,9 @@ module Stripe
       when 'unsupported_grant_type'    then OAuth::UnsupportedGrantTypeError.new(*args)
       when 'unsupported_response_type' then OAuth::UnsupportedResponseTypeError.new(*args)
       else
-        nil
+        # We'd prefer that all errors are typed, but we create a generic
+        # OAuthError in case we run into a code that we don't recognize.
+        OAuth::OAuthError.new(*args)
       end
     end
 
diff --git a/test/stripe/stripe_client_test.rb b/test/stripe/stripe_client_test.rb
index 1caea2d4..46eff5b0 100644
--- a/test/stripe/stripe_client_test.rb
+++ b/test/stripe/stripe_client_test.rb
@@ -386,6 +386,24 @@ module Stripe
           assert_equal('invalid_client', e.code)
           assert_equal('This application is not connected to stripe account acct_19tLK7DSlTMT26Mk, or that account does not exist.', e.message)
         end
+
+        should "raise Stripe::OAuthError on indeterminate OAuth error" do
+          stub_request(:post, "#{Stripe.connect_base}/oauth/deauthorize").
+            to_return(body: JSON.generate({
+              error: "new_code_not_recognized",
+              error_description: "Something.",
+            }), status: 401)
+
+          client = StripeClient.new
+          opts = {api_base: Stripe.connect_base}
+          e = assert_raises Stripe::OAuth::OAuthError do
+            client.execute_request(:post, '/oauth/deauthorize', opts)
+          end
+
+          assert_equal(401, e.http_status)
+          assert_equal("new_code_not_recognized", e.code)
+          assert_equal("Something.", e.message)
+        end
       end
 
       context "idempotency keys" do