Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Social Authentication, when user already exists #154

Open
pelmenept opened this issue Mar 17, 2020 · 7 comments
Open

Social Authentication, when user already exists #154

pelmenept opened this issue Mar 17, 2020 · 7 comments
Labels
duplicate This issue or pull request already exists

Comments

@pelmenept
Copy link

Hi! First thank you very much for the library, pretty much the only authentication for phoenix.

I am experiencing some difficulties with authentication flow using social providers when user already exists. Let me know if I am wrong, but pretty much steps to reproduce:

I have below extensions installed:

  1. PowInvitation
  2. PowEmailconfirmation
  3. PowAssent

In my app, users can invite other users. This flow seems to be working correctly. Once user invites another one, they receive email. Here is where it breaks for me:

User has 2 ways of continuing after confirming email:

  1. Enter password, and log in -> works perfectly fine
  2. Click on "sign in with google", get couple redirects, and get logged in -> works perfectly fine.

As long as users use the same method to login, the way they logged in first, there are no problems.

  1. If user confirmed email using google authentication, they are still able to "reset password" and then login. This user now can login using both google or password login methods.
  2. Hovewer user that has confirmed email and set up password, is not able to login using google authentication anymore. The error that I am getting with this approach "failed to insert into users table due to constraint". Which seems pow_assent is trying to insert user, and then insert user_identity, which does not work, since user already exists.

After some digging, the flow for authentication or upsert is done in PowAssent.Plug:

defp maybe_authenticate(
         %{
           private: %{
             pow_assent_callback_state: {:ok, :strategy},
             pow_assent_callback_params: params
           }
         } = conn
       ) do
    user_identity_params = Map.fetch!(params, :user_identity)

    case Pow.Plug.current_user(conn) do
      nil ->
        conn
        |> authenticate(user_identity_params)
        |> case do
          {:ok, conn} -> conn
          {:error, conn} -> conn
        end

      _user ->
        conn
    end
  end

Up until maybe_authenticate call everything works correctly. But "authenticate" call only tries to pull user from user_identities and not from users. Thus user is not found and nil is returned. Since authentication failed, assent tries to insert user, but failes, since user exists in "users" table but not in "user_identities" table.

authenticate code:

  @spec authenticate(Conn.t(), map()) :: {:ok, Conn.t()} | {:error, Conn.t()}
  def authenticate(conn, %{"provider" => provider, "uid" => uid}) do
    config = fetch_config(conn)

    provider
    |> Operations.get_user_by_provider_uid(uid, config)
    |> case do
      nil -> {:error, conn}
      user -> {:ok, Plug.create(conn, user)}
    end
  end

Thank you very much, please let me know if I am wrong somewhere.

@danschultzer
Copy link
Collaborator

danschultzer commented Mar 17, 2020

Thanks!

  1. Hovewer user that has confirmed email and set up password, is not able to login using google authentication anymore. The error that I am getting with this approach "failed to insert into users table due to constraint". Which seems pow_assent is trying to insert user, and then insert user_identity, which does not work, since user already exists.

Can you help me understand the exact state the user is in here? They were invited, clicked the link, update with password, and then they try to sign in with google after?

And is failed to insert into users table due to constraint the exact error message, if not can you post the one, and possibly stacktrace?

It sounds like the issue experienced in #113 and I'm thinking of resolving like #115.

@pelmenept
Copy link
Author

pelmenept commented Mar 17, 2020

For sure.

The state of the user:

  1. They were invited
  2. They clicked the invite link in the email
  3. They set up password
  4. They got automatically logged in
  5. They decided to log out
  6. Next time they want to log in with google. They click "log in with google", go through authentication steps, and end up on an "/auth/google/add-user-id" with an error "Oops, something went wrong!. Email has already been taken".

It used to just error out on Repo.Insert into users, but now it handles error (Not sure if it is due to recent update)



