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.
This commit is contained in:
153
Docs/Tasks.md
153
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 <IP>`) 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 ? (
|
||||
<JailConfigDetail
|
||||
jail={selectedActiveJail} // no key prop
|
||||
...
|
||||
/>
|
||||
) : selectedInactiveJail !== undefined ? (
|
||||
<InactiveJailDetail
|
||||
jail={selectedInactiveJail} // no key prop
|
||||
...
|
||||
/>
|
||||
) : 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
|
||||
<ActionDetail
|
||||
key={selectedAction.name} // ← forces remount on action change
|
||||
action={selectedAction}
|
||||
...
|
||||
/>
|
||||
```
|
||||
|
||||
**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 ? (
|
||||
<JailConfigDetail
|
||||
key={selectedActiveJail.name}
|
||||
jail={selectedActiveJail}
|
||||
onSave={updateJail}
|
||||
onDeactivate={() => { handleDeactivate(selectedActiveJail.name); }}
|
||||
/>
|
||||
) : selectedInactiveJail !== undefined ? (
|
||||
<InactiveJailDetail
|
||||
key={selectedInactiveJail.name}
|
||||
jail={selectedInactiveJail}
|
||||
onActivate={() => { 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 `<Spinner>` 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`.
|
||||
|
||||
---
|
||||
|
||||
Reference in New Issue
Block a user