diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e8b98ee..0d801a02 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ and this project adheres to - 🔧(backend) backport logging configuration from docs +### Fixed + +- 🐛(backend) prevent duplicate pending users on concurrent requests + ## [1.17.0] - 2026-05-31 ### Added diff --git a/src/backend/core/migrations/0019_user_unique_email_when_sub_is_null.py b/src/backend/core/migrations/0019_user_unique_email_when_sub_is_null.py new file mode 100644 index 00000000..154128fe --- /dev/null +++ b/src/backend/core/migrations/0019_user_unique_email_when_sub_is_null.py @@ -0,0 +1,19 @@ +# Generated by Django 5.2.14 on 2026-06-02 17:31 + +import django.db.models.functions.text +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('auth', '0012_alter_user_first_name_max_length'), + ('core', '0018_rename_active_application_is_active'), + ] + + operations = [ + migrations.AddConstraint( + model_name='user', + constraint=models.UniqueConstraint(django.db.models.functions.text.Lower('email'), condition=models.Q(('sub__isnull', True)), name='unique_email_when_sub_is_null'), + ), + ] diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 45fb271f..32d17db8 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -211,6 +211,13 @@ class User(AbstractBaseUser, BaseModel, auth_models.PermissionsMixin): ordering = ("-created_at",) verbose_name = _("user") verbose_name_plural = _("users") + constraints = [ + models.UniqueConstraint( + models.functions.Lower("email"), + condition=models.Q(sub__isnull=True), + name="unique_email_when_sub_is_null", + ) + ] def __str__(self): return self.email or self.admin_email or str(self.id) diff --git a/src/backend/core/tests/authentication/test_backends.py b/src/backend/core/tests/authentication/test_backends.py index 4e6d649d..2f707120 100644 --- a/src/backend/core/tests/authentication/test_backends.py +++ b/src/backend/core/tests/authentication/test_backends.py @@ -346,8 +346,11 @@ def test_authentication_getter_existing_user_change_fields( # One and only one additional update query when a field has changed # Note: .save() triggers uniqueness validation queries for unique fields, - # adding extra SELECT queries before the UPDATE (e.g., checking unique=True on 'sub') - with django_assert_num_queries(3): + # adding extra SELECT queries before the UPDATE: + # - unique=True on 'sub' + # - unique=True on 'admin_email' + # - partial unique index 'unique_email_when_sub_is_null' + with django_assert_num_queries(5): authenticated_user = klass.get_or_create_user( access_token="test-token", id_token=None, payload=None ) diff --git a/src/backend/core/tests/test_models_users.py b/src/backend/core/tests/test_models_users.py index edea5bb9..cb23cecd 100644 --- a/src/backend/core/tests/test_models_users.py +++ b/src/backend/core/tests/test_models_users.py @@ -44,3 +44,53 @@ def test_models_users_send_mail_main_missing(): user.email_user("my subject", "my message") assert str(excinfo.value) == "User has no email address." + + +def test_models_users_email_unique_when_sub_is_null(): + """Email should be unique among users with no sub (pending users).""" + user = factories.UserFactory(sub=None, email="test@example.com") + with pytest.raises( + ValidationError, match="Constraint “unique_email_when_sub_is_null” is violated." + ): + factories.UserFactory(sub=None, email=user.email) + + +def test_models_users_email_unique_case_insensitive_when_sub_is_null(): + """Email uniqueness should be case-insensitive among users with no sub (pending users).""" + factories.UserFactory(sub=None, email="Test@example.com") + with pytest.raises( + ValidationError, match="Constraint “unique_email_when_sub_is_null” is violated." + ): + factories.UserFactory(sub=None, email="test@example.com") + + +def test_models_users_email_not_unique_when_sub_is_set(): + """Email uniqueness should not be enforced when users have a sub.""" + user = factories.UserFactory(sub="sub-1", email="test@example.com") + user2 = factories.UserFactory(sub="sub-2", email=user.email) + assert user2.email == user.email + + +def test_models_users_email_not_unique_between_sub_null_and_sub_set(): + """A user with a sub and a pending user (sub=None) can share the same email.""" + user = factories.UserFactory(sub="sub-1", email="test@example.com") + user2 = factories.UserFactory(sub=None, email=user.email) + assert user2.email == user.email + + +def test_models_users_email_unique_constraint_allows_multiple_null_emails(): + """Multiple users with sub=None and email=None should be allowed.""" + factories.UserFactory(sub=None, email=None) + factories.UserFactory(sub=None, email=None) + + +def test_models_users_sub_null_email_null_does_not_prevent_creation(): + """Multiple pending users (sub=None, email=None) can be created without conflict. + + sub=None is not unique-constrained. email uniqueness is only enforced among + sub=None users with a non-null email, so email=None bypasses it (NULL != NULL in SQL). + """ + # Ghost row can still appear from bad code path + u1 = factories.UserFactory(sub=None, email=None) + u2 = factories.UserFactory(sub=None, email=None) + assert u1.pk != u2.pk