Standardize API response envelopes: use items for collection responses and update tests
This commit is contained in:
@@ -72,12 +72,33 @@ class JailListResponse(CollectionResponse[JailSummary]):
|
||||
pass
|
||||
|
||||
|
||||
class IgnoreListResponse(CollectionResponse[str]):
|
||||
"""Response for ``GET /api/jails/{name}/ignoreip``.
|
||||
|
||||
Returns the jailed ignore list as a standard collection response.
|
||||
"""
|
||||
|
||||
pass
|
||||
|
||||
|
||||
class JailDetailResponse(BaseModel):
|
||||
"""Response for ``GET /api/jails/{name}``."""
|
||||
"""Response for ``GET /api/jails/{name}``.
|
||||
|
||||
Includes the primary jail object together with supplemental metadata
|
||||
required by the UI.
|
||||
"""
|
||||
|
||||
model_config = ConfigDict(strict=True)
|
||||
|
||||
jail: Jail
|
||||
ignore_list: list[str] = Field(
|
||||
default_factory=list,
|
||||
description="List of IP addresses and networks currently ignored by the jail.",
|
||||
)
|
||||
ignore_self: bool = Field(
|
||||
default=False,
|
||||
description="Whether the jail ignores the server's own IP addresses.",
|
||||
)
|
||||
|
||||
|
||||
class JailCommandResponse(CommandResponse):
|
||||
|
||||
@@ -19,6 +19,7 @@ Provides CRUD and control operations for fail2ban jails:
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
from typing import Annotated
|
||||
|
||||
from fastapi import APIRouter, Body, HTTPException, Path, status
|
||||
@@ -34,6 +35,7 @@ from app.dependencies import (
|
||||
from app.models.ban import JailBannedIpsResponse
|
||||
from app.models.jail import (
|
||||
IgnoreIpRequest,
|
||||
IgnoreListResponse,
|
||||
JailCommandResponse,
|
||||
JailDetailResponse,
|
||||
JailListResponse,
|
||||
@@ -103,7 +105,16 @@ async def get_jail(
|
||||
HTTPException: 404 when the jail does not exist.
|
||||
HTTPException: 502 when fail2ban is unreachable.
|
||||
"""
|
||||
return await jail_service.get_jail(socket_path, name)
|
||||
jail, ignore_list, ignore_self = await asyncio.gather(
|
||||
jail_service.get_jail(socket_path, name),
|
||||
jail_service.get_ignore_list(socket_path, name),
|
||||
jail_service.get_ignore_self(socket_path, name),
|
||||
)
|
||||
return JailDetailResponse(
|
||||
jail=jail,
|
||||
ignore_list=ignore_list,
|
||||
ignore_self=ignore_self,
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -278,14 +289,14 @@ class _IgnoreSelfRequest(IgnoreIpRequest):
|
||||
|
||||
@router.get(
|
||||
"/{name}/ignoreip",
|
||||
response_model=list[str],
|
||||
response_model=IgnoreListResponse,
|
||||
summary="List the ignore IPs for a jail",
|
||||
)
|
||||
async def get_ignore_list(
|
||||
_auth: AuthDep,
|
||||
name: _NamePath,
|
||||
socket_path: Fail2BanSocketDep,
|
||||
) -> list[str]:
|
||||
) -> IgnoreListResponse:
|
||||
"""Return the current ignore list (IP whitelist) for a fail2ban jail.
|
||||
|
||||
Args:
|
||||
@@ -299,7 +310,8 @@ async def get_ignore_list(
|
||||
HTTPException: 404 when the jail does not exist.
|
||||
HTTPException: 502 when fail2ban is unreachable.
|
||||
"""
|
||||
return await jail_service.get_ignore_list(socket_path, name)
|
||||
ignore_list = await jail_service.get_ignore_list(socket_path, name)
|
||||
return IgnoreListResponse(items=ignore_list, total=len(ignore_list))
|
||||
|
||||
|
||||
@router.post(
|
||||
|
||||
@@ -218,7 +218,7 @@ async def list_jail_configs(socket_path: str) -> JailConfigListResponse:
|
||||
)
|
||||
|
||||
if not jail_names:
|
||||
return JailConfigListResponse(jails=[], total=0)
|
||||
return JailConfigListResponse(items=[], total=0)
|
||||
|
||||
responses: list[JailConfigResponse] = await asyncio.gather(
|
||||
*[get_jail_config(socket_path, name) for name in jail_names],
|
||||
@@ -227,7 +227,7 @@ async def list_jail_configs(socket_path: str) -> JailConfigListResponse:
|
||||
|
||||
jails = [r.jail for r in responses]
|
||||
log.info("jail_configs_listed", count=len(jails))
|
||||
return JailConfigListResponse(jails=jails, total=len(jails))
|
||||
return JailConfigListResponse(items=jails, total=len(jails))
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -311,7 +311,7 @@ async def list_inactive_jails(
|
||||
active=len(active_names),
|
||||
inactive=len(inactive),
|
||||
)
|
||||
return InactiveJailListResponse(jails=inactive, total=len(inactive))
|
||||
return InactiveJailListResponse(items=inactive, total=len(inactive))
|
||||
|
||||
|
||||
async def activate_jail(
|
||||
|
||||
@@ -47,6 +47,7 @@ async def config_client(tmp_path: Path) -> AsyncClient: # type: ignore[misc]
|
||||
session_duration_minutes=60,
|
||||
timezone="UTC",
|
||||
log_level="debug",
|
||||
session_cookie_secure=False,
|
||||
)
|
||||
app = create_app(settings=settings)
|
||||
|
||||
@@ -98,7 +99,7 @@ class TestGetJailConfigs:
|
||||
async def test_200_returns_jail_list(self, config_client: AsyncClient) -> None:
|
||||
"""GET /api/config/jails returns 200 with JailConfigListResponse."""
|
||||
mock_response = JailConfigListResponse(
|
||||
jails=[_make_jail_config("sshd")], total=1
|
||||
items=[_make_jail_config("sshd")], total=1
|
||||
)
|
||||
with patch(
|
||||
"app.routers.jail_config.config_service.list_jail_configs",
|
||||
@@ -109,7 +110,7 @@ class TestGetJailConfigs:
|
||||
assert resp.status_code == 200
|
||||
data = resp.json()
|
||||
assert data["total"] == 1
|
||||
assert data["jails"][0]["name"] == "sshd"
|
||||
assert data["items"][0]["name"] == "sshd"
|
||||
|
||||
async def test_401_when_unauthenticated(self, config_client: AsyncClient) -> None:
|
||||
"""GET /api/config/jails returns 401 without a valid session."""
|
||||
@@ -694,7 +695,7 @@ class TestGetInactiveJails:
|
||||
source_file="/etc/fail2ban/jail.conf",
|
||||
enabled=False,
|
||||
)
|
||||
mock_response = InactiveJailListResponse(jails=[mock_jail], total=1)
|
||||
mock_response = InactiveJailListResponse(items=[mock_jail], total=1)
|
||||
|
||||
with patch(
|
||||
"app.routers.jail_config.jail_config_service.list_inactive_jails",
|
||||
@@ -705,7 +706,7 @@ class TestGetInactiveJails:
|
||||
assert resp.status_code == 200
|
||||
data = resp.json()
|
||||
assert data["total"] == 1
|
||||
assert data["jails"][0]["name"] == "apache-auth"
|
||||
assert data["items"][0]["name"] == "apache-auth"
|
||||
|
||||
async def test_200_empty_list(self, config_client: AsyncClient) -> None:
|
||||
"""GET /api/config/jails/inactive returns 200 with empty list."""
|
||||
@@ -713,13 +714,13 @@ class TestGetInactiveJails:
|
||||
|
||||
with patch(
|
||||
"app.routers.jail_config.jail_config_service.list_inactive_jails",
|
||||
AsyncMock(return_value=InactiveJailListResponse(jails=[], total=0)),
|
||||
AsyncMock(return_value=InactiveJailListResponse(items=[], total=0)),
|
||||
):
|
||||
resp = await config_client.get("/api/config/jails/inactive")
|
||||
|
||||
assert resp.status_code == 200
|
||||
assert resp.json()["total"] == 0
|
||||
assert resp.json()["jails"] == []
|
||||
assert resp.json()["items"] == []
|
||||
|
||||
async def test_401_when_unauthenticated(self, config_client: AsyncClient) -> None:
|
||||
"""GET /api/config/jails/inactive returns 401 without a valid session."""
|
||||
|
||||
@@ -66,7 +66,7 @@ class FakeAuthService:
|
||||
|
||||
class FakeJailService:
|
||||
async def list_jails(self, _socket_path: str) -> JailListResponse:
|
||||
return JailListResponse(jails=[], total=0)
|
||||
return JailListResponse(items=[], total=0)
|
||||
|
||||
async def get_jail(self, _socket_path: str, _name: str) -> JailListResponse:
|
||||
raise NotImplementedError
|
||||
|
||||
@@ -34,7 +34,7 @@ async def jails_client(tmp_path: Path) -> AsyncClient: # type: ignore[misc]
|
||||
settings = Settings(
|
||||
database_path=str(tmp_path / "jails_test.db"),
|
||||
fail2ban_socket="/tmp/fake.sock",
|
||||
session_secret="test-jails-secret",
|
||||
session_secret="test-jails-secret-0000000000000000000000",
|
||||
session_duration_minutes=60,
|
||||
timezone="UTC",
|
||||
log_level="debug",
|
||||
@@ -108,7 +108,9 @@ def _detail(name: str = "sshd") -> JailDetailResponse:
|
||||
currently_failed=1,
|
||||
total_failed=50,
|
||||
),
|
||||
)
|
||||
),
|
||||
ignore_list=["127.0.0.1"],
|
||||
ignore_self=False,
|
||||
)
|
||||
|
||||
|
||||
@@ -122,7 +124,7 @@ class TestGetJails:
|
||||
|
||||
async def test_200_when_authenticated(self, jails_client: AsyncClient) -> None:
|
||||
"""GET /api/jails returns 200 with a JailListResponse."""
|
||||
mock_response = JailListResponse(jails=[_summary()], total=1)
|
||||
mock_response = JailListResponse(items=[_summary()], total=1)
|
||||
with patch(
|
||||
"app.routers.jails.jail_service.list_jails",
|
||||
AsyncMock(return_value=mock_response),
|
||||
@@ -132,7 +134,7 @@ class TestGetJails:
|
||||
assert resp.status_code == 200
|
||||
data = resp.json()
|
||||
assert data["total"] == 1
|
||||
assert data["jails"][0]["name"] == "sshd"
|
||||
assert data["items"][0]["name"] == "sshd"
|
||||
|
||||
async def test_401_when_unauthenticated(self, jails_client: AsyncClient) -> None:
|
||||
"""GET /api/jails returns 401 without a session cookie."""
|
||||
@@ -144,14 +146,14 @@ class TestGetJails:
|
||||
|
||||
async def test_response_shape(self, jails_client: AsyncClient) -> None:
|
||||
"""GET /api/jails response contains expected fields."""
|
||||
mock_response = JailListResponse(jails=[_summary()], total=1)
|
||||
mock_response = JailListResponse(items=[_summary()], total=1)
|
||||
with patch(
|
||||
"app.routers.jails.jail_service.list_jails",
|
||||
AsyncMock(return_value=mock_response),
|
||||
):
|
||||
resp = await jails_client.get("/api/jails")
|
||||
|
||||
jail = resp.json()["jails"][0]
|
||||
jail = resp.json()["items"][0]
|
||||
assert "name" in jail
|
||||
assert "enabled" in jail
|
||||
assert "running" in jail
|
||||
@@ -359,7 +361,7 @@ class TestIgnoreIpEndpoints:
|
||||
"""Tests for ignore-list management endpoints."""
|
||||
|
||||
async def test_get_ignore_list(self, jails_client: AsyncClient) -> None:
|
||||
"""GET /api/jails/sshd/ignoreip returns 200 with a list."""
|
||||
"""GET /api/jails/sshd/ignoreip returns 200 with a wrapped response."""
|
||||
with patch(
|
||||
"app.routers.jails.jail_service.get_ignore_list",
|
||||
AsyncMock(return_value=["127.0.0.1"]),
|
||||
@@ -367,7 +369,7 @@ class TestIgnoreIpEndpoints:
|
||||
resp = await jails_client.get("/api/jails/sshd/ignoreip")
|
||||
|
||||
assert resp.status_code == 200
|
||||
assert "127.0.0.1" in resp.json()
|
||||
assert resp.json() == {"items": ["127.0.0.1"], "total": 1}
|
||||
|
||||
async def test_add_ignore_ip_returns_201(self, jails_client: AsyncClient) -> None:
|
||||
"""POST /api/jails/sshd/ignoreip returns 201 on success."""
|
||||
|
||||
@@ -381,7 +381,7 @@ class TestListInactiveJails:
|
||||
):
|
||||
result = await list_inactive_jails(str(tmp_path), "/fake.sock")
|
||||
|
||||
names = [j.name for j in result.jails]
|
||||
names = [j.name for j in result.items]
|
||||
assert "sshd" not in names
|
||||
assert "apache-auth" in names
|
||||
|
||||
@@ -393,7 +393,7 @@ class TestListInactiveJails:
|
||||
):
|
||||
result = await list_inactive_jails(str(tmp_path), "/fake.sock")
|
||||
|
||||
assert result.total == len(result.jails)
|
||||
assert result.total == len(result.items)
|
||||
|
||||
async def test_empty_config_dir(self, tmp_path: Path) -> None:
|
||||
with patch(
|
||||
@@ -402,7 +402,7 @@ class TestListInactiveJails:
|
||||
):
|
||||
result = await list_inactive_jails(str(tmp_path), "/fake.sock")
|
||||
|
||||
assert result.jails == []
|
||||
assert result.items == []
|
||||
assert result.total == 0
|
||||
|
||||
async def test_all_active_returns_empty(self, tmp_path: Path) -> None:
|
||||
@@ -413,7 +413,7 @@ class TestListInactiveJails:
|
||||
):
|
||||
result = await list_inactive_jails(str(tmp_path), "/fake.sock")
|
||||
|
||||
assert result.jails == []
|
||||
assert result.items == []
|
||||
|
||||
async def test_fail2ban_unreachable_shows_all(self, tmp_path: Path) -> None:
|
||||
# When fail2ban is unreachable, _get_active_jail_names returns empty set,
|
||||
@@ -425,7 +425,7 @@ class TestListInactiveJails:
|
||||
):
|
||||
result = await list_inactive_jails(str(tmp_path), "/fake.sock")
|
||||
|
||||
names = {j.name for j in result.jails}
|
||||
names = {j.name for j in result.items}
|
||||
assert "sshd" in names
|
||||
assert "apache-auth" in names
|
||||
|
||||
@@ -440,7 +440,7 @@ class TestListInactiveJails:
|
||||
new=AsyncMock(return_value=set()),
|
||||
):
|
||||
result = await list_inactive_jails(str(tmp_path), "/fake.sock")
|
||||
jail = next(j for j in result.jails if j.name == "apache-auth")
|
||||
jail = next(j for j in result.items if j.name == "apache-auth")
|
||||
assert jail.has_local_override is True
|
||||
|
||||
async def test_has_local_override_false_when_no_local_file(self, tmp_path: Path) -> None:
|
||||
@@ -451,7 +451,7 @@ class TestListInactiveJails:
|
||||
new=AsyncMock(return_value=set()),
|
||||
):
|
||||
result = await list_inactive_jails(str(tmp_path), "/fake.sock")
|
||||
jail = next(j for j in result.jails if j.name == "apache-auth")
|
||||
jail = next(j for j in result.items if j.name == "apache-auth")
|
||||
assert jail.has_local_override is False
|
||||
|
||||
|
||||
|
||||
@@ -220,7 +220,7 @@ class TestListJailConfigs:
|
||||
|
||||
assert isinstance(result, JailConfigListResponse)
|
||||
assert result.total == 1
|
||||
assert result.jails[0].name == "sshd"
|
||||
assert result.items[0].name == "sshd"
|
||||
|
||||
async def test_empty_when_no_jails(self) -> None:
|
||||
"""list_jail_configs returns empty list when no jails are active."""
|
||||
@@ -229,7 +229,7 @@ class TestListJailConfigs:
|
||||
result = await config_service.list_jail_configs(_SOCKET)
|
||||
|
||||
assert result.total == 0
|
||||
assert result.jails == []
|
||||
assert result.items == []
|
||||
|
||||
async def test_multiple_jails(self) -> None:
|
||||
"""list_jail_configs handles comma-separated jail names."""
|
||||
@@ -245,7 +245,7 @@ class TestListJailConfigs:
|
||||
result = await config_service.list_jail_configs(_SOCKET)
|
||||
|
||||
assert result.total == 2
|
||||
names = {j.name for j in result.jails}
|
||||
names = {j.name for j in result.items}
|
||||
assert names == {"sshd", "nginx"}
|
||||
|
||||
|
||||
@@ -887,7 +887,7 @@ class TestConfigModuleIntegration:
|
||||
):
|
||||
result = await list_inactive_jails(str(tmp_path), "/fake.sock")
|
||||
|
||||
names = {j.name for j in result.jails}
|
||||
names = {j.name for j in result.items}
|
||||
assert "apache-auth" in names
|
||||
assert "sshd" not in names
|
||||
|
||||
|
||||
Reference in New Issue
Block a user