churichard commented 2 years ago

Bug report

Describe the bug

Sometimes, the local storage entry for Supabase Auth (supabase.auth.token) gets cleared, causing the user to be logged out of the application. Local storage keys unrelated to Supabase are unchanged and remain persisted, leading me to believe that this is a Supabase issue.

To Reproduce

I'm not entirely sure how to reproduce this. It happens consistently for me, but I don't know the cause. Usually, after logging in and waiting for a few days, the local storage entry gets cleared automatically.

You can try the following:

Implement client-side Supabase Auth
Log into the application
Wait some period of time (usually 1-2 days)
Observe that you are logged out and the local storage entry has been cleared

For details on how I am implementing Supabase Auth, take a look at my public repository.

Expected behavior

supabase.auth.token should never be cleared unless the user explicitly logs out or clears their cache.

System information

This happens on multiple OS's and browsers.

OS: Windows, Android, iOS
Browser (if applies): Chrome, Safari
Version of supabase-js: 1.22.5

Additional context

My JWT expiry is 604800, but I've also used 60 before with the same behavior happening. It seems to be unaffected by the JWT expiry value.

This happens both on localhost and in production.

I'm not sure about the implementation details of Supabase Auth, but what could be happening is that the refreshToken that I am passing into the signIn method is expired, so the user gets automatically logged out and the local storage entry is cleared. What should happen is that the token is automatically refreshed, the user stays signed in, and the local storage entry is preserved/updated.

Edit: I've noticed this happening sometimes after I deploy. Is there something that changes after each deployment that invalidates the session? I'm using Vercel.

adam-beck commented 2 years ago

Check for any log messages in your browser's console. Waiting a few days and expecting the session to still be persisted is a vulnerability.

Eventually, the access token will expire. If the refresh token cannot obtain a new access token then the session is automatically deleted.

churichard commented 2 years ago

No messages are logged in the console.

It's fine and good that the access token expires, but a new access token should be retrieved when this happens. The user should not be logged out. If you look at popular websites like Google, YouTube, even GitHub, you're pretty much always logged in, and I expect the same to happen for my website. It's a big problem for me because many of my users are on mobile devices where it's a hassle to keep logging in every day.

For what's it worth, Firebase Auth does not have this problem. You are always logged in unless you explicitly log out.

It's also fine if Supabase Auth chooses not to make this the default behavior (although I definitely think it should be), but there should be some way to keep the user signed in.

adam-beck commented 2 years ago

So after doing some more digging it doesn't look like there is anything explicitly deleting the session. I don't know a good way to reproduce this either. I did something stupid and provided you an assumption: that eventually the refresh token would stop working. That, however, doesn't seem to be the case. Every 3600 seconds (i.e. 1 hour) the refresh token is used to obtain a new access token. This is perpetual until the user signs out.

Is there anything in the auth.audit_log_entries table that would point to the cause? Going through your code, do you need the following lines:

  // Initialize the user based on the stored session
  const initUser = useCallback(async () => {
    const session = supabase.auth.session();
    await supabase.auth.signIn({
      refreshToken: session?.refresh_token,
    });
    setIsLoaded(true);
  }, []);

  useEffect(() => {
    initUser();
  }, [initUser]);

https://github.com/churichard/notabase/blob/main/utils/useAuth.tsx#L35-L46

I'm wondering if the refresh_token is somehow used twice which would cause an issue. If you have a supabase.auth.session(), what is the purpose of signing in? Wouldn't you already be signed in?

churichard commented 2 years ago

The docs are rather lacking for auth, so I'm not exactly sure how to restore a user's session; it would be great to have an example of that. In my code, I kind of just did what seemed reasonable to me, and it works fine except for this one issue.

My reasoning is that even though a session is stored in local storage, it will eventually expire. Does Supabase automatically restore expired sessions somehow (like on the next network request, or does it coincide with the every 3600 seconds for the refresh token)? I recall getting 401 errors when making requests sometimes before I switched to my current implementation, but maybe that's been fixed now.

In my audit log, it seems like every time I refresh or use the app, there is a token_refreshed and token_revoked action happening. I tried removing supabase.auth.signIn and those two actions don't happen anymore when I refresh the app.

adam-beck commented 2 years ago

So, the docs could probably use some TLC regarding auth but if you look into the gotrue-js library (which handles client-side auth for supabase), you'll find that it restores the session automatically when initializing supabase (assuming the refresh token exists in LocalStorage).

