From 0225f32901ea4a7b923f6c0f86238071bc7e52f6 Mon Sep 17 00:00:00 2001 From: Lukas Date: Tue, 10 Mar 2026 17:20:13 +0100 Subject: [PATCH] Fix country not shown in ban list due to geo rate limiting list_bans() was calling geo_service.lookup() once per IP on the page (e.g. 100 sequential HTTP requests), hitting the ip-api.com free-tier single-IP limit of 45 req/min. IPs beyond the ~45th were added to the in-process negative cache (5 min TTL) and showed as no country until the TTL expired. The map endpoint never had this problem because it used lookup_batch (100 IPs per POST). Add http_session and app_db params to list_bans(). When http_session is provided (production path), the entire page is resolved in one lookup_batch() call instead of N individual ones. The legacy geo_enricher callback is kept for test compatibility. Update the dashboard router to use the batch path directly. Adds 3 tests covering the batch geo path, failure resilience, and http_session priority over geo_enricher. --- Docs/Tasks.md | 421 +----------------- backend/app/routers/dashboard.py | 8 +- backend/app/services/ban_service.py | 40 +- .../tests/test_services/test_ban_service.py | 98 ++++ 4 files changed, 159 insertions(+), 408 deletions(-) diff --git a/Docs/Tasks.md b/Docs/Tasks.md index 31602e1..1b3d793 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -4,405 +4,24 @@ This document breaks the entire BanGUI project into development stages, ordered --- -## Task 1 — Mark Imported Blocklist IP Addresses ✅ DONE +## Completed + +### [DONE] Fix: Country column shows "—" for blocklist-import IPs in ban list + +**Root cause:** `ban_service.list_bans()` resolved geo data one IP at a time via +`geo_service.lookup()`, which uses the ip-api.com single-IP endpoint (45 req/min +free tier limit). A page of 100 bans triggered 100 sequential HTTP requests; +after the ~45th request ip-api.com applied rate limiting, all remaining IPs were +added to the in-process negative cache (5 min TTL), and they showed "—" in the +country column permanently until the TTL expired. Because the map endpoint +(`bans_by_country`) used `lookup_batch` (100 IPs per POST), it never hit the +rate limit, which is why the map showed colours while the list did not. + +**Fix:** Added `http_session` and `app_db` parameters to `list_bans()`. When +`http_session` is provided (production path via the dashboard router), the entire +page of IPs is resolved in a single `geo_service.lookup_batch()` call instead of +100 individual ones. The legacy `geo_enricher` callback is kept for backwards +compatibility in tests. Updated `dashboard.py` to pass `http_session` and `db` +instead of constructing a per-IP enricher closure. Added 3 new tests covering +the batch path, failure resilience, and priority over `geo_enricher`. -**Completed:** Added `origin` field (`"blocklist"` / `"selfblock"`) to `Ban` and `DashboardBanItem` models, derived from jail name in `ban_service.py`. BanTable and MapPage companion table display an Origin badge column. Tests added to `test_ban_service.py` and `test_dashboard.py`. - -### Problem - -When IPs are imported from an external blocklist they are applied to the `blocklist-import` jail via the fail2ban socket. Once they land in the fail2ban SQLite `bans` table they look identical to IPs that were banned organically by fail2ban filters. There is no way for the dashboard or map to tell the user whether a given ban came from a blocklist import or from a real failed-login detection. - -### Goal - -Every ban displayed in the UI must carry a visible **origin** indicator: either `blocklist` (imported from a blocklist source) or `selfblock` (detected by fail2ban itself). - -### Implementation Details - -**Backend — derive origin from jail name** - -The blocklist import service already uses a dedicated jail called `blocklist-import`. This can be used as the discriminator: any ban whose `jail` column equals `blocklist-import` is a blocklist ban; everything else is a self-block. - -1. **Model change** — In `backend/app/models/ban.py`, add an `origin` field of type `Literal["blocklist", "selfblock"]` to both `Ban` and `DashboardBanItem`. Compute it from the `jail` value during construction (if `jail == "blocklist-import"` → `"blocklist"`, else `"selfblock"`). -2. **Service change** — In `backend/app/services/ban_service.py`, make sure `list_bans()` and `bans_by_country()` populate the new `origin` field when building ban objects. -3. **API response** — The JSON payloads from `GET /api/dashboard/bans` and `GET /api/dashboard/bans/by-country` already serialise every field of `DashboardBanItem`, so `origin` will appear automatically once the model is updated. - -**Frontend — show origin badge** - -4. **BanTable** — In `frontend/src/components/BanTable.tsx`, add an **Origin** column. Render a small coloured badge: blue label `Blocklist` or grey label `Selfblock`. -5. **MapPage detail table** — The table below the world map in `frontend/src/pages/MapPage.tsx` also lists bans. Add the same origin badge column there. - -**Tests** - -6. Add a unit test in `backend/tests/test_services/test_ban_service.py` that inserts bans into a mock fail2ban DB under both jail names (`blocklist-import` and e.g. `sshd`) and asserts the returned objects carry the correct `origin` value. -7. Add a router test in `backend/tests/test_routers/test_dashboard.py` that verifies the JSON response contains the `origin` field. - -### Files Touched - -| Layer | File | -|-------|------| -| Model | `backend/app/models/ban.py` | -| Service | `backend/app/services/ban_service.py` | -| Frontend | `frontend/src/components/BanTable.tsx` | -| Frontend | `frontend/src/pages/MapPage.tsx` | -| Tests | `backend/tests/test_services/test_ban_service.py` | -| Tests | `backend/tests/test_routers/test_dashboard.py` | - ---- - -## Task 2 — Add Origin Filter to Dashboard and World Map ✅ DONE - -**Completed:** Added optional `origin` query parameter (`"blocklist"` / `"selfblock"`) to both dashboard router endpoints, filtered via `jail`-based WHERE clause in `ban_service`. Frontend: `BanOriginFilter` type + `BAN_ORIGIN_FILTER_LABELS` in `types/ban.ts`; `fetchBans` and `fetchBansByCountry` APIs forward the param; `useBans` and `useMapData` hooks accept `origin`; `DashboardPage` and `MapPage` show a segmented toggle / select for All / Blocklist / Selfblock. Tests extended in `test_ban_service.py` and `test_dashboard.py`. - -### Problem - -Once Task 1 exposes the `origin` field, users need a way to filter the view so they can see **all** bans, **only blocklist** bans, or **only self-blocked** bans. This must work on both the dashboard ban table and the world map. - -### Goal - -Add a filter dropdown (or segmented toggle) with three options — `All`, `Blocklist`, `Selfblock` — to the dashboard toolbar and the map toolbar. The selection must propagate to the backend so that only matching bans are returned and pagination / country aggregation remain correct. - -### Implementation Details - -**Backend — add `origin` query parameter** - -1. **Dashboard router** — `GET /api/dashboard/bans` in `backend/app/routers/dashboard.py`: add an optional query parameter `origin: Optional[Literal["blocklist", "selfblock"]] = None`. Pass it through to the service layer. -2. **Map router** — `GET /api/dashboard/bans/by-country` in the same router: add the same `origin` parameter. -3. **Service layer** — In `backend/app/services/ban_service.py`: - - `list_bans()`: when `origin` is provided, append a WHERE clause on the `jail` column (`jail = 'blocklist-import'` for blocklist, `jail != 'blocklist-import'` for selfblock). - - `bans_by_country()`: apply the same jail-based filter so that country aggregation only counts matching bans. - -**Frontend — filter controls** - -4. **Shared state** — Create a small shared type `BanOriginFilter = "all" | "blocklist" | "selfblock"` (e.g. in `frontend/src/types/` or inline). -5. **DashboardPage** — In `frontend/src/pages/DashboardPage.tsx`, add a dropdown or segmented control next to the existing time-range toolbar. Store the selected value in component state. Pass it to `useBans` hook, which forwards it as the `origin` query parameter. -6. **useBans hook** — In `frontend/src/hooks/useBans.ts`, accept an optional `origin` parameter and include it in the API call via `fetchBans()`. -7. **API function** — In `frontend/src/api/dashboard.ts`, update `fetchBans()` to accept and forward the `origin` query parameter. -8. **MapPage** — In `frontend/src/pages/MapPage.tsx`, add the same dropdown. Pass the selected value to `useMapData` hook. -9. **useMapData hook** — In `frontend/src/hooks/useMapData.ts`, accept `origin` and forward it to `fetchBansByCountry()`. -10. **Map API function** — In `frontend/src/api/map.ts`, update `fetchBansByCountry()` to include `origin` in the query string. - -**Tests** - -11. Extend `backend/tests/test_services/test_ban_service.py`: insert bans under multiple jails, call `list_bans(origin="blocklist")` and assert only `blocklist-import` jail bans are returned; repeat for `"selfblock"` and `None`. -12. Extend `backend/tests/test_routers/test_dashboard.py`: hit `GET /api/dashboard/bans?origin=blocklist` and verify the response only contains blocklist bans. - -### Files Touched - -| Layer | File | -|-------|------| -| Router | `backend/app/routers/dashboard.py` | -| Service | `backend/app/services/ban_service.py` | -| Frontend | `frontend/src/pages/DashboardPage.tsx` | -| Frontend | `frontend/src/pages/MapPage.tsx` | -| Frontend | `frontend/src/hooks/useBans.ts` | -| Frontend | `frontend/src/hooks/useMapData.ts` | -| Frontend | `frontend/src/api/dashboard.ts` | -| Frontend | `frontend/src/api/map.ts` | -| Tests | `backend/tests/test_services/test_ban_service.py` | -| Tests | `backend/tests/test_routers/test_dashboard.py` | - ---- - -## Task 3 — Performance Optimisation for 10 k+ IPs (Dashboard & World Map) ✅ DONE - -**Completed:** -- Added persistent `geo_cache` SQLite table in `db.py`; loaded into in-memory cache at startup via `geo_service.load_cache_from_db()`. -- Rewrote `geo_service.py`: added `lookup_batch()` using `ip-api.com/batch` (100 IPs/call); failed lookups no longer cached so they are retried; only successful resolutions written to persistent store. -- Rewrote `bans_by_country()` in `ban_service.py`: SQL `GROUP BY ip` aggregation instead of loading 2 000 raw rows, batch geo-resolution via `lookup_batch()`, companion table limited to 200 rows (already geo-cached). -- Updated `dashboard.py` router `GET /bans/by-country` to pass `http_session` + `app_db` directly to `bans_by_country()`. -- Added geo cache pre-warm in `blocklist_service.import_source()`: after import, newly banned IPs are batch-resolved and persisted. -- Added debounce (300 ms) to `useMapData` hook to cancel stale in-flight requests when filters change rapidly; sets loading=true immediately for instant skeleton feedback. -- BanTable: page size capped at 100 per page with next/prev pagination — DOM perf not an issue, no virtualisation needed. -- Performance benchmark `tests/test_services/test_ban_service_perf.py`: seeds 10 000 bans in a temp DB, pre-warms geo cache, asserts `list_bans` and `bans_by_country` both complete in < 2 seconds. -- Seed script `tests/scripts/seed_10k_bans.py`: inserts 10 000 synthetic bans + pre-caches geo data for browser-level load-time verification. - -### Problem - -With a large number of banned IPs (> 10 000), both the dashboard and the world map take over 10 seconds to load and become unusable. The root cause is the geo-enrichment step: `geo_service.py` calls the external `ip-api.com` endpoint **one IP at a time** with a 5-second timeout, and the free tier is rate-limited to 45 requests/minute. On a cold cache with 10 k IPs, this would take hours. - -### Goal - -Dashboard and world map must load within **2 seconds** for 10 k banned IPs. Write a reproducible benchmark test to prove it. - -### Implementation Details - -**Backend — persistent geo cache** - -1. **SQLite geo cache table** — In `backend/app/db.py`, add a `geo_cache` table: - ```sql - CREATE TABLE IF NOT EXISTS geo_cache ( - ip TEXT PRIMARY KEY, - country_code TEXT, - country_name TEXT, - asn TEXT, - org TEXT, - cached_at TEXT NOT NULL - ); - ``` - On startup the service loads from this table instead of starting with an empty dict. Lookups hit the in-memory dict first, then fall through to the DB, and only as a last resort call the external API. Successful API responses are written back to both caches. - -2. **Batch lookup via ip-api.com batch endpoint** — The free `ip-api.com` API supports a **batch POST** to `http://ip-api.com/batch` accepting up to 100 IPs per request. Refactor `geo_service.py` to: - - Collect all uncached IPs from the requested page. - - Send them in chunks of 100 to the batch endpoint. - - Parse the JSON array response and populate caches in one pass. - This alone reduces 10 000 cold lookups from 10 000 sequential requests to ≈ 100 batch calls. - -3. **Pre-warm cache on import** — In `backend/app/services/blocklist_service.py`, after a successful blocklist import, fire a background task that batch-resolves all newly imported IPs. This way the dashboard never faces a cold cache for blocklist IPs. - -**Backend — limit the country-aggregation query** - -4. In `ban_service.py`, `bans_by_country()` currently loads up to 2 000 bans and enriches every one. Change it to: - - Run the aggregation (GROUP BY `jail`, count per jail) **in SQL** directly against the fail2ban DB. - - Only enrich the distinct IPs, batch-resolved. - - Return the aggregated country → count map without fetching full ban rows. - -**Frontend — virtualised table** - -5. The `BanTable` component currently renders all rows in the DOM. For 10 k+ rows (even paginated at 100), scrolling is fine, but if page sizes are increased or if pagination is removed, install a virtual-scrolling library (e.g. `@tanstack/react-virtual`) and render only visible rows. Alternatively, ensure page size stays capped at ≤ 500 (already enforced) and measure whether DOM performance is acceptable — only virtualise if needed. - -**Frontend — map debounce** - -6. In `WorldMap.tsx`, add a loading skeleton / spinner and debounce the data fetch. If the user switches time ranges rapidly, cancel in-flight requests to avoid piling up stale responses. - -**Performance test** - -7. Write a pytest benchmark in `backend/tests/test_services/test_ban_service_perf.py`: - - Seed a temporary fail2ban SQLite with 10 000 synthetic bans (random IPs, mixed jails, spread over 365 days). - - Pre-populate the geo cache with matching entries so the test does not hit the network. - - Call `list_bans(range="365d", page=1, page_size=100)` and `bans_by_country(range="365d")`. - - Assert both return within **2 seconds** wall-clock time. -8. Add a manual/integration test script `backend/tests/scripts/seed_10k_bans.py` that inserts 10 000 bans into the real fail2ban dev DB and pre-caches their geo data so developers can visually test dashboard and map load times in the browser. - -### Files Touched - -| Layer | File | -|-------|------| -| DB schema | `backend/app/db.py` | -| Geo service | `backend/app/services/geo_service.py` | -| Ban service | `backend/app/services/ban_service.py` | -| Blocklist service | `backend/app/services/blocklist_service.py` | -| Frontend | `frontend/src/components/BanTable.tsx` | -| Frontend | `frontend/src/components/WorldMap.tsx` | -| Tests | `backend/tests/test_services/test_ban_service_perf.py` (new) | -| Scripts | `backend/tests/scripts/seed_10k_bans.py` (new) | - ---- - -## Task 4 — Fix Missing Country for Resolved IPs ✅ DONE - -**Completed:** -- `geo_service.py`: Added `_neg_cache: dict[str, float]` with 5-minute TTL (`_NEG_CACHE_TTL = 300`). Failed lookups (any cause) are written to the neg cache and returned immediately without querying the API until the TTL expires. `clear_neg_cache()` flushes it (used by the re-resolve endpoint). -- `geo_service.py`: Added `init_geoip(mmdb_path)` + `_geoip_lookup(ip)` using `geoip2.database.Reader`. When ip-api fails, the local GeoLite2-Country `.mmdb` is tried as fallback. Only fires if `BANGUI_GEOIP_DB_PATH` is set and the file exists; otherwise silently skipped. -- `geo_service.py`: Fixed `lookup_batch()` bug where failed API results were stored in the positive in-memory cache (`_store` was called unconditionally). Now only positive results go into `_cache`; failures try geoip2 fallback then go into `_neg_cache`. -- `geo_service.py`: Added `_persist_neg_entry(db, ip)` — `INSERT OR IGNORE` into `geo_cache` with `country_code=NULL` so the re-resolve endpoint can find previously failed IPs without overwriting existing positive entries. -- `config.py`: Added `geoip_db_path: str | None` setting (env `BANGUI_GEOIP_DB_PATH`). -- `pyproject.toml`: Added `geoip2>=4.8.0` dependency. -- `main.py`: Calls `geo_service.init_geoip(settings.geoip_db_path)` during lifespan startup. -- `routers/geo.py`: Added `POST /api/geo/re-resolve` — queries `geo_cache WHERE country_code IS NULL`, clears neg cache, batch-re-resolves all those IPs, returns `{"resolved": N, "total": M}`. -- `BanTable.tsx`: Country cell now wraps the `—` placeholder in a Fluent UI Tooltip with message "Country could not be resolved — will retry automatically." -- `MapPage.tsx`: Same Tooltip treatment for the `—` placeholder in the companion table. -- Tests: Updated `test_geo_service.py` — removed outdated `result is None` assertions (lookup now always returns GeoInfo), updated neg-cache test, added `TestNegativeCache` (4 tests) and `TestGeoipFallback` (4 tests). Added `TestReResolve` (4 tests) in `test_geo.py`. **430 total tests pass.** - -### Problem - -Many IPs (e.g. `5.167.71.2`, `5.167.71.3`, `5.167.71.4`) show no country in the dashboard and map. The current `geo_service.py` calls `ip-api.com` with a 5-second timeout and silently returns `None` fields on any error or timeout. Common causes: - -- **Rate limiting** — the free tier allows 45 req/min; once exceeded, responses return `"status": "fail"` and the service caches `None` values, permanently hiding the country. -- **Cache poisoning with empty entries** — a failed lookup stores `GeoInfo(country_code=None, ...)` in the in-memory cache. Until the cache is flushed (at 10 000 entries), that IP will always appear without a country. -- **External API unreachable** — network issues or container DNS problems cause timeouts that are treated the same as "no data". - -### Goal - -Every IP that has a valid geographic mapping should display its country. Failed lookups must be retried on subsequent requests rather than permanently cached as empty. - -### Implementation Details - -**Backend — do not cache failed lookups** - -1. In `backend/app/services/geo_service.py`, change the caching logic: only store a result in the in-memory cache (and the new persistent `geo_cache` table from Task 3) when `country_code is not None`. If the API returned a failure or the request timed out, do **not** cache the result so it will be retried on the next request. - -**Backend — negative-cache with short TTL** - -2. To avoid hammering the API for the same failing IP on every request, introduce a separate **negative cache** — a dict mapping IP → timestamp of last failed attempt. Skip re-lookup if the last failure was less than **5 minutes** ago. After 5 minutes the IP becomes eligible for retry. - -**Backend — fallback to a local GeoIP database** - -3. Add `geoip2` (MaxMind GeoLite2) as an optional fallback. If the free `ip-api.com` lookup fails or is rate-limited, attempt a local lookup using the GeoLite2-Country database (`.mmdb` file). This provides offline country resolution for the vast majority of IPv4 addresses. - - Add `geoip2` to `backend/pyproject.toml` dependencies. - - Download the GeoLite2-Country database during Docker build or via a setup script (requires free MaxMind license key). - - In `geo_service.py`, try `ip-api.com` first; on failure, fall back to geoip2; only return `None` if both fail. - -**Backend — bulk re-resolve endpoint** - -4. Add `POST /api/geo/re-resolve` in `backend/app/routers/geo.py` that: - - Queries all currently cached IPs with `country_code IS NULL`. - - Re-resolves them in batches (using the batch endpoint from Task 3). - - Returns a count of newly resolved IPs. - - This lets the admin manually fix historical gaps. - -**Frontend — visual indicator for unknown country** - -5. In `BanTable.tsx` and the MapPage detail table, display a small "unknown" icon or `—` placeholder when country is null, instead of leaving the cell empty. Tooltip: "Country could not be resolved — will retry automatically." - -**Tests** - -6. In `backend/tests/test_services/test_geo_service.py`: - - Test that a failed lookup is **not** persisted in the positive cache. - - Test that a failed lookup **is** stored in the negative cache and skipped for 5 minutes. - - Test that after the negative-cache TTL expires, the IP is re-queried. - - Mock the geoip2 fallback and verify it is called when ip-api fails. -7. In `backend/tests/test_routers/test_geo.py`, test the new `POST /api/geo/re-resolve` endpoint. - -### Files Touched - -| Layer | File | -|-------|------| -| Geo service | `backend/app/services/geo_service.py` | -| Geo router | `backend/app/routers/geo.py` | -| Dependencies | `backend/pyproject.toml` | -| Frontend | `frontend/src/components/BanTable.tsx` | -| Frontend | `frontend/src/pages/MapPage.tsx` | -| Tests | `backend/tests/test_services/test_geo_service.py` | -| Tests | `backend/tests/test_routers/test_geo.py` | - ---- - -## Task 5 — Blocklist Import Error Badge in Navigation ✅ DONE - -**Completed:** Added `last_run_errors: bool | None` to `ScheduleInfo` model and updated `get_schedule_info()` to derive it from the last import log entry. Frontend: added `last_run_errors` to `ScheduleInfo` type; added `useBlocklistStatus` hook that polls `GET /api/blocklists/schedule` every 60 s; `MainLayout` renders a warning `MessageBar` and an amber badge on the Blocklists nav item when `hasErrors` is `true`. Tests: 3 new service tests + 1 new router test; 433 tests pass. - -### Problem - -[Features.md § 8](Features.md) requires: *"Show a warning badge in the navigation if the most recent import encountered errors"* and *"Notify the user (via the UI status bar) when a scheduled import fails so it does not go unnoticed."* - -Currently `ScheduleInfo` (returned by `GET /api/blocklists/schedule`) contains `last_run_at` but no indicator of whether the last run had errors. The `MainLayout` sidebar only warns about fail2ban being offline; there is no blocklist-import failure indicator anywhere in the shell. - -### Goal - -When the most recent blocklist import run completed with errors, a warning indicator must be visible in the persistent app shell until the condition clears (i.e. a successful import runs). Concretely: - -1. A warning `MessageBar` appears at the top of the main content area (alongside the existing fail2ban-offline bar). -2. A small warning badge is rendered on the **Blocklists** navigation item in the sidebar. - -### Implementation Details - -**Backend — expose error flag in `ScheduleInfo`** - -1. **`app/models/blocklist.py`** — Add `last_run_errors: bool | None = None` to `ScheduleInfo`. `True` means the most recent run's `errors` column was non-null; `None` means no run has ever completed. -2. **`app/services/blocklist_service.py`** — In `get_schedule_info()`, after fetching `last_log`, set `last_run_errors = last_log["errors"] is not None` when `last_log` is not `None`, else leave it as `None`. - -**Frontend — poll and display** - -3. **`frontend/src/types/blocklist.ts`** — Add `last_run_errors: boolean | null` to `ScheduleInfo`. -4. **`frontend/src/hooks/useBlocklist.ts`** — Add a new exported hook `useBlocklistStatus` that polls `GET /api/blocklists/schedule` every 60 seconds (plus on mount) and returns `{ hasErrors: boolean }`. Errors from the poll itself should not surface to the user — silently treat as "unknown". -5. **`frontend/src/layouts/MainLayout.tsx`**: - - Import and call `useBlocklistStatus`. - - When `hasErrors` is `true`, render a second `MessageBar` (intent `"warning"`) in the warning-bar slot describing the blocklist import failure. - - Add a small amber `Badge` (number `!` or just the dot shape) to the Blocklists `NavLink` entry so users see the indicator even when they're on another page. - -**Tests** - -6. **`backend/tests/test_services/test_blocklist_service.py`** — Three new tests in `TestSchedule`: - - `test_get_schedule_info_no_errors_when_log_has_no_errors` — inserts a successful import log entry (errors=None), asserts `info.last_run_errors is False`. - - `test_get_schedule_info_errors_when_log_has_errors` — inserts a log entry with a non-null `errors` string, asserts `info.last_run_errors is True`. - - `test_get_schedule_info_none_when_no_log` — already covered by the existing test; verify it now also asserts `info.last_run_errors is None`. -7. **`backend/tests/test_routers/test_blocklist.py`** — One new test in `TestGetSchedule`: - - `test_schedule_response_includes_last_run_errors` — patches `get_schedule_info` to return a `ScheduleInfo` with `last_run_errors=True`, confirms the JSON field is present and `True`. - -### Files Touched - -| Layer | File | -|-------|------| -| Model | `backend/app/models/blocklist.py` | -| Service | `backend/app/services/blocklist_service.py` | -| Frontend type | `frontend/src/types/blocklist.ts` | -| Frontend hook | `frontend/src/hooks/useBlocklist.ts` | -| Frontend layout | `frontend/src/layouts/MainLayout.tsx` | -| Tests | `backend/tests/test_services/test_blocklist_service.py` | -| Tests | `backend/tests/test_routers/test_blocklist.py` | - ---- - -## Task 6 — Mass Unban: Clear All Currently Banned IPs ✅ DONE - -**Completed:** Added `unban_all_ips()` service function using fail2ban's `unban --all` command. Added `DELETE /api/bans/all` endpoint returning `UnbanAllResponse` with count. Frontend: `bansAll` endpoint constant, `unbanAllBans()` API function, `UnbanAllResponse` type in `types/jail.ts`, `unbanAll` action exposed from `useActiveBans` hook. JailsPage `ActiveBansSection` shows a "Clear All Bans" button (only when bans > 0) that opens a Fluent UI confirmation Dialog before executing. Tests: 7 new tests (3 service + 4 router); 440 total pass. - -### Problem - -[Features.md § 5](Features.md) specifies: *"Option to unban all IPs at once across every jail."* -Currently the Jails page allows unbanning a single IP from a specific jail or from all jails, but there is no mechanism to clear every active ban globally in one operation. fail2ban supports this natively via the `unban --all` socket command, which returns the count of unbanned IPs. - -### Goal - -Add a **"Clear All Bans"** action that: -1. Sends `unban --all` to fail2ban, removing every active ban across every jail in a single command. -2. Returns a count of how many IPs were unbanned. -3. Is exposed in the UI as a "Clear All Bans" button with a confirmation dialog so users cannot trigger it accidentally. -4. Refreshes the active-bans list automatically after the operation completes. - -### Implementation Details - -**Backend — service function** - -1. **`backend/app/services/jail_service.py`** — Add `unban_all_ips(socket_path: str) -> int`: - - Sends `["unban", "--all"]` via `Fail2BanClient`. - - Returns the integer count reported by fail2ban (`_ok()` extracts it from the `(0, count)` tuple). - - Logs the operation at `info` level with the returned count. - -**Backend — new response model** - -2. **`backend/app/models/ban.py`** — Add `UnbanAllResponse(BaseModel)` with fields: - - `message: str` — human-readable summary. - - `count: int` — number of IPs that were unbanned. - -**Backend — new endpoint** - -3. **`backend/app/routers/bans.py`** — Add: - ``` - DELETE /api/bans/all — unban every currently banned IP across all jails - ``` - - Returns `UnbanAllResponse` with `count` from the service call. - - No request body required. - - Handles `Fail2BanConnectionError` → 502. - -**Frontend — API** - -4. **`frontend/src/api/endpoints.ts`** — Add `bansAll: "/bans/all"` to the `ENDPOINTS` map. -5. **`frontend/src/api/jails.ts`** — Add `unbanAllBans(): Promise` that calls `del(ENDPOINTS.bansAll)`. -6. **`frontend/src/types/jail.ts`** — Add `UnbanAllResponse` interface `{ message: string; count: number }`. - -**Frontend — hook** - -7. **`frontend/src/hooks/useJails.ts`** — In `useActiveBans`, add `unbanAll: () => Promise` action and expose it from the hook return value. - -**Frontend — UI** - -8. **`frontend/src/pages/JailsPage.tsx`** — In `ActiveBansSection`: - - Add a "Clear All Bans" `Button` (appearance `"outline"`, intent `"danger"`) in the section header next to the existing Refresh button. - - Wrap the confirm action in a Fluent UI `Dialog` with a warning body explaining the operation is irreversible. - - On confirmation, call `unbanAll()`, show a success `MessageBar` with the count, and call `refresh()`. - -**Tests** - -9. **`backend/tests/test_services/test_jail_service.py`** — Add `TestUnbanAllIps`: - - `test_unban_all_ips_returns_count` — mocks client with `(0, 5)` response, asserts return is `5`. - - `test_unban_all_ips_raises_on_fail2ban_error` — mocks client to raise `Fail2BanConnectionError`, asserts it propagates. -10. **`backend/tests/test_routers/test_bans.py`** — Add `TestUnbanAll`: - - `test_204_clears_all_bans` — patches `unban_all_ips` returning `3`, asserts 200 response with `count == 3`. - - `test_502_when_fail2ban_unreachable` — patches `unban_all_ips` raising `Fail2BanConnectionError`, asserts 502. - - `test_401_when_unauthenticated` — unauthenticated request returns 401. - -### Files Touched - -| Layer | File | -|-------|------| -| Model | `backend/app/models/ban.py` | -| Service | `backend/app/services/jail_service.py` | -| Router | `backend/app/routers/bans.py` | -| Frontend type | `frontend/src/types/jail.ts` | -| Frontend API | `frontend/src/api/endpoints.ts` | -| Frontend API | `frontend/src/api/jails.ts` | -| Frontend hook | `frontend/src/hooks/useJails.ts` | -| Frontend UI | `frontend/src/pages/JailsPage.tsx` | -| Tests | `backend/tests/test_services/test_jail_service.py` | -| Tests | `backend/tests/test_routers/test_bans.py` | diff --git a/backend/app/routers/dashboard.py b/backend/app/routers/dashboard.py index 27be928..7460d4a 100644 --- a/backend/app/routers/dashboard.py +++ b/backend/app/routers/dashboard.py @@ -26,7 +26,7 @@ from app.models.ban import ( TimeRange, ) from app.models.server import ServerStatus, ServerStatusResponse -from app.services import ban_service, geo_service +from app.services import ban_service router: APIRouter = APIRouter(prefix="/api/dashboard", tags=["Dashboard"]) @@ -109,15 +109,13 @@ async def get_dashboard_bans( socket_path: str = request.app.state.settings.fail2ban_socket http_session: aiohttp.ClientSession = request.app.state.http_session - async def _enricher(ip: str) -> geo_service.GeoInfo | None: - return await geo_service.lookup(ip, http_session, db=db) - return await ban_service.list_bans( socket_path, range, page=page, page_size=page_size, - geo_enricher=_enricher, + http_session=http_session, + app_db=db, origin=origin, ) diff --git a/backend/app/services/ban_service.py b/backend/app/services/ban_service.py index 4633155..12ea30c 100644 --- a/backend/app/services/ban_service.py +++ b/backend/app/services/ban_service.py @@ -171,6 +171,8 @@ async def list_bans( *, page: int = 1, page_size: int = _DEFAULT_PAGE_SIZE, + http_session: aiohttp.ClientSession | None = None, + app_db: aiosqlite.Connection | None = None, geo_enricher: Any | None = None, origin: BanOrigin | None = None, ) -> DashboardBanListResponse: @@ -180,6 +182,15 @@ async def list_bans( ``timeofban`` falls within the specified *range_*. Results are ordered newest-first. + Geo enrichment strategy (highest priority first): + + 1. When *http_session* is provided the entire page of IPs is resolved in + one :func:`~app.services.geo_service.lookup_batch` call (up to 100 IPs + per HTTP request). This avoids the 45 req/min rate limit of the + single-IP endpoint and is the preferred production path. + 2. When only *geo_enricher* is provided (legacy / test path) each IP is + resolved individually via the supplied async callable. + Args: socket_path: Path to the fail2ban Unix domain socket. range_: Time-range preset (``"24h"``, ``"7d"``, ``"30d"``, or @@ -187,8 +198,13 @@ async def list_bans( page: 1-based page number (default: ``1``). page_size: Maximum items per page, capped at ``_MAX_PAGE_SIZE`` (default: ``100``). + http_session: Optional shared :class:`aiohttp.ClientSession`. When + provided, :func:`~app.services.geo_service.lookup_batch` is used + for efficient bulk geo resolution. + app_db: Optional BanGUI application database used to persist newly + resolved geo entries and to read back cached results. geo_enricher: Optional async callable ``(ip: str) -> GeoInfo | None``. - When supplied every result is enriched with country and ASN data. + Used as a fallback when *http_session* is ``None`` (e.g. tests). origin: Optional origin filter — ``"blocklist"`` restricts results to the ``blocklist-import`` jail, ``"selfblock"`` excludes it. @@ -196,6 +212,8 @@ async def list_bans( :class:`~app.models.ban.DashboardBanListResponse` containing the paginated items and total count. """ + from app.services import geo_service # noqa: PLC0415 + since: int = _since_unix(range_) effective_page_size: int = min(page_size, _MAX_PAGE_SIZE) offset: int = (page - 1) * effective_page_size @@ -231,6 +249,17 @@ async def list_bans( ) as cur: rows = await cur.fetchall() + # Batch-resolve geo data for all IPs on this page in a single API call. + # This avoids hitting the 45 req/min single-IP rate limit when the + # page contains many bans (e.g. after a large blocklist import). + geo_map: dict[str, Any] = {} + if http_session is not None and rows: + page_ips: list[str] = [str(r["ip"]) for r in rows] + try: + geo_map = await geo_service.lookup_batch(page_ips, http_session, db=app_db) + except Exception: # noqa: BLE001 + log.warning("ban_service_batch_geo_failed_list_bans") + items: list[DashboardBanItem] = [] for row in rows: jail: str = str(row["jail"]) @@ -245,7 +274,14 @@ async def list_bans( asn: str | None = None org: str | None = None - if geo_enricher is not None: + if geo_map: + geo = geo_map.get(ip) + if geo is not None: + country_code = geo.country_code + country_name = geo.country_name + asn = geo.asn + org = geo.org + elif geo_enricher is not None: try: geo = await geo_enricher(ip) if geo is not None: diff --git a/backend/tests/test_services/test_ban_service.py b/backend/tests/test_services/test_ban_service.py index 16e90c2..6853b0e 100644 --- a/backend/tests/test_services/test_ban_service.py +++ b/backend/tests/test_services/test_ban_service.py @@ -290,6 +290,104 @@ class TestListBansGeoEnrichment: assert item.country_code is None +# --------------------------------------------------------------------------- +# list_bans — batch geo enrichment via http_session +# --------------------------------------------------------------------------- + + +class TestListBansBatchGeoEnrichment: + """Verify that list_bans uses lookup_batch when http_session is provided.""" + + async def test_batch_geo_applied_via_http_session( + self, f2b_db_path: str + ) -> None: + """Geo fields are populated via lookup_batch when http_session is given.""" + from app.services.geo_service import GeoInfo + from unittest.mock import MagicMock + + fake_session = MagicMock() + fake_geo_map = { + "1.2.3.4": GeoInfo(country_code="DE", country_name="Germany", asn="AS3320", org="Deutsche Telekom"), + "5.6.7.8": GeoInfo(country_code="US", country_name="United States", asn="AS15169", org="Google"), + } + + with patch( + "app.services.ban_service._get_fail2ban_db_path", + new=AsyncMock(return_value=f2b_db_path), + ), patch( + "app.services.geo_service.lookup_batch", + new=AsyncMock(return_value=fake_geo_map), + ): + result = await ban_service.list_bans( + "/fake/sock", "24h", http_session=fake_session + ) + + assert result.total == 2 + de_item = next(i for i in result.items if i.ip == "1.2.3.4") + us_item = next(i for i in result.items if i.ip == "5.6.7.8") + assert de_item.country_code == "DE" + assert de_item.country_name == "Germany" + assert us_item.country_code == "US" + assert us_item.country_name == "United States" + + async def test_batch_failure_does_not_break_results( + self, f2b_db_path: str + ) -> None: + """A lookup_batch failure still returns items with null geo fields.""" + from unittest.mock import MagicMock + + fake_session = MagicMock() + + with patch( + "app.services.ban_service._get_fail2ban_db_path", + new=AsyncMock(return_value=f2b_db_path), + ), patch( + "app.services.geo_service.lookup_batch", + new=AsyncMock(side_effect=RuntimeError("batch geo down")), + ): + result = await ban_service.list_bans( + "/fake/sock", "24h", http_session=fake_session + ) + + assert result.total == 2 + for item in result.items: + assert item.country_code is None + + async def test_http_session_takes_priority_over_geo_enricher( + self, f2b_db_path: str + ) -> None: + """When both http_session and geo_enricher are provided, batch wins.""" + from app.services.geo_service import GeoInfo + from unittest.mock import MagicMock + + fake_session = MagicMock() + fake_geo_map = { + "1.2.3.4": GeoInfo(country_code="DE", country_name="Germany", asn=None, org=None), + "5.6.7.8": GeoInfo(country_code="DE", country_name="Germany", asn=None, org=None), + } + + async def enricher_should_not_be_called(ip: str) -> GeoInfo: + raise AssertionError(f"geo_enricher was called for {ip!r} — should not happen") + + with patch( + "app.services.ban_service._get_fail2ban_db_path", + new=AsyncMock(return_value=f2b_db_path), + ), patch( + "app.services.geo_service.lookup_batch", + new=AsyncMock(return_value=fake_geo_map), + ): + result = await ban_service.list_bans( + "/fake/sock", + "24h", + http_session=fake_session, + geo_enricher=enricher_should_not_be_called, + ) + + assert result.total == 2 + for item in result.items: + assert item.country_code == "DE" + + # --------------------------------------------------------------------------- # list_bans — pagination # ---------------------------------------------------------------------------