From 461e84acaea088ef93b4c7603211b79789b467ba Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fran=C3=A7ois=20Voron?= <fvoron@gmail.com>
Date: Tue, 20 Apr 2021 13:49:00 +0200
Subject: [PATCH] Fix #561: Update a user with an email already existing in DB
 raises an error

---
 fastapi_users/router/common.py |  1 +
 fastapi_users/router/users.py  | 24 ++++++++++++++++---
 tests/test_router_users.py     | 43 +++++++++++++++++++++++++++++++++-
 3 files changed, 64 insertions(+), 4 deletions(-)

diff --git a/fastapi_users/router/common.py b/fastapi_users/router/common.py
index d6a6266d..2506780f 100644
--- a/fastapi_users/router/common.py
+++ b/fastapi_users/router/common.py
@@ -10,6 +10,7 @@ class ErrorCode:
     VERIFY_USER_BAD_TOKEN = "VERIFY_USER_BAD_TOKEN"
     VERIFY_USER_ALREADY_VERIFIED = "VERIFY_USER_ALREADY_VERIFIED"
     VERIFY_USER_TOKEN_EXPIRED = "VERIFY_USER_TOKEN_EXPIRED"
+    UPDATE_USER_EMAIL_ALREADY_EXISTS = "UPDATE_USER_EMAIL_ALREADY_EXISTS"
 
 
 async def run_handler(handler: Callable, *args, **kwargs):
diff --git a/fastapi_users/router/users.py b/fastapi_users/router/users.py
index 622ff48d..dbeac4b1 100644
--- a/fastapi_users/router/users.py
+++ b/fastapi_users/router/users.py
@@ -7,7 +7,7 @@ from fastapi_users import models
 from fastapi_users.authentication import Authenticator
 from fastapi_users.db import BaseUserDatabase
 from fastapi_users.password import get_password_hash
-from fastapi_users.router.common import run_handler
+from fastapi_users.router.common import ErrorCode, run_handler
 
 
 def get_users_router(
@@ -35,6 +35,20 @@ def get_users_router(
             raise HTTPException(status_code=status.HTTP_404_NOT_FOUND)
         return user
 
+    async def _check_unique_email(
+        updated_user: user_update_model,  # type: ignore
+    ) -> None:
+        updated_user = cast(
+            models.BaseUserUpdate, updated_user
+        )  # Prevent mypy complain
+        if updated_user.email:
+            user = await user_db.get_by_email(updated_user.email)
+            if user is not None:
+                raise HTTPException(
+                    status.HTTP_400_BAD_REQUEST,
+                    detail=ErrorCode.UPDATE_USER_EMAIL_ALREADY_EXISTS,
+                )
+
     async def _update_user(
         user: models.BaseUserDB, update_dict: Dict[str, Any], request: Request
     ):
@@ -55,7 +69,11 @@ def get_users_router(
     ):
         return user
 
-    @router.patch("/me", response_model=user_model)
+    @router.patch(
+        "/me",
+        response_model=user_model,
+        dependencies=[Depends(get_current_active_user), Depends(_check_unique_email)],
+    )
     async def update_me(
         request: Request,
         updated_user: user_update_model,  # type: ignore
@@ -81,7 +99,7 @@ def get_users_router(
     @router.patch(
         "/{id:uuid}",
         response_model=user_model,
-        dependencies=[Depends(get_current_superuser)],
+        dependencies=[Depends(get_current_superuser), Depends(_check_unique_email)],
     )
     async def update_user(
         id: UUID4, updated_user: user_update_model, request: Request  # type: ignore
diff --git a/tests/test_router_users.py b/tests/test_router_users.py
index e273963c..1f7989df 100644
--- a/tests/test_router_users.py
+++ b/tests/test_router_users.py
@@ -7,7 +7,7 @@ import pytest
 from fastapi import FastAPI, Request, status
 
 from fastapi_users.authentication import Authenticator
-from fastapi_users.router import get_users_router
+from fastapi_users.router import ErrorCode, get_users_router
 from tests.conftest import MockAuthentication, User, UserDB, UserUpdate
 
 SECRET = "SECRET"
@@ -144,6 +144,28 @@ class TestUpdateMe:
         assert response.status_code == status.HTTP_401_UNAUTHORIZED
         assert after_update.called is False
 
+    async def test_existing_email(
+        self,
+        test_app_client: Tuple[httpx.AsyncClient, bool],
+        user: UserDB,
+        verified_user: UserDB,
+        after_update,
+    ):
+        client, requires_verification = test_app_client
+        response = await client.patch(
+            "/me",
+            json={"email": verified_user.email},
+            headers={"Authorization": f"Bearer {user.id}"},
+        )
+        if requires_verification:
+            assert response.status_code == status.HTTP_401_UNAUTHORIZED
+            assert after_update.called is False
+        else:
+            assert response.status_code == status.HTTP_400_BAD_REQUEST
+            data = cast(Dict[str, Any], response.json())
+            assert data["detail"] == ErrorCode.UPDATE_USER_EMAIL_ALREADY_EXISTS
+            assert after_update.called is False
+
     async def test_empty_body(
         self,
         test_app_client: Tuple[httpx.AsyncClient, bool],
@@ -685,6 +707,25 @@ class TestUpdateUser:
             data = cast(Dict[str, Any], response.json())
             assert data["email"] == "king.arthur@tintagel.bt"
 
+    async def test_existing_email_verified_superuser(
+        self,
+        test_app_client: Tuple[httpx.AsyncClient, bool],
+        user: UserDB,
+        verified_user: UserDB,
+        verified_superuser: UserDB,
+        after_update,
+    ):
+        client, _ = test_app_client
+        response = await client.patch(
+            f"/{user.id}",
+            json={"email": verified_user.email},
+            headers={"Authorization": f"Bearer {verified_superuser.id}"},
+        )
+        assert response.status_code == status.HTTP_400_BAD_REQUEST
+        data = cast(Dict[str, Any], response.json())
+        assert data["detail"] == ErrorCode.UPDATE_USER_EMAIL_ALREADY_EXISTS
+        assert after_update.called is False
+
     async def test_valid_body_verified_superuser(
         self,
         test_app_client: Tuple[httpx.AsyncClient, bool],