Fix vertical alignment of DNS Mode dropdown in jail config

Add alignItems: "end" to the fieldRow grid style so that all grid
cells align their content to the bottom edge of the row. This ensures
the DNS Mode <Select> and the Date Pattern <Combobox> sit on the same
horizontal baseline even though Date Pattern carries a hint line that
makes it taller.

All other fieldRow usages have consistent hint presence across their
fields, so no visual regressions are introduced.
This commit is contained in:
2026-03-14 09:51:00 +01:00
parent c110352e9e
commit 3e4f688484
2 changed files with 31 additions and 185 deletions

View File

@@ -4,203 +4,48 @@ This document breaks the entire BanGUI project into development stages, ordered
--- ---
## Task 1 — Convert Backend, Log Encoding, and Date Pattern to Dropdowns in Jail Config ## Task 1 — Fix vertical alignment of "DNS Mode" dropdown with "Date Pattern" dropdown in Jail Configuration ✅ DONE
**Completed.** Backend and Log Encoding are now `<Select>` dropdowns; Date Pattern is a `<Combobox>` with presets. Both are wired to auto-save. The `JailConfigDetail` component also gained a `readOnly` prop (wired throughout all fields, toggles, and buttons) in preparation for Task 3. Backend updated `JailConfigUpdate` model to accept `backend` and `log_encoding`, and `update_jail_config` sends them to the daemon. **Status:** Completed — Added `alignItems: "end"` to `fieldRow` in `configStyles.ts`. The two dropdowns now baseline-align to the bottom of their grid row regardless of the "Date Pattern" hint text. Verified no regressions in any other `fieldRow` usages (all other rows have consistent hint presence across their fields).
--- **Component:** `frontend/src/components/config/JailsTab.tsx`
**Styles:** `frontend/src/components/config/configStyles.ts`
## ✅ Task 2 — Fix Raw Action Configuration Always Blank in Actions Tab ### Problem
**Completed.** Added `key={selectedAction.name}` to `<ActionDetail>` in `ActionsTab.tsx` and `key={selectedFilter.name}` to `<FilterDetail>` in `FiltersTab.tsx`. This forces a full remount on item switch, resetting `RawConfigSection`'s `loadedRef` so it re-fetches content for every selected item. In the jail configuration detail view (`JailConfigDetail`), the "Date Pattern" and "DNS Mode" fields sit side-by-side in a 2-column CSS grid row (class `fieldRow`). The "Date Pattern" `<Field>` has a `hint` prop (`"Leave blank for auto-detect."`) which renders extra text between the label and the input, pushing the `<Combobox>` control downward. The "DNS Mode" `<Field>` has no `hint`, so its `<Select>` control sits higher. This causes the two dropdowns to be vertically misaligned.
--- ### Location in code
## ✅ Task 3 — Give Inactive Jail Configs the Same GUI as Active Ones - Around **line 321** of `JailsTab.tsx` there is a `<div className={styles.fieldRow}>` that contains both fields.
- The "Date Pattern" field (lines ~322341) uses `<Combobox>` with a `hint` prop.
- The "DNS Mode" field (lines ~342353) uses `<Select>` without a `hint` prop.
**Completed.** Extended `InactiveJail` backend Pydantic model with all config fields (`ban_time_seconds`, `find_time_seconds`, `log_encoding`, `backend`, `date_pattern`, `use_dns`, `prefregex`, `fail_regex`, `ignore_regex`, `bantime_escalation`). Added `_parse_time_to_seconds` helper to `config_file_service.py` and updated `_build_inactive_jail` to populate all new fields. Extended the `InactiveJail` TypeScript type to match. Rewrote `InactiveJailDetail` to map fields into a `JailConfig`-compatible object and render `JailConfigDetail` with `readOnly=true`, showing filter/port/source_file as informational fields above the shared form. ### Acceptance criteria
--- 1. The bottom edges of the "Date Pattern" dropdown and the "DNS Mode" dropdown must be visually aligned on the same horizontal line.
2. The fix must not break the responsive layout (on narrow screens the grid collapses to a single column via `@media (max-width: 900px)`).
3. No other fields in the jail config form should be affected.
## ✅ Task 4 — Fix `banaction` Interpolation Error When Activating Jails ### Suggested approach
**Completed.** `_write_local_override_sync` now always writes `banaction = iptables-multiport` and `banaction_allports = iptables-allports` into the per-jail `.local` file (Option A), ensuring fail2ban can resolve the `%(banaction)s` interpolation used in `action_`. **Option A — Align grid items to the end (preferred):**
In `configStyles.ts`, add `alignItems: "end"` to the `fieldRow` style. This makes each grid cell align its content to the bottom, so the two inputs line up regardless of whether one field has a hint and the other doesn't. Verify that this does not break other rows that also use `fieldRow` (Backend/Log Encoding row, and any rows in `DefaultsTab.tsx` or `FiltersTab.tsx`).
**Option B — Add a matching hint to DNS Mode:**
Add a `hint` with a non-breaking space (`hint={"\u00A0"}`) to the DNS Mode `<Field>` so it takes up the same vertical space as the Date Pattern hint. This is simpler but slightly hacky.
### Files to modify
**Problem:** In the Config → Jails page, the **Backend**, **Log Encoding**, and **Date Pattern** fields for active jails are currently plain text `<Input>` fields (Backend and Log Encoding are read-only, Date Pattern is a free-text input). These should be `<Select>` dropdowns so users pick from valid options instead of typing arbitrary values. | File | Change |
|------|--------|
| `frontend/src/components/config/configStyles.ts` | Add `alignItems: "end"` to `fieldRow` (Option A) |
| — *or* — | |
| `frontend/src/components/config/JailsTab.tsx` | Add `hint` to DNS Mode `<Field>` (Option B) |
**Where to change:** ### Testing
- **`frontend/src/components/config/JailsTab.tsx`** — the `JailConfigDetail` component, around lines 250270. This is the right-pane form for active jails. Replace the three `<Input>` fields with `<Select>` dropdowns.
**Dropdown options to use:**
### Backend
Reference: `fail2ban-master/config/jail.conf` lines 115132. The `JailFileForm.tsx` already defines a `BACKENDS` constant at line 163 — reuse or mirror it.
| Value | Label |
|-------------|----------------------------------------------|
| `auto` | auto — pyinotify, then polling |
| `polling` | polling — standard polling algorithm |
| `pyinotify` | pyinotify — requires pyinotify library |
| `systemd` | systemd — uses systemd journal |
| `gamin` | gamin — legacy file alteration monitor |
### Log Encoding
Reference: `fail2ban-master/config/jail.conf` lines 145150. The option `auto` uses the system locale; the rest are standard Python codec names.
| Value | Label |
|----------|---------------------------------------|
| `auto` | auto — use system locale |
| `ascii` | ascii |
| `utf-8` | utf-8 |
| `latin-1` | latin-1 (ISO 8859-1) |
### Date Pattern
Date Pattern should remain a **combobox** (editable dropdown) since users may enter custom patterns, but offer common presets as suggestions.
| Value | Label |
|--------------------------------|---------------------------------------|
| *(empty)* | auto-detect (leave blank) |
| `{^LN-BEG}` | {^LN-BEG} — line beginning |
| `%%Y-%%m-%%d %%H:%%M:%%S` | YYYY-MM-DD HH:MM:SS |
| `%%d/%%b/%%Y:%%H:%%M:%%S` | DD/Mon/YYYY:HH:MM:SS (Apache) |
| `%%b %%d %%H:%%M:%%S` | Mon DD HH:MM:SS (syslog) |
| `EPOCH` | EPOCH — Unix timestamp |
| `%%Y-%%m-%%dT%%H:%%M:%%S` | ISO 8601 |
| `TAI64N` | TAI64N |
**Implementation notes:**
1. Backend and Log Encoding are currently `readOnly`. They must become editable `<Select>` fields. Wire their `onChange` to local state and include them in the auto-save payload sent to the backend.
2. The backend model at `backend/app/models/config.py` line 67 already accepts `backend: str` and `log_encoding: str`, so no backend changes are needed unless you want to add server-side validation.
3. Date Pattern should use a Fluent UI `Combobox` (editable dropdown) instead of a `Select`, so users can still type a custom pattern.
4. Follow the existing DNS Mode dropdown pattern at lines 271280 of `JailsTab.tsx` as a reference for styling and onChange handling.
---
## Task 2 — Fix Raw Action Configuration Always Blank in Actions Tab
**Problem:** In the Config → Actions page, the **"Raw Action Configuration"** accordion section is always blank when expanded.
**Where to look:**
- **`frontend/src/components/config/ActionsTab.tsx`** — the `ActionDetail` component (lines 65185). The `fetchRaw` callback (line 82) calls `fetchActionFile(action.name)` and returns `result.content`.
- **`frontend/src/components/config/RawConfigSection.tsx`** — the collapsible raw editor. It uses a `loadedRef` (line 62) to fetch content only on first accordion expansion, and never resets.
- **`frontend/src/api/config.ts`** line 257 — `fetchActionFile()` calls `GET /config/actions/{name}`.
- **`backend/app/services/file_config_service.py`** line 734 — `get_action_file()` reads from `action.d/` using `_read_conf_file()`, which tries `.conf` first, then `.local`.
**Root cause investigation — check these two possibilities:**
### Possibility A: Stale `loadedRef` across action switches
`ActionDetail` at line 301 of `ActionsTab.tsx` is rendered **without a `key` prop**:
```tsx
<ActionDetail
action={selectedAction}
onAssignClick={() => { setAssignOpen(true); }}
onRemovedFromJail={handleRemovedFromJail}
/>
```
Without `key={selectedAction.name}`, React reuses the same component instance when switching between actions. The `RawConfigSection` inside `ActionDetail` keeps its `loadedRef.current = true` from a previous expansion, so it never re-fetches for the new action.
**Fix:** Add `key={selectedAction.name}` to `ActionDetail` so it fully unmounts/remounts on action switch, resetting all internal state including `loadedRef`.
### Possibility B: Backend returning empty content
The `_read_conf_file` function at `file_config_service.py` line 492 tries `.conf` first then `.local`. If the action only has a `.conf` file and it's readable, content should be returned. Verify by:
1. Checking browser DevTools Network tab for the `GET /api/config/actions/{name}` response body.
2. Confirming the response has a non-empty `content` field.
**Implementation:**
1. Add `key={selectedAction.name}` to the `<ActionDetail>` component in `ActionsTab.tsx` line 301.
2. Test by selecting different actions and expanding the raw config accordion — each should show the correct file content.
3. If the backend is returning empty content, debug `_read_conf_file` to check which file path it resolves and whether it can read the file.
---
## Task 3 — Give Inactive Jail Configs the Same GUI as Active Ones
**Problem:** In the Config → Jails page, **inactive jails** show a minimal read-only preview (`InactiveJailDetail` component) with only basic fields (filter, port, ban time, find time, max retry, log paths, actions, source file). **Active jails** like `bangui-sim` show a full editable form (`JailConfigDetail` component) with all fields including Backend, Log Encoding, Date Pattern, DNS Mode, Prefix Regex, editable log paths, Fail/Ignore Regex lists, Ban-time Escalation, and a Raw Configuration editor.
The inactive jail detail should display the same rich GUI as the active one — same layout, same fields — but in read-only mode (since the jail is not running on fail2ban).
**Where to change:**
- **`frontend/src/components/config/JailsTab.tsx`**:
- `InactiveJailDetail` component (lines 493580): the current minimal read-only view.
- `JailConfigDetail` component (lines ~200490): the full editable form for active jails.
- Selection logic at lines 721752 determines which component renders based on jail kind.
- **`frontend/src/types/config.ts`** — `InactiveJail` type (around line 484) defines the data shape for inactive jails. Compare with `JailConfig` to see which fields are missing.
- **Backend** — check that the inactive jail endpoint returns enough data. The backend model `InactiveJail` may need additional fields (backend, log_encoding, date_pattern, dns_mode, prefix_regex, failregex, ignoreregex, ban-time escalation settings) to match the active `JailConfig` model.
**Implementation approach:**
1. **Extend the `InactiveJail` model** in the backend (`backend/app/models/config.py`) to include all fields that `JailConfig` has: `backend`, `log_encoding`, `date_pattern`, `usedns`, `prefixregex`, `failregex`, `ignoreregex`, `bantime_increment` (escalation settings), etc.
2. **Update the backend service** that parses inactive jail configs to extract these additional fields from the `.conf`/`.local` files.
3. **Rewrite `InactiveJailDetail`** to mirror `JailConfigDetail`'s layout but with all fields in **read-only** mode (all `<Input>` have `readOnly`, all `<Select>` are disabled). Keep the "Activate Jail" button at the bottom.
4. Alternatively, refactor `JailConfigDetail` to accept a `readOnly` prop and reuse it for both active and inactive jails, avoiding code duplication.
5. Include the **Raw Configuration** accordion (using `RawConfigSection`) for inactive jails as well, pointed at the jail's config file.
**Testing:** Select an inactive jail and verify it shows the same fields and layout as an active jail like `bangui-sim`, with all values populated from the config file but not editable.
---
## Task 4 — Fix `banaction` Interpolation Error When Activating Jails
**Problem:** Activating certain jails (e.g. `airsonic-auth`) causes fail2ban to crash with:
```
ERROR Failed during configuration: Bad value substitution: option 'action'
in section 'airsonic-auth' contains an interpolation key 'banaction' which
is not a valid option name. Raw value: '%(action_)s'
```
**Root cause — the interpolation chain breaks:**
1. Most jails inherit `action = %(action_)s` from `[DEFAULT]`.
2. `action_` is defined in `fail2ban-master/config/jail.conf` line 212 as:
```ini
action_ = %(banaction)s[port="%(port)s", protocol="%(protocol)s", chain="%(chain)s"]
```
3. `banaction` is **commented out** in the `[DEFAULT]` section of `jail.conf` (line 208):
```ini
#banaction = iptables-multiport
#banaction_allports = iptables-allports
```
4. When BanGUI activates a jail, `_write_local_override_sync()` in `backend/app/services/config_file_service.py` (line 478) writes a `.local` file with only `enabled`, `bantime`, `findtime`, `maxretry`, `port`, and `logpath` — it does **not** include `banaction` or `action`.
5. fail2ban merges the configs, hits `%(banaction)s`, finds no value, and crashes.
**Where to change:**
- **`backend/app/services/config_file_service.py`** — function `_write_local_override_sync()` around line 478. This writes the per-jail `.local` override file.
- **`backend/app/models/config.py`** — the `ActivateJailRequest` or equivalent model that defines which fields are accepted when activating a jail.
**Fix options (pick one):**
### Option A: Write `banaction` in every jail `.local` file (recommended)
When BanGUI writes a jail `.local` file, include a `banaction` field with a sensible default (e.g. `iptables-multiport`). This ensures the interpolation chain always resolves.
1. Add a `banaction` field (defaulting to `iptables-multiport`) to the jail activation model/request.
2. In `_write_local_override_sync()`, always write `banaction = <value>` into the `[jailname]` section of the `.local` file.
3. Also include `banaction_allports = iptables-allports` as a fallback for jails that reference it.
### Option B: Write a global `jail.local` with DEFAULT banaction
Create or update a `jail.local` file in the fail2ban config directory with:
```ini
[DEFAULT]
banaction = iptables-multiport
banaction_allports = iptables-allports
```
This provides the missing defaults for all jails at once.
### Option C: Expose `banaction` in the jail config UI
Add `banaction` as a configurable field in the jail config form so users can choose the ban action (iptables-multiport, nftables-multiport, firewallcmd-rich-rules, etc.). Write the selected value to the `.local` file on save/activate.
**Testing:**
1. Activate `airsonic-auth` (or any jail that inherits `action = %(action_)s` without defining its own `banaction`).
2. Verify fail2ban starts without the interpolation error.
3. Verify `fail2ban-client status airsonic-auth` shows the jail running with the correct ban action.
- Open the app, navigate to **Configuration → Jails**, select any jail.
- Confirm the "Date Pattern" combobox and "DNS Mode" select are vertically aligned.
- Resize the browser below 900 px and confirm both fields stack into a single column correctly.
- Check other config tabs (Defaults, Filters) that reuse `fieldRow` to ensure no regression.

View File

@@ -129,6 +129,7 @@ export const useConfigStyles = makeStyles({
gridTemplateColumns: "1fr 1fr", gridTemplateColumns: "1fr 1fr",
gap: tokens.spacingHorizontalM, gap: tokens.spacingHorizontalM,
marginBottom: tokens.spacingVerticalS, marginBottom: tokens.spacingVerticalS,
alignItems: "end",
"@media (max-width: 900px)": { "@media (max-width: 900px)": {
gridTemplateColumns: "1fr", gridTemplateColumns: "1fr",
}, },