Skip to content

Commit 4cb4163

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 45d7b83 commit 4cb4163

File tree

2 files changed

+28
-3
lines changed

2 files changed

+28
-3
lines changed

backend/app/api/routes/users.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,8 @@ def read_user_by_id(
170170
status_code=403,
171171
detail="The user doesn't have enough privileges",
172172
)
173+
if user is None:
174+
raise HTTPException(status_code=404, detail="User not found")
173175
return user
174176

175177

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

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
from app.core.config import settings
99
from app.core.security import verify_password
1010
from app.models import User, UserCreate
11+
from app.tests.utils.user import create_random_user
1112
from app.tests.utils.utils import random_email, random_lower_string
1213

1314

@@ -56,7 +57,7 @@ def test_create_user_new_email(
5657
assert user.email == created_user["email"]
5758

5859

59-
def test_get_existing_user(
60+
def test_get_existing_user_as_superuser(
6061
client: TestClient, superuser_token_headers: dict[str, str], db: Session
6162
) -> None:
6263
username = random_email()
@@ -75,6 +76,17 @@ def test_get_existing_user(
7576
assert existing_user.email == api_user["email"]
7677

7778

79+
def test_get_non_existing_user_as_superuser(
80+
client: TestClient, superuser_token_headers: dict[str, str]
81+
):
82+
r = client.get(
83+
f"{settings.API_V1_STR}/users/{uuid.uuid4()}",
84+
headers=superuser_token_headers,
85+
)
86+
assert r.status_code == 404
87+
assert r.json() == {"detail": "User not found"}
88+
89+
7890
def test_get_existing_user_current_user(client: TestClient, db: Session) -> None:
7991
username = random_email()
8092
password = random_lower_string()
@@ -102,11 +114,22 @@ def test_get_existing_user_current_user(client: TestClient, db: Session) -> None
102114
assert existing_user.email == api_user["email"]
103115

104116

117+
@pytest.mark.parametrize(
118+
"exists", (True, False), ids=lambda x: "Existing user" if x else "No user"
119+
)
105120
def test_get_existing_user_permissions_error(
106-
client: TestClient, normal_user_token_headers: dict[str, str]
121+
db: Session,
122+
client: TestClient,
123+
normal_user_token_headers: dict[str, str],
124+
exists: bool,
107125
) -> None:
126+
if exists:
127+
user = create_random_user(db)
128+
user_id = user.id
129+
else:
130+
user_id = uuid.uuid4()
108131
r = client.get(
109-
f"{settings.API_V1_STR}/users/{uuid.uuid4()}",
132+
f"{settings.API_V1_STR}/users/{user_id}",
110133
headers=normal_user_token_headers,
111134
)
112135
assert r.status_code == 403

0 commit comments

Comments
 (0)