-
Notifications
You must be signed in to change notification settings - Fork 699
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
use online tokens when available #1566
Conversation
def online_token_configured? | ||
!ShopifyApp.configuration.user_session_repository.blank? && ShopifyApp::SessionRepository.user_storage.present? | ||
end | ||
|
||
def user_session_expected? | ||
return false if shop_session.nil? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shop_session
here would only be available during OAuth since it does a shop look up by the shop
parameter which is only sent during the beginning of OAuth. this would always return false since subsequent proxy API requests would not have the shop query parameter required to do this look up
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Glad we chased this down! 🎉
27bff07
to
e63846f
Compare
e63846f
to
0da8947
Compare
What this PR does
Related to https://github.com/Shopify/shopify-graphiql-app/issues/1069
There is currently logic on when to retrieve an online or offline session token where
current_shopify_session
in theLoginProtection
concern makes an incorrect assumption on checking if a shop session exists when determining to pass inis_online
to load the session from the API gem. This check is required for the original OAuth request but any subsequent requests after OAuth should just check if a User session is configured.Reviewer's guide to testing
Checklist
Before submitting the PR, please consider if any of the following are needed:
CHANGELOG.md
if the changes would impact usersREADME.md
, if appropriate./docs
, if necessary