Existing users not recognized

[adamwathan]

Existing users not recognized

[adamwathan]

The latest release allows you to override which user gets matched on login by returning a user from the closure, like so:

SocialAuth::login('facebook', function ($user, $details) {
    $existingUser = User::where('email', $details->email)->first();

    if ($existingUser) {
        return $existingUser;
    }

    return $user;
});

I will never bake this in out of the box because I still think it's difficult to implement securely in a generic way across all applications, but if you are comfortable with the risks and think you can work out any security issues, it's possible now and you are free to do what you choose :)

Closing this as a result.

[adamwathan]

Hey! Yeah this is a pretty common question, the package makes no attempt to reconcile accounts created via social login with accounts created using email/password.

The reason for this is that it introduces a security vulnerability, discussed here: adamwathan/eloquent-oauth#19 (comment)

I'd like to make a couple changes soon that would allow people to add this sort of behavior themselves, but I'll never make it the default behavior.

I just tried it with Trello to see how they handle it since they have email and Google based login and got some interesting results.

There's 2 different flows, right:

1. Create account with email ([email protected]), log out, sign up using Google account thats associated with the same email (John Doe's Google account).

   In this case, Trello actually treats the second account as the same account as the first one. I was surprised by this, but in thinking about it that actually seems ok.

2. Sign up with Google account first, log out, try to sign up again using [email protected].

   In this case, when trying to sign up again, Trello said the email is already in use. This is good because this is the flow with the security vulnerability. Imagine you had a Trello account that you created by logging in with Google. It would be very bad if I could sign up for a new Trello account using your email address and have the accounts automatically merged, because now I would have access to your account.

   This can actually be simulated with the package already. Just make sure when someone signs up with Google first, you capture their email. Then when someone tries to sign up with email/password using the same email that was associated with a Google account, you can throw an error. This is outside of the scope of this package as you would implement it in your normal email/password flow, but it prevents the duplicate accounts thing from happening.

So the first flow is actually pretty safe, and I could make some changes to make that possible.

It'd basically look like this:

OAuth::login('google', function ($user, $details) {
    $existing_user = User::where('email', $details->email)->first();

    if ($existing_user !== null) {
        return $existing_user; // Tell the package to use this user instead of creating a new one.
    }

    // usual stuff as documented in current version
    $user->email = $details->email;
    $user->photo_url = $details->avatar;
    // etc. etc. etc.

    // by returning nothing, the package knows to use the $user it originally passed in, which is either the one it found based on the logic it already contains, or a brand new user.
});

The change would be that if you return anything from that closure, the package will use that user instead of the one it would otherwise find on it's own or create.

If you don't return anything or return null (like in the current example in the docs), the package would either find the existing Google user or create a new one as needed, as it currently behaves.

Does that sound like it would introduce the flexibility you need to control that sort of thing? I'm still somewhat hesitant about it because it would allow people to build very unsafe login systems if they weren't aware of the security vulnerability, and it would really suck if someone did something dumb like that not realizing and came back raging at me for it :/

[niknetniko]

I see your point. I've also read the issue you mentioned, and there are valid points there as well.

A solution for the second flow that was also suggested in the other issue is enabling the user to link to other accounts. Maybe this could be supported with something like this:

$user = Auth::user(); //The current user
OAuth::link('google', $user);

Behind the scenes this will probably use a lot of code from the changes you suggested above (with the existing user), but it might be convenient to have a separate function.

The first flow is, like you said, fairly safe. It could be an idea to include a warning that if people enable the first flow, they should limit the social accounts to the ones that require email verification. That way, if you login using a social account, you are sure the user actually has the email address. If there is no email verification on the social account, nothing stops a malicious user from making a social account with someone else's email.

Anyway, thanks for your support!

[jerauf]

I second this request.

I've used a library called SiteLok in the past. It's not a Laravel package. And how he handles it is that existing users can link their social account once they've logged in. That partially removes the security issue since the existing user will already be logged in and authorizing the action. It also gives them the option to unlink, which is helpful in some cases (like if someone deletes their linked Facebook account or whatever).

[adamwathan]

Yep still want to get the link/unlink stuff in, as well as the ability to override which user ends up getting logged in in case someone really wants this email matching behavior. Maybe I can dedicate a few hours to it this week and get a new version out, fingers crossed.

[jerauf]

That'd be amazing!

[subdesign]

My question is partly related. What if I have a google and facebook login on my page, and the user logs in with both providers (not at the same time), and he has the same email address. Usually websites combine users to one in these situations (by email), what about this package?
Thanks

[adamwathan]

Hey @subdesign, same behavior there. It introduces a security vulnerability that I'd rather not have any responsibility for :) Once I've implemented the option to override which user gets used manually, it will allow people to add that functionality themselves pretty easily if they want their app to behave that way.

[bsheikh]

Any update on this feature?

[adamwathan]

Nothing yet, sorry.

[DenisMir]

Why does it introduce a security vulnerability? The right way would be to validate the email address before merging the account. And why is approach 1 safer than approach 2? If the social provider would not have validated the e-mail you would have the same problem as with approach 1.

[adamwathan]

@DenisMir the reason for the vulnerability is discussed here: adamwathan/eloquent-oauth#19 (comment)

Reading back again I think you're actually right, I don't know why I thought approach 1 was any safer. If you knew someone had created an account via email and you wanted access, you could easily create a GitHub account or similar using the same email, and login with that social provider and get access, as long as the social provider allows API access before email verification, which many of them do.

[DenisMir]

@adamwathan
In the end it is only possible to merge two accounts if you can make sure that the e-mail address has been validated on both accounts.

[adamwathan]

@DenisMir yep totally agree, and I don't think that info is gonna be consistent enough between providers to make it worth trying to support this out of the box :/ Especially since several providers won't provide an email at all.

[kingbri90z]

Thanks for this @adamwathan because I was in the process of writing something to get around the duplicated accounts so this saved me some time.