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

Feature request: Automatically connecting an account to an already registered email #418

Closed
pauloneves opened this issue Oct 16, 2013 · 27 comments

Comments

@pauloneves
Copy link
Contributor

After making this question in StackOverflow, I've just discovered this comment in the source code helpers.py file:

            if account_settings.UNIQUE_EMAIL:
                if email_address_exists(email):
                    # Oops, another user already has this address.  We
                    # cannot simply connect this social account to the
                    # existing user. Reason is that the email adress may
                    # not be verified, meaning, the user may be a hacker
                    # that has added your email address to his account in
                    # the hope that you fall in his trap.  We cannot check
                    # on 'email_address.verified' either, because
                    # 'email_address' is not guaranteed to be verified.

I want to argue that this would be a nice feature, and I'd like to offer myself to implement it if there is a reasonable chance that my patch will be accepted in the main trunk.

First, I agree that it is a security risk and that it should be turned off by default. There should be an configuration option to turn on the automatic connection.

Not all social providers are equal. Some of them, like Facebook, verify the user email and account. Facebook even sends a "verified_email": true that is saved in the extra_data column of socialaccount_socialaccount table. If the email if from Gmail, Google provider is who created the account! It can't be more verified than this.

Automatically connecting, would be a great usability enhancement. Just one click authenticating. Less friction to use my site.

Automatically connecting the accounts is a reasonable security/usability tradeoff for some selected providers.

Ultimately, I believe the security tradeoffs should be made by the site owner. The user of the library decides which risks are acceptable to run, and if a improved user experience is more important than a remote probability security exploit.

Just using myself as an example. I plan to offer just Facebook and Google as login options. My site is a hobby site, there is nothing worth to hack their user accounts. I want just an easy way to manage the identity of my users. As I said before, both FB and Google check the users email. I really think that if someone wants to hack my users accounts, it would be easier to directly hack my site than to hack FB and to use to it hack my site. The better user experience would be a greater benefit.

Please give me a "go forward" signal before I start implementing it.

@pennersr
Copy link
Owner

I agree with you that allauth should not be too opiniated and make life difficult for you want you want to move forward and implement your requirements. Whether or not this is something that needs to be supported out of the box needs a bit more thought though.

The way I see it is that only if all of the following conditions hold, then the login can be shortcircuited:

  • a social login occurs using a social account that was previously not known
  • the provider certifies that the e-mail address for that social login is verified
  • a local user account exists (perhaps with one or more other social accounts attached -- this is not relevant) that has a least one verified e-mail address matching that from the social login.

Google clearly indicates that e-mail addresses are verified. For Facebook this is not so clear -- the account is said to be verified (which could mean you verified your phone number, not your e-mail address). For Twitter this will not work at all -- no e-mail address is handed over here.

As for the implementation, there is a pre_social_login adapter method / signal that you can use. Here, you can do the lookups as indicated above.

@pauloneves
Copy link
Contributor Author

Thanks for your answer. I'll try to implement something tonight at home. Here what I plan to do:

I'll create the new setting SOCIALACCOUNT_AUTOMATICALLY_CONNECT(=False). If this option is true, then SOCIALACCOUNT_QUERY_EMAIL must also be true.

Initally, it will be implemented just for Google and Facebook. I've checked that even if you open a Google Account without an Gmail address, that they will always verify if you are the email owner:
image

The login from Facebook will just be accepted if the "verified_email": true is set in the extra_data.

Considering that there could be 3 kinds of accounts: Facebool account (FB), Google account (G) and Local account (L). These would be the possible workflows:

  1. Existing local account
    1. User has account L with verified email (user may be already authenticated)
    2. Try to login with FB or G
    3. Social site authorizes
    4. If conditions satisfied (*) user would be authenticated and their Social account is now associated in the socialaccount_socialaccount table.
  2. User already connected with a social account, e.g, G.
    1. Try to login with FB
    2. Social site authorizes
    3. If email is verified in FB account, user would be authenticated and their Social accounts are now associated in the socialaccount_socialaccount table.
  3. User already connected with a social account, e.g, G.
    1. Try to create local account with G registered email
    2. If user is already authenticated from G, the local account is created and marked as verified, else a verification email is sent.
    3. after email is verified, user can use the created account. Ops, now I see that the ACCOUNT_EMAIL_REQUIRED must also be True.

(*) Conditions: FB sends verified_email: true and local email is verified

I plan to add some unit tests. I still didn't execute allauth tests. May I contact you if I have any difficulty running the tests? Is here the best place to ask advice?

Please tell me if I'm missing something.

@pennersr
Copy link
Owner

A few points:

  • Facebook does not have a "verfied_email" field as you suggest. Only a verified account field. Whether or not that implies that the email address is verified is not documented.
  • All of this can be implemented complete provider agnostic. At the time the pre_social_login signal is fired, the sociallogin instance already contains all information on the account, including whether or not it comes with verified e-mail addresses.
  • Your scenario 1) and 2) are identifcal as far as allauth is concerned, as each social account has an implicit local account attached.
  • I am not sure I fully grasp scenario 3. In 3-ii you suggest that the user is already signed in, and at the same time he is trying to sign up. IMHO that is confusing and should be avoided, rendering this scenario void?

I am not sure yet if and how I would like to have this functionality end up in allauth. For one, I am not convinced it is a good idea to implicitly hookup the FB account to his local account without at least informing the user what is going on. Furthermore, if something like this is implemented it could take situations where there is no email verified into account as well.

All in all, for now I suggest that you implement this using the pre_social_login hook that is already offered. allauth does not need any modifications to get this going. Simply do the necessary lookups and log the user in.

@pauloneves
Copy link
Contributor Author

Ok, I'll try to implement it using pre_social_login. I tell you if it works.

Excuse me, I was wrong. The verified_email was from Google provider, not Facebook.

The scenario 3 is if the user is authenticated from Google, but need to get his friend list from Facebook so he can use a specific site feature. I'd ask him to login to Facebook so he can use the feature. His identity would stay the same for the site.

@pennersr
Copy link
Owner

Ok, but then I think your scenario 3 is already supported. Simply use something like this:

<a href="{% provider_login_url "facebook" process="connect" %}">Connect your FB account</a>

@nchudleigh
Copy link

I am in in the similar situation where I want the verified status to set the verified email to true.
Thus I have gone in and changed the setting 'VERIFIED_EMAIL': True. But when I test the login my verified email is still set to false. I checked the 'verified' field sent by facebook and it is 'true'.

Perhaps this is more of a stack question though I seem to not be getting any traction there.

@gjcourt
Copy link

gjcourt commented Mar 20, 2014

What does one have to do in the pre_social_login function in order to make this work? I've tried simply setting sociallogin.account.user and that does not work. Does the user also have to be logged in?

@pennersr
Copy link
Owner

@gjcourt I am not sure what you are referring to by "make this work".
@nchudleigh VERIFIED_EMAIL previously was something FB specific, but this has been changed as part of #569.

@gjcourt
Copy link

gjcourt commented Mar 23, 2014

@pennersr I figured it out, should probably have this as an example of "auto login"

class SocialAdapter(DefaultSocialAccountAdapter):
    def is_auto_signup_allowed(self, request, sociallogin):
        return True

    def pre_social_login(self, request, sociallogin):
        user = sociallogin.account.user
        if user.id:
            return
        try:
            existing_user = User.objects.get(email=user.email)
        except ObjectDoesNotExist:
            pass
        else:
            perform_login(request, existing_user, app_settings.EmailVerificationMethod.NONE)

@epowers
Copy link

epowers commented Aug 11, 2014

+1 for this feature. In the mean time, trying the pre hook similar to the above per stackoverflow.

@sspross
Copy link

sspross commented Jun 2, 2015

hey guys, don't know if I missed something, but this is still an issue for me and I came up with this solution. is this totally wrong and/or are there better solutions? thanks you!

from allauth.account.models import EmailAddress
from allauth.socialaccount.adapter import DefaultSocialAccountAdapter

class SocialAccountAdapter(DefaultSocialAccountAdapter):
    def pre_social_login(self, request, sociallogin):
        """
        Invoked just after a user successfully authenticates via a
        social provider, but before the login is actually processed
        (and before the pre_social_login signal is emitted).

        We're trying to solve different use cases:
        - social account already exists, just go on
        - social account has no email or email is unknown, just go on
        - social account's email exists, link social account to existing user
        """

        # Ignore existing social accounts, just do this stuff for new ones
        if sociallogin.is_existing:
            return

        # some social logins don't have an email address, e.g. facebook accounts
        # with mobile numbers only, but allauth takes care of this case so just
        # ignore it
        if 'email' not in sociallogin.account.extra_data:
            return

        # check if given email address already exists.
        # Note: __iexact is used to ignore cases
        try:
            email = sociallogin.account.extra_data['email'].lower()
            email_address = EmailAddress.objects.get(email__iexact=email)

        # if it does not, let allauth take care of this new social account
        except EmailAddress.DoesNotExist:
            return

        # if it does, connect this new social login to the existing user
        user = email_address.user
        sociallogin.connect(request, user)

@lukeburden
Copy link
Contributor

@sspross, your approach doesn't consider whether email addresses have been verified.

Before dynamically linking social accounts to users in this way, I'd recommend at least ensuring that email addresses are verified. Unless of course the system you're working on doesn't merit this best practice.

I'd consider something like:

def pre_social_login(self, request, sociallogin):

        # social account already exists, so this is just a login
        if sociallogin.is_existing:
            return

        # some social logins don't have an email address
        if not sociallogin.email_addresses:
            return

        # find the first verified email that we get from this sociallogin
        verified_email = None
        for email in sociallogin.email_addresses:
            if email.verified:
                verified_email = email
                break

        # no verified emails found, nothing more to do
        if not verified_email:
            return

        # check if given email address already exists as a verified email on
        # an existing user's account
        try:
            existing_email = EmailAddress.objects.get(email__iexact=email.email, verified=True)
        except EmailAddress.DoesNotExist:
            return

        # if it does, connect this new social login to the existing user
        sociallogin.connect(request, existing_email.user)

@sspross
Copy link

sspross commented Sep 3, 2015

@lukeburden we consider email addresses from social accounts as verified. or in other words, we just trust our social login partners to verify the users "good enough". if a user creates a normal account, we confirm the email address of course.

but you are right, this way we've to be careful which providers we turn on.

@deepai-org
Copy link

@lukeburden's solution was the best way for me.

@rogeriosmorais
Copy link

@deepai-org, @lukeburden how did you change the behavior in your own code? Did you create a local fork of the code?

@steverecio
Copy link

Using Google Login, the sociallogin object doesn't have any emailaddresses so the email address has to get accessed from sociallogin.account.extra_data:

    def pre_social_login(self, request, sociallogin):
        # social account already exists, so this is just a login
        if sociallogin.is_existing:
            return

        email = sociallogin.account.extra_data.get('email', None)
        email_verified = sociallogin.account.extra_data.get('email_verified', False)
        # verify we have a verified email address
        if not (email and email_verified):
            return

        # check if given email address already exists as a verified email on
        # an existing user's account
        try:
            existing_email = EmailAddress.objects.get(email__iexact=email, verified=True)
        except EmailAddress.DoesNotExist:
            return

        # if it does, connect this new social login to the existing user
        sociallogin.connect(request, existing_email.user)

@lukeburden
Copy link
Contributor

@steverecio that doesn't sound right - the email addresses are being extracted and made available on sociallogin.email_addresses in my projects. I recommend you debug your particular situation as that sounds broken.

@rogeriosmorais I just built it into my projects by overriding adapters. It's pretty straightforward that way - don't forget to write some tests to make sure behaviour is as you expect it!

@lukeburden
Copy link
Contributor

@sspross I'm a bit worried people are copy/pasting your code without being aware that it is insecure if they support email/password auth or some provider that does not properly validate email addresses. It might be worth updating/removing. Cheers!

@steverecio
Copy link

steverecio commented Jan 21, 2020

@lukeburden looks like this changed between 0.39.1 and 0.41.0. I'm now seeing the email addresses in the adapter however there are two issues with the out of the box GoogleProvider.

The Google API returns email_verified but allauth checks for verified_email

The Google API returns the uid in sub but allauth checks for id

I have an open PR for fixing the id vs sub issue but I'm not sure if this would break any old code / APIs.

Here is the custom provider I built to make this work for Google:

from allauth.socialaccount import providers
from allauth.socialaccount.providers.google.provider import GoogleProvider
from allauth.account.models import EmailAddress


class GoogleModifiedProvider(GoogleProvider):
    id = 'google_modified'

    def extract_uid(self, data):
        return str(data['sub'])

    def extract_email_addresses(self, data):
        ret = []
        email = data.get('email')
        if email and data.get('email_verified'):
            ret.append(EmailAddress(email=email,
                       verified=True,
                       primary=True))
        return ret

providers.registry.register(GoogleModifiedProvider)

@lukeburden
Copy link
Contributor

@steverecio it sounds like this has changed on Google's side at some point, although I've found it hard to find any changelog explicitly stating that it has. Version 1 of their TokenInfo has both email_verified and verified_email, whereas version 2 of TokenInfo only has the latter.

Hopefully your PR to change the Google Provider can be aware of the version on Google's side. Thanks for your efforts!

@richardbalwane
Copy link

