-
Notifications
You must be signed in to change notification settings - Fork 7
feat: implement audit events and principal roles #346
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
File renamed without changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| # Audit events use OTel Logs API with a SQLite exporter owned by the server | ||
|
|
||
| The proxy runs on the client machine and the server runs remotely, so writing audit events to a local file produces two disjoint logs that an IT admin cannot view in one place. We need a single server-owned audit store that both the proxy and the server write into. | ||
|
|
||
| **Decision:** `audit/` is a pure leaf that imports only `opentelemetry-api`. It defines `AuditEvent`, translates it to an OTel `LogRecord`, and emits via the globally registered `LoggerProvider` — with no knowledge of where events go. `server/` owns a custom `SQLiteLogExporter` (implementing the OTel `LogExporter` interface), registers a `LoggerProvider` with a `BatchLogRecordProcessor` at daemon startup, and exposes `GET /audit/events` for admin queries. The proxy ships External AuditEvents to the server fire-and-forget via `POST /audit/events` rather than writing to a local file; the server enriches each inbound event with `principal_id` resolved from the PoP JWT. | ||
|
|
||
| **Considered alternatives:** | ||
|
|
||
| - *Flat JSON-lines file per process* — the current approach. Rejected because it produces two disjoint audit logs in the client/server topology, with no queryable interface for the admin view. | ||
| - *Proxy hosted on server* — rejected because it routes all agent traffic through the server machine, adding a network round-trip to every API call and making the server a traffic bottleneck. | ||
| - *Pure OTLP to an external collector* — rejected as the primary store because it requires operator-provisioned infrastructure. OTLP remains a valid future second exporter on the same `LoggerProvider` for teams that already run Grafana or Datadog. | ||
| - *SQLite owned by `audit/`* — rejected to preserve `audit/` as a dependency-free leaf. Storage decisions belong to `server/`, consistent with how all other server-owned state is managed. | ||
|
|
||
| **Not considered:** replacing loguru with OTel for operational logging. Loguru (68 call sites) serves developers debugging live systems — free-form, level-filtered, short-retention. Audit serves IT admins answering compliance questions — structured, required fields, long-retention, queryable. Routing loguru through the SQLite exporter would fill the admin view with operational noise. They are different things with different audiences and must stay separate. | ||
|
|
||
| **Consequences:** | ||
|
|
||
| - `audit/` gains `opentelemetry-api` as a dependency; `server/` gains `opentelemetry-sdk` and `aiosqlite` (or `sqlite3`). | ||
| - `audit.setup()` / `audit.clear()` are removed; `server/app.py` lifespan manages the `LoggerProvider`. | ||
| - The proxy's two existing direct `audit.log()` calls (`proxy_no_credentials`, `proxy_deny`) move to fire-and-forget HTTP posts to the server. | ||
| - A second OTLP exporter can be added to the `LoggerProvider` at any time without touching `audit/` or `proxy/`. | ||
| - All audit events — Internal (server) and External (proxy) — are queryable from a single `GET /audit/events` endpoint. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| # Principal roles: admin / user, first-created principal is admin | ||
|
|
||
| Principals need an authorization tier to gate deployment-level operations (audit log access, provider registration/deletion, cross-vault credential revocation) from per-principal operations (own connections, claim accept/reject). We store a `role` column (`admin` | `user`) directly on the `principals` table. The first Principal created on a server is assigned `admin`; all subsequent Principals receive `user`. Role assignment is immutable at creation time (mutation is deferred to a future milestone). | ||
|
|
||
| ## Considered options | ||
|
|
||
| **Environment variable (`AUTHSOME_ADMIN_PRINCIPALS`)** — the prior approach. Rejected because it requires knowing the PrincipalId before the server starts, cannot be changed without a restart, and is invisible to the UI and route layer. | ||
|
|
||
| **Separate `principal_roles` table** — considered for future extensibility (multiple roles per principal). Rejected as premature: the role model is binary and a join adds complexity without benefit today. | ||
|
|
||
| **Default admin account created at server init** — would require deciding on credentials before any real user exists. Rejected in favour of first-user-becomes-admin, which is zero-config and correct for both local and hosted deployments. | ||
|
|
||
| ## Consequences | ||
|
|
||
| - `AUTHSOME_ADMIN_PRINCIPALS` env var and `is_admin_principal()` are removed entirely. | ||
| - Admin enforcement at the route level uses a `get_admin_auth_service` FastAPI dependency (parallel to `get_protected_auth_service`) that raises `HTTP 403` for non-admin principals. | ||
| - Admin-only routes: `GET /audit/events`, `POST /providers`, `DELETE /providers/{provider}`, `POST /connections/{provider}/revoke`. | ||
| - Schema migration: `ALTER TABLE principals ADD COLUMN role TEXT NOT NULL DEFAULT 'user'`, followed by setting the earliest-created principal to `'admin'`. | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,85 +1,74 @@ | ||
| """Structured server-side event logging helpers.""" | ||
| """Structured audit event emission. | ||
|
|
||
| The audit package is intentionally storage-free. It turns Authsome audit | ||
| events into OpenTelemetry log records and leaves export decisions to the | ||
| server composition root. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import json | ||
| import threading | ||
| import uuid | ||
| from datetime import datetime | ||
| from pathlib import Path | ||
| from typing import Any | ||
| from typing import Any, Literal | ||
|
|
||
| from opentelemetry._logs import SeverityNumber, get_logger | ||
| from pydantic import BaseModel, Field | ||
|
|
||
| from authsome.utils import utc_now | ||
|
|
||
| AuditSource = Literal["internal", "external"] | ||
|
|
||
|
|
||
| class AuditEvent(BaseModel): | ||
| """Structured server-side event record.""" | ||
| """Structured audit event record emitted through OpenTelemetry logs.""" | ||
|
|
||
| event_id: str = Field(default_factory=lambda: f"audit_{uuid.uuid4().hex}") | ||
| timestamp: datetime = Field(default_factory=utc_now) | ||
| event: str | ||
| source: AuditSource = "internal" | ||
| provider: str | None = None | ||
| connection: str | None = None | ||
| identity: str | None = None | ||
| principal_id: str | None = None | ||
| status: str | None = None | ||
| metadata: dict[str, Any] = Field(default_factory=dict) | ||
|
|
||
|
|
||
| _log_path: Path | None = None | ||
| _lock = threading.Lock() | ||
|
|
||
|
|
||
| def _build_event(event_type: str, **kwargs: Any) -> AuditEvent: | ||
| filtered_kwargs = {k: v for k, v in kwargs.items() if v is not None} | ||
| return AuditEvent( | ||
| event=event_type, | ||
| provider=filtered_kwargs.pop("provider", None), | ||
| connection=filtered_kwargs.pop("connection", None), | ||
| identity=filtered_kwargs.pop("identity", None), | ||
| status=filtered_kwargs.pop("status", None), | ||
| metadata=filtered_kwargs, | ||
| def emit(event: AuditEvent) -> AuditEvent: | ||
| """Emit an audit event through the globally registered OTel logger.""" | ||
| logger = get_logger("authsome.audit") | ||
| logger.emit( | ||
| timestamp=int(event.timestamp.timestamp() * 1_000_000_000), | ||
| severity_number=SeverityNumber.INFO, | ||
| severity_text="INFO", | ||
| body=event.event, | ||
| attributes=event.model_dump(mode="json"), | ||
| event_name=event.event, | ||
| ) | ||
| return event | ||
|
|
||
|
|
||
| def emit_event( | ||
| event: str, | ||
| *, | ||
| source: AuditSource = "internal", | ||
| identity: str | None = None, | ||
| principal_id: str | None = None, | ||
| provider: str | None = None, | ||
| connection: str | None = None, | ||
| status: str | None = None, | ||
| **metadata: Any, | ||
| ) -> AuditEvent: | ||
| """Build and emit a structured audit event.""" | ||
| return emit( | ||
| AuditEvent( | ||
| event=event, | ||
| source=source, | ||
| identity=identity, | ||
| principal_id=principal_id, | ||
| provider=provider, | ||
| connection=connection, | ||
| status=status, | ||
| metadata={key: value for key, value in metadata.items() if value is not None}, | ||
| ) | ||
| ) | ||
|
|
||
|
|
||
| def setup(path: Path) -> None: | ||
| """Configure the server-side structured log path.""" | ||
| global _log_path | ||
| path.parent.mkdir(parents=True, exist_ok=True) | ||
| if not path.exists(): | ||
| path.touch() | ||
| _log_path = path | ||
|
|
||
|
|
||
| def clear() -> None: | ||
| """Clear configured server-side log state.""" | ||
| global _log_path | ||
| _log_path = None | ||
|
|
||
|
|
||
| def _serialize_event(event: AuditEvent) -> str: | ||
| payload = event.model_dump(mode="json") | ||
| metadata = payload.pop("metadata", {}) | ||
| if isinstance(metadata, dict): | ||
| payload.update(metadata) | ||
| return json.dumps(payload, separators=(",", ":")) | ||
|
|
||
|
|
||
| # TODO: Better to use an audit library: otel or something similar | ||
| def log(event_type: str, **kwargs: Any) -> None: | ||
| """Append a structured server event to the configured log file.""" | ||
| if _log_path is None: | ||
| return | ||
| line = _serialize_event(_build_event(event_type, **kwargs)) | ||
| with _lock: | ||
| _log_path.parent.mkdir(parents=True, exist_ok=True) | ||
| with _log_path.open("a", encoding="utf-8") as handle: | ||
| handle.write(line) | ||
| handle.write("\n") | ||
|
|
||
|
|
||
| # FIXME: Why is there a log and alog ? | ||
| async def alog(event_type: str, **kwargs: Any) -> None: | ||
| """Async wrapper around structured server event logging.""" | ||
| log(event_type, **kwargs) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need a separate
get_admin_auth_servicefastapi dependency? The guardrails are only in two places - one while mutating the provider client credentials and the other when displaying the provider UI (which is not an auth service property). Maybe we can just check for principal role in these two places