diff --git a/.env.example b/.env.example index fd65dac..e743d3e 100644 --- a/.env.example +++ b/.env.example @@ -26,3 +26,18 @@ BANGUI_FRONTEND_PORT=5173 # Public port (optional, defaults to 8080) # When using production compose, this is the public-facing port BANGUI_PORT=8080 + +# IP Geolocation (optional) +# Path to MaxMind GeoLite2-Country MMDB database file (primary resolver). +# Download from: https://www.maxmind.com/en/geolite2/signup +# If not set, geolocation is disabled (or falls back to HTTP if enabled below). +# Example: /data/GeoLite2-Country.mmdb +BANGUI_GEOIP_DB_PATH= + +# IP Geolocation HTTP Fallback (optional, defaults to false) +# ⚠️ SECURITY WARNING: Only enable if you cannot mount the MaxMind database. +# When enabled, unresolved IP addresses are sent unencrypted to ip-api.com. +# This is a privacy and GDPR/CCPA concern. Do NOT enable in production unless necessary. +# Set to "true" to enable (default is "false" for security). +BANGUI_GEOIP_ALLOW_HTTP_FALLBACK=false + diff --git a/Docs/Architekture.md b/Docs/Architekture.md index 9a751e6..5b99c69 100644 --- a/Docs/Architekture.md +++ b/Docs/Architekture.md @@ -39,7 +39,8 @@ BanGUI is a two-tier web application with a clear separation between frontend an | **Backend** | Python 3.12+, FastAPI, Pydantic v2, aiosqlite | Business logic, data persistence, fail2ban communication, scheduling | | **Application Database** | SQLite (via aiosqlite) | Stores BanGUI's own data: configuration, session state, blocklist sources, import logs | | **fail2ban** | Unix domain socket | The monitored service — BanGUI reads status, issues commands, and reads the fail2ban database | -| **External APIs** | HTTP (via aiohttp) | IP geolocation, ASN/RIR lookups, blocklist downloads | +| **MaxMind GeoLite2** | Offline MMDB file (mounted into container) | IP geolocation (primary resolver) — local, encrypted | +| **External APIs** | HTTP (via aiohttp) | Blocklist downloads; IP geolocation fallback (only if MMDB unavailable and HTTP fallback enabled) | --- @@ -196,7 +197,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_cache.py` | **GeoCache** class that encapsulates all IP geolocation caching: resolves IP addresses to country, ASN, and organization using external APIs or a local MaxMind database, maintains in-memory and persistent caches with negative cache support, and manages background re-resolution. Instantiated once at startup and stored on `app.state.geo_cache` | +| `geo_cache.py` | **GeoCache** class that encapsulates all IP geolocation caching: resolves IP addresses to country, ASN, and organization using a primary local MaxMind GeoLite2-Country database (if available) with optional HTTP fallback to ip-api.com (disabled by default for security). Maintains in-memory and persistent caches with negative cache support, and manages background re-resolution. Instantiated once at startup with allow_http_fallback flag and stored on `app.state.geo_cache` | | `geo_service.py` | (Deprecated) Backward-compatibility wrappers that delegate to the `GeoCache` instance. Kept for compatibility with existing code. New code should use `GeoCache` directly or via dependency injection | | `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 | @@ -671,7 +672,7 @@ BanGUI maintains its **own SQLite database** (separate from the fail2ban databas - The frontend `AuthProvider` checks session validity on mount and redirects to `/login` if invalid. - The backend `dependencies.py` provides an `authenticated` dependency that validates the session cookie on every protected endpoint. - **Session validation cache** (`InMemorySessionCache` in `app.utils.session_cache`) — validated session tokens are cached in memory for 10 seconds (configurable via `session_cache_ttl_seconds`) to avoid a SQLite round-trip on every request from the same browser. The cache is invalidated immediately on logout. **⚠️ This cache is process-local and not safe for multi-worker or distributed deployments.** In single-worker mode (enforced by TASK-002), this is safe and improves performance. For multi-worker deployments, replace `InMemorySessionCache` with a shared backend (Redis, database, shared memory) implementing the `SessionCache` protocol. See `app/utils/session_cache.py` module docstring for implementation details. -- **GeoCache** — `GeoCache` instance is created at startup and stored on `app.state.geo_cache`. It encapsulates all IP geolocation caching: in-memory lookup cache, negative cache for unresolvable IPs (with TTL), dirty set for persistence, and thread-safe async locking. Cache is loaded from the `geo_cache` SQLite table on startup. New resolutions are accumulated in memory and periodically flushed to the database by the `geo_cache_flush` background task. Stale entries are re-resolved by the `geo_re_resolve` task. Injected into routes and tasks via FastAPI's dependency system. +- **GeoCache** — `GeoCache` instance is created at startup with a configurable `allow_http_fallback` flag and stored on `app.state.geo_cache`. It implements a primary + fallback resolution strategy: (1) try local MaxMind GeoLite2-Country MMDB database (primary, encrypted, no network traffic), (2) if unavailable/no result and allowed, fall back to ip-api.com HTTP API (unencrypted, disabled by default for security). Encapsulates in-memory lookup cache, negative cache for unresolvable IPs (5-minute TTL), dirty set for persistence, and thread-safe async locking. Cache is loaded from the `geo_cache` SQLite table on startup. New resolutions are accumulated in memory and periodically flushed to the database by the `geo_cache_flush` background task. Stale entries are re-resolved by the `geo_re_resolve` task. Injected into routes and tasks via FastAPI's dependency system. See Backend-Development.md § IP Geolocation Resolution for setup and security details. - **Runtime state** (`RuntimeState` in `app.utils.runtime_state`) — stores mutable application state: `server_status` (fail2ban online/offline), `last_activation` (jail activation tracking), `pending_recovery` (crash detection), and `runtime_settings` (effective configuration). **⚠️ RuntimeState is process-local and only safe when BanGUI runs as a single asyncio worker.** Mutations must not span `await` points (cooperative scheduling within a single event loop is safe). In multi-worker deployments, each process has its own copy — logouts from worker A don't affect worker B's cache, health status updates are per-worker, and activation tracking is unreliable. BanGUI enforces single-worker mode (TASK-002) to prevent this issue. For future multi-worker support, replace RuntimeState with a shared coordination backend (Redis, shared memory, database). See `app/utils/runtime_state.py` module docstring for details. - **Setup-completion flag** — once `is_setup_complete()` returns `True`, the result is stored in `app.state._setup_complete_cached`. The `SetupRedirectMiddleware` skips the DB query on all subsequent requests, removing 1 SQL query per request for the common post-setup case. The completion flag is only written after the runtime database is successfully initialized and all initial setup settings are persisted, preventing a failed setup from permanently bypassing the setup wizard. diff --git a/Docs/Backend-Development.md b/Docs/Backend-Development.md index 1c05e25..3b766cd 100644 --- a/Docs/Backend-Development.md +++ b/Docs/Backend-Development.md @@ -917,12 +917,76 @@ BANGUI_FAIL2BAN_START_COMMAND='"/opt/my tools/fail2ban" start' # Quoted path **Common Pitfall:** Using `.split()` instead of `shlex.split()` would break commands with spaces in paths. Always use quoted strings for paths that contain whitespace. +### IP Geolocation Resolution + +BanGUI resolves IP addresses to country codes and network organization information for ban analytics and geomapping. The geolocation system implements a **primary + fallback** resolution strategy to balance security and availability: + +1. **Primary Resolver (MaxMind GeoLite2-Country):** All IP lookups first attempt resolution using a local MaxMind GeoLite2-Country MMDB database file (if available). The MMDB is downloaded offline and mounted into the container — no IP data is sent over the network. +2. **Fallback Resolver (ip-api.com HTTP):** If the MMDB is unavailable or returns no result, the system can fall back to the ip-api.com HTTP API. **This fallback must be explicitly enabled** and only sends unresolved IPs over HTTP. HTTP is disabled by default for security (to avoid sending IP addresses in cleartext). + +**Download & Configure MaxMind GeoLite2:** + +The MaxMind GeoLite2-Country MMDB requires a free account and license key. To set up the database: + +1. **Create a free MaxMind account** at https://www.maxmind.com/en/geolite2/signup and download your license key. +2. **Download the GeoLite2-Country MMDB** using the provided script or manually from the MaxMind downloads page. +3. **Mount the MMDB into the BanGUI container** at a known path (e.g., `/data/GeoLite2-Country.mmdb`). +4. **Set `BANGUI_GEOIP_DB_PATH`** to the mounted path in your environment. + +Example Docker Compose configuration: + +```yaml +services: + bangui: + volumes: + - ./GeoLite2-Country.mmdb:/data/GeoLite2-Country.mmdb:ro + environment: + BANGUI_GEOIP_DB_PATH: /data/GeoLite2-Country.mmdb +``` + +**Fallback to HTTP (Not Recommended):** + +If the MMDB cannot be mounted (e.g., in restricted environments), you can enable the HTTP fallback: + +```yaml +services: + bangui: + environment: + BANGUI_GEOIP_ALLOW_HTTP_FALLBACK: "true" +``` + +**⚠️ Security Warning:** Enabling HTTP fallback causes unresolved IP addresses to be sent **unencrypted** to ip-api.com. This is a privacy and GDPR/CCPA concern. Only enable this if the MMDB absolutely cannot be provisioned, and understand the implications. + +**Data Structure:** + +The `GeoInfo` returned by the resolution system includes: +- `country_code` (str | None): ISO 3166-1 alpha-2 country code (e.g., `"US"`, `"DE"`). +- `country_name` (str | None): Human-readable country name (e.g., `"United States"`). +- `asn` (str | None): Autonomous System Number (e.g., `"AS3320"`). Only populated when using the HTTP API; local MMDB lookups return `None`. +- `org` (str | None): Organization name associated with the ASN. Only populated when using the HTTP API; local MMDB lookups return `None`. + +**Environment Variables:** + +```bash +BANGUI_GEOIP_DB_PATH=/data/GeoLite2-Country.mmdb # Path to MaxMind MMDB (primary) +BANGUI_GEOIP_ALLOW_HTTP_FALLBACK="false" # Default: false (MMDB-only) +BANGUI_GEOIP_ALLOW_HTTP_FALLBACK="true" # Enable HTTP fallback (not recommended) +``` + +**Caching & Performance:** + +- Resolved IPs are cached in-memory and persisted to SQLite for fast subsequent lookups. +- Failed lookups are cached for 5 minutes to avoid hammering external APIs. +- The background `geo_cache_flush` task (runs every 60 seconds) persists newly resolved entries to the database. +- The background `geo_re_resolve` task (configurable schedule) periodically re-resolves stale entries to keep data fresh. + ### API Documentation Configuration The `enable_docs` setting controls whether FastAPI serves interactive API documentation at `/api/docs` (Swagger UI) and `/api/redoc` (ReDoc). **Default:** `false` — API documentation is disabled by default to prevent information disclosure in production. + **When to Enable:** - Set `BANGUI_ENABLE_DOCS=true` in development and debugging environments only. - Never enable in production. Exposed API documentation reveals all endpoints, request/response schemas, and allows direct API invocation from the browser. diff --git a/Docs/Tasks.md b/Docs/Tasks.md index accdb78..b964a34 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -1,40 +1,3 @@ -## TASK-029 — `Fail2BanConnectionError` leaks socket path in HTTP error responses - -**Severity:** Medium - -### Where found -`backend/app/exceptions.py` — `Fail2BanConnectionError.__init__()` formats the message as `f"{message} (socket: {socket_path})"`. `backend/app/main.py` — `_fail2ban_connection_handler()` returns `{"detail": f"Cannot reach fail2ban: {exc}"}` verbatim. - -### Why this is needed -Every 502 response caused by fail2ban being unreachable includes the full socket path (e.g., `Cannot reach fail2ban: [Errno 2] No such file or directory (socket: /var/run/fail2ban/fail2ban.sock)`) in the JSON error body. This discloses internal infrastructure details to unauthenticated users who can trigger the error. Similarly, `_fail2ban_protocol_handler` includes raw exception details that may expose internal parsing logic. - -### Goal -Return generic, user-friendly error messages in HTTP responses. Log full details server-side only. - -### What to do -1. In `_fail2ban_connection_handler()`, replace: - ```python - content={"detail": f"Cannot reach fail2ban: {exc}"} - ``` - with: - ```python - content={"detail": "Cannot reach the fail2ban service. Check the server status page."} - ``` -2. In `_fail2ban_protocol_handler()`, similarly return a generic message. -3. Both handlers already log `error=str(exc)` server-side — this is correct and should remain. - -### Possible traps and issues -- Update any tests that assert the exact `detail` string in 502 responses. -- If the frontend displays this error message directly to the user, ensure it still makes sense after genericizing. - -### Docs changes needed -- `Backend-Development.md` — error message hygiene (no internal paths/details in responses). - -### Doc references -- [Backend-Development.md](Backend-Development.md) — error handling - ---- - ## TASK-030 — ip-api.com geo lookups use plain HTTP — IP addresses sent unencrypted **Severity:** Medium diff --git a/backend/app/config.py b/backend/app/config.py index c2607c8..8967557 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -134,7 +134,17 @@ class Settings(BaseSettings): default=None, description=( "Optional path to a MaxMind GeoLite2-Country .mmdb file. " - "When set, failed ip-api.com lookups fall back to local resolution." + "When set, it is used as the primary resolver for IP geolocation. " + "The ip-api.com HTTP API is only used as a fallback when the MMDB is unavailable or returns no result." + ), + ) + geoip_allow_http_fallback: bool = Field( + default=False, + description=( + "Allow fallback to ip-api.com HTTP API when the MaxMind database is unavailable. " + "WARNING: Enabling this sends unencrypted IP addresses over HTTP. " + "Only use this flag when the MMDB cannot be mounted and you understand the security implications. " + "Default is False (only use local MMDB, fail if unavailable)." ), ) fail2ban_config_dir: str = Field( diff --git a/backend/app/services/geo_cache.py b/backend/app/services/geo_cache.py index 04419b4..ffa3600 100644 --- a/backend/app/services/geo_cache.py +++ b/backend/app/services/geo_cache.py @@ -76,22 +76,37 @@ class GeoCache: Encapsulates all mutable state needed for geo-IP resolution. Provides methods for single lookups, batch lookups, persistence, and cache management. + Primary resolution strategy: + 1. Check in-memory cache + 2. Check negative cache (recently failed IPs within TTL) + 3. Try local MaxMind GeoLite2-Country database (if available) + 4. If allow_http_fallback is True, try ip-api.com HTTP API + 5. Record as negative cache entry if all resolvers fail + State: _cache: In-memory positive results cache (``ip → GeoInfo``). _neg_cache: Failed lookup timestamps (``ip → epoch``). _dirty: IPs added but not yet persisted to database. _geoip_reader: Optional MaxMind GeoLite2 reader. _geoip_initialized: Indicates whether init_geoip() has been called. + _allow_http_fallback: Whether to use ip-api.com as fallback. _cache_lock: Async lock protecting cache mutations. """ - def __init__(self) -> None: - """Initialize an empty GeoCache.""" + def __init__(self, allow_http_fallback: bool = False) -> None: + """Initialize an empty GeoCache. + + Args: + allow_http_fallback: Whether to fall back to ip-api.com HTTP API + when the MaxMind database is unavailable. Default is False + (fail rather than send IPs unencrypted). + """ self._cache: dict[str, GeoInfo] = {} self._neg_cache: dict[str, float] = {} self._dirty: set[str] = set() self._geoip_reader: geoip2.database.Reader | None = None self._geoip_initialized: bool = False + self._allow_http_fallback: bool = allow_http_fallback self._cache_lock: asyncio.Lock = asyncio.Lock() async def clear(self) -> None: @@ -323,6 +338,13 @@ class GeoCache: ) -> GeoInfo | None: """Resolve an IP address to country, ASN, and organisation metadata. + Resolution strategy (in order): + 1. Check in-memory cache + 2. Check negative cache (skip if within TTL) + 3. Try local MaxMind GeoLite2-Country database (primary resolver) + 4. If allow_http_fallback is True, try ip-api.com HTTP API (unencrypted) + 5. Record as negative cache entry if all resolvers fail + Results are cached in-process. If the cache exceeds ``_MAX_CACHE_SIZE`` entries it is flushed before the new result is stored. @@ -350,12 +372,44 @@ class GeoCache: 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) + # PRIMARY RESOLVER: Try local MaxMind database first. + result = self._geoip_lookup(ip) + if result is not None: + await self._store(ip, result) + if result.country_code is not None and db is not None: + try: + await geo_cache_repo.upsert_entry_and_commit( + db=db, + ip=ip, + country_code=result.country_code, + country_name=result.country_name, + asn=result.asn, + org=result.org, + ) + except Exception as exc: # noqa: BLE001 + log.warning("geo_persist_failed", ip=ip, error=str(exc)) + log.debug("geo_lookup_success_mmdb", ip=ip, country=result.country_code) + return result + + # FALLBACK RESOLVER: Try ip-api.com HTTP API only if explicitly allowed. + if not self._allow_http_fallback: + log.debug("geo_lookup_failed_no_http_fallback", ip=ip) + async with self._cache_lock: + self._neg_cache[ip] = time.monotonic() + if db is not None: + try: + await geo_cache_repo.upsert_neg_entry_and_commit(db=db, ip=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) + + # HTTP API call (only when allow_http_fallback is True). url: str = _API_URL.format(ip=ip) api_ok = False try: async with http_session.get(url, timeout=aiohttp.ClientTimeout(total=_REQUEST_TIMEOUT)) as resp: if resp.status != 200: - log.warning("geo_lookup_non_200", ip=ip, status=resp.status) + log.warning("geo_lookup_http_non_200", ip=ip, status=resp.status) else: data: dict[str, object] = await resp.json(content_type=None) if data.get("status") == "success": @@ -374,41 +428,22 @@ class GeoCache: ) 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) + log.debug("geo_lookup_success_http", ip=ip, country=result.country_code, asn=result.asn) return result log.debug( - "geo_lookup_failed", + "geo_lookup_http_failed", ip=ip, message=data.get("message", "unknown"), ) except Exception as exc: # noqa: BLE001 log.warning( - "geo_lookup_request_failed", + "geo_lookup_http_request_failed", ip=ip, exc_type=type(exc).__name__, error=repr(exc), ) if not api_ok: - # Try local MaxMind database as fallback. - fallback = self._geoip_lookup(ip) - if fallback is not None: - await self._store(ip, fallback) - if fallback.country_code is not None and db is not None: - try: - await geo_cache_repo.upsert_entry_and_commit( - db=db, - ip=ip, - country_code=fallback.country_code, - country_name=fallback.country_name, - asn=fallback.asn, - org=fallback.org, - ) - 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 - # Both resolvers failed — record in negative cache to avoid hammering. async with self._cache_lock: self._neg_cache[ip] = time.monotonic() @@ -461,10 +496,17 @@ class GeoCache: http_session: aiohttp.ClientSession, db: aiosqlite.Connection | None = None, ) -> dict[str, GeoInfo]: - """Resolve multiple IP addresses in bulk using ip-api.com batch endpoint. + """Resolve multiple IP addresses in bulk. + + Resolution strategy: + 1. Return cached entries immediately (both positive and negative cache) + 2. For uncached IPs, try local MaxMind database first + 3. If allow_http_fallback is True, use ip-api.com batch endpoint for remaining + 4. Record unresolvable IPs in negative cache IPs already present in the in-memory cache are returned immediately - without making an HTTP request. Uncached IPs are sent to + without making an HTTP request. Uncached IPs are first resolved via + the local MaxMind database, then (if enabled) sent to ``http://ip-api.com/batch`` in chunks of up to :data:`_BATCH_SIZE`. Only successful resolutions (``country_code is not None``) are written to @@ -491,7 +533,7 @@ class GeoCache: if ip in self._cache: geo_result[ip] = self._cache[ip] elif ip in self._neg_cache and (now - self._neg_cache[ip]) < _NEG_CACHE_TTL: - # Recently failed — skip API call, return empty result. + # Recently failed — skip resolution, return empty result. geo_result[ip] = _empty else: uncached.append(ip) @@ -501,8 +543,67 @@ class GeoCache: log.info("geo_batch_lookup_start", total=len(uncached)) - for batch_idx, chunk_start in enumerate(range(0, len(uncached), _BATCH_SIZE)): - chunk = uncached[chunk_start : chunk_start + _BATCH_SIZE] + # PRIMARY: Try local MaxMind database for all uncached IPs. + pos_rows: list[tuple[str, str | None, str | None, str | None, str | None]] = [] + neg_ips: list[str] = [] + remaining_uncached: list[str] = [] + + for ip in uncached: + mmdb_result = self._geoip_lookup(ip) + if mmdb_result is not None: + await self._store(ip, mmdb_result) + geo_result[ip] = mmdb_result + if db is not None: + pos_rows.append( + (ip, mmdb_result.country_code, mmdb_result.country_name, mmdb_result.asn, mmdb_result.org) + ) + else: + # MMDB lookup failed — keep for potential HTTP fallback or final failure. + remaining_uncached.append(ip) + + # Persist MMDB results if any. + if db is not None and pos_rows: + try: + await geo_cache_repo.bulk_upsert_entries_and_commit(db, pos_rows) + except Exception as exc: # noqa: BLE001 + log.warning( + "geo_batch_persist_mmdb_failed", + count=len(pos_rows), + error=str(exc), + ) + + # FALLBACK: Try HTTP API only if enabled and there are remaining IPs. + if not self._allow_http_fallback or not remaining_uncached: + # Record remaining as negative cache. + for ip in remaining_uncached: + async with self._cache_lock: + self._neg_cache[ip] = time.monotonic() + geo_result[ip] = _empty + neg_ips.append(ip) + + if db is not None and neg_ips: + try: + await geo_cache_repo.bulk_upsert_neg_entries_and_commit(db, neg_ips) + except Exception as exc: # noqa: BLE001 + log.warning( + "geo_batch_persist_neg_failed", + count=len(neg_ips), + error=str(exc), + ) + + log.info( + "geo_batch_lookup_complete", + requested=len(uncached), + resolved=sum(1 for g in geo_result.values() if g.country_code is not None), + ) + return geo_result + + # HTTP API batch processing. + pos_rows.clear() + neg_ips.clear() + + for batch_idx, chunk_start in enumerate(range(0, len(remaining_uncached), _BATCH_SIZE)): + chunk = remaining_uncached[chunk_start : chunk_start + _BATCH_SIZE] # Throttle: pause between consecutive HTTP calls to stay within the # ip-api.com free-tier rate limit (45 req/min). @@ -532,13 +633,9 @@ class GeoCache: assert chunk_result is not None # noqa: S101 - # Collect bulk-write rows instead of one execute per IP. - pos_rows: list[tuple[str, str | None, str | None, str | None, str | None]] = [] - neg_ips: list[str] = [] - for ip, info in chunk_result.items(): if info.country_code is not None: - # Successful API resolution. + # Successful HTTP resolution. await self._store(ip, info) geo_result[ip] = info if db is not None: @@ -546,28 +643,12 @@ class GeoCache: (ip, info.country_code, info.country_name, info.asn, info.org) ) else: - # API failed — try local GeoIP fallback. - fallback = self._geoip_lookup(ip) - if fallback is not None: - await self._store(ip, fallback) - geo_result[ip] = fallback - if db is not None: - pos_rows.append( - ( - ip, - fallback.country_code, - fallback.country_name, - fallback.asn, - fallback.org, - ) - ) - else: - # Both resolvers failed — record in negative cache. - async with self._cache_lock: - self._neg_cache[ip] = time.monotonic() - geo_result[ip] = _empty - if db is not None: - neg_ips.append(ip) + # HTTP failed — record as negative cache. + async with self._cache_lock: + self._neg_cache[ip] = time.monotonic() + geo_result[ip] = _empty + if db is not None: + neg_ips.append(ip) if db is not None and (pos_rows or neg_ips): try: @@ -583,6 +664,8 @@ class GeoCache: negative_count=len(neg_ips), error=str(exc), ) + pos_rows.clear() + neg_ips.clear() log.info( "geo_batch_lookup_complete", diff --git a/backend/app/startup.py b/backend/app/startup.py index d1c10cf..2d774f2 100644 --- a/backend/app/startup.py +++ b/backend/app/startup.py @@ -143,7 +143,7 @@ async def startup_shared_resources( ) # Create and initialize the GeoCache instance - geo_cache = GeoCache() + geo_cache = GeoCache(allow_http_fallback=settings.geoip_allow_http_fallback) if Path(settings.database_path).resolve() != original_db_path: runtime_db = await open_db(settings.database_path) try: @@ -164,6 +164,18 @@ async def startup_shared_resources( http_session: aiohttp.ClientSession = _create_http_session(settings) geo_cache.init_geoip(settings.geoip_db_path) + + # Warn if HTTP fallback is enabled (security warning). + if settings.geoip_allow_http_fallback: + log.warning( + "geoip_http_fallback_enabled", + message=( + "WARNING: IP geolocation HTTP fallback is enabled. " + "IP addresses will be sent unencrypted to ip-api.com if the MaxMind database is unavailable. " + "This is a security and privacy risk. Disable BANGUI_GEOIP_ALLOW_HTTP_FALLBACK in production." + ), + ) + app.state.geo_cache = geo_cache scheduler: AsyncIOScheduler | None = None diff --git a/backend/tests/test_services/test_geo_service.py b/backend/tests/test_services/test_geo_service.py index 89994b3..06ad741 100644 --- a/backend/tests/test_services/test_geo_service.py +++ b/backend/tests/test_services/test_geo_service.py @@ -9,11 +9,11 @@ import pytest from app.models.geo import GeoInfo from app.services.geo_cache import ( - GeoCache, _BATCH_DELAY, _BATCH_MAX_RETRIES, _BATCH_SIZE, _NEG_CACHE_TTL, + GeoCache, ) # --------------------------------------------------------------------------- @@ -52,8 +52,18 @@ def _make_session(response_json: dict[str, object], status: int = 200) -> MagicM @pytest.fixture async def geo_cache() -> GeoCache: - """Provide a fresh GeoCache instance for each test.""" - return GeoCache() + """Provide a fresh GeoCache instance for each test with HTTP fallback enabled. + + Most tests expect HTTP API to be available and do not set up MMDB. + For testing MMDB-first behavior, use geo_cache_mmdb_only fixture instead. + """ + return GeoCache(allow_http_fallback=True) + + +@pytest.fixture +async def geo_cache_mmdb_only() -> GeoCache: + """Provide a fresh GeoCache instance with HTTP fallback disabled (MMDB-only mode).""" + return GeoCache(allow_http_fallback=False) def test_init_geoip_is_startup_only(geo_cache: GeoCache, tmp_path) -> None: @@ -316,7 +326,12 @@ class TestNegativeCache: class TestGeoipFallback: - """Verify the MaxMind GeoLite2 fallback is used when ip-api fails.""" + """Verify the MaxMind GeoLite2 is used as the primary resolver. + + With the new implementation, MMDB (MaxMind GeoLite2-Country) is the primary + resolver, tried first before the HTTP API. HTTP is only used if MMDB is + unavailable or returns no result (and allow_http_fallback is enabled). + """ def _make_geoip_reader(self, iso_code: str, name: str) -> MagicMock: """Build a mock geoip2.database.Reader that returns *iso_code*.""" @@ -331,8 +346,33 @@ class TestGeoipFallback: reader.country = MagicMock(return_value=response_mock) return reader - async def test_geoip_fallback_called_when_api_fails(self, geo_cache: GeoCache) -> None: - """When ip-api returns status=fail, the geoip2 reader is consulted.""" + async def test_geoip_primary_resolver_success(self, geo_cache: GeoCache) -> None: + """MMDB is used as the primary resolver and HTTP API is skipped on success.""" + session = _make_session( + { + "status": "success", + "countryCode": "JP", + "country": "Japan", + "as": "AS12345", + "org": "NTT", + } + ) + mock_reader = self._make_geoip_reader("DE", "Germany") + + with patch.object(geo_cache, "_geoip_reader", mock_reader): + result = await geo_cache.lookup("1.2.3.4", session) + + # MMDB should be called (primary resolver). + mock_reader.country.assert_called_once_with("1.2.3.4") + # HTTP API should NOT be called since MMDB succeeded. + session.get.assert_not_called() + # Result should be from MMDB, not HTTP. + assert result is not None + assert result.country_code == "DE" + assert result.country_name == "Germany" + + async def test_geoip_fallback_when_http_fails(self, geo_cache: GeoCache) -> None: + """When HTTP API fails, MMDB is used (it's already tried first).""" session = _make_session({"status": "fail", "message": "reserved range"}) mock_reader = self._make_geoip_reader("DE", "Germany") @@ -345,46 +385,30 @@ class TestGeoipFallback: assert result.country_name == "Germany" async def test_geoip_fallback_result_stored_in_cache(self, geo_cache: GeoCache) -> None: - """A successful geoip2 fallback result is stored in the positive cache.""" + """A successful geoip2 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_cache, "_geoip_reader", mock_reader): await geo_cache.lookup("8.8.8.8", session) - # Second call must be served from positive cache without hitting API. + # Second call must be served from positive cache without hitting API or MMDB. await geo_cache.lookup("8.8.8.8", session) - assert session.get.call_count == 1 + # MMDB should only be called once (not on second call due to cache). + assert mock_reader.country.call_count == 1 + # HTTP API should never be called since MMDB succeeded. + assert session.get.call_count == 0 assert "8.8.8.8" in geo_cache._cache - async def test_geoip_fallback_not_called_on_api_success(self, geo_cache: GeoCache) -> 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_cache, "_geoip_reader", mock_reader): - result = await geo_cache.lookup("1.2.3.4", session) - - 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, geo_cache: GeoCache) -> None: - """When no geoip2 reader is configured, the fallback silently does nothing.""" + """When no geoip2 reader is configured, resolution falls back to HTTP (if enabled).""" session = _make_session({"status": "fail", "message": "private range"}) with patch.object(geo_cache, "_geoip_reader", None): result = await geo_cache.lookup("10.0.0.1", session) assert result is not None + # With HTTP fallback enabled but no result from HTTP, country_code is None. assert result.country_code is None @@ -725,7 +749,7 @@ class TestErrorLogging: """ async def test_empty_message_exception_logs_exc_type(self, geo_cache: GeoCache) -> None: - """When exception str() is empty, exc_type and repr are still logged.""" + """When HTTP exception str() is empty, exc_type and repr are still logged.""" class _EmptyMessageError(Exception): """Exception whose str() representation is empty.""" @@ -741,13 +765,16 @@ class TestErrorLogging: import structlog.testing - with structlog.testing.capture_logs() as captured: + with structlog.testing.capture_logs() as captured, patch.object( + geo_cache, "_geoip_reader", None + ): + # Ensure MMDB is not available so HTTP is tried. result = await geo_cache.lookup("197.221.98.153", session) assert result is not None assert result.country_code is None - request_failed = [e for e in captured if e.get("event") == "geo_lookup_request_failed"] + request_failed = [e for e in captured if e.get("event") == "geo_lookup_http_request_failed"] assert len(request_failed) == 1 event = request_failed[0] # exc_type must name the exception class — never empty. @@ -756,7 +783,7 @@ class TestErrorLogging: assert "_EmptyMessageError" in event["error"] async def test_connection_error_logs_exc_type(self, geo_cache: GeoCache) -> None: - """A standard OSError with message is logged both in error and exc_type.""" + """A standard OSError with message is logged in HTTP request failure log.""" session = MagicMock() mock_ctx = AsyncMock() mock_ctx.__aenter__ = AsyncMock(side_effect=OSError("connection refused")) @@ -765,10 +792,13 @@ class TestErrorLogging: import structlog.testing - with structlog.testing.capture_logs() as captured: + with structlog.testing.capture_logs() as captured, patch.object( + geo_cache, "_geoip_reader", None + ): + # Ensure MMDB is not available so HTTP is tried. await geo_cache.lookup("10.0.0.1", session) - request_failed = [e for e in captured if e.get("event") == "geo_lookup_request_failed"] + request_failed = [e for e in captured if e.get("event") == "geo_lookup_http_request_failed"] assert len(request_failed) == 1 event = request_failed[0] assert event["exc_type"] == "OSError"