From 86517860ffc8ab453985422214d41b944b591d51 Mon Sep 17 00:00:00 2001 From: Codex Date: Tue, 4 Aug 2026 18:14:45 +0200 Subject: [PATCH] fix(auth): allow setup after authentication is declined --- AppImage/scripts/auth_manager.py | 15 +-- .../scripts/tests/test_auth_manager_setup.py | 113 ++++++++++++++++++ 2 files changed, 121 insertions(+), 7 deletions(-) create mode 100644 AppImage/scripts/tests/test_auth_manager_setup.py diff --git a/AppImage/scripts/auth_manager.py b/AppImage/scripts/auth_manager.py index 52e9ba2f..a7b7f0a9 100644 --- a/AppImage/scripts/auth_manager.py +++ b/AppImage/scripts/auth_manager.py @@ -653,12 +653,13 @@ def setup_auth(username, password): Set up authentication with username and password Returns (success: bool, message: str) """ - # Refuse if auth has already been configured. Without this guard an + # Refuse if real credentials already exist. Without this guard an # unauthenticated POST to /api/auth/setup would let an attacker overwrite - # the existing admin credentials and take over the account. See audit - # Tier 1 #4. + # the existing admin credentials and take over the account. A declined + # setup is marked configured but deliberately has no credentials, so it + # must remain possible to finish setup later. See audit Tier 1 #4. existing = load_auth_config() - if existing.get("configured", False): + if existing.get("username") and existing.get("password_hash"): return False, "Authentication is already configured" if not username or not password: @@ -668,7 +669,7 @@ def setup_auth(username, password): if pw_err: return False, pw_err - config = { + existing.update({ "enabled": True, "username": username, "password_hash": hash_password(password), @@ -677,9 +678,9 @@ def setup_auth(username, password): "totp_enabled": False, "totp_secret": None, "backup_codes": [] - } + }) - if save_auth_config(config): + if save_auth_config(existing): return True, "Authentication configured successfully" else: return False, "Failed to save authentication configuration" diff --git a/AppImage/scripts/tests/test_auth_manager_setup.py b/AppImage/scripts/tests/test_auth_manager_setup.py new file mode 100644 index 00000000..8e45a22d --- /dev/null +++ b/AppImage/scripts/tests/test_auth_manager_setup.py @@ -0,0 +1,113 @@ +import importlib.util +import json +import tempfile +import unittest +from pathlib import Path +from unittest import mock + + +MODULE_PATH = Path(__file__).resolve().parents[1] / "auth_manager.py" +SPEC = importlib.util.spec_from_file_location("auth_manager_under_test", MODULE_PATH) +auth_manager = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(auth_manager) + + +class SetupAuthTests(unittest.TestCase): + def setUp(self): + self.temp_dir = tempfile.TemporaryDirectory() + self.addCleanup(self.temp_dir.cleanup) + config_dir = Path(self.temp_dir.name) + self.config_patch = mock.patch.multiple( + auth_manager, + CONFIG_DIR=config_dir, + AUTH_CONFIG_FILE=config_dir / "auth.json", + ) + self.config_patch.start() + self.addCleanup(self.config_patch.stop) + self.hash_patch = mock.patch.object( + auth_manager, "hash_password", return_value="test-password-hash" + ) + self.hash_patch.start() + self.addCleanup(self.hash_patch.stop) + + def read_config(self): + return json.loads(auth_manager.AUTH_CONFIG_FILE.read_text()) + + def write_config(self, config): + auth_manager.AUTH_CONFIG_FILE.write_text(json.dumps(config)) + + def test_fresh_setup_succeeds(self): + success, message = auth_manager.setup_auth("admin", "StrongPass1!") + + self.assertTrue(success, message) + config = self.read_config() + self.assertTrue(config["enabled"]) + self.assertTrue(config["configured"]) + self.assertFalse(config["declined"]) + self.assertEqual(config["username"], "admin") + self.assertEqual(config["password_hash"], "test-password-hash") + + def test_setup_succeeds_after_decline(self): + success, message = auth_manager.decline_auth() + self.assertTrue(success, message) + + success, message = auth_manager.setup_auth("admin", "StrongPass1!") + + self.assertTrue(success, message) + config = self.read_config() + self.assertTrue(config["enabled"]) + self.assertFalse(config["declined"]) + self.assertEqual(config["username"], "admin") + self.assertEqual(config["password_hash"], "test-password-hash") + + def test_existing_credentials_cannot_be_overwritten(self): + self.write_config({ + "enabled": True, + "configured": True, + "declined": False, + "username": "existing-admin", + "password_hash": "existing-password-hash", + }) + + success, message = auth_manager.setup_auth("attacker", "StrongPass1!") + + self.assertFalse(success) + self.assertEqual(message, "Authentication is already configured") + config = self.read_config() + self.assertEqual(config["username"], "existing-admin") + self.assertEqual(config["password_hash"], "existing-password-hash") + + def test_weak_password_is_rejected_without_writing_config(self): + success, message = auth_manager.setup_auth("admin", "weak") + + self.assertFalse(success) + self.assertEqual(message, "Password must be at least 10 characters") + self.assertFalse(auth_manager.AUTH_CONFIG_FILE.exists()) + + def test_unrelated_fields_are_preserved(self): + preserved = { + "jwt_secret": "s" * 48, + "api_tokens": [{"id": "token-1"}], + "revoked_tokens": ["revoked-token-hash"], + "display_name": "Server Owner", + "custom_future_field": {"keep": True}, + } + self.write_config({ + "enabled": False, + "configured": True, + "declined": True, + "username": None, + "password_hash": None, + **preserved, + }) + + success, message = auth_manager.setup_auth("admin", "StrongPass1!") + + self.assertTrue(success, message) + config = self.read_config() + for key, value in preserved.items(): + self.assertEqual(config[key], value) + + +if __name__ == "__main__": + unittest.main()