/**
   * Recovers the session from LocalStorage and refreshes
   * Note: this method is async to accommodate for AsyncStorage e.g. in React native.
   */
  private async _recoverAndRefresh() {
    try {
      const json = isBrowser() && (await this.localStorage.getItem(STORAGE_KEY))
      if (!json) {
        return null
      }

      const data = JSON.parse(json)
      const { currentSession, expiresAt } = data
      const timeNow = Math.round(Date.now() / 1000)

      if (expiresAt < timeNow) {
        if (this.autoRefreshToken && currentSession.refresh_token) {
          const { error } = await this._callRefreshToken(currentSession.refresh_token)
          if (error) {
            console.log(error.message)
            await this._removeSession()
          }
        } else {
          this._removeSession()
        }
      } else if (!currentSession || !currentSession.user) {
        console.log('Current session is missing data.')
        this._removeSession()
      } else {
        // should be handled on _recoverSession method already
        // But we still need the code here to accommodate for AsyncStorage e.g. in React native
        this._saveSession(currentSession)
        this._notifyAllSubscribers('SIGNED_IN')
      }
    } catch (err) {
      console.error(err)
      return null
    }
  }

My advice: try to remove the explicit call to signIn on initialization and see if the problem is resolved.

churichard commented 2 years ago

Got it, I'll try it out 👍

I'll give it a few days or so to see if I still run into the same issue.

churichard commented 2 years ago

After trying this method out for a few weeks, I'm still getting logged out randomly. But now, I have the additional problem of sometimes having 401 errors when sending requests 😅

I think the 401 errors might be resolved by https://github.com/supabase/gotrue-js/issues/403, but getting logged out randomly is probably a separate issue.

denull0 commented 2 years ago

After trying this method out for a few weeks, I'm still getting logged out randomly. But now, I have the additional problem of sometimes having 401 errors when sending requests 😅

I think the 401 errors might be resolved by supabase/gotrue-js#403, but getting logged out randomly is probably a separate issue.

I am in the same boat...

davut commented 2 years ago

Why it takes too long to solve this issue :( ?

markwcollins commented 2 years ago

Might not be related but I noticed that supabase.auth.session() may return null when first called, even if there is a user token in storage. When I saw null, I assumed the user was logged out, but in fact, supabase was refreshing the token behind the scenes. see this, which contain more context and a solution. https://github.com/supabase/gotrue-js/issues/23

TeddyLourson commented 2 years ago

I'm having the same problem where I get logged out randomly. It mostly happens when refreshing the page after some amount of time. I tried putting a low JWT expiry time (30 seconds) and a console.log() inside the onAuthStateChanged(). If I keep the app running, it's all fine and the token is refreshed. What logs me out is when I try reloading the page near the 30 seconds mark (let's say at 28 seconds I start reloading the page). Then, the user gets logged out and I get an error in the console saying the token is invalid.

ydennisy commented 2 years ago

+1 same issue as @Teio07

naripok commented 2 years ago

Maybe this is related?

TeddyLourson commented 2 years ago

It was just a mistake on my side, after I fixed it, I haven't been logged out for a while.

naripok commented 2 years ago

It was just a mistake on my side, after I fixed it, I haven't been logged out for a while.

Would you mind sharing the gist of it, @Teio07? Maybe it will help me fix it on my side :laughing:

TeddyLourson commented 2 years ago

I don't know if it will help you because it was pretty specific to my implementation, but here is how I solved it. I am listening to the person table which is a table that I created and linked to the users table generated by Supabase (it's just holding more details about the user.) I created a layer of abstraction to listen for changes and return an Observable that returns Either a Failure or an instance of the retrieved/updated Person. The error I had was when I tried to unsubscribe from the stream when refreshing the page. I had a warning saying that the stream was already closed or something. All I had to do was to remove the call from the return method of the useEffect hook for it to work.

  useEffect(() => {
    let handleSessionSubscription: Subscription | null;
    const maybeSession = supabase.auth.session();
    handleSessionSubscription = handleSession(maybeSession);
    const { data: subscription } = supabase.auth.onAuthStateChange(
      (_event, session) => {
        handleSessionSubscription = handleSession(session);
      }
    );
    return () => {
      // Here be dragons 🐉
      // 👇 This was giving me an error, I removed it
      handleSessionSubscription?.unsubscribe();
      subscription?.unsubscribe();
    };
  }, []);

My handleSession method :

  const handleSession = (session: Session | null) => {
    if (null === session || null === session.user) {
      setAuthStateUnauthenticated();
      return null;
    }
    const user = session.user;
    const id = new UniqueId(user.id, 'fromUniqueString');
    return personRepository.watchPerson(id).subscribe((failureOrPerson) => {
      failureOrPerson.fold(toastFailure, (personSuccess) => {
        const person = personSuccess.data;
        setAuthStateAuthenticated({ person: person });
      });
    });
  };

billscheidel commented 2 years ago

Has there been any update on this? I thought the tokens were supposed to be refreshed automatically but I've never seen that to be the case in the logs.

2022-04-06 23:03:15Z | token_revoked | d8a90912-0757-445d-be17-437735100ca0 | foo@example.com
2022-04-06 23:03:15Z | token_refreshed | d8a90912-0757-445d-be17-437735100ca0 | foo@example.com

Every time a token is attempted to be refreshed it seems like it is just immediately revoked.

saiabishek1 commented 2 years ago

Facing the same issue as @billscheidel mentioned. @thorwebdev do you know if there is a fix for this when using auth-helpers? Or @garyaustin1 do you any potential solutions for this?