From bbd99cfce5a2616106b9952219fb26fad369d61c Mon Sep 17 00:00:00 2001 From: Raymond Penners Date: Fri, 1 Dec 2023 13:47:30 +0100 Subject: [PATCH] fix(socialaccount): record_authentication only for logins --- allauth/socialaccount/helpers.py | 23 ++++++++------- allauth/socialaccount/tests/__init__.py | 25 +--------------- allauth/socialaccount/tests/test_login.py | 36 ++++++++++++++++++++++- 3 files changed, 49 insertions(+), 35 deletions(-) diff --git a/allauth/socialaccount/helpers.py b/allauth/socialaccount/helpers.py index e104eebbc9..eec6233270 100644 --- a/allauth/socialaccount/helpers.py +++ b/allauth/socialaccount/helpers.py @@ -4,9 +4,8 @@ from django.shortcuts import render from django.urls import reverse -from allauth.account import app_settings as account_settings +from allauth.account import app_settings as account_settings, authentication from allauth.account.adapter import get_adapter as get_account_adapter -from allauth.account.authentication import record_authentication from allauth.account.reauthentication import reauthenticate_then_callback from allauth.account.utils import ( assess_unique_email, @@ -94,6 +93,17 @@ def _process_signup(request, sociallogin): return resp +def record_authentication(request, sociallogin): + authentication.record_authentication( + request, + "socialaccount", + **{ + "provider": sociallogin.account.provider, + "uid": sociallogin.account.uid, + } + ) + + def _login_social_account(request, sociallogin): return perform_login( request, @@ -196,14 +206,6 @@ def _add_social_account(request, sociallogin): def complete_social_login(request, sociallogin): assert not sociallogin.is_existing sociallogin.lookup() - record_authentication( - request, - "socialaccount", - **{ - "provider": sociallogin.account.provider, - "uid": sociallogin.account.uid, - } - ) try: get_adapter().pre_social_login(request, sociallogin) signals.pre_social_login.send( @@ -229,6 +231,7 @@ def _complete_social_login(request, sociallogin): if request.user.is_authenticated: get_account_adapter(request).logout(request) if sociallogin.is_existing: + record_authentication(request, sociallogin) # Login existing user ret = _login_social_account(request, sociallogin) else: diff --git a/allauth/socialaccount/tests/__init__.py b/allauth/socialaccount/tests/__init__.py index f0fb1a5793..8777b128f6 100644 --- a/allauth/socialaccount/tests/__init__.py +++ b/allauth/socialaccount/tests/__init__.py @@ -4,7 +4,7 @@ import random import requests import warnings -from unittest.mock import ANY, Mock, patch +from unittest.mock import Mock, patch from urllib.parse import parse_qs, urlparse from django.conf import settings @@ -15,7 +15,6 @@ from django.utils.http import urlencode import allauth.app_settings -from allauth.account.authentication import AUTHENTICATION_METHODS_SESSION_KEY from allauth.account.models import EmailAddress from allauth.account.utils import user_email, user_username from allauth.socialaccount import app_settings @@ -82,17 +81,6 @@ def test_login(self): provider_account.get_profile_url() provider_account.get_brand() provider_account.to_str() - self.assertEqual( - self.client.session[AUTHENTICATION_METHODS_SESSION_KEY], - [ - { - "at": ANY, - "provider": self.provider_id, - "method": "socialaccount", - "uid": account.uid, - } - ], - ) return account @override_settings( @@ -223,17 +211,6 @@ def test_login(self): resp_mock, ) self.assertRedirects(resp, reverse("socialaccount_signup")) - self.assertEqual( - self.client.session[AUTHENTICATION_METHODS_SESSION_KEY], - [ - { - "at": ANY, - "provider": self.provider_id, - "method": "socialaccount", - "uid": ANY, - } - ], - ) @override_settings(SOCIALACCOUNT_AUTO_SIGNUP=False) def test_login_with_pkce_disabled(self): diff --git a/allauth/socialaccount/tests/test_login.py b/allauth/socialaccount/tests/test_login.py index ce72a40c2c..74ab72fd02 100644 --- a/allauth/socialaccount/tests/test_login.py +++ b/allauth/socialaccount/tests/test_login.py @@ -1,5 +1,5 @@ import copy -from unittest.mock import patch +from unittest.mock import ANY, patch from django.contrib.auth import get_user_model from django.contrib.auth.models import AnonymousUser @@ -10,9 +10,11 @@ import pytest from pytest_django.asserts import assertTemplateUsed +from allauth.account.authentication import AUTHENTICATION_METHODS_SESSION_KEY from allauth.core import context from allauth.socialaccount.helpers import complete_social_login from allauth.socialaccount.models import SocialAccount +from allauth.socialaccount.providers.base import AuthProcess @pytest.mark.parametrize("with_emailaddress", [False, True]) @@ -88,3 +90,35 @@ def test_login_cancelled(client): resp = client.get(reverse("socialaccount_login_cancelled")) assert resp.status_code == 200 assertTemplateUsed(resp, "socialaccount/login_cancelled.html") + + +@pytest.mark.parametrize( + "process,did_record", + [ + (AuthProcess.LOGIN, True), + (AuthProcess.CONNECT, False), + ], +) +def test_record_authentication( + db, sociallogin_factory, client, rf, user, process, did_record +): + sociallogin = sociallogin_factory(provider="unittest-server", uid="123") + sociallogin.state["process"] = process + SocialAccount.objects.create(user=user, uid="123", provider="unittest-server") + request = rf.get("/") + SessionMiddleware(lambda request: None).process_request(request) + MessageMiddleware(lambda request: None).process_request(request) + request.user = AnonymousUser() + with context.request_context(request): + complete_social_login(request, sociallogin) + if did_record: + assert request.session[AUTHENTICATION_METHODS_SESSION_KEY] == [ + { + "at": ANY, + "provider": sociallogin.account.provider, + "method": "socialaccount", + "uid": "123", + } + ] + else: + assert AUTHENTICATION_METHODS_SESSION_KEY not in request.session