From 973c404884cbf66775660b638cfdfb8a67b6a06a Mon Sep 17 00:00:00 2001
From: saltie2193 <saltie@protonmail.com>
Date: Sat, 12 Oct 2024 20:34:43 +0200
Subject: [PATCH 1/2] =?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 | 50 ++++++++++++++++------
 2 files changed, 39 insertions(+), 13 deletions(-)

diff --git a/backend/app/api/routes/users.py b/backend/app/api/routes/users.py
index c636b094ee..9ace200972 100644
--- a/backend/app/api/routes/users.py
+++ b/backend/app/api/routes/users.py
@@ -172,6 +172,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..0d97a160e3 100644
--- a/backend/app/tests/api/routes/test_users.py
+++ b/backend/app/tests/api/routes/test_users.py
@@ -1,6 +1,7 @@
 import uuid
 from unittest.mock import patch
 
+import pytest
 from fastapi.testclient import TestClient
 from sqlmodel import Session, select
 
@@ -8,6 +9,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, user_authentication_headers
 from app.tests.utils.utils import random_email, random_lower_string
 
 
@@ -56,7 +58,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,21 +77,32 @@ def test_get_existing_user(
     assert existing_user.email == api_user["email"]
 
 
-def test_get_existing_user_current_user(client: TestClient, db: Session) -> None:
+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"}
+
+
+@pytest.mark.parametrize(
+    "is_superuser", (True, False), ids=lambda x: "superuser" if x else "normal user"
+)
+def test_get_existing_user_current_user(
+    client: TestClient, db: Session, is_superuser: bool
+) -> None:
     username = random_email()
     password = random_lower_string()
-    user_in = UserCreate(email=username, password=password)
+    user_in = UserCreate(email=username, password=password, is_superuser=is_superuser)
     user = crud.create_user(session=db, user_create=user_in)
     user_id = user.id
 
-    login_data = {
-        "username": username,
-        "password": password,
-    }
-    r = client.post(f"{settings.API_V1_STR}/login/access-token", data=login_data)
-    tokens = r.json()
-    a_token = tokens["access_token"]
-    headers = {"Authorization": f"Bearer {a_token}"}
+    headers = user_authentication_headers(
+        client=client, email=username, password=password
+    )
 
     r = client.get(
         f"{settings.API_V1_STR}/users/{user_id}",
@@ -102,11 +115,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 03eb016e49c3165a677d2e9db15ad6d206642ef1 Mon Sep 17 00:00:00 2001
From: saltie2193 <saltie@protonmail.com>
Date: Sat, 12 Oct 2024 20:48:14 +0200
Subject: [PATCH 2/2] 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 0d97a160e3..593012a7bb 100644
--- a/backend/app/tests/api/routes/test_users.py
+++ b/backend/app/tests/api/routes/test_users.py
@@ -79,7 +79,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,