Auth Flow Refactoring

[gnikyt]

Dev Branch: [feature/cookieless]

Introduction

Shopify is now requiring apps to work with session tokens (JWT).

This package has changed several times due to requirement changes with either Shopify or browsers.

At this point, a restructure will be worked on as a priority for authentication flow and validation.

Off the top of my head as some planning, the following will be done and worked on:

Removal of cookies & JWT first-class

Still toying with this in my head, however, while the recent releases helps with cookie issues by handling ITP, its a messy solution and also breaks a good userflow when they see the warnings pop up to enable cookies.

Currently, I see abolishing all cookie and ITP related code and switch to JWT only.

This would involve the removal of ITP views, ITP middleware, cookie class, a good chunk of AuthShopify middleware, and more.

In place, we can promote AuthToken middleware to the forefront, and modify it to utilize Laravel's request context so place in the decoded JWT data into the request lifecycle.

This will allow for the JWT data to be available within the code and views, allowing us to then lookup the user based on the JWT data by request, instead of cookies.

---

A lot of work will be required to complete this, please be patient. If someone sees an issue with promoting JWT to the forefront, please let me know.

[gnikyt]

Open to input here as I'm stuck. The token stuff is fine for SPAs but if I am to remove dependence on cookies, then for non-SPAs, this would be an issue as the token won't be in the next request.

Currently the only thought I had was to have the token appended on URLs and added to forms with helpers. So all links and forms used will have the token. Something like:

{{ token_url('/orders/search') }}
{{ token_url(route('home')) }}

Open to suggestions if others think there's a better way.

[ghost]

Why not store the token in a database like laravel sanctum does?

[gnikyt]

Database is useless. We store the offline token there now, but that doesn't tell Laravel who it is. We can't access the correct information without knowing who.. and if going cookie-less route, this information is in the token.

For SPAs, this is no issue since you just issue the token in the headers with the AJAX requests.

For non-SPAs, this is an issue. Currently my only solution, which seems to be used by other Laravel projects utlizing tokens, is to check for it in the query param.

When I get some time this weekend, I'll be diving in to try all this out.

[alexweissman]

Why not just store the token in some form of client-side storage, and then have a client-side helper that injects it as an Authorization: Bearer header with each request? This would basically be a roll-your-own-cookie without the restrictions imposed on actual third-party cookies.

[arvesolland]

Related to this - is there a good step-by-step guide or docs around how to use this package with JWT for a Laravel SPA?

[LHongy]

What about use turbolink to make non-SPAs acting like SPAs? so it can use ajax requests which can issue token in the request header, is this a possible solution?
@alexweissman I think for non-SPAs, the requests will always be non-ajax, non-ajax requests can't change the request header, right?

[diemah77]

What about use turbolink to make non-SPAs acting like SPAs? so it can use ajax requests which can issue token in the request header, is this a possible solution?
@alexweissman I think for non-SPAs, the requests will always be non-ajax, non-ajax requests can't change the request header, right?

In our Shopify apps we use InertiaJS to handle frontend with backend routing. I guess it might be a better solution to turbolinks.

[gnikyt]

I would rather avoid turbolinks. I don't see an issue with it as part of a post or query param.. it appears many other projects do this. With a blade helper function to automatically append it to the url or form, then it should be fine.

…

On Fri., Mar. 26, 2021, 08:15 diemah77, ***@***.***> wrote: What about use turbolink to make non-SPAs acting like SPAs? so it can use ajax requests which can issue token in the request header, is this a possible solution? @alexweissman <https://github.com/alexweissman> I think for non-SPAs, the requests will always be non-ajax, non-ajax requests can't change the request header, right? In our Shopify apps we use InertiaJS to handle frontend with backend routing. I guess it might be a better solution to turbolinks. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#744 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AASO4OTFEIP2X4YNNLQLOTTTFRQTHANCNFSM4ZLA3ICA> .

