supabase / auth-js

An isomorphic Javascript library for Supabase Auth.
MIT License
354 stars 159 forks source link

Session not saved after `updateUser()` #709

Open imageck opened 1 year ago

imageck commented 1 year ago

Bug report

Describe the bug

Local session is not updated after calling updateUser(). The session data remains the same.

To Reproduce

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

  1. Have some metadata to update, e.g. posts.
  2. updateUser({ data: { posts: count + 1 } }) after user successfully creates a new post.
  3. Read the user metadata with useUser or reload the page.
  4. The count is not increased.

Expected behavior

Client-side session data and token should be updated to reflect the changes.

System information

Additional context

After a PUT request to update the user, the retrieved session data is supposed to be saved locally with _saveSession(). That, in turn, hands it down to _persistSession(). During a debugger session, first two arguments to setItemAsync(this.storage, this.storageKey, currentSession) appear to be undefined. This could be the reason why the JWT remains outdated client-side.

It appears that the new session data is lost in stringifySupabaseSession(). As you can see, it only returns a select number of items, the rest is extracted from the current token. So when it's time to modify the session cookie, the new session data is mixed with the old one. As a result, most of the data modified in updateUser() is not recorded in the cookie. https://github.com/supabase/auth-helpers/blob/6e5b5b37dacd07ca74f7486419dda6b10bf14730/packages/shared/src/utils/cookies.ts#L69

imageck commented 1 year ago

I see now. UserUpdate() only returns the user portion of the data and doesn't refresh the JWT. This is why stringifySupabaseSession() returns the old value and the new data is never recorded in the cookie.

In my view, the session data should be refreshed as well (and consequently, a new access token generated) because the client is updating the user data explicitly.

As a workaround, the client is allowed to refreshSession() but that's another round trip…

j4w8n commented 1 year ago

Looking at the code, I see an opportunity where it could be changed to refresh the session on the dev's behalf, but it'd still be another trip.

imageck commented 1 year ago

Yes. Perhaps it should be done in UserUpdate()?

ivandamyanov commented 6 months ago

What's the proper approach here? I am using user metadata for some functionality and after updating the user and deleting said metadata, my session is outdated and the user object inside still has the deleted metadata, causing a loop. We manually need to refresh the session after updating a user?

J0 commented 4 weeks ago

Hey @ivandamyanov,

Thanks for the feedback. For now, manually refreshing might be the way to go. Unfortunately we can't change the behaviour as it would be a breaking change for folks who may depend on the metadata not being refreshed.

Refreshing within the function is an option though it would also add a round trip as @j4w8n mentioned which not everyone might want.

We'll take this as feedback for future feature development though