Adriano
5ca3dc14cd
action era action=f"{method} {path}" senza limite: un path lungo falliva
l'INSERT (String(100)), inghiottito dal try/except generico, e la riga di
audit spariva in silenzio. Troncata a 100 caratteri. ip_address usava
request.client.host, che dietro Nginx/Flask e' sempre il proxy: ora usa
RateLimitMiddleware._client_ip(request), lo stesso risolutore proxy-aware
del rate limiter (X-Forwarded-For con trusted_proxy_count).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
119 lines
3.7 KiB
Python
119 lines
3.7 KiB
Python
"""Unit tests for AccessLogMiddleware (gitea#36 point 6).
|
|
|
|
- action must be truncated to fit AccessLog.action (String(100)): an
|
|
untruncated long path used to fail the INSERT silently (swallowed by the
|
|
broad except in dispatch()).
|
|
- ip_address must come from the same `_client_ip` the rate limiter uses
|
|
(proxy-aware), not the raw `request.client.host`, which behind
|
|
Nginx/Flask is always localhost.
|
|
|
|
These stub out async_session_factory and inspect the compiled INSERT
|
|
statement directly, rather than going through a real DB round trip: Core
|
|
`insert()` against the test's in-memory SQLite has an unrelated
|
|
autoincrement quirk (unrelated to this fix) that makes end-to-end
|
|
assertions on the persisted row unreliable here.
|
|
"""
|
|
from unittest.mock import patch
|
|
|
|
from starlette.requests import Request
|
|
from starlette.responses import Response
|
|
|
|
from src.backend.api.middleware.logging import AccessLogMiddleware
|
|
|
|
|
|
class _FakeSession:
|
|
"""Records the statement passed to execute(); no real DB involved."""
|
|
|
|
def __init__(self):
|
|
self.executed_stmt = None
|
|
|
|
async def execute(self, stmt):
|
|
self.executed_stmt = stmt
|
|
|
|
async def commit(self):
|
|
pass
|
|
|
|
async def __aenter__(self):
|
|
return self
|
|
|
|
async def __aexit__(self, *exc):
|
|
return False
|
|
|
|
|
|
def _make_request(path: str, extra_headers: list[tuple[bytes, bytes]] | None = None) -> Request:
|
|
scope = {
|
|
"type": "http",
|
|
"method": "GET",
|
|
"path": path,
|
|
"query_string": b"",
|
|
"headers": extra_headers or [],
|
|
"client": ("127.0.0.1", 12345),
|
|
}
|
|
return Request(scope)
|
|
|
|
|
|
async def test_long_path_action_is_truncated():
|
|
fake_session = _FakeSession()
|
|
long_path = "/api/files/" + "a" * 200
|
|
|
|
with patch(
|
|
"src.backend.api.middleware.logging.async_session_factory",
|
|
return_value=fake_session,
|
|
):
|
|
middleware = AccessLogMiddleware(app=None)
|
|
request = _make_request(long_path)
|
|
|
|
async def call_next(_req):
|
|
return Response(status_code=404)
|
|
|
|
await middleware.dispatch(request, call_next)
|
|
|
|
assert fake_session.executed_stmt is not None
|
|
params = fake_session.executed_stmt.compile().params
|
|
assert len(params["action"]) <= 100
|
|
assert params["action"] == f"GET {long_path}"[:100]
|
|
|
|
|
|
async def test_short_path_action_is_unaffected():
|
|
fake_session = _FakeSession()
|
|
|
|
with patch(
|
|
"src.backend.api.middleware.logging.async_session_factory",
|
|
return_value=fake_session,
|
|
):
|
|
middleware = AccessLogMiddleware(app=None)
|
|
request = _make_request("/api/settings/")
|
|
|
|
async def call_next(_req):
|
|
return Response(status_code=200)
|
|
|
|
await middleware.dispatch(request, call_next)
|
|
|
|
params = fake_session.executed_stmt.compile().params
|
|
assert params["action"] == "GET /api/settings/"
|
|
|
|
|
|
async def test_ip_address_uses_client_ip_resolution():
|
|
"""trusted_proxy_count defaults to 1: the last XFF hop (our own proxy's
|
|
add) is trusted, not request.client.host (always the proxy itself)."""
|
|
fake_session = _FakeSession()
|
|
|
|
with patch(
|
|
"src.backend.api.middleware.logging.async_session_factory",
|
|
return_value=fake_session,
|
|
):
|
|
middleware = AccessLogMiddleware(app=None)
|
|
request = _make_request(
|
|
"/api/settings/",
|
|
extra_headers=[(b"x-forwarded-for", b"203.0.113.7, 10.0.0.1")],
|
|
)
|
|
|
|
async def call_next(_req):
|
|
return Response(status_code=200)
|
|
|
|
await middleware.dispatch(request, call_next)
|
|
|
|
params = fake_session.executed_stmt.compile().params
|
|
assert params["ip_address"] == "10.0.0.1"
|
|
assert params["ip_address"] != "127.0.0.1" # not request.client.host
|