[alexweissman]

Isn't it a rather significant security issue to expose access tokens in query string parameters? See https://owasp.org/www-community/vulnerabilities/Information_exposure_through_query_strings_in_url

Seems like authentication in non-SPA contexts has been a major blind spot in the rush to banish third-party cookies from browsers.

[darrynten]

Alex is correct, JWTs should not be passed in query params, they should only ever be in the Authorization header.

[gnikyt]

I'm open to suggestions on how to handle for non-SPAs then. I've been given a good hint that the end goal is to eventually force apps to completely remove dependence on cookies, good to plan for this now than later.

[darrynten]

For non-SPAs, this is an issue. Currently my only solution, which seems to be used by other Laravel projects utlizing tokens, is to check for it in the query param.

You should fetch a fresh token from the frontend app instance before posting, and Shopify will always give you a fresh, valid token so the backend never needs to know who the token is for, you just decode a valid token from Shopify and the URL inside will be valid.

If you mean wanting to know the token from a get request, there isn't really much more of an option than adding it to the query params.

Ideally non-SPA would add something like @hotwired/turbo (not turbolinks) and add a few lines of code to make it work with fetch, but if Turbo is not an option then query params are pretty much your only solution.

[diemah77]

@osiset Are you planning to consume tokens issued by Shopify App Bridge or use a custom implementation like Laravel Sanctum?

[darrynten]

@diemah77 the ones issued by Shopify, which are already supported on the auth.token middleware.

[darrynten]

Related to this - is there a good step-by-step guide or docs around how to use this package with JWT for a Laravel SPA?

Just add the JWT you get to an Authorization header and send it to a auth.token route and it will work.

[darrynten]

The announcement from Shopify that apps must be using JWT in order to be considered for listing.

[gnikyt]

I've decided to go ahead and kill off ITP and cookie dependencies. Will be pushing tokens as the solution. For non-SPAs, since theres no much choice on plain GET/POST, then it'll have to do as a trade off. I'll leave it up to the developer to modify if they are not happy with it as a param.

I'll begin this work tomorrow on my day off.

[gnikyt]

I've started the work this morning on prototyping this.

So initially if no token, then the user will be taken to a route (still behind the auth.shopify middleware) which loads app bridge and gets a token.

Then, redirected to home route with the token and verified (auth.shopify + auth.token) to the home route where you can then use the token with your requests for SPAs or use a Blade helper for appending to URLs.

So far that's what I have planned..

[diemah77]

So auth.shopify will then basically varify hmac?

[gnikyt]

It will handle everything it currently does yes.

[mkmaker]

Hi @osiset thanks for this awesome package.

Any estimate do you have in mind for this as I was planning to go live but Shopify now doesn't accept apps without session based auth? Also, any part of this I can help you with let me know and I will issue a PR.

[gnikyt]

No I don't. I am picking at it slowly.

[darrynten]

@osiset

So initially if no token, then the user will be taken to a route (still behind the auth.shopify middleware) which loads app bridge and gets a token

The user should get sent to a route that has no middleware at all, just a basic page that loads app bridge and grabs a token from Shopify

[gnikyt]

@darrynten That's what I have done :) so far so good.

[gnikyt]

It seems its unknown who the shop is when returning from billing. I have time booked off Wednesday to look at this remaining bug.

[andthink]

Regarding the infinite loop in #744 (comment)

I've found that it happens only on my development server, and not locally

and specifically in

     * Checks the token to ensure its not expired.
     *
     * @throws AssertionFailedException If token is expired.
     *
     * @return void
     */
    protected function verifyExpiration(): void
    {
        $now = Carbon::now();
        Assert::thatAll([
            $now->greaterThan($this->exp),
            $now->lessThan($this->nbf),
            $now->lessThan($this->iat),
        ])->false(self::EXCEPTION_EXPIRED);
    }

