Introduce the token exchange API for fetching access tokens #1254
Conversation
gbzodek
force-pushed
the
token-exchange
branch
from
54dc3a3 to
5b08767
Compare
lib/shopify_api/auth/oauth.rb
Outdated
| requested_token_type: RequestedTokenType, | ||
| ).returns(ShopifyAPI::Auth::Session) | ||
| end | ||
| def token_exchange(shop:, session_token:, requested_token_type:) |
There was a problem hiding this comment.
Could we move this into a different file? (ex: lib/shopify_api/auth/token_exchange.rb). The purpose of the oauth.rb file seems to be to handle the auth code flow.
There was a problem hiding this comment.
Ya, I see the intent for moving this into its own file and like that idea. The confusing part is token exchange still is oauth. This oauth.rb file should probably be renamed to auth_code_flow.rb to follow remix pattern.
There was a problem hiding this comment.
I have moved the token exchange to its own module and extracted the logic to create a Session from the access token response, which is shared by both auth code and token exchange flows.
@nelsonwittwer I did not rename oauth.rb to avoid a breaking change for now. It also follows the same naming as shopify-api-js file. But if you think that's worth a breaking change, I am happy to do it.
lib/shopify_api/auth/oauth.rb
Outdated
| requested_token_type: RequestedTokenType, | ||
| ).returns(ShopifyAPI::Auth::Session) | ||
| end | ||
| def token_exchange(shop:, session_token:, requested_token_type:) |
There was a problem hiding this comment.
Ya, I see the intent for moving this into its own file and like that idea. The confusing part is token exchange still is oauth. This oauth.rb file should probably be renamed to auth_code_flow.rb to follow remix pattern.
|
Thanks @nelsonwittwer and @rezaansyed for the review! I will address the comments right after the Christmas break, as I have a few other things to finish before that. |
gbzodek
force-pushed
the
token-exchange
branch
3 times, most recently
from
7ddf785 to
00c0511
Compare
lib/shopify_api/auth/session.rb
Outdated
| @@ -89,6 +89,32 @@ def temp(shop:, access_token:, &blk) | |||
| end | |||
| end | |||
|
|
|||
| sig { params(shop: String, access_token_response: Oauth::AccessTokenResponse).returns(Session) } | |||
| def for(shop:, access_token_response:) | |||
gbzodek
force-pushed
the
token-exchange
branch
3 times, most recently
from
3723a8a to
4837de6
Compare
gbzodek
force-pushed
the
token-exchange
branch
from
4837de6 to
b5dcf68
Compare
Description
This introduces the token exchange API to fetch access tokens based on the Token Exchange spec
This has been already added in
shopify-api-jshere, and we would like to make it available to Rails apps in the future as well.How has this been tested?
Unit tests are provided in the PR to cover the new method. This has also been tested internally for our app (POS channel)
Checklist: