From 71d52ad0f8d658df949c5f0949e650332c26fd42 Mon Sep 17 00:00:00 2001 From: beubax Date: Mon, 15 Jun 2026 18:06:50 +0530 Subject: [PATCH] refactor: remove audit event filters --- docs/site/reference/audit-log.mdx | 8 +- src/authsome/server/routes/audit.py | 20 +-- src/authsome/server/store/repositories.py | 35 +---- tests/server/test_audit_events.py | 50 ++++-- ui/src/components/authsome-dashboard.tsx | 179 +--------------------- ui/src/lib/authsome-api.ts | 22 +-- 6 files changed, 61 insertions(+), 253 deletions(-) diff --git a/docs/site/reference/audit-log.mdx b/docs/site/reference/audit-log.mdx index 6a95e07..5ceebc7 100644 --- a/docs/site/reference/audit-log.mdx +++ b/docs/site/reference/audit-log.mdx @@ -53,8 +53,7 @@ authsome log --json # Output JSON format `GET /api/audit/events` is role-aware. Admin principals can review the global audit log. Non-admin principals receive only events scoped to their own principal, including their -claimed identities, vault, providers, and credential lifecycle activity. Passing another -`principal_id`, vault, or identity in query parameters cannot widen a non-admin query. +claimed identities, vault, providers, and credential lifecycle activity. Supported query parameters: @@ -62,11 +61,6 @@ Supported query parameters: | --- | --- | | `limit` | Number of events to return, clamped to the server maximum. | | `cursor` | Cursor returned by the previous page. | -| `event` | Exact event name. | -| `provider` | Exact provider name, such as `github`. | -| `identity` | Exact identity handle. | -| `from` | Inclusive lower timestamp bound. | -| `to` | Exclusive upper timestamp bound. | Results are sorted newest-first and include `next_cursor` when another page is available. diff --git a/src/authsome/server/routes/audit.py b/src/authsome/server/routes/audit.py index 04d4459..19d770e 100644 --- a/src/authsome/server/routes/audit.py +++ b/src/authsome/server/routes/audit.py @@ -1,9 +1,8 @@ """Audit event routes.""" -from datetime import datetime -from typing import Annotated, Any, Literal +from typing import Any, Literal -from fastapi import APIRouter, Depends, HTTPException, Query, Request, status +from fastapi import APIRouter, Depends, HTTPException, Request, status from authsome import audit from authsome.identity.principal import PrincipalRole @@ -17,29 +16,18 @@ router = APIRouter(prefix="/audit", tags=["audit"]) @router.get("/events") -async def list_audit_events( # noqa: PLR0913 +async def list_audit_events( request: Request, limit: int = 50, cursor: str | None = None, - event: str | None = None, - provider: str | None = None, - identity: str | None = None, - principal_id: str | None = None, - from_: Annotated[datetime | None, Query(alias="from")] = None, - to: datetime | None = None, auth: CredentialService = Depends(get_daemon_or_browser_auth_service), ) -> dict[str, Any]: - effective_principal_id = principal_id if auth.principal_role == PrincipalRole.ADMIN else auth.principal_id + effective_principal_id = None if auth.principal_role == PrincipalRole.ADMIN else auth.principal_id scope: Literal["global", "principal"] = "global" if effective_principal_id is None else "principal" try: page = await request.app.state.audit_log.query_events( limit=limit, principal_id=effective_principal_id, - event=event, - provider=provider, - identity=identity, - since=from_, - until=to, cursor=cursor, ) except ValueError as exc: diff --git a/src/authsome/server/store/repositories.py b/src/authsome/server/store/repositories.py index e8d5976..e36b08a 100644 --- a/src/authsome/server/store/repositories.py +++ b/src/authsome/server/store/repositories.py @@ -134,29 +134,20 @@ class AuditEventRegistry: ], ) - async def query_events( # noqa: PLR0913 + async def query_events( self, *, limit: int = 50, principal_id: str | None = None, - event: str | None = None, - provider: str | None = None, - identity: str | None = None, - since: datetime | None = None, - until: datetime | None = None, cursor: str | None = None, ) -> AuditEventPage: bounded_limit = min(max(limit, 1), 500) - filter_specs = ( - ("principal_id = ?", principal_id, principal_id is not None), - ("event = ?", event, bool(event)), - ("provider = ?", provider, bool(provider)), - ("identity = ?", identity, bool(identity)), - ("timestamp >= ?", _dump_dt(since) if since is not None else None, since is not None), - ("timestamp < ?", _dump_dt(until) if until is not None else None, until is not None), - ) - conditions = [condition for condition, _value, enabled in filter_specs if enabled] - params = [value for _condition, value, enabled in filter_specs if enabled] + conditions: list[str] = [] + params: list[Any] = [] + + if principal_id is not None: + conditions.append("principal_id = ?") + params.append(principal_id) if cursor: cursor_timestamp, cursor_event_id = _decode_audit_cursor(cursor) @@ -339,27 +330,17 @@ class ServerAuditLog: _delegating_audit_exporter.set_active(None) self._exporter.close() - async def query_events( # noqa: PLR0913 + async def query_events( self, *, limit: int = 50, principal_id: str | None = None, - event: str | None = None, - provider: str | None = None, - identity: str | None = None, - since: datetime | None = None, - until: datetime | None = None, cursor: str | None = None, ) -> AuditEventPage: await self.async_force_flush() return await self._registry.query_events( limit=limit, principal_id=principal_id, - event=event, - provider=provider, - identity=identity, - since=since, - until=until, cursor=cursor, ) diff --git a/tests/server/test_audit_events.py b/tests/server/test_audit_events.py index 096ccc3..595d0f8 100644 --- a/tests/server/test_audit_events.py +++ b/tests/server/test_audit_events.py @@ -45,7 +45,7 @@ def _emit_audit_event( # noqa: PLR0913 emit( AuditEvent( event_id=event_id, - timestamp=timestamp or datetime(2026, 6, 15, 8, 0, tzinfo=UTC), + timestamp=timestamp or datetime(2099, 1, 1, 8, 0, tzinfo=UTC), event=event, principal_id=principal_id, identity=identity, @@ -82,6 +82,17 @@ def test_audit_events_endpoint_returns_internal_events_for_admin(monkeypatch, tm assert entries[0]["provider"] == "github" +def test_audit_events_endpoint_only_documents_pagination_params(monkeypatch, tmp_path: Path) -> None: + monkeypatch.setenv("AUTHSOME_HOME", str(tmp_path)) + + with create_server_test_client() as client: + response = client.get("/openapi.json") + + assert response.status_code == status.HTTP_200_OK + params = response.json()["paths"]["/api/audit/events"]["get"]["parameters"] + assert {param["name"] for param in params} == {"limit", "cursor"} + + def test_external_audit_post_is_enriched_from_pop_identity(monkeypatch, tmp_path: Path) -> None: monkeypatch.setenv("AUTHSOME_HOME", str(tmp_path)) payload = {"event": {"event": "proxy_deny", "metadata": {"host": "api.example.com", "reason": "no_match"}}} @@ -156,7 +167,7 @@ def test_admin_sees_all_audit_events_and_user_sees_only_own_principal(monkeypatc assert all(entry["principal_id"] == user_whoami["principal_id"] for entry in user_entries) -def test_non_admin_audit_filters_stay_within_own_principal(monkeypatch, tmp_path: Path) -> None: +def test_non_admin_audit_query_params_do_not_filter_or_widen_scope(monkeypatch, tmp_path: Path) -> None: monkeypatch.setenv("AUTHSOME_HOME", str(tmp_path)) with create_server_test_client() as client: @@ -205,10 +216,9 @@ def test_non_admin_audit_filters_stay_within_own_principal(monkeypatch, tmp_path assert response.status_code == status.HTTP_200_OK body = response.json() assert body["scope"] == "principal" - assert body["next_cursor"] is None - assert [entry["event_id"] for entry in body["entries"]] == ["audit_002"] - assert body["entries"][0]["principal_id"] == user_whoami["principal_id"] - assert body["entries"][0]["provider"] == "github" + manual_entries = [entry for entry in body["entries"] if entry["event_id"].startswith("audit_00")] + assert [entry["event_id"] for entry in manual_entries] == ["audit_003", "audit_002"] + assert all(entry["principal_id"] == user_whoami["principal_id"] for entry in body["entries"]) def test_non_admin_audit_query_cannot_widen_scope_with_principal_or_identity( @@ -259,10 +269,13 @@ def test_non_admin_audit_query_cannot_widen_scope_with_principal_or_identity( assert response.status_code == status.HTTP_200_OK body = response.json() assert body["scope"] == "principal" - assert body["entries"] == [] + event_ids = {entry["event_id"] for entry in body["entries"]} + assert "audit_011" in event_ids + assert "audit_010" not in event_ids + assert all(entry["principal_id"] == user_whoami["principal_id"] for entry in body["entries"]) -def test_admin_audit_events_support_filters_and_cursor_pagination(monkeypatch, tmp_path: Path) -> None: +def test_admin_audit_events_support_cursor_pagination(monkeypatch, tmp_path: Path) -> None: monkeypatch.setenv("AUTHSOME_HOME", str(tmp_path)) with create_server_test_client() as client: @@ -282,7 +295,15 @@ def test_admin_audit_events_support_filters_and_cursor_pagination(monkeypatch, t principal_id=admin_whoami["principal_id"], identity="admin-ready-boldly-0001", provider="github", - timestamp=datetime(2026, 6, 15, 8, 0, tzinfo=UTC), + timestamp=datetime(2099, 1, 1, 8, 0, tzinfo=UTC), + ) + _emit_audit_event( + "audit_099", + "connection.logout", + principal_id=admin_whoami["principal_id"], + identity="admin-ready-boldly-0001", + provider="linear", + timestamp=datetime(2099, 1, 1, 7, 59, tzinfo=UTC), ) _emit_audit_event( "audit_101", @@ -290,7 +311,7 @@ def test_admin_audit_events_support_filters_and_cursor_pagination(monkeypatch, t principal_id=user_whoami["principal_id"], identity="steady-wisely-boldly-0042", provider="github", - timestamp=datetime(2026, 6, 15, 8, 1, tzinfo=UTC), + timestamp=datetime(2099, 1, 1, 8, 1, tzinfo=UTC), ) _emit_audit_event( "audit_102", @@ -298,10 +319,10 @@ def test_admin_audit_events_support_filters_and_cursor_pagination(monkeypatch, t principal_id=user_whoami["principal_id"], identity="steady-wisely-boldly-0042", provider="github", - timestamp=datetime(2026, 6, 15, 8, 2, tzinfo=UTC), + timestamp=datetime(2099, 1, 1, 8, 2, tzinfo=UTC), ) - first_path = "/api/audit/events?provider=github&limit=2" + first_path = "/api/audit/events?limit=2" first_response = client.get( first_path, headers=_auth_header( @@ -313,7 +334,7 @@ def test_admin_audit_events_support_filters_and_cursor_pagination(monkeypatch, t ) assert first_response.status_code == status.HTTP_200_OK first_body = first_response.json() - second_path = f"/api/audit/events?provider=github&limit=2&cursor={first_body['next_cursor']}" + second_path = f"/api/audit/events?limit=2&cursor={first_body['next_cursor']}" second_response = client.get( second_path, headers=_auth_header( @@ -331,8 +352,7 @@ def test_admin_audit_events_support_filters_and_cursor_pagination(monkeypatch, t assert second_response.status_code == status.HTTP_200_OK second_body = second_response.json() assert second_body["scope"] == "global" - assert [entry["event_id"] for entry in second_body["entries"]] == ["audit_100"] - assert second_body["next_cursor"] is None + assert [entry["event_id"] for entry in second_body["entries"]] == ["audit_100", "audit_099"] @pytest.mark.asyncio diff --git a/ui/src/components/authsome-dashboard.tsx b/ui/src/components/authsome-dashboard.tsx index 1680329..fd091f6 100644 --- a/ui/src/components/authsome-dashboard.tsx +++ b/ui/src/components/authsome-dashboard.tsx @@ -33,7 +33,6 @@ import useSWR from "swr"; import { ApiError, - AuditFilters, ConnectionDetail, DashboardData, GlobalConnectionRow, @@ -1437,62 +1436,20 @@ export function PrincipalsView() { ); } -function normalizeAuditFilters(filters: AuditFilters): AuditFilters { - return Object.fromEntries( - Object.entries(filters) - .map(([key, value]) => [key, typeof value === "string" ? value.trim() : value]) - .filter(([, value]) => value !== undefined && value !== null && String(value).trim() !== ""), - ) as AuditFilters; -} - -function localDateTimeToIso(value: string): string | undefined { - if (!value) return undefined; - const parsed = new Date(value); - return Number.isNaN(parsed.valueOf()) ? undefined : parsed.toISOString(); -} - export function AuditView({ data }: { data: DashboardData }) { - const [filters, setFilters] = useState({ - event: "", - provider: "", - identity: "", - from: "", - to: "", - }); const [auditResult, setAuditResult] = useState<{ - activeFilters: AuditFilters; events: DashboardData["audit"]["events"]; nextCursor: string | null; } | null>(null); const [errorMessage, setErrorMessage] = useState(""); - const [loadingAction, setLoadingAction] = useState<"apply" | "more" | null>(null); + const [loadingMore, setLoadingMore] = useState(false); const requestSequence = useRef(0); const events = auditResult?.events ?? data.audit.events; const nextCursor = auditResult?.nextCursor ?? data.audit.nextCursor; - const activeFilters = auditResult?.activeFilters ?? {}; - const providerNames = useMemo(() => data.providers.map((provider) => provider.name).sort(), [data.providers]); - const identityHandles = useMemo(() => data.identities.map((identity) => identity.handle).sort(), [data.identities]); - const eventNames = useMemo(() => Array.from(new Set(events.map((event) => event.eventName))).sort(), [events]); const description = data.account.isAdmin ? "Recent administrative and credential events." : "Recent account, identity, vault, and credential events for this principal."; - function updateFilter(name: keyof Pick, value: string) { - setFilters((current) => ({ ...current, [name]: value })); - } - - function filtersForRequest(cursor?: string | null): AuditFilters { - return normalizeAuditFilters({ - event: filters.event, - provider: filters.provider, - identity: filters.identity, - from: localDateTimeToIso(filters.from || ""), - to: localDateTimeToIso(filters.to || ""), - cursor, - limit: 50, - }); - } - function nextRequestId(): number { requestSequence.current += 1; return requestSequence.current; @@ -1502,64 +1459,15 @@ export function AuditView({ data }: { data: DashboardData }) { return requestSequence.current === requestId; } - async function applyFilters(event: FormEvent) { - event.preventDefault(); - const requestFilters = filtersForRequest(); - const requestId = nextRequestId(); - setLoadingAction("apply"); - setErrorMessage(""); - try { - const result = await fetchAuditEvents(requestFilters); - if (!isLatestRequest(requestId)) return; - setAuditResult({ - activeFilters: requestFilters, - events: result.events, - nextCursor: result.nextCursor, - }); - } catch (error) { - if (!isLatestRequest(requestId)) return; - setErrorMessage(error instanceof Error ? error.message : "Failed to load audit events."); - } finally { - if (isLatestRequest(requestId)) { - setLoadingAction(null); - } - } - } - - async function clearFilters() { - const emptyFilters = { event: "", provider: "", identity: "", from: "", to: "" }; - const requestId = nextRequestId(); - setFilters(emptyFilters); - setLoadingAction("apply"); - setErrorMessage(""); - try { - const result = await fetchAuditEvents({ limit: 50 }); - if (!isLatestRequest(requestId)) return; - setAuditResult({ - activeFilters: {}, - events: result.events, - nextCursor: result.nextCursor, - }); - } catch (error) { - if (!isLatestRequest(requestId)) return; - setErrorMessage(error instanceof Error ? error.message : "Failed to load audit events."); - } finally { - if (isLatestRequest(requestId)) { - setLoadingAction(null); - } - } - } - async function loadMore() { if (!nextCursor) return; const requestId = nextRequestId(); - setLoadingAction("more"); + setLoadingMore(true); setErrorMessage(""); try { - const result = await fetchAuditEvents({ ...activeFilters, cursor: nextCursor, limit: 50 }); + const result = await fetchAuditEvents({ cursor: nextCursor, limit: 50 }); if (!isLatestRequest(requestId)) return; setAuditResult({ - activeFilters, events: [...events, ...result.events], nextCursor: result.nextCursor, }); @@ -1568,7 +1476,7 @@ export function AuditView({ data }: { data: DashboardData }) { setErrorMessage(error instanceof Error ? error.message : "Failed to load more audit events."); } finally { if (isLatestRequest(requestId)) { - setLoadingAction(null); + setLoadingMore(false); } } } @@ -1576,80 +1484,7 @@ export function AuditView({ data }: { data: DashboardData }) { return (
- - - Filters - {data.audit.scope === "global" ? "Global audit scope" : "Principal audit scope"} - - -
void applyFilters(event)}> - - - - - - - {eventNames.map((event) => ( - - - {providerNames.map((provider) => ( - - - {identityHandles.map((identity) => ( - -
- - -
-
- {errorMessage ?

{errorMessage}

: null} - - + {errorMessage ?

{errorMessage}

: null} {events.length ? ( @@ -1694,8 +1529,8 @@ export function AuditView({ data }: { data: DashboardData }) { {nextCursor ? (
-
) : null} diff --git a/ui/src/lib/authsome-api.ts b/ui/src/lib/authsome-api.ts index 926e989..2a9176c 100644 --- a/ui/src/lib/authsome-api.ts +++ b/ui/src/lib/authsome-api.ts @@ -51,12 +51,7 @@ export type AuditRow = { metadata: Record; }; -export type AuditFilters = { - event?: string; - provider?: string; - identity?: string; - from?: string; - to?: string; +export type AuditEventsQuery = { cursor?: string | null; limit?: number; }; @@ -517,20 +512,15 @@ function buildAuditRows(entries: AuditResponse["entries"]): AuditRow[] { }); } -function auditQueryString(filters: AuditFilters = {}): string { +function auditQueryString(query: AuditEventsQuery = {}): string { const params = new URLSearchParams(); - params.set("limit", String(filters.limit ?? 50)); - if (filters.event) params.set("event", filters.event); - if (filters.provider) params.set("provider", filters.provider); - if (filters.identity) params.set("identity", filters.identity); - if (filters.from) params.set("from", filters.from); - if (filters.to) params.set("to", filters.to); - if (filters.cursor) params.set("cursor", filters.cursor); + params.set("limit", String(query.limit ?? 50)); + if (query.cursor) params.set("cursor", query.cursor); return params.toString(); } -export async function fetchAuditEvents(filters: AuditFilters = {}): Promise { - const data = await requestJson(`/api/audit/events?${auditQueryString(filters)}`); +export async function fetchAuditEvents(query: AuditEventsQuery = {}): Promise { + const data = await requestJson(`/api/audit/events?${auditQueryString(query)}`); const events = buildAuditRows(data.entries); return { scope: data.scope ?? "principal",