auth-js icon indicating copy to clipboard operation
auth-js copied to clipboard

Functions that call `_removeSession` internally do not trigger `SIGNED_OUT` event when the function fails

Open bombillazo opened this issue 6 months ago • 5 comments

Bug report

  • [x] I confirm this is a bug with Supabase, not with my own application.
  • [x] I confirm I have searched the Docs, GitHub Discussions, and Discord.

Describe the bug

When using the client and auth functions, some internally call a _removeSession function, presumably to clear the session before attempting any auth-related logic. This effectively clears out the local session data used to keep the user logged in. In many cases the function runs and restores or resets the session data eventually. However, if the function fails, this does not trigger the SIGNED_OUT to notify any even handlers and hooks that the session data is no longer present, causing inconsistent auth data states.

To Reproduce

Steps to reproduce the behavior, please provide code snippets or a repository:

  1. Create a supabase client instance.
  2. In your app, use the auth-helpers-react lib and use the useSession hook to get the session data.
  3. Trigger a function like supabase.auth.resend with invalid params to trigger an error and session remove.
  4. This causes the session data to be erased, but the context data of the useSession hook is not updated and retains the old values.

Expected behavior

Any auth function that fails and deletes the auth session data should trigger a SIGNED_OUT event to notify any other serives.

System information

  • Version of supabase-js: 2.64.2

bombillazo avatar Feb 15 '24 21:02 bombillazo

Any update on this issue?

bombillazo avatar Apr 19 '24 12:04 bombillazo

@bombillazo hmm there shouldn't be a session return by useSession when the supabase.auth.resend method is called because no user is actually signed in yet

kangmingtay avatar May 09 '24 09:05 kangmingtay

We can call resend to change email or phone. I dont expect users to be signed out for this

bombillazo avatar May 09 '24 11:05 bombillazo

@bombillazo yeah we handle that case already by not removing the session if the type passed indicates an email change or phone change

kangmingtay avatar May 09 '24 17:05 kangmingtay

This issue is more related to what happens when something fails with the auth calls while using the useSession react hook that returns the session. Since this library isn't properly triggering the SIGNED_OUT event, the react hook keeps returning the cached session when an auth call fails and erases the session from the local client.

The PR I created fixes this: https://github.com/supabase/auth-js/pull/854/files#diff-3522461172efd6058d6b8da62fc2d30d8b524d2b64894ea2c67218c52f7fdff5R1960

bombillazo avatar May 09 '24 18:05 bombillazo

This is still open and should be fixed with #854

bombillazo avatar Jun 18 '24 17:06 bombillazo