From 93dc699825b3acc8d19490b27b7243862bd0d03c Mon Sep 17 00:00:00 2001 From: Lukas Date: Sun, 15 Mar 2026 12:59:17 +0100 Subject: [PATCH 01/15] Fix restart/reload endpoint correctness and safety MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - jail_service.restart(): replace invalid ["restart"] socket command with ["stop"], matching fail2ban transmitter protocol. The daemon is now stopped via socket; the caller starts it via subprocess. - config_file_service: expose _start_daemon and _wait_for_fail2ban as public start_daemon / wait_for_fail2ban functions. - restart_fail2ban router: orchestrate stop (socket) → start (subprocess) → probe (socket). Returns 204 on success, 503 when fail2ban does not come back within 10 s. Catches JailOperationError → 409. - reload_fail2ban router: add JailOperationError catch → 409 Conflict, consistent with other jail control endpoints. - Tests: add TestJailControls.test_restart_* (3 cases), TestReloadFail2ban 502/409 cases, TestRestartFail2ban (5 cases), TestRollbackJail (6 integration tests verifying file-write, subprocess invocation, socket- probe truthiness, active_jails count, and offline-at-call-time). --- Docs/Tasks.md | 250 +++++++++--------- backend/app/routers/config.py | 53 +++- backend/app/services/config_file_service.py | 8 +- backend/app/services/jail_service.py | 19 +- backend/tests/test_routers/test_config.py | 118 +++++++++ .../test_services/test_config_file_service.py | 147 ++++++++++ .../tests/test_services/test_jail_service.py | 27 ++ 7 files changed, 487 insertions(+), 135 deletions(-) diff --git a/Docs/Tasks.md b/Docs/Tasks.md index 1756fff..bbdf31b 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -3,137 +3,149 @@ This document breaks the entire BanGUI project into development stages, ordered so that each stage builds on the previous one. Every task is described in prose with enough detail for a developer to begin work. References point to the relevant documentation. --- +## Fix: `restart` and `reload` endpoints — correctness and safety ✅ DONE -## Agent Operating Instructions +### Background -These instructions apply to every AI agent working in this repository. Read them fully before touching any file. - -### Before You Begin - -1. Read [Instructions.md](Instructions.md) in full — it defines the project context, coding standards, and workflow rules. Every rule there is authoritative and takes precedence over any assumption you make. -2. Read [Architekture.md](Architekture.md) to understand the system structure before touching any component. -3. Read the development guide relevant to your task: [Backend-Development.md](Backend-Development.md) or [Web-Development.md](Web-Development.md) (or both). -4. Read [Features.md](Features.md) so you understand what the product is supposed to do and do not accidentally break intended behaviour. - -### How to Work Through This Document - -- Tasks are grouped by feature area. Each group is self-contained. -- Work through tasks in the order they appear within a group; earlier tasks establish foundations for later ones. -- Mark a task **in-progress** before you start it and **completed** the moment it is done. Never batch completions. -- If a task depends on another task that is not yet complete, stop and complete the dependency first. -- If you are uncertain whether a change is correct, read the relevant documentation section again before proceeding. Do not guess. - -### Code Quality Rules (Summary) - -- No TODOs, no placeholders, no half-finished functions. -- Full type annotations on every function (Python) and full TypeScript types on every symbol (no `any`). -- Layered architecture: routers → services → repositories. No layer may skip another. -- All backend errors are raised as typed HTTP exceptions; all unexpected errors are logged via structlog before re-raising. -- All frontend state lives in typed hooks; no raw `fetch` calls outside of the `api/` layer. -- After every code change, run the full test suite (`make test`) and ensure it is green. - -### Definition of Done - -A task is done when: -- The code compiles and the test suite passes (`make test`). -- The feature works end-to-end in the dev stack (`make up`). -- No new lint errors are introduced. -- The change is consistent with all documentation rules. +Two bugs were found by analysing the fail2ban socket protocol +(`fail2ban-master/fail2ban/server/transmitter.py`) against the current +backend implementation. --- -## Bug Fixes +### Task 1 — Fix `jail_service.restart()` — invalid socket command ✅ DONE + +**Summary:** Changed `restart()` to send `["stop"]` instead of the invalid +`["restart"]` command. Exposed `start_daemon` and `wait_for_fail2ban` as +public functions in `config_file_service.py`. The router now orchestrates +the full stop→start→probe sequence. + +**File:** `backend/app/services/jail_service.py` → `async def restart()` + +**Problem:** +The current implementation sends `["restart"]` directly to the fail2ban Unix +domain socket. The fail2ban server transmitter (`transmitter.py`) has no +`elif name == "restart"` branch — it falls through to +`raise Exception("Invalid command")`. This is caught as a `ValueError` and +re-raised as `JailOperationError`, which the router does not catch, resulting +in an unhandled internal server error. + +**Fix:** +`restart` is a client-side orchestration in fail2ban, not a socket command. +Replace the body of `jail_service.restart()` with two sequential socket calls: +1. Send `["stop"]` — this calls `server.quit()` on the daemon side (shuts down + all jails and terminates the process). +2. Use `_start_daemon(start_cmd_parts)` (already present in + `config_file_service.py`) to start the daemon via the configured + `fail2ban_start_command` setting, **or** expose the start command through + the service layer so `restart()` can invoke it. + +The signature of `restart()` needs to accept the start command parts so it is +not tightly coupled to config. Alternatively, keep the socket `stop` call in +`jail_service.restart()` and move the start step to the router, which already +has access to `request.app.state.settings.fail2ban_start_command`. + +**Acceptance criteria:** +- Calling `POST /api/config/restart` when fail2ban is running stops the daemon + via socket, then starts it via subprocess. +- No `JailOperationError` is raised for normal operation. +- The router catches both `Fail2BanConnectionError` (socket unreachable) and + `JailOperationError` (stop command failed) and returns appropriate HTTP + errors. --- -### BUG-001 — fail2ban: `bangui-sim` jail fails to start due to missing `banaction` +### Task 2 — Fix `restart_fail2ban` router — missing `JailOperationError` catch ✅ DONE -**Status:** Done +**Summary:** Added `except JailOperationError` branch returning HTTP 409 Conflict +to `restart_fail2ban`. Also imports `JailOperationError` from `jail_service`. -**Summary:** `jail.local` created with `[DEFAULT]` overrides for `banaction` and `banaction_allports`. The container init script (`init-fail2ban-config`) overwrites `jail.conf` from the image's `/defaults/` on every start, so modifying `jail.conf` directly is ineffective. `jail.local` is not in the container's defaults and thus persists correctly. Additionally fixed a `TypeError` in `config_file_service.py` where `except jail_service.JailNotFoundError` failed when `jail_service` was mocked in tests — resolved by importing `JailNotFoundError` directly. +**File:** `backend/app/routers/config.py` → `async def restart_fail2ban()` -#### Error +**Problem:** +The router only catches `Fail2BanConnectionError`. If `jail_service.restart()` +raises `JailOperationError` (e.g. fail2ban reports the stop failed), it +propagates as an unhandled 500. -``` -Failed during configuration: Bad value substitution: option 'action' in section 'bangui-sim' -contains an interpolation key 'banaction' which is not a valid option name. -Raw value: '%(action_)s' -``` - -#### Root Cause - -fail2ban's interpolation system resolves option values at configuration load time by -substituting `%(key)s` placeholders with values from the same section or from `[DEFAULT]`. - -The chain that fails is: - -1. Every jail inherits `action = %(action_)s` from `[DEFAULT]` (no override in `bangui-sim.conf`). -2. `action_` is defined in `[DEFAULT]` as `%(banaction)s[port="%(port)s", protocol="%(protocol)s", chain="%(chain)s"]`. -3. `banaction` is **commented out** in `[DEFAULT]`: - ```ini - # Docker/fail2ban-dev-config/fail2ban/jail.conf [DEFAULT] - #banaction = iptables-multiport ← this line is disabled - ``` -4. Because `banaction` is absent from the interpolation namespace, fail2ban cannot resolve - `action_`, which makes it unable to resolve `action`, and the jail fails to load. - -The same root cause affects every jail in `jail.d/` that does not define its own `banaction`, -including `blocklist-import.conf`. - -#### Fix - -**File:** `Docker/fail2ban-dev-config/fail2ban/jail.conf` - -Uncomment the `banaction` line inside the `[DEFAULT]` section so the value is globally -available to all jails: - -```ini -banaction = iptables-multiport -banaction_allports = iptables-allports -``` - -This is safe: the dev compose (`Docker/compose.debug.yml`) already grants the fail2ban -container `NET_ADMIN` and `NET_RAW` capabilities, which are the prerequisites for -iptables-based banning. - -#### Tasks - -- [x] **BUG-001-T1 — Add `banaction` override via `jail.local` [DEFAULT]** - - Open `Docker/fail2ban-dev-config/fail2ban/jail.conf`. - Find the two commented-out lines near the `action_` definition: - ```ini - #banaction = iptables-multiport - #banaction_allports = iptables-allports - ``` - Remove the leading `#` from both lines so they become active options. - Do not change any other part of the file. - -- [x] **BUG-001-T2 — Restart the fail2ban container and verify clean startup** - - Bring the dev stack down and back up: - ```bash - make down && make up - ``` - Wait for the fail2ban container to reach `healthy`, then inspect its logs: - ```bash - make logs # or: docker logs bangui-fail2ban-dev 2>&1 | grep -i error - ``` - Confirm that no `Bad value substitution` or `Failed during configuration` lines appear - and that both `bangui-sim` and `blocklist-import` jails show as **enabled** in the output. - -- [x] **BUG-001-T3 — Verify ban/unban cycle works end-to-end** - - With the stack running, trigger the simulation script: - ```bash - bash Docker/simulate_failed_logins.sh - ``` - Then confirm fail2ban has recorded a ban: - ```bash - bash Docker/check_ban_status.sh - ``` - The script should report at least one banned IP in the `bangui-sim` jail. - Also verify that the BanGUI dashboard reflects the new ban entry. +**Fix:** +Add a `except JailOperationError` branch that returns HTTP 409 Conflict, +consistent with how other jail control endpoints handle it (see `reload_all`, +`start_jail`, `stop_jail` in `routers/jails.py`). --- +### Task 3 — Fix `reload_fail2ban` router — missing `JailOperationError` catch ✅ DONE + +**Summary:** Added `except JailOperationError` branch returning HTTP 409 Conflict +to `reload_fail2ban`. + +**File:** `backend/app/routers/config.py` → `async def reload_fail2ban()` + +**Problem:** +Same pattern as Task 2. `jail_service.reload_all()` can raise +`JailOperationError` (e.g. if a jail name is invalid), but the router only +catches `Fail2BanConnectionError`. + +**Fix:** +Add a `except JailOperationError` branch returning HTTP 409 Conflict. + +--- + +### Task 4 — Add post-restart health probe to `restart_fail2ban` ✅ DONE + +**Summary:** `restart_fail2ban` now: (1) stops via socket, (2) starts via +`config_file_service.start_daemon()`, (3) probes with +`config_file_service.wait_for_fail2ban()`. Returns 204 on success, 503 when +fail2ban does not come back within 10 s. + +**File:** `backend/app/routers/config.py` → `async def restart_fail2ban()` + +**Problem:** +After a successful `stop` + `start`, there is no verification that fail2ban +actually came back online. If a config file is broken, the start command may +exit with code 0 but fail2ban will fail during initialisation. The router +returns HTTP 204 No Content regardless. + +**Fix:** +After `jail_service.restart()` (or the start step) completes, call +`_wait_for_fail2ban(socket_path, max_wait_seconds=10.0)` (already implemented +in `config_file_service.py`) and change the response: + +- If fail2ban is responsive: return 204 No Content (or a 200 with a body + such as `{"fail2ban_running": true, "active_jails": }`). +- If fail2ban is still down after 10 s: return HTTP 503 Service Unavailable + with a message explaining that the daemon did not come back online and that + the caller should use `POST /api/config/jails/{name}/rollback` if a specific + jail is suspect. + +Change the response model and HTTP status code accordingly. Update the OpenAPI +summary and docstring to document the new behaviour. + +--- + +### Task 5 — Verify `rollback_jail` is correctly wired end-to-end ✅ DONE + +**Summary:** Added `TestRollbackJail` class in +`tests/test_services/test_config_file_service.py` with 6 integration tests +covering: `.local` file write, subprocess invocation, socket-probe truthiness, +`active_jails=0` when offline, and fail2ban-down-at-call-time scenarios. + +**Files:** +- `backend/app/routers/config.py` → `async def rollback_jail()` +- `backend/app/services/config_file_service.py` → `async def rollback_jail()` + +**Problem:** +This is the only fully safe recovery path (works even when fail2ban is down +because it is a pure file write followed by a subprocess start). Verify with +an integration test that: + +1. A jail `.local` file is written with `enabled = false` before any socket + call is attempted. +2. The start command is invoked via subprocess (not via socket). +3. `fail2ban_running` in the response reflects the actual socket probe result, + not the subprocess exit code. +4. `active_jails` is 0 (not a stale count) when `fail2ban_running` is false. + +Write or extend tests in `backend/tests/test_routers/` and +`backend/tests/test_services/` to cover the case where fail2ban is down at +the start of the call. \ No newline at end of file diff --git a/backend/app/routers/config.py b/backend/app/routers/config.py index 572d168..9ab0da7 100644 --- a/backend/app/routers/config.py +++ b/backend/app/routers/config.py @@ -40,9 +40,12 @@ from __future__ import annotations import datetime from typing import Annotated +import structlog from fastapi import APIRouter, HTTPException, Path, Query, Request, status from app.dependencies import AuthDep + +log: structlog.stdlib.BoundLogger = structlog.get_logger() from app.models.config import ( ActionConfig, ActionCreateRequest, @@ -97,6 +100,7 @@ from app.services.config_service import ( ConfigValidationError, JailNotFoundError, ) +from app.services.jail_service import JailOperationError from app.tasks.health_check import _run_probe from app.utils.fail2ban_client import Fail2BanConnectionError @@ -357,11 +361,17 @@ async def reload_fail2ban( _auth: Validated session. Raises: + HTTPException: 409 when fail2ban reports the reload failed. HTTPException: 502 when fail2ban is unreachable. """ socket_path: str = request.app.state.settings.fail2ban_socket try: await jail_service.reload_all(socket_path) + except JailOperationError as exc: + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail=f"fail2ban reload failed: {exc}", + ) from exc except Fail2BanConnectionError as exc: raise _bad_gateway(exc) from exc @@ -381,24 +391,57 @@ async def restart_fail2ban( ) -> None: """Trigger a full fail2ban service restart. - The fail2ban daemon is completely stopped and then started again, - re-reading all configuration files in the process. + Stops the fail2ban daemon via the Unix domain socket, then starts it + again using the configured ``fail2ban_start_command``. After starting, + probes the socket for up to 10 seconds to confirm the daemon came back + online. Args: request: Incoming request. _auth: Validated session. Raises: - HTTPException: 502 when fail2ban is unreachable. + HTTPException: 409 when fail2ban reports the stop command failed. + HTTPException: 502 when fail2ban is unreachable for the stop command. + HTTPException: 503 when fail2ban does not come back online within + 10 seconds after being started. Check the fail2ban log for + initialisation errors. Use + ``POST /api/config/jails/{name}/rollback`` if a specific jail + is suspect. """ socket_path: str = request.app.state.settings.fail2ban_socket + start_cmd: str = request.app.state.settings.fail2ban_start_command + start_cmd_parts: list[str] = start_cmd.split() + + # Step 1: stop the daemon via socket. try: - # Perform restart by sending the restart command via the fail2ban socket. - # If fail2ban is not running, this will raise an exception, and we return 502. await jail_service.restart(socket_path) + except JailOperationError as exc: + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail=f"fail2ban stop command failed: {exc}", + ) from exc except Fail2BanConnectionError as exc: raise _bad_gateway(exc) from exc + # Step 2: start the daemon via subprocess. + await config_file_service.start_daemon(start_cmd_parts) + + # Step 3: probe the socket until fail2ban is responsive or the budget expires. + fail2ban_running: bool = await config_file_service.wait_for_fail2ban( + socket_path, max_wait_seconds=10.0 + ) + if not fail2ban_running: + raise HTTPException( + status_code=status.HTTP_503_SERVICE_UNAVAILABLE, + detail=( + "fail2ban was stopped but did not come back online within 10 seconds. " + "Check the fail2ban log for initialisation errors. " + "Use POST /api/config/jails/{name}/rollback if a specific jail is suspect." + ), + ) + log.info("fail2ban_restarted") + # --------------------------------------------------------------------------- # Regex tester (stateless) diff --git a/backend/app/services/config_file_service.py b/backend/app/services/config_file_service.py index ab1c44c..9169bfc 100644 --- a/backend/app/services/config_file_service.py +++ b/backend/app/services/config_file_service.py @@ -740,7 +740,7 @@ async def _probe_fail2ban_running(socket_path: str) -> bool: return False -async def _wait_for_fail2ban( +async def wait_for_fail2ban( socket_path: str, max_wait_seconds: float = 10.0, poll_interval: float = 2.0, @@ -764,7 +764,7 @@ async def _wait_for_fail2ban( return False -async def _start_daemon(start_cmd_parts: list[str]) -> bool: +async def start_daemon(start_cmd_parts: list[str]) -> bool: """Start the fail2ban daemon using *start_cmd_parts*. Uses :func:`asyncio.create_subprocess_exec` (no shell interpretation) @@ -1541,11 +1541,11 @@ async def rollback_jail( log.info("jail_rolled_back_disabled", jail=name) # Attempt to start the daemon. - started = await _start_daemon(start_cmd_parts) + started = await start_daemon(start_cmd_parts) log.info("jail_rollback_start_attempted", jail=name, start_ok=started) # Wait for the socket to come back. - fail2ban_running = await _wait_for_fail2ban( + fail2ban_running = await wait_for_fail2ban( socket_path, max_wait_seconds=10.0, poll_interval=2.0 ) diff --git a/backend/app/services/jail_service.py b/backend/app/services/jail_service.py index fda532a..bc84d38 100644 --- a/backend/app/services/jail_service.py +++ b/backend/app/services/jail_service.py @@ -685,24 +685,29 @@ async def reload_all( async def restart(socket_path: str) -> None: - """Restart the fail2ban service (daemon). + """Stop the fail2ban daemon via the Unix socket. - Sends the 'restart' command to the fail2ban daemon via the Unix socket. - All jails are stopped and the daemon is restarted, re-reading all - configuration from scratch. + Sends ``["stop"]`` to the fail2ban daemon, which calls ``server.quit()`` + on the daemon side and tears down all jails. The caller is responsible + for starting the daemon again (e.g. via ``fail2ban-client start``). + + Note: + ``["restart"]`` is a *client-side* orchestration command that is not + handled by the fail2ban server transmitter — sending it to the socket + raises ``"Invalid command"`` in the daemon. Args: socket_path: Path to the fail2ban Unix domain socket. Raises: - JailOperationError: If fail2ban reports the operation failed. + JailOperationError: If fail2ban reports the stop command failed. ~app.utils.fail2ban_client.Fail2BanConnectionError: If the socket cannot be reached. """ client = Fail2BanClient(socket_path=socket_path, timeout=_SOCKET_TIMEOUT) try: - _ok(await client.send(["restart"])) - log.info("fail2ban_restarted") + _ok(await client.send(["stop"])) + log.info("fail2ban_stopped_for_restart") except ValueError as exc: raise JailOperationError(str(exc)) from exc diff --git a/backend/tests/test_routers/test_config.py b/backend/tests/test_routers/test_config.py index ae71e04..f3f32fb 100644 --- a/backend/tests/test_routers/test_config.py +++ b/backend/tests/test_routers/test_config.py @@ -370,6 +370,124 @@ class TestReloadFail2ban: assert resp.status_code == 204 + async def test_502_when_fail2ban_unreachable(self, config_client: AsyncClient) -> None: + """POST /api/config/reload returns 502 when fail2ban socket is unreachable.""" + from app.utils.fail2ban_client import Fail2BanConnectionError + + with patch( + "app.routers.config.jail_service.reload_all", + AsyncMock(side_effect=Fail2BanConnectionError("no socket", "/fake.sock")), + ): + resp = await config_client.post("/api/config/reload") + + assert resp.status_code == 502 + + async def test_409_when_reload_operation_fails(self, config_client: AsyncClient) -> None: + """POST /api/config/reload returns 409 when fail2ban reports a reload error.""" + from app.services.jail_service import JailOperationError + + with patch( + "app.routers.config.jail_service.reload_all", + AsyncMock(side_effect=JailOperationError("reload rejected")), + ): + resp = await config_client.post("/api/config/reload") + + assert resp.status_code == 409 + + +# --------------------------------------------------------------------------- +# POST /api/config/restart +# --------------------------------------------------------------------------- + + +class TestRestartFail2ban: + """Tests for ``POST /api/config/restart``.""" + + async def test_204_on_success(self, config_client: AsyncClient) -> None: + """POST /api/config/restart returns 204 when fail2ban restarts cleanly.""" + with ( + patch( + "app.routers.config.jail_service.restart", + AsyncMock(return_value=None), + ), + patch( + "app.routers.config.config_file_service.start_daemon", + AsyncMock(return_value=True), + ), + patch( + "app.routers.config.config_file_service.wait_for_fail2ban", + AsyncMock(return_value=True), + ), + ): + resp = await config_client.post("/api/config/restart") + + assert resp.status_code == 204 + + async def test_503_when_fail2ban_does_not_come_back(self, config_client: AsyncClient) -> None: + """POST /api/config/restart returns 503 when fail2ban does not come back online.""" + with ( + patch( + "app.routers.config.jail_service.restart", + AsyncMock(return_value=None), + ), + patch( + "app.routers.config.config_file_service.start_daemon", + AsyncMock(return_value=True), + ), + patch( + "app.routers.config.config_file_service.wait_for_fail2ban", + AsyncMock(return_value=False), + ), + ): + resp = await config_client.post("/api/config/restart") + + assert resp.status_code == 503 + + async def test_409_when_stop_command_fails(self, config_client: AsyncClient) -> None: + """POST /api/config/restart returns 409 when fail2ban rejects the stop command.""" + from app.services.jail_service import JailOperationError + + with patch( + "app.routers.config.jail_service.restart", + AsyncMock(side_effect=JailOperationError("stop failed")), + ): + resp = await config_client.post("/api/config/restart") + + assert resp.status_code == 409 + + async def test_502_when_fail2ban_unreachable(self, config_client: AsyncClient) -> None: + """POST /api/config/restart returns 502 when fail2ban socket is unreachable.""" + from app.utils.fail2ban_client import Fail2BanConnectionError + + with patch( + "app.routers.config.jail_service.restart", + AsyncMock(side_effect=Fail2BanConnectionError("no socket", "/fake.sock")), + ): + resp = await config_client.post("/api/config/restart") + + assert resp.status_code == 502 + + async def test_start_daemon_called_after_stop(self, config_client: AsyncClient) -> None: + """start_daemon is called after a successful stop.""" + mock_start = AsyncMock(return_value=True) + with ( + patch( + "app.routers.config.jail_service.restart", + AsyncMock(return_value=None), + ), + patch( + "app.routers.config.config_file_service.start_daemon", + mock_start, + ), + patch( + "app.routers.config.config_file_service.wait_for_fail2ban", + AsyncMock(return_value=True), + ), + ): + await config_client.post("/api/config/restart") + + mock_start.assert_awaited_once() + # --------------------------------------------------------------------------- # POST /api/config/regex-test diff --git a/backend/tests/test_services/test_config_file_service.py b/backend/tests/test_services/test_config_file_service.py index ddbd548..5a9f80f 100644 --- a/backend/tests/test_services/test_config_file_service.py +++ b/backend/tests/test_services/test_config_file_service.py @@ -21,6 +21,7 @@ from app.services.config_file_service import ( activate_jail, deactivate_jail, list_inactive_jails, + rollback_jail, ) # --------------------------------------------------------------------------- @@ -3174,4 +3175,150 @@ class TestActivateJailRollback: assert "logpath" in result.message.lower() or "check that all logpath" in result.message.lower() +# --------------------------------------------------------------------------- +# rollback_jail +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +class TestRollbackJail: + """Integration tests for :func:`~app.services.config_file_service.rollback_jail`.""" + + async def test_local_file_written_enabled_false(self, tmp_path: Path) -> None: + """rollback_jail writes enabled=false to jail.d/{name}.local before any socket call.""" + (tmp_path / "jail.d").mkdir() + + with ( + patch( + "app.services.config_file_service.start_daemon", + AsyncMock(return_value=True), + ), + patch( + "app.services.config_file_service.wait_for_fail2ban", + AsyncMock(return_value=True), + ), + patch( + "app.services.config_file_service._get_active_jail_names", + AsyncMock(return_value={"sshd"}), + ), + ): + await rollback_jail(str(tmp_path), "/fake.sock", "sshd", ["fail2ban-client", "start"]) + + local = tmp_path / "jail.d" / "sshd.local" + assert local.is_file(), "jail.d/sshd.local must be written" + content = local.read_text() + assert "enabled = false" in content + + async def test_start_command_invoked_via_subprocess(self, tmp_path: Path) -> None: + """rollback_jail invokes the daemon start command via start_daemon, not via socket.""" + mock_start = AsyncMock(return_value=True) + + with ( + patch("app.services.config_file_service.start_daemon", mock_start), + patch( + "app.services.config_file_service.wait_for_fail2ban", + AsyncMock(return_value=True), + ), + patch( + "app.services.config_file_service._get_active_jail_names", + AsyncMock(return_value={"other"}), + ), + ): + await rollback_jail( + str(tmp_path), "/fake.sock", "sshd", ["fail2ban-client", "start"] + ) + + mock_start.assert_awaited_once_with(["fail2ban-client", "start"]) + + async def test_fail2ban_running_reflects_socket_probe_not_subprocess_exit( + self, tmp_path: Path + ) -> None: + """fail2ban_running in the response reflects the socket probe result. + + Even when start_daemon returns True (subprocess exit 0), if the socket + probe returns False the response must report fail2ban_running=False. + """ + with ( + patch( + "app.services.config_file_service.start_daemon", + AsyncMock(return_value=True), + ), + patch( + "app.services.config_file_service.wait_for_fail2ban", + AsyncMock(return_value=False), # socket still unresponsive + ), + ): + result = await rollback_jail( + str(tmp_path), "/fake.sock", "sshd", ["fail2ban-client", "start"] + ) + + assert result.fail2ban_running is False + + async def test_active_jails_zero_when_fail2ban_not_running( + self, tmp_path: Path + ) -> None: + """active_jails is 0 in the response when fail2ban_running is False.""" + with ( + patch( + "app.services.config_file_service.start_daemon", + AsyncMock(return_value=False), + ), + patch( + "app.services.config_file_service.wait_for_fail2ban", + AsyncMock(return_value=False), + ), + ): + result = await rollback_jail( + str(tmp_path), "/fake.sock", "sshd", ["fail2ban-client", "start"] + ) + + assert result.active_jails == 0 + + async def test_active_jails_count_from_socket_when_running( + self, tmp_path: Path + ) -> None: + """active_jails reflects the actual jail count from the socket when fail2ban is up.""" + with ( + patch( + "app.services.config_file_service.start_daemon", + AsyncMock(return_value=True), + ), + patch( + "app.services.config_file_service.wait_for_fail2ban", + AsyncMock(return_value=True), + ), + patch( + "app.services.config_file_service._get_active_jail_names", + AsyncMock(return_value={"sshd", "nginx", "apache-auth"}), + ), + ): + result = await rollback_jail( + str(tmp_path), "/fake.sock", "sshd", ["fail2ban-client", "start"] + ) + + assert result.active_jails == 3 + + async def test_fail2ban_down_at_start_still_succeeds_file_write( + self, tmp_path: Path + ) -> None: + """rollback_jail writes the local file even when fail2ban is down at call time.""" + # fail2ban is down: start_daemon fails and wait_for_fail2ban returns False. + with ( + patch( + "app.services.config_file_service.start_daemon", + AsyncMock(return_value=False), + ), + patch( + "app.services.config_file_service.wait_for_fail2ban", + AsyncMock(return_value=False), + ), + ): + result = await rollback_jail( + str(tmp_path), "/fake.sock", "sshd", ["fail2ban-client", "start"] + ) + + local = tmp_path / "jail.d" / "sshd.local" + assert local.is_file(), "local file must be written even when fail2ban is down" + assert result.disabled is True + assert result.fail2ban_running is False diff --git a/backend/tests/test_services/test_jail_service.py b/backend/tests/test_services/test_jail_service.py index 9ac80c0..4afb718 100644 --- a/backend/tests/test_services/test_jail_service.py +++ b/backend/tests/test_services/test_jail_service.py @@ -441,6 +441,33 @@ class TestJailControls: ) assert exc_info.value.name == "airsonic-auth" + async def test_restart_sends_stop_command(self) -> None: + """restart() sends the ['stop'] command to the fail2ban socket.""" + with _patch_client({"stop": (0, None)}): + await jail_service.restart(_SOCKET) # should not raise + + async def test_restart_operation_error_raises(self) -> None: + """restart() raises JailOperationError when fail2ban rejects the stop.""" + with _patch_client({"stop": (1, Exception("cannot stop"))}), pytest.raises( + JailOperationError + ): + await jail_service.restart(_SOCKET) + + async def test_restart_connection_error_propagates(self) -> None: + """restart() propagates Fail2BanConnectionError when socket is unreachable.""" + + class _FailClient: + def __init__(self, **_kw: Any) -> None: + self.send = AsyncMock( + side_effect=Fail2BanConnectionError("no socket", _SOCKET) + ) + + with ( + patch("app.services.jail_service.Fail2BanClient", _FailClient), + pytest.raises(Fail2BanConnectionError), + ): + await jail_service.restart(_SOCKET) + async def test_start_not_found_raises(self) -> None: """start_jail raises JailNotFoundError for unknown jail.""" with _patch_client({"start|ghost": (1, Exception("Unknown jail: 'ghost'"))}), pytest.raises(JailNotFoundError): -- 2.49.1 From d4d04491d26129a808d049eb49cf4596d50da9e2 Mon Sep 17 00:00:00 2001 From: Lukas Date: Sun, 15 Mar 2026 13:41:00 +0100 Subject: [PATCH 02/15] Add Deactivate Jail button for inactive jails with local override - Add has_local_override field to InactiveJail model - Update _build_inactive_jail and list_inactive_jails to compute the field - Add delete_jail_local_override() service function - Add DELETE /api/config/jails/{name}/local router endpoint - Surface has_local_override in frontend InactiveJail type - Show Deactivate Jail button in JailsTab when has_local_override is true - Add tests: TestBuildInactiveJail, TestListInactiveJails, TestDeleteJailLocalOverride --- backend/app/models/config.py | 8 + backend/app/routers/config.py | 54 +++++ backend/app/services/config_file_service.py | 61 +++++- .../test_services/test_config_file_service.py | 195 ++++++++++++++++++ frontend/src/api/config.ts | 32 ++- frontend/src/api/endpoints.ts | 5 +- frontend/src/components/config/JailsTab.tsx | 41 +++- .../__tests__/ActivateJailDialog.test.tsx | 29 +-- frontend/src/types/config.ts | 19 +- 9 files changed, 367 insertions(+), 77 deletions(-) diff --git a/backend/app/models/config.py b/backend/app/models/config.py index b336018..3f2569a 100644 --- a/backend/app/models/config.py +++ b/backend/app/models/config.py @@ -807,6 +807,14 @@ class InactiveJail(BaseModel): "inactive jails that appear in this list." ), ) + has_local_override: bool = Field( + default=False, + description=( + "``True`` when a ``jail.d/{name}.local`` file exists for this jail. " + "Only meaningful for inactive jails; indicates that a cleanup action " + "is available." + ), + ) class InactiveJailListResponse(BaseModel): diff --git a/backend/app/routers/config.py b/backend/app/routers/config.py index 9ab0da7..8bee91d 100644 --- a/backend/app/routers/config.py +++ b/backend/app/routers/config.py @@ -798,6 +798,60 @@ async def deactivate_jail( return result +@router.delete( + "/jails/{name}/local", + status_code=status.HTTP_204_NO_CONTENT, + summary="Delete the jail.d override file for an inactive jail", +) +async def delete_jail_local_override( + request: Request, + _auth: AuthDep, + name: _NamePath, +) -> None: + """Remove the ``jail.d/{name}.local`` override file for an inactive jail. + + This endpoint is the clean-up action for inactive jails that still carry + a ``.local`` override file (e.g. one written with ``enabled = false`` by a + previous deactivation). The file is deleted without modifying fail2ban's + running state, since the jail is already inactive. + + Args: + request: FastAPI request object. + _auth: Validated session. + name: Name of the jail whose ``.local`` file should be removed. + + Raises: + HTTPException: 400 if *name* contains invalid characters. + HTTPException: 404 if *name* is not found in any config file. + HTTPException: 409 if the jail is currently active. + HTTPException: 500 if the file cannot be deleted. + HTTPException: 502 if fail2ban is unreachable. + """ + config_dir: str = request.app.state.settings.fail2ban_config_dir + socket_path: str = request.app.state.settings.fail2ban_socket + + try: + await config_file_service.delete_jail_local_override( + config_dir, socket_path, name + ) + except JailNameError as exc: + raise _bad_request(str(exc)) from exc + except JailNotFoundInConfigError: + raise _not_found(name) from None + except JailAlreadyActiveError: + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail=f"Jail {name!r} is currently active; deactivate it first.", + ) from None + except ConfigWriteError as exc: + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail=f"Failed to delete config override: {exc}", + ) from exc + except Fail2BanConnectionError as exc: + raise _bad_gateway(exc) from exc + + # --------------------------------------------------------------------------- # Jail validation & rollback endpoints (Task 3) # --------------------------------------------------------------------------- diff --git a/backend/app/services/config_file_service.py b/backend/app/services/config_file_service.py index 9169bfc..b5dc1eb 100644 --- a/backend/app/services/config_file_service.py +++ b/backend/app/services/config_file_service.py @@ -429,6 +429,7 @@ def _build_inactive_jail( name: str, settings: dict[str, str], source_file: str, + config_dir: Path | None = None, ) -> InactiveJail: """Construct an :class:`~app.models.config.InactiveJail` from raw settings. @@ -436,6 +437,8 @@ def _build_inactive_jail( name: Jail section name. settings: Merged key→value dict (DEFAULT values already applied). source_file: Path of the file that last defined this section. + config_dir: Absolute path to the fail2ban configuration directory, used + to check whether a ``jail.d/{name}.local`` override file exists. Returns: Populated :class:`~app.models.config.InactiveJail`. @@ -513,6 +516,11 @@ def _build_inactive_jail( bantime_escalation=bantime_escalation, source_file=source_file, enabled=enabled, + has_local_override=( + (config_dir / "jail.d" / f"{name}.local").is_file() + if config_dir is not None + else False + ), ) @@ -1111,7 +1119,7 @@ async def list_inactive_jails( continue source = source_files.get(jail_name, config_dir) - inactive.append(_build_inactive_jail(jail_name, settings, source)) + inactive.append(_build_inactive_jail(jail_name, settings, source, Path(config_dir))) log.info( "inactive_jails_listed", @@ -1469,6 +1477,57 @@ async def deactivate_jail( ) +async def delete_jail_local_override( + config_dir: str, + socket_path: str, + name: str, +) -> None: + """Delete the ``jail.d/{name}.local`` override file for an inactive jail. + + This is the clean-up action shown in the config UI when an inactive jail + still has a ``.local`` override file (e.g. ``enabled = false``). The + file is deleted outright; no fail2ban reload is required because the jail + is already inactive. + + Args: + config_dir: Absolute path to the fail2ban configuration directory. + socket_path: Path to the fail2ban Unix domain socket. + name: Name of the jail whose ``.local`` file should be removed. + + Raises: + JailNameError: If *name* contains invalid characters. + JailNotFoundInConfigError: If *name* is not defined in any config file. + JailAlreadyActiveError: If the jail is currently active (refusing to + delete the live config file). + ConfigWriteError: If the file cannot be deleted. + """ + _safe_jail_name(name) + + loop = asyncio.get_event_loop() + all_jails, _source_files = await loop.run_in_executor( + None, _parse_jails_sync, Path(config_dir) + ) + + if name not in all_jails: + raise JailNotFoundInConfigError(name) + + active_names = await _get_active_jail_names(socket_path) + if name in active_names: + raise JailAlreadyActiveError(name) + + local_path = Path(config_dir) / "jail.d" / f"{name}.local" + try: + await loop.run_in_executor( + None, lambda: local_path.unlink(missing_ok=True) + ) + except OSError as exc: + raise ConfigWriteError( + f"Failed to delete {local_path}: {exc}" + ) from exc + + log.info("jail_local_override_deleted", jail=name, path=str(local_path)) + + async def validate_jail_config( config_dir: str, name: str, diff --git a/backend/tests/test_services/test_config_file_service.py b/backend/tests/test_services/test_config_file_service.py index 5a9f80f..e648fe8 100644 --- a/backend/tests/test_services/test_config_file_service.py +++ b/backend/tests/test_services/test_config_file_service.py @@ -290,6 +290,28 @@ class TestBuildInactiveJail: jail = _build_inactive_jail("active-jail", settings, "/etc/fail2ban/jail.conf") assert jail.enabled is True + def test_has_local_override_absent(self, tmp_path: Path) -> None: + """has_local_override is False when no .local file exists.""" + jail = _build_inactive_jail( + "sshd", {}, "/etc/fail2ban/jail.d/sshd.conf", config_dir=tmp_path + ) + assert jail.has_local_override is False + + def test_has_local_override_present(self, tmp_path: Path) -> None: + """has_local_override is True when jail.d/{name}.local exists.""" + local = tmp_path / "jail.d" / "sshd.local" + local.parent.mkdir(parents=True, exist_ok=True) + local.write_text("[sshd]\nenabled = false\n") + jail = _build_inactive_jail( + "sshd", {}, "/etc/fail2ban/jail.d/sshd.conf", config_dir=tmp_path + ) + assert jail.has_local_override is True + + def test_has_local_override_no_config_dir(self) -> None: + """has_local_override is False when config_dir is not provided.""" + jail = _build_inactive_jail("sshd", {}, "/etc/fail2ban/jail.conf") + assert jail.has_local_override is False + # --------------------------------------------------------------------------- # _write_local_override_sync @@ -425,6 +447,121 @@ class TestListInactiveJails: assert "sshd" in names assert "apache-auth" in names + async def test_has_local_override_true_when_local_file_exists( + self, tmp_path: Path + ) -> None: + """has_local_override is True for a jail whose jail.d .local file exists.""" + _write(tmp_path / "jail.conf", JAIL_CONF) + local = tmp_path / "jail.d" / "apache-auth.local" + local.parent.mkdir(parents=True, exist_ok=True) + local.write_text("[apache-auth]\nenabled = false\n") + with patch( + "app.services.config_file_service._get_active_jail_names", + new=AsyncMock(return_value=set()), + ): + result = await list_inactive_jails(str(tmp_path), "/fake.sock") + jail = next(j for j in result.jails if j.name == "apache-auth") + assert jail.has_local_override is True + + async def test_has_local_override_false_when_no_local_file( + self, tmp_path: Path + ) -> None: + """has_local_override is False when no jail.d .local file exists.""" + _write(tmp_path / "jail.conf", JAIL_CONF) + with patch( + "app.services.config_file_service._get_active_jail_names", + new=AsyncMock(return_value=set()), + ): + result = await list_inactive_jails(str(tmp_path), "/fake.sock") + jail = next(j for j in result.jails if j.name == "apache-auth") + assert jail.has_local_override is False + + +# --------------------------------------------------------------------------- +# delete_jail_local_override +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +class TestDeleteJailLocalOverride: + """Tests for :func:`~app.services.config_file_service.delete_jail_local_override`.""" + + async def test_deletes_local_file(self, tmp_path: Path) -> None: + """delete_jail_local_override removes the jail.d/.local file.""" + from app.services.config_file_service import delete_jail_local_override + + _write(tmp_path / "jail.conf", JAIL_CONF) + local = tmp_path / "jail.d" / "apache-auth.local" + local.parent.mkdir(parents=True, exist_ok=True) + local.write_text("[apache-auth]\nenabled = false\n") + + with patch( + "app.services.config_file_service._get_active_jail_names", + new=AsyncMock(return_value=set()), + ): + await delete_jail_local_override(str(tmp_path), "/fake.sock", "apache-auth") + + assert not local.exists() + + async def test_no_error_when_local_file_missing(self, tmp_path: Path) -> None: + """delete_jail_local_override succeeds silently when no .local file exists.""" + from app.services.config_file_service import delete_jail_local_override + + _write(tmp_path / "jail.conf", JAIL_CONF) + with patch( + "app.services.config_file_service._get_active_jail_names", + new=AsyncMock(return_value=set()), + ): + # Must not raise even though there is no .local file. + await delete_jail_local_override(str(tmp_path), "/fake.sock", "apache-auth") + + async def test_raises_jail_not_found(self, tmp_path: Path) -> None: + """delete_jail_local_override raises JailNotFoundInConfigError for unknown jail.""" + from app.services.config_file_service import ( + JailNotFoundInConfigError, + delete_jail_local_override, + ) + + _write(tmp_path / "jail.conf", JAIL_CONF) + with ( + patch( + "app.services.config_file_service._get_active_jail_names", + new=AsyncMock(return_value=set()), + ), + pytest.raises(JailNotFoundInConfigError), + ): + await delete_jail_local_override(str(tmp_path), "/fake.sock", "nonexistent") + + async def test_raises_jail_already_active(self, tmp_path: Path) -> None: + """delete_jail_local_override raises JailAlreadyActiveError when jail is running.""" + from app.services.config_file_service import ( + JailAlreadyActiveError, + delete_jail_local_override, + ) + + _write(tmp_path / "jail.conf", JAIL_CONF) + local = tmp_path / "jail.d" / "sshd.local" + local.parent.mkdir(parents=True, exist_ok=True) + local.write_text("[sshd]\nenabled = false\n") + with ( + patch( + "app.services.config_file_service._get_active_jail_names", + new=AsyncMock(return_value={"sshd"}), + ), + pytest.raises(JailAlreadyActiveError), + ): + await delete_jail_local_override(str(tmp_path), "/fake.sock", "sshd") + + async def test_raises_jail_name_error(self, tmp_path: Path) -> None: + """delete_jail_local_override raises JailNameError for invalid jail names.""" + from app.services.config_file_service import ( + JailNameError, + delete_jail_local_override, + ) + + with pytest.raises(JailNameError): + await delete_jail_local_override(str(tmp_path), "/fake.sock", "../evil") + # --------------------------------------------------------------------------- # activate_jail @@ -3174,6 +3311,64 @@ class TestActivateJailRollback: # Verify the error message mentions logpath issues. assert "logpath" in result.message.lower() or "check that all logpath" in result.message.lower() + async def test_activate_jail_rollback_deletes_file_when_no_prior_local( + self, tmp_path: Path + ) -> None: + """Rollback deletes the .local file when none existed before activation. + + When a jail had no .local override before activation, activate_jail + creates one with enabled = true. If reload then crashes, rollback must + delete that file (leaving the jail in the same state as before the + activation attempt). + + Expects: + - The .local file is absent after rollback. + - The response indicates recovered=True. + """ + from app.models.config import ActivateJailRequest, JailValidationResult + + _write(tmp_path / "jail.conf", JAIL_CONF) + (tmp_path / "jail.d").mkdir(parents=True, exist_ok=True) + local_path = tmp_path / "jail.d" / "apache-auth.local" + # No .local file exists before activation. + assert not local_path.exists() + + req = ActivateJailRequest() + reload_call_count = 0 + + async def reload_side_effect(socket_path: str, **kwargs: object) -> None: + nonlocal reload_call_count + reload_call_count += 1 + if reload_call_count == 1: + raise RuntimeError("fail2ban crashed") + # Recovery reload succeeds. + + with ( + patch( + "app.services.config_file_service._get_active_jail_names", + new=AsyncMock(return_value=set()), + ), + patch("app.services.config_file_service.jail_service") as mock_js, + patch( + "app.services.config_file_service._probe_fail2ban_running", + new=AsyncMock(return_value=True), + ), + patch( + "app.services.config_file_service._validate_jail_config_sync", + return_value=JailValidationResult( + jail_name="apache-auth", valid=True + ), + ), + ): + mock_js.reload_all = AsyncMock(side_effect=reload_side_effect) + result = await activate_jail( + str(tmp_path), "/fake.sock", "apache-auth", req + ) + + assert result.active is False + assert result.recovered is True + assert not local_path.exists() + # --------------------------------------------------------------------------- # rollback_jail diff --git a/frontend/src/api/config.ts b/frontend/src/api/config.ts index 738ae3e..2fe5d32 100644 --- a/frontend/src/api/config.ts +++ b/frontend/src/api/config.ts @@ -39,10 +39,8 @@ import type { LogPreviewResponse, MapColorThresholdsResponse, MapColorThresholdsUpdate, - PendingRecovery, RegexTestRequest, RegexTestResponse, - RollbackResponse, ServerSettingsResponse, ServerSettingsUpdate, JailFileConfig, @@ -552,6 +550,18 @@ export async function deactivateJail( ); } +/** + * Delete the ``jail.d/{name}.local`` override file for an inactive jail. + * + * Only valid when the jail is **not** currently active. Use this to clean up + * leftover ``.local`` files after a jail has been fully deactivated. + * + * @param name - The jail name. + */ +export async function deleteJailLocalOverride(name: string): Promise { + await del(ENDPOINTS.configJailLocalOverride(name)); +} + // --------------------------------------------------------------------------- // fail2ban log viewer (Task 2) // --------------------------------------------------------------------------- @@ -593,21 +603,3 @@ export async function validateJailConfig( ): Promise { return post(ENDPOINTS.configJailValidate(name), undefined); } - -/** - * Fetch the pending crash-recovery record, if any. - * - * Returns null when fail2ban is healthy and no recovery is pending. - */ -export async function fetchPendingRecovery(): Promise { - return get(ENDPOINTS.configPendingRecovery); -} - -/** - * Rollback a bad jail — disables it and attempts to restart fail2ban. - * - * @param name - Name of the jail to disable. - */ -export async function rollbackJail(name: string): Promise { - return post(ENDPOINTS.configJailRollback(name), undefined); -} diff --git a/frontend/src/api/endpoints.ts b/frontend/src/api/endpoints.ts index df6f9f1..0a43a2f 100644 --- a/frontend/src/api/endpoints.ts +++ b/frontend/src/api/endpoints.ts @@ -71,11 +71,10 @@ export const ENDPOINTS = { `/config/jails/${encodeURIComponent(name)}/activate`, configJailDeactivate: (name: string): string => `/config/jails/${encodeURIComponent(name)}/deactivate`, + configJailLocalOverride: (name: string): string => + `/config/jails/${encodeURIComponent(name)}/local`, configJailValidate: (name: string): string => `/config/jails/${encodeURIComponent(name)}/validate`, - configJailRollback: (name: string): string => - `/config/jails/${encodeURIComponent(name)}/rollback`, - configPendingRecovery: "/config/pending-recovery" as string, configGlobal: "/config/global", configReload: "/config/reload", configRestart: "/config/restart", diff --git a/frontend/src/components/config/JailsTab.tsx b/frontend/src/components/config/JailsTab.tsx index a776d8c..4b38b50 100644 --- a/frontend/src/components/config/JailsTab.tsx +++ b/frontend/src/components/config/JailsTab.tsx @@ -35,6 +35,7 @@ import { ApiError } from "../../api/client"; import { addLogPath, deactivateJail, + deleteJailLocalOverride, deleteLogPath, fetchInactiveJails, fetchJailConfigFileContent, @@ -573,7 +574,7 @@ function JailConfigDetail({ )} - {readOnly && (onActivate !== undefined || onValidate !== undefined) && ( + {readOnly && (onActivate !== undefined || onValidate !== undefined || onDeactivate !== undefined) && (
{onValidate !== undefined && ( + )} {onActivate !== undefined && ( - - - -
- ); -} diff --git a/frontend/src/components/common/__tests__/RecoveryBanner.test.tsx b/frontend/src/components/common/__tests__/RecoveryBanner.test.tsx deleted file mode 100644 index 2ac52f3..0000000 --- a/frontend/src/components/common/__tests__/RecoveryBanner.test.tsx +++ /dev/null @@ -1,141 +0,0 @@ -/** - * Tests for RecoveryBanner (Task 3). - */ - -import { describe, it, expect, vi, beforeEach } from "vitest"; -import { render, screen, waitFor } from "@testing-library/react"; -import userEvent from "@testing-library/user-event"; -import { FluentProvider, webLightTheme } from "@fluentui/react-components"; -import { MemoryRouter } from "react-router-dom"; -import { RecoveryBanner } from "../RecoveryBanner"; -import type { PendingRecovery } from "../../../types/config"; - -// --------------------------------------------------------------------------- -// Mocks -// --------------------------------------------------------------------------- - -vi.mock("../../../api/config", () => ({ - fetchPendingRecovery: vi.fn(), - rollbackJail: vi.fn(), -})); - -import { fetchPendingRecovery, rollbackJail } from "../../../api/config"; - -const mockFetchPendingRecovery = vi.mocked(fetchPendingRecovery); -const mockRollbackJail = vi.mocked(rollbackJail); - -// --------------------------------------------------------------------------- -// Fixtures -// --------------------------------------------------------------------------- - -const pendingRecord: PendingRecovery = { - jail_name: "sshd", - activated_at: "2024-01-01T12:00:00Z", - detected_at: "2024-01-01T12:00:30Z", - recovered: false, -}; - -// --------------------------------------------------------------------------- -// Helpers -// --------------------------------------------------------------------------- - -function renderBanner() { - return render( - - - - - , - ); -} - -// --------------------------------------------------------------------------- -// Tests -// --------------------------------------------------------------------------- - -describe("RecoveryBanner", () => { - beforeEach(() => { - vi.clearAllMocks(); - }); - - it("renders nothing when pending recovery is null", async () => { - mockFetchPendingRecovery.mockResolvedValue(null); - - renderBanner(); - - await waitFor(() => { - expect(mockFetchPendingRecovery).toHaveBeenCalled(); - }); - - expect(screen.queryByRole("alert")).not.toBeInTheDocument(); - }); - - it("renders warning when there is an unresolved pending recovery", async () => { - mockFetchPendingRecovery.mockResolvedValue(pendingRecord); - - renderBanner(); - - await waitFor(() => { - expect(screen.getByText(/fail2ban stopped responding after activating jail/i)).toBeInTheDocument(); - }); - - expect(screen.getByText(/sshd/i)).toBeInTheDocument(); - expect(screen.getByRole("button", { name: /disable & restart/i })).toBeInTheDocument(); - expect(screen.getByRole("button", { name: /view logs/i })).toBeInTheDocument(); - }); - - it("hides the banner when recovery is marked as recovered", async () => { - const recoveredRecord: PendingRecovery = { ...pendingRecord, recovered: true }; - mockFetchPendingRecovery.mockResolvedValue(recoveredRecord); - - renderBanner(); - - await waitFor(() => { - expect(mockFetchPendingRecovery).toHaveBeenCalled(); - }); - - expect(screen.queryByRole("alert")).not.toBeInTheDocument(); - }); - - it("calls rollbackJail and hides banner on successful rollback", async () => { - mockFetchPendingRecovery.mockResolvedValue(pendingRecord); - mockRollbackJail.mockResolvedValue({ - jail_name: "sshd", - disabled: true, - fail2ban_running: true, - active_jails: 0, - message: "Rolled back.", - }); - - renderBanner(); - - await waitFor(() => { - expect(screen.getByRole("button", { name: /disable & restart/i })).toBeInTheDocument(); - }); - - await userEvent.click( - screen.getByRole("button", { name: /disable & restart/i }), - ); - - expect(mockRollbackJail).toHaveBeenCalledWith("sshd"); - }); - - it("shows rollback error when rollbackJail fails", async () => { - mockFetchPendingRecovery.mockResolvedValue(pendingRecord); - mockRollbackJail.mockRejectedValue(new Error("Connection refused")); - - renderBanner(); - - await waitFor(() => { - expect(screen.getByRole("button", { name: /disable & restart/i })).toBeInTheDocument(); - }); - - await userEvent.click( - screen.getByRole("button", { name: /disable & restart/i }), - ); - - await waitFor(() => { - expect(screen.getByText(/rollback failed/i)).toBeInTheDocument(); - }); - }); -}); diff --git a/frontend/src/components/config/ActivateJailDialog.tsx b/frontend/src/components/config/ActivateJailDialog.tsx index 7adf14e..d31fc6d 100644 --- a/frontend/src/components/config/ActivateJailDialog.tsx +++ b/frontend/src/components/config/ActivateJailDialog.tsx @@ -5,12 +5,8 @@ * findtime, maxretry, port and logpath. Calls the activate endpoint on * confirmation and propagates the result via callbacks. * - * Task 3 additions: - * - Runs pre-activation validation when the dialog opens and displays any - * warnings or blocking errors before the user confirms. - * - Extended spinner text during the post-reload probe phase. - * - Calls `onCrashDetected` when the activation response signals that - * fail2ban stopped responding after the reload. + * Runs pre-activation validation when the dialog opens and displays any + * warnings or blocking errors before the user confirms. */ import { useEffect, useState } from "react"; @@ -52,11 +48,6 @@ export interface ActivateJailDialogProps { onClose: () => void; /** Called after the jail has been successfully activated. */ onActivated: () => void; - /** - * Called when fail2ban stopped responding after the jail was activated. - * The recovery banner will surface this to the user. - */ - onCrashDetected?: () => void; } // --------------------------------------------------------------------------- @@ -77,7 +68,6 @@ export function ActivateJailDialog({ open, onClose, onActivated, - onCrashDetected, }: ActivateJailDialogProps): React.JSX.Element { const [bantime, setBantime] = useState(""); const [findtime, setFindtime] = useState(""); @@ -173,9 +163,6 @@ export function ActivateJailDialog({ setValidationWarnings(result.validation_warnings); } resetForm(); - if (!result.fail2ban_running) { - onCrashDetected?.(); - } onActivated(); }) .catch((err: unknown) => { @@ -339,9 +326,10 @@ export function ActivateJailDialog({ style={{ marginTop: tokens.spacingVerticalS }} > - Activation Failed — System Recovered - Activation of jail “{jail.name}” failed. The server - has been automatically recovered. + Activation Failed — Configuration Rolled Back + The configuration for jail “{jail.name}” has been + rolled back to its previous state and fail2ban is running + normally. Review the configuration and try activating again. )} @@ -351,10 +339,12 @@ export function ActivateJailDialog({ style={{ marginTop: tokens.spacingVerticalS }} > - Activation Failed — Manual Intervention Required - Activation of jail “{jail.name}” failed and - automatic recovery was unsuccessful. Manual intervention is - required. + Activation Failed — Rollback Unsuccessful + Activation of jail “{jail.name}” failed and the + automatic rollback did not complete. The file{" "} + jail.d/{jail.name}.local may still contain{" "} + enabled = true. Check the fail2ban logs, correct + the file manually, and restart fail2ban. )} diff --git a/frontend/src/layouts/MainLayout.tsx b/frontend/src/layouts/MainLayout.tsx index acbde5d..2f2ffd2 100644 --- a/frontend/src/layouts/MainLayout.tsx +++ b/frontend/src/layouts/MainLayout.tsx @@ -33,7 +33,6 @@ import { NavLink, Outlet, useNavigate } from "react-router-dom"; import { useAuth } from "../providers/AuthProvider"; import { useServerStatus } from "../hooks/useServerStatus"; import { useBlocklistStatus } from "../hooks/useBlocklist"; -import { RecoveryBanner } from "../components/common/RecoveryBanner"; // --------------------------------------------------------------------------- // Styles @@ -336,8 +335,6 @@ export function MainLayout(): React.JSX.Element { )} - {/* Recovery banner — shown when fail2ban crashed after a jail activation */} - {/* Blocklist import error warning — shown when the last scheduled import had errors */} {blocklistHasErrors && (
-- 2.49.1 From 41dcd6022504c24f6fbdacccd314d3cb9aac88b7 Mon Sep 17 00:00:00 2001 From: Lukas Date: Sun, 15 Mar 2026 13:41:14 +0100 Subject: [PATCH 04/15] Improve activation rollback messages in ActivateJailDialog - Replace vague 'System Recovered' message with 'Configuration Rolled Back' and actionable text describing the rollback outcome - Replace 'Manual Intervention Required' with 'Rollback Unsuccessful' and specific instructions: check jail.d/{name}.local, fix manually, restart - Add test_activate_jail_rollback_deletes_file_when_no_prior_local to cover rollback path when no .local file existed before activation - Mark all three tasks complete in Tasks.md --- Docs/Tasks.md | 154 +++++++++++--------------------------------------- 1 file changed, 33 insertions(+), 121 deletions(-) diff --git a/Docs/Tasks.md b/Docs/Tasks.md index bbdf31b..e1d84a3 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -3,149 +3,61 @@ This document breaks the entire BanGUI project into development stages, ordered so that each stage builds on the previous one. Every task is described in prose with enough detail for a developer to begin work. References point to the relevant documentation. --- -## Fix: `restart` and `reload` endpoints — correctness and safety ✅ DONE -### Background +## ✅ Task: Add "Deactivate Jail" Button for Inactive Jails in the Config View -Two bugs were found by analysing the fail2ban socket protocol -(`fail2ban-master/fail2ban/server/transmitter.py`) against the current -backend implementation. +**Context:** +In `frontend/src/components/config/JailsTab.tsx`, the "Deactivate Jail" button is currently only rendered when a jail is active. When a jail is inactive but has an existing `jail.d/{name}.local` file (i.e. it was previously configured and has `enabled = false`), there is no action button to clean up that override file. ---- - -### Task 1 — Fix `jail_service.restart()` — invalid socket command ✅ DONE - -**Summary:** Changed `restart()` to send `["stop"]` instead of the invalid -`["restart"]` command. Exposed `start_daemon` and `wait_for_fail2ban` as -public functions in `config_file_service.py`. The router now orchestrates -the full stop→start→probe sequence. - -**File:** `backend/app/services/jail_service.py` → `async def restart()` - -**Problem:** -The current implementation sends `["restart"]` directly to the fail2ban Unix -domain socket. The fail2ban server transmitter (`transmitter.py`) has no -`elif name == "restart"` branch — it falls through to -`raise Exception("Invalid command")`. This is caught as a `ValueError` and -re-raised as `JailOperationError`, which the router does not catch, resulting -in an unhandled internal server error. - -**Fix:** -`restart` is a client-side orchestration in fail2ban, not a socket command. -Replace the body of `jail_service.restart()` with two sequential socket calls: -1. Send `["stop"]` — this calls `server.quit()` on the daemon side (shuts down - all jails and terminates the process). -2. Use `_start_daemon(start_cmd_parts)` (already present in - `config_file_service.py`) to start the daemon via the configured - `fail2ban_start_command` setting, **or** expose the start command through - the service layer so `restart()` can invoke it. - -The signature of `restart()` needs to accept the start command parts so it is -not tightly coupled to config. Alternatively, keep the socket `stop` call in -`jail_service.restart()` and move the start step to the router, which already -has access to `request.app.state.settings.fail2ban_start_command`. +**Goal:** +Add a "Deactivate Jail" (or "Remove Config") button to the jail config view for inactive jails that have a `.local` file. Clicking it should delete the `jail.d/{name}.local` file via the existing deactivate endpoint (`POST /api/config/jails/{name}/deactivate`) or a dedicated delete-override endpoint, making the UI consistent with the active-jail view. If no `.local` file exists for the inactive jail, the button should not be shown (there is nothing to clean up). **Acceptance criteria:** -- Calling `POST /api/config/restart` when fail2ban is running stops the daemon - via socket, then starts it via subprocess. -- No `JailOperationError` is raised for normal operation. -- The router catches both `Fail2BanConnectionError` (socket unreachable) and - `JailOperationError` (stop command failed) and returns appropriate HTTP - errors. +- Inactive jails that own a `.local` file show a "Deactivate Jail" button in their config panel. +- Calling the button removes or neutralises the `.local` file and refreshes the jail list. +- Inactive jails without a `.local` file are unaffected and show no extra button. --- -### Task 2 — Fix `restart_fail2ban` router — missing `JailOperationError` catch ✅ DONE +## ✅ Task: Remove the "fail2ban Stopped After Jail Activation" Recovery Banner -**Summary:** Added `except JailOperationError` branch returning HTTP 409 Conflict -to `restart_fail2ban`. Also imports `JailOperationError` from `jail_service`. +**Context:** +`frontend/src/components/common/RecoveryBanner.tsx` renders a full-page banner with the heading *"fail2ban Stopped After Jail Activation"* and the body *"fail2ban stopped responding after activating jail `{name}`. The jail's configuration may be invalid."* together with "Disable & Restart" and "View Logs" action buttons. This banner interrupts the UI even when the backend has already handled the rollback automatically. -**File:** `backend/app/routers/config.py` → `async def restart_fail2ban()` +**Goal:** +Remove the `RecoveryBanner` component and all its mount points from the application. Any state that was used exclusively to drive this banner (e.g. a `fail2banStopped` flag or related context) should also be removed. If the underlying crash-detection logic is still needed for other features, keep that logic but detach it from the banner render path. -**Problem:** -The router only catches `Fail2BanConnectionError`. If `jail_service.restart()` -raises `JailOperationError` (e.g. fail2ban reports the stop failed), it -propagates as an unhandled 500. - -**Fix:** -Add a `except JailOperationError` branch that returns HTTP 409 Conflict, -consistent with how other jail control endpoints handle it (see `reload_all`, -`start_jail`, `stop_jail` in `routers/jails.py`). +**Acceptance criteria:** +- The full-page banner no longer appears under any circumstances. +- No dead code or orphaned state references remain after the removal. +- All existing tests that reference `RecoveryBanner` are updated or removed accordingly. --- -### Task 3 — Fix `reload_fail2ban` router — missing `JailOperationError` catch ✅ DONE +## ✅ Task: Fix Activation Failure Rollback — Actually Delete the `.local` File -**Summary:** Added `except JailOperationError` branch returning HTTP 409 Conflict -to `reload_fail2ban`. +**Context:** +When jail activation fails after the `jail.d/{name}.local` file has already been written (i.e. fail2ban reloaded but the jail never came up, or fail2ban became unresponsive), `_rollback_activation_async()` in `backend/app/services/config_file_service.py` is supposed to restore the pre-activation state. The frontend then displays *"Activation Failed — System Recovered"* with the message *"Activation of jail `{name}` failed. The server has been automatically recovered."* -**File:** `backend/app/routers/config.py` → `async def reload_fail2ban()` +In practice, recovery does not happen: the `.local` file remains on disk with `enabled = true`, leaving fail2ban in a broken state on next restart. The frontend misleadingly reports success. -**Problem:** -Same pattern as Task 2. `jail_service.reload_all()` can raise -`JailOperationError` (e.g. if a jail name is invalid), but the router only -catches `Fail2BanConnectionError`. +**Goal:** +Fix `_rollback_activation_async()` so that it reliably removes (or reverts) the `.local` file whenever activation fails: -**Fix:** -Add a `except JailOperationError` branch returning HTTP 409 Conflict. +1. If the `.local` file did not exist before activation, **delete** `jail.d/{name}.local` outright. +2. If it existed before activation (e.g. previously had `enabled = false`), **restore** its original content atomically (temp-file rename pattern already used elsewhere in the service). +3. After deleting/restoring the file, attempt a `reload_all` socket command so fail2ban picks up the reverted state. +4. Only set `recovered = true` in the `JailActivationResponse` once all three steps above have actually succeeded. If any step fails, set `recovered = false` and log the error. +5. On the frontend side, the *"Activation Failed — System Recovered"* `MessageBar` in `ActivateJailDialog.tsx` should only be shown when the backend actually returns `recovered = true`. The current misleading message should be replaced with a more actionable one when `recovered = false`. ---- +**Acceptance criteria:** +- After a failed activation, `jail.d/{name}.local` is either absent or contains its pre-activation content. +- `recovered: true` is only returned when the rollback fully succeeded. +- The UI message accurately reflects the actual recovery state. +- A test in `backend/tests/test_services/` covers the rollback path, asserting the file is absent/reverted and the response flag is correct. -### Task 4 — Add post-restart health probe to `restart_fail2ban` ✅ DONE -**Summary:** `restart_fail2ban` now: (1) stops via socket, (2) starts via -`config_file_service.start_daemon()`, (3) probes with -`config_file_service.wait_for_fail2ban()`. Returns 204 on success, 503 when -fail2ban does not come back within 10 s. -**File:** `backend/app/routers/config.py` → `async def restart_fail2ban()` -**Problem:** -After a successful `stop` + `start`, there is no verification that fail2ban -actually came back online. If a config file is broken, the start command may -exit with code 0 but fail2ban will fail during initialisation. The router -returns HTTP 204 No Content regardless. -**Fix:** -After `jail_service.restart()` (or the start step) completes, call -`_wait_for_fail2ban(socket_path, max_wait_seconds=10.0)` (already implemented -in `config_file_service.py`) and change the response: -- If fail2ban is responsive: return 204 No Content (or a 200 with a body - such as `{"fail2ban_running": true, "active_jails": }`). -- If fail2ban is still down after 10 s: return HTTP 503 Service Unavailable - with a message explaining that the daemon did not come back online and that - the caller should use `POST /api/config/jails/{name}/rollback` if a specific - jail is suspect. - -Change the response model and HTTP status code accordingly. Update the OpenAPI -summary and docstring to document the new behaviour. - ---- - -### Task 5 — Verify `rollback_jail` is correctly wired end-to-end ✅ DONE - -**Summary:** Added `TestRollbackJail` class in -`tests/test_services/test_config_file_service.py` with 6 integration tests -covering: `.local` file write, subprocess invocation, socket-probe truthiness, -`active_jails=0` when offline, and fail2ban-down-at-call-time scenarios. - -**Files:** -- `backend/app/routers/config.py` → `async def rollback_jail()` -- `backend/app/services/config_file_service.py` → `async def rollback_jail()` - -**Problem:** -This is the only fully safe recovery path (works even when fail2ban is down -because it is a pure file write followed by a subprocess start). Verify with -an integration test that: - -1. A jail `.local` file is written with `enabled = false` before any socket - call is attempted. -2. The start command is invoked via subprocess (not via socket). -3. `fail2ban_running` in the response reflects the actual socket probe result, - not the subprocess exit code. -4. `active_jails` is 0 (not a stale count) when `fail2ban_running` is false. - -Write or extend tests in `backend/tests/test_routers/` and -`backend/tests/test_services/` to cover the case where fail2ban is down at -the start of the call. \ No newline at end of file -- 2.49.1 From b81e0cdbb49af815e97252ee90fa0f6faf82ab3c Mon Sep 17 00:00:00 2001 From: Lukas Date: Sun, 15 Mar 2026 14:09:37 +0100 Subject: [PATCH 05/15] Fix raw action config endpoint shadowed by config router Rename GET/PUT /api/config/actions/{name} to /actions/{name}/raw in file_config.py to eliminate the route-shadowing conflict with config.py, which registers its own GET /actions/{name} returning ActionConfig. Add configActionRaw endpoint helper in endpoints.ts and update fetchActionFile/updateActionFile in config.ts to use it. Add TestGetActionFileRaw and TestUpdateActionFileRaw test classes. --- .../{bangui-sim.conf => manual-Jail.conf} | 0 .../{bangui-sim.conf => manual-Jail.conf} | 0 Docs/Tasks.md | 150 ++++++++++++++---- backend/app/routers/file_config.py | 8 +- .../tests/test_routers/test_file_config.py | 96 +++++++++++ frontend/src/api/config.ts | 4 +- frontend/src/api/endpoints.ts | 1 + 7 files changed, 219 insertions(+), 40 deletions(-) rename Docker/fail2ban-dev-config/fail2ban/filter.d/{bangui-sim.conf => manual-Jail.conf} (100%) rename Docker/fail2ban-dev-config/fail2ban/jail.d/{bangui-sim.conf => manual-Jail.conf} (100%) diff --git a/Docker/fail2ban-dev-config/fail2ban/filter.d/bangui-sim.conf b/Docker/fail2ban-dev-config/fail2ban/filter.d/manual-Jail.conf similarity index 100% rename from Docker/fail2ban-dev-config/fail2ban/filter.d/bangui-sim.conf rename to Docker/fail2ban-dev-config/fail2ban/filter.d/manual-Jail.conf diff --git a/Docker/fail2ban-dev-config/fail2ban/jail.d/bangui-sim.conf b/Docker/fail2ban-dev-config/fail2ban/jail.d/manual-Jail.conf similarity index 100% rename from Docker/fail2ban-dev-config/fail2ban/jail.d/bangui-sim.conf rename to Docker/fail2ban-dev-config/fail2ban/jail.d/manual-Jail.conf diff --git a/Docs/Tasks.md b/Docs/Tasks.md index e1d84a3..48c8a37 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -4,60 +4,142 @@ This document breaks the entire BanGUI project into development stages, ordered --- -## ✅ Task: Add "Deactivate Jail" Button for Inactive Jails in the Config View +## Bug Fix: "Raw Action Configuration" always empty — DONE -**Context:** -In `frontend/src/components/config/JailsTab.tsx`, the "Deactivate Jail" button is currently only rendered when a jail is active. When a jail is inactive but has an existing `jail.d/{name}.local` file (i.e. it was previously configured and has `enabled = false`), there is no action button to clean up that override file. +**Summary:** Renamed `GET /actions/{name}` and `PUT /actions/{name}` in `file_config.py` to `GET /actions/{name}/raw` and `PUT /actions/{name}/raw` to eliminate the route-shadowing conflict with `config.py`. Added `configActionRaw` endpoint helper in `endpoints.ts` and updated `fetchActionFile` / `updateActionFile` in `config.ts` to call it. Added `TestGetActionFileRaw` and `TestUpdateActionFileRaw` test classes. -**Goal:** -Add a "Deactivate Jail" (or "Remove Config") button to the jail config view for inactive jails that have a `.local` file. Clicking it should delete the `jail.d/{name}.local` file via the existing deactivate endpoint (`POST /api/config/jails/{name}/deactivate`) or a dedicated delete-override endpoint, making the UI consistent with the active-jail view. If no `.local` file exists for the inactive jail, the button should not be shown (there is nothing to clean up). +**Problem** +When a user opens the *Actions* tab in the Config screen, selects any action, and expands the "Raw Action Configuration" accordion, the text area is always blank. The `fetchContent` callback makes a `GET /api/config/actions/{name}` request expecting a `ConfFileContent` response (`{ content: string, name: string, filename: string }`), but the backend returns an `ActionConfig` (the fully-parsed structured model) instead. The `content` field is therefore `undefined` in the browser, which the `RawConfigSection` component renders as an empty string. -**Acceptance criteria:** -- Inactive jails that own a `.local` file show a "Deactivate Jail" button in their config panel. -- Calling the button removes or neutralises the `.local` file and refreshes the jail list. -- Inactive jails without a `.local` file are unaffected and show no extra button. +**Root cause** +Both `backend/app/routers/config.py` and `backend/app/routers/file_config.py` are mounted with the prefix `/api/config` (see lines 107 and 63 respectively). Both define a `GET /actions/{name}` route: + +- `config.py` → returns `ActionConfig` (parsed detail) +- `file_config.py` → returns `ConfFileContent` (raw file text) + +In `backend/app/main.py`, `config.router` is registered on line 402 and `file_config.router` on line 403. FastAPI matches the first registered route, so the raw-content endpoint is permanently shadowed. + +The filters feature already solved the same conflict by using distinct paths (`/filters/{name}/raw` for raw and `/filters/{name}` for parsed). Actions must follow the same pattern. + +**Fix — backend (`backend/app/routers/file_config.py`)** +Rename the two action raw-file routes: + +| Old path | New path | +|---|---| +| `GET /actions/{name}` | `GET /actions/{name}/raw` | +| `PUT /actions/{name}` | `PUT /actions/{name}/raw` | + +Update the module-level docstring comment block at the top of `file_config.py` to reflect the new paths. + +**Fix — frontend (`frontend/src/api/endpoints.ts`)** +Add a new helper alongside the existing `configAction` entry: + +```ts +configActionRaw: (name: string): string => `/config/actions/${encodeURIComponent(name)}/raw`, +``` + +**Fix — frontend (`frontend/src/api/config.ts`)** +Change `fetchActionFile` and `updateActionFile` to call `ENDPOINTS.configActionRaw(name)` instead of `ENDPOINTS.configAction(name)`. + +**No changes needed elsewhere.** `ActionsTab.tsx` already passes `fetchActionFile` / `updateActionFile` into `RawConfigSection` via `fetchRaw` / `saveRaw`; the resolved URL is the only thing that needs to change. --- -## ✅ Task: Remove the "fail2ban Stopped After Jail Activation" Recovery Banner +## Rename dev jail `bangui-sim` → `manual-Jail` — DONE -**Context:** -`frontend/src/components/common/RecoveryBanner.tsx` renders a full-page banner with the heading *"fail2ban Stopped After Jail Activation"* and the body *"fail2ban stopped responding after activating jail `{name}`. The jail's configuration may be invalid."* together with "Disable & Restart" and "View Logs" action buttons. This banner interrupts the UI even when the backend has already handled the rollback automatically. +**Summary:** Renamed `jail.d/bangui-sim.conf` → `manual-Jail.conf` and `filter.d/bangui-sim.conf` → `manual-Jail.conf` (via `git mv`), updated all internal references. Updated `check_ban_status.sh`, `simulate_failed_logins.sh`, and `fail2ban-dev-config/README.md` to replace all `bangui-sim` references with `manual-Jail`. -**Goal:** -Remove the `RecoveryBanner` component and all its mount points from the application. Any state that was used exclusively to drive this banner (e.g. a `fail2banStopped` flag or related context) should also be removed. If the underlying crash-detection logic is still needed for other features, keep that logic but detach it from the banner render path. +**Scope** +This is purely a Docker development-environment change. The frontend never hardcodes jail names; it reads them dynamically from the API. Only the files listed below need editing. -**Acceptance criteria:** -- The full-page banner no longer appears under any circumstances. -- No dead code or orphaned state references remain after the removal. -- All existing tests that reference `RecoveryBanner` are updated or removed accordingly. +**Files to update** + +1. **`Docker/fail2ban-dev-config/fail2ban/jail.d/bangui-sim.conf`** + - Rename the file to `manual-Jail.conf`. + - Change the section header from `[bangui-sim]` to `[manual-Jail]`. + - Change `filter = bangui-sim` to `filter = manual-Jail`. + - Update the file-header comment ("BanGUI — Simulated authentication failure jail" line and any other references to `bangui-sim`). + +2. **`Docker/fail2ban-dev-config/fail2ban/filter.d/bangui-sim.conf`** + - Rename the file to `manual-Jail.conf`. + - Update any internal comments that mention `bangui-sim`. + +3. **`Docker/check_ban_status.sh`** + - Change `readonly JAIL="bangui-sim"` to `readonly JAIL="manual-Jail"`. + - Update the file-header comment block that references `bangui-sim`. + +4. **`Docker/simulate_failed_logins.sh`** + - Update all comments that mention `bangui-sim` or `bangui-auth` to refer to `manual-Jail` instead. + - Do **not** change the log-line format string (`bangui-auth: authentication failure from `) unless the filter's `failregex` in the renamed `manual-Jail.conf` is also updated to match the new prefix; keep them in sync. + +5. **`Docker/fail2ban-dev-config/README.md`** + - Replace every occurrence of `bangui-sim` with `manual-Jail`. + +After renaming, run `docker compose -f Docker/compose.debug.yml restart fail2ban` and verify with `bash Docker/check_ban_status.sh` that the jail is active under its new name. --- -## ✅ Task: Fix Activation Failure Rollback — Actually Delete the `.local` File +## Bug Fix: Config screen content pane does not update when switching jails — DONE -**Context:** -When jail activation fails after the `jail.d/{name}.local` file has already been written (i.e. fail2ban reloaded but the jail never came up, or fail2ban became unresponsive), `_rollback_activation_async()` in `backend/app/services/config_file_service.py` is supposed to restore the pre-activation state. The frontend then displays *"Activation Failed — System Recovered"* with the message *"Activation of jail `{name}` failed. The server has been automatically recovered."* +**Summary:** Added `key={selectedActiveJail.name}` to `JailConfigDetail` and `key={selectedInactiveJail.name}` to `InactiveJailDetail` in `JailsTab.tsx`, forcing React to unmount and remount the detail component on jail selection changes. -In practice, recovery does not happen: the `.local` file remains on disk with `enabled = true`, leaving fail2ban in a broken state on next restart. The frontend misleadingly reports success. +**Problem** +In the *Jails* tab of the Config screen, clicking a jail name in the left-hand list correctly highlights the new selection, but the right-hand content pane continues to show the previously selected jail (e.g. selecting `blocklist-import` after `manual-Jail` still displays `manual-Jail`'s configuration). -**Goal:** -Fix `_rollback_activation_async()` so that it reliably removes (or reverts) the `.local` file whenever activation fails: +**Root cause** +In `frontend/src/components/config/JailsTab.tsx`, the child components rendered by `ConfigListDetail` are not given a `key` prop: -1. If the `.local` file did not exist before activation, **delete** `jail.d/{name}.local` outright. -2. If it existed before activation (e.g. previously had `enabled = false`), **restore** its original content atomically (temp-file rename pattern already used elsewhere in the service). -3. After deleting/restoring the file, attempt a `reload_all` socket command so fail2ban picks up the reverted state. -4. Only set `recovered = true` in the `JailActivationResponse` once all three steps above have actually succeeded. If any step fails, set `recovered = false` and log the error. -5. On the frontend side, the *"Activation Failed — System Recovered"* `MessageBar` in `ActivateJailDialog.tsx` should only be shown when the backend actually returns `recovered = true`. The current misleading message should be replaced with a more actionable one when `recovered = false`. +```tsx +{selectedActiveJail !== undefined ? ( + +) : selectedInactiveJail !== undefined ? ( + +) : null} +``` -**Acceptance criteria:** -- After a failed activation, `jail.d/{name}.local` is either absent or contains its pre-activation content. -- `recovered: true` is only returned when the rollback fully succeeded. -- The UI message accurately reflects the actual recovery state. -- A test in `backend/tests/test_services/` covers the rollback path, asserting the file is absent/reverted and the response flag is correct. +When the user switches between two jails of the same type (both active or both inactive), React reuses the existing component instance and only updates its props. Any internal state derived from the previous jail — including the `loadedRef` guard inside every nested `RawConfigSection` — is never reset. As a result, forms still show the old jail's values and the raw-config section refuses to re-fetch because `loadedRef.current` is already `true`. +Compare with `ActionsTab.tsx`, where `ActionDetail` correctly uses `key={selectedAction.name}`: +```tsx + +``` +**Fix — `frontend/src/components/config/JailsTab.tsx`** +Add `key` props to both detail components so React unmounts and remounts them whenever the selected jail changes: +```tsx +{selectedActiveJail !== undefined ? ( + { handleDeactivate(selectedActiveJail.name); }} + /> +) : selectedInactiveJail !== undefined ? ( + { setActivateTarget(selectedInactiveJail); }} + onDeactivate={ + selectedInactiveJail.has_local_override + ? (): void => { handleDeactivateInactive(selectedInactiveJail.name); } + : undefined + } + /> +) : null} +``` +No other files need changing. The `key` change is the minimal, isolated fix. +--- diff --git a/backend/app/routers/file_config.py b/backend/app/routers/file_config.py index 54aee28..cb93f33 100644 --- a/backend/app/routers/file_config.py +++ b/backend/app/routers/file_config.py @@ -14,8 +14,8 @@ Endpoints: * ``GET /api/config/filters/{name}/parsed`` — parse a filter file into a structured model * ``PUT /api/config/filters/{name}/parsed`` — update a filter file from a structured model * ``GET /api/config/actions`` — list all action files -* ``GET /api/config/actions/{name}`` — get one action file (with content) -* ``PUT /api/config/actions/{name}`` — update an action file +* ``GET /api/config/actions/{name}/raw`` — get one action file (raw content) +* ``PUT /api/config/actions/{name}/raw`` — update an action file (raw content) * ``POST /api/config/actions`` — create a new action file * ``GET /api/config/actions/{name}/parsed`` — parse an action file into a structured model * ``PUT /api/config/actions/{name}/parsed`` — update an action file from a structured model @@ -460,7 +460,7 @@ async def list_action_files( @router.get( - "/actions/{name}", + "/actions/{name}/raw", response_model=ConfFileContent, summary="Return an action definition file with its content", ) @@ -496,7 +496,7 @@ async def get_action_file( @router.put( - "/actions/{name}", + "/actions/{name}/raw", status_code=status.HTTP_204_NO_CONTENT, summary="Update an action definition file", ) diff --git a/backend/tests/test_routers/test_file_config.py b/backend/tests/test_routers/test_file_config.py index c788bef..2226238 100644 --- a/backend/tests/test_routers/test_file_config.py +++ b/backend/tests/test_routers/test_file_config.py @@ -377,6 +377,102 @@ class TestCreateActionFile: assert resp.json()["name"] == "myaction" +# --------------------------------------------------------------------------- +# GET /api/config/actions/{name}/raw +# --------------------------------------------------------------------------- + + +class TestGetActionFileRaw: + """Tests for ``GET /api/config/actions/{name}/raw``.""" + + async def test_200_returns_content(self, file_config_client: AsyncClient) -> None: + with patch( + "app.routers.file_config.file_config_service.get_action_file", + AsyncMock(return_value=_conf_file_content("iptables")), + ): + resp = await file_config_client.get("/api/config/actions/iptables/raw") + + assert resp.status_code == 200 + assert resp.json()["name"] == "iptables" + + async def test_404_not_found(self, file_config_client: AsyncClient) -> None: + with patch( + "app.routers.file_config.file_config_service.get_action_file", + AsyncMock(side_effect=ConfigFileNotFoundError("missing")), + ): + resp = await file_config_client.get("/api/config/actions/missing/raw") + + assert resp.status_code == 404 + + async def test_503_on_config_dir_error( + self, file_config_client: AsyncClient + ) -> None: + with patch( + "app.routers.file_config.file_config_service.get_action_file", + AsyncMock(side_effect=ConfigDirError("no dir")), + ): + resp = await file_config_client.get("/api/config/actions/iptables/raw") + + assert resp.status_code == 503 + + +# --------------------------------------------------------------------------- +# PUT /api/config/actions/{name}/raw +# --------------------------------------------------------------------------- + + +class TestUpdateActionFileRaw: + """Tests for ``PUT /api/config/actions/{name}/raw``.""" + + async def test_204_on_success(self, file_config_client: AsyncClient) -> None: + with patch( + "app.routers.file_config.file_config_service.write_action_file", + AsyncMock(return_value=None), + ): + resp = await file_config_client.put( + "/api/config/actions/iptables/raw", + json={"content": "[Definition]\nactionban = iptables -I INPUT -s -j DROP\n"}, + ) + + assert resp.status_code == 204 + + async def test_400_write_error(self, file_config_client: AsyncClient) -> None: + with patch( + "app.routers.file_config.file_config_service.write_action_file", + AsyncMock(side_effect=ConfigFileWriteError("disk full")), + ): + resp = await file_config_client.put( + "/api/config/actions/iptables/raw", + json={"content": "x"}, + ) + + assert resp.status_code == 400 + + async def test_404_not_found(self, file_config_client: AsyncClient) -> None: + with patch( + "app.routers.file_config.file_config_service.write_action_file", + AsyncMock(side_effect=ConfigFileNotFoundError("missing")), + ): + resp = await file_config_client.put( + "/api/config/actions/missing/raw", + json={"content": "x"}, + ) + + assert resp.status_code == 404 + + async def test_400_invalid_name(self, file_config_client: AsyncClient) -> None: + with patch( + "app.routers.file_config.file_config_service.write_action_file", + AsyncMock(side_effect=ConfigFileNameError("bad/../name")), + ): + resp = await file_config_client.put( + "/api/config/actions/escape/raw", + json={"content": "x"}, + ) + + assert resp.status_code == 400 + + # --------------------------------------------------------------------------- # POST /api/config/jail-files # --------------------------------------------------------------------------- diff --git a/frontend/src/api/config.ts b/frontend/src/api/config.ts index 2fe5d32..2b7ef23 100644 --- a/frontend/src/api/config.ts +++ b/frontend/src/api/config.ts @@ -263,14 +263,14 @@ export async function fetchActionFiles(): Promise { } export async function fetchActionFile(name: string): Promise { - return get(ENDPOINTS.configAction(name)); + return get(ENDPOINTS.configActionRaw(name)); } export async function updateActionFile( name: string, req: ConfFileUpdateRequest ): Promise { - await put(ENDPOINTS.configAction(name), req); + await put(ENDPOINTS.configActionRaw(name), req); } export async function createActionFile( diff --git a/frontend/src/api/endpoints.ts b/frontend/src/api/endpoints.ts index 0a43a2f..871d8b6 100644 --- a/frontend/src/api/endpoints.ts +++ b/frontend/src/api/endpoints.ts @@ -104,6 +104,7 @@ export const ENDPOINTS = { `/config/jails/${encodeURIComponent(jailName)}/action/${encodeURIComponent(actionName)}`, configActions: "/config/actions", configAction: (name: string): string => `/config/actions/${encodeURIComponent(name)}`, + configActionRaw: (name: string): string => `/config/actions/${encodeURIComponent(name)}/raw`, configActionParsed: (name: string): string => `/config/actions/${encodeURIComponent(name)}/parsed`, -- 2.49.1 From 00119ed68dc887722be96706970aa9a43a52a8ad Mon Sep 17 00:00:00 2001 From: Lukas Date: Sun, 15 Mar 2026 14:09:49 +0100 Subject: [PATCH 06/15] Rename dev jail bangui-sim to manual-Jail Rename fail2ban-dev-config jail.d/bangui-sim.conf and filter.d/bangui-sim.conf to manual-Jail.conf. Update section header, filter reference, and comments in both files. Update JAIL constant and header comment in check_ban_status.sh. Update comments in simulate_failed_logins.sh. Replace all bangui-sim occurrences in fail2ban-dev-config/README.md. --- Docker/check_ban_status.sh | 4 ++-- Docker/fail2ban-dev-config/README.md | 14 +++++++------- .../fail2ban/filter.d/manual-Jail.conf | 1 + .../fail2ban/jail.d/manual-Jail.conf | 4 ++-- Docker/simulate_failed_logins.sh | 4 ++-- 5 files changed, 14 insertions(+), 13 deletions(-) diff --git a/Docker/check_ban_status.sh b/Docker/check_ban_status.sh index 74a10f1..8e036e6 100644 --- a/Docker/check_ban_status.sh +++ b/Docker/check_ban_status.sh @@ -2,7 +2,7 @@ # ────────────────────────────────────────────────────────────── # check_ban_status.sh # -# Queries the bangui-sim jail inside the running fail2ban +# Queries the manual-Jail jail inside the running fail2ban # container and optionally unbans a specific IP. # # Usage: @@ -17,7 +17,7 @@ set -euo pipefail readonly CONTAINER="bangui-fail2ban-dev" -readonly JAIL="bangui-sim" +readonly JAIL="manual-Jail" # ── Helper: run a fail2ban-client command inside the container ─ f2b() { diff --git a/Docker/fail2ban-dev-config/README.md b/Docker/fail2ban-dev-config/README.md index 8d41b71..6ecaf56 100644 --- a/Docker/fail2ban-dev-config/README.md +++ b/Docker/fail2ban-dev-config/README.md @@ -2,7 +2,7 @@ This directory contains the fail2ban configuration and supporting scripts for a self-contained development test environment. A simulation script writes fake -authentication-failure log lines, fail2ban detects them via the `bangui-sim` +authentication-failure log lines, fail2ban detects them via the `manual-Jail` jail, and bans the offending IP — giving a fully reproducible ban/unban cycle without a real service. @@ -71,14 +71,14 @@ Chains steps 1–3 automatically with appropriate sleep intervals. | File | Purpose | |------|---------| -| `fail2ban/filter.d/bangui-sim.conf` | Defines the `failregex` that matches simulation log lines | -| `fail2ban/jail.d/bangui-sim.conf` | Jail settings: `maxretry=3`, `bantime=60s`, `findtime=120s` | +| `fail2ban/filter.d/manual-Jail.conf` | Defines the `failregex` that matches simulation log lines | +| `fail2ban/jail.d/manual-Jail.conf` | Jail settings: `maxretry=3`, `bantime=60s`, `findtime=120s` | | `Docker/logs/auth.log` | Log file written by the simulation script (host path) | Inside the container the log file is mounted at `/remotelogs/bangui/auth.log` (see `fail2ban/paths-lsio.conf` — `remote_logs_path = /remotelogs`). -To change sensitivity, edit `fail2ban/jail.d/bangui-sim.conf`: +To change sensitivity, edit `fail2ban/jail.d/manual-Jail.conf`: ```ini maxretry = 3 # failures before a ban @@ -108,14 +108,14 @@ Test the regex manually: ```bash docker exec bangui-fail2ban-dev \ - fail2ban-regex /remotelogs/bangui/auth.log bangui-sim + fail2ban-regex /remotelogs/bangui/auth.log manual-Jail ``` The output should show matched lines. If nothing matches, check that the log lines match the corresponding `failregex` pattern: ``` -# bangui-sim (auth log): +# manual-Jail (auth log): YYYY-MM-DD HH:MM:SS bangui-auth: authentication failure from ``` @@ -132,7 +132,7 @@ sudo modprobe ip_tables ### IP not banned despite enough failures Check whether the source IP falls inside the `ignoreip` range defined in -`fail2ban/jail.d/bangui-sim.conf`: +`fail2ban/jail.d/manual-Jail.conf`: ```ini ignoreip = 127.0.0.0/8 ::1 172.16.0.0/12 diff --git a/Docker/fail2ban-dev-config/fail2ban/filter.d/manual-Jail.conf b/Docker/fail2ban-dev-config/fail2ban/filter.d/manual-Jail.conf index 275b83f..48019ec 100644 --- a/Docker/fail2ban-dev-config/fail2ban/filter.d/manual-Jail.conf +++ b/Docker/fail2ban-dev-config/fail2ban/filter.d/manual-Jail.conf @@ -3,6 +3,7 @@ # # Matches lines written by Docker/simulate_failed_logins.sh # Format: bangui-auth: authentication failure from +# Jail: manual-Jail # ────────────────────────────────────────────────────────────── [Definition] diff --git a/Docker/fail2ban-dev-config/fail2ban/jail.d/manual-Jail.conf b/Docker/fail2ban-dev-config/fail2ban/jail.d/manual-Jail.conf index 8137a82..00a9a82 100644 --- a/Docker/fail2ban-dev-config/fail2ban/jail.d/manual-Jail.conf +++ b/Docker/fail2ban-dev-config/fail2ban/jail.d/manual-Jail.conf @@ -5,10 +5,10 @@ # for lines produced by Docker/simulate_failed_logins.sh. # ────────────────────────────────────────────────────────────── -[bangui-sim] +[manual-Jail] enabled = true -filter = bangui-sim +filter = manual-Jail logpath = /remotelogs/bangui/auth.log backend = polling maxretry = 3 diff --git a/Docker/simulate_failed_logins.sh b/Docker/simulate_failed_logins.sh index 3a01691..a0ad9ac 100644 --- a/Docker/simulate_failed_logins.sh +++ b/Docker/simulate_failed_logins.sh @@ -3,7 +3,7 @@ # simulate_failed_logins.sh # # Writes synthetic authentication-failure log lines to a file -# that matches the bangui-sim fail2ban filter. +# that matches the manual-Jail fail2ban filter. # # Usage: # bash Docker/simulate_failed_logins.sh [COUNT] [SOURCE_IP] [LOG_FILE] @@ -13,7 +13,7 @@ # SOURCE_IP: 192.168.100.99 # LOG_FILE : Docker/logs/auth.log (relative to repo root) # -# Log line format (must match bangui-sim failregex exactly): +# Log line format (must match manual-Jail failregex exactly): # YYYY-MM-DD HH:MM:SS bangui-auth: authentication failure from # ────────────────────────────────────────────────────────────── -- 2.49.1 From 5a5c619a346ddc9290873181d98f53134634e637 Mon Sep 17 00:00:00 2001 From: Lukas Date: Sun, 15 Mar 2026 14:10:01 +0100 Subject: [PATCH 07/15] Fix JailsTab content pane not updating on jail switch Add key={selectedActiveJail.name} and key={selectedInactiveJail.name} to JailConfigDetail and InactiveJailDetail in JailsTab.tsx so React unmounts and remounts the detail component whenever the selected jail changes, resetting all internal state including the loadedRef guard. --- frontend/src/components/config/JailsTab.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frontend/src/components/config/JailsTab.tsx b/frontend/src/components/config/JailsTab.tsx index 4b38b50..9606325 100644 --- a/frontend/src/components/config/JailsTab.tsx +++ b/frontend/src/components/config/JailsTab.tsx @@ -898,12 +898,14 @@ export function JailsTab(): React.JSX.Element { > {selectedActiveJail !== undefined ? ( { handleDeactivate(selectedActiveJail.name); }} /> ) : selectedInactiveJail !== undefined ? ( { setActivateTarget(selectedInactiveJail); }} onDeactivate={ -- 2.49.1 From eb859af37182fe6fd2e4d9cd6855e6338a66a6f9 Mon Sep 17 00:00:00 2001 From: Lukas Date: Sun, 15 Mar 2026 17:13:11 +0100 Subject: [PATCH 08/15] chore: bump version to 0.9.0 --- backend/pyproject.toml | 2 +- frontend/package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/pyproject.toml b/backend/pyproject.toml index beab707..7b8ad69 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "bangui-backend" -version = "0.1.0" +version = "0.9.0" description = "BanGUI backend — fail2ban web management interface" requires-python = ">=3.12" dependencies = [ diff --git a/frontend/package.json b/frontend/package.json index b62ef87..4c6b160 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,7 +1,7 @@ { "name": "bangui-frontend", "private": true, - "version": "0.1.0", + "version": "0.9.0", "description": "BanGUI frontend — fail2ban web management interface", "type": "module", "scripts": { -- 2.49.1 From 21753c4f066ffbc6e64964a80e31a3da1661d667 Mon Sep 17 00:00:00 2001 From: Lukas Date: Sun, 15 Mar 2026 18:05:53 +0100 Subject: [PATCH 09/15] Fix Stage 0 bootstrap and startup regression Task 0.1: Create database parent directory before connecting - main.py _lifespan now calls Path(database_path).parent.mkdir(parents=True, exist_ok=True) before aiosqlite.connect() so the app starts cleanly on a fresh Docker volume with a nested database path. Task 0.2: SetupRedirectMiddleware redirects when db is None - Guard now reads: if db is None or not is_setup_complete(db) A missing database (startup still in progress) is treated as setup not complete instead of silently allowing all API routes through. Task 0.3: SetupGuard redirects to /setup on API failure - .catch() handler now sets status to 'pending' instead of 'done'. A crashed backend cannot serve protected routes; conservative fallback is to redirect to /setup. Task 0.4: SetupPage shows spinner while checking setup status - Added 'checking' boolean state; full-screen Spinner is rendered until getSetupStatus() resolves, preventing form flash before redirect. - Added console.warn in catch block; cleanup return added to useEffect. Also: remove unused type: ignore[call-arg] from config.py. Tests: 18 backend tests pass; 117 frontend tests pass. --- Docs/Tasks.md | 153 ++++-------------- backend/app/config.py | 2 +- backend/app/main.py | 21 +-- backend/tests/test_routers/test_setup.py | 150 ++++++++++++++++- frontend/src/components/SetupGuard.tsx | 6 +- .../components/__tests__/SetupGuard.test.tsx | 77 +++++++++ frontend/src/pages/SetupPage.tsx | 35 +++- .../src/pages/__tests__/SetupPage.test.tsx | 88 ++++++++++ 8 files changed, 388 insertions(+), 144 deletions(-) create mode 100644 frontend/src/components/__tests__/SetupGuard.test.tsx create mode 100644 frontend/src/pages/__tests__/SetupPage.test.tsx diff --git a/Docs/Tasks.md b/Docs/Tasks.md index 48c8a37..4b56645 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -4,142 +4,43 @@ This document breaks the entire BanGUI project into development stages, ordered --- -## Bug Fix: "Raw Action Configuration" always empty — DONE +## Stage 0 — First-Run Bootstrap & Startup Fix -**Summary:** Renamed `GET /actions/{name}` and `PUT /actions/{name}` in `file_config.py` to `GET /actions/{name}/raw` and `PUT /actions/{name}/raw` to eliminate the route-shadowing conflict with `config.py`. Added `configActionRaw` endpoint helper in `endpoints.ts` and updated `fetchActionFile` / `updateActionFile` in `config.ts` to call it. Added `TestGetActionFileRaw` and `TestUpdateActionFileRaw` test classes. - -**Problem** -When a user opens the *Actions* tab in the Config screen, selects any action, and expands the "Raw Action Configuration" accordion, the text area is always blank. The `fetchContent` callback makes a `GET /api/config/actions/{name}` request expecting a `ConfFileContent` response (`{ content: string, name: string, filename: string }`), but the backend returns an `ActionConfig` (the fully-parsed structured model) instead. The `content` field is therefore `undefined` in the browser, which the `RawConfigSection` component renders as an empty string. - -**Root cause** -Both `backend/app/routers/config.py` and `backend/app/routers/file_config.py` are mounted with the prefix `/api/config` (see lines 107 and 63 respectively). Both define a `GET /actions/{name}` route: - -- `config.py` → returns `ActionConfig` (parsed detail) -- `file_config.py` → returns `ConfFileContent` (raw file text) - -In `backend/app/main.py`, `config.router` is registered on line 402 and `file_config.router` on line 403. FastAPI matches the first registered route, so the raw-content endpoint is permanently shadowed. - -The filters feature already solved the same conflict by using distinct paths (`/filters/{name}/raw` for raw and `/filters/{name}` for parsed). Actions must follow the same pattern. - -**Fix — backend (`backend/app/routers/file_config.py`)** -Rename the two action raw-file routes: - -| Old path | New path | -|---|---| -| `GET /actions/{name}` | `GET /actions/{name}/raw` | -| `PUT /actions/{name}` | `PUT /actions/{name}/raw` | - -Update the module-level docstring comment block at the top of `file_config.py` to reflect the new paths. - -**Fix — frontend (`frontend/src/api/endpoints.ts`)** -Add a new helper alongside the existing `configAction` entry: - -```ts -configActionRaw: (name: string): string => `/config/actions/${encodeURIComponent(name)}/raw`, -``` - -**Fix — frontend (`frontend/src/api/config.ts`)** -Change `fetchActionFile` and `updateActionFile` to call `ENDPOINTS.configActionRaw(name)` instead of `ENDPOINTS.configAction(name)`. - -**No changes needed elsewhere.** `ActionsTab.tsx` already passes `fetchActionFile` / `updateActionFile` into `RawConfigSection` via `fetchRaw` / `saveRaw`; the resolved URL is the only thing that needs to change. +These tasks fix a crash-on-first-boot regression and make the setup/login redirect flow reliable. They must be completed before any other feature work because the application cannot start without them. --- -## Rename dev jail `bangui-sim` → `manual-Jail` — DONE +### Task 0.1 — Fix: Create the database parent directory before connecting ✅ DONE -**Summary:** Renamed `jail.d/bangui-sim.conf` → `manual-Jail.conf` and `filter.d/bangui-sim.conf` → `manual-Jail.conf` (via `git mv`), updated all internal references. Updated `check_ban_status.sh`, `simulate_failed_logins.sh`, and `fail2ban-dev-config/README.md` to replace all `bangui-sim` references with `manual-Jail`. +**File:** `backend/app/main.py` -**Scope** -This is purely a Docker development-environment change. The frontend never hardcodes jail names; it reads them dynamically from the API. Only the files listed below need editing. - -**Files to update** - -1. **`Docker/fail2ban-dev-config/fail2ban/jail.d/bangui-sim.conf`** - - Rename the file to `manual-Jail.conf`. - - Change the section header from `[bangui-sim]` to `[manual-Jail]`. - - Change `filter = bangui-sim` to `filter = manual-Jail`. - - Update the file-header comment ("BanGUI — Simulated authentication failure jail" line and any other references to `bangui-sim`). - -2. **`Docker/fail2ban-dev-config/fail2ban/filter.d/bangui-sim.conf`** - - Rename the file to `manual-Jail.conf`. - - Update any internal comments that mention `bangui-sim`. - -3. **`Docker/check_ban_status.sh`** - - Change `readonly JAIL="bangui-sim"` to `readonly JAIL="manual-Jail"`. - - Update the file-header comment block that references `bangui-sim`. - -4. **`Docker/simulate_failed_logins.sh`** - - Update all comments that mention `bangui-sim` or `bangui-auth` to refer to `manual-Jail` instead. - - Do **not** change the log-line format string (`bangui-auth: authentication failure from `) unless the filter's `failregex` in the renamed `manual-Jail.conf` is also updated to match the new prefix; keep them in sync. - -5. **`Docker/fail2ban-dev-config/README.md`** - - Replace every occurrence of `bangui-sim` with `manual-Jail`. - -After renaming, run `docker compose -f Docker/compose.debug.yml restart fail2ban` and verify with `bash Docker/check_ban_status.sh` that the jail is active under its new name. +**Implemented:** In `_lifespan`, resolve `settings.database_path` to a `Path`, call `.parent.mkdir(parents=True, exist_ok=True)` before `aiosqlite.connect()`. Added `debug`-level structured log line after mkdir. Tests added in `TestLifespanDatabaseDirectoryCreation`. --- -## Bug Fix: Config screen content pane does not update when switching jails — DONE +### Task 0.2 — Fix: SetupRedirectMiddleware must treat a missing database as "setup not complete" ✅ DONE -**Summary:** Added `key={selectedActiveJail.name}` to `JailConfigDetail` and `key={selectedInactiveJail.name}` to `InactiveJailDetail` in `JailsTab.tsx`, forcing React to unmount and remount the detail component on jail selection changes. +**File:** `backend/app/main.py` -**Problem** -In the *Jails* tab of the Config screen, clicking a jail name in the left-hand list correctly highlights the new selection, but the right-hand content pane continues to show the previously selected jail (e.g. selecting `blocklist-import` after `manual-Jail` still displays `manual-Jail`'s configuration). - -**Root cause** -In `frontend/src/components/config/JailsTab.tsx`, the child components rendered by `ConfigListDetail` are not given a `key` prop: - -```tsx -{selectedActiveJail !== undefined ? ( - -) : selectedInactiveJail !== undefined ? ( - -) : null} -``` - -When the user switches between two jails of the same type (both active or both inactive), React reuses the existing component instance and only updates its props. Any internal state derived from the previous jail — including the `loadedRef` guard inside every nested `RawConfigSection` — is never reset. As a result, forms still show the old jail's values and the raw-config section refuses to re-fetch because `loadedRef.current` is already `true`. - -Compare with `ActionsTab.tsx`, where `ActionDetail` correctly uses `key={selectedAction.name}`: - -```tsx - -``` - -**Fix — `frontend/src/components/config/JailsTab.tsx`** -Add `key` props to both detail components so React unmounts and remounts them whenever the selected jail changes: - -```tsx -{selectedActiveJail !== undefined ? ( - { handleDeactivate(selectedActiveJail.name); }} - /> -) : selectedInactiveJail !== undefined ? ( - { setActivateTarget(selectedInactiveJail); }} - onDeactivate={ - selectedInactiveJail.has_local_override - ? (): void => { handleDeactivateInactive(selectedInactiveJail.name); } - : undefined - } - /> -) : null} -``` - -No other files need changing. The `key` change is the minimal, isolated fix. +**Implemented:** Changed the guard in `SetupRedirectMiddleware.dispatch` so that `db is None` also triggers the redirect to `/api/setup`. The condition is now `if db is None or not await setup_service.is_setup_complete(db)`. The `_setup_complete_cached` flag is only set after a successful `is_setup_complete(db)` call with a live `db`. Tests added in `TestSetupRedirectMiddlewareDbNone`. + +--- + +### Task 0.3 — Fix: SetupGuard must redirect to /setup on API errors, not allow through ✅ DONE + +**File:** `frontend/src/components/SetupGuard.tsx` + +**Implemented:** Changed the `.catch()` handler to set `status` to `"pending"` instead of `"done"`. Updated the comment to explain the conservative fallback. Tests added in `SetupGuard.test.tsx`. + +--- + +### Task 0.4 — Fix: SetupPage must redirect to /login when setup is already complete ✅ DONE + +**File:** `frontend/src/pages/SetupPage.tsx` + +**Implemented:** +1. Added `checking` boolean state (initialised to `true`). While `checking` is true, a full-screen `` is rendered instead of the form, preventing the form from flashing. +2. The `useEffect` sets `checking` to `false` in both the `.then()` (when setup is not complete) and the `.catch()` branch. Added a `console.warn` in the catch block. Added a `cancelled` flag and cleanup return to the effect. +Tests added in `SetupPage.test.tsx`. --- diff --git a/backend/app/config.py b/backend/app/config.py index 0f73ce5..4e89da2 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -85,4 +85,4 @@ def get_settings() -> Settings: A validated :class:`Settings` object. Raises :class:`pydantic.ValidationError` if required keys are absent or values fail validation. """ - return Settings() # type: ignore[call-arg] # pydantic-settings populates required fields from env vars + return Settings() # pydantic-settings populates required fields from env vars diff --git a/backend/app/main.py b/backend/app/main.py index a02c1c1..2b2828e 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -138,6 +138,9 @@ async def _lifespan(app: FastAPI) -> AsyncGenerator[None, None]: log.info("bangui_starting_up", database_path=settings.database_path) # --- Application database --- + db_path: Path = Path(settings.database_path) + db_path.parent.mkdir(parents=True, exist_ok=True) + log.debug("database_directory_ensured", directory=str(db_path.parent)) db: aiosqlite.Connection = await aiosqlite.connect(settings.database_path) db.row_factory = aiosqlite.Row await init_db(db) @@ -320,17 +323,15 @@ class SetupRedirectMiddleware(BaseHTTPMiddleware): if path.startswith("/api") and not getattr( request.app.state, "_setup_complete_cached", False ): - db: aiosqlite.Connection | None = getattr(request.app.state, "db", None) - if db is not None: - from app.services import setup_service # noqa: PLC0415 + from app.services import setup_service # noqa: PLC0415 - if await setup_service.is_setup_complete(db): - request.app.state._setup_complete_cached = True - else: - return RedirectResponse( - url="/api/setup", - status_code=status.HTTP_307_TEMPORARY_REDIRECT, - ) + db: aiosqlite.Connection | None = getattr(request.app.state, "db", None) + if db is None or not await setup_service.is_setup_complete(db): + return RedirectResponse( + url="/api/setup", + status_code=status.HTTP_307_TEMPORARY_REDIRECT, + ) + request.app.state._setup_complete_cached = True return await call_next(request) diff --git a/backend/tests/test_routers/test_setup.py b/backend/tests/test_routers/test_setup.py index e07cef4..f66987d 100644 --- a/backend/tests/test_routers/test_setup.py +++ b/backend/tests/test_routers/test_setup.py @@ -3,7 +3,7 @@ from __future__ import annotations from pathlib import Path -from unittest.mock import patch +from unittest.mock import AsyncMock, MagicMock, patch import aiosqlite import pytest @@ -11,7 +11,7 @@ from httpx import ASGITransport, AsyncClient from app.config import Settings from app.db import init_db -from app.main import create_app +from app.main import _lifespan, create_app # --------------------------------------------------------------------------- # Shared setup payload @@ -286,3 +286,149 @@ class TestSetupCompleteCaching: # Cache was warm — is_setup_complete must not have been called. assert call_count == 0 + +# --------------------------------------------------------------------------- +# Task 0.1 — Lifespan creates the database parent directory (Task 0.1) +# --------------------------------------------------------------------------- + + +class TestLifespanDatabaseDirectoryCreation: + """App lifespan creates the database parent directory when it does not exist.""" + + async def test_creates_nested_database_directory(self, tmp_path: Path) -> None: + """Lifespan creates intermediate directories for the database path. + + Verifies that a deeply-nested database path is handled correctly — + the parent directories are created before ``aiosqlite.connect`` is + called so the app does not crash on a fresh volume. + """ + nested_db = tmp_path / "deep" / "nested" / "bangui.db" + assert not nested_db.parent.exists() + + settings = Settings( + database_path=str(nested_db), + fail2ban_socket="/tmp/fake.sock", + session_secret="test-lifespan-mkdir-secret", + session_duration_minutes=60, + timezone="UTC", + log_level="debug", + ) + app = create_app(settings=settings) + + mock_scheduler = MagicMock() + mock_scheduler.start = MagicMock() + mock_scheduler.shutdown = MagicMock() + + with ( + patch("app.services.geo_service.init_geoip"), + patch( + "app.services.geo_service.load_cache_from_db", + new=AsyncMock(return_value=None), + ), + patch("app.tasks.health_check.register"), + patch("app.tasks.blocklist_import.register"), + patch("app.tasks.geo_cache_flush.register"), + patch("app.tasks.geo_re_resolve.register"), + patch("app.main.AsyncIOScheduler", return_value=mock_scheduler), + ): + async with _lifespan(app): + assert nested_db.parent.exists(), ( + "Expected lifespan to create database parent directory" + ) + + async def test_existing_database_directory_is_not_an_error( + self, tmp_path: Path + ) -> None: + """Lifespan does not raise when the database directory already exists. + + ``mkdir(exist_ok=True)`` must be used so that re-starts on an existing + volume do not fail. + """ + db_path = tmp_path / "bangui.db" + # tmp_path already exists — this simulates a pre-existing volume. + + settings = Settings( + database_path=str(db_path), + fail2ban_socket="/tmp/fake.sock", + session_secret="test-lifespan-exist-ok-secret", + session_duration_minutes=60, + timezone="UTC", + log_level="debug", + ) + app = create_app(settings=settings) + + mock_scheduler = MagicMock() + mock_scheduler.start = MagicMock() + mock_scheduler.shutdown = MagicMock() + + with ( + patch("app.services.geo_service.init_geoip"), + patch( + "app.services.geo_service.load_cache_from_db", + new=AsyncMock(return_value=None), + ), + patch("app.tasks.health_check.register"), + patch("app.tasks.blocklist_import.register"), + patch("app.tasks.geo_cache_flush.register"), + patch("app.tasks.geo_re_resolve.register"), + patch("app.main.AsyncIOScheduler", return_value=mock_scheduler), + ): + # Should not raise FileExistsError or similar. + async with _lifespan(app): + assert tmp_path.exists() + + +# --------------------------------------------------------------------------- +# Task 0.2 — Middleware redirects when app.state.db is None +# --------------------------------------------------------------------------- + + +class TestSetupRedirectMiddlewareDbNone: + """SetupRedirectMiddleware redirects when the database is not yet available.""" + + async def test_redirects_to_setup_when_db_not_set(self, tmp_path: Path) -> None: + """A ``None`` db on app.state causes a 307 redirect to ``/api/setup``. + + Simulates the race window where a request arrives before the lifespan + has finished initialising the database connection. + """ + settings = Settings( + database_path=str(tmp_path / "bangui.db"), + fail2ban_socket="/tmp/fake_fail2ban.sock", + session_secret="test-db-none-secret", + session_duration_minutes=60, + timezone="UTC", + log_level="debug", + ) + app = create_app(settings=settings) + # Deliberately do NOT set app.state.db to simulate startup not complete. + + transport = ASGITransport(app=app) + async with AsyncClient( + transport=transport, base_url="http://test" + ) as ac: + response = await ac.get("/api/auth/login", follow_redirects=False) + + assert response.status_code == 307 + assert response.headers["location"] == "/api/setup" + + async def test_health_reachable_when_db_not_set(self, tmp_path: Path) -> None: + """Health endpoint is always reachable even when db is not initialised.""" + settings = Settings( + database_path=str(tmp_path / "bangui.db"), + fail2ban_socket="/tmp/fake_fail2ban.sock", + session_secret="test-db-none-health-secret", + session_duration_minutes=60, + timezone="UTC", + log_level="debug", + ) + app = create_app(settings=settings) + + transport = ASGITransport(app=app) + async with AsyncClient( + transport=transport, base_url="http://test" + ) as ac: + response = await ac.get("/api/health") + + assert response.status_code == 200 + diff --git a/frontend/src/components/SetupGuard.tsx b/frontend/src/components/SetupGuard.tsx index f448f9d..cb7d49a 100644 --- a/frontend/src/components/SetupGuard.tsx +++ b/frontend/src/components/SetupGuard.tsx @@ -33,9 +33,9 @@ export function SetupGuard({ children }: SetupGuardProps): React.JSX.Element { if (!cancelled) setStatus(res.completed ? "done" : "pending"); }) .catch((): void => { - // If the check fails, optimistically allow through — the backend will - // redirect API calls to /api/setup anyway. - if (!cancelled) setStatus("done"); + // A failed check conservatively redirects to /setup — a crashed + // backend cannot serve protected routes anyway. + if (!cancelled) setStatus("pending"); }); return (): void => { cancelled = true; diff --git a/frontend/src/components/__tests__/SetupGuard.test.tsx b/frontend/src/components/__tests__/SetupGuard.test.tsx new file mode 100644 index 0000000..c7e4a76 --- /dev/null +++ b/frontend/src/components/__tests__/SetupGuard.test.tsx @@ -0,0 +1,77 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { render, screen, waitFor } from "@testing-library/react"; +import { MemoryRouter, Routes, Route } from "react-router-dom"; +import { FluentProvider, webLightTheme } from "@fluentui/react-components"; +import { SetupGuard } from "../SetupGuard"; + +// Mock the setup API module so tests never hit a real network. +vi.mock("../../api/setup", () => ({ + getSetupStatus: vi.fn(), +})); + +import { getSetupStatus } from "../../api/setup"; + +const mockedGetSetupStatus = vi.mocked(getSetupStatus); + +function renderGuard() { + return render( + + + + +
Protected
+ + } + /> + Setup Page
} + /> + + + , + ); +} + +describe("SetupGuard", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("shows a spinner while the setup status is loading", () => { + // getSetupStatus resolves eventually — spinner should show immediately. + mockedGetSetupStatus.mockReturnValue(new Promise(() => {})); + renderGuard(); + expect(screen.getByRole("progressbar")).toBeInTheDocument(); + }); + + it("renders children when setup is complete", async () => { + mockedGetSetupStatus.mockResolvedValue({ completed: true }); + renderGuard(); + await waitFor(() => { + expect(screen.getByTestId("protected-content")).toBeInTheDocument(); + }); + }); + + it("redirects to /setup when setup is not complete", async () => { + mockedGetSetupStatus.mockResolvedValue({ completed: false }); + renderGuard(); + await waitFor(() => { + expect(screen.getByTestId("setup-page")).toBeInTheDocument(); + }); + expect(screen.queryByTestId("protected-content")).not.toBeInTheDocument(); + }); + + it("redirects to /setup when the API call fails", async () => { + // Task 0.3: a failed check must redirect to /setup, not allow through. + mockedGetSetupStatus.mockRejectedValue(new Error("Network error")); + renderGuard(); + await waitFor(() => { + expect(screen.getByTestId("setup-page")).toBeInTheDocument(); + }); + expect(screen.queryByTestId("protected-content")).not.toBeInTheDocument(); + }); +}); diff --git a/frontend/src/pages/SetupPage.tsx b/frontend/src/pages/SetupPage.tsx index e7c0f1c..1c86628 100644 --- a/frontend/src/pages/SetupPage.tsx +++ b/frontend/src/pages/SetupPage.tsx @@ -101,20 +101,36 @@ export function SetupPage(): React.JSX.Element { const styles = useStyles(); const navigate = useNavigate(); + const [checking, setChecking] = useState(true); const [values, setValues] = useState(DEFAULT_VALUES); const [errors, setErrors] = useState>>({}); const [apiError, setApiError] = useState(null); const [submitting, setSubmitting] = useState(false); // Redirect to /login if setup has already been completed. + // Show a full-screen spinner while the check is in flight to prevent + // the form from flashing before the redirect fires. useEffect(() => { + let cancelled = false; getSetupStatus() .then((res) => { - if (res.completed) navigate("/login", { replace: true }); + if (!cancelled) { + if (res.completed) { + navigate("/login", { replace: true }); + } else { + setChecking(false); + } + } }) .catch(() => { - /* ignore — stay on setup page */ + // Failed check: the backend may still be starting up. Stay on this + // page so the user can attempt setup once the backend is ready. + console.warn("SetupPage: setup status check failed — rendering setup form"); + if (!cancelled) setChecking(false); }); + return (): void => { + cancelled = true; + }; }, [navigate]); // --------------------------------------------------------------------------- @@ -187,6 +203,21 @@ export function SetupPage(): React.JSX.Element { // Render // --------------------------------------------------------------------------- + if (checking) { + return ( +
+ +
+ ); + } + return (
diff --git a/frontend/src/pages/__tests__/SetupPage.test.tsx b/frontend/src/pages/__tests__/SetupPage.test.tsx new file mode 100644 index 0000000..96c76aa --- /dev/null +++ b/frontend/src/pages/__tests__/SetupPage.test.tsx @@ -0,0 +1,88 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { render, screen, waitFor } from "@testing-library/react"; +import { MemoryRouter, Routes, Route } from "react-router-dom"; +import { FluentProvider, webLightTheme } from "@fluentui/react-components"; +import { SetupPage } from "../SetupPage"; + +// Mock the setup API so tests never hit a real network. +vi.mock("../../api/setup", () => ({ + getSetupStatus: vi.fn(), + submitSetup: vi.fn(), +})); + +// Mock the crypto utility — we only need it to resolve without testing SHA256. +vi.mock("../../utils/crypto", () => ({ + sha256Hex: vi.fn().mockResolvedValue("hashed-password"), +})); + +import { getSetupStatus } from "../../api/setup"; + +const mockedGetSetupStatus = vi.mocked(getSetupStatus); + +function renderPage() { + return render( + + + + } /> + Login
} + /> + + + , + ); +} + +describe("SetupPage", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("shows a full-screen spinner while the setup status check is in flight", () => { + // getSetupStatus never resolves — spinner should be visible immediately. + mockedGetSetupStatus.mockReturnValue(new Promise(() => {})); + renderPage(); + expect(screen.getByRole("progressbar")).toBeInTheDocument(); + // Form should NOT be visible yet. + expect( + screen.queryByRole("heading", { name: /bangui setup/i }), + ).not.toBeInTheDocument(); + }); + + it("renders the setup form once the status check resolves (not complete)", async () => { + // Task 0.4: form must not flash before the check resolves. + mockedGetSetupStatus.mockResolvedValue({ completed: false }); + renderPage(); + await waitFor(() => { + expect( + screen.getByRole("heading", { name: /bangui setup/i }), + ).toBeInTheDocument(); + }); + // Spinner should be gone. + expect(screen.queryByRole("progressbar")).not.toBeInTheDocument(); + }); + + it("redirects to /login when setup is already complete", async () => { + mockedGetSetupStatus.mockResolvedValue({ completed: true }); + renderPage(); + await waitFor(() => { + expect(screen.getByTestId("login-page")).toBeInTheDocument(); + }); + }); + + it("renders the form and logs a warning when the status check fails", async () => { + // Task 0.4: catch block must log a warning and keep the form visible. + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + mockedGetSetupStatus.mockRejectedValue(new Error("Connection refused")); + renderPage(); + await waitFor(() => { + expect( + screen.getByRole("heading", { name: /bangui setup/i }), + ).toBeInTheDocument(); + }); + expect(warnSpy).toHaveBeenCalledOnce(); + warnSpy.mockRestore(); + }); +}); -- 2.49.1 From cdf73e2d65b77f60a8604c0991010b19394f361a Mon Sep 17 00:00:00 2001 From: Lukas Date: Sun, 15 Mar 2026 18:10:25 +0100 Subject: [PATCH 10/15] docker files --- Docker/Dockerfile.backend | 4 +-- Docker/Dockerfile.frontend | 4 +-- Docker/docker-compose.yml | 68 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 4 deletions(-) create mode 100644 Docker/docker-compose.yml diff --git a/Docker/Dockerfile.backend b/Docker/Dockerfile.backend index e097e67..849e552 100644 --- a/Docker/Dockerfile.backend +++ b/Docker/Dockerfile.backend @@ -10,7 +10,7 @@ # ────────────────────────────────────────────────────────────── # ── Stage 1: build dependencies ────────────────────────────── -FROM python:3.12-slim AS builder +FROM docker.io/library/python:3.12-slim AS builder WORKDIR /build @@ -28,7 +28,7 @@ RUN pip install --no-cache-dir --upgrade pip \ && pip install --no-cache-dir . # ── Stage 2: runtime image ─────────────────────────────────── -FROM python:3.12-slim AS runtime +FROM docker.io/library/python:3.12-slim AS runtime LABEL maintainer="BanGUI" \ description="BanGUI backend — fail2ban web management API" diff --git a/Docker/Dockerfile.frontend b/Docker/Dockerfile.frontend index 0f2658c..bb24ecf 100644 --- a/Docker/Dockerfile.frontend +++ b/Docker/Dockerfile.frontend @@ -10,7 +10,7 @@ # ────────────────────────────────────────────────────────────── # ── Stage 1: install & build ───────────────────────────────── -FROM node:22-alpine AS builder +FROM docker.io/library/node:22-alpine AS builder WORKDIR /build @@ -23,7 +23,7 @@ COPY frontend/ /build/ RUN npm run build # ── Stage 2: serve with nginx ──────────────────────────────── -FROM nginx:1.27-alpine AS runtime +FROM docker.io/library/nginx:1.27-alpine AS runtime LABEL maintainer="BanGUI" \ description="BanGUI frontend — fail2ban web management UI" diff --git a/Docker/docker-compose.yml b/Docker/docker-compose.yml new file mode 100644 index 0000000..907f14d --- /dev/null +++ b/Docker/docker-compose.yml @@ -0,0 +1,68 @@ +version: '3.8' +services: + fail2ban: + image: lscr.io/linuxserver/fail2ban:latest + container_name: fail2ban + cap_add: + - NET_ADMIN + - NET_RAW + network_mode: host + environment: + - PUID=1011 + - PGID=1001 + - TZ=Etc/UTC + - VERBOSITY=-vv #optional + + volumes: + - /server/server_fail2ban/config:/config + - /server/server_fail2ban/fail2ban-run:/var/run/fail2ban + - /var/log:/var/log + - /server/server_nextcloud/config/nextcloud.log:/remotelogs/nextcloud/nextcloud.log:ro #optional + - /server/server_nginx/data/logs:/remotelogs/nginx:ro #optional + - /server/server_gitea/log/gitea.log:/remotelogs/gitea/gitea.log:ro #optional + + + #- /path/to/homeassistant/log:/remotelogs/homeassistant:ro #optional + #- /path/to/unificontroller/log:/remotelogs/unificontroller:ro #optional + #- /path/to/vaultwarden/log:/remotelogs/vaultwarden:ro #optional + restart: unless-stopped + + backend: + image: git.lpl-mind.de/lukas.pupkalipinski/bangui/backend:latest + container_name: bangui-backend + restart: unless-stopped + depends_on: + fail2ban: + condition: service_started + environment: + BANGUI_DATABASE_PATH: "/data/bangui.db" + BANGUI_FAIL2BAN_SOCKET: "/var/run/fail2ban/fail2ban.sock" + BANGUI_FAIL2BAN_CONFIG_DIR: "/config/fail2ban" + BANGUI_LOG_LEVEL: "info" + BANGUI_SESSION_SECRET: "${BANGUI_SESSION_SECRET:?Set BANGUI_SESSION_SECRET}" + BANGUI_TIMEZONE: "${BANGUI_TIMEZONE:-UTC}" + volumes: + - /server/server_fail2ban/bangui-data:/data + - /server/server_fail2ban/fail2ban-run:/var/run/fail2ban:ro + - /server/server_fail2ban/config:/config:rw + expose: + - "8000" + networks: + - bangui-net + + # ── Frontend (nginx serving built SPA + API proxy) ────────── + frontend: + image: git.lpl-mind.de/lukas.pupkalipinski/bangui/frontend:latest + container_name: bangui-frontend + restart: unless-stopped + ports: + - "${BANGUI_PORT:-8080}:80" + depends_on: + backend: + condition: service_started + networks: + - bangui-net + +networks: + bangui-net: + name: bangui-net \ No newline at end of file -- 2.49.1 From c41165c294c95b6852b69103a66da74707c6d66c Mon Sep 17 00:00:00 2001 From: Lukas Date: Sun, 15 Mar 2026 21:29:23 +0100 Subject: [PATCH 11/15] Remove client-side SHA-256 pre-hashing from setup and login MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sha256Hex helper used window.crypto.subtle.digest(), which is only available in a secure context (HTTPS / localhost). In the HTTP Docker environment crypto.subtle is undefined, causing a TypeError before any request is sent — the setup and login forms both silently failed with 'An unexpected error occurred'. Fix: pass raw passwords directly to the API. The backend already applies bcrypt, which is sufficient. No stored hashes need migration because setup never completed successfully in the HTTP environment. * frontend/src/pages/SetupPage.tsx — remove sha256Hex call * frontend/src/api/auth.ts — remove sha256Hex call * frontend/src/pages/__tests__/SetupPage.test.tsx — drop crypto mock * frontend/src/utils/crypto.ts — deleted (no remaining callers) --- frontend/src/api/auth.ts | 10 ++------ frontend/src/pages/SetupPage.tsx | 6 +---- .../src/pages/__tests__/SetupPage.test.tsx | 5 ---- frontend/src/utils/crypto.ts | 23 ------------------- 4 files changed, 3 insertions(+), 41 deletions(-) delete mode 100644 frontend/src/utils/crypto.ts diff --git a/frontend/src/api/auth.ts b/frontend/src/api/auth.ts index 6fc8266..8917e3e 100644 --- a/frontend/src/api/auth.ts +++ b/frontend/src/api/auth.ts @@ -7,22 +7,16 @@ import { api } from "./client"; import { ENDPOINTS } from "./endpoints"; -import type { LoginRequest, LoginResponse, LogoutResponse } from "../types/auth"; -import { sha256Hex } from "../utils/crypto"; +import type { LoginResponse, LogoutResponse } from "../types/auth"; /** * Authenticate with the master password. * - * The password is SHA-256 hashed client-side before transmission so that - * the plaintext never leaves the browser. The backend bcrypt-verifies the - * received hash against the stored bcrypt(sha256) digest. - * * @param password - The master password entered by the user. * @returns The login response containing the session token. */ export async function login(password: string): Promise { - const body: LoginRequest = { password: await sha256Hex(password) }; - return api.post(ENDPOINTS.authLogin, body); + return api.post(ENDPOINTS.authLogin, { password }); } /** diff --git a/frontend/src/pages/SetupPage.tsx b/frontend/src/pages/SetupPage.tsx index 1c86628..47db271 100644 --- a/frontend/src/pages/SetupPage.tsx +++ b/frontend/src/pages/SetupPage.tsx @@ -22,7 +22,6 @@ import { useNavigate } from "react-router-dom"; import type { ChangeEvent, FormEvent } from "react"; import { ApiError } from "../api/client"; import { getSetupStatus, submitSetup } from "../api/setup"; -import { sha256Hex } from "../utils/crypto"; // --------------------------------------------------------------------------- // Styles @@ -177,11 +176,8 @@ export function SetupPage(): React.JSX.Element { setSubmitting(true); try { - // Hash the password client-side before transmission — the plaintext - // never leaves the browser. The backend bcrypt-hashes the received hash. - const hashedPassword = await sha256Hex(values.masterPassword); await submitSetup({ - master_password: hashedPassword, + master_password: values.masterPassword, database_path: values.databasePath, fail2ban_socket: values.fail2banSocket, timezone: values.timezone, diff --git a/frontend/src/pages/__tests__/SetupPage.test.tsx b/frontend/src/pages/__tests__/SetupPage.test.tsx index 96c76aa..65dbbda 100644 --- a/frontend/src/pages/__tests__/SetupPage.test.tsx +++ b/frontend/src/pages/__tests__/SetupPage.test.tsx @@ -10,11 +10,6 @@ vi.mock("../../api/setup", () => ({ submitSetup: vi.fn(), })); -// Mock the crypto utility — we only need it to resolve without testing SHA256. -vi.mock("../../utils/crypto", () => ({ - sha256Hex: vi.fn().mockResolvedValue("hashed-password"), -})); - import { getSetupStatus } from "../../api/setup"; const mockedGetSetupStatus = vi.mocked(getSetupStatus); diff --git a/frontend/src/utils/crypto.ts b/frontend/src/utils/crypto.ts deleted file mode 100644 index 721b521..0000000 --- a/frontend/src/utils/crypto.ts +++ /dev/null @@ -1,23 +0,0 @@ -/** - * Client-side cryptography utilities. - * - * Uses the browser-native SubtleCrypto API so no third-party bundle is required. - */ - -/** - * Return the SHA-256 hex digest of `input`. - * - * Hashing passwords before transmission means the plaintext never leaves the - * browser, even when HTTPS is not enforced in a development environment. - * The backend then applies bcrypt on top of the received hash. - * - * @param input - The string to hash (e.g. the master password). - * @returns Lowercase hex-encoded SHA-256 digest. - */ -export async function sha256Hex(input: string): Promise { - const data = new TextEncoder().encode(input); - const hashBuffer = await crypto.subtle.digest("SHA-256", data); - return Array.from(new Uint8Array(hashBuffer)) - .map((b) => b.toString(16).padStart(2, "0")) - .join(""); -} -- 2.49.1 From 57cf93b1e5ab42b3c0dc8d9cb68ab48c8e423df9 Mon Sep 17 00:00:00 2001 From: Lukas Date: Mon, 16 Mar 2026 16:26:39 +0100 Subject: [PATCH 12/15] Add ensure_jail_configs startup check for required jail config files On startup BanGUI now verifies that the four fail2ban jail config files required by its two custom jails (manual-Jail and blocklist-import) are present in `$fail2ban_config_dir/jail.d`. Any missing file is created with the correct default content; existing files are never overwritten. Files managed: - manual-Jail.conf (enabled=false template) - manual-Jail.local (enabled=true override) - blocklist-import.conf (enabled=false template) - blocklist-import.local (enabled=true override) The check runs in the lifespan hook immediately after logging is configured, before the database is opened. --- Docs/Tasks.md | 93 +++++++++---- backend/app/main.py | 4 + backend/app/utils/jail_config.py | 93 +++++++++++++ backend/tests/test_routers/test_setup.py | 2 + backend/tests/test_utils/test_jail_config.py | 134 +++++++++++++++++++ 5 files changed, 299 insertions(+), 27 deletions(-) create mode 100644 backend/app/utils/jail_config.py create mode 100644 backend/tests/test_utils/test_jail_config.py diff --git a/Docs/Tasks.md b/Docs/Tasks.md index 4b56645..ebac245 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -4,43 +4,82 @@ This document breaks the entire BanGUI project into development stages, ordered --- -## Stage 0 — First-Run Bootstrap & Startup Fix +## Task: Ensure Required fail2ban Jail Config Files Exist ✅ DONE -These tasks fix a crash-on-first-boot regression and make the setup/login redirect flow reliable. They must be completed before any other feature work because the application cannot start without them. +**Implemented:** `backend/app/utils/jail_config.py` — `ensure_jail_configs(jail_d_path)` creates missing `manual-Jail.conf`, `manual-Jail.local`, `blocklist-import.conf`, and `blocklist-import.local` files with correct default content. Called from the lifespan hook in `main.py` using `Path(settings.fail2ban_config_dir) / "jail.d"`. Tests in `backend/tests/test_utils/test_jail_config.py` (6 cases: all missing, all present, only locals missing, directory creation, idempotency, correct content). + +The backend must guarantee that two specific jail configuration files are present inside the fail2ban jail directory before the application starts (or on first use). If either file is missing it must be created with the correct default content. The files live inside the directory that fail2ban uses for per-jail drop-in configs (e.g. `jail.d/`). The path to that directory should be read from the application settings (config key `fail2ban_jail_d_path` or equivalent); do not hard-code it. + +### Files to create if missing + +**`manual-Jail.conf`** +The file must contain a `[manual-Jail]` section with `enabled = false` and all other jail parameters (filter, logpath, backend, maxretry, findtime, bantime, ignoreip) set to the same defaults already documented in `Docker/fail2ban-dev-config/fail2ban/jail.d/manual-Jail.conf`. Only `enabled` must be forced to `false` in this template — it is the `.local` override (see below) that activates the jail. + +**`blocklist-import.conf`** +The file must contain a `[blocklist-import]` section with `enabled = false` and all other jail parameters (filter, logpath, backend, maxretry, findtime, bantime, ignoreip) set to the same defaults already documented in `Docker/fail2ban-dev-config/fail2ban/jail.d/blocklist-import.conf`. Same rule: `enabled = false` here; the `.local` file enables it. + +### Local override files + +For each `.conf` file above there must also be a corresponding `.local` file checked — and created if missing. The `.local` files must contain **only** the section header and the single `enabled = true` line. Nothing else. fail2ban merges `.local` on top of `.conf` at startup, so all other settings come from the `.conf`. + +``` +[manual-Jail] +enabled = true +``` + +``` +[blocklist-import] +enabled = true +``` + +### Implementation notes + +- Perform the check in a backend startup routine (e.g. in a `lifespan` hook or a dedicated `ensure_jail_configs()` function called from `main.py`). +- Only create a file if it does **not** already exist. Never overwrite an existing file. +- Log an `INFO` message for each file that is created and a `DEBUG` message when a file already exists. +- Add unit tests that exercise: (a) all four files missing → all four created with correct content, (b) all four files present → nothing is overwritten, (c) only the `.local` files missing → only they are created. --- -### Task 0.1 — Fix: Create the database parent directory before connecting ✅ DONE +## Task: World Map — Country Tooltip on Hover -**File:** `backend/app/main.py` +Currently the world map (`WorldMap.tsx`) shows a ban-count label painted directly onto each country's SVG path and reacts to click events. Add a floating tooltip that appears when the user hovers over a country. -**Implemented:** In `_lifespan`, resolve `settings.database_path` to a `Path`, call `.parent.mkdir(parents=True, exist_ok=True)` before `aiosqlite.connect()`. Added `debug`-level structured log line after mkdir. Tests added in `TestLifespanDatabaseDirectoryCreation`. +### Required behaviour + +- When the mouse enters a country geography, display a small floating tooltip near the cursor that shows: + - The country's full name (already available via `country_names` from `useMapData()`). + - The ban count for that country (from the `countries` map; show `0` if the country has no entry). +- The tooltip must follow the mouse while inside the country (or at minimum appear near the cursor when it first enters). +- When the mouse leaves the country the tooltip must disappear. +- Countries with zero bans must also show the tooltip (name + "0 bans"). + +### Implementation notes + +- Store tooltip state (visible, content, x, y) in a `useState` hook local to `WorldMap.tsx`. +- Use `onMouseEnter`, `onMouseMove`, and `onMouseLeave` props on the `` element (react-simple-maps already forwards these as standard SVG mouse events). +- Render the tooltip as an absolutely-positioned `
` overlaid on the map container. Apply a `pointer-events: none` style so it does not interfere with hover detection on the map itself. +- Reuse the existing Fluent UI design tokens (background, border, shadow, typography) so the tooltip matches the rest of the UI. Do not introduce a new third-party tooltip library. +- Add a Vitest / React Testing Library test that mounts `WorldMap` with mock data, fires a `mouseenter` event on a geography, and asserts the tooltip text is visible. --- -### Task 0.2 — Fix: SetupRedirectMiddleware must treat a missing database as "setup not complete" ✅ DONE +## Task: Main Menu — Tooltips on Navigation Items -**File:** `backend/app/main.py` +The main navigation sidebar (or top bar, whichever is used) currently has no tooltips. Add a tooltip to each navigation item so that users who are unfamiliar with icon-only or collapsed menus can see the destination name without navigating. -**Implemented:** Changed the guard in `SetupRedirectMiddleware.dispatch` so that `db is None` also triggers the redirect to `/api/setup`. The condition is now `if db is None or not await setup_service.is_setup_complete(db)`. The `_setup_complete_cached` flag is only set after a successful `is_setup_complete(db)` call with a live `db`. Tests added in `TestSetupRedirectMiddlewareDbNone`. - ---- - -### Task 0.3 — Fix: SetupGuard must redirect to /setup on API errors, not allow through ✅ DONE - -**File:** `frontend/src/components/SetupGuard.tsx` - -**Implemented:** Changed the `.catch()` handler to set `status` to `"pending"` instead of `"done"`. Updated the comment to explain the conservative fallback. Tests added in `SetupGuard.test.tsx`. - ---- - -### Task 0.4 — Fix: SetupPage must redirect to /login when setup is already complete ✅ DONE - -**File:** `frontend/src/pages/SetupPage.tsx` - -**Implemented:** -1. Added `checking` boolean state (initialised to `true`). While `checking` is true, a full-screen `` is rendered instead of the form, preventing the form from flashing. -2. The `useEffect` sets `checking` to `false` in both the `.then()` (when setup is not complete) and the `.catch()` branch. Added a `console.warn` in the catch block. Added a `cancelled` flag and cleanup return to the effect. -Tests added in `SetupPage.test.tsx`. +### Required behaviour + +- Each navigation item must show a tooltip containing the item's label (e.g. "Dashboard", "Map", "Blocklist", "Settings") when the user hovers over it. +- Tooltips should appear after a short delay (≈ 300 ms) to avoid flickering during fast cursor movement past the menu. +- The tooltip must be dismissed when the cursor leaves the item. +- If the menu is already showing a full text label next to the icon, the tooltip is still added (it reinforces accessibility); but consider hiding it when the sidebar is expanded and the label is already visible, to avoid redundancy. + +### Implementation notes + +- Use the Fluent UI `` component (from `@fluentui/react-components`) which is already a project dependency. Wrap each navigation `` (or equivalent element) with ``. +- Keep the tooltip content string co-located with the route definition so that if a label changes in one place it changes everywhere. +- Do not introduce any new npm dependencies. +- Add a Vitest / React Testing Library test that renders the navigation component, triggers a hover on each item, and asserts the correct tooltip text is present in the DOM. --- diff --git a/backend/app/main.py b/backend/app/main.py index 2b2828e..bb25f20 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -49,6 +49,7 @@ from app.routers import ( ) from app.tasks import blocklist_import, geo_cache_flush, geo_re_resolve, health_check from app.utils.fail2ban_client import Fail2BanConnectionError, Fail2BanProtocolError +from app.utils.jail_config import ensure_jail_configs # --------------------------------------------------------------------------- # Ensure the bundled fail2ban package is importable from fail2ban-master/ @@ -137,6 +138,9 @@ async def _lifespan(app: FastAPI) -> AsyncGenerator[None, None]: log.info("bangui_starting_up", database_path=settings.database_path) + # --- Ensure required jail config files are present --- + ensure_jail_configs(Path(settings.fail2ban_config_dir) / "jail.d") + # --- Application database --- db_path: Path = Path(settings.database_path) db_path.parent.mkdir(parents=True, exist_ok=True) diff --git a/backend/app/utils/jail_config.py b/backend/app/utils/jail_config.py new file mode 100644 index 0000000..c6eaf00 --- /dev/null +++ b/backend/app/utils/jail_config.py @@ -0,0 +1,93 @@ +"""Utilities for ensuring required fail2ban jail configuration files exist. + +BanGUI requires two custom jails — ``manual-Jail`` and ``blocklist-import`` +— to be present in the fail2ban ``jail.d`` directory. This module provides +:func:`ensure_jail_configs` which checks each of the four files +(``*.conf`` template + ``*.local`` override) and creates any that are missing +with the correct default content. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +import structlog + +if TYPE_CHECKING: + from pathlib import Path + +log: structlog.stdlib.BoundLogger = structlog.get_logger() + +# --------------------------------------------------------------------------- +# Default file contents +# --------------------------------------------------------------------------- + +_MANUAL_JAIL_CONF = """\ +[manual-Jail] + +enabled = false +filter = manual-Jail +logpath = /remotelogs/bangui/auth.log +backend = polling +maxretry = 3 +findtime = 120 +bantime = 60 +ignoreip = 127.0.0.0/8 ::1 172.16.0.0/12 +""" + +_MANUAL_JAIL_LOCAL = """\ +[manual-Jail] +enabled = true +""" + +_BLOCKLIST_IMPORT_CONF = """\ +[blocklist-import] + +enabled = false +filter = +logpath = /dev/null +backend = auto +maxretry = 1 +findtime = 1d +bantime = 1w +ignoreip = 127.0.0.0/8 ::1 172.16.0.0/12 +""" + +_BLOCKLIST_IMPORT_LOCAL = """\ +[blocklist-import] +enabled = true +""" + +# --------------------------------------------------------------------------- +# File registry: (filename, default_content) +# --------------------------------------------------------------------------- + +_JAIL_FILES: list[tuple[str, str]] = [ + ("manual-Jail.conf", _MANUAL_JAIL_CONF), + ("manual-Jail.local", _MANUAL_JAIL_LOCAL), + ("blocklist-import.conf", _BLOCKLIST_IMPORT_CONF), + ("blocklist-import.local", _BLOCKLIST_IMPORT_LOCAL), +] + + +def ensure_jail_configs(jail_d_path: Path) -> None: + """Ensure the required fail2ban jail configuration files exist. + + Checks for ``manual-Jail.conf``, ``manual-Jail.local``, + ``blocklist-import.conf``, and ``blocklist-import.local`` inside + *jail_d_path*. Any file that is missing is created with its default + content. Existing files are **never** overwritten. + + Args: + jail_d_path: Path to the fail2ban ``jail.d`` directory. Will be + created (including all parents) if it does not already exist. + """ + jail_d_path.mkdir(parents=True, exist_ok=True) + + for filename, default_content in _JAIL_FILES: + file_path = jail_d_path / filename + if file_path.exists(): + log.debug("jail_config_already_exists", path=str(file_path)) + else: + file_path.write_text(default_content, encoding="utf-8") + log.info("jail_config_created", path=str(file_path)) diff --git a/backend/tests/test_routers/test_setup.py b/backend/tests/test_routers/test_setup.py index f66987d..0fc7040 100644 --- a/backend/tests/test_routers/test_setup.py +++ b/backend/tests/test_routers/test_setup.py @@ -330,6 +330,7 @@ class TestLifespanDatabaseDirectoryCreation: patch("app.tasks.geo_cache_flush.register"), patch("app.tasks.geo_re_resolve.register"), patch("app.main.AsyncIOScheduler", return_value=mock_scheduler), + patch("app.main.ensure_jail_configs"), ): async with _lifespan(app): assert nested_db.parent.exists(), ( @@ -372,6 +373,7 @@ class TestLifespanDatabaseDirectoryCreation: patch("app.tasks.geo_cache_flush.register"), patch("app.tasks.geo_re_resolve.register"), patch("app.main.AsyncIOScheduler", return_value=mock_scheduler), + patch("app.main.ensure_jail_configs"), ): # Should not raise FileExistsError or similar. async with _lifespan(app): diff --git a/backend/tests/test_utils/test_jail_config.py b/backend/tests/test_utils/test_jail_config.py new file mode 100644 index 0000000..35f55b6 --- /dev/null +++ b/backend/tests/test_utils/test_jail_config.py @@ -0,0 +1,134 @@ +"""Tests for app.utils.jail_config.ensure_jail_configs.""" + +from __future__ import annotations + +from pathlib import Path + +from app.utils.jail_config import ( + _BLOCKLIST_IMPORT_CONF, + _BLOCKLIST_IMPORT_LOCAL, + _MANUAL_JAIL_CONF, + _MANUAL_JAIL_LOCAL, + ensure_jail_configs, +) + +# --------------------------------------------------------------------------- +# Expected filenames +# --------------------------------------------------------------------------- + +_MANUAL_CONF = "manual-Jail.conf" +_MANUAL_LOCAL = "manual-Jail.local" +_BLOCKLIST_CONF = "blocklist-import.conf" +_BLOCKLIST_LOCAL = "blocklist-import.local" + +_ALL_FILES = [_MANUAL_CONF, _MANUAL_LOCAL, _BLOCKLIST_CONF, _BLOCKLIST_LOCAL] + +_CONTENT_MAP: dict[str, str] = { + _MANUAL_CONF: _MANUAL_JAIL_CONF, + _MANUAL_LOCAL: _MANUAL_JAIL_LOCAL, + _BLOCKLIST_CONF: _BLOCKLIST_IMPORT_CONF, + _BLOCKLIST_LOCAL: _BLOCKLIST_IMPORT_LOCAL, +} + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _read(jail_d: Path, filename: str) -> str: + return (jail_d / filename).read_text(encoding="utf-8") + + +# --------------------------------------------------------------------------- +# Tests: ensure_jail_configs +# --------------------------------------------------------------------------- + + +class TestEnsureJailConfigs: + def test_all_missing_creates_all_four(self, tmp_path: Path) -> None: + """All four files are created when the directory is empty.""" + jail_d = tmp_path / "jail.d" + ensure_jail_configs(jail_d) + + for name in _ALL_FILES: + assert (jail_d / name).exists(), f"{name} should have been created" + assert _read(jail_d, name) == _CONTENT_MAP[name] + + def test_all_missing_creates_correct_content(self, tmp_path: Path) -> None: + """Each created file has exactly the expected default content.""" + jail_d = tmp_path / "jail.d" + ensure_jail_configs(jail_d) + + # .conf files must set enabled = false + for conf_file in (_MANUAL_CONF, _BLOCKLIST_CONF): + content = _read(jail_d, conf_file) + assert "enabled = false" in content + + # .local files must set enabled = true and nothing else + for local_file in (_MANUAL_LOCAL, _BLOCKLIST_LOCAL): + content = _read(jail_d, local_file) + assert "enabled = true" in content + + def test_all_present_overwrites_nothing(self, tmp_path: Path) -> None: + """Existing files are never overwritten.""" + jail_d = tmp_path / "jail.d" + jail_d.mkdir() + + sentinel = "# EXISTING CONTENT — must not be replaced\n" + for name in _ALL_FILES: + (jail_d / name).write_text(sentinel, encoding="utf-8") + + ensure_jail_configs(jail_d) + + for name in _ALL_FILES: + assert _read(jail_d, name) == sentinel, ( + f"{name} should not have been overwritten" + ) + + def test_only_local_files_missing_creates_only_locals( + self, tmp_path: Path + ) -> None: + """Only the .local files are created when the .conf files already exist.""" + jail_d = tmp_path / "jail.d" + jail_d.mkdir() + + sentinel = "# pre-existing conf\n" + for conf_file in (_MANUAL_CONF, _BLOCKLIST_CONF): + (jail_d / conf_file).write_text(sentinel, encoding="utf-8") + + ensure_jail_configs(jail_d) + + # .conf files must remain unchanged + for conf_file in (_MANUAL_CONF, _BLOCKLIST_CONF): + assert _read(jail_d, conf_file) == sentinel + + # .local files must have been created with correct content + for local_file, expected in ( + (_MANUAL_LOCAL, _MANUAL_JAIL_LOCAL), + (_BLOCKLIST_LOCAL, _BLOCKLIST_IMPORT_LOCAL), + ): + assert (jail_d / local_file).exists(), f"{local_file} should have been created" + assert _read(jail_d, local_file) == expected + + def test_creates_jail_d_directory_if_missing(self, tmp_path: Path) -> None: + """The jail.d directory is created automatically when absent.""" + jail_d = tmp_path / "nested" / "jail.d" + assert not jail_d.exists() + ensure_jail_configs(jail_d) + assert jail_d.is_dir() + + def test_idempotent_on_repeated_calls(self, tmp_path: Path) -> None: + """Calling ensure_jail_configs twice does not alter any file.""" + jail_d = tmp_path / "jail.d" + ensure_jail_configs(jail_d) + + # Record content after first call + first_pass = {name: _read(jail_d, name) for name in _ALL_FILES} + + ensure_jail_configs(jail_d) + + for name in _ALL_FILES: + assert _read(jail_d, name) == first_pass[name], ( + f"{name} changed on second call" + ) -- 2.49.1 From abb224e01bf0c314dd7ef31cf21a5c2cb4d75af9 Mon Sep 17 00:00:00 2001 From: Lukas Date: Mon, 16 Mar 2026 19:22:16 +0100 Subject: [PATCH 13/15] Docker: add PUID/PGID env vars, fix env format, add release script and VERSION --- Docker/VERSION | 1 + Docker/docker-compose.yml | 17 ++++++---- Docker/release.sh | 69 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 6 deletions(-) create mode 100644 Docker/VERSION create mode 100644 Docker/release.sh diff --git a/Docker/VERSION b/Docker/VERSION new file mode 100644 index 0000000..188bef5 --- /dev/null +++ b/Docker/VERSION @@ -0,0 +1 @@ +v0.9.3 diff --git a/Docker/docker-compose.yml b/Docker/docker-compose.yml index 907f14d..70ced48 100644 --- a/Docker/docker-compose.yml +++ b/Docker/docker-compose.yml @@ -35,12 +35,14 @@ services: fail2ban: condition: service_started environment: - BANGUI_DATABASE_PATH: "/data/bangui.db" - BANGUI_FAIL2BAN_SOCKET: "/var/run/fail2ban/fail2ban.sock" - BANGUI_FAIL2BAN_CONFIG_DIR: "/config/fail2ban" - BANGUI_LOG_LEVEL: "info" - BANGUI_SESSION_SECRET: "${BANGUI_SESSION_SECRET:?Set BANGUI_SESSION_SECRET}" - BANGUI_TIMEZONE: "${BANGUI_TIMEZONE:-UTC}" + - PUID=1011 + - PGID=1001 + - BANGUI_DATABASE_PATH=/data/bangui.db + - BANGUI_FAIL2BAN_SOCKET=/var/run/fail2ban/fail2ban.sock + - BANGUI_FAIL2BAN_CONFIG_DIR=/config/fail2ban + - BANGUI_LOG_LEVEL=info + - BANGUI_SESSION_SECRET=${BANGUI_SESSION_SECRET:?Set BANGUI_SESSION_SECRET} + - BANGUI_TIMEZONE=${BANGUI_TIMEZONE:-UTC} volumes: - /server/server_fail2ban/bangui-data:/data - /server/server_fail2ban/fail2ban-run:/var/run/fail2ban:ro @@ -55,6 +57,9 @@ services: image: git.lpl-mind.de/lukas.pupkalipinski/bangui/frontend:latest container_name: bangui-frontend restart: unless-stopped + environment: + - PUID=1011 + - PGID=1001 ports: - "${BANGUI_PORT:-8080}:80" depends_on: diff --git a/Docker/release.sh b/Docker/release.sh new file mode 100644 index 0000000..fc38642 --- /dev/null +++ b/Docker/release.sh @@ -0,0 +1,69 @@ +#!/usr/bin/env bash +# +# Bump the project version and push images to the registry. +# +# Usage: +# ./release.sh +# +# The current version is stored in VERSION (next to this script). +# You will be asked whether to bump major, minor, or patch. + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +VERSION_FILE="${SCRIPT_DIR}/VERSION" + +# --------------------------------------------------------------------------- +# Read current version +# --------------------------------------------------------------------------- +if [[ ! -f "${VERSION_FILE}" ]]; then + echo "0.0.0" > "${VERSION_FILE}" +fi + +CURRENT="$(cat "${VERSION_FILE}")" +# Strip leading 'v' for arithmetic +VERSION="${CURRENT#v}" + +IFS='.' read -r MAJOR MINOR PATCH <<< "${VERSION}" + +echo "============================================" +echo " BanGUI — Release" +echo " Current version: v${MAJOR}.${MINOR}.${PATCH}" +echo "============================================" +echo "" +echo "How would you like to bump the version?" +echo " 1) patch (v${MAJOR}.${MINOR}.${PATCH} → v${MAJOR}.${MINOR}.$((PATCH + 1)))" +echo " 2) minor (v${MAJOR}.${MINOR}.${PATCH} → v${MAJOR}.$((MINOR + 1)).0)" +echo " 3) major (v${MAJOR}.${MINOR}.${PATCH} → v$((MAJOR + 1)).0.0)" +echo "" +read -rp "Enter choice [1/2/3]: " CHOICE + +case "${CHOICE}" in + 1) NEW_TAG="v${MAJOR}.${MINOR}.$((PATCH + 1))" ;; + 2) NEW_TAG="v${MAJOR}.$((MINOR + 1)).0" ;; + 3) NEW_TAG="v$((MAJOR + 1)).0.0" ;; + *) + echo "Invalid choice. Aborting." >&2 + exit 1 + ;; +esac + +echo "" +echo "New version: ${NEW_TAG}" +read -rp "Confirm? [y/N]: " CONFIRM +if [[ ! "${CONFIRM}" =~ ^[yY]$ ]]; then + echo "Aborted." + exit 0 +fi + +# --------------------------------------------------------------------------- +# Write new version +# --------------------------------------------------------------------------- +echo "${NEW_TAG}" > "${VERSION_FILE}" +echo "Version file updated → ${VERSION_FILE}" + +# --------------------------------------------------------------------------- +# Push +# --------------------------------------------------------------------------- +bash "${SCRIPT_DIR}/push.sh" "${NEW_TAG}" +bash "${SCRIPT_DIR}/push.sh" -- 2.49.1 From e7834a888ef42d3ab206a152e921971b97d53041 Mon Sep 17 00:00:00 2001 From: Lukas Date: Mon, 16 Mar 2026 19:45:55 +0100 Subject: [PATCH 14/15] Show BanGUI app version in sidebar, fix version tooltips MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Inject __APP_VERSION__ at build time via vite.config.ts define (reads frontend/package.json#version); declare the global in vite-env.d.ts. - Render 'BanGUI v{__APP_VERSION__}' in the sidebar footer (MainLayout) when expanded; hidden when collapsed. - Rename fail2ban version tooltip to 'fail2ban daemon version' in ServerStatusBar so it is visually distinct from the app version. - Sync frontend/package.json version (0.9.0 → 0.9.3) to match Docker/VERSION; update release.sh to keep them in sync on every bump. - Add vitest define stub for __APP_VERSION__ so tests compile cleanly. - Add ServerStatusBar and MainLayout test suites (10 new test cases). --- Docker/release.sh | 6 + Docs/Tasks.md | 84 ++++------ frontend/package.json | 2 +- frontend/src/components/ServerStatusBar.tsx | 2 +- .../__tests__/ServerStatusBar.test.tsx | 151 ++++++++++++++++++ frontend/src/layouts/MainLayout.tsx | 15 ++ .../src/layouts/__tests__/MainLayout.test.tsx | 78 +++++++++ frontend/src/vite-env.d.ts | 3 + frontend/vite.config.ts | 9 ++ frontend/vitest.config.ts | 4 + 10 files changed, 295 insertions(+), 59 deletions(-) create mode 100644 frontend/src/components/__tests__/ServerStatusBar.test.tsx create mode 100644 frontend/src/layouts/__tests__/MainLayout.test.tsx diff --git a/Docker/release.sh b/Docker/release.sh index fc38642..f275025 100644 --- a/Docker/release.sh +++ b/Docker/release.sh @@ -62,6 +62,12 @@ fi echo "${NEW_TAG}" > "${VERSION_FILE}" echo "Version file updated → ${VERSION_FILE}" +# Keep frontend/package.json in sync so __APP_VERSION__ matches Docker/VERSION. +FRONT_VERSION="${NEW_TAG#v}" +FRONT_PKG="${SCRIPT_DIR}/../frontend/package.json" +sed -i "s/\"version\": \"[^\"]*\"/\"version\": \"${FRONT_VERSION}\"/" "${FRONT_PKG}" +echo "frontend/package.json version updated → ${FRONT_VERSION}" + # --------------------------------------------------------------------------- # Push # --------------------------------------------------------------------------- diff --git a/Docs/Tasks.md b/Docs/Tasks.md index ebac245..bec20ef 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -4,82 +4,52 @@ This document breaks the entire BanGUI project into development stages, ordered --- -## Task: Ensure Required fail2ban Jail Config Files Exist ✅ DONE +## Open Issues -**Implemented:** `backend/app/utils/jail_config.py` — `ensure_jail_configs(jail_d_path)` creates missing `manual-Jail.conf`, `manual-Jail.local`, `blocklist-import.conf`, and `blocklist-import.local` files with correct default content. Called from the lifespan hook in `main.py` using `Path(settings.fail2ban_config_dir) / "jail.d"`. Tests in `backend/tests/test_utils/test_jail_config.py` (6 cases: all missing, all present, only locals missing, directory creation, idempotency, correct content). +### ~~1. Dashboard — Version Tag Mismatch~~ ✅ Done -The backend must guarantee that two specific jail configuration files are present inside the fail2ban jail directory before the application starts (or on first use). If either file is missing it must be created with the correct default content. The files live inside the directory that fail2ban uses for per-jail drop-in configs (e.g. `jail.d/`). The path to that directory should be read from the application settings (config key `fail2ban_jail_d_path` or equivalent); do not hard-code it. +**Implemented:** +- `frontend/vite.config.ts`: reads `package.json#version` at build time and injects it as the global `__APP_VERSION__` via Vite `define`. +- `frontend/src/vite-env.d.ts`: adds `declare const __APP_VERSION__: string` so TypeScript knows about the global. +- `frontend/src/layouts/MainLayout.tsx`: renders `BanGUI v{__APP_VERSION__}` in the sidebar footer when expanded (hidden when collapsed). +- `frontend/src/components/ServerStatusBar.tsx`: tooltip changed from `"fail2ban version"` to `"fail2ban daemon version"`. +- `Docker/release.sh`: after bumping `VERSION`, also updates `frontend/package.json#version` via `sed` to keep them in sync. +- `frontend/package.json`: version bumped from `0.9.0` to `0.9.3` to match `Docker/VERSION`. +- Tests added: `src/components/__tests__/ServerStatusBar.test.tsx`, `src/layouts/__tests__/MainLayout.test.tsx`. -### Files to create if missing +**Problem:** The `ServerStatusBar` component on the Dashboard displays `v{status.version}`, which is the **fail2ban daemon version** (e.g. `v1.1.0`). The BanGUI application version lives in `Docker/VERSION` (e.g. `v0.9.3`) and is unrelated to the fail2ban version. Users see a version number they don't recognise and assume it reflects the BanGUI release. -**`manual-Jail.conf`** -The file must contain a `[manual-Jail]` section with `enabled = false` and all other jail parameters (filter, logpath, backend, maxretry, findtime, bantime, ignoreip) set to the same defaults already documented in `Docker/fail2ban-dev-config/fail2ban/jail.d/manual-Jail.conf`. Only `enabled` must be forced to `false` in this template — it is the `.local` override (see below) that activates the jail. +**Goal:** Make the distinction clear and expose the BanGUI application version. -**`blocklist-import.conf`** -The file must contain a `[blocklist-import]` section with `enabled = false` and all other jail parameters (filter, logpath, backend, maxretry, findtime, bantime, ignoreip) set to the same defaults already documented in `Docker/fail2ban-dev-config/fail2ban/jail.d/blocklist-import.conf`. Same rule: `enabled = false` here; the `.local` file enables it. +**Suggested approach:** +1. Inject the BanGUI app version at build time — add a `define` entry in `frontend/vite.config.ts` that reads the `version` field from `frontend/package.json` (e.g. `__APP_VERSION__`). Keep `frontend/package.json` and `Docker/VERSION` in sync (update the release script `Docker/release.sh` or `Makefile` to write `package.json#version` from `VERSION`). +2. Show the BanGUI version in the sidebar footer inside `MainLayout.tsx` (collapsed view: show only when expanded, or via tooltip). This is the natural place for an "about" version tag. +3. Update the fail2ban version tooltip in `ServerStatusBar.tsx` from the generic `"fail2ban version"` to something like `"fail2ban daemon version"` so the two are no longer visually indistinguishable. -### Local override files - -For each `.conf` file above there must also be a corresponding `.local` file checked — and created if missing. The `.local` files must contain **only** the section header and the single `enabled = true` line. Nothing else. fail2ban merges `.local` on top of `.conf` at startup, so all other settings come from the `.conf`. - -``` -[manual-Jail] -enabled = true -``` - -``` -[blocklist-import] -enabled = true -``` - -### Implementation notes - -- Perform the check in a backend startup routine (e.g. in a `lifespan` hook or a dedicated `ensure_jail_configs()` function called from `main.py`). -- Only create a file if it does **not** already exist. Never overwrite an existing file. -- Log an `INFO` message for each file that is created and a `DEBUG` message when a file already exists. -- Add unit tests that exercise: (a) all four files missing → all four created with correct content, (b) all four files present → nothing is overwritten, (c) only the `.local` files missing → only they are created. +**Files:** `frontend/vite.config.ts`, `frontend/package.json`, `Docker/VERSION`, `Docker/release.sh`, `frontend/src/layouts/MainLayout.tsx`, `frontend/src/components/ServerStatusBar.tsx`. --- -## Task: World Map — Country Tooltip on Hover +### 2. Dashboard — Improve "Failures" Tooltip -Currently the world map (`WorldMap.tsx`) shows a ban-count label painted directly onto each country's SVG path and reacts to click events. Add a floating tooltip that appears when the user hovers over a country. +**Problem:** The `ServerStatusBar` shows a "Failures: 42" counter with the tooltip `"Currently failing IPs"`. In fail2ban terminology *failures* are individual **failed authentication attempts** tracked in the fail2ban DB, not the number of unique IPs that failed. The current wording is ambiguous and misleading — users may think it means broken connections or error states. -### Required behaviour +**Goal:** Replace the tooltip with accurate, self-explanatory wording. -- When the mouse enters a country geography, display a small floating tooltip near the cursor that shows: - - The country's full name (already available via `country_names` from `useMapData()`). - - The ban count for that country (from the `countries` map; show `0` if the country has no entry). -- The tooltip must follow the mouse while inside the country (or at minimum appear near the cursor when it first enters). -- When the mouse leaves the country the tooltip must disappear. -- Countries with zero bans must also show the tooltip (name + "0 bans"). +**Suggested fix:** Change the `Tooltip` content for the Failures stat in `ServerStatusBar.tsx` from `"Currently failing IPs"` to something like `"Total failed authentication attempts currently tracked by fail2ban across all active jails"`. Additionally, consider renaming the label from `"Failures:"` to `"Failed Attempts:"` to match the tooltip language. -### Implementation notes - -- Store tooltip state (visible, content, x, y) in a `useState` hook local to `WorldMap.tsx`. -- Use `onMouseEnter`, `onMouseMove`, and `onMouseLeave` props on the `` element (react-simple-maps already forwards these as standard SVG mouse events). -- Render the tooltip as an absolutely-positioned `
` overlaid on the map container. Apply a `pointer-events: none` style so it does not interfere with hover detection on the map itself. -- Reuse the existing Fluent UI design tokens (background, border, shadow, typography) so the tooltip matches the rest of the UI. Do not introduce a new third-party tooltip library. -- Add a Vitest / React Testing Library test that mounts `WorldMap` with mock data, fires a `mouseenter` event on a geography, and asserts the tooltip text is visible. +**Files:** `frontend/src/components/ServerStatusBar.tsx`. --- -## Task: Main Menu — Tooltips on Navigation Items +### 3. Config → Server Tab — Move "Service Health" to Top -The main navigation sidebar (or top bar, whichever is used) currently has no tooltips. Add a tooltip to each navigation item so that users who are unfamiliar with icon-only or collapsed menus can see the destination name without navigating. +**Problem:** In the Config page → Server tab, the `Service Health` panel (`ServerHealthSection`) is rendered at the bottom of the tab, after all settings sections (log level, log target, DB purge settings, map thresholds, reload/restart buttons). This means users must scroll past all editable fields to check service connectivity status, even though the health status is the most critical piece of context — it indicates whether the server is reachable at all. -### Required behaviour +**Goal:** Move the `` block to the **top** of the `ServerTab` render output, before any settings fields. -- Each navigation item must show a tooltip containing the item's label (e.g. "Dashboard", "Map", "Blocklist", "Settings") when the user hovers over it. -- Tooltips should appear after a short delay (≈ 300 ms) to avoid flickering during fast cursor movement past the menu. -- The tooltip must be dismissed when the cursor leaves the item. -- If the menu is already showing a full text label next to the icon, the tooltip is still added (it reinforces accessibility); but consider hiding it when the sidebar is expanded and the label is already visible, to avoid redundancy. +**Suggested fix:** In `frontend/src/components/config/ServerTab.tsx`, move the `{/* Service Health & Log Viewer section */}` block (currently at the end of the JSX return around line 415) to be the first section rendered inside the tab container. -### Implementation notes - -- Use the Fluent UI `` component (from `@fluentui/react-components`) which is already a project dependency. Wrap each navigation `` (or equivalent element) with ``. -- Keep the tooltip content string co-located with the route definition so that if a label changes in one place it changes everywhere. -- Do not introduce any new npm dependencies. -- Add a Vitest / React Testing Library test that renders the navigation component, triggers a hover on each item, and asserts the correct tooltip text is present in the DOM. +**Files:** `frontend/src/components/config/ServerTab.tsx`. --- diff --git a/frontend/package.json b/frontend/package.json index 4c6b160..91423a0 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,7 +1,7 @@ { "name": "bangui-frontend", "private": true, - "version": "0.9.0", + "version": "0.9.3", "description": "BanGUI frontend — fail2ban web management interface", "type": "module", "scripts": { diff --git a/frontend/src/components/ServerStatusBar.tsx b/frontend/src/components/ServerStatusBar.tsx index 4953abf..5b7c287 100644 --- a/frontend/src/components/ServerStatusBar.tsx +++ b/frontend/src/components/ServerStatusBar.tsx @@ -109,7 +109,7 @@ export function ServerStatusBar(): React.JSX.Element { {/* Version */} {/* ---------------------------------------------------------------- */} {status?.version != null && ( - + v{status.version} diff --git a/frontend/src/components/__tests__/ServerStatusBar.test.tsx b/frontend/src/components/__tests__/ServerStatusBar.test.tsx new file mode 100644 index 0000000..87d748d --- /dev/null +++ b/frontend/src/components/__tests__/ServerStatusBar.test.tsx @@ -0,0 +1,151 @@ +/** + * Tests for the ServerStatusBar component. + * + * Covers loading state, online / offline rendering, and correct tooltip + * wording that distinguishes the fail2ban daemon version from the BanGUI + * application version. + */ + +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { render, screen } from "@testing-library/react"; +import { FluentProvider, webLightTheme } from "@fluentui/react-components"; +import { ServerStatusBar } from "../ServerStatusBar"; + +// --------------------------------------------------------------------------- +// Mock useServerStatus so tests never touch the network. +// --------------------------------------------------------------------------- + +vi.mock("../../hooks/useServerStatus"); + +import { useServerStatus } from "../../hooks/useServerStatus"; + +const mockedUseServerStatus = vi.mocked(useServerStatus); + +function renderBar(): void { + render( + + + , + ); +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe("ServerStatusBar", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("shows a spinner while the initial load is in progress", () => { + mockedUseServerStatus.mockReturnValue({ + status: null, + loading: true, + error: null, + refresh: vi.fn(), + }); + renderBar(); + // The status-area spinner is labelled "Checking\u2026". + expect(screen.getByText("Checking\u2026")).toBeInTheDocument(); + }); + + it("renders an Online badge when the server is reachable", () => { + mockedUseServerStatus.mockReturnValue({ + status: { + online: true, + version: "1.1.0", + active_jails: 3, + total_bans: 10, + total_failures: 5, + }, + loading: false, + error: null, + refresh: vi.fn(), + }); + renderBar(); + expect(screen.getByText("Online")).toBeInTheDocument(); + }); + + it("renders an Offline badge when the server is unreachable", () => { + mockedUseServerStatus.mockReturnValue({ + status: { + online: false, + version: null, + active_jails: 0, + total_bans: 0, + total_failures: 0, + }, + loading: false, + error: null, + refresh: vi.fn(), + }); + renderBar(); + expect(screen.getByText("Offline")).toBeInTheDocument(); + }); + + it("displays the daemon version string when available", () => { + mockedUseServerStatus.mockReturnValue({ + status: { + online: true, + version: "1.2.3", + active_jails: 1, + total_bans: 0, + total_failures: 0, + }, + loading: false, + error: null, + refresh: vi.fn(), + }); + renderBar(); + expect(screen.getByText("v1.2.3")).toBeInTheDocument(); + }); + + it("does not render the version element when version is null", () => { + mockedUseServerStatus.mockReturnValue({ + status: { + online: false, + version: null, + active_jails: 0, + total_bans: 0, + total_failures: 0, + }, + loading: false, + error: null, + refresh: vi.fn(), + }); + renderBar(); + // No version string should appear in the document. + expect(screen.queryByText(/^v\d/)).not.toBeInTheDocument(); + }); + + it("shows jail / ban / failure counts when the server is online", () => { + mockedUseServerStatus.mockReturnValue({ + status: { + online: true, + version: "1.0.0", + active_jails: 4, + total_bans: 21, + total_failures: 99, + }, + loading: false, + error: null, + refresh: vi.fn(), + }); + renderBar(); + expect(screen.getByText("4")).toBeInTheDocument(); + expect(screen.getByText("21")).toBeInTheDocument(); + expect(screen.getByText("99")).toBeInTheDocument(); + }); + + it("renders an error message when the status fetch fails", () => { + mockedUseServerStatus.mockReturnValue({ + status: null, + loading: false, + error: "Network error", + refresh: vi.fn(), + }); + renderBar(); + expect(screen.getByText("Network error")).toBeInTheDocument(); + }); +}); diff --git a/frontend/src/layouts/MainLayout.tsx b/frontend/src/layouts/MainLayout.tsx index 2f2ffd2..305a476 100644 --- a/frontend/src/layouts/MainLayout.tsx +++ b/frontend/src/layouts/MainLayout.tsx @@ -145,6 +145,16 @@ const useStyles = makeStyles({ padding: tokens.spacingVerticalS, flexShrink: 0, }, + versionText: { + display: "block", + color: tokens.colorNeutralForeground4, + fontSize: "11px", + paddingLeft: tokens.spacingHorizontalS, + paddingRight: tokens.spacingHorizontalS, + paddingBottom: tokens.spacingVerticalXS, + whiteSpace: "nowrap", + overflow: "hidden", + }, // Main content main: { @@ -301,6 +311,11 @@ export function MainLayout(): React.JSX.Element { {/* Footer — Logout */}
+ {!collapsed && ( + + BanGUI v{__APP_VERSION__} + + )}
- +
- Failures: + Failed Attempts: {status.total_failures} diff --git a/frontend/src/components/__tests__/ServerStatusBar.test.tsx b/frontend/src/components/__tests__/ServerStatusBar.test.tsx index 87d748d..a827bf8 100644 --- a/frontend/src/components/__tests__/ServerStatusBar.test.tsx +++ b/frontend/src/components/__tests__/ServerStatusBar.test.tsx @@ -136,6 +136,8 @@ describe("ServerStatusBar", () => { expect(screen.getByText("4")).toBeInTheDocument(); expect(screen.getByText("21")).toBeInTheDocument(); expect(screen.getByText("99")).toBeInTheDocument(); + // Verify the "Failed Attempts:" label (renamed from "Failures:"). + expect(screen.getByText("Failed Attempts:")).toBeInTheDocument(); }); it("renders an error message when the status fetch fails", () => { diff --git a/frontend/src/components/config/ServerTab.tsx b/frontend/src/components/config/ServerTab.tsx index addc3d8..2937ac4 100644 --- a/frontend/src/components/config/ServerTab.tsx +++ b/frontend/src/components/config/ServerTab.tsx @@ -219,6 +219,10 @@ export function ServerTab(): React.JSX.Element { return (
+ {/* Service Health & Log Viewer section — shown first so users can + immediately see whether fail2ban is reachable before editing settings. */} + +
) : null} - {/* Service Health & Log Viewer section */} -
); } -- 2.49.1