08:32:30.650 request_id=Ff0XYTdaIOfbOe8AAAyB [debug] QUERY OK db=0.1ms idle=8470.1ms
begin []
%Plug.Conn{
08:32:30.653 request_id=Ff0XYTdaIOfbOe8AAAyB [debug] QUERY ERROR db=2.9ms
INSERT INTO "users" ("email","email_confirmed_at","first_name","last_name","role","thumbnail","inserted_at","updated_at") VALUES ($1,$2,$3,$4,$5,$6,$7,$8) RETURNING "id" ["[email protected]", ~U[2020-03-17 12:32:30Z], "first_name", "last_name", "user", "https://lh3.googleusercontent.com/a-/Arandom_keyw", ~U[2020-03-17 12:32:30Z], ~U[2020-03-17 12:32:30Z]]
08:32:30.654 request_id=Ff0XYTdaIOfbOe8AAAyB [debug] QUERY OK db=0.1ms
rollback []

It seems to rollback transaction now on error.

Basically social login will always fail if user already exists in database in "users" table but has no identity assigned in "user_identities"

thank you

Edit: It looks like a similar issue to the ones you referenced, but I am not 100% sure. What I am struggling to understand, is that authentication through google works fine if user already exists in "users" table (was invited) AND query string contains "invitation token". Then POW does not try to insert user, but only inserts user_identity, and works correctly.
So how do I turn authentication through similar flow on regular logins, when user exists in "users" but identity is not yet setup?

@danschultzer
Copy link
Collaborator

It's because when the user follows the invite link, the struct is already loaded in the conn, so it's in an "authenticated" state and the identity will be upserted. The same happens when a user is authenticated and then links a provider. When there's no user in the conn the logic will instead attempt to create identity and user at the same time.

This is the same as discussed in the linked issues.

So a constraint error will occur and the user is redirected to set the e-mail manually. I think permitted verified provider emails to automatically link up with the user could work, but really #115 is the best solution. Ideally you have to authenticate with an existing method for your account before you can add a new auth method. I hope I can open a PR to do this very soon.

If you want to do something similar to the above, then I would recommend that you create a custom controller overriding the :add_user_id logic. You could load the provider params or changeset stored in the session, and then attempt to load the user based on the email in either of those.

@pelmenept
Copy link
Author

Thank you very much for your help.

@akashvibhute
Copy link

@danschultzer As this issue is open, I will add another scenario here that I came across related to social auth.

I'm using both pow & pow_assent for api only authentication.

I have set user_id_field: :username on my user schema.

First I created a user using username say 'akash' along with email/password which worked just fine.

The issue comes when I'm trying to register another account with oauth (discord in this case).

The discord account returns the same username akash when registering with discord (has different email than first user).

This throws a unique constraint error (users_username_index) as a user with akash as username already exists.

How can I handle such cases? (I think probability of this occuring is high with discord as discord allows having same username for multiple users and differentiate with username#12123, but returns only the part before # in oauth callback flow).

I'm not sure how to handle it within my user model, would always doing a update_change on username field to add random string (and allow the user to change username later ) in the assent changeset (user_identity_changset) be a good idea?

@danschultzer
Copy link
Collaborator

How can I handle such cases? (I think probability of this occuring is high with discord as discord allows having same username for multiple users and differentiate with username#12123, but returns only the part before # in oauth callback flow).

I'm not sure how to handle it within my user model, would always doing a update_change on username field to add random string (and allow the user to change username later ) in the assent changeset (user_identity_changset) be a good idea?

Yeah, this would be the easiest solution!

It's not great though, what would be better UX would be that the user corrects the username themselves. This goes together with the refactor I want to do with #195 #170 and this one, to allow for more dynamic handling of changeset errors. Ideally any errors that the end-user can fix should be displayed to them, and anything apart from that should result in redirect error.

@akashvibhute
Copy link

Thanks, my current implementation is to add a random string with # prefix and complete the upsert as I did not see any way to take additional input from user with the api and keep the oauth flow going. I am adding additional logic on frontend to redirect user to change username if it is marked to change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

3 participants