diff --git a/netx_api/audit_async.py b/netx_api/audit_async.py index a035a13..d7fc2d1 100644 --- a/netx_api/audit_async.py +++ b/netx_api/audit_async.py @@ -5,7 +5,6 @@ from __future__ import annotations import logging import queue import threading -import time from typing import Any from .config import settings @@ -19,13 +18,6 @@ _lock = threading.Lock() _counter = 0 _dropped = 0 -# Dedupe unauthenticated request floods (SPA fires many parallel 401s). -_unauth_lock = threading.Lock() -_unauth_recent: dict[str, float] = {} -_UNAUTH_DEDUP_GET_SEC = 60.0 -_UNAUTH_DEDUP_WRITE_SEC = 15.0 -_UNAUTH_RECENT_MAX = 4000 - # Middleware-tagged HTTP wrappers that duplicate semantic business audits. _MIDDLEWARE_NOISE_ACTIONS = frozenset( { @@ -37,6 +29,10 @@ _MIDDLEWARE_NOISE_ACTIONS = frozenset( "webcrt.delete", "users.get", "api_tokens.get", + # Legacy auto-gate noise (no longer written; hide historical rows). + "auth.unauthorized", + "auth.password_change_required", + "auth.forbidden_scope", } ) @@ -46,6 +42,7 @@ _AUTH_READ_NOISE_ACTIONS = frozenset( { "auth.me", "auth.sessions", + "auth.refresh", } ) @@ -69,32 +66,6 @@ def _sample_ok() -> bool: return (_counter % n) == 0 -def should_audit_unauthorized(*, client_ip: str, method: str, path: str) -> bool: - """Rate-limit auth.unauthorized writes: one row per IP+method+path per window. - - Unauthenticated page loads often fan out dozens of GETs in the same second; - keeping every 401 drowns real login/security events. - """ - method_u = str(method or "GET").upper() - window = ( - _UNAUTH_DEDUP_WRITE_SEC - if method_u in ("POST", "PUT", "PATCH", "DELETE") - else _UNAUTH_DEDUP_GET_SEC - ) - key = f"{client_ip or '-'}|{method_u}|{path or '/'}" - now = time.monotonic() - with _unauth_lock: - global _unauth_recent - last = float(_unauth_recent.get(key) or 0.0) - if now - last < window: - return False - _unauth_recent[key] = now - if len(_unauth_recent) > _UNAUTH_RECENT_MAX: - cutoff = now - max(_UNAUTH_DEDUP_GET_SEC * 5, 300.0) - _unauth_recent = {k: v for k, v in _unauth_recent.items() if float(v) >= cutoff} - return True - - def audit_should_persist( *, action: str, @@ -105,13 +76,12 @@ def audit_should_persist( """Decide whether a candidate audit event is worth writing. Policy: - - Always keep auth login/logout/security events, users/tokens/NE/port_traffic/config_sync, - and semantic webcrt session/command events. - - Drop successful ``auth.me`` / ``auth.sessions`` polls (UI session checks). - - Always keep HTTP failures (status >= 400), except pure list noise. - - Drop successful GET/HEAD/OPTIONS ``http.*`` (page polling). - - Always keep mutating ``http.*`` (POST/PUT/PATCH/DELETE) — no sampling. - - Drop middleware ``webcrt.{method}`` / ``audit.list`` (covered by business events). + - Keep intentional ops: auth.login/logout/login_failed, users/tokens/NE/port_traffic/ + config_sync, semantic webcrt session/command. + - Drop anonymous 401 / auto-gate noise (unauthorized, forbidden_scope, …). + - Drop successful ``auth.me`` / ``auth.sessions`` / ``auth.refresh`` polls. + - Drop successful GET/HEAD/OPTIONS ``http.*``; keep mutating http.* and failures. + - Drop middleware ``webcrt.{method}`` / ``audit.list``. """ del path # reserved for future path allow/deny lists act = str(action or "") diff --git a/netx_api/auth_middleware.py b/netx_api/auth_middleware.py index b21fda1..8127d9b 100644 --- a/netx_api/auth_middleware.py +++ b/netx_api/auth_middleware.py @@ -12,7 +12,6 @@ from starlette.responses import JSONResponse, Response from .auth_deps import resolve_user_from_token from .auth_scopes import has_scope, required_scope_for_request -from .auth_service import write_audit from .config import settings from .db import SessionLocal @@ -113,55 +112,19 @@ class AuthAuditMiddleware(BaseHTTPMiddleware): try: resolved = resolve_user_from_token(db, token) if token else None if resolved is None: - client_ip = _client_ip(request) - from .audit_async import should_audit_unauthorized - - if should_audit_unauthorized(client_ip=client_ip, method=request.method, path=path): - write_audit( - db, - action="auth.unauthorized", - method=request.method, - path=path, - status_code=401, - client_ip=client_ip, - user_agent=str(request.headers.get("user-agent") or "")[:512], - detail={}, - ) + # Do not audit anonymous 401s — SPA fan-out floods the log. + # Intentional auth ops (login/logout/login_failed) are written in auth_router. return JSONResponse(status_code=401, content={"detail": "unauthorized"}) user, via, scopes, token_id, jti = resolved if bool(getattr(user, "must_change_password", False)): allow_key = (request.method.upper(), path.rstrip("/") if len(path) > 1 else path) if allow_key not in _PASSWORD_CHANGE_ALLOW: - write_audit( - db, - action="auth.password_change_required", - actor_user_id=str(user.id), - actor_username=str(user.username), - method=request.method, - path=path, - status_code=403, - client_ip=_client_ip(request), - user_agent=str(request.headers.get("user-agent") or "")[:512], - detail={"auth_via": via}, - ) return JSONResponse( status_code=403, content={"detail": "password_change_required"}, ) need = required_scope_for_request(request.method, path) if need and not has_scope(scopes, need): - write_audit( - db, - action="auth.forbidden_scope", - actor_user_id=str(user.id), - actor_username=str(user.username), - method=request.method, - path=path, - status_code=403, - client_ip=_client_ip(request), - user_agent=str(request.headers.get("user-agent") or "")[:512], - detail={"required": need, "granted": sorted(scopes), "auth_via": via}, - ) return JSONResponse( status_code=403, content={ diff --git a/netx_api/auth_router.py b/netx_api/auth_router.py index b1fe839..8a70024 100644 --- a/netx_api/auth_router.py +++ b/netx_api/auth_router.py @@ -153,19 +153,7 @@ def api_refresh( detail={}, ) raise - user = out.get("user") or {} - write_audit( - db, - action="auth.refresh", - actor_user_id=str(user.get("id") or ""), - actor_username=str(user.get("username") or ""), - method="POST", - path="/v1/auth/refresh", - status_code=200, - client_ip=ip, - user_agent=ua, - detail={}, - ) + # Successful silent token refresh is not an operator action — skip audit. return _token_response(response, request, out) diff --git a/netx_api/auth_service.py b/netx_api/auth_service.py index fac13b3..9ab2b30 100644 --- a/netx_api/auth_service.py +++ b/netx_api/auth_service.py @@ -747,7 +747,10 @@ def list_audit_logs( "api_tokens.get", "auth.me", "auth.sessions", + "auth.refresh", "auth.unauthorized", + "auth.password_change_required", + "auth.forbidden_scope", ) q = q.filter(~AuditLog.action.like("http.%")) q = q.filter(~AuditLog.action.in_(noise_actions)) diff --git a/tests/test_audit_noise.py b/tests/test_audit_noise.py index c12a44d..acd5cf0 100644 --- a/tests/test_audit_noise.py +++ b/tests/test_audit_noise.py @@ -31,7 +31,16 @@ class AuditShouldPersistTests(unittest.TestCase): ) def test_drop_middleware_noise(self) -> None: - for action in ("audit.list", "webcrt.get", "webcrt.post", "users.get", "api_tokens.get"): + for action in ( + "audit.list", + "webcrt.get", + "webcrt.post", + "users.get", + "api_tokens.get", + "auth.unauthorized", + "auth.password_change_required", + "auth.forbidden_scope", + ): self.assertFalse( audit_should_persist(action=action, method="GET", status_code=200), msg=action, @@ -42,15 +51,18 @@ class AuditShouldPersistTests(unittest.TestCase): self.assertTrue(audit_should_persist(action="webcrt.command", status_code=0)) self.assertTrue(audit_should_persist(action="webcrt.session_closed", status_code=0)) - def test_drop_auth_me_poll(self) -> None: + def test_drop_auth_polls(self) -> None: self.assertFalse(audit_should_persist(action="auth.me", method="GET", status_code=200)) self.assertFalse(audit_should_persist(action="auth.sessions", method="GET", status_code=200)) - # Failures still useful (expired session / forbidden). - self.assertTrue(audit_should_persist(action="auth.me", method="GET", status_code=401)) + self.assertFalse(audit_should_persist(action="auth.refresh", method="POST", status_code=200)) + + def test_keep_intentional_auth_ops(self) -> None: + self.assertTrue(audit_should_persist(action="auth.login", status_code=200)) + self.assertTrue(audit_should_persist(action="auth.login_failed", status_code=401)) + self.assertTrue(audit_should_persist(action="auth.logout", status_code=200)) + self.assertTrue(audit_should_persist(action="auth.change_password", status_code=200)) def test_keep_business_prefixes(self) -> None: - self.assertTrue(audit_should_persist(action="auth.login", status_code=200)) - self.assertTrue(audit_should_persist(action="auth.logout", status_code=200)) self.assertTrue(audit_should_persist(action="ne.exec", status_code=200)) self.assertTrue(audit_should_persist(action="port_traffic.device.start", status_code=200)) self.assertTrue(audit_should_persist(action="config_sync.start", status_code=200)) @@ -60,33 +72,5 @@ class AuditShouldPersistTests(unittest.TestCase): self.assertFalse(audit_should_persist(action="ume.token.get", method="GET", status_code=200)) -class UnauthorizedDedupeTests(unittest.TestCase): - def setUp(self) -> None: - from netx_api import audit_async as aa - - with aa._unauth_lock: - aa._unauth_recent.clear() - - def test_dedupes_same_ip_path(self) -> None: - from netx_api.audit_async import should_audit_unauthorized - - self.assertTrue( - should_audit_unauthorized(client_ip="127.0.0.1", method="GET", path="/v1/topology") - ) - self.assertFalse( - should_audit_unauthorized(client_ip="127.0.0.1", method="GET", path="/v1/topology") - ) - # Different path still recorded once. - self.assertTrue( - should_audit_unauthorized(client_ip="127.0.0.1", method="GET", path="/v1/managed-ne") - ) - - def test_different_ip_not_deduped(self) -> None: - from netx_api.audit_async import should_audit_unauthorized - - self.assertTrue(should_audit_unauthorized(client_ip="1.1.1.1", method="GET", path="/v1/x")) - self.assertTrue(should_audit_unauthorized(client_ip="2.2.2.2", method="GET", path="/v1/x")) - - if __name__ == "__main__": unittest.main()