Skip to content
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

Post authentication tasks after token exchange #1821

Merged
merged 2 commits into from
Apr 3, 2024

Conversation

zzooeeyy
Copy link
Contributor

@zzooeeyy zzooeeyy commented Mar 28, 2024

What this PR does

  • Trigger post_authenticate_tasks after token exchange completion

Top hatting

PostAuthenticateTask methods are called after token exchange completes

post-auth-tasks

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

@zzooeeyy zzooeeyy force-pushed the token-exchange-post-auth-tasks branch from 1c198c5 to dd66074 Compare April 2, 2024 16:36
@zzooeeyy zzooeeyy marked this pull request as ready for review April 2, 2024 16:37
@zzooeeyy zzooeeyy requested a review from a team as a code owner April 2, 2024 16:37
shop: domain, # TODO: use jwt_shopify_domain ?
session_token: session_token,
requested_token_type: ShopifyAPI::Auth::TokenExchange::RequestedTokenType::ONLINE_ACCESS_TOKEN,
)
end

# TODO: Refactor and add post authenticate tasks
# ShopifyApp.configuration.post_authenticate_tasks.perform(session) if session
ShopifyApp.configuration.post_authenticate_tasks.perform(session) if session
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there isn't a session here, should we fail somehow?

Copy link
Contributor Author

@zzooeeyy zzooeeyy Apr 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm yea you're right, but I think that's already handled in the shopify-api-ruby layer --
We raise if there's any HttpResponseError in the TokenExchange method, and I don't think it'll go beyond that point if there's any errors in token exchange call. So we could probably just let the exception raise and assume session will be there if no exceptions were raised..

@zzooeeyy zzooeeyy requested a review from paulomarg April 3, 2024 16:55
@zzooeeyy zzooeeyy force-pushed the token-exchange-post-auth-tasks branch from 6d24584 to 5defd48 Compare April 3, 2024 17:13
@zzooeeyy zzooeeyy merged commit 7800821 into main Apr 3, 2024
7 checks passed
@zzooeeyy zzooeeyy deleted the token-exchange-post-auth-tasks branch April 3, 2024 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants