Skip to content

Commit fba9743

Browse files
committed
security: remove APP_PASSWORD_HASH login fallback — email+password required
1 parent 156c8c0 commit fba9743

9 files changed

Lines changed: 44 additions & 68 deletions

.env.example

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
# ─── Backend (.env at repo root — read by the openstudy container) ─────────
22

3-
# Argon2id hash of the password that gates the app. Generate with:
4-
# docker run --rm python:3.12-slim sh -c \
5-
# 'pip install -q argon2-cffi && python -c "from argon2 import PasswordHasher; print(PasswordHasher().hash(input(\"password: \")))"'
3+
# Bootstrap-only: the scripts/seed_operator_password.py script reads this on
4+
# first deploy to set users.password_hash for the operator account in the DB.
5+
# After that, login uses users.password_hash from the DB — this env var is no
6+
# longer read at request time. Generate the hash with:
7+
# uv run app/tools/hashpw.py
68
APP_PASSWORD_HASH=
79

810
# Random secret for session signing. REQUIRED — app refuses to start without one.

app/auth.py

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -86,19 +86,6 @@ def hash_password(plain: str) -> str:
8686
return _ph.hash(plain)
8787

8888

89-
def verify_password(plain: str) -> bool:
90-
s = get_settings()
91-
if not s.app_password_hash:
92-
return False
93-
try:
94-
_ph.verify(s.app_password_hash, plain)
95-
return True
96-
except VerifyMismatchError:
97-
return False
98-
except Exception:
99-
return False
100-
101-
10289
async def verify_password_for_user(email: str, plain: str) -> Optional[User]:
10390
"""Look up user by email; argon2-verify; return User or None.
10491

app/config.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ class Settings(BaseSettings):
1212
)
1313

1414
# Auth
15-
app_password_hash: str = Field(default="")
1615
# No default — production deploys must set SESSION_SECRET. The setter
1716
# below raises if it's left empty so we fail-closed instead of signing
1817
# cookies with a publicly-known string.

app/routers/auth.py

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,8 @@
1212
issue_session,
1313
optional_auth,
1414
require_auth,
15-
verify_password,
1615
verify_password_for_user,
1716
verify_totp,
18-
_sentinel_user,
1917
)
2018
from ..ratelimit import check_login_rate, record_login_attempt, check_auth_rate
2119
from ..schemas import LoginRequest, SessionInfo, TotpSetupResponse, TotpVerifyRequest, SignupRequest, ForgotRequest, ResetRequest
@@ -34,16 +32,7 @@ def _verify_totp_code(code: str, secret: str) -> bool:
3432
async def login(body: LoginRequest, request: Request, response: Response) -> SessionInfo:
3533
await check_login_rate(request)
3634

37-
user: Optional[User] = None
38-
39-
if body.email:
40-
# Email+password path: lookup + argon2 verify
41-
user = await verify_password_for_user(body.email, body.password)
42-
else:
43-
# Operator-legacy fallback: verify against APP_PASSWORD_HASH
44-
legacy_ok = verify_password(body.password)
45-
if legacy_ok:
46-
user = _sentinel_user()
35+
user: Optional[User] = await verify_password_for_user(body.email, body.password)
4736

4837
if user is None:
4938
await record_login_attempt(request, False)

app/schemas.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,7 @@ class DashboardSummary(BaseModel):
392392

393393
# ---------- Auth ----------
394394
class LoginRequest(BaseModel):
395-
email: Optional[str] = None # If omitted, falls back to operator-legacy
395+
email: str
396396
password: str
397397
totp_code: Optional[str] = None
398398

tests/mcp/test_bearer_user_binding.py

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,24 +20,19 @@
2020

2121
import pytest
2222
import pytest_asyncio
23-
from argon2 import PasswordHasher
2423
from httpx import ASGITransport, AsyncClient
2524

2625
import app.db as db_module
2726
from app.config import get_settings
2827

2928

30-
_TEST_PASSWORD = "test-password-1234"
31-
_TEST_PASSWORD_HASH = PasswordHasher().hash(_TEST_PASSWORD)
32-
3329
_USER_A_ID = "00000000-0000-0000-0000-000000000001" # operator (already seeded)
3430
_USER_B_ID = "11111111-1111-1111-1111-111111111111"
3531

3632

3733
@pytest_asyncio.fixture
3834
async def https_client(db_conn, monkeypatch):
3935
"""https://test base_url so Secure cookies survive the round-trip."""
40-
monkeypatch.setenv("APP_PASSWORD_HASH", _TEST_PASSWORD_HASH)
4136
get_settings.cache_clear()
4237
monkeypatch.setattr(db_module, "_pool", db_conn)
4338

