Handle invalid access tokens in token exchange

[zzooeeyy]

• closes https://github.com/Shopify/develop-app-access/issues/146

📹 Demo video

What this PR does

• Handles 401 error in around_action in token exchange concern activate_shopfiy_session.
• It'll retry once by performing token exchange to retrieve new access token.
• Subsequent errors will be thrown.
• Introduces ShopifyApp::Auth::TokenExchange.perform(session_token)
• Introduces ShopifyApp::AdminAPI::WithTokenRefetch module with a method to allow easy token refetch/retry on Unauthorized errors

with_token_refetch(@session, @shopify_id_token) do
  # Admin API call here
end

So developers handling their own errors can still have their access tokens refetched:

# my_controller.rb

def count_and_rescue
  with_token_refetch(current_shopify_session, shopify_id_token) do
    ShopifyAPI::Product.count
  end
rescue =>
  # performing error handling
end

Checklist

Before submitting the PR, please consider if any of the following are needed:

• [ will update when token exchange feature is complete ] Update CHANGELOG.md if the changes would impact users
• [ will update when token exchange feature is complete ] Update README.md, if appropriate.
• [ will update when token exchange feature is complete ] Update any relevant pages in /docs, if necessary
• [ will update when token exchange feature is complete ] For security fixes, the Disclosure Policy must be followed.

[rachel-carvalho]

We'll create an API wrapper and handle errors differently, so I'm moving this back to draft.

[rachel-carvalho]

We decided to keep retrying in the around_action, but will also introduce a module ShopifyApp::AdminAPI::WithTokenRefetch that offers a method that will automatically re-fetch an access token on Unauthorized errors and retry the block.

with_token_refetch(@session, @session_token) do
  # Admin API call here
end

[zzooeeyy]

Thanks for the clean up, I just have some questions! ❤️

[paulomarg]

I think this is great! We should also make sure to document the new public-facing aspects of these changes - ideally with some examples of how folks might use them.

[rachel-carvalho]

The tests that execute the new Session#copy_attributes_from method are now failing until we update the shopify_api version to include it.

[rachel-carvalho]

I added this section to the sessions doc to document usage cc @zzooeeyy @paulomarg

[zzooeeyy]

Thanks! Some more 💭

[zzooeeyy]

hmm looking at the example here, do you think it might be confusing to devs on the difference between current_shopify_session and session_token? There are so many cases where we just use "session" to describe something, should we rename (in the actual method implementation as well) session_token to be like session_jwt_token to be more explicit on which one we're talking about?

[rachel-carvalho]

Yeah I agree it's very confusing. I noticed we call it id_token in the URL and also in App Bridge, so I'm leaning towards shopify_id_token here to keep consistence, especially because those are the two sources of the token we're using here.

[paulomarg]

While I think that's good, we also use the phrase session token in quite a few places (docs, etc), even though it's really an id token. We'd probably want to be consistent with that, right?

[rachel-carvalho]

I was chatting with @ragalie about this to get a better understanding of session token vs ID token, and what I got is that this session token is a type of id token, which is part of the Open ID spec and is supposed to be a claim by the server about a user's identity. session token is Shopify's claim about the current user that's logged in, it's an ID token and it's in JWT format.

I do agree that we need to make it clear that id_token in some places and session_token in others means the same thing, just like session in the context of the libraries usually means access_token.

[paulomarg]

Had some nits but looking really good!

[ragalie]

LGTM, thanks!

[paulomarg]

Just one last nit, non-blocking if it is the case :)

[paulomarg]

I assume this TODO also covers requests where no id token is present?

[zzooeeyy]

Yesss