From f3811b7520c23bc54e3541e51a71db2b0bc35308 Mon Sep 17 00:00:00 2001 From: mohammad Date: Sat, 14 Mar 2026 15:11:19 +0300 Subject: [PATCH] fix: catch race conditions when creating users --- .../tests/test_phone_auth_request_contract.py | 59 +++++++++++++++++++ backend/apps/accounts/views.py | 26 +++++--- 2 files changed, 78 insertions(+), 7 deletions(-) diff --git a/backend/apps/accounts/tests/test_phone_auth_request_contract.py b/backend/apps/accounts/tests/test_phone_auth_request_contract.py index 4d61cfa..bb39248 100644 --- a/backend/apps/accounts/tests/test_phone_auth_request_contract.py +++ b/backend/apps/accounts/tests/test_phone_auth_request_contract.py @@ -1,6 +1,7 @@ from unittest.mock import patch import pytest +from django.db import IntegrityError from django.test import override_settings from django.urls import reverse @@ -61,6 +62,64 @@ def test_phone_auth_request_existing_phone_no_duplicate_user(client): assert PhoneOTP.objects.filter(phone_number="+966512345678").count() == 1 +@pytest.mark.django_db +@override_settings(OTP_PROVIDER="console") +def test_phone_auth_request_handles_duplicate_user_creation(client): + original_create_user = User.objects.create_user + otp_code = "123456" + + def create_user_and_raise(*args, **kwargs): + original_create_user(*args, **kwargs) + raise IntegrityError("duplicate user") + + with patch("apps.accounts.views.User.objects.create_user", side_effect=create_user_and_raise): + with patch("apps.accounts.services.otp.generate_code", return_value=otp_code): + response = client.post( + reverse("phone_auth_request"), + { + "phone_number": "0512345678", + "channel": "sms", + "first_name": "Sara", + "last_name": "Ali", + "email": "sara@example.com", + }, + content_type="application/json", + ) + + assert response.status_code == 201 + assert User.objects.filter(phone_number="+966512345678").count() == 1 + assert PhoneOTP.objects.filter(phone_number="+966512345678").count() == 1 + + +@pytest.mark.django_db +@override_settings(OTP_PROVIDER="console") +def test_phone_auth_request_race_with_email_conflict(client): + original_create_user = User.objects.create_user + target_email = "race@example.com" + + def create_conflict_user_then_raise(*args, **kwargs): + original_create_user(phone_number="+966500000002", email=target_email) + raise IntegrityError("email already claimed") + + before_otp_count = PhoneOTP.objects.count() + + with patch("apps.accounts.views.User.objects.create_user", side_effect=create_conflict_user_then_raise): + response = client.post( + reverse("phone_auth_request"), + { + "phone_number": "0512345678", + "channel": "sms", + "email": target_email, + }, + content_type="application/json", + ) + + assert response.status_code == 400 + assert "detail" in response.json() + assert User.objects.filter(phone_number="+966512345678").count() == 0 + assert PhoneOTP.objects.count() == before_otp_count + + @pytest.mark.django_db @override_settings(OTP_PROVIDER="console") def test_phone_auth_request_rejects_email_already_used(client): diff --git a/backend/apps/accounts/views.py b/backend/apps/accounts/views.py index a33eae8..390cb25 100644 --- a/backend/apps/accounts/views.py +++ b/backend/apps/accounts/views.py @@ -1,4 +1,5 @@ from django.contrib.auth import get_user_model +from django.db import IntegrityError from rest_framework import generics, permissions, status from rest_framework.response import Response from django.utils.translation import gettext as _ @@ -125,13 +126,24 @@ class PhoneAuthRequestView(APIView): {"detail": _("Email already in use.")}, status=status.HTTP_400_BAD_REQUEST, ) - user = User.objects.create_user( - email=email, - phone_number=phone_number, - first_name=data.get("first_name", ""), - last_name=data.get("last_name", ""), - role="customer", - ) + try: + user = User.objects.create_user( + email=email, + phone_number=phone_number, + first_name=data.get("first_name", ""), + last_name=data.get("last_name", ""), + role="customer", + ) + except IntegrityError: + user = User.objects.filter(phone_number=phone_number).first() + if not user: + # Another worker may have claimed this phone or email after our initial checks. + if email and User.objects.filter(email=email).exists(): + return Response( + {"detail": _("Email already in use.")}, + status=status.HTTP_400_BAD_REQUEST, + ) + raise try: result = create_and_send_otp(