fix(socialaccount): record_authentication only for logins

pennersr · Dec 1, 2023 · bbd99cf · bbd99cf
1 parent 0e25434
commit bbd99cf
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 35 deletions.
diff --git 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
@@ -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
@@ -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