From 926a4c59cb2b5e59ef5f18fc609247cad6c9ba9c Mon Sep 17 00:00:00 2001 From: Afonso Coutinho Date: Tue, 2 Jun 2026 17:26:47 +0100 Subject: [PATCH] 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 --- core/auth.py | 5 ++++- tests/test_totp_failclosed.py | 21 +++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 tests/test_totp_failclosed.py diff --git a/core/auth.py b/core/auth.py index 0ed140f..54635d8 100644 --- a/core/auth.py +++ b/core/auth.py @@ -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: diff --git a/tests/test_totp_failclosed.py b/tests/test_totp_failclosed.py new file mode 100644 index 0000000..b55c54d --- /dev/null +++ b/tests/test_totp_failclosed.py @@ -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