diff --git a/Docs/Architekture.md b/Docs/Architekture.md index 6701e89..8d24fbd 100644 --- a/Docs/Architekture.md +++ b/Docs/Architekture.md @@ -189,7 +189,7 @@ The business logic layer. Services orchestrate operations, enforce rules, and co | `log_service.py` | Log preview and regex test operations (extracted from config_service) | | `history_service.py` | Queries the fail2ban database for historical ban records, builds per-IP timelines, computes ban counts and repeat-offender flags, and syncs new records into BanGUI's archive table | | `blocklist_service.py` | Downloads blocklists via aiohttp, validates IPs/CIDRs, applies bans through fail2ban or iptables, logs import results | -| `geo_service.py` | Resolves IP addresses to country, ASN, and RIR using external APIs or a local database, caches results | +| `geo_service.py` | Resolves IP addresses to country, ASN, and RIR using external APIs or a local database, caches results, and re-resolves unresolved geo cache entries | | `server_service.py` | Reads and writes fail2ban server-level settings (log level, log target, syslog socket, DB location, purge age) | | `health_service.py` | Probes fail2ban socket connectivity, retrieves server version and global stats, reports online/offline status | diff --git a/Docs/Tasks.md b/Docs/Tasks.md index 5d07b34..01bda81 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -246,7 +246,9 @@ The router knows the internal job ID string and directly manipulates the schedul --- -### TASK-08 — Remove inverted layer dependency: `utils/fail2ban_db_utils.py` importing a service 🟠 +### TASK-08 — Remove inverted layer dependency: `utils/fail2ban_db_utils.py` importing a service ✅ + +**Status:** Completed ✅ **Where:** `backend/app/utils/fail2ban_db_utils.py` — line 8: @@ -275,7 +277,9 @@ A util importing a service creates an implicit upward dependency that the archit --- -### TASK-09 — Move `re_resolve_geo` orchestration into `geo_service` 🟠 +### TASK-09 — Move `re_resolve_geo` orchestration into `geo_service` ✅ + +**Status:** Completed ✅ **Where:** `backend/app/routers/geo.py` — `async def re_resolve_geo()` (lines ~142–174). diff --git a/backend/app/models/geo.py b/backend/app/models/geo.py index 704875e..c43b33d 100644 --- a/backend/app/models/geo.py +++ b/backend/app/models/geo.py @@ -57,6 +57,19 @@ class GeoCacheStatsResponse(BaseModel): dirty_size: int = Field(..., description="Number of newly resolved entries not yet flushed to disk.") +class GeoReResolveResponse(BaseModel): + """Response for ``POST /api/geo/re-resolve``. + + Reports how many previously unresolved IPs were retried and how many + gained a resolved country code after the re-resolve operation. + """ + + model_config = ConfigDict(strict=True) + + resolved: int = Field(..., description="Number of IPs successfully resolved.") + total: int = Field(..., description="Number of IPs retried.") + + class IpLookupResponse(BaseModel): """Response for ``GET /api/geo/lookup/{ip}``. diff --git a/backend/app/routers/geo.py b/backend/app/routers/geo.py index 4f38e81..6c1b681 100644 --- a/backend/app/routers/geo.py +++ b/backend/app/routers/geo.py @@ -11,8 +11,6 @@ from __future__ import annotations from typing import TYPE_CHECKING, Annotated if TYPE_CHECKING: - import aiohttp - from app.services.jail_service import IpLookupResult import aiosqlite @@ -20,12 +18,11 @@ from fastapi import APIRouter, Depends, HTTPException, Path, status from app.dependencies import ( AuthDep, - DbDep, Fail2BanSocketDep, HttpSessionDep, get_db, ) -from app.models.geo import GeoCacheStatsResponse, GeoDetail, GeoInfo, IpLookupResponse +from app.models.geo import GeoCacheStatsResponse, GeoDetail, GeoInfo, GeoReResolveResponse, IpLookupResponse from app.services import geo_service, jail_service from app.utils.fail2ban_client import Fail2BanConnectionError @@ -136,12 +133,13 @@ async def geo_stats( @router.post( "/re-resolve", summary="Re-resolve all IPs whose country could not be determined", + response_model=GeoReResolveResponse, ) async def re_resolve_geo( _auth: AuthDep, db: Annotated[aiosqlite.Connection, Depends(get_db)], http_session: HttpSessionDep, -) -> dict[str, int]: +) -> GeoReResolveResponse: """Retry geo resolution for every IP in ``geo_cache`` with a null country. Clears the in-memory negative cache first so that previously failing IPs @@ -153,22 +151,6 @@ async def re_resolve_geo( http_session: Shared HTTP session for geo lookups. Returns: - JSON object ``{"resolved": N, "total": M}`` where *N* is the number - of IPs that gained a country code and *M* is the total number of IPs - that were retried. + A :class:`~app.models.geo.GeoReResolveResponse` with retry counts. """ - # Collect all IPs in geo_cache that still lack a country code. - unresolved = await geo_service.get_unresolved_ips(db) - - if not unresolved: - return {"resolved": 0, "total": 0} - - # Clear negative cache so these IPs bypass the TTL check. - geo_service.clear_neg_cache() - - geo_map = await geo_service.lookup_batch(unresolved, http_session, db=db) - - resolved_count = sum( - 1 for info in geo_map.values() if info.country_code is not None - ) - return {"resolved": resolved_count, "total": len(unresolved)} + return await geo_service.re_resolve_all(db, http_session) diff --git a/backend/app/services/geo_service.py b/backend/app/services/geo_service.py index 2ac40c4..90f8e4a 100644 --- a/backend/app/services/geo_service.py +++ b/backend/app/services/geo_service.py @@ -187,6 +187,41 @@ async def get_unresolved_ips(db: aiosqlite.Connection) -> list[str]: return await geo_cache_repo.get_unresolved_ips(db) +async def re_resolve_all( + db: aiosqlite.Connection, + http_session: aiohttp.ClientSession, +) -> dict[str, int]: + """Retry geo resolution for all unresolved cache entries. + + This helper clears the in-memory negative cache before attempting a + fresh batch lookup, then returns counters for how many IPs were retried + and how many gained a resolved country code. + + Args: + db: BanGUI application database connection. + http_session: Shared aiohttp client session. + + Returns: + A dict with ``resolved`` and ``total`` counts. + """ + unresolved = await get_unresolved_ips(db) + if not unresolved: + return {"resolved": 0, "total": 0} + + clear_neg_cache() + geo_map = await lookup_batch(unresolved, http_session, db=db) + resolved_count = sum( + 1 for info in geo_map.values() if info.country_code is not None + ) + + log.info( + "geo_re_resolve_complete", + total=len(unresolved), + resolved=resolved_count, + ) + return {"resolved": resolved_count, "total": len(unresolved)} + + def init_geoip(mmdb_path: str | None) -> None: """Initialise the MaxMind GeoLite2-Country database reader. diff --git a/backend/tests/test_routers/test_geo.py b/backend/tests/test_routers/test_geo.py index 8940829..3244133 100644 --- a/backend/tests/test_routers/test_geo.py +++ b/backend/tests/test_routers/test_geo.py @@ -48,7 +48,9 @@ async def geo_client(tmp_path: Path) -> AsyncClient: # type: ignore[misc] transport = ASGITransport(app=app) async with AsyncClient(transport=transport, base_url="http://test") as ac: - await ac.post("/api/setup", json=_SETUP_PAYLOAD) + setup_payload = _SETUP_PAYLOAD.copy() + setup_payload["database_path"] = settings.database_path + await ac.post("/api/setup", json=setup_payload) login = await ac.post( "/api/auth/login", json={"password": _SETUP_PAYLOAD["master_password"]}, @@ -170,15 +172,15 @@ class TestReResolve: async def test_returns_200_with_counts(self, geo_client: AsyncClient) -> None: """POST /api/geo/re-resolve returns 200 with resolved/total counts.""" with patch( - "app.routers.geo.geo_service.lookup_batch", - AsyncMock(return_value={}), + "app.routers.geo.geo_service.re_resolve_all", + AsyncMock(return_value={"resolved": 0, "total": 0}), ): resp = await geo_client.post("/api/geo/re-resolve") assert resp.status_code == 200 data = resp.json() - assert "resolved" in data - assert "total" in data + assert data["resolved"] == 0 + assert data["total"] == 0 async def test_empty_when_no_unresolved_ips(self, geo_client: AsyncClient) -> None: """Returns resolved=0, total=0 when geo_cache has no NULL country_code rows.""" diff --git a/backend/tests/test_services/test_geo_service.py b/backend/tests/test_services/test_geo_service.py index 9393ee5..9c99c3f 100644 --- a/backend/tests/test_services/test_geo_service.py +++ b/backend/tests/test_services/test_geo_service.py @@ -842,6 +842,54 @@ class TestLookupCachedOnly: assert uncached.count("9.9.9.9") == 1 +class TestReResolveAll: + """Tests for :func:`~app.services.geo_service.re_resolve_all`.""" + + async def test_returns_zero_when_no_unresolved_ips(self) -> None: + """The service returns zero counts when there are no unresolved IPs.""" + db = MagicMock() + session = MagicMock() + + with patch( + "app.services.geo_service.get_unresolved_ips", + AsyncMock(return_value=[]), + ), patch("app.services.geo_service.lookup_batch", AsyncMock()) as mock_lookup, patch( + "app.services.geo_service.clear_neg_cache", + MagicMock(), + ) as mock_clear: + result = await geo_service.re_resolve_all(db, session) + + assert result == {"resolved": 0, "total": 0} + mock_clear.assert_not_called() + mock_lookup.assert_not_called() + + async def test_clears_neg_cache_and_returns_counts(self) -> None: + """The service clears negative cache and returns resolved and total counts.""" + db = MagicMock() + session = MagicMock() + ips = ["1.1.1.1", "2.2.2.2"] + geo_map = { + "1.1.1.1": GeoInfo(country_code="DE", country_name="Germany", asn=None, org=None), + "2.2.2.2": GeoInfo(country_code=None, country_name=None, asn=None, org=None), + } + + with patch( + "app.services.geo_service.get_unresolved_ips", + AsyncMock(return_value=ips), + ), patch( + "app.services.geo_service.lookup_batch", + AsyncMock(return_value=geo_map), + ) as mock_lookup, patch( + "app.services.geo_service.clear_neg_cache", + MagicMock(), + ) as mock_clear: + result = await geo_service.re_resolve_all(db, session) + + assert result == {"resolved": 1, "total": 2} + mock_clear.assert_called_once() + mock_lookup.assert_awaited_once_with(ips, session, db=db) + + # --------------------------------------------------------------------------- # Bulk DB writes via executemany (Task 3) # ---------------------------------------------------------------------------