Remove JWT middleware

[danielpgross]

What this PR does

Closes #1744

Removes the JWT middleware, since JWT authentication was moved to shopify-api-ruby in v19 and then the related ShopifyApp::JWT class was deprecated here (see upgrading guide).

• There were a few remaining references to the decoded JWT values set by the middleware (e.g. request.env["jwt.shopify_domain"]), notably in ShopifyApp::WithShopifyIdToken
  • ShopifyApp::WithShopifyIdToken has been updated to no longer check values in request.env["jwt.*"], and instead only check header and URL param values
  • Other uses of request.env["jwt.*"] have been updated to use the respective methods in ShopifyApp::WithShopifyIdToken.
• The jwt gem has been changed from runtime dependency to dev dependency (it's still used to create JWTs in unit tests)

Checklist

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.

[tgwizard]

Very nice! This LGTM, though others should review too.

Admittedly, I'm having troubles following the code that actually ensures a shopify app verifies the ID token is present and valid. The code I'm reading seems to indicate we do some kind of token exchange - but I thought we fully relied on the JWT, without any external calls.

Could you please walk me through the flow?

[danielpgross]

Very nice! This LGTM, though others should review too.

Admittedly, I'm having troubles following the code that actually ensures a shopify app verifies the ID token is present and valid. The code I'm reading seems to indicate we do some kind of token exchange - but I thought we fully relied on the JWT, without any external calls.

Could you please walk me through the flow?

Thanks @tgwizard! Will walk you through it according to my understanding:

1. An app includes the EnsureHasSession concern in a given controller to restrict that controller to requests that are authenticated with a valid session -- think of this as the entry point to the flow.
2. EnsureHasSession includes LoginProtection and sets up activate_shopify_session from there as an around_action:

   shopify_app/app/controllers/concerns/shopify_app/ensure_has_session.rb

   Line 16 in ef54cdc

   around_action :activate_shopify_session

3. activate_shopify_session calls current_shopify_session, which calls load_current_sessionand in turn ShopifyAPI::Utils::SessionUtils.current_session_id
   

   shopify_app/lib/shopify_app/controller_concerns/login_protection.rb

   Line 25 in ef54cdc

   if current_shopify_session.blank?

   
   

   shopify_app/lib/shopify_app/controller_concerns/login_protection.rb

   Line 56 in ef54cdc

   load_current_session(

   
   

   shopify_app/lib/shopify_app/controller_concerns/login_protection.rb

   Line 264 in ef54cdc

   session_id = ShopifyAPI::Utils::SessionUtils.current_session_id(shopify_id_token, cookies, is_online)

4. Now we're in ShopifyAPI land, and session_id_from_shopify_id_token gets called:
   https://github.com/Shopify/shopify-api-ruby/blob/a64f7dd391306033ca58399b42fa32207ba6dc34/lib/shopify_api/utils/session_utils.rb#L23
5. The JWT finally gets decoded and then a session ID is generated from it:
   https://github.com/Shopify/shopify-api-ruby/blob/a64f7dd391306033ca58399b42fa32207ba6dc34/lib/shopify_api/utils/session_utils.rb#L48

[tgwizard]

Is there some way we can skip parsing the JWT token twice, once here, and once in https://github.com/Shopify/shopify-api-ruby/blob/a64f7dd391306033ca58399b42fa32207ba6dc34/lib/shopify_api/utils/session_utils.rb#L48?

(That could be a follow-up optimization).

[danielpgross]

Good idea. Looks like that would be a change in the shopify-api-ruby repo, so will leave it as a follow-up.

[zzooeeyy]

The changes makes sense to me, have you tried top-hatting with a rails app ?

[zzooeeyy]

I'm not sure what impact this might have? how did this line get moved?

[danielpgross]

This was a result of updating the gemspec as explained in the PR description:

The jwt gem has been changed from runtime dependency to dev dependency (it's still used to create JWTs in unit tests)

[paulomarg]

LGTM!

[danielpgross]

I tested this using a new test app created with https://github.com/Shopify/shopify-app-template-ruby and it's working as expected 👍

[zzooeeyy]

Looks good to me! Thanks for top-hatting!