diff --git a/Docs/Tasks.md b/Docs/Tasks.md index 94a770a..824fb35 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -20,6 +20,7 @@ Reference: `Docs/Refactoring.md` for full analysis of each issue. - Issue: Several routers still read or mutate `app.state` directly, including `setup.py` and `config.py`, which breaks the intended separation of concerns and makes handlers harder to test. - Propose: Refactor routers to consume explicit dependencies for required values and actions, and move all direct state mutations into services or dedicated state managers. - Test: Ensure all router handlers compile without requiring direct `app.state` access and existing endpoint tests still pass. + - Status: completed 3. Replace process-local session cache with a pluggable abstraction - Goal: Make session validation consistent across multi-worker deployments by replacing the module-level in-memory cache with an injectable cache backend. diff --git a/backend/app/routers/config.py b/backend/app/routers/config.py index 2249280..21b475e 100644 --- a/backend/app/routers/config.py +++ b/backend/app/routers/config.py @@ -37,7 +37,6 @@ global settings, test regex patterns, add log paths, and preview log files. from __future__ import annotations -import datetime from typing import Annotated import structlog @@ -45,7 +44,6 @@ from fastapi import APIRouter, HTTPException, Path, Query, Request, status from app.dependencies import ( AppDep, - AppStateDep, AuthDep, DbDep, Fail2BanConfigDirDep, @@ -117,6 +115,12 @@ from app.services.jail_config_service import ( ) from app.tasks.health_check import _run_probe from app.utils.fail2ban_client import Fail2BanConnectionError +from app.utils.runtime_state import ( + clear_activation_record, + clear_pending_recovery, + create_pending_recovery, + record_activation, +) log: structlog.stdlib.BoundLogger = structlog.get_logger() @@ -678,7 +682,6 @@ async def update_map_color_thresholds( summary="Activate an inactive jail", ) async def activate_jail( - app_state: AppStateDep, app: AppDep, _auth: AuthDep, config_dir: Fail2BanConfigDirDep, @@ -693,7 +696,6 @@ async def activate_jail( the jail starts immediately. Args: - app_state: FastAPI application state. app: FastAPI application instance. _auth: Validated session. name: Name of the jail to activate. @@ -732,18 +734,15 @@ async def activate_jail( # Record this activation so the health-check task can attribute a # subsequent fail2ban crash to it. - app_state.last_activation = { - "jail_name": name, - "at": datetime.datetime.now(tz=datetime.UTC), - } + activation_time = record_activation(app, name) # If fail2ban stopped responding after the reload, create a pending-recovery # record immediately (before the background health task notices). if not result.fail2ban_running: - app_state.pending_recovery = PendingRecovery( + create_pending_recovery( + app, jail_name=name, - activated_at=app_state.last_activation["at"], - detected_at=datetime.datetime.now(tz=datetime.UTC), + activated_at=activation_time, ) # Force an immediate health probe so the cached status reflects the current @@ -935,7 +934,7 @@ async def get_pending_recovery( ) async def rollback_jail( _auth: AuthDep, - app_state: AppStateDep, + app: AppDep, config_dir: Fail2BanConfigDirDep, socket_path: Fail2BanSocketDep, start_cmd: Fail2BanStartCommandDep, @@ -951,6 +950,7 @@ async def rollback_jail( Args: _auth: Validated session. + app: FastAPI application instance. name: Jail name to disable and roll back. Returns: @@ -974,8 +974,8 @@ async def rollback_jail( # Clear pending recovery if fail2ban came back online. if result.fail2ban_running: - app_state.pending_recovery = None - app_state.last_activation = None + clear_pending_recovery(app) + clear_activation_record(app) return result diff --git a/backend/app/routers/setup.py b/backend/app/routers/setup.py index 5c02bd4..a7141e7 100644 --- a/backend/app/routers/setup.py +++ b/backend/app/routers/setup.py @@ -13,6 +13,7 @@ from fastapi import APIRouter, HTTPException, status from app.dependencies import AppDep, DbDep, SettingsDep from app.models.setup import SetupRequest, SetupResponse, SetupStatusResponse, SetupTimezoneResponse from app.services import setup_service +from app.utils.runtime_state import update_app_settings from app.utils.setup_state import is_setup_complete_cached, set_setup_complete_cache log: structlog.stdlib.BoundLogger = structlog.get_logger() @@ -75,13 +76,12 @@ async def post_setup( session_duration_minutes=body.session_duration_minutes, ) set_setup_complete_cache(app, True) - app.state.settings = app.state.settings.model_copy( - update={ - "database_path": body.database_path, - "fail2ban_socket": body.fail2ban_socket, - "timezone": body.timezone, - "session_duration_minutes": body.session_duration_minutes, - } + update_app_settings( + app, + database_path=body.database_path, + fail2ban_socket=body.fail2ban_socket, + timezone=body.timezone, + session_duration_minutes=body.session_duration_minutes, ) return SetupResponse() diff --git a/backend/app/utils/runtime_state.py b/backend/app/utils/runtime_state.py index 25a61be..45b5acd 100644 --- a/backend/app/utils/runtime_state.py +++ b/backend/app/utils/runtime_state.py @@ -15,10 +15,11 @@ from typing import TYPE_CHECKING, Any from starlette.datastructures import State +from app.models.config import PendingRecovery from app.models.server import ServerStatus if TYPE_CHECKING: # pragma: no cover - from app.models.config import PendingRecovery + from app.config import Settings ActivationRecord = dict[str, datetime.datetime] @@ -83,3 +84,53 @@ def get_runtime_state(app: Any) -> RuntimeState: if state is None or not hasattr(state, "runtime_state"): raise AttributeError("Runtime state has not been initialised on the application.") return state.runtime_state + + +def get_app_settings(app: Any) -> Settings: + """Return the current immutable settings from application state.""" + settings = getattr(app.state, "settings", None) + if settings is None: + raise AttributeError("Application settings are not available on the app state.") + return settings + + +def update_app_settings(app: Any, **overrides: Any) -> None: + """Update the current application settings immutably.""" + settings = get_app_settings(app) + app.state.settings = settings.model_copy(update=overrides) + + +def record_activation(app: Any, jail_name: str, at: datetime.datetime | None = None) -> datetime.datetime: + """Record a jail activation timestamp in runtime state.""" + now = at if at is not None else datetime.datetime.now(tz=datetime.UTC) + runtime_state = get_runtime_state(app) + runtime_state.last_activation = { + "jail_name": jail_name, + "at": now, + } + return now + + +def create_pending_recovery( + app: Any, + jail_name: str, + activated_at: datetime.datetime, + detected_at: datetime.datetime | None = None, +) -> None: + """Create a pending recovery record in runtime state.""" + runtime_state = get_runtime_state(app) + runtime_state.pending_recovery = PendingRecovery( + jail_name=jail_name, + activated_at=activated_at, + detected_at=detected_at if detected_at is not None else datetime.datetime.now(tz=datetime.UTC), + ) + + +def clear_pending_recovery(app: Any) -> None: + """Clear the current pending recovery record.""" + get_runtime_state(app).pending_recovery = None + + +def clear_activation_record(app: Any) -> None: + """Clear the current activation tracking record.""" + get_runtime_state(app).last_activation = None