fix: 2FA bypassed when enabled but TOTP secret is missing (fail-open) (#1286)
* fix: fail closed when 2FA is enabled but the TOTP secret is missing * test: totp_verify fails closed when secret missing, passes when 2FA off
This commit is contained in:
@@ -376,7 +376,10 @@ class AuthManager:
|
||||
return True # 2FA not enabled, always pass
|
||||
secret = user.get("totp_secret")
|
||||
if not secret:
|
||||
return True
|
||||
# 2FA is enabled but no secret is stored (corrupt/partially-written
|
||||
# auth.json). Fail closed — returning True here bypassed the second
|
||||
# factor entirely.
|
||||
return False
|
||||
# Check backup codes first
|
||||
backup = user.get("totp_backup_codes", [])
|
||||
if code in backup:
|
||||
|
||||
21
tests/test_totp_failclosed.py
Normal file
21
tests/test_totp_failclosed.py
Normal file
@@ -0,0 +1,21 @@
|
||||
"""Regression: 2FA must fail closed when enabled but the secret is missing."""
|
||||
import json
|
||||
|
||||
from core.auth import AuthManager
|
||||
|
||||
|
||||
def test_totp_fails_closed_when_enabled_but_secret_missing(tmp_path):
|
||||
auth_path = tmp_path / "auth.json"
|
||||
auth_path.write_text(json.dumps({"users": {
|
||||
"alice": {"password_hash": "x", "totp_enabled": True}, # no totp_secret
|
||||
}}))
|
||||
mgr = AuthManager(str(auth_path))
|
||||
# Previously returned True, bypassing the second factor entirely.
|
||||
assert mgr.totp_verify("alice", "123456") is False
|
||||
|
||||
|
||||
def test_totp_passes_when_2fa_disabled(tmp_path):
|
||||
auth_path = tmp_path / "auth.json"
|
||||
auth_path.write_text(json.dumps({"users": {"bob": {"password_hash": "x"}}}))
|
||||
mgr = AuthManager(str(auth_path))
|
||||
assert mgr.totp_verify("bob", "000000") is True
|
||||
Reference in New Issue
Block a user