mirror of
https://github.com/suitenumerique/meet.git
synced 2026-07-26 11:58:53 +00:00
🐛(backend) prevent duplicate pending users on concurrent requests
Fix a race condition in the external viewset where concurrent requests could create multiple pending users with the same email address. No database constraint enforced email uniqueness for pending users, allowing duplicates to be created under load. This caused issues during user reconciliation, which expects a single matching pending user and raises a warning when multiple records are found. Add a narrowly scoped migration to enforce the uniqueness constraint and address the identified issue.
This commit is contained in:
committed by
aleb_the_flash
parent
28f652e035
commit
a8b79740e9
@@ -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
|
||||
|
||||
@@ -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'),
|
||||
),
|
||||
]
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
)
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user