Ensure post authenticate jobs run after for jwt based callback requests

[rezaansyed]

What does this PR solve?

There is an issue noticed where the OAuth flow for getting an online (user) token fails to redirect back into the callback endpoint. The online token is eventually retrieved on failed requests to the Shopify API and we exchange an auth code for the online token via App Bridge Auth. However, when making the callback request to with session tokens (as JWTs) in the Authorization header, the post authenticate background jobs are not run, which is undesirable.

This PR ensures that the post authenticated background jobs are triggered after the callback request is made with session tokens in the request.

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

• Update CHANGELOG.md if the changes would impact users
• Update README.md, if appropriate.
• Update any relevant pages in docs/, if necessary
• For security fixes, the Disclosure Policy must be followed.

[ragalie]

No test for the install_script_tags action?

[ragalie]

LGTM.

This is easier to read than the previous implementation, which is good! Curious why you decided to refactor in this direction where every sub-method contains a conditional on JWT vs. splitting into two callback methods for JWT vs. normal browser? There's very little overlap other than persisting the token and running these callbacks.

[lstp]

Are we intentionally not replacing the debug logs?

[rezaansyed]

Just added the logs back

[paulomarg]

LGTM!

[paulomarg]

Do we need to check native_browser_request? here, if it's also checked in prevent_session_fixation?

[rezaansyed]

You're right! Just refactored it.

[rezaansyed]

LGTM.

This is easier to read than the previous implementation, which is good! Curious why you decided to refactor in this direction where every sub-method contains a conditional on JWT vs. splitting into two callback methods for JWT vs. normal browser? There's very little overlap other than persisting the token and running these callbacks.

We had thought of splitting it into two but there seems to be a general pattern of processing the callback where we are doing the following:

respond_with_error if invalid_request?
store_access_token
perform_post_authenticate_jobs
respond_successfully

The only difference is that the other flow returns early when needing to get an online (user) token. So we had to leave the extra conditional. Also the reason why we have conditional splits between JWT and non-JWT is so that there is symmetry in where they diverge and that it's easier to see the boundaries for further refactoring in the future.

[rezaansyed]

No test for the install_script_tags action?

Nice catch! Just added :)