gnikyt / laravel-shopify

A full-featured Laravel package for aiding in Shopify App development
MIT License
1.24k stars 374 forks source link

Support for new JWT auth flow #551

Closed arvesolland closed 4 years ago

arvesolland commented 4 years ago

Are there anyone working on support for the new auth flow using JWT instead of cookies?

Ref: https://shopify.dev/tools/app-bridge/authentication

faisal-saeed-brainx commented 4 years ago

Hi @Kyon147 @osiset, What would you say on this? Any idea how can we implement in the package?

Kyon147 commented 4 years ago

From the last time I looked into this, the JWT flow does not 100% help with the cookies that are more on the Laravel side as this sits more around authenticating with app bridge / Shopify.

@osiset would know a lot more than myself - no doubt he is probably looking at potential solutions.

gnikyt commented 4 years ago

JWT doesn't solve the issue no, it's made for SPAs. Cookie issue something I'm still trying to solve.

niveshsaharan commented 4 years ago

For someone interested in implementing JWT session token based authentication, I have implemented it in one of my own project. Here's the commit: https://github.com/niveshsaharan/shark/commit/1bd88f7bf91d0d30805cb9aa5c996bed3283fa55

gnikyt commented 4 years ago

@niveshsaharan I currently have no time to implement a new auth flow to support JWT. Would you mind creating a guard PR for this? If not I can revisit at a later date.

niveshsaharan commented 4 years ago

@osiset I also don't have any time because Shopify has banned all of my apps and now I'm in survival mode. You might have heard about it already and if you haven't, you can read here: https://community.shopify.com/c/Shopify-Apps/Sellify-apps-no-longer-available-on-Shopify-details-and/td-p/837355

gnikyt commented 4 years ago

No worries. Since this is for SPAs anyways, it will be lower priority on my list. Cookie is the top right now. Thanks!

darrynten commented 4 years ago

I've been working on this.

I've got it all working, and am at the point where I can do 2 things, but am unsure if I should do them in this package or not.

  1. Remove all Cookies

I'm at a point where I could probably remove 95% of the Cookie code and things will still work. This will resolve a massive amount of issues we've all experienced, including #443 #522 #531 and this one (#551) - resolves problems like being logged into 2 shops at once on the same browser and having them affect each others data.

The only place I'm still using Cookies here is with the initial signup and billing, but we could probably get rid of that completely - however, Shopify documentation has no info on how to do this with the new token flow.

To bring this functionality in means I would have to:

  1. Support Single Page Apps by Default

I've got a private project where I converted this package to work with an SPA (with it's own JWT setup) - I could reuse a bunch of this to quickly convert this package into being SPA by default.

However

I do not personally use the official shopify React SPA. I much prefer Vue, and my implementation is Vue based.

Pragmatically speaking, the bulk of the community behind this app will be using React and the official Polaris packages, and I don't want to do a bunch of work getting Vue into place only to have it be rejected.

In that light I'm thinking of making a barebones version of the JWT functionality, and let it live side-by-side with Cookies for now, just to get it out there for the everyone else to try.

We could include this JWT in the next big release as a stipped down optional feature, iron out the kinks and eventually make it the default, replacing the need for Cookies when interacting with the app from any frontend (besides for initial auth and billing selection).

Another option is, apparently, TurboLinks. I haven't looked into it.

So I need to know - @osiset @Kyon147 @lucasmichot - how should we proceed? Tyler, is there a way for us to discuss this offline?

niveshsaharan commented 4 years ago

I'm not sure if need to create a guard for this. What do you think about just adding a method in AuthShopify middleware that checks if there is a bearer token in the request and validate it. If everything's good, we return early otherwise let it go through the rest of middleware functions.

public function handle(Request $request, Closure $next)
{
    if($this->tryLoginWithShopifyJwtToken($request))
    {
        return $next($request);
    }

    // rest of code ....
}
private function tryLoginWithShopifyJwtToken(Request $request): bool
{
    if( $request->bearerToken())
    {
        $exploded = explode('.', $request->bearerToken(), 3);

        if (3 == \count($exploded)) {
            list($header, $payload, $signature) = $exploded;

            $payloadArray = json_decode(base64_decode($payload), true);

            if ($payloadArray['exp'] > now()->utc()->timestamp
                && $payloadArray['nbf'] <= now()->utc()->timestamp
                && \Illuminate\Support\Str::startsWith($payloadArray['iss'], $payloadArray['dest'])
            ) {
                $shopDomain = \Osiset\ShopifyApp\Objects\Values\ShopDomain::fromNative($payloadArray['dest']);

                if (! $shopDomain->isNull()) {
                    $shop = resolve(\Osiset\ShopifyApp\Contracts\Queries\Shop::class)->getByDomain($shopDomain);

                    if ($shop) {
                        // Generate api helper from shop
                        $apiHelper = $shop->apiHelper();

                        $apiSecret = $apiHelper->getApi()->getOptions()->getApiSecret();

                        $hmacLocal = rtrim(strtr(base64_encode(hash_hmac('sha256', $header.'.'.$payload, $apiSecret, true)), '+/', '-_'), '=');

                        if (hash_equals($hmacLocal, $signature)) {
                            return $this->loginShop($request, $shopDomain);
                        }
                    }
                }
            }
        }
    }
    return false;
}
gnikyt commented 4 years ago

@niveshsaharan this is a great starting point

@darrynten yes, it would solve a world of problems but leave the package only supporting an SPA would limit, hmm, yes please email me! Seems GitHub discussions is not active feature yet :/

darrynten commented 4 years ago

@osiset where should I send the mail to?

lucasmichot commented 4 years ago

Why not just using https://github.com/tymondesigns/jwt-auth ?

gnikyt commented 4 years ago

Thats a good option to try.

darrynten commented 4 years ago

@lucasmichot @osiset I oppose the usage of that package for this basic functionality.

I think most SPAs will be authenticating with their own API endpoints using this package exact JWT package for non-Shopify related things in order to guard endpoints that do not interact with Shopify.

An example is when an app interacts with a 3rd party like a shipping company. This use case requires a stateless API be guarded to confirm the user is who they say they are before interacting with non-Shopify entities on the backend, and the most common way of doing this is with the tymon package.

Using this package directly when we only need like 5% of the functionality may hinder applications that build on top of this package.

darrynten commented 4 years ago

I've completed my work on this and am currently adding additional unit tests. Will open a PR into this repo as soon as all checks are green.

darrynten commented 4 years ago

Update: Figuring out why tests run locally but not on CI. Should be done soon.

darrynten commented 4 years ago

I've opened up PR #601 which contains the JWT functionality.

gnikyt commented 4 years ago

Currently in review! Thanks @darrynten