Repository navigation
Conversation
Every management mutation is recorded before it runs (`action.attempted`) and after (`action.completed`, with outcome and HTTP status), and every authorization denial as `authorization.denied`. Records name the actor, permission, target, time and request. If the attempt cannot be recorded, the request fails with 503 `audit_unavailable` and nothing changes. A mutating request that never checked a permission is recorded as `unaudited`. Records go to an append-only `admin_audit_events` table, whose triggers reject UPDATE and DELETE, and to stdout as `agentdesktop.audit.v1` JSON lines for the deployment's log shipper. Each controller process numbers its lines (`stream.id`, `stream.seq`), so the SIEM can detect a lost line by a gap. - Migration files ending in `_sqlite` or `_postgres` run only on that database; the audit table uses them for its auto-increment key and its triggers. - New `audit.read` permission, covered by `admin.read`; the `GET /api/v1/audit` endpoint; and an Audit page in the controller UI. - Compiled-in authentication providers can record their own sign-in and sign-out events through `AuditLog::record`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Enrollment and certificate renewal happen on the device-facing fleet API, so they were missing from the audit log. Both issue a credential, and enrollment is how a phished login gets a rogue device in, so the SIEM needs them. - Enrollment records `action.attempted` and `action.completed` with action `devices.enroll`. The actor is the signed-in user and the target is the new device with its reported hostname. The device is admitted only after the attempt is recorded (fail closed); the insert and its completion record run in their own task so a disconnect cannot separate them. - `device.enrollment_denied` is recorded when an enrollment is refused after the user's ID token was verified: nonce or token subject mismatch, a rejected access token, or an invalid certificate request. Earlier failures name nobody and anyone can cause them on the public fleet API, so they are not recorded. `OidcProvider::complete` now reports the verified sign-in with its rejection. - `device.certificate_renewed` is recorded, failing closed, with the device as actor (kind `device`). - Audit events gain an optional `target.name`, and the Audit page shows device actors and enrollment events. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The fleet API records device enrollment and renewal with the same audit types as the management API, so `management::audit` becomes `crate::audit`. The `Principal` conversion for `Actor` moves to `management::auth`, so the audit module no longer depends on management. Import paths change; the record format does not. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Critical flaws allow timed-out requests to delete devices later and SQLite INSERT OR REPLACE to silently rewrite supposedly append-only audit history.
4 open issues
What changed in this PR
Adds audit logging to the controller’s management and fleet APIs, with persistent records, structured stdout output, and an operator-facing Audit page.
Changes:
- Adds audit event types, database-specific migrations, append-only protections, and paginated retrieval.
- Records management authorization decisions, mutation outcomes, device enrollments, and certificate renewals.
- Adds audit navigation, record details, pagination, backend tests, and Storybook coverage.
| File | Description |
|---|---|
| README.md | Documents audit coverage, storage, and log forwarding. |
| frontend/controller/src/views/AuditView.tsx | Displays audit records and expandable details. |
| frontend/controller/src/views/AuditView.stories.tsx | Covers audit display states and interactions. |
| frontend/controller/src/types.ts | Defines audit event and pagination types. |
| frontend/controller/src/styles.css | Styles audit details and pagination controls. |
| frontend/controller/src/stories/fixtures.ts | Adds representative audit records. |
| frontend/controller/src/components/ControllerShell.tsx | Adds permission-aware Audit navigation. |
| frontend/controller/src/App.tsx | Routes the Audit page and loads older records. |
| crates/controller/src/service.rs | Audits enrollment and certificate renewal. |
| crates/controller/src/oidc.rs | Supplies verified identity evidence for enrollment denials. |
| crates/controller/src/management/handlers.rs | Adds the protected, paginated audit endpoint. |
| crates/controller/src/management/auth/tests.rs | Tests audit recording, failures, pagination, and cancellation. |
| crates/controller/src/management/auth/authorization.rs | Registers the audit-read permission. |
| crates/controller/src/management/auth.rs | Records authorization denials and mutation attempts and outcomes. |
| crates/controller/src/management.rs | Wires audit state, routing, and peer-address capture. |
| crates/controller/src/lib.rs | Exposes the audit module. |
| crates/controller/src/database/audit.rs | Implements audit storage, retrieval, and database tests. |
| crates/controller/src/database.rs | Selects database-specific migrations. |
| crates/controller/src/audit.rs | Defines events and coordinates database and stdout writes. |
| crates/controller/src/application.rs | Supplies the audit sink during authentication setup. |
| crates/controller/migrations/0005_admin_audit_sqlite.sql | Creates SQLite audit storage and protection triggers. |
| crates/controller/migrations/0005_admin_audit_postgres.sql | Creates PostgreSQL audit storage and protection triggers. |
| crates/controller/Cargo.toml | Enables timestamp formatting support. |
Review effort: Balanced
Comment on lines
+22
to
+24
| CREATE TRIGGER admin_audit_events_no_delete | ||
| BEFORE DELETE ON admin_audit_events | ||
| BEGIN SELECT RAISE(ABORT, 'admin_audit_events is append-only'); END; |
Comment on lines
+314
to
+318
| let task = tokio::spawn({ | ||
| let context = context.clone(); | ||
| async move { | ||
| let response = next.run(request).await; | ||
| record_completion(&context, response.status()).await; |
Comment on lines
+822
to
+823
| The controller records every management mutation and every denied management | ||
| request. Each record has the actor (credential kind, issuer, subject and, when |
| Outcome::Denied, | ||
| ) | ||
| .target(DEVICES, None) | ||
| .target_name(&attempt.hostname) |
This branch has not been deployed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This pull request implements the admin audit logging feature as outlined in item A5 of the project plan. The changes include significant updates to both the backend and frontend components to support audit logging functionality.
Key Changes:
Backend Enhancements:
auditmodule in the controller to handle audit logging.managementmodule to include audit logging capabilities, including logging of actions, denials, and unchecked mutations.Authmiddleware to log relevant audit events, including the client's network address.Frontend Updates:
AuditViewand associated stories.Configuration Changes:
X-Forwarded-Forheader.Code Cleanup:
EventKindandOutcometo reduce duplication and improve maintainability.This implementation ensures that all relevant actions are logged for security and compliance purposes, providing a comprehensive audit trail for administrative actions.