supabase.auth.logout() throws "Invalid user" error.

[thorwebdev]

I found this to only happen sometimes, but couldn't figure out why:

@kiwicopple have you seen this before?

[kiwicopple]

The only thing I could think is that the token is expired when you try to log out. A couple of questions:

• Is the browser left open, and then you try to log out after >3 hours?
• There is a logout network call about 10 requests above the erroneous network call. Did you hit the button twice? Perhaps you're logged out already and the redirect to the home screen failed for some reason

[thorwebdev]

Is the browser left open, and then you try to log out after >3 hours?

I'm able to reproduce this multiple times within a time window of less than 1h

There is a logout network call about 10 requests above the erroneous network call. Did you hit the button twice? Perhaps you're logged out already and the redirect to the home screen failed for some reason

Yes, there are times where the logout call is successful. You can see in the network comms though that after the logout there is a login (token?grant_types=password, right?). So don't think it's that.

[awalias]

@thorwebdev are you just using the OG slack clone example here?

[thorwebdev]

@awalias yes, I'm able to observe this also on the official https://supabase-slack-clone.vercel.app/

[awalias]

could be some race-condition here? https://github.com/supabase/supabase/blob/1e49eaeb7e0540c9cc1e4dad627e779a33b0500f/examples/slack-clone/pages/_app.js#L43

[awalias]

issue is when a user instantiates two or more supabase clients - fix is to always check localStorage before adding this.accessToken as an auth bearer token to see if there is one on the browser

[phamhieu]

It's gotrue logout api bug. The cookie is cleared before calling getUserFromClaims .

[phamhieu]

On slack-clone app, after an Invalid user error fires, I can login and logout successfully!!! no idea why the logout can goes through getUserFromClaims check

However the next login/logout will trigger Invalid user error again.

[awalias]

Oh we didn't update this issue after meeting @thorwebdev last week about this.

The issue here is that Slack-clone app uses two different supabase client instances, and supabase-js only reads from local storage on initiation, and manages it's own state of which user is logged in after that point - so multiple clients can easily get out of sync on current user state

Our options here are:

1. supabase-js should check local storage before each call to see if there has been a change in user auth by a different client instance. This may be expensive

2. we direct people to only init a single supabase client if they're using auth

3. we make createClient return a singleton

any other?

[phamhieu]

@awalias what you describe seems to be another issue related to front-end.

The original error reported by @thorwebdev is from gotrue logout api. It can be reproduced easily.

1. start your test supabase project, run slack clone quick start to create required tables
2. pull slack clone app, update env with your test project keys and run it
3. login and logout multiple times slack-clone client (with the same acc and client) you can see the error

I already check logout api on gotrue. The last commit on logout.go breaks the check mechanism.

Before
it works properly, because the claim is retrieved before clearing token
https://github.com/netlify/gotrue/blob/47cc9ce137a24c96985ee3e742b0f0adfb6f146c/api/logout.go

After
https://github.com/netlify/gotrue/blob/8304885327eb93a7346f4b27658f470499c39107/api/logout.go

[awalias]

I think the response from gotrue is actually correct in this instance, if you look at the request headers on the slack clone app, you will see that the apikey and auth bearer headers are the same, in this case the jwt being (mistakenly) sent is the anon key:

for comparison here is an example of a successful logout, with decoded jwt below:

The bug seems to be that the supabase-js client calling logout does not have the current user token, since it was already cleared from the client by the other instance

the "double logout" seems to be coming from here: https://github.com/supabase/supabase/blob/fed822f48c5e441eb867fa756443e362ac47423f/examples/slack-clone/components/Layout.js#L59

and here: https://github.com/supabase/supabase/blob/fed822f48c5e441eb867fa756443e362ac47423f/examples/slack-clone/pages/channels/%5Bid%5D.js#L16

[awalias]

also as a side note - we actually don't make use of the cookies set by go-true, we manage these ourselves using local storage inside supabase-js

[phamhieu]

Thanks @awalias for clarification. After modifying slack clone example to use a single Supabase client, everything works properly.

[phamhieu]

Currently, supabase-js persists accessToken, refreshToken and currentUser to localstorage while also keeps them as class params inside supabase.auth. When we have multi supabase clients, these params can be out of sync.

We should use localstorage as the source of true and don't keep them as class params. It's the same as how we get authHeader to supply PostgrestClient. What do you think? @kiwicopple

[awalias]

1. is it inefficient to fetch from local storage every time?
2. what about server-side / node

maybe keep track of them internally but always check local storage first (if it exists?)

[phamhieu]

1. right now everytime you call a request with postgrest client, it will read localstorage for accessToken to include in the header. It works ok until now. So i think it's efficient enough.
   
2. for server-side, accessToken should included in the header. To get refreshToken and currentUser we can call gotrue /user endpoint with accessToken