Hello Gents, am stuck on this issue... been 3 days! I wonder how to proceed. Did you agree here on the way forward for pre_social_login? And, what GoogleProvider to use>
Thanks.

@richardbalwane
Copy link

Wow! Finally I got things working. I used @steverecio's solution where he suggests that with Google Login, the sociallogin object doesn't have any emailaddresses, therefore we are bound to source the email address from sociallogin.account.extra_data.
Thank you dude, works great!
And, just to be clear on my bad, I wasn't also generating the proper URI from the way my URLs were coined. Allauth claims the app "accounts/... for additional social paths, this I had called "account/ as I had a fully fledged app called accounts, to avoid collisions. So, had to change the app name back to "account", the allauth way along with the bloody nose involved in changing an app_name! All this for generating the Oauth2 authorised http://127.0.0.1:8000/accounts/google/login/callback/. Remember, I didn't install allauth in my env, just forked their account and socialaccount and made them apps. Allauth is great!
Thank you...

@matthijskooijman
Copy link
Contributor

It seems that @steverecio is using a custom adapter, that is feeding different info into the provider, which explains why he's seeing different properties. See #2443 for some more details.

It might be that this different adapter is somehow relevant to this discussion (I'm not sure), though.

Also, I just set up things with the default allauth google adapter and provider, and my e-mailaddress was connected to the newly created account as expected, so my alluath does have access to it (but maybe I'm missing the point here).

@steverecio
Copy link

Here is the custom adapter for transparency (running allauth==0.41.0)

from allauth.socialaccount.providers.google.views import GoogleOAuth2Adapter
from allauth.socialaccount.providers.oauth2.views import (
    OAuth2Adapter,
    OAuth2CallbackView,
    OAuth2LoginView,
)

from google.auth.transport import requests
from google.oauth2 import id_token


class GoogleOAuth2AdapterIdToken(GoogleOAuth2Adapter):
    provider_id = GoogleModifiedProvider.id

    def complete_login(self, request, app, token, **kwargs):
        idinfo = id_token.verify_oauth2_token(token.token, requests.Request(), app.client_id)
        if idinfo["iss"] not in ["accounts.google.com", "https://accounts.google.com"]:
            raise ValueError("Wrong issuer.")
        extra_data = idinfo
        login = self.get_provider().sociallogin_from_response(request, extra_data)
        return login

oauth2_login = OAuth2LoginView.adapter_view(GoogleOAuth2AdapterIdToken)
oauth2_callback = OAuth2CallbackView.adapter_view(GoogleOAuth2AdapterIdToken)

@richardbalwane
Copy link

@matthijskooijman Please just mention the error you are getting... It is important to focus laser like on the error in order to find out what code is throwing it.

@matthijskooijman
Copy link
Contributor

@richardbalwane, I'm not getting any errors, things are working as expected for me. I have not applied any of the modifications here to autoconnect an account to an existing e-mail, but I was just responding to earlier comments that the e-mailaddress would not be available. I might have misunderstood some of the discussion, so if my remarks seem confusing, better just ignore them :-)

@Saviodiow95
Copy link

Eu fiz desse jeito e deu certo

`from allauth.account.models import EmailAddress
from allauth.account.utils import perform_login
from allauth.socialaccount.adapter import DefaultSocialAccountAdapter
from django.core.exceptions import ObjectDoesNotExist
from project import settings
from .models import User

class SocialAdapter(DefaultSocialAccountAdapter):
def pre_social_login(self, request, sociallogin):
"""
Invoked just after a user successfully authenticates via a
social provider, but before the login is actually processed
(and before the pre_social_login signal is emitted).

    We're trying to solve different use cases:
    - social account already exists, just go on
    - social account has no email or email is unknown, just go on
    - social account's email exists, link social account to existing user
    """

    # Ignore existing social accounts, just do this stuff for new ones
    if sociallogin.is_existing:
        return

    # some social logins don't have an email address, e.g. facebook accounts
    # with mobile numbers only, but allauth takes care of this case so just
    # ignore it
    if 'email' not in sociallogin.account.extra_data:
        return

    # check if given email address already exists.
    # Note: __iexact is used to ignore cases
    try:
        email = sociallogin.account.extra_data['email'].lower()
       
        #email_address = EmailAddress.objects.get(email__iexact=email)
        user = User.objects.get(email=email)
        if user:
            pass
        else:
            return


    # if it does not, let allauth take care of this new social account
    except EmailAddress.DoesNotExist:
        return

    # if it does, connect this new social login to the existing user
    user = user
    sociallogin.connect(request, user)`

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