Stripe::OAuth.token is broken

[volkanunsal]

I'm noticing that this endpoint doesn't work in version 5.1.1. Using curl, I can exchange the OAuth token easily:

curl https://connect.stripe.com/oauth/token \
    -d client_secret=**** \
    -d code="ac_Fy1RF9WU1Vw27vwqx4H7rXX6lMBhIzOX" \
    -d grant_type=authorization_code
{
  "access_token": "***",
  "livemode": false,
  "refresh_token": "***",
  "token_type": "bearer",
  "stripe_publishable_key": "***",
  "stripe_user_id": "***",
  "scope": "express"
}

But this doesn't work with the Ruby client:

params = {"grant_type" => "authorization_code", "client_secret" =>"***", "code" =>"ac_Fy1XLfuYmEzwPIaZqOWOsrCllvbcYmZJ"}
Stripe::OAuth.token(params)

Here is the error:

Stripe::OAuth::InvalidGrantError: Authorization code provided does not belong to you
from /Users/newuser/.rvm/gems/ruby-2.5.5/gems/stripe-5.1.1/lib/stripe/stripe_client.rb:465:in `handle_error_response'

[ob-stripe]

Hi @volkanunsal. According to the error message, it sounds like the API key sent in the client_secret parameter does not match the account that created the authorization code. Can you double check that you're using the correct credentials?

[volkanunsal]

Yes, it's the same key. The client_secret is the same in both examples. It's just that when I use curl, it works, and with the Ruby client, it doesn't. I'm as puzzled as you are.

[ob-stripe]

🤔 Just to be 100% sure, you're using the same client_id to get the authorization code in both cases?

[volkanunsal]

Absolutely, positively certain.

[volkanunsal]

Try it yourself. I'll change this secret key soon. And it's just a test key anyway. First try the Ruby code, then the curl code.

[volkanunsal]

I pasted the wrong code.

[brandur-stripe]

That exact code you pasted actually works for me, haha. I think the problem might be something like you having a different Stripe.api_key = global set than what's in the client_secret parameter, and the global key taking precedence in the final API call.

I can repro the error by setting Stripe.api_key to my own key.

@ob-stripe I may have broken this in the V5 refactor. Do you know if the idea for the OAuth module was that we should never send the global API key?

[volkanunsal]

Oh, that helps. Yes, I can see that the global key is different.

[ob-stripe]

Ohh, okay, I think I see what's happening. The client_secret parameter is there because it's part of the OAuth spec, but in Stripe's OAuth implementation, you can also omit it and just authenticate the request like a normal API request (i.e. with the Authorization HTTP header). I bet that when both the Authorization header and the client_secret parameter are present, the former takes precedence.

[volkanunsal]

Very helpful. Thank you all. You can close this issue anytime you like.

[ob-stripe]

This is arguably an actual bug with the client library (and I suspect most/all of our client libraries for other languages have the same issue), if only because the behavior is very non-obvious and confusing.

We should probably omit the Authorization header if the client_secret parameter is present, but the current implementation does not make that particularly easy. It might be easier to replace the API key in the Authorization header with the one provided in the client_secret parameter when it's present:

    def self.token(params = {}, opts = {})
      opts = Util.normalize_opts(opts)
+     opts[:api_key] = params[:client_secret] if params[:client_secret]
      resp, opts = OAuthOperations.request(
        :post, "/oauth/token", params, opts
      )
      # This is just going to return a generic StripeObject, but that's okay
      Util.convert_to_stripe_object(resp.data, opts)
    end

@brandur-stripe wdyt?

[brandur-stripe]

We should probably omit the Authorization header if the client_secret parameter is present, but the current implementation does not make that particularly easy. It might be easier to replace the API key in the Authorization header with the one provided in the client_secret parameter when it's present:

Yep, my thought process was ~identical. I'd prefer just not to include an API key at all, but this line in #execute_request makes that difficult:

api_key ||= Stripe.api_key

We could have some dummy value that tells the client not to set it, but ugh. I think your solution is probably the best — the secret gets doubled up in both the header and client_secret parameter, but it doesn't do anything too non-intuitive.

[ob-stripe]

Fixed in 5.12.1. Sorry it took us so long!