From 12a859061ce8c2a6448e8a267957d0404542c885 Mon Sep 17 00:00:00 2001 From: Lukas Date: Sat, 7 Mar 2026 20:42:34 +0100 Subject: [PATCH] Fix missing country: neg cache, geoip2 fallback, re-resolve endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add 5-min negative cache (_neg_cache) so failing IPs are throttled rather than hammering the API on every request - Add MaxMind GeoLite2 fallback (init_geoip / _geoip_lookup) that fires when ip-api fails; controlled by BANGUI_GEOIP_DB_PATH env var - Fix lookup_batch bug: failed API results were stored in positive cache - Add _persist_neg_entry: INSERT OR IGNORE into geo_cache with NULL country_code so re-resolve can find historically failed IPs - Add POST /api/geo/re-resolve: clears neg cache, batch-retries all geo_cache rows with country_code IS NULL, returns resolved/total count - BanTable + MapPage: wrap the country — placeholder in a Fluent UI Tooltip explaining the retry behaviour - Add geoip2>=4.8.0 dependency; geoip_db_path config setting - Tests: add TestNegativeCache (4), TestGeoipFallback (4), TestReResolve (4) --- Docs/Tasks.md | 15 +- backend/app/config.py | 7 + backend/app/main.py | 1 + backend/app/routers/geo.py | 60 +++++- backend/app/services/geo_service.py | 202 +++++++++++++++--- backend/pyproject.toml | 1 + backend/tests/test_routers/test_geo.py | 58 +++++ .../tests/test_services/test_geo_service.py | 170 +++++++++++++-- frontend/src/components/BanTable.tsx | 18 +- frontend/src/pages/MapPage.tsx | 14 +- 10 files changed, 494 insertions(+), 52 deletions(-) diff --git a/Docs/Tasks.md b/Docs/Tasks.md index 052e645..078b54a 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -188,7 +188,20 @@ Dashboard and world map must load within **2 seconds** for 10 k banned IPs. Writ --- -## Task 4 — Fix Missing Country for Resolved IPs +## 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 diff --git a/backend/app/config.py b/backend/app/config.py index 56a0db3..8b865ae 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -45,6 +45,13 @@ class Settings(BaseSettings): default="info", description="Application log level: debug | info | warning | error | critical.", ) + geoip_db_path: str | None = Field( + default=None, + description=( + "Optional path to a MaxMind GeoLite2-Country .mmdb file. " + "When set, failed ip-api.com lookups fall back to local resolution." + ), + ) model_config = SettingsConfigDict( env_prefix="BANGUI_", diff --git a/backend/app/main.py b/backend/app/main.py index 0117bc6..aa97bdf 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -137,6 +137,7 @@ async def _lifespan(app: FastAPI) -> AsyncGenerator[None, None]: # --- Pre-warm geo cache from the persistent store --- from app.services import geo_service # noqa: PLC0415 + geo_service.init_geoip(settings.geoip_db_path) await geo_service.load_cache_from_db(db) # --- Background task scheduler --- diff --git a/backend/app/routers/geo.py b/backend/app/routers/geo.py index 36ebf79..70c3044 100644 --- a/backend/app/routers/geo.py +++ b/backend/app/routers/geo.py @@ -1,8 +1,9 @@ """Geo / IP lookup router. -Provides the IP enrichment endpoint: +Provides the IP enrichment endpoints: * ``GET /api/geo/lookup/{ip}`` — ban status, ban history, and geo info for an IP +* ``POST /api/geo/re-resolve`` — retry all previously failed geo lookups """ from __future__ import annotations @@ -12,9 +13,10 @@ from typing import TYPE_CHECKING, Annotated if TYPE_CHECKING: import aiohttp -from fastapi import APIRouter, HTTPException, Path, Request, status +import aiosqlite +from fastapi import APIRouter, Depends, HTTPException, Path, Request, status -from app.dependencies import AuthDep +from app.dependencies import AuthDep, get_db from app.models.geo import GeoDetail, IpLookupResponse from app.services import geo_service, jail_service from app.utils.fail2ban_client import Fail2BanConnectionError @@ -90,3 +92,55 @@ async def lookup_ip( currently_banned_in=result["currently_banned_in"], geo=geo_detail, ) + + +# --------------------------------------------------------------------------- +# POST /api/geo/re-resolve +# --------------------------------------------------------------------------- + + +@router.post( + "/re-resolve", + summary="Re-resolve all IPs whose country could not be determined", +) +async def re_resolve_geo( + request: Request, + _auth: AuthDep, + db: Annotated[aiosqlite.Connection, Depends(get_db)], +) -> dict[str, int]: + """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 + are immediately eligible for a new API attempt. + + Args: + request: Incoming request (used to access ``app.state.http_session``). + _auth: Validated session — enforces authentication. + db: BanGUI application database (for reading/writing ``geo_cache``). + + 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. + """ + # Collect all IPs in geo_cache that still lack a country code. + unresolved: list[str] = [] + async with db.execute( + "SELECT ip FROM geo_cache WHERE country_code IS NULL" + ) as cur: + async for row in cur: + unresolved.append(str(row[0])) + + if not unresolved: + return {"resolved": 0, "total": 0} + + # Clear negative cache so these IPs bypass the TTL check. + geo_service.clear_neg_cache() + + http_session: aiohttp.ClientSession = request.app.state.http_session + 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)} diff --git a/backend/app/services/geo_service.py b/backend/app/services/geo_service.py index 5ef2c28..93b869c 100644 --- a/backend/app/services/geo_service.py +++ b/backend/app/services/geo_service.py @@ -38,9 +38,12 @@ Usage:: from __future__ import annotations +import time from dataclasses import dataclass from typing import TYPE_CHECKING +import geoip2.database +import geoip2.errors import structlog if TYPE_CHECKING: @@ -74,6 +77,10 @@ _MAX_CACHE_SIZE: int = 50_000 #: Timeout for outgoing geo API requests in seconds. _REQUEST_TIMEOUT: float = 5.0 +#: How many seconds a failed lookup result is suppressed before the IP is +#: eligible for a new API attempt. Default: 5 minutes. +_NEG_CACHE_TTL: float = 300.0 + # --------------------------------------------------------------------------- # Domain model # --------------------------------------------------------------------------- @@ -104,16 +111,83 @@ class GeoInfo: # Internal cache # --------------------------------------------------------------------------- -#: Module-level in-memory cache: ``ip → GeoInfo``. +#: Module-level in-memory cache: ``ip → GeoInfo`` (positive results only). _cache: dict[str, GeoInfo] = {} +#: Negative cache: ``ip → epoch timestamp`` of last failed lookup attempt. +#: Entries within :data:`_NEG_CACHE_TTL` seconds are not re-queried. +_neg_cache: dict[str, float] = {} + +#: Optional MaxMind GeoLite2 reader initialised by :func:`init_geoip`. +_geoip_reader: geoip2.database.Reader | None = None + def clear_cache() -> None: - """Flush the entire lookup cache. + """Flush both the positive and negative lookup caches. Useful in tests and when the operator suspects stale data. """ _cache.clear() + _neg_cache.clear() + + +def clear_neg_cache() -> None: + """Flush only the negative (failed-lookups) cache. + + Useful when triggering a manual re-resolve so that previously failed + IPs are immediately eligible for a new API attempt. + """ + _neg_cache.clear() + + +def init_geoip(mmdb_path: str | None) -> None: + """Initialise the MaxMind GeoLite2-Country database reader. + + If *mmdb_path* is ``None``, empty, or the file does not exist the + fallback is silently disabled — ip-api.com remains the sole resolver. + + Args: + mmdb_path: Absolute path to a ``GeoLite2-Country.mmdb`` file. + """ + global _geoip_reader # noqa: PLW0603 + if not mmdb_path: + return + from pathlib import Path # noqa: PLC0415 + + if not Path(mmdb_path).is_file(): + log.warning("geoip_mmdb_not_found", path=mmdb_path) + return + _geoip_reader = geoip2.database.Reader(mmdb_path) + log.info("geoip_mmdb_loaded", path=mmdb_path) + + +def _geoip_lookup(ip: str) -> GeoInfo | None: + """Attempt a local MaxMind GeoLite2 lookup for *ip*. + + Returns ``None`` when the reader is not initialised, the IP is not in + the database, or any other error occurs. + + Args: + ip: IPv4 or IPv6 address string. + + Returns: + A :class:`GeoInfo` with at least ``country_code`` populated, or + ``None`` when resolution is impossible. + """ + if _geoip_reader is None: + return None + try: + response = _geoip_reader.country(ip) + code: str | None = response.country.iso_code or None + name: str | None = response.country.name or None + if code is None: + return None + return GeoInfo(country_code=code, country_name=name, asn=None, org=None) + except geoip2.errors.AddressNotFoundError: + return None + except Exception as exc: # noqa: BLE001 + log.warning("geoip_lookup_failed", ip=ip, error=str(exc)) + return None # --------------------------------------------------------------------------- @@ -181,6 +255,23 @@ async def _persist_entry( await db.commit() +async def _persist_neg_entry(db: aiosqlite.Connection, ip: str) -> None: + """Record a failed lookup attempt in ``geo_cache`` with all-NULL fields. + + Uses ``INSERT OR IGNORE`` so that an existing *positive* entry (one that + has a ``country_code``) is never overwritten by a later failure. + + Args: + db: BanGUI application database connection. + ip: IP address string whose resolution failed. + """ + await db.execute( + "INSERT OR IGNORE INTO geo_cache (ip) VALUES (?)", + (ip,), + ) + await db.commit() + + # --------------------------------------------------------------------------- # Public API — single lookup # --------------------------------------------------------------------------- @@ -215,36 +306,60 @@ async def lookup( if ip in _cache: return _cache[ip] + # Negative cache: skip IPs that recently failed to avoid hammering the API. + neg_ts = _neg_cache.get(ip) + if neg_ts is not None and (time.monotonic() - neg_ts) < _NEG_CACHE_TTL: + return GeoInfo(country_code=None, country_name=None, asn=None, org=None) + url: str = _API_URL.format(ip=ip) + api_ok = False try: async with http_session.get(url, timeout=_REQUEST_TIMEOUT) as resp: # type: ignore[arg-type] if resp.status != 200: log.warning("geo_lookup_non_200", ip=ip, status=resp.status) - return None - - data: dict[str, object] = await resp.json(content_type=None) + else: + data: dict[str, object] = await resp.json(content_type=None) + if data.get("status") == "success": + api_ok = True + result = _parse_single_response(data) + _store(ip, result) + if result.country_code is not None and db is not None: + try: + await _persist_entry(db, ip, result) + except Exception as exc: # noqa: BLE001 + log.warning("geo_persist_failed", ip=ip, error=str(exc)) + log.debug("geo_lookup_success", ip=ip, country=result.country_code, asn=result.asn) + return result + log.debug( + "geo_lookup_failed", + ip=ip, + message=data.get("message", "unknown"), + ) except Exception as exc: # noqa: BLE001 log.warning("geo_lookup_request_failed", ip=ip, error=str(exc)) - return None - if data.get("status") != "success": - log.debug( - "geo_lookup_failed", - ip=ip, - message=data.get("message", "unknown"), - ) - # Do NOT cache failed lookups — they will be retried on the next call. - return GeoInfo(country_code=None, country_name=None, asn=None, org=None) + if not api_ok: + # Try local MaxMind database as fallback. + fallback = _geoip_lookup(ip) + if fallback is not None: + _store(ip, fallback) + if fallback.country_code is not None and db is not None: + try: + await _persist_entry(db, ip, fallback) + except Exception as exc: # noqa: BLE001 + log.warning("geo_persist_failed", ip=ip, error=str(exc)) + log.debug("geo_geoip_fallback_success", ip=ip, country=fallback.country_code) + return fallback - result = _parse_single_response(data) - _store(ip, result) - if result.country_code is not None and db is not None: - try: - await _persist_entry(db, ip, result) - except Exception as exc: # noqa: BLE001 - log.warning("geo_persist_failed", ip=ip, error=str(exc)) - log.debug("geo_lookup_success", ip=ip, country=result.country_code, asn=result.asn) - return result + # Both resolvers failed — record in negative cache to avoid hammering. + _neg_cache[ip] = time.monotonic() + if db is not None: + try: + await _persist_neg_entry(db, ip) + except Exception as exc: # noqa: BLE001 + log.warning("geo_persist_neg_failed", ip=ip, error=str(exc)) + + return GeoInfo(country_code=None, country_name=None, asn=None, org=None) # --------------------------------------------------------------------------- @@ -277,11 +392,16 @@ async def lookup_batch( """ geo_result: dict[str, GeoInfo] = {} uncached: list[str] = [] + _empty = GeoInfo(country_code=None, country_name=None, asn=None, org=None) unique_ips = list(dict.fromkeys(ips)) # deduplicate, preserve order + now = time.monotonic() for ip in unique_ips: if ip in _cache: geo_result[ip] = _cache[ip] + elif ip in _neg_cache and (now - _neg_cache[ip]) < _NEG_CACHE_TTL: + # Recently failed — skip API call, return empty result. + geo_result[ip] = _empty else: uncached.append(ip) @@ -295,13 +415,35 @@ async def lookup_batch( chunk_result = await _batch_api_call(chunk, http_session) for ip, info in chunk_result.items(): - _store(ip, info) - geo_result[ip] = info - if info.country_code is not None and db is not None: - try: - await _persist_entry(db, ip, info) - except Exception as exc: # noqa: BLE001 - log.warning("geo_persist_failed", ip=ip, error=str(exc)) + if info.country_code is not None: + # Successful API resolution. + _store(ip, info) + geo_result[ip] = info + if db is not None: + try: + await _persist_entry(db, ip, info) + except Exception as exc: # noqa: BLE001 + log.warning("geo_persist_failed", ip=ip, error=str(exc)) + else: + # API failed — try local GeoIP fallback. + fallback = _geoip_lookup(ip) + if fallback is not None: + _store(ip, fallback) + geo_result[ip] = fallback + if db is not None: + try: + await _persist_entry(db, ip, fallback) + except Exception as exc: # noqa: BLE001 + log.warning("geo_persist_failed", ip=ip, error=str(exc)) + else: + # Both resolvers failed — record in negative cache. + _neg_cache[ip] = time.monotonic() + geo_result[ip] = _empty + if db is not None: + try: + await _persist_neg_entry(db, ip) + except Exception as exc: # noqa: BLE001 + log.warning("geo_persist_neg_failed", ip=ip, error=str(exc)) log.info( "geo_batch_lookup_complete", diff --git a/backend/pyproject.toml b/backend/pyproject.toml index 04f4e94..beab707 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -17,6 +17,7 @@ dependencies = [ "apscheduler>=3.10,<4.0", "structlog>=24.4.0", "bcrypt>=4.2.0", + "geoip2>=4.8.0", ] [project.optional-dependencies] diff --git a/backend/tests/test_routers/test_geo.py b/backend/tests/test_routers/test_geo.py index 45194cc..7410ca7 100644 --- a/backend/tests/test_routers/test_geo.py +++ b/backend/tests/test_routers/test_geo.py @@ -157,3 +157,61 @@ class TestGeoLookup: assert resp.status_code == 200 assert resp.json()["ip"] == "2001:db8::1" + + +# --------------------------------------------------------------------------- +# POST /api/geo/re-resolve +# --------------------------------------------------------------------------- + + +class TestReResolve: + """Tests for ``POST /api/geo/re-resolve``.""" + + 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={}), + ): + 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 + + 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.""" + resp = await geo_client.post("/api/geo/re-resolve") + + assert resp.status_code == 200 + assert resp.json() == {"resolved": 0, "total": 0} + + async def test_re_resolves_null_ips(self, geo_client: AsyncClient) -> None: + """IPs with null country_code in geo_cache are re-resolved via lookup_batch.""" + # Insert a NULL entry into geo_cache. + app = geo_client._transport.app # type: ignore[attr-defined] + db: aiosqlite.Connection = app.state.db + await db.execute("INSERT OR IGNORE INTO geo_cache (ip) VALUES (?)", ("5.5.5.5",)) + await db.commit() + + geo_result = {"5.5.5.5": GeoInfo(country_code="FR", country_name="France", asn=None, org=None)} + with patch( + "app.routers.geo.geo_service.lookup_batch", + AsyncMock(return_value=geo_result), + ): + resp = await geo_client.post("/api/geo/re-resolve") + + assert resp.status_code == 200 + data = resp.json() + assert data["total"] == 1 + assert data["resolved"] == 1 + + async def test_401_when_unauthenticated(self, geo_client: AsyncClient) -> None: + """POST /api/geo/re-resolve requires authentication.""" + app = geo_client._transport.app # type: ignore[attr-defined] + resp = await AsyncClient( + transport=ASGITransport(app=app), + base_url="http://test", + ).post("/api/geo/re-resolve") + assert resp.status_code == 401 diff --git a/backend/tests/test_services/test_geo_service.py b/backend/tests/test_services/test_geo_service.py index d9eb0b2..4487b44 100644 --- a/backend/tests/test_services/test_geo_service.py +++ b/backend/tests/test_services/test_geo_service.py @@ -2,7 +2,7 @@ from __future__ import annotations -from unittest.mock import AsyncMock, MagicMock +from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -166,8 +166,8 @@ class TestLookupCaching: assert session.get.call_count == 2 - async def test_negative_result_not_cached(self) -> None: - """A failed lookup (status != success) is NOT cached so it is retried.""" + async def test_negative_result_stored_in_neg_cache(self) -> None: + """A failed lookup is stored in the negative cache, so the second call is blocked.""" session = _make_session( {"status": "fail", "message": "reserved range"} ) @@ -175,8 +175,8 @@ class TestLookupCaching: await geo_service.lookup("192.168.1.1", session) # type: ignore[arg-type] await geo_service.lookup("192.168.1.1", session) # type: ignore[arg-type] - # Failed lookups must not be cached — both calls must reach the API. - assert session.get.call_count == 2 + # Second call is blocked by the negative cache — only one API hit. + assert session.get.call_count == 1 # --------------------------------------------------------------------------- @@ -187,19 +187,26 @@ class TestLookupCaching: class TestLookupFailures: """geo_service.lookup() when things go wrong.""" - async def test_non_200_response_returns_none(self) -> None: - """A 429 or 500 status returns ``None`` without caching.""" + async def test_non_200_response_returns_null_geo_info(self) -> None: + """A 429 or 500 status returns GeoInfo with null fields (not None).""" session = _make_session({}, status=429) result = await geo_service.lookup("1.2.3.4", session) # type: ignore[arg-type] - assert result is None + assert result is not None + assert isinstance(result, GeoInfo) + assert result.country_code is None - async def test_network_error_returns_none(self) -> None: - """A network exception returns ``None``.""" + async def test_network_error_returns_null_geo_info(self) -> None: + """A network exception returns GeoInfo with null fields (not None).""" session = MagicMock() - session.get = MagicMock(side_effect=OSError("connection refused")) + mock_ctx = AsyncMock() + mock_ctx.__aenter__ = AsyncMock(side_effect=OSError("connection refused")) + mock_ctx.__aexit__ = AsyncMock(return_value=False) + session.get = MagicMock(return_value=mock_ctx) result = await geo_service.lookup("10.0.0.1", session) # type: ignore[arg-type] - assert result is None + assert result is not None + assert isinstance(result, GeoInfo) + assert result.country_code is None async def test_failed_status_returns_geo_info_with_nulls(self) -> None: """When ip-api returns ``status=fail`` a GeoInfo with null fields is returned (but not cached).""" @@ -210,3 +217,142 @@ class TestLookupFailures: assert isinstance(result, GeoInfo) assert result.country_code is None assert result.country_name is None + + +# --------------------------------------------------------------------------- +# Negative cache +# --------------------------------------------------------------------------- + + +class TestNegativeCache: + """Verify the negative cache throttles retries for failing IPs.""" + + async def test_neg_cache_blocks_second_lookup(self) -> None: + """After a failed lookup the second call is served from the neg cache.""" + session = _make_session({"status": "fail", "message": "private range"}) + + r1 = await geo_service.lookup("192.0.2.1", session) # type: ignore[arg-type] + r2 = await geo_service.lookup("192.0.2.1", session) # type: ignore[arg-type] + + # Only one HTTP call should have been made; second served from neg cache. + assert session.get.call_count == 1 + assert r1 is not None and r1.country_code is None + assert r2 is not None and r2.country_code is None + + async def test_neg_cache_retries_after_ttl(self) -> None: + """When the neg-cache entry is older than the TTL a new API call is made.""" + session = _make_session({"status": "fail", "message": "private range"}) + + await geo_service.lookup("192.0.2.2", session) # type: ignore[arg-type] + + # Manually expire the neg-cache entry. + geo_service._neg_cache["192.0.2.2"] -= geo_service._NEG_CACHE_TTL + 1 # type: ignore[attr-defined] + + await geo_service.lookup("192.0.2.2", session) # type: ignore[arg-type] + + # Both calls should have hit the API. + assert session.get.call_count == 2 + + async def test_clear_neg_cache_allows_immediate_retry(self) -> None: + """After clearing the neg cache the IP is eligible for a new API call.""" + session = _make_session({"status": "fail", "message": "private range"}) + + await geo_service.lookup("192.0.2.3", session) # type: ignore[arg-type] + geo_service.clear_neg_cache() + await geo_service.lookup("192.0.2.3", session) # type: ignore[arg-type] + + assert session.get.call_count == 2 + + async def test_successful_lookup_does_not_pollute_neg_cache(self) -> None: + """A successful lookup must not create a neg-cache entry.""" + session = _make_session( + { + "status": "success", + "countryCode": "DE", + "country": "Germany", + "as": "AS3320", + "org": "Telekom", + } + ) + + await geo_service.lookup("1.2.3.4", session) # type: ignore[arg-type] + + assert "1.2.3.4" not in geo_service._neg_cache # type: ignore[attr-defined] + + +# --------------------------------------------------------------------------- +# GeoIP2 (MaxMind) fallback +# --------------------------------------------------------------------------- + + +class TestGeoipFallback: + """Verify the MaxMind GeoLite2 fallback is used when ip-api fails.""" + + def _make_geoip_reader(self, iso_code: str, name: str) -> MagicMock: + """Build a mock geoip2.database.Reader that returns *iso_code*.""" + country_mock = MagicMock() + country_mock.iso_code = iso_code + country_mock.name = name + + response_mock = MagicMock() + response_mock.country = country_mock + + reader = MagicMock() + reader.country = MagicMock(return_value=response_mock) + return reader + + async def test_geoip_fallback_called_when_api_fails(self) -> None: + """When ip-api returns status=fail, the geoip2 reader is consulted.""" + session = _make_session({"status": "fail", "message": "reserved range"}) + mock_reader = self._make_geoip_reader("DE", "Germany") + + with patch.object(geo_service, "_geoip_reader", mock_reader): + result = await geo_service.lookup("1.2.3.4", session) # type: ignore[arg-type] + + mock_reader.country.assert_called_once_with("1.2.3.4") + assert result is not None + assert result.country_code == "DE" + assert result.country_name == "Germany" + + async def test_geoip_fallback_result_stored_in_cache(self) -> None: + """A successful geoip2 fallback result is stored in the positive cache.""" + session = _make_session({"status": "fail", "message": "reserved range"}) + mock_reader = self._make_geoip_reader("US", "United States") + + with patch.object(geo_service, "_geoip_reader", mock_reader): + await geo_service.lookup("8.8.8.8", session) # type: ignore[arg-type] + # Second call must be served from positive cache without hitting API. + await geo_service.lookup("8.8.8.8", session) # type: ignore[arg-type] + + assert session.get.call_count == 1 + assert "8.8.8.8" in geo_service._cache # type: ignore[attr-defined] + + async def test_geoip_fallback_not_called_on_api_success(self) -> None: + """When ip-api succeeds, the geoip2 reader must not be consulted.""" + session = _make_session( + { + "status": "success", + "countryCode": "JP", + "country": "Japan", + "as": "AS12345", + "org": "NTT", + } + ) + mock_reader = self._make_geoip_reader("XX", "Nowhere") + + with patch.object(geo_service, "_geoip_reader", mock_reader): + result = await geo_service.lookup("1.2.3.4", session) # type: ignore[arg-type] + + mock_reader.country.assert_not_called() + assert result is not None + assert result.country_code == "JP" + + async def test_geoip_fallback_not_called_when_no_reader(self) -> None: + """When no geoip2 reader is configured, the fallback silently does nothing.""" + session = _make_session({"status": "fail", "message": "private range"}) + + with patch.object(geo_service, "_geoip_reader", None): + result = await geo_service.lookup("10.0.0.1", session) # type: ignore[arg-type] + + assert result is not None + assert result.country_code is None diff --git a/frontend/src/components/BanTable.tsx b/frontend/src/components/BanTable.tsx index 1a8ddef..4becf40 100644 --- a/frontend/src/components/BanTable.tsx +++ b/frontend/src/components/BanTable.tsx @@ -153,11 +153,19 @@ function buildBanColumns(styles: ReturnType): TableColumnDefin createTableColumn({ columnId: "country", renderHeaderCell: () => "Country", - renderCell: (item) => ( - - {item.country_name ?? item.country_code ?? "—"} - - ), + renderCell: (item) => + item.country_name ?? item.country_code ? ( + {item.country_name ?? item.country_code} + ) : ( + + + — + + + ), }), createTableColumn({ columnId: "jail", diff --git a/frontend/src/pages/MapPage.tsx b/frontend/src/pages/MapPage.tsx index 731f564..df59c88 100644 --- a/frontend/src/pages/MapPage.tsx +++ b/frontend/src/pages/MapPage.tsx @@ -24,6 +24,7 @@ import { Text, Toolbar, ToolbarButton, + Tooltip, makeStyles, tokens, } from "@fluentui/react-components"; @@ -286,7 +287,18 @@ export function MapPage(): React.JSX.Element { - {ban.country_name ?? ban.country_code ?? "—"} + {ban.country_name ?? ban.country_code ? ( + ban.country_name ?? ban.country_code + ) : ( + + + — + + + )}