lukas.pupkalipinski/BanGUI
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix missing country: neg cache, geoip2 fallback, re-resolve endpoint

Browse Source 
- 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)
This commit is contained in:
Lukas
2026-03-07 20:42:34 +01:00
parent ddfc8a0b02
commit 12a859061c
10 changed files with 494 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
backend/app/config.py
View File

@@ -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_",

1
backend/app/main.py
View File

@@ -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 ---

60
backend/app/routers/geo.py
View File

@@ -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)}

202
backend/app/services/geo_service.py
View File

@@ -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",

1
backend/pyproject.toml
View File

@@ -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]

58
backend/tests/test_routers/test_geo.py
View File

@@ -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

170
backend/tests/test_services/test_geo_service.py
View File

@@ -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