Refactor geo enrichment into jail_service and mark Task 14 done
This commit is contained in:
@@ -271,6 +271,8 @@ Reference: `Docs/Refactoring.md` for full analysis of each issue.
|
|||||||
|
|
||||||
**Docs changes needed:** Update `Docs/Refactoring.md`.
|
**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.
|
**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.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|||||||
@@ -58,14 +58,11 @@ async def lookup_ip(
|
|||||||
HTTPException: 400 when *ip* is not a valid IP address.
|
HTTPException: 400 when *ip* is not a valid IP address.
|
||||||
HTTPException: 502 when fail2ban is unreachable.
|
HTTPException: 502 when fail2ban is unreachable.
|
||||||
"""
|
"""
|
||||||
async def _enricher(addr: str) -> geo_service.GeoInfo | None:
|
|
||||||
return await geo_service.lookup(addr, http_session)
|
|
||||||
|
|
||||||
try:
|
try:
|
||||||
result: IpLookupResult = await jail_service.lookup_ip(
|
result: IpLookupResult = await jail_service.lookup_ip(
|
||||||
socket_path,
|
socket_path,
|
||||||
ip,
|
ip,
|
||||||
geo_enricher=_enricher,
|
http_session=http_session,
|
||||||
)
|
)
|
||||||
except ValueError as exc:
|
except ValueError as exc:
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
|
|||||||
@@ -19,7 +19,7 @@ from typing import TYPE_CHECKING, TypedDict, cast
|
|||||||
import structlog
|
import structlog
|
||||||
|
|
||||||
from app.exceptions import JailNotFoundError, JailOperationError
|
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.config import BantimeEscalation
|
||||||
from app.models.geo import GeoDetail
|
from app.models.geo import GeoDetail
|
||||||
from app.models.jail import (
|
from app.models.jail import (
|
||||||
@@ -29,6 +29,7 @@ from app.models.jail import (
|
|||||||
JailStatus,
|
JailStatus,
|
||||||
JailSummary,
|
JailSummary,
|
||||||
)
|
)
|
||||||
|
from app.services import geo_service
|
||||||
from app.utils.config_file_utils import start_daemon, wait_for_fail2ban
|
from app.utils.config_file_utils import start_daemon, wait_for_fail2ban
|
||||||
from app.utils.fail2ban_client import (
|
from app.utils.fail2ban_client import (
|
||||||
Fail2BanClient,
|
Fail2BanClient,
|
||||||
@@ -179,6 +180,22 @@ def _ensure_list(value: object | None) -> list[str]:
|
|||||||
return [str(value)]
|
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:
|
def _is_not_found_error(exc: Exception) -> bool:
|
||||||
"""Return ``True`` if *exc* indicates a jail does not exist.
|
"""Return ``True`` if *exc* indicates a jail does not exist.
|
||||||
|
|
||||||
@@ -1163,6 +1180,7 @@ async def lookup_ip(
|
|||||||
socket_path: str,
|
socket_path: str,
|
||||||
ip: str,
|
ip: str,
|
||||||
geo_enricher: GeoEnricher | None = None,
|
geo_enricher: GeoEnricher | None = None,
|
||||||
|
http_session: aiohttp.ClientSession | None = None,
|
||||||
) -> IpLookupResult:
|
) -> IpLookupResult:
|
||||||
"""Return ban status and history for a single IP address.
|
"""Return ban status and history for a single IP address.
|
||||||
|
|
||||||
@@ -1223,9 +1241,13 @@ async def lookup_ip(
|
|||||||
pass
|
pass
|
||||||
|
|
||||||
geo: GeoDetail | None = None
|
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
|
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:
|
if raw_geo is not None:
|
||||||
geo = GeoDetail(
|
geo = GeoDetail(
|
||||||
country_code=raw_geo.country_code,
|
country_code=raw_geo.country_code,
|
||||||
|
|||||||
@@ -864,6 +864,34 @@ class TestLookupIp:
|
|||||||
assert result["geo"].asn == "AS123"
|
assert result["geo"].asn == "AS123"
|
||||||
assert result["geo"].org == "Acme"
|
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:
|
async def test_invalid_ip_raises(self) -> None:
|
||||||
"""lookup_ip raises ValueError for invalid IP."""
|
"""lookup_ip raises ValueError for invalid IP."""
|
||||||
with pytest.raises(ValueError, match="Invalid IP"):
|
with pytest.raises(ValueError, match="Invalid IP"):
|
||||||
|
|||||||
Reference in New Issue
Block a user