tests/test_integration_full_flow.py

Lines changed: 26 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,19 @@
2727
_TEST_PASSWORD_HASH = PasswordHasher().hash(_TEST_PASSWORD)
2828

2929

30+
_OPERATOR_EMAIL = "operator@local"
31+
32+
3033
@pytest_asyncio.fixture
3134
async def https_client(db_conn, monkeypatch):
3235
"""Like the conftest `client` fixture but with `https://test` base_url
33-
so Secure cookies survive the round-trip, plus a known
34-
APP_PASSWORD_HASH so /api/auth/login can succeed."""
35-
monkeypatch.setenv("APP_PASSWORD_HASH", _TEST_PASSWORD_HASH)
36+
so Secure cookies survive the round-trip. Seeds the operator's
37+
password_hash directly in the DB so /api/auth/login can succeed."""
38+
async with db_conn.connection() as conn, conn.cursor() as cur:
39+
await cur.execute(
40+
"UPDATE users SET password_hash = %s WHERE email = %s",
41+
(_TEST_PASSWORD_HASH, _OPERATOR_EMAIL),
42+
)
3643
get_settings.cache_clear()
3744
monkeypatch.setattr(db_module, "_pool", db_conn)
3845

@@ -66,7 +73,7 @@ async def test_oauth_full_lifecycle(https_client, db_conn):
6673

6774
# 2. Login with the test password (no TOTP yet).
6875
login = await https_client.post(
69-
"/api/auth/login", json={"password": _TEST_PASSWORD}
76+
"/api/auth/login", json={"email": _OPERATOR_EMAIL, "password": _TEST_PASSWORD}
7077
)
7178
assert login.status_code == 200, login.text
7279
assert https_client.cookies.get("study_session")
@@ -166,20 +173,20 @@ async def test_login_rate_limit_then_success(https_client, db_conn):
166173
# 5 failed attempts: each 401 with "invalid credentials".
167174
for _ in range(5):
168175
bad = await https_client.post(
169-
"/api/auth/login", json={"password": "wrong"}
176+
"/api/auth/login", json={"email": _OPERATOR_EMAIL, "password": "wrong"}
170177
)
171178
assert bad.status_code == 401, bad.text
172179

173180
# 6th attempt: bucket is full → 429 BEFORE reaching verify_password.
174181
capped = await https_client.post(
175-
"/api/auth/login", json={"password": "wrong"}
182+
"/api/auth/login", json={"email": _OPERATOR_EMAIL, "password": "wrong"}
176183
)
177184
assert capped.status_code == 429, capped.text
178185

179186
# Even a correct password is rejected once rate-limited (the limit is
180-
# checked BEFORE verify_password).
187+
# checked BEFORE verify_password_for_user).
181188
still_capped = await https_client.post(
182-
"/api/auth/login", json={"password": _TEST_PASSWORD}
189+
"/api/auth/login", json={"email": _OPERATOR_EMAIL, "password": _TEST_PASSWORD}
183190
)
184191
assert still_capped.status_code == 429
185192

@@ -190,7 +197,7 @@ async def test_login_rate_limit_then_success(https_client, db_conn):
190197

191198
# Correct password now succeeds.
192199
ok = await https_client.post(
193-
"/api/auth/login", json={"password": _TEST_PASSWORD}
200+
"/api/auth/login", json={"email": _OPERATOR_EMAIL, "password": _TEST_PASSWORD}
194201
)
195202
assert ok.status_code == 200, ok.text
196203
assert https_client.cookies.get("study_session")
@@ -202,9 +209,9 @@ async def test_login_rate_limit_then_success(https_client, db_conn):
202209

203210
@pytest.mark.asyncio
204211
async def test_totp_enroll_and_login(https_client, db_conn):
205-
# 1. Login with password only (TOTP not yet enabled).
212+
# 1. Login with email + password (TOTP not yet enabled).
206213
login = await https_client.post(
207-
"/api/auth/login", json={"password": _TEST_PASSWORD}
214+
"/api/auth/login", json={"email": _OPERATOR_EMAIL, "password": _TEST_PASSWORD}
208215
)
209216
assert login.status_code == 200, login.text
210217

@@ -224,25 +231,26 @@ async def test_totp_enroll_and_login(https_client, db_conn):
224231
# Drop the cookie so subsequent /login attempts are unauthenticated.
225232
https_client.cookies.clear()
226233

227-
# 4. Password without code → 401 totp_required.
234+
# 4. Email + password without code → 401 totp_required.
228235
pw_only = await https_client.post(
229-
"/api/auth/login", json={"password": _TEST_PASSWORD}
236+
"/api/auth/login", json={"email": _OPERATOR_EMAIL, "password": _TEST_PASSWORD}
230237
)
231238
assert pw_only.status_code == 401, pw_only.text
232239
assert pw_only.json()["detail"] == "totp_required"
233240

234-
# 5. Password + wrong code → 401 invalid totp.
241+
# 5. Email + password + wrong code → 401 invalid totp.
235242
bad_code = await https_client.post(
236243
"/api/auth/login",
237-
json={"password": _TEST_PASSWORD, "totp_code": "000000"},
244+
json={"email": _OPERATOR_EMAIL, "password": _TEST_PASSWORD, "totp_code": "000000"},
238245
)
239246
assert bad_code.status_code == 401, bad_code.text
240247
assert bad_code.json()["detail"] == "invalid totp code"
241248

242-
# 6. Password + correct code → 200, cookie set.
249+
# 6. Email + password + correct code → 200, cookie set.
243250
good = await https_client.post(
244251
"/api/auth/login",
245252
json={
253+
"email": _OPERATOR_EMAIL,
246254
"password": _TEST_PASSWORD,
247255
"totp_code": pyotp.TOTP(secret).now(),
248256
},
@@ -253,18 +261,9 @@ async def test_totp_enroll_and_login(https_client, db_conn):
253261

254262
@pytest.mark.asyncio
255263
async def test_login_with_email_and_password(https_client, db_conn):
256-
"""The new email+password path works against an operator-seeded user."""
257-
# Operator user is seeded by Phase 1 migration with NULL password_hash.
258-
# Set a password hash directly via SQL (mimicking the seed script).
259-
pw_hash = PasswordHasher().hash(_TEST_PASSWORD)
260-
async with db_conn.connection() as conn, conn.cursor() as cur:
261-
await cur.execute(
262-
"UPDATE users SET password_hash = %s WHERE email = 'operator@local'",
263-
(pw_hash,),
264-
)
265-
# Now login with email + password
264+
"""Email+password login works; password_hash is seeded by the fixture."""
266265
resp = await https_client.post("/api/auth/login", json={
267-
"email": "operator@local",
266+
"email": _OPERATOR_EMAIL,
268267
"password": _TEST_PASSWORD,
269268
})
270269
assert resp.status_code == 200, resp.text

tests/test_integration_routes.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,10 @@ async def test_dashboard_aggregates_every_service(db_conn, monkeypatch):
5656

5757
@pytest.mark.asyncio
5858
async def test_login_flow_uses_async_db_paths(client, db_conn, monkeypatch):
59-
"""Drive /auth/login through the async ratelimit + auth.get_totp_state
60-
paths. Expects a 401 (no password configured, but the request must reach
59+
"""Drive /auth/login through the async ratelimit + verify_password_for_user
60+
paths. Expects a 401 (unknown email, but the request must reach
6161
that error rather than 500-ing on a missed await)."""
62-
# No app_password_hash configured → verify_password returns False → 401.
63-
resp = await client.post("/api/auth/login", json={"password": "irrelevant"})
62+
# Unknown email → verify_password_for_user returns None → 401.
63+
resp = await client.post("/api/auth/login", json={"email": "nobody@test.local", "password": "irrelevant"})
6464
assert resp.status_code == 401, resp.text
6565
assert resp.json()["detail"] == "invalid credentials"

tests/test_oauth_consent_csrf.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,16 @@
2020

2121
_TEST_PASSWORD = "test-password-csrf"
2222
_TEST_PASSWORD_HASH = PasswordHasher().hash(_TEST_PASSWORD)
23+
_OPERATOR_EMAIL = "operator@local"
2324

2425

2526
@pytest_asyncio.fixture
2627
async def https_client(db_conn, monkeypatch):
27-
monkeypatch.setenv("APP_PASSWORD_HASH", _TEST_PASSWORD_HASH)
28+
async with db_conn.connection() as conn, conn.cursor() as cur:
29+
await cur.execute(
30+
"UPDATE users SET password_hash = %s WHERE email = %s",
31+
(_TEST_PASSWORD_HASH, _OPERATOR_EMAIL),
32+
)
2833
get_settings.cache_clear()
2934
monkeypatch.setattr(db_module, "_pool", db_conn)
3035

@@ -57,7 +62,7 @@ async def _register_and_login(client: AsyncClient) -> str:
5762
client_id = reg.json()["client_id"]
5863

5964
login = await client.post(
60-
"/api/auth/login", json={"password": _TEST_PASSWORD}
65+
"/api/auth/login", json={"email": _OPERATOR_EMAIL, "password": _TEST_PASSWORD}
6166
)
6267
assert login.status_code == 200
6368
return client_id

0 commit comments

Comments
 (0)