From 4cb4163d8aa19f6ddf3960a3471c5e3248c7a86b Mon Sep 17 00:00:00 2001 From: saltie2193 Date: Sat, 12 Oct 2024 20:34:43 +0200 Subject: [PATCH 1/4] =?UTF-8?q?=F0=9F=91=B7=20Handle=20non-existing=20user?= =?UTF-8?q?=20IDs=20in=20`read=5Fuser=5Fby=5Fid`.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix an issue where `read_user_by_id` would fail to return if the requested user ID did not exist. * Return `404 - Not Found` when ID does not exist. * Request without sufficient permission will always result in `403 - Unauthorized`. * Add tests to test requesting non-existing user IDs as superuser and normal user. --- backend/app/api/routes/users.py | 2 ++ backend/app/tests/api/routes/test_users.py | 29 +++++++++++++++++++--- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/backend/app/api/routes/users.py b/backend/app/api/routes/users.py index 6429818458..d14525e6ca 100644 --- a/backend/app/api/routes/users.py +++ b/backend/app/api/routes/users.py @@ -170,6 +170,8 @@ def read_user_by_id( status_code=403, detail="The user doesn't have enough privileges", ) + if user is None: + raise HTTPException(status_code=404, detail="User not found") return user diff --git a/backend/app/tests/api/routes/test_users.py b/backend/app/tests/api/routes/test_users.py index ba9be65426..537a2b0720 100644 --- a/backend/app/tests/api/routes/test_users.py +++ b/backend/app/tests/api/routes/test_users.py @@ -8,6 +8,7 @@ from app.core.config import settings from app.core.security import verify_password from app.models import User, UserCreate +from app.tests.utils.user import create_random_user from app.tests.utils.utils import random_email, random_lower_string @@ -56,7 +57,7 @@ def test_create_user_new_email( assert user.email == created_user["email"] -def test_get_existing_user( +def test_get_existing_user_as_superuser( client: TestClient, superuser_token_headers: dict[str, str], db: Session ) -> None: username = random_email() @@ -75,6 +76,17 @@ def test_get_existing_user( assert existing_user.email == api_user["email"] +def test_get_non_existing_user_as_superuser( + client: TestClient, superuser_token_headers: dict[str, str] +): + r = client.get( + f"{settings.API_V1_STR}/users/{uuid.uuid4()}", + headers=superuser_token_headers, + ) + assert r.status_code == 404 + assert r.json() == {"detail": "User not found"} + + def test_get_existing_user_current_user(client: TestClient, db: Session) -> None: username = random_email() password = random_lower_string() @@ -102,11 +114,22 @@ def test_get_existing_user_current_user(client: TestClient, db: Session) -> None assert existing_user.email == api_user["email"] +@pytest.mark.parametrize( + "exists", (True, False), ids=lambda x: "Existing user" if x else "No user" +) def test_get_existing_user_permissions_error( - client: TestClient, normal_user_token_headers: dict[str, str] + db: Session, + client: TestClient, + normal_user_token_headers: dict[str, str], + exists: bool, ) -> None: + if exists: + user = create_random_user(db) + user_id = user.id + else: + user_id = uuid.uuid4() r = client.get( - f"{settings.API_V1_STR}/users/{uuid.uuid4()}", + f"{settings.API_V1_STR}/users/{user_id}", headers=normal_user_token_headers, ) assert r.status_code == 403 From 1a0bb4a4084e53c0f197349bd3dfc75eaae09e73 Mon Sep 17 00:00:00 2001 From: saltie2193 Date: Sat, 12 Oct 2024 20:48:14 +0200 Subject: [PATCH 2/4] Fix linting issue --- backend/app/tests/api/routes/test_users.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/app/tests/api/routes/test_users.py b/backend/app/tests/api/routes/test_users.py index 537a2b0720..eba22c9d73 100644 --- a/backend/app/tests/api/routes/test_users.py +++ b/backend/app/tests/api/routes/test_users.py @@ -78,7 +78,7 @@ def test_get_existing_user_as_superuser( def test_get_non_existing_user_as_superuser( client: TestClient, superuser_token_headers: dict[str, str] -): +) -> None: r = client.get( f"{settings.API_V1_STR}/users/{uuid.uuid4()}", headers=superuser_token_headers, From e031948564013d64c12b2df2b6807ab1ad8f5f44 Mon Sep 17 00:00:00 2001 From: saltie2193 Date: Fri, 5 Sep 2025 18:05:33 +0200 Subject: [PATCH 3/4] Separate permission error tests for existing and non-existing users. --- backend/app/tests/api/routes/test_users.py | 26 ++++++++++++++-------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/backend/app/tests/api/routes/test_users.py b/backend/app/tests/api/routes/test_users.py index eba22c9d73..c77c260bfe 100644 --- a/backend/app/tests/api/routes/test_users.py +++ b/backend/app/tests/api/routes/test_users.py @@ -114,20 +114,28 @@ def test_get_existing_user_current_user(client: TestClient, db: Session) -> None assert existing_user.email == api_user["email"] -@pytest.mark.parametrize( - "exists", (True, False), ids=lambda x: "Existing user" if x else "No user" -) def test_get_existing_user_permissions_error( db: Session, client: TestClient, normal_user_token_headers: dict[str, str], - exists: bool, ) -> None: - if exists: - user = create_random_user(db) - user_id = user.id - else: - user_id = uuid.uuid4() + user = create_random_user(db) + user_id = user.id + + r = client.get( + f"{settings.API_V1_STR}/users/{user_id}", + headers=normal_user_token_headers, + ) + assert r.status_code == 403 + assert r.json() == {"detail": "The user doesn't have enough privileges"} + + +def test_get_non_existing_user_permissions_error( + client: TestClient, + normal_user_token_headers: dict[str, str], +) -> None: + user_id = uuid.uuid4() + r = client.get( f"{settings.API_V1_STR}/users/{user_id}", headers=normal_user_token_headers, From 1489617693719ae264cf97a349a973054f4d7426 Mon Sep 17 00:00:00 2001 From: Saltie Date: Tue, 9 Sep 2025 09:18:28 +0200 Subject: [PATCH 4/4] Update backend/app/tests/api/routes/test_users.py Co-authored-by: Motov Yurii <109919500+YuriiMotov@users.noreply.github.com> --- backend/app/tests/api/routes/test_users.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/backend/app/tests/api/routes/test_users.py b/backend/app/tests/api/routes/test_users.py index c77c260bfe..90058226e2 100644 --- a/backend/app/tests/api/routes/test_users.py +++ b/backend/app/tests/api/routes/test_users.py @@ -120,10 +120,9 @@ def test_get_existing_user_permissions_error( normal_user_token_headers: dict[str, str], ) -> None: user = create_random_user(db) - user_id = user.id r = client.get( - f"{settings.API_V1_STR}/users/{user_id}", + f"{settings.API_V1_STR}/users/{user.id}", headers=normal_user_token_headers, ) assert r.status_code == 403