supabase / auth

A JWT based API for managing users and issuing JWT tokens
https://supabase.com/docs/guides/auth
MIT License
1.54k stars 373 forks source link

Anonymous user identity not linking #1525

Open muezz opened 7 months ago

muezz commented 7 months ago

Describe the bug If I sign in using Google, re-install the app, open it (which automatically signs the user in anonymously) and then try to link identity using the Google provider, I get an error.

To Reproduce Steps to reproduce the behavior:

  1. Create a fresh Flutter project and add supabase_flutter as a dependancy.
  2. Run the project and set up a button to performt he following action:
    await Supabase.instance.client.auth.signInWithOAuth(
    OAuthProvider.google,
    redirectTo: 'com.my_app://login-callback/',
    authScreenLaunchMode: LaunchMode.externalApplication,
    );
  3. After signing up successfully, uninstall the app from the simulator.
  4. Add the following code before runApp():
    if (Supabase.instance.client.auth.currentSession == null) {
    await Supabase.instance.client.auth.signInAnonymously();
    }
  5. Update the onTap callback on the previous button to trigger the following code:
    await Supabase.instance.client.auth.linkIdentity(
    OAuthProvider.google,
    redirectTo: 'com.my_app://login-callback/',
    authScreenLaunchMode: LaunchMode.externalApplication,
    );
  6. You will be able to sign in with google successfully but as soon as you get back to the app, you will get this error: AuthException(message: Identity is already linked to another user, statusCode: null)

Expected behavior The anonymous user ID (created on the second app run at step 4) should get merged with the already existing user ID from step 2.

Version: ├── supabase_flutter 2.5.0 │ ├── supabase 2.1.0 │ │ ├── functions_client 2.0.0 │ │ ├── gotrue 2.6.0 │ │ ├── postgrest 2.1.1 │ │ ├── realtime_client 2.0.3 │ │ ├── storage_client 2.0.1

Additional context This also brings up a question about what would happen to the data created by the anonymous user? Ideally, I would expect that data to be "moved" to the previously existing user.

dshukertjr commented 7 months ago

I'm going to transfer this issue to our Auth server repo to discuss further.

muezz commented 7 months ago

@dshukertjr any update on this issue?

dshukertjr commented 7 months ago

@muezz Actually, I was reading through the issue again, and the behavior you outlined is the expected behavior.

You simply cannot have two auth users to be tied to a single Google OAuth account, and that is what the error message is saying. When you get the Identity is already linked to another user error message, what you can do is to just perform the regular Google sign-in using either signInWithIdToken or the signInWithOAuth. Once the sign-in is complete, the user will be brought back to the original account. Note that when this happens, the activities from the second anonymous account will be lost, so you are going to have to set something up manually if you want to persist those data.

hala94 commented 4 months ago

@dshukertjr

Maybe I misunderstood something, but this breaks the flow. ( repeated login gets weird double redirect UX )

Existing user can always log out and another "anonymous" user be created on the same device, probably automatically for most use cases. This means that when they try to "sign in" again -> we have an option to call either linkIdentity or signInWithOAuth -> but we cannot know which one to call, the current session user is anonymous again.

(I'm using Next.js, server side flows)

dshukertjr commented 4 months ago

Fair enough. Passing the feedback to the auth team.

Let me just re-open this issue.

silentworks commented 4 months ago

@hala94 I think what @dshukertjr stated is correct and a fine way of going about it. The only change I would make to his recommendation is that after the Identity is already linked to another user message is given to a user, they should be given a sign in button to perform the signInWithOAuth. The reason why it would be bad for linkIdentity to perform the signInWithOAuth internally is that if a user was signed in already via email/password lets say and they now decide they want to linkIdentity to a OAuth provider and it turns out that the OAuth they are trying to link is already linked to another account, they would get signed out of their current session (email/password session) and then signed back into the OAuth session instead which is a completely different account all together. I think with the current implementation, it allows the developer to chose the flow they want to provide for the user of their app.

NB: I'm not a Supabase employee, just to be clear that this is my personal take on this matter.

sapphire008 commented 4 months ago

Related to this, when linking the user via email, updateUser works when updating email, but throws AuthException(message: Anonymous user cannot update password) when also specifying a password. It would be nice to allow them to sign up with email and update their password.

LeakedDave commented 3 months ago

@sapphire008 Definitely bumping this, I won't be able to use this feature at all until it allows normal sign up flows with passwords

raymondctc commented 3 months ago

@muezz Actually, I was reading through the issue again, and the behavior you outlined is the expected behavior.

You simply cannot have two auth users to be tied to a single Google OAuth account, and that is what the error message is saying. When you get the Identity is already linked to another user error message, what you can do is to just perform the regular Google sign-in using either signInWithIdToken or the signInWithOAuth. Once the sign-in is complete, the user will be brought back to the original account. Note that when this happens, the activities from the second anonymous account will be lost, so you are going to have to set something up manually if you want to persist those data.

Hello, I have an issue with linkIdentity. When user is trying to link a social account that has been linked with another account, the user will be forced to logout automatically. Can we have a control on that? Even with a custom server callback, the user will still be forced logout.

What I want to do is to just simply display an error message to the user saying that the account that they are trying to link is already linked.

eifr commented 3 months ago

The only mention to this kind of behavior is here, but there is no "how to" actually solve it. How do I merge the "carts"?

jonnylangefeld commented 2 months ago

There is also a technical issue here that prevents me from "merging the carts" in the first place: As I get redirected to my own page after the oAuth flow, I get the query parameters ?error=server_error&error_code=422&error_description=Identity+is+already+linked+to+another+user. So I should be able to handle the error as described here. But the issue is that at this point the sb-127-auth-token cookie for the anonymous user is gone. So I don't have the data of the anonymous user anymore at this point. So how could I "merge the carts" now?

kangmingtay commented 2 months ago

@sapphire008 @LeakedDave this is possible now since https://github.com/supabase/auth/pull/1739, we're planning to roll it out to the hosted platform soon

kangmingtay commented 2 months ago

Hello, I have an issue with linkIdentity. When user is trying to link a social account that has been linked with another account, the user will be forced to logout automatically. Can we have a control on that? Even with a custom server callback, the user will still be forced logout.

@raymondctc i'll look into this but i think we're removing the session if an error is returned by linkIdentity in the JS client lib

kangmingtay commented 2 months ago

@eifr @jonnylangefeld

this would be the way to handle merging:

// sign in anonymously
const { data, error } = await supabase.auth.signInAnonymously()

// try to link to a user 
const { data, error } = await supabase.auth.updateUser({ email: "existing_user@example.com" })

// or if you want to link an oauth identity
const { data, error } = await supabase.auth.linkIdentity({ provider: 'google' })

// since the user is an existing user, an error will be thrown
// handle the error by just asking the user to sign in to the existing account
const { data: { user: newUser} , error } = await signInWithPassword(...)

// reassign any entities tied to the anonymous user previously
const { data, error } = await supabase.from("table1").update().eq("id", newUser.id)

thanks for calling this out, i realised an outline of this is missing from the docs and it would be useful to add this to it

But the issue is that at this point the sb-127-auth-token cookie for the anonymous user is gone. So I don't have the data of the anonymous user anymore at this point.

@jonnylangefeld, i think the issue here is because linkIdentity removes the session locally if an error is returned - will look into a fix for this!

trevorpfiz commented 1 month ago

@eifr @jonnylangefeld

this would be the way to handle merging:

// sign in anonymously
const { data, error } = await supabase.auth.signInAnonymously()

// try to link to a user 
const { data, error } = await supabase.auth.updateUser({ email: "existing_user@example.com" })

// or if you want to link an oauth identity
const { data, error } = await supabase.auth.linkIdentity({ provider: 'google' })

// since the user is an existing user, an error will be thrown
// handle the error by just asking the user to sign in to the existing account
const { data: { user: newUser} , error } = await signInWithPassword(...)

// reassign any entities tied to the anonymous user previously
const { data, error } = await supabase.from("table1").update().eq("id", newUser.id)

thanks for calling this out, i realised an outline of this is missing from the docs and it would be useful to add this to it

But the issue is that at this point the sb-127-auth-token cookie for the anonymous user is gone. So I don't have the data of the anonymous user anymore at this point.

@jonnylangefeld, i think the issue here is because linkIdentity removes the session locally if an error is returned - will look into a fix for this!

Is there an update on this? I am trying to handle linkIdentity gracefully, but I am losing the session as @jonnylangefeld pointed out. Thanks!