Consolidate setup persistence into bootstrap metadata and runtime DB
This commit is contained in:
@@ -86,6 +86,7 @@ Reference: `Docs/Refactoring.md` for full analysis of each issue.
|
|||||||
- Issue: `setup_service.run_setup` writes the same master password and runtime settings to both the bootstrap configuration database and the runtime database, making setup logic brittle and difficult to reason about.
|
- Issue: `setup_service.run_setup` writes the same master password and runtime settings to both the bootstrap configuration database and the runtime database, making setup logic brittle and difficult to reason about.
|
||||||
- Propose: Consolidate setup persistence so initial configuration is stored in one clearly defined location, with bootstrap metadata in the startup DB and runtime configuration pulled from the single persisted source.
|
- Propose: Consolidate setup persistence so initial configuration is stored in one clearly defined location, with bootstrap metadata in the startup DB and runtime configuration pulled from the single persisted source.
|
||||||
- Test: Add tests that confirm setup writes each value exactly once and that startup uses a single persisted store to resolve runtime settings, removing duplicate write paths.
|
- Test: Add tests that confirm setup writes each value exactly once and that startup uses a single persisted store to resolve runtime settings, removing duplicate write paths.
|
||||||
|
- Status: completed
|
||||||
|
|
||||||
12. Replace custom fail2ban socket protocol duplication with a dedicated adapter over the vendored client
|
12. Replace custom fail2ban socket protocol duplication with a dedicated adapter over the vendored client
|
||||||
- Goal: Reduce maintenance risk by aligning fail2ban integration with the bundled `fail2ban-master` codebase and isolating protocol implementation behind a swapable adapter.
|
- Goal: Reduce maintenance risk by aligning fail2ban integration with the bundled `fail2ban-master` codebase and isolating protocol implementation behind a swapable adapter.
|
||||||
|
|||||||
@@ -82,22 +82,11 @@ async def run_setup(
|
|||||||
# Run in a thread executor so the blocking bcrypt operation does not stall
|
# Run in a thread executor so the blocking bcrypt operation does not stall
|
||||||
# the asyncio event loop.
|
# the asyncio event loop.
|
||||||
password_bytes = master_password.encode()
|
password_bytes = master_password.encode()
|
||||||
loop = asyncio.get_running_loop()
|
|
||||||
hashed: str = await run_blocking(
|
hashed: str = await run_blocking(
|
||||||
lambda: bcrypt.hashpw(password_bytes, bcrypt.gensalt()).decode()
|
lambda: bcrypt.hashpw(password_bytes, bcrypt.gensalt()).decode()
|
||||||
)
|
)
|
||||||
|
|
||||||
await settings_repo.set_setting(db, _KEY_PASSWORD_HASH, hashed)
|
|
||||||
await settings_repo.set_setting(db, _KEY_DATABASE_PATH, database_path)
|
await settings_repo.set_setting(db, _KEY_DATABASE_PATH, database_path)
|
||||||
await settings_repo.set_setting(db, _KEY_FAIL2BAN_SOCKET, fail2ban_socket)
|
|
||||||
await settings_repo.set_setting(db, _KEY_TIMEZONE, timezone)
|
|
||||||
await settings_repo.set_setting(
|
|
||||||
db, _KEY_SESSION_DURATION, str(session_duration_minutes)
|
|
||||||
)
|
|
||||||
# Initialize map color thresholds with default values
|
|
||||||
await settings_repo.set_setting(db, _KEY_MAP_COLOR_THRESHOLD_HIGH, "100")
|
|
||||||
await settings_repo.set_setting(db, _KEY_MAP_COLOR_THRESHOLD_MEDIUM, "50")
|
|
||||||
await settings_repo.set_setting(db, _KEY_MAP_COLOR_THRESHOLD_LOW, "20")
|
|
||||||
|
|
||||||
runtime_initialized = await _ensure_database_initialized(database_path)
|
runtime_initialized = await _ensure_database_initialized(database_path)
|
||||||
|
|
||||||
@@ -138,6 +127,11 @@ async def get_password_hash(db: aiosqlite.Connection) -> str | None:
|
|||||||
return await util_get_password_hash(db)
|
return await util_get_password_hash(db)
|
||||||
|
|
||||||
|
|
||||||
|
async def get_runtime_database_path(db: aiosqlite.Connection) -> str | None:
|
||||||
|
"""Return the runtime database path persisted during initial setup."""
|
||||||
|
return await settings_repo.get_setting(db, _KEY_DATABASE_PATH)
|
||||||
|
|
||||||
|
|
||||||
async def get_persisted_runtime_settings(db: aiosqlite.Connection) -> dict[str, str | int]:
|
async def get_persisted_runtime_settings(db: aiosqlite.Connection) -> dict[str, str | int]:
|
||||||
"""Return runtime configuration values persisted during initial setup."""
|
"""Return runtime configuration values persisted during initial setup."""
|
||||||
runtime_settings: dict[str, str | int] = {}
|
runtime_settings: dict[str, str | int] = {}
|
||||||
|
|||||||
@@ -82,19 +82,27 @@ async def startup_shared_resources(
|
|||||||
log.debug("setup_completion_cached", completed=setup_complete)
|
log.debug("setup_completion_cached", completed=setup_complete)
|
||||||
|
|
||||||
if setup_complete:
|
if setup_complete:
|
||||||
persisted_runtime_settings = (
|
runtime_database_path = await setup_service.get_runtime_database_path(startup_db)
|
||||||
await setup_service.get_persisted_runtime_settings(startup_db)
|
if runtime_database_path:
|
||||||
)
|
if Path(runtime_database_path).resolve() != original_db_path:
|
||||||
if persisted_runtime_settings:
|
await _ensure_database_schema(runtime_database_path)
|
||||||
updated_settings = settings.model_copy(update=persisted_runtime_settings)
|
|
||||||
if Path(updated_settings.database_path).resolve() != original_db_path:
|
runtime_db = await open_db(runtime_database_path)
|
||||||
await _ensure_database_schema(updated_settings.database_path)
|
try:
|
||||||
set_runtime_settings(app, updated_settings)
|
persisted_runtime_settings = (
|
||||||
settings = updated_settings
|
await setup_service.get_persisted_runtime_settings(runtime_db)
|
||||||
log.info(
|
)
|
||||||
"runtime_settings_overridden_from_setup",
|
finally:
|
||||||
overrides=persisted_runtime_settings,
|
await runtime_db.close()
|
||||||
)
|
|
||||||
|
if persisted_runtime_settings:
|
||||||
|
updated_settings = settings.model_copy(update=persisted_runtime_settings)
|
||||||
|
set_runtime_settings(app, updated_settings)
|
||||||
|
settings = updated_settings
|
||||||
|
log.info(
|
||||||
|
"runtime_settings_overridden_from_setup",
|
||||||
|
overrides=persisted_runtime_settings,
|
||||||
|
)
|
||||||
|
|
||||||
if Path(settings.database_path).resolve() != original_db_path:
|
if Path(settings.database_path).resolve() != original_db_path:
|
||||||
runtime_db = await open_db(settings.database_path)
|
runtime_db = await open_db(settings.database_path)
|
||||||
|
|||||||
@@ -334,6 +334,7 @@ async def test_startup_loads_geo_cache_from_persisted_runtime_database(tmp_path:
|
|||||||
patch("app.services.geo_service.load_cache_from_db", new=load_cache),
|
patch("app.services.geo_service.load_cache_from_db", new=load_cache),
|
||||||
patch("app.services.geo_service.count_unresolved", new=AsyncMock(return_value=0)),
|
patch("app.services.geo_service.count_unresolved", new=AsyncMock(return_value=0)),
|
||||||
patch("app.services.setup_service.is_setup_complete", new=AsyncMock(return_value=True)),
|
patch("app.services.setup_service.is_setup_complete", new=AsyncMock(return_value=True)),
|
||||||
|
patch("app.services.setup_service.get_runtime_database_path", new=AsyncMock(return_value=runtime_db_path)),
|
||||||
patch(
|
patch(
|
||||||
"app.services.setup_service.get_persisted_runtime_settings",
|
"app.services.setup_service.get_persisted_runtime_settings",
|
||||||
new=AsyncMock(
|
new=AsyncMock(
|
||||||
|
|||||||
@@ -23,7 +23,7 @@ async def db(tmp_path: Path) -> aiosqlite.Connection: # type: ignore[misc]
|
|||||||
await setup_service.run_setup(
|
await setup_service.run_setup(
|
||||||
conn,
|
conn,
|
||||||
master_password="correctpassword1",
|
master_password="correctpassword1",
|
||||||
database_path="bangui.db",
|
database_path=str(tmp_path / "auth.db"),
|
||||||
fail2ban_socket="/var/run/fail2ban/fail2ban.sock",
|
fail2ban_socket="/var/run/fail2ban/fail2ban.sock",
|
||||||
timezone="UTC",
|
timezone="UTC",
|
||||||
session_duration_minutes=60,
|
session_duration_minutes=60,
|
||||||
|
|||||||
@@ -32,13 +32,13 @@ class TestIsSetupComplete:
|
|||||||
assert await setup_service.is_setup_complete(db) is False
|
assert await setup_service.is_setup_complete(db) is False
|
||||||
|
|
||||||
async def test_returns_true_after_run_setup(
|
async def test_returns_true_after_run_setup(
|
||||||
self, db: aiosqlite.Connection
|
self, db: aiosqlite.Connection, tmp_path: Path
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Setup is marked complete after run_setup() succeeds."""
|
"""Setup is marked complete after run_setup() succeeds."""
|
||||||
await setup_service.run_setup(
|
await setup_service.run_setup(
|
||||||
db,
|
db,
|
||||||
master_password="mypassword1",
|
master_password="mypassword1",
|
||||||
database_path="bangui.db",
|
database_path=str(tmp_path / "test.db"),
|
||||||
fail2ban_socket="/var/run/fail2ban/fail2ban.sock",
|
fail2ban_socket="/var/run/fail2ban/fail2ban.sock",
|
||||||
timezone="UTC",
|
timezone="UTC",
|
||||||
session_duration_minutes=60,
|
session_duration_minutes=60,
|
||||||
@@ -47,24 +47,59 @@ class TestIsSetupComplete:
|
|||||||
|
|
||||||
|
|
||||||
class TestRunSetup:
|
class TestRunSetup:
|
||||||
async def test_persists_all_settings(self, db: aiosqlite.Connection) -> None:
|
async def test_persists_all_settings(
|
||||||
"""run_setup() stores every provided setting."""
|
self, db: aiosqlite.Connection, tmp_path: Path
|
||||||
|
) -> None:
|
||||||
|
"""run_setup() stores every provided setting when runtime DB equals the bootstrap DB."""
|
||||||
await setup_service.run_setup(
|
await setup_service.run_setup(
|
||||||
db,
|
db,
|
||||||
master_password="mypassword1",
|
master_password="mypassword1",
|
||||||
database_path="/data/bangui.db",
|
database_path=str(tmp_path / "test.db"),
|
||||||
fail2ban_socket="/tmp/f2b.sock",
|
fail2ban_socket="/tmp/f2b.sock",
|
||||||
timezone="Europe/Berlin",
|
timezone="Europe/Berlin",
|
||||||
session_duration_minutes=120,
|
session_duration_minutes=120,
|
||||||
)
|
)
|
||||||
all_settings = await settings_repo.get_all_settings(db)
|
all_settings = await settings_repo.get_all_settings(db)
|
||||||
assert all_settings["database_path"] == "/data/bangui.db"
|
assert all_settings["database_path"] == str(tmp_path / "test.db")
|
||||||
assert all_settings["fail2ban_socket"] == "/tmp/f2b.sock"
|
assert all_settings["fail2ban_socket"] == "/tmp/f2b.sock"
|
||||||
assert all_settings["timezone"] == "Europe/Berlin"
|
assert all_settings["timezone"] == "Europe/Berlin"
|
||||||
assert all_settings["session_duration_minutes"] == "120"
|
assert all_settings["session_duration_minutes"] == "120"
|
||||||
|
|
||||||
|
async def test_runs_setup_into_separate_runtime_database(
|
||||||
|
self, db: aiosqlite.Connection, tmp_path: Path
|
||||||
|
) -> None:
|
||||||
|
"""run_setup() stores runtime configuration in the runtime DB only."""
|
||||||
|
runtime_db_path = str(tmp_path / "runtime.db")
|
||||||
|
await setup_service.run_setup(
|
||||||
|
db,
|
||||||
|
master_password="mypassword1",
|
||||||
|
database_path=runtime_db_path,
|
||||||
|
fail2ban_socket="/tmp/f2b.sock",
|
||||||
|
timezone="Europe/Berlin",
|
||||||
|
session_duration_minutes=120,
|
||||||
|
)
|
||||||
|
|
||||||
|
bootstrap_settings = await settings_repo.get_all_settings(db)
|
||||||
|
assert bootstrap_settings["database_path"] == runtime_db_path
|
||||||
|
assert bootstrap_settings["setup_completed"] == "1"
|
||||||
|
assert "fail2ban_socket" not in bootstrap_settings
|
||||||
|
assert "timezone" not in bootstrap_settings
|
||||||
|
assert "session_duration_minutes" not in bootstrap_settings
|
||||||
|
|
||||||
|
runtime_db = await aiosqlite.connect(runtime_db_path)
|
||||||
|
runtime_db.row_factory = aiosqlite.Row
|
||||||
|
try:
|
||||||
|
runtime_settings = await settings_repo.get_all_settings(runtime_db)
|
||||||
|
finally:
|
||||||
|
await runtime_db.close()
|
||||||
|
|
||||||
|
assert runtime_settings["fail2ban_socket"] == "/tmp/f2b.sock"
|
||||||
|
assert runtime_settings["timezone"] == "Europe/Berlin"
|
||||||
|
assert runtime_settings["session_duration_minutes"] == "120"
|
||||||
|
assert runtime_settings["master_password_hash"] != "mypassword1"
|
||||||
|
|
||||||
async def test_password_stored_as_bcrypt_hash(
|
async def test_password_stored_as_bcrypt_hash(
|
||||||
self, db: aiosqlite.Connection
|
self, db: aiosqlite.Connection, tmp_path: Path
|
||||||
) -> None:
|
) -> None:
|
||||||
"""The master password is stored as a bcrypt hash, not plain text."""
|
"""The master password is stored as a bcrypt hash, not plain text."""
|
||||||
import bcrypt
|
import bcrypt
|
||||||
@@ -72,7 +107,7 @@ class TestRunSetup:
|
|||||||
await setup_service.run_setup(
|
await setup_service.run_setup(
|
||||||
db,
|
db,
|
||||||
master_password="mypassword1",
|
master_password="mypassword1",
|
||||||
database_path="bangui.db",
|
database_path=str(tmp_path / "test.db"),
|
||||||
fail2ban_socket="/var/run/fail2ban/fail2ban.sock",
|
fail2ban_socket="/var/run/fail2ban/fail2ban.sock",
|
||||||
timezone="UTC",
|
timezone="UTC",
|
||||||
session_duration_minutes=60,
|
session_duration_minutes=60,
|
||||||
@@ -84,12 +119,12 @@ class TestRunSetup:
|
|||||||
assert bcrypt.checkpw(b"mypassword1", stored.encode())
|
assert bcrypt.checkpw(b"mypassword1", stored.encode())
|
||||||
|
|
||||||
async def test_raises_if_setup_already_complete(
|
async def test_raises_if_setup_already_complete(
|
||||||
self, db: aiosqlite.Connection
|
self, db: aiosqlite.Connection, tmp_path: Path
|
||||||
) -> None:
|
) -> None:
|
||||||
"""run_setup() raises RuntimeError if called a second time."""
|
"""run_setup() raises RuntimeError if called a second time."""
|
||||||
kwargs = {
|
kwargs = {
|
||||||
"master_password": "mypassword1",
|
"master_password": "mypassword1",
|
||||||
"database_path": "bangui.db",
|
"database_path": str(tmp_path / "test.db"),
|
||||||
"fail2ban_socket": "/var/run/fail2ban/fail2ban.sock",
|
"fail2ban_socket": "/var/run/fail2ban/fail2ban.sock",
|
||||||
"timezone": "UTC",
|
"timezone": "UTC",
|
||||||
"session_duration_minutes": 60,
|
"session_duration_minutes": 60,
|
||||||
@@ -99,13 +134,13 @@ class TestRunSetup:
|
|||||||
await setup_service.run_setup(db, **kwargs) # type: ignore[arg-type]
|
await setup_service.run_setup(db, **kwargs) # type: ignore[arg-type]
|
||||||
|
|
||||||
async def test_initializes_map_color_thresholds_with_defaults(
|
async def test_initializes_map_color_thresholds_with_defaults(
|
||||||
self, db: aiosqlite.Connection
|
self, db: aiosqlite.Connection, tmp_path: Path
|
||||||
) -> None:
|
) -> None:
|
||||||
"""run_setup() initializes map color thresholds with default values."""
|
"""run_setup() initializes map color thresholds with default values."""
|
||||||
await setup_service.run_setup(
|
await setup_service.run_setup(
|
||||||
db,
|
db,
|
||||||
master_password="mypassword1",
|
master_password="mypassword1",
|
||||||
database_path="bangui.db",
|
database_path=str(tmp_path / "test.db"),
|
||||||
fail2ban_socket="/var/run/fail2ban/fail2ban.sock",
|
fail2ban_socket="/var/run/fail2ban/fail2ban.sock",
|
||||||
timezone="UTC",
|
timezone="UTC",
|
||||||
session_duration_minutes=60,
|
session_duration_minutes=60,
|
||||||
@@ -122,13 +157,13 @@ class TestGetTimezone:
|
|||||||
assert await setup_service.get_timezone(db) == "UTC"
|
assert await setup_service.get_timezone(db) == "UTC"
|
||||||
|
|
||||||
async def test_returns_configured_timezone(
|
async def test_returns_configured_timezone(
|
||||||
self, db: aiosqlite.Connection
|
self, db: aiosqlite.Connection, tmp_path: Path
|
||||||
) -> None:
|
) -> None:
|
||||||
"""get_timezone() returns the value set during setup."""
|
"""get_timezone() returns the value set during setup."""
|
||||||
await setup_service.run_setup(
|
await setup_service.run_setup(
|
||||||
db,
|
db,
|
||||||
master_password="mypassword1",
|
master_password="mypassword1",
|
||||||
database_path="bangui.db",
|
database_path=str(tmp_path / "test.db"),
|
||||||
fail2ban_socket="/var/run/fail2ban/fail2ban.sock",
|
fail2ban_socket="/var/run/fail2ban/fail2ban.sock",
|
||||||
timezone="America/New_York",
|
timezone="America/New_York",
|
||||||
session_duration_minutes=60,
|
session_duration_minutes=60,
|
||||||
@@ -187,13 +222,13 @@ class TestMapColorThresholds:
|
|||||||
)
|
)
|
||||||
|
|
||||||
async def test_run_setup_initializes_default_thresholds(
|
async def test_run_setup_initializes_default_thresholds(
|
||||||
self, db: aiosqlite.Connection
|
self, db: aiosqlite.Connection, tmp_path: Path
|
||||||
) -> None:
|
) -> None:
|
||||||
"""run_setup() initializes map color thresholds with defaults."""
|
"""run_setup() initializes map color thresholds with defaults."""
|
||||||
await setup_service.run_setup(
|
await setup_service.run_setup(
|
||||||
db,
|
db,
|
||||||
master_password="mypassword1",
|
master_password="mypassword1",
|
||||||
database_path="bangui.db",
|
database_path=str(tmp_path / "test.db"),
|
||||||
fail2ban_socket="/var/run/fail2ban/fail2ban.sock",
|
fail2ban_socket="/var/run/fail2ban/fail2ban.sock",
|
||||||
timezone="UTC",
|
timezone="UTC",
|
||||||
session_duration_minutes=60,
|
session_duration_minutes=60,
|
||||||
@@ -212,7 +247,7 @@ class TestRunSetupAsync:
|
|||||||
assert inspect.iscoroutinefunction(setup_service.run_setup)
|
assert inspect.iscoroutinefunction(setup_service.run_setup)
|
||||||
|
|
||||||
async def test_password_hash_does_not_block_event_loop(
|
async def test_password_hash_does_not_block_event_loop(
|
||||||
self, db: aiosqlite.Connection
|
self, db: aiosqlite.Connection, tmp_path: Path
|
||||||
) -> None:
|
) -> None:
|
||||||
"""run_setup completes without blocking; other coroutines can interleave."""
|
"""run_setup completes without blocking; other coroutines can interleave."""
|
||||||
|
|
||||||
@@ -224,7 +259,7 @@ class TestRunSetupAsync:
|
|||||||
setup_coro = setup_service.run_setup(
|
setup_coro = setup_service.run_setup(
|
||||||
db,
|
db,
|
||||||
master_password="mypassword1",
|
master_password="mypassword1",
|
||||||
database_path="bangui.db",
|
database_path=str(tmp_path / "test.db"),
|
||||||
fail2ban_socket="/var/run/fail2ban/fail2ban.sock",
|
fail2ban_socket="/var/run/fail2ban/fail2ban.sock",
|
||||||
timezone="UTC",
|
timezone="UTC",
|
||||||
session_duration_minutes=60,
|
session_duration_minutes=60,
|
||||||
|
|||||||
Reference in New Issue
Block a user