social-app-django icon indicating copy to clipboard operation
social-app-django copied to clipboard

create_user returns an existing non-social user account

Open KennyMonster opened this issue 5 years ago • 9 comments

I'm new to PSA so forgive me if this is silly.

I'm working on a site where we want to handle the case of a user trying to login with a social account, when a non-social account does not already exist with that associated email.

I started with a stock pipeline with the relevant config:

    ...
    # Associates the current social details with another user account with
    # a similar email address. Disabled by default.
    # 'social_core.pipeline.social_auth.associate_by_email',

    # Create a user account if we haven't found one yet.
    'social_core.pipeline.user.create_user',

    # Create the record that associates the social account with the user.
    # this only works if create_user or associate_by_email returns a user
    'social_core.pipeline.social_auth.associate_user',
    ...

My understanding is that 'social_core.pipeline.user.create_user' will create a new User if one does not exist, but should not return one if one did not exist. If this unsafe behavior was desired, one could enable 'social_core.pipeline.social_auth.associate_by_email'.

The problem I'm facing is that create_user does in fact return an existing user, thus allowing someone with a social account (with an unverified email address) to effectively take over a legit non-social account.

I tracked this behavior down to this code: https://github.com/python-social-auth/social-app-django/blob/master/social_django/storage.py#L90

which has a comment stating:

# User might have been created on a different thread, try and find them.
# If we don't, re-raise the IntegrityError.

It seems to me that a fix for a multi-threading issue may have opened up some unintended security consequences.

Reproduction steps:

  1. Create a normal user using ./manage.py createsuperuser that has the same email as a social account
  2. Login with a social auth provider (I'm testing with google oauth2)
  3. See that login succeeds and that a row now exists in social_auth_usersocialauth table

expected behavior:

An error about a failed login.

Versions: social-auth-app-django 3.1.0
social-auth-core 3.2.0

KennyMonster avatar Sep 09 '19 22:09 KennyMonster

See https://python-social-auth.readthedocs.io/en/latest/use_cases.html#associate-users-by-email

You shouldn't be using associate_by_email unless the emails can be verified for all backends.

rtran9 avatar Oct 02 '19 21:10 rtran9

I think you've misunderstood the original poster's issue. I've just encountered the same problem, and I think this is definitely a security hole, one that could be used to access a super user account remotely.

I don't have 'associate_by_email' enabled. I also think the problem is with https://github.com/python-social-auth/social-app-django/blob/master/social_django/storage.py#L90 . I'd add to the reproduction steps that the UserModel must have a unique constraint on email.

The issue is not mutliple social_auth backends (though it may extend to them - I haven't tested). It's using a social_auth backend to access a user created by a ModelBackend.

You may be able to add a task to the pipeline before 'create_user' that raises an error if a different user with the same email address exists (unless the user is logged in, perhaps).

mel-mason avatar Nov 04 '19 15:11 mel-mason

Strangely enough I ran into exact same issue today in dev environment. I have a pretty new project, only social-auth integration. Here is a brief background of what I experienced:

  1. Created a super user from command line (user-1)
  2. Logged in as a different user (user-2) using Google Oauth2
  3. Both user-1 and user-2 show up in the database
  4. Tried logging in with user-3 using Google Oauth2
  5. Viola!!! user-3 is merged with user-1 which is a super user

I deleted db.sqlite3

  1. Created super user (user-1)
  2. Logged in using user-2
  3. Viola!! Now user-2 got merged with the super user

Unsure what exactly is happening but surprised to see an open issue reporting exactly the same issue.

@KennyMonster Ddid you find a resolution for this?

abhinavsingh avatar Dec 26 '20 11:12 abhinavsingh

If a logged in user starts OAuth flow, accounts are simply linked together. Can be easily reproduced. I don't think this should be the default behavior. How can this be disabled? Thanks

abhinavsingh avatar Dec 26 '20 11:12 abhinavsingh

This is an answer to @abhinavsingh's question (not the main security issue). I believe you can disable the default behavior by adding a custom pipeline to the beginning of your pipelines.

# settings.py

SOCIAL_AUTH_PIPELINE = (
    'my_app.pipelines.redirect_logged_in_user',
    'social_core.pipeline.social_auth.social_details',
    'social_core.pipeline.social_auth.social_uid',
    ...

# my_app/pipelines.py

from django.conf import settings
from django.shortcuts import redirect

def redirect_logged_in_user(backend, user, response, *args, **kwargs):
    """If a logged in user starts OAuth flow, accounts are simply linked together. Prevent this behavior."""
    if user:
        return redirect(settings.LOGIN_REDIRECT_URL)

cspollar avatar Feb 10 '21 16:02 cspollar

@cspollar Thank you Chris. I have done something similar to avoid this from happening.

I still think this is potentially a security issue and should not be the default behavior. Imagine this:

  1. I am logged in as superuser
  2. My friend who wants to try the site, logs in using their social account
  3. Viola!!! Now my friend is also a superuser

I can understand association based upon same email, but I think such associations must be explicitly approved, otherwise is a possible attack vector.

abhinavsingh avatar Feb 11 '21 16:02 abhinavsingh

In Weblate we deal with this by showing confirmation and asking for user password to confirm this: https://github.com/WeblateOrg/weblate/blob/c770bf8130d4e2484d1872c51abdf2b1ea6aa2b4/weblate/accounts/pipeline.py#L79-L95

nijel avatar Feb 11 '21 19:02 nijel

@cspollar @nijel Wanted to point out that above solutions doesn't prevent a logged in user from logging in again. E.g. we want users to click "Add another account", if they wish to keep have multiple accounts. We maintain a multi SITE setup and requirement varies from app to app.

abhinavsingh avatar Feb 21 '21 11:02 abhinavsingh

We do it pretty much this way in Weblate (the above is just a portion of code around that). On the other side we enforce unique e-mails so users really do not have an option to create another account with e-mail which is already associated elsewhere...

The python-social-auth behavior built-in behavior is that when the user is authenticated it creates additional association with the current user.

nijel avatar Feb 22 '21 15:02 nijel