fix: avoid associating with existing user when creating fails

This behavior was introduced in 9f86059
to workaround concurrency issues, but the only safe way to deal with
this is to restart the pipeline to make sure that all possible policies
apply. This is currently not possible, so let's fail with
AuthAlreadyAssociated and let user restart the authentication pipeline
manually.

Author: nijel

social_django/storage.py

@@ -9,6 +9,7 @@
 from django.core.exceptions import FieldDoesNotExist
 from django.db import router, transaction
 from django.db.utils import IntegrityError
+from social_core.exceptions import AuthAlreadyAssociated
 from social_core.storage import (
     AssociationMixin,
     BaseStorage,
@@ -81,20 +82,15 @@ def create_user(cls, *args, **kwargs):
                     cls.user_model()._meta.get_field("username")  # noqa: SLF001
                 except FieldDoesNotExist:
                     kwargs.pop("username")
+
+        # If the create fails below due to an IntegrityError, ensure that the transaction
+        # stays undamaged by wrapping the create in an atomic.
+        using = router.db_for_write(cls.user_model())
         try:
-            # If the create fails below due to an IntegrityError, ensure that the transaction
-            # stays undamaged by wrapping the create in an atomic.
-            using = router.db_for_write(cls.user_model())
             with transaction.atomic(using=using):
                 return manager.create_user(*args, **kwargs)
         except IntegrityError as exc:
-            # If email comes in as None it won't get found in the get
-            if kwargs.get("email", True) is None:
-                kwargs["email"] = ""
-            try:
-                return manager.get(*args, **kwargs)
-            except cls.user_model().DoesNotExist:
-                raise exc from None
+            raise AuthAlreadyAssociated(None) from exc
 
     @classmethod
     def filter_users(cls, *args, **kwargs) -> QuerySet:

tests/test_models.py

@@ -5,6 +5,7 @@
 from django.core.management import call_command
 from django.db import IntegrityError
 from django.test import TestCase, override_settings
+from social_core.exceptions import AuthAlreadyAssociated
 
 from social_django.models import (
     AbstractUserSocialAuth,
@@ -101,18 +102,22 @@ def test_get_username(self):
         self.assertEqual(UserSocialAuth.get_username(self.user), self.user.username)
 
     def test_create_user(self):
-        # Catch integrity error and find existing user
-        UserSocialAuth.create_user(username=self.user.username)
+        UserSocialAuth.create_user(username="testuser")
 
     def test_create_user_reraise(self):
-        with self.assertRaises(IntegrityError):
+        with self.assertRaises(AuthAlreadyAssociated):
             UserSocialAuth.create_user(username=self.user.username, email=None)
 
     @mock.patch("social_django.models.UserSocialAuth.username_field", return_value="email")
-    @mock.patch("django.contrib.auth.models.UserManager.create_user", side_effect=IntegrityError)
+    @mock.patch("django.contrib.auth.models.UserManager.create_user", return_value="<User>")
     def test_create_user_custom_username(self, *args):
         UserSocialAuth.create_user(username=self.user.email)
 
+    @mock.patch("django.contrib.auth.models.UserManager.create_user", side_effect=IntegrityError)
+    def test_create_user_existing(self, *args):
+        with self.assertRaises(AuthAlreadyAssociated):
+            UserSocialAuth.create_user(username=self.user.email)
+
     def test_get_user(self):
         self.assertEqual(UserSocialAuth.get_user(pk=self.user.pk), self.user)
         self.assertIsNone(UserSocialAuth.get_user(pk=123))