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

Social Sign In for existing local user causes confusion (suggested solution): #215

Closed
arctelix opened this issue Mar 16, 2013 · 16 comments
Closed

Comments

@arctelix
Copy link
Contributor

If an existing local user tries to sign with a social account and the social email or user name conflicts with the local user they will not be able to proceed unless they create a new user by providing a unique username & email. I think this will be confusing for users and have many new accounts created unintentionally. The flowing would seem to make more sense:

Upon social log in: First check to see if the social email & or username are already assigned to a user. If so, confirm that the user intended to link this social account to their local account with a simple yes no form. If yes add it to the user's connections and log them in. If no, redirect to the social sign up form where they can create a new local user.

Does this make sense? If so i would be happy to work on it.

@arctelix arctelix reopened this Mar 16, 2013
@arctelix
Copy link
Contributor Author

I have come up with a temporary solution using just the form design.

{% extends "socialaccount/base.html" %}
{% load url from future %}
{% load i18n %}

{% block head_title %}{% trans "Signup" %}{% endblock %}

{% block content %}
    <h1>{% trans "Sign Up" %}</h1>

<p>{% blocktrans with provider_name=account.get_provider.name site_name=site.name %}
Please complete the following form to link your {{provider_name}} account to a new {{site_name}} account:{% endblocktrans %}</p>
{% if form.errors %}
    <ul class="errorlist">
        <li>{% blocktrans with provider_name=account.get_provider.name site_name=site.name %}
        NOTE: If you already have a {{site_name}} account and want to link your {{provider_name}} account please click {% endblocktrans %}
        <a href="{% url 'socialaccount_connections' %}">{% trans " Here" %}</a> to log in first.</li>
    </ul>
{% endif %}

<form class="signup" id="signup_form" method="post" action="">
  {% csrf_token %}
  {% form.as_p %}
  {% if redirect_field_value %}
  <input type="hidden" name="{{ redirect_field_name }}" value="{{ redirect_field_value }}" />
  {% endif %}
  <button type="submit">{% trans "Sign Up" %} &raquo;</button>
</form>
{% endblock %}

If the user tries to enter an existing email or username they will be warned:
NOTE: If you have already have a {{site_name}} account and want to link your {{provider_name}} account please click HERE to log in first. Once they log in they will be redirected to the social connection screen where they can add a connection without crating a new user.

@pennersr
Copy link
Owner

This scenario has been discussed before in #191 (comment).

I am not yet convinced that we need to "pollute" the signup flow by making it a hybrid signup-connect flow, especially given that there is already a proper account connect flow that the user could use. I've been managing other systems that do implement such flows and non-technical users simply cannot seem to grasp this concept. So I really prefer to keep the signup flow all about signup.

I do agree that the current execution of this scenario could use some work. Specifically:

  • The form validation error message in case of duplicate username/e-mail could be more specific (something along the lines you have written above).
  • This FIXME needs to be addressed:
    # FIXME: We redirect to signup form -- user will
    -- it now redirects to the (unvalidated) signup form, but it should probably show the validation errors immediately.

@davidsickmiller
Copy link

Improving this flow would be particularly useful to sites with large local userbases that are finally adding Facebook login. I wonder how many non-technical users even understand "connect" because usually all they have to do is click a site's Facebook button and they're logged in.

@gcbirzan
Copy link

gcbirzan commented Apr 2, 2013

One thing that could help immensely if there was an easy way to configure what happens when the user already signed up. Right now, the only way to do that is to override the whole signup view, which is ugly, at best.

Edit:
Actually, I ended up overriding the signup view and just redirecting to our own URL if further action is required.
Further edit:
Also, I had to override the connections page, as it's hardcoded as where you go back after linking another account.

@dgmdan
Copy link

dgmdan commented Oct 2, 2013

I agree that this is confusing for users. Since I'm only using the Facebook and Google providers and I know that these sites verify user email addresses, I've opted to trust the email address I receive from the provider and attach it to any local account with a matching email. I'm doing this by catching the pre_social_loginsignal and logging into the local user if it exists:

@receiver(pre_social_login)
def link_to_local_user(sender, request, sociallogin, **kwargs):
    email_address = sociallogin.account.extra_data['email']
    users = User.objects.filter(email=email_address)
    if users:
        perform_login(request, users[0], email_verification=app_settings.EMAIL_VERIFICATION)

I realize this issue is getting old but hopefully this will help someone.

@cool-RR
Copy link
Contributor

cool-RR commented Jun 8, 2014

Thanks for the tip @dgmdan ! You da real MVP :)

@iamtennislover
Copy link

@dgmdan ! Thanks for the code. Its working. Just one question - At the end, it redirects the page to signup page. How can I avoid this and redirect to LOGIN_REDIRECT_URL? I have document it here: http://stackoverflow.com/questions/24357907/django-allauth-facebook-redirects-to-signup-when-retrieved-email-matches-an-exis

@dgmdan
Copy link

dgmdan commented Jun 24, 2014

@abarik1981 Glad to see you figured out a solution on the SO page. Just to document it here, the fix was to raise an ImmediateHttpResponse right after the perform_login line.

@ghost
Copy link

ghost commented Feb 5, 2015

@dgmdan Your solution works! But just to be mentioned there a huge security hole is introduced as from allauth docs, http://django-allauth.readthedocs.org/en/latest/providers.html#facebook

"It is not clear from the Facebook documentation whether or not the fact that the account is verified implies that the e-mail address is verified as well. For example, verification could also be done by phone or credit card. To be on the safe side, the default is to treat e-mail addresses from Facebook as unverified."

Saying so, I can signup in facebook with your email ID or change my email to yours in facebook and login to the website to get access to your account.

@lacabra
Copy link

lacabra commented Feb 4, 2016

Incorporating @babus observation to the back and forth on this other related thread, I came up with the following adapter.py to address the issue at hand, that seems to work fine.

@RobertoMaurizzi
Copy link

Update: it looks that now Facebook guarantees that all emails returned are verified (unverified ones won't be returned), from: https://stackoverflow.com/questions/14280535/is-it-possible-to-check-if-an-email-is-confirmed-on-facebook that someone linked under @lacabra post on SO.

At this point using account.connect() instead of redirecting using the ImmediateHttpResponse exception logs the user in and display a warning message about what happened.

I was honestly confused by the previous flow, because "Log In" was redirecting to the "Sign Up" flow then throwing an error.

@EvanZ
Copy link

EvanZ commented Feb 7, 2018

@RobertoMaurizzi I think they have "guaranteed" it for many years. I don't understand why this project (and django-rest-auth which depends on it) still thinks it's a security issue. The whole point of social authentication is that you have trusted providers to authenticate. If you don't trust FB, what's the point of any of this?

@NourEldin-Osama
Copy link

I agree that this is confusing for users. Since I'm only using the Facebook and Google providers and I know that these sites verify user email addresses, I've opted to trust the email address I receive from the provider and attach it to any local account with a matching email. I'm doing this by catching the pre_social_loginsignal and logging into the local user if it exists:

@receiver(pre_social_login)
def link_to_local_user(sender, request, sociallogin, **kwargs):
    email_address = sociallogin.account.extra_data['email']
    users = User.objects.filter(email=email_address)
    if users:
        perform_login(request, users[0], email_verification=app_settings.EMAIL_VERIFICATION)

I realize this issue is getting old but hopefully this will help someone.

please can you tell me where to put this code

@aehlke
Copy link

aehlke commented Jul 28, 2022

"If you use an email address as the unique credential which identifies each account, your app should verify that the email address associated with the person's Facebook account (and obtained during Facebook Login) is valid. You can do this by creating code in your app to send a verification email to the address obtained after Facebook Login (you will probably need to have this step as part of your regular login system anyway)."

Facebook says we need to verify independently

@derek-adair
Copy link

derek-adair commented Apr 4, 2023

please can you tell me where to put this code

@NourEldin-Osama - this is a signal. You can put it anywhere. Read up on signals here.

@pennersr
Copy link
Owner

Closing -- see SOCIALACCOUNT_EMAIL_AUTHENTICATION.

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