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

Users are able to register with identity and impersonate oauth accounts #23

Open
mercury2269 opened this issue Mar 13, 2016 · 5 comments

Comments

@mercury2269
Copy link

It seems like a bug, to reproduce.

  1. Authenticate with google -> creates user record with email address from google and authorization
  2. Logout
  3. Create a new identity account with username/password with the same email address -> creates new authorization for the existing user.
  4. New user has got access to the account originated from external providers.
@hassox
Copy link
Owner

hassox commented Mar 14, 2016

hmm I wonder if there's an alternative UID for the google authorization we could use instead of an email so that the linkage isn't there :\

@hassox
Copy link
Owner

hassox commented Mar 14, 2016

I haven't yet merged the google integration, I think we should hold off until we to the bottom of this.

@hassox
Copy link
Owner

hassox commented Mar 14, 2016

@mercury2269 I believe this is the offending line: https://github.com/hassox/phoenix_guardian/blob/ueberauth-guardian/web/auth/user_from_auth.ex#L73

That should not lookup the user when current_user is nil. I'm wondering if it should do it at all really. Looking at it I don't think it should.

Thoughts?

@mercury2269
Copy link
Author

Maybe before creating a new user from authorization there should be a check to see if the user already exists with the same email address and fail if it does.

@ben-ic
Copy link

ben-ic commented Jul 18, 2016

I see 2 possibilities.

  1. Only allow the addition of a new login method when the user is logged in. In that case if someone tries to register a new user, you should give email already exists(I think this is the best option)
  2. Add a confirmation email step if someone tries to register a user in which the email already exists(should probably add an email confirmation option anyhow).

I changed create_user_from_auth so that you have to be logged in to connect a user to an existing account.

     defp create_user_from_auth(auth, current_user, repo) do
        user = current_user
        if !user, do: user = repo.get_by(User, email: auth.info.email)
        if !is_nil(user) and is_nil(current_user) do
            {:error, :user_exists}
        else
            if !user, do: user = create_user(auth, repo)
            authorization_from_auth(user, auth, repo)
            {:ok, user}
        end
     end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants