Authorise on existing account

[georgeboot]

Can this package be extended with the option to authorise to an already existing account? For me this would be very useful.

[adamwathan]

Hey George, not yet but I'm hoping to add support for linking existing accounts in the near future.

[georgeboot]

Awesome, looks good! Will this also be possible if the user is already logged in?

[michakpl]

@georgeboot, I didn't tested this on logged in users but you can check if it works

[adamwathan]

@georgeboot My plan for linking accounts is that it will only work if the user is currently logged in, otherwise it introduces a security vulnerability

It'll be a bit limited but this is all I believe I can reasonably support:

1. User is already logged in with Facebook
2. User hits "link GitHub account"
3. Package will check to make sure that GitHub account isn't already in use.
   1. If it's unused, link the accounts
   2. If it's already been used, throw an error. Can't link because package has no idea how to combine all the various data in your app related to the two accounts.

[georgeboot]

@michakpl Thanks Michal! Can you please merge your commit into this repo so I can start using it :-) ?

[adamwathan]

@georgeboot Unfortunately had to close @michakpl's pull request because it's not functionality I can responsibly include. I need to work out the proper strategy for connecting two accounts, and if I can make it flexible to give people the control they need to merge existing accounts if necessary.

[georgeboot]

@adamwathan Ok, I understand that. Can https://github.com/michakpl/eloquent-oauth/commit/84f750bfef88417c1bf6338d46db47b6ad11e6f9 be merged though?

This commit also fixes the following bug:

1. Create a account by hand (no social login).
2. Try to login using this plugin, using a social account containing the already registered email
3. Laravel will throw an PDOException: Integrity constraint violation: 1062 Duplicate entry 'user@domain.com' for key 'users_email_unique''

The above commit takes care of this issue, with linking the verified email (I assume all providers only share validated emails) to a new oauth_identity, keeping (and updating) the already existing user.

[georgeboot]

@adamwathan Update: I'm only seeing the closed pull-request now. My apologies. Is there any way around the above outlined issue?

[michakpl]

@georgeboot, I made this changes on me client's wish, so you can set your composer.json file to get this package straight from my repository if you want

"repositories": [
    {
        "type": "git",
        "url": "https://github.com/michakpl/eloquent-oauth.git"
    }
],

[Metrakit]

Thanks for your fix @michakpl

[igitur]

@adamwathan I understand that there is a security risk here. But can't we identify a subset of approved providers whom we trust. After all, we know that if a user has authenticated on Facebook, Google and Github with the same email addresses that they really all do belong to the same user.

[adamwathan]

@igitur That's not true though, GitHub nor Facebook require email confirmation before being able to access their OAuth APIs.

Doing this automatically is not something I will ever support in this package, sorry everyone. I think you would be extremely hard pressed to find an example of another web application that does support this behavior. If anyone knows of any, I'd be interested to hear about them and get in touch with the developers about the strategies they've used to do it safely.

[igitur]

I guess the question is how do BitBucket do it then. I find it very hard to believe that they'd expose a security risk like that. A proof of concept of such an exploit, anyone?

[adamwathan]

@igitur I tried on BitBucket, it gives me errors saying it looks like I've already tried to create an account and can't use that service.

So if I have signed up with Facebook, then attempt to sign up with a Twitter account that uses the same email, it rejects that sign up.

Twitter did not require me to verify my email address before logging into other apps through their API, so they can't be treated as "trusted" either.

It's very obviously a security vulnerability to treat accounts as the same just because they have the same email, especially when these services do not require me to confirm my email.

Here's the scenario:

1. You sign up with Facebook and the email associated with your Facebook account is "helloworld@example.com", but you don't have a Twitter account.
2. I create a Twitter account using your email address (Twitter doesn't require me to confirm before using their API)
3. I sign up using Twitter

Now based on this feature request, the package would see "Oh this person already logged in with Facebook" because our emails are the same, even though we are not the same person. I would now have access to your account.

The only way to merge accounts safely is to make sure the person is already logged in to their existing account, then allow them to merge in another account, which is a feature I would like to eventually add.

[igitur]

Yes, I was unsure about Twitter. That's why I didn't include it in my list of 'authorised providers'. I still want to test Twitter myself. But I see Disqus uses Facebook / Twitter / Google. Can you replicate the hack there? I'll try myself later.

[adamwathan]

Well what most people do (including Disqus) is create a new account. On Disqus, I logged in with Google, then logged in with Twitter (I use the same email for both) and now I have 2 Disqus accounts. That's the standard way this stuff works. It's only a security vulnerability when you want to automatically merge accounts based on a field that the user can set to whatever they want, like an unconfirmed email address, or their first name + their last name, etc. etc. etc.

[adamwathan]

The multiple accounts thing is a common UX problem with this sort of social login. Most people in that field will tell you you should only support one provider, otherwise you get into the situation of people not knowing which one they used, and if they use the wrong one the second time, they get two accounts.

Many providers also don't even provide an email address, including SoundCloud and Instagram. So if anyone ever uses those, it's not possible to merge based on email address, and as we've established, email is no more trustworthy than their name, birthday, or favorite color.

[igitur]

Hmm, that's not what I experienced just now. I logged on to Disqus using Twitter and Facebook, both for the first time ever (I always use Google for OAuth) and it correctly linked/merged my account with my original Disqus account. No multiple accounts here...

[adamwathan]

Huh, weird! I'd love to learn more about how people are doing this in a secure way. Just joining based on unconfirmed emails is definitely not enough and that's the only idea I have :/ I also don't want to build in special case logic for different providers, but I would like to improve the ability to hook into the current login behavior and modify how it's working, so people can at least do it if they want to.

I'm thinking maybe when you use the optional closure, you have to return the user that you want the app to associate with the account. It'll pass in the one it thinks it should use, but you can return a different one and it'll use that if you want. Then maybe returning false cancels the signup/login process and throws a different exception you can catch.

This is probably the approach I will take just because I'm not comfortable being responsible for the security aspects I'm worried about, but I also don't want to handcuff people and prevent them from doing what they want.

[igitur]

Are you sure those providers allow OAuth when the email is unconfirmed? I want to test that, but I'll have time to create new accounts only later. My gut feeling says that no sane, respectable company will allow OAuth on unconfirmed email addresses. Or... if it does allow OAuth... then it won't return the unconfirmed email address, even if you request it.

Francois Botha

On 13 May 2015 at 15:38, Adam Wathan notifications@github.com wrote:

Huh, weird! I'd love to learn more about how people are doing this in a secure way. Just joining based on unconfirmed emails is definitely not enough and that's the only idea I have :/ I also don't want to build in special case logic for different providers, but I would like to improve the ability to hook into the current login behavior and modify how it's working, so people can at least do it if they want to.

I'm thinking maybe when you use the optional closure, you have to return the user that you want the app to associate with the account. It'll pass in the one it thinks it should use, but you can return a different one and it'll use that if you want. Then maybe returning false cancels the signup/login process and throws a different exception you can catch.

This is probably the approach I will take just because I'm not comfortable being responsible for the security aspects I'm worried about, but I also don't want to handcuff people and prevent them from doing what they want.

— Reply to this email directly or view it on GitHub https://github.com/adamwathan/eloquent-oauth/issues/41#issuecomment-101665908 .

[adamwathan]

Twitter definitely does, that's the one I've been testing with mostly because it's the quickest to create these throwaway accounts for, haha...

Adam

On Wed, May 13, 2015 at 9:41 AM, Francois Botha notifications@github.com wrote:

Are you sure those providers allow OAuth when the email is unconfirmed? I want to test that, but I'll have time to create new accounts only later. My gut feeling says that no sane, respectable company will allow OAuth on unconfirmed email addresses. Or... if it does allow OAuth... then it won't return the unconfirmed email address, even if you request it. Francois Botha On 13 May 2015 at 15:38, Adam Wathan notifications@github.com wrote:

Huh, weird! I'd love to learn more about how people are doing this in a secure way. Just joining based on unconfirmed emails is definitely not enough and that's the only idea I have :/ I also don't want to build in special case logic for different providers, but I would like to improve the ability to hook into the current login behavior and modify how it's working, so people can at least do it if they want to.

I'm thinking maybe when you use the optional closure, you have to return the user that you want the app to associate with the account. It'll pass in the one it thinks it should use, but you can return a different one and it'll use that if you want. Then maybe returning false cancels the signup/login process and throws a different exception you can catch.

This is probably the approach I will take just because I'm not comfortable being responsible for the security aspects I'm worried about, but I also don't want to handcuff people and prevent them from doing what they want.

— Reply to this email directly or view it on GitHub https://github.com/adamwathan/eloquent-oauth/issues/41#issuecomment-101665908 .

---

Reply to this email directly or view it on GitHub: https://github.com/adamwathan/eloquent-oauth/issues/41#issuecomment-101667385

[Leeibrah]

I would like to throw the error 'It looks like you tried to sign up with another service' like the one @adamwathan just listed above from Twitter when you account is already in use from another provider.

Where/How in the code can I achieve this?

[adamwathan]

@Leeibrah you should be able to just throw an exception inside the login closure.

This is just written off the cuff but this sort of thing should do it...

public function login($provider)
{
    try {
        OAuth::login($provider, function ($user, $details) {
            if (! $user->exists && User::where('email', $details->email)->first !== null) {
                throw new AlreadyRegisteredException;
            }

            // continue details assignment stuff...
        });
    } catch (AlreadyRegisteredException $e) {
        return redirect('login')->with('message', "It looks like you've already signed up with another service.");
    }
}

[Leeibrah]

Thanks for the feedback. Tried it but I am getting "Class 'App\Http\Controllers\Auth\AlreadyRegisteredException' not found"

Tried creating the Exception in handler.php but maybe I am doing it wrong.

How do I overcome this to display the message?

[adamwathan]

Sorry, yeah you'd have to create the exception class. You probably want to put it in your app/Exceptions folder, as it's own file and make sure it's in the correct namespace. Here's a little info on creating your own exceptions: http://php.net/manual/en/language.exceptions.extending.php

[adamwathan]

Latest release allows you to override which user is matched, so if you'd like your app to allow this sort of behavior, it's very easy to do now, just not baked in out of the box.

More info here: https://github.com/adamwathan/eloquent-oauth-l5/issues/10#issuecomment-172864961