Skip to content

Commit 3597646

Browse files
fix(api): require auth on GET /api/users roster
list_users had no Request param and no auth dependency, so any anonymous caller received the full roster with every user's name decrypted (users.name is encrypted at rest). Inject request and call get_session_user_id before any DB access so unauthenticated callers get 401. The frontend already calls this with credentials:'include' from authenticated shell routes, so requiring auth is non-breaking. Closes #156
1 parent 2ed485f commit 3597646

3 files changed

Lines changed: 62 additions & 1 deletion

File tree

backend/main.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,13 +162,18 @@ def health():
162162

163163

164164
@app.get("/api/users")
165-
def list_users():
165+
def list_users(request: Request):
166166
"""List users with decrypted display names.
167167
168168
The `users.name` column is encrypted at rest (see
169169
services.encryption); decrypt before returning so clients render the
170170
human-readable name, not ciphertext. Sort by the decrypted value.
171+
172+
Requires an authenticated session: this returns decrypted legal names,
173+
so an unauthenticated caller must never reach the roster (401).
171174
"""
175+
from services.auth_guard import get_session_user_id
176+
get_session_user_id(request) # 401 if unauthenticated
172177
from db.connection import table
173178
from services.encryption import decrypt_if_present
174179
rows = table("users").select("id,name,room_id")

backend/tests/conftest.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ def _checker(request):
5353
return None
5454
return _checker
5555

56+
auth_guard._real_decode_session = auth_guard._decode_session
5657
auth_guard._real_require_self = auth_guard.require_self
5758
auth_guard._real_get_session_user_id = auth_guard.get_session_user_id
5859
auth_guard._real_require_admin = auth_guard.require_admin
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
"""
2+
Regression tests for GET /api/users (main.list_users) auth.
3+
4+
The roster endpoint returns each user's decrypted legal name, so it must
5+
require an authenticated session. See issue #156: it previously had no
6+
auth dependency and leaked the full roster to anonymous callers.
7+
8+
The autouse `_bypass_session_auth` fixture in conftest.py stubs the auth
9+
guard so authenticated-path tests don't need real tokens; the
10+
unauthenticated test restores the real guard to assert the 401.
11+
"""
12+
from unittest.mock import patch
13+
14+
from fastapi.testclient import TestClient
15+
16+
from main import app
17+
18+
client = TestClient(app)
19+
20+
21+
class TestListUsersAuth:
22+
def test_unauthenticated_returns_401(self):
23+
# Restore the real guard (and its session decoder) so a request with no
24+
# cookie/token => 401, undoing the autouse bypass from conftest.
25+
from services import auth_guard
26+
27+
with patch.object(
28+
auth_guard, "get_session_user_id", auth_guard._real_get_session_user_id
29+
), patch.object(
30+
auth_guard, "_decode_session", auth_guard._real_decode_session
31+
):
32+
# No cookie, no auth_token: _decode_session raises 401 before any
33+
# DB access, so we never touch the (unmocked) users table.
34+
r = client.get("/api/users")
35+
36+
assert r.status_code == 401
37+
38+
def test_authenticated_returns_200_with_decrypted_names(self):
39+
rows = [
40+
{"id": "u1", "name": "ENC_BOB", "room_id": "r1"},
41+
{"id": "u2", "name": "ENC_ALICE", "room_id": "r2"},
42+
]
43+
decrypt_map = {"ENC_BOB": "Bob", "ENC_ALICE": "Alice"}
44+
45+
with patch("db.connection.table") as t, patch(
46+
"services.encryption.decrypt_if_present",
47+
side_effect=lambda v: decrypt_map.get(v, v),
48+
):
49+
t.return_value.select.return_value = rows
50+
r = client.get("/api/users")
51+
52+
assert r.status_code == 200
53+
names = [u["name"] for u in r.json()["users"]]
54+
# Sorted by decrypted name, lowercased.
55+
assert names == ["Alice", "Bob"]

0 commit comments

Comments
 (0)