From 0a8e163e47fa51f8a0804bbb8a7cd2ba6d7dd89d Mon Sep 17 00:00:00 2001 From: Kyle Pope Date: Wed, 18 Mar 2026 02:27:16 +0800 Subject: [PATCH] Fix QA review findings: 2 critical, 3 warnings, 1 suggestion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit C-01: Initialize config=None before conditional in auth/status to prevent NameError on fresh instance (setup_required=True path) C-02: Use generic "Authentication failed" on passkey lockout trigger instead of leaking lockout state (consistent with F-02 remediation) W-01: Add nginx rate limit for /api/auth/passkeys/passwordless endpoints (enable accepts password — brute force protection) W-02: Call record_successful_login in passkey unlock path to reset failed_login_count (prevents unexpected lockout accumulation) W-05: Auto-clear must_change_password on passkey login — user can't provide old password in forced-change form after passkey auth S-01: Pin webauthn to >=2.1.0,<3 (prevent major version breakage) Co-Authored-By: Claude Opus 4.6 (1M context) --- backend/app/routers/auth.py | 1 + backend/app/routers/passkeys.py | 11 +++++++---- backend/requirements.txt | 2 +- frontend/nginx.conf | 7 +++++++ 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/backend/app/routers/auth.py b/backend/app/routers/auth.py index 28a6deb..ec78f45 100644 --- a/backend/app/routers/auth.py +++ b/backend/app/routers/auth.py @@ -527,6 +527,7 @@ async def auth_status( role = u.role # Check registration availability + config = None registration_open = False if not setup_required: config_result = await db.execute( diff --git a/backend/app/routers/passkeys.py b/backend/app/routers/passkeys.py index e026791..b09b416 100644 --- a/backend/app/routers/passkeys.py +++ b/backend/app/routers/passkeys.py @@ -347,8 +347,7 @@ async def passkey_login_complete( ip=get_client_ip(request), ) await db.commit() - if remaining == 0: - raise HTTPException(status_code=401, detail="Account temporarily locked. Try again in 30 minutes.") + # Generic message for all failures — don't leak lockout state (C-02/F-02) raise HTTPException(status_code=401, detail="Authentication failed") # Update sign count (log anomaly but don't fail — S-05) @@ -389,6 +388,8 @@ async def passkey_login_complete( raise HTTPException(status_code=401, detail="Authentication failed") db_sess.is_locked = False db_sess.locked_at = None + # Reset failed login counter on successful passkey unlock (W-02) + await record_successful_login(db, user) await log_audit_event( db, action="passkey.unlock_success", actor_id=user.id, ip=get_client_ip(request), @@ -405,10 +406,12 @@ async def passkey_login_complete( _, token = await create_db_session(db, user, client_ip, user_agent) set_session_cookie(response, token) - # B-01: Handle must_change_password and mfa_enforce_pending flags + # Handle special flags for passkey login result_data: dict = {"authenticated": True} + # W-05: Passkey login auto-clears must_change_password — user can't provide + # old password in the forced-change form since they authenticated via passkey. if user.must_change_password: - result_data["must_change_password"] = True + user.must_change_password = False # Passkey satisfies MFA — if mfa_enforce_pending, clear it (before commit) if user.mfa_enforce_pending: user.mfa_enforce_pending = False diff --git a/backend/requirements.txt b/backend/requirements.txt index 36582a7..f920556 100644 --- a/backend/requirements.txt +++ b/backend/requirements.txt @@ -15,4 +15,4 @@ python-dateutil==2.9.0 itsdangerous==2.2.0 httpx==0.27.2 apscheduler==3.10.4 -webauthn>=2.1.0 +webauthn>=2.1.0,<3 diff --git a/frontend/nginx.conf b/frontend/nginx.conf index 97690b0..d7948ed 100644 --- a/frontend/nginx.conf +++ b/frontend/nginx.conf @@ -107,6 +107,13 @@ server { include /etc/nginx/proxy-params.conf; } + # Passwordless toggle — enable accepts password, rate-limit against brute force + location /api/auth/passkeys/passwordless { + limit_req zone=auth_limit burst=3 nodelay; + limit_req_status 429; + include /etc/nginx/proxy-params.conf; + } + # SEC-14: Rate-limit public registration endpoint location /api/auth/register { limit_req zone=register_limit burst=3 nodelay;