social-app-django
social-app-django copied to clipboard
create_user returns an existing non-social user account
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:
- Create a normal user using
./manage.py createsuperuser
that has the same email as a social account - Login with a social auth provider (I'm testing with google oauth2)
- 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
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.
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).
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:
- Created a super user from command line (user-1)
- Logged in as a different user (user-2) using Google Oauth2
- Both user-1 and user-2 show up in the database
- Tried logging in with user-3 using Google Oauth2
- Viola!!! user-3 is merged with user-1 which is a super user
I deleted db.sqlite3
- Created super user (user-1)
- Logged in using user-2
- 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?
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
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 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:
- I am logged in as superuser
- My friend who wants to try the site, logs in using their social account
- 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.
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
@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.
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.