Fix QA review findings: 2 critical, 3 warnings, 1 suggestion
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) <noreply@anthropic.com>
This commit is contained in:
parent
94891d8a70
commit
0a8e163e47
@ -527,6 +527,7 @@ async def auth_status(
|
|||||||
role = u.role
|
role = u.role
|
||||||
|
|
||||||
# Check registration availability
|
# Check registration availability
|
||||||
|
config = None
|
||||||
registration_open = False
|
registration_open = False
|
||||||
if not setup_required:
|
if not setup_required:
|
||||||
config_result = await db.execute(
|
config_result = await db.execute(
|
||||||
|
|||||||
@ -347,8 +347,7 @@ async def passkey_login_complete(
|
|||||||
ip=get_client_ip(request),
|
ip=get_client_ip(request),
|
||||||
)
|
)
|
||||||
await db.commit()
|
await db.commit()
|
||||||
if remaining == 0:
|
# Generic message for all failures — don't leak lockout state (C-02/F-02)
|
||||||
raise HTTPException(status_code=401, detail="Account temporarily locked. Try again in 30 minutes.")
|
|
||||||
raise HTTPException(status_code=401, detail="Authentication failed")
|
raise HTTPException(status_code=401, detail="Authentication failed")
|
||||||
|
|
||||||
# Update sign count (log anomaly but don't fail — S-05)
|
# 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")
|
raise HTTPException(status_code=401, detail="Authentication failed")
|
||||||
db_sess.is_locked = False
|
db_sess.is_locked = False
|
||||||
db_sess.locked_at = None
|
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(
|
await log_audit_event(
|
||||||
db, action="passkey.unlock_success", actor_id=user.id,
|
db, action="passkey.unlock_success", actor_id=user.id,
|
||||||
ip=get_client_ip(request),
|
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)
|
_, token = await create_db_session(db, user, client_ip, user_agent)
|
||||||
set_session_cookie(response, token)
|
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}
|
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:
|
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)
|
# Passkey satisfies MFA — if mfa_enforce_pending, clear it (before commit)
|
||||||
if user.mfa_enforce_pending:
|
if user.mfa_enforce_pending:
|
||||||
user.mfa_enforce_pending = False
|
user.mfa_enforce_pending = False
|
||||||
|
|||||||
@ -15,4 +15,4 @@ python-dateutil==2.9.0
|
|||||||
itsdangerous==2.2.0
|
itsdangerous==2.2.0
|
||||||
httpx==0.27.2
|
httpx==0.27.2
|
||||||
apscheduler==3.10.4
|
apscheduler==3.10.4
|
||||||
webauthn>=2.1.0
|
webauthn>=2.1.0,<3
|
||||||
|
|||||||
@ -107,6 +107,13 @@ server {
|
|||||||
include /etc/nginx/proxy-params.conf;
|
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
|
# SEC-14: Rate-limit public registration endpoint
|
||||||
location /api/auth/register {
|
location /api/auth/register {
|
||||||
limit_req zone=register_limit burst=3 nodelay;
|
limit_req zone=register_limit burst=3 nodelay;
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user