pennersr / django-allauth

Integrated set of Django applications addressing authentication, registration, account management as well as 3rd party (social) account authentication.
https://allauth.org
MIT License
9.55k stars 3.04k forks source link

Social login always fails #3446

Closed Frohus closed 1 year ago

Frohus commented 1 year ago

Using:

I'm stuck on making social auth work for a couple of days now. It always fails in complete_social_login at assert not sociallogin.is_existing after the login is confirmed with the provider stage whether I use an existing local account or a completely new one.

The sociallogin.user that is passed to complete_social_login is always populated with a User instance (<django.db.models.base.ModelState object at 0x7fbac15b1dd0>) for some reason even if I use a completely new account so sociallogin.is_existing always returns True. This is happening for both GitHub and Google logins.

The URL is created with href="{% provider_login_url 'github' %}" and it redirects to Github and allows me to authorize the app just fine.

This is my config

ACCOUNT_AUTHENTICATION_METHOD = "email"
ACCOUNT_EMAIL_REQUIRED = True
ACCOUNT_CONFIRM_EMAIL_ON_GET = True
ACCOUNT_EMAIL_CONFIRMATION_EXPIRE_DAYS = 1
ACCOUNT_EMAIL_VERIFICATION = "mandatory"
ACCOUNT_EMAIL_SUBJECT_PREFIX = ""
ACCOUNT_MAX_EMAIL_ADDRESSES = 1
ACCOUNT_FORMS = {
    "login": "src.users.forms.CustomLoginForm",
    "signup": "src.users.forms.RegistrationForm",
}
ACCOUNT_LOGOUT_ON_PASSWORD_CHANGE = True
ACCOUNT_SIGNUP_PASSWORD_ENTER_TWICE = False
ACCOUNT_USERNAME_REQUIRED = False
ACCOUNT_USER_MODEL_USERNAME_FIELD = None
ACCOUNT_LOGIN_ATTEMPTS_LIMIT = 100

SOCIALACCOUNT_EMAIL_AUTHENTICATION = True
SOCIALACCOUNT_EMAIL_AUTHENTICATION_AUTO_CONNECT = True
SOCIALACCOUNT_EMAIL_VERIFICATION = "none"

I tried changing the existing socialaccount config not none of the config worked

Error trace

Internal Server Error: /auth/google/login/callback/
Traceback (most recent call last):
  File "python3.11/site-packages/django/core/handlers/exception.py", line 55, in inner
    response = get_response(request)
               ^^^^^^^^^^^^^^^^^^^^^
  File "python3.11/site-packages/django/core/handlers/base.py", line 197, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "python3.11/site-packages/allauth/socialaccount/providers/oauth2/views.py", line 86, in view
    return self.dispatch(request, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "python3.11/site-packages/allauth/socialaccount/providers/oauth2/views.py", line 164, in dispatch
    return complete_social_login(request, login)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "python3.11/site-packages/allauth/socialaccount/helpers.py", line 183, in complete_social_login
    assert not sociallogin.is_existing
AssertionError
pennersr commented 1 year ago

If that assert fires, that is due to:

    @property
    def is_existing(self):
        """When `False`, this social login represents a temporary account, not
        yet backed by a database record.
        """
        return self.user.pk is not None

So, that means that somehow a real user is already attached to the sociallogin, which should not be possible at this point. Are you using a custom adapter by any chance? You might want to try setting a breakpont there and inspect that user and try and determine why it has a pk.

Frohus commented 1 year ago

No, I'm not using a custom adapter. That's the user object that is being used in is_existing() (print(self.user.__dict__)). This is just a temporary user as no such user exists in the User table, nor does the EmailAddress or SocialAccount before or after the social login attempt

Note: I've got a custom user model that inherits from AbstractBaseUser, and a custom BaseModel that uses uuid field id as the primary key.

{
'_state': <django.db.models.base.ModelState object at 0x7f9fd2246310>, 
'password': '!hCeuO10Pb8757roh73dmUTba3mFJ7XyNOWs2Wsun', 
'last_login': None, 
'is_superuser': False, 
'id': UUID('438cde65-15ec-45f3-bb59-caabd3ca1e08'), 
'created_at': None, 
'updated_at': None, 
'first_name': 'dasdasd', 
'last_name': 'adsasd', 
'email': 'lajdasddaljk@gmail.com', 
'is_staff': False, 
'is_active': True
}
pennersr commented 1 year ago

I see. With regular pk fields, if a model has a pk of value None that implies that the model was not saved yet. In your case, the model does have a pk value, yet it does not point to a DB record. How is the UUID field defined?

Likely, this is what #3419 was pointing at.

pennersr commented 1 year ago

Can you try this commit: 38514495 ?

Frohus commented 1 year ago

That's the pk field

    id = models.UUIDField(
        primary_key=True,
        db_index=True,
        default=uuid.uuid4,
        editable=False,
        unique=True,
    )

3851449 makes no difference as user.pk is linked to the id field and is returning the uuid of the temporary User

The other way to determine if User exists in the DB could be by checking:

    @property
    def is_existing(self):
        """When `False`, this social login represents a temporary account, not
        yet backed by a database record.
        """
        return self.user._state.adding is False

Tried it and it works. Also it's seem more universal than always asuming user uses default primary key.

https://docs.djangoproject.com/en/4.2/ref/models/instances/#state

pennersr commented 1 year ago

3851449 makes no difference as user.pk is linked to the id field and is returning the uuid of the temporary User

The changes made there do:

        if self.user.pk is None:
            return False
        return get_user_model().objects.filter(pk=self.user.pk).exists()

So:

Did you try running that?

As for _state -- that is populated based on whether or not the instance originates from a queryset. Yet, SocialLogin instances can end up being serialized and stored in a session. When deserializing, the instances no longer originate from a queryset, meaning, the state is lost.

Frohus commented 1 year ago

Apologies, I read the code and thought it wouldn't work without trying, it was late evening and I was half asleep 😄 . Tried it now and it works

pennersr commented 1 year ago

Thanks for confirming. Now, the question is, was this the only issue or is there more needed before #3419 can be closed ?

KirillovProj commented 8 months ago

I have persistent users inside my app before registration. I want to use allauth to associate them with their social accounts and use information from their social accounts to fill user's data.

Previously I used override like this in my custom SocialAccountAdapter class:

def new_user(self, request, sociallogin):
    user = request.user
    if user.id and user.date_registered is None:
        return user

    return super().new_user(request, sociallogin)

After this change this obviously does not work, as social login fails with AssertionError. Any tips for a workaround? Besides not keeping persistent users before registration in the first place.