diff --git a/backend/app/api/routes/users.py b/backend/app/api/routes/users.py index 1748f58484..b13e5006bf 100644 --- a/backend/app/api/routes/users.py +++ b/backend/app/api/routes/users.py @@ -2,6 +2,7 @@ from typing import Any from fastapi import APIRouter, Depends, HTTPException +from sqlalchemy.exc import IntegrityError from sqlmodel import col, delete, func, select from app import crud @@ -65,7 +66,15 @@ def create_user(*, session: SessionDep, user_in: UserCreate) -> Any: detail="The user with this email already exists in the system.", ) - user = crud.create_user(session=session, user_create=user_in) + try: + user = crud.create_user(session=session, user_create=user_in) + except IntegrityError: + # Another request created this email between the check and the insert + session.rollback() + raise HTTPException( + status_code=400, + detail="The user with this email already exists in the system.", + ) if settings.emails_enabled and user_in.email: email_data = generate_new_account_email( email_to=user_in.email, username=user_in.email, password=user_in.password @@ -155,7 +164,15 @@ def register_user(session: SessionDep, user_in: UserRegister) -> Any: detail="The user with this email already exists in the system", ) user_create = UserCreate.model_validate(user_in) - user = crud.create_user(session=session, user_create=user_create) + try: + user = crud.create_user(session=session, user_create=user_create) + except IntegrityError: + # Another request created this email between the check and the insert + session.rollback() + raise HTTPException( + status_code=400, + detail="The user with this email already exists in the system", + ) return user diff --git a/backend/tests/api/routes/test_users.py b/backend/tests/api/routes/test_users.py index 9c4cdd5991..0a12034c4e 100644 --- a/backend/tests/api/routes/test_users.py +++ b/backend/tests/api/routes/test_users.py @@ -162,6 +162,24 @@ def test_create_user_existing_username( assert "_id" not in created_user +def test_create_user_email_taken_between_check_and_insert( + client: TestClient, superuser_token_headers: dict[str, str] +) -> None: + # Two requests with the same email can both pass the check. The unique + # index stops the second insert, which must still answer 400. + data = {"email": settings.FIRST_SUPERUSER, "password": random_lower_string()} + with patch("app.crud.get_user_by_email", return_value=None): + r = client.post( + f"{settings.API_V1_STR}/users/", + headers=superuser_token_headers, + json=data, + ) + assert r.status_code == 400 + assert ( + r.json()["detail"] == "The user with this email already exists in the system." + ) + + def test_create_user_by_normal_user( client: TestClient, normal_user_token_headers: dict[str, str] ) -> None: @@ -355,6 +373,22 @@ def test_register_user_already_exists_error(client: TestClient) -> None: assert r.json()["detail"] == "The user with this email already exists in the system" +def test_register_user_email_taken_between_check_and_insert( + client: TestClient, +) -> None: + # Two signups with the same email can both pass the check. The unique + # index stops the second insert, which must still answer 400. + data = { + "email": settings.FIRST_SUPERUSER, + "password": random_lower_string(), + "full_name": random_lower_string(), + } + with patch("app.crud.get_user_by_email", return_value=None): + r = client.post(f"{settings.API_V1_STR}/users/signup", json=data) + assert r.status_code == 400 + assert r.json()["detail"] == "The user with this email already exists in the system" + + def test_update_user( client: TestClient, superuser_token_headers: dict[str, str], db: Session ) -> None: