Refactor JWT to use around_action when needed instead of middleware used with every request

[nelsonwittwer]

Overview/summary

We currently use a JWT middleware to parse the JWT for every request that is made to a rails app that uses this gem. For actions that don't need to make API calls to Shopify we are still spending time logging and trying to parse a token that doesn't exist. For applications that have a large amount of traffic, this inefficiency adds up.

We should explore moving that logic to our controller concerns as part of a around_action or before_action filter so we only parse JWT when we know it'll be there.

[danielpgross]

As far as I can tell, this middleware is actually not needed at all anymore in shopify_app itself, since JWT auth was moved to shopify-api-ruby in v19. That gem has its own independent JWT handling, for example here.

So the only reason we’d want to hold on to the middleware is to maintain compatibility with apps that rely on the JWT values being parsed into environment variables, e.g. jwt.shopify_domain.

Considering that, I’d propose a simple solution:

1. Add a default false configuration option enable_jwt_middleware to shopify_app
2. Only load the JWT middleware if enable_jwt_middleware == true (for opt-in backwards compatibility)
3. Optional: Add a deprecation notice somewhere explaining that the middleware will be removed entirely at some point in the future

I don't see a need for a new controller concern to hold the old middleware logic, since:

• app developers writing new code should directly use the JWT parsing utils in shopify-api-ruby
• app developers looking to support their existing middleware-dependent code will just activate enable_jwt_middleware

[zzooeeyy]

Hey Daniel! I like your suggestion of making this a configurable option. I think for minimal friction backwards compatibility, we should make it an opt-out configuration. disable_jwt_middleware = false as default. And apps can set disable_jwt_middleware = true if they want to by-pass the middleware.

The downside to this is the confusing language with the double negatives, but I think that's worth making version upgrades easier for 3rd party apps. Open to other suggestions as well.

[nelsonwittwer]

Agreed that we no longer need the JWT middleware.

The good news we have already signaled that JWT middleware is deprecated and is set to be deleted with the next major release version 23.0. Seeing as we have it firing deprecation warnings already, I recommend we remove it from the codebase rather than keeping it as a configurable option with the next major release.

[zzooeeyy]

The good news we have already signaled that JWT middleware is deprecated and is set to be deleted with the next major release version 23.0.

Just to clarify, we're only deprecating the ShopifyAPP::JWT class that's responsible for decoding and validating the token and not the middleware itself. That deprecation is to unify to a single parser class ShopifyAPI::JtwPayload. The JWT middleware still uses the ShopifyAPI::JtwPayload to decode the token.