diff --git a/Docs/Tasks.md b/Docs/Tasks.md index 310a4db..50be70c 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -271,6 +271,8 @@ Reference: `Docs/Refactoring.md` for full analysis of each issue. **Docs changes needed:** Update `Docs/Refactoring.md`. +**Status:** Completed ✅ + **Why this is needed:** Adapter/closure construction is glue code that belongs at the service boundary or in a factory, not in the HTTP handler. Routers should not need to understand the geo service's lookup interface to serve history requests. --- diff --git a/backend/app/routers/geo.py b/backend/app/routers/geo.py index 59cedd6..bef5d29 100644 --- a/backend/app/routers/geo.py +++ b/backend/app/routers/geo.py @@ -58,14 +58,11 @@ async def lookup_ip( HTTPException: 400 when *ip* is not a valid IP address. HTTPException: 502 when fail2ban is unreachable. """ - async def _enricher(addr: str) -> geo_service.GeoInfo | None: - return await geo_service.lookup(addr, http_session) - try: result: IpLookupResult = await jail_service.lookup_ip( socket_path, ip, - geo_enricher=_enricher, + http_session=http_session, ) except ValueError as exc: raise HTTPException( diff --git a/backend/app/services/jail_service.py b/backend/app/services/jail_service.py index 3c45026..2f1db5b 100644 --- a/backend/app/services/jail_service.py +++ b/backend/app/services/jail_service.py @@ -19,7 +19,7 @@ from typing import TYPE_CHECKING, TypedDict, cast import structlog from app.exceptions import JailNotFoundError, JailOperationError -from app.models.ban import ActiveBan, ActiveBanListResponse, JailBannedIpsResponse +from app.models.ban import ActiveBan, JailBannedIpsResponse from app.models.config import BantimeEscalation from app.models.geo import GeoDetail from app.models.jail import ( @@ -29,6 +29,7 @@ from app.models.jail import ( JailStatus, JailSummary, ) +from app.services import geo_service from app.utils.config_file_utils import start_daemon, wait_for_fail2ban from app.utils.fail2ban_client import ( Fail2BanClient, @@ -179,6 +180,22 @@ def _ensure_list(value: object | None) -> list[str]: return [str(value)] +async def _resolve_geo_info( + ip: str, + *, + http_session: aiohttp.ClientSession | None = None, + geo_enricher: GeoEnricher | None = None, +) -> GeoInfo | None: + """Resolve geolocation using either a custom enricher or HTTP session.""" + if geo_enricher is not None: + return await geo_enricher(ip) + + if http_session is not None: + return await geo_service.lookup(ip, http_session) + + return None + + def _is_not_found_error(exc: Exception) -> bool: """Return ``True`` if *exc* indicates a jail does not exist. @@ -1163,6 +1180,7 @@ async def lookup_ip( socket_path: str, ip: str, geo_enricher: GeoEnricher | None = None, + http_session: aiohttp.ClientSession | None = None, ) -> IpLookupResult: """Return ban status and history for a single IP address. @@ -1223,9 +1241,13 @@ async def lookup_ip( pass geo: GeoDetail | None = None - if geo_enricher is not None: + if geo_enricher is not None or http_session is not None: with contextlib.suppress(Exception): # noqa: BLE001 - raw_geo = await geo_enricher(ip) + raw_geo = await _resolve_geo_info( + ip, + http_session=http_session, + geo_enricher=geo_enricher, + ) if raw_geo is not None: geo = GeoDetail( country_code=raw_geo.country_code, diff --git a/backend/tests/test_services/test_jail_service.py b/backend/tests/test_services/test_jail_service.py index 1f574b8..5ad91a6 100644 --- a/backend/tests/test_services/test_jail_service.py +++ b/backend/tests/test_services/test_jail_service.py @@ -864,6 +864,34 @@ class TestLookupIp: assert result["geo"].asn == "AS123" assert result["geo"].org == "Acme" + async def test_http_session_uses_geo_service_lookup(self) -> None: + """lookup_ip uses geo_service.lookup when http_session is provided.""" + responses = { + "get|--all|banned|1.2.3.4": (0, []), + "status": _make_global_status("sshd"), + "get|sshd|banip": (0, ["1.2.3.4", "5.6.7.8"]), + } + + mock_geo = GeoInfo(country_code="JP", country_name="Japan", asn=None, org=None) + mock_session = AsyncMock() + + with _patch_client(responses), patch( + "app.services.jail_service.geo_service.lookup", + AsyncMock(return_value=mock_geo), + ) as mock_lookup: + result = await jail_service.lookup_ip( + _SOCKET, + "1.2.3.4", + http_session=mock_session, + ) + + mock_lookup.assert_awaited_once_with("1.2.3.4", mock_session) + assert isinstance(result["geo"], GeoDetail) + assert result["geo"].country_code == "JP" + assert result["geo"].country_name == "Japan" + assert result["geo"].asn is None + assert result["geo"].org is None + async def test_invalid_ip_raises(self) -> None: """lookup_ip raises ValueError for invalid IP.""" with pytest.raises(ValueError, match="Invalid IP"):