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

Refactor ban management with domain models and mappers

Browse Source 
- Add ban domain model for core business logic separation
- Implement mapper pattern for DTO/domain conversions
- Update ban service with new domain-driven approach
- Refactor router endpoints to use new architecture
- Add comprehensive mapper tests
- Update documentation with architecture changes

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Lukas
2026-04-28 07:46:02 +02:00
parent 507f153ab9
commit 3888c5eb3f
11 changed files with 640 additions and 68 deletions
Download Patch File Download Diff File Expand all files Collapse all files

104
backend/app/services/ban_service.py
View File

@@ -23,21 +23,21 @@ from app.models.ban import (
BLOCKLIST_JAIL,
BUCKET_SECONDS,
BUCKET_SIZE_LABEL,
ActiveBan,
ActiveBanListResponse,
BanOrigin,
BansByCountryResponse,
BansByJailResponse,
BanTrendBucket,
BanTrendResponse,
DashboardBanItem,
DashboardBanListResponse,
TimeRange,
_derive_origin,
bucket_count,
)
from app.models.ban import (
JailBanCount as JailBanCountModel,
from app.models.ban_domain import (
DomainActiveBan,
DomainActiveBanList,
DomainBansByCountry,
DomainBansByJail,
DomainBanTrend,
DomainBanTrendBucket,
DomainDashboardBanItem,
DomainDashboardBanList,
DomainJailBanCount,
)
from app.repositories import fail2ban_db_repo
from app.repositories import history_archive_repo as default_history_archive_repo
@@ -140,7 +140,7 @@ def _origin_sql_filter(origin: BanOrigin | None) -> tuple[str, tuple[str, ...]]:
return "", ()
def _parse_ban_entry(entry: str, jail: str) -> ActiveBan | None:
def _parse_ban_entry(entry: str, jail: str) -> DomainActiveBan | None:
"""Parse a ban entry from ``get <jail> banip --with-time`` output."""
from datetime import UTC, datetime
@@ -151,7 +151,7 @@ def _parse_ban_entry(entry: str, jail: str) -> ActiveBan | None:
ipaddress.ip_address(ip)
if len(parts) < 2:
return ActiveBan(
return DomainActiveBan(
ip=ip,
jail=jail,
banned_at=None,
@@ -187,7 +187,7 @@ def _parse_ban_entry(entry: str, jail: str) -> ActiveBan | None:
if expires_at_str:
expires_at_iso = _to_iso(expires_at_str)
return ActiveBan(
return DomainActiveBan(
ip=ip,
jail=jail,
banned_at=banned_at_iso,
@@ -201,19 +201,29 @@ def _parse_ban_entry(entry: str, jail: str) -> ActiveBan | None:
async def _enrich_bans(
bans: list[ActiveBan],
bans: list[DomainActiveBan],
geo_enricher: GeoEnricher,
) -> list[ActiveBan]:
) -> list[DomainActiveBan]:
"""Enrich ban records with geo data asynchronously."""
geo_results: list[object | Exception] = await asyncio.gather(
*[cast("Awaitable[object]", geo_enricher(ban.ip)) for ban in bans],
return_exceptions=True,
)
enriched: list[ActiveBan] = []
enriched: list[DomainActiveBan] = []
for ban, geo in zip(bans, geo_results, strict=False):
if geo is not None and not isinstance(geo, Exception):
geo_info = cast("GeoInfo", geo)
enriched.append(ban.model_copy(update={"country": geo_info.country_code}))
# Create new instance with updated country
enriched.append(
DomainActiveBan(
ip=ban.ip,
jail=ban.jail,
banned_at=ban.banned_at,
expires_at=ban.expires_at,
ban_count=ban.ban_count,
country=geo_info.country_code,
)
)
else:
enriched.append(ban)
return enriched
@@ -225,7 +235,7 @@ async def get_active_bans(
geo_enricher: GeoEnricher | None = None,
http_session: aiohttp.ClientSession | None = None,
app_db: aiosqlite.Connection | None = None,
) -> ActiveBanListResponse:
) -> DomainActiveBanList:
"""Return all currently banned IPs across every jail.
For each jail the ``get <jail> banip --with-time`` command is used
@@ -253,7 +263,7 @@ async def get_active_bans(
meaningful when *http_session* is provided.
Returns:
:class:`~app.models.ban.ActiveBanListResponse` with all active bans.
:class:`~app.models.ban_domain.DomainActiveBanList` with all active bans.
Raises:
~app.utils.fail2ban_client.Fail2BanConnectionError: If the socket
@@ -271,14 +281,14 @@ async def get_active_bans(
)
if not jail_names:
return ActiveBanListResponse(bans=[], total=0)
return DomainActiveBanList(bans=[], total=0)
results: list[object | Exception] = await asyncio.gather(
*[client.send(["get", jn, "banip", "--with-time"]) for jn in jail_names],
return_exceptions=True,
)
bans: list[ActiveBan] = []
bans: list[DomainActiveBan] = []
for jail_name, raw_result in zip(jail_names, results, strict=False):
if isinstance(raw_result, Exception):
log.warning(
@@ -322,7 +332,7 @@ async def get_active_bans(
bans = await _enrich_bans(bans, geo_enricher)
log.info("active_bans_fetched", total=len(bans))
return ActiveBanListResponse(bans=bans, total=len(bans))
return DomainActiveBanList(bans=bans, total=len(bans))
# ---------------------------------------------------------------------------
# Public API
@@ -342,7 +352,7 @@ async def list_bans(
geo_enricher: GeoEnricher | None = None,
history_archive_repo: HistoryArchiveRepository = default_history_archive_repo,
origin: BanOrigin | None = None,
) -> DashboardBanListResponse:
) -> DomainDashboardBanList:
"""Return a paginated list of bans within the selected time window.
Queries the fail2ban database ``bans`` table for records whose
@@ -376,7 +386,7 @@ async def list_bans(
the ``blocklist-import`` jail, ``"selfblock"`` excludes it.
Returns:
:class:`~app.models.ban.DashboardBanListResponse` containing the
:class:`~app.models.ban_domain.DomainDashboardBanList` containing the
paginated items and total count.
"""
@@ -428,7 +438,7 @@ async def list_bans(
except (TimeoutError, aiohttp.ClientError, OSError):
log.warning("ban_service_batch_geo_failed_list_bans")
items: list[DashboardBanItem] = []
items: list[DomainDashboardBanItem] = []
for row in rows:
if source == "archive":
jail = str(row["jail"])
@@ -471,7 +481,7 @@ async def list_bans(
log.error("ban_service_geo_lookup_unexpected_error", ip=ip, error=type(exc).__name__)
items.append(
DashboardBanItem(
DomainDashboardBanItem(
ip=ip,
jail=jail,
banned_at=banned_at,
@@ -485,7 +495,7 @@ async def list_bans(
)
)
return DashboardBanListResponse(
return DomainDashboardBanList(
items=items,
total=total,
page=page,
@@ -514,7 +524,7 @@ async def bans_by_country(
app_db: aiosqlite.Connection | None = None,
origin: BanOrigin | None = None,
country_code: str | None = None,
) -> BansByCountryResponse:
) -> DomainBansByCountry:
"""Aggregate ban counts per country for the selected time window.
Uses a two-step strategy optimised for large datasets:
@@ -547,7 +557,7 @@ async def bans_by_country(
the ``blocklist-import`` jail, ``"selfblock"`` excludes it.
Returns:
:class:`~app.models.ban.BansByCountryResponse` with per-country
:class:`~app.models.ban_domain.DomainBansByCountry` with per-country
aggregation and the companion ban list.
"""
@@ -722,7 +732,7 @@ async def bans_by_country(
country_names[cc] = cn
# Build companion table from recent rows (geo already cached from batch step).
bans: list[ActiveBan] = []
bans: list[DomainDashboardBanItem] = []
for companion_row in companion_rows:
if source == "archive":
ip = companion_row["ip"]
@@ -745,7 +755,7 @@ async def bans_by_country(
org: str | None = geo.org if geo else None
bans.append(
DashboardBanItem(
DomainDashboardBanItem(
ip=ip,
jail=jail,
banned_at=banned_at,
@@ -759,10 +769,10 @@ async def bans_by_country(
)
)
return BansByCountryResponse(
return DomainBansByCountry(
countries=countries,
country_names=country_names,
bans=bans,
items=bans,
total=total,
)
@@ -780,7 +790,7 @@ async def ban_trend(
history_archive_repo: HistoryArchiveRepository = default_history_archive_repo,
app_db: aiosqlite.Connection | None = None,
origin: BanOrigin | None = None,
) -> BanTrendResponse:
) -> DomainBanTrend:
"""Return ban counts aggregated into equal-width time buckets.
Queries the fail2ban database ``bans`` table and groups records by a
@@ -804,7 +814,7 @@ async def ban_trend(
``blocklist-import`` jail, ``"selfblock"`` excludes it.
Returns:
:class:`~app.models.ban.BanTrendResponse` with a full bucket list
:class:`~app.models.ban_domain.DomainBanTrend` with a full bucket list
and the human-readable bucket-size label.
"""
since: int = since_unix(range_)
@@ -861,15 +871,15 @@ async def ban_trend(
origin=origin,
)
buckets: list[BanTrendBucket] = [
BanTrendBucket(
buckets: list[DomainBanTrendBucket] = [
DomainBanTrendBucket(
timestamp=ts_to_iso(since + i * bucket_secs),
count=counts[i],
)
for i in range(num_buckets)
]
return BanTrendResponse(
return DomainBanTrend(
buckets=buckets,
bucket_size=BUCKET_SIZE_LABEL[range_],
)
@@ -888,7 +898,7 @@ async def bans_by_jail(
history_archive_repo: HistoryArchiveRepository = default_history_archive_repo,
app_db: aiosqlite.Connection | None = None,
origin: BanOrigin | None = None,
) -> BansByJailResponse:
) -> DomainBansByJail:
"""Return ban counts aggregated per jail for the selected time window.
Queries the fail2ban database ``bans`` table, groups records by jail
@@ -904,7 +914,7 @@ async def bans_by_jail(
``blocklist-import`` jail, ``"selfblock"`` excludes it.
Returns:
:class:`~app.models.ban.BansByJailResponse` with per-jail counts
:class:`~app.models.ban_domain.DomainBansByJail` with per-jail counts
sorted descending and the total ban count.
"""
since: int = since_unix(range_)
@@ -930,7 +940,7 @@ async def bans_by_jail(
total = sum(jail_counter.values())
jail_counts = [
JailBanCountModel(jail=jail_name, count=count)
DomainJailBanCount(jail=jail_name, count=count)
for jail_name, count in sorted(jail_counter.items(), key=lambda x: x[1], reverse=True)
]
@@ -955,12 +965,18 @@ async def bans_by_jail(
origin=origin,
)
total, jail_counts = await fail2ban_db_repo.get_bans_by_jail(
total, jail_counts_repo = await fail2ban_db_repo.get_bans_by_jail(
db_path=db_path,
since=since,
origin=origin,
)
# Convert repository models to domain models
jail_counts = [
DomainJailBanCount(jail=jc.jail, count=jc.count)
for jc in jail_counts_repo
]
# Diagnostic guard: if zero results were returned, check whether the table
# has *any* rows and log a warning with min/max timeofban so operators can
# diagnose timezone or filter mismatches from logs.
@@ -982,7 +998,7 @@ async def bans_by_jail(
jail_count=len(jail_counts),
)
return BansByJailResponse(
jails=[JailBanCountModel(jail=j.jail, count=j.count) for j in jail_counts],
return DomainBansByJail(
jails=jail_counts,
total=total,
)