Skip to content

Commit 973c404

Browse files
committed
👷 Handle non-existing user IDs in read_user_by_id.
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.
1 parent 6768359 commit 973c404

File tree

2 files changed

+39
-13
lines changed

2 files changed

+39
-13
lines changed

backend/app/api/routes/users.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,8 @@ def read_user_by_id(
172172
status_code=403,
173173
detail="The user doesn't have enough privileges",
174174
)
175+
if user is None:
176+
raise HTTPException(status_code=404, detail="User not found")
175177
return user
176178

177179

backend/app/tests/api/routes/test_users.py

Lines changed: 37 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
import uuid
22
from unittest.mock import patch
33

4+
import pytest
45
from fastapi.testclient import TestClient
56
from sqlmodel import Session, select
67

78
from app import crud
89
from app.core.config import settings
910
from app.core.security import verify_password
1011
from app.models import User, UserCreate
12+
from app.tests.utils.user import create_random_user, user_authentication_headers
1113
from app.tests.utils.utils import random_email, random_lower_string
1214

1315

@@ -56,7 +58,7 @@ def test_create_user_new_email(
5658
assert user.email == created_user["email"]
5759

5860

59-
def test_get_existing_user(
61+
def test_get_existing_user_as_superuser(
6062
client: TestClient, superuser_token_headers: dict[str, str], db: Session
6163
) -> None:
6264
username = random_email()
@@ -75,21 +77,32 @@ def test_get_existing_user(
7577
assert existing_user.email == api_user["email"]
7678

7779

78-
def test_get_existing_user_current_user(client: TestClient, db: Session) -> None:
80+
def test_get_non_existing_user_as_superuser(
81+
client: TestClient, superuser_token_headers: dict[str, str]
82+
):
83+
r = client.get(
84+
f"{settings.API_V1_STR}/users/{uuid.uuid4()}",
85+
headers=superuser_token_headers,
86+
)
87+
assert r.status_code == 404
88+
assert r.json() == {"detail": "User not found"}
89+
90+
91+
@pytest.mark.parametrize(
92+
"is_superuser", (True, False), ids=lambda x: "superuser" if x else "normal user"
93+
)
94+
def test_get_existing_user_current_user(
95+
client: TestClient, db: Session, is_superuser: bool
96+
) -> None:
7997
username = random_email()
8098
password = random_lower_string()
81-
user_in = UserCreate(email=username, password=password)
99+
user_in = UserCreate(email=username, password=password, is_superuser=is_superuser)
82100
user = crud.create_user(session=db, user_create=user_in)
83101
user_id = user.id
84102

85-
login_data = {
86-
"username": username,
87-
"password": password,
88-
}
89-
r = client.post(f"{settings.API_V1_STR}/login/access-token", data=login_data)
90-
tokens = r.json()
91-
a_token = tokens["access_token"]
92-
headers = {"Authorization": f"Bearer {a_token}"}
103+
headers = user_authentication_headers(
104+
client=client, email=username, password=password
105+
)
93106

94107
r = client.get(
95108
f"{settings.API_V1_STR}/users/{user_id}",
@@ -102,11 +115,22 @@ def test_get_existing_user_current_user(client: TestClient, db: Session) -> None
102115
assert existing_user.email == api_user["email"]
103116

104117

118+
@pytest.mark.parametrize(
119+
"exists", (True, False), ids=lambda x: "Existing user" if x else "No user"
120+
)
105121
def test_get_existing_user_permissions_error(
106-
client: TestClient, normal_user_token_headers: dict[str, str]
122+
db: Session,
123+
client: TestClient,
124+
normal_user_token_headers: dict[str, str],
125+
exists: bool,
107126
) -> None:
127+
if exists:
128+
user = create_random_user(db)
129+
user_id = user.id
130+
else:
131+
user_id = uuid.uuid4()
108132
r = client.get(
109-
f"{settings.API_V1_STR}/users/{uuid.uuid4()}",
133+
f"{settings.API_V1_STR}/users/{user_id}",
110134
headers=normal_user_token_headers,
111135
)
112136
assert r.status_code == 403

0 commit comments

Comments
 (0)