here commenting the check the infinite loops stops. I'm trying to understand what happens. Any suggestion?
I was thinking maybe some sort of cache (opcache?) or something like that

[gnikyt]

Only thing I can suggest at the moment is to log all the array values to see which one is false.

…

On Mon., Jun. 21, 2021, 15:15 andthink, ***@***.***> wrote: Regarding the infinite loop in #744 (comment) <#744 (comment)> I've found that it happens only on my development server, and not locally and specifically in /** * Checks the token to ensure its not expired. * * @throws AssertionFailedException If token is expired. * * @return void */ protected function verifyExpiration(): void { $now = Carbon::now(); Assert::thatAll([ $now->greaterThan($this->exp), $now->lessThan($this->nbf), $now->lessThan($this->iat), ])->false(self::EXCEPTION_EXPIRED); } here commenting the check the infinite loops stops. I'm trying to understand what happens. Any suggestion? I was thinking maybe some sort of cache (opcache?) or something like that — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#744 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AASO4OWC4Z24GXPOX4XYLALTT53EJANCNFSM4ZLA3ICA> .

[andthink]

@osiset thanks, i found out that the time of the server was two minutes in the past, and that caused the infinite loop

[andthink]

@squatto Hi!

I've a question about

async function retrieveToken(app) {
  window.sessionToken = await utils.getSessionToken(app);
}

to me window.sessionToken never changes even if it's called every x seconds....

any suggestion on what Can I possibily do wrong?

[gnikyt]

I have booked off some time today to look into the billing issue.
I will also review the PRs.
Thanks everyone... hopefully we can nail out this last issue and get a release up.

[gnikyt]

I'll be reviewing this comment from @restyler, and see what I can come up with in terms of this security issue, maybe we can encode/decode the shop param with the secret key or something... I'll see what I can come up with. We're almost there!

[gnikyt]

Hey everyone... I was able to resolve the billing issue based on previous comments and some modifications. I have merged in the latest PRs sent to master as well, and updated everything to be in-sync.

In my testing today:

• Non-billing/install flow worked correctly
• Billing/install flow worked correctly (recurring and onetime)
• Session tokens are updating (window.sessionToken as well as anything with .session-token class)
• Test cases are passing

If you're available to do so, you're welcome to test the branch and confirm things are fine as well.

If nothing else urgent pops up, I will be formatting some upgrade docs and looking to push this out sometime this or next week.

[LonnyX]

@osiset do you plan to merge cookieless to master?

[gnikyt]

Once the branch is good to do so.

[LonnyX]

Ok good, if have some note of what is still not working right now I will try to help :)

[gnikyt]

I had a couple hours last night to go through it again.

I have not found any red flags to prevent this from going into master for adoption and a beta release ATM. There is quite a bit of pressure to get this out for people's failing apps due to the new requirements.

If no objections, I'll probably merge this into master around 1500 NST, and begin some draft updates to the wiki, upgrade notes, and changelog notes.

[andthink]

HI @osiset

I'm trying to understand how to make to work standard forms

I've added a token input with session-token class into my form (standard not ajax)

when I post my form i get a redirect 302 to the same url, with token parameter attached, but now it's a GET and I've lost all my post parameters so my form cannot be saved.

I was omitting value="" in my <input name="_token" class="session-token">

BUT
I've issues also with ajax forms, but excepting the routes from CSRF made them work. Here I see that the very long token generated by await utils.getSessionToken(app); fails compare with laravel session token that is a lot shorter, so it seems I'm comparing "apples and pears" I really don't understand the relation between the token returned (by shopify?) in utils.getSessionToken(app); and the laravel session one...

@osiset can you confirm that for now we have to except all routes from CSRF to make form work and trust only VerifyShopify middleware?

[gnikyt]

Sadly yes, at the moment, unless someone can come up with a neat work around, CSRF has to be disabled for routes as the shop technically has a new session token from Laravel each route visit. @andthink

Shopify's token and Laravel's are not the same. Laravel has its own tokens which is generated using the secret key of your Laravel instance... normally this would be fine with cookies since its saved in a cookie and passed on to each request. Now, without cookies due to the requirements, this token is useless when hitting a new request because a new login is created thus a new CSRF token is created. This creates a mis-match with the Laravel tokens, which makes CSRF not work since it can't compare them.

[andthink]

Thanks @osiset! I'had the suspect! It's a pity we cannot disable CSRF directly altogether then, but it's ok.
Maybe let's add a note in the wiki :-P

Sadly yes, at the moment, unless someone can come up with a neat work around, CSRF has to be disabled for routes as the shop technically has a new session token from Laravel each route visit. @andthink

Shopify's token and Laravel's are not the same. Laravel has its own tokens which is generated using the secret key of your Laravel instance... normally this would be fine with cookies since its saved in a cookie and passed on to each request. Now, without cookies due to the requirements, this token is useless when hitting a new request because a new login is created thus a new CSRF token is created. This creates a mis-match with the Laravel tokens, which makes CSRF not work since it can't compare them.

[gnikyt]

PR has been opened

[gnikyt]

We have merged the work into master.

Whats next?

1. Draft updates to upgrading wiki
2. Draft updates to installation wiki (if needed)
3. Expanded information on tokens in wiki
4. Removal of any mentions of cookies/ITP in wiki
5. New wiki page for CSRF issues
6. Release as major release, as it has breaking changes

Thanks everyone for your support and getting us to this point of having a working product without the reliance of cookies.

If there is any issues once released, we will of course attempt to resolve them and issue out patches.

Once released, any issues previously pertaining to cookies will be closed.

[squatto]

@squatto Hi!

I've a question about

async function retrieveToken(app) {
  window.sessionToken = await utils.getSessionToken(app);
}

to me window.sessionToken never changes even if it's called every x seconds....

any suggestion on what Can I possibily do wrong?

@andthink sorry to not get back to you sooner! Were you able to figure this out? I've run into situations (typically after leaving the browser open for a while) where the async doesn't resolve once, and it gets stuck forever. Are you never able to get a response?

[andthink]

@squatto Hi!
I've a question about

async function retrieveToken(app) {
  window.sessionToken = await utils.getSessionToken(app);
}

to me window.sessionToken never changes even if it's called every x seconds....
any suggestion on what Can I possibily do wrong?

@andthink sorry to not get back to you sooner! Were you able to figure this out? I've run into situations (typically after leaving the browser open for a while) where the async doesn't resolve once, and it gets stuck forever. Are you never able to get a response?

@squatto i think it was the same situation, cause I tried again with a fresh browser and it worked! Thanks for the answer!

[ingalesachin7]

I have upgraded to 17.0.0 for Removal of dependency on cookies,
I try to install the app getting an authentication error
Exception
Variable $topic of type WebhookSubscriptionTopic! was provided invalid value

after refresh
Osiset\ShopifyApp\Exceptions\MissingAuthUrlException
Missing auth url

Please help anyone to resolve this...Is something is missing while upgrading.?

[squatto]

@ingalesachin7 I'm having the same problem. Can you start a new issue for it and we'll discuss it there?

[squatto]

For those that land here looking for the solution to the WebhookSubscriptionTopic issue, please refer to #866 and the "Creating Webhooks" wiki page for the solution.

[TzoLy]

Just uppercase the topic and replace / with _ like orders/create => ORDERS_CREATE

[gnikyt]

Yes, sorry, forgot to update the docs here, I believe squatto has updated the wiki for this yesterday so the information there should be good now.

…

On Wed., Jul. 7, 2021, 03:52 TzoLy, ***@***.***> wrote: Just uppercase the topic and replace / with _ like orders/create => ORDERS_CREATE — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#744 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AASO4OTZKPASVIDJNPHTPUTTWQBSXANCNFSM4ZLA3ICA> .