fix: catch race conditions when creating users
This commit is contained in:
@@ -1,6 +1,7 @@
|
|||||||
from unittest.mock import patch
|
from unittest.mock import patch
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
from django.db import IntegrityError
|
||||||
from django.test import override_settings
|
from django.test import override_settings
|
||||||
from django.urls import reverse
|
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
|
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
|
@pytest.mark.django_db
|
||||||
@override_settings(OTP_PROVIDER="console")
|
@override_settings(OTP_PROVIDER="console")
|
||||||
def test_phone_auth_request_rejects_email_already_used(client):
|
def test_phone_auth_request_rejects_email_already_used(client):
|
||||||
|
|||||||
@@ -1,4 +1,5 @@
|
|||||||
from django.contrib.auth import get_user_model
|
from django.contrib.auth import get_user_model
|
||||||
|
from django.db import IntegrityError
|
||||||
from rest_framework import generics, permissions, status
|
from rest_framework import generics, permissions, status
|
||||||
from rest_framework.response import Response
|
from rest_framework.response import Response
|
||||||
from django.utils.translation import gettext as _
|
from django.utils.translation import gettext as _
|
||||||
@@ -125,6 +126,7 @@ class PhoneAuthRequestView(APIView):
|
|||||||
{"detail": _("Email already in use.")},
|
{"detail": _("Email already in use.")},
|
||||||
status=status.HTTP_400_BAD_REQUEST,
|
status=status.HTTP_400_BAD_REQUEST,
|
||||||
)
|
)
|
||||||
|
try:
|
||||||
user = User.objects.create_user(
|
user = User.objects.create_user(
|
||||||
email=email,
|
email=email,
|
||||||
phone_number=phone_number,
|
phone_number=phone_number,
|
||||||
@@ -132,6 +134,16 @@ class PhoneAuthRequestView(APIView):
|
|||||||
last_name=data.get("last_name", ""),
|
last_name=data.get("last_name", ""),
|
||||||
role="customer",
|
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:
|
try:
|
||||||
result = create_and_send_otp(
|
result = create_and_send_otp(
|
||||||
|
|||||||
Reference in New Issue
Block a user