From 10808e4a176a685f315400c4a2fd654ca07e98bf Mon Sep 17 00:00:00 2001 From: Roland Rodriguez Date: Tue, 26 May 2026 15:44:48 -0500 Subject: [PATCH] feat(audit): emit user-management and file-lifecycle events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit routes/users.rs and routes/files.rs were the largest remaining gap in the production audit chain: user CRUD, role mutations, password changes, and every file upload/scan/download path produced zero durable chain entries. Tracing-only events don't satisfy NIST 800-53 AU-2 for high-sensitivity admin operations or CUI access. routes/users.rs: forge.user.created create_user success forge.user.deleted delete_user success forge.user.updated update_user success (roles/display_name) forge.user.active_toggled update_user success (active flag) forge.access.denied every Cedar role-rank guard deny auth.password.changed change_password success (built-in kind) forge.user.password_changed change_password success (carries actor + self-service flag — the built-in kind only carries the target) routes/files.rs: forge.file.upload_minted mint_upload_url success forge.file.uploaded confirm_upload success forge.file.scan_complete scan_complete success (Warning severity when quarantined; Notice when clean) forge.file.downloaded download_file success A small `audit_user` / `audit_file` helper centralises actor / target / metadata shape; `FileTarget` packs the schema/entity/field triple so the audit_file signature stays under clippy's too_many_arguments ceiling. Second of four PRs from the production-audit gap review. Follows #61 (auth audit events + last_login). C and D coming next. --- crates/schema-forge-acton/src/routes/files.rs | 125 ++++++++++++ crates/schema-forge-acton/src/routes/users.rs | 188 +++++++++++++++++- 2 files changed, 312 insertions(+), 1 deletion(-) diff --git a/crates/schema-forge-acton/src/routes/files.rs b/crates/schema-forge-acton/src/routes/files.rs index 7a9dea7..51b2cec 100644 --- a/crates/schema-forge-acton/src/routes/files.rs +++ b/crates/schema-forge-acton/src/routes/files.rs @@ -18,6 +18,7 @@ use std::collections::BTreeMap; use std::sync::Arc; use std::time::Duration; +use acton_service::audit::AuditSeverity; use acton_service::middleware::Claims; use acton_service::prelude::ActorHandleInterface; use acton_service::state::AppState; @@ -54,6 +55,42 @@ use crate::storage::{S3Client, StorageRegistry}; /// budgets stay aligned across the runtime. const ACTOR_TIMEOUT: Duration = Duration::from_secs(5); +/// Identifier triple for a file attachment, packed so audit emission +/// signatures don't exceed clippy's `too_many_arguments` ceiling. +struct FileTarget<'a> { + schema: &'a str, + entity_id: &'a str, + field: &'a str, +} + +/// Emit a file-lifecycle audit event to the durable chain. +/// +/// File handlers carry highly variable per-event metadata (key, mime, size, +/// reason). Keeping this in one helper means a future "redact this field +/// from chain entries" change touches one place. +async fn audit_file( + state: &AppState, + event: &'static str, + severity: AuditSeverity, + actor: Option<&str>, + target: FileTarget<'_>, + extra: serde_json::Value, +) { + let Some(logger) = state.audit_logger() else { return }; + let mut metadata = serde_json::json!({ + "actor": actor.unwrap_or("_anonymous"), + "schema": target.schema, + "entity_id": target.entity_id, + "field": target.field, + }); + if let (Some(obj), Some(extra_obj)) = (metadata.as_object_mut(), extra.as_object()) { + for (k, v) in extra_obj { + obj.insert(k.clone(), v.clone()); + } + } + logger.log_custom(event, severity, Some(metadata)).await; +} + // --------------------------------------------------------------------------- // Request / response types // --------------------------------------------------------------------------- @@ -186,6 +223,26 @@ pub async fn mint_upload_url( "minted upload url" ); + audit_file( + &state, + "forge.file.upload_minted", + AuditSeverity::Informational, + claims.as_ref().map(|c| c.sub.as_str()), + FileTarget { + schema: ctx.schema.name.as_str(), + entity_id: ctx.entity.id.as_str(), + field: &field, + }, + serde_json::json!({ + "key": presigned.key, + "filename": body.filename, + "mime": body.mime, + "size": body.size, + "expires_at": expires_at.to_rfc3339(), + }), + ) + .await; + Ok(Json(MintUploadUrlResponse { upload_url: presigned.url, key: presigned.key, @@ -315,6 +372,26 @@ pub async fn confirm_upload( ) .await; + audit_file( + &state, + "forge.file.uploaded", + AuditSeverity::Notice, + claims.as_ref().map(|c| c.sub.as_str()), + FileTarget { + schema: ctx.schema.name.as_str(), + entity_id: ctx.entity.id.as_str(), + field: &field, + }, + serde_json::json!({ + "key": attachment.key, + "mime": attachment.mime, + "size": attachment.size, + "status": attachment.status.as_str(), + "checksum_sha256": attachment.checksum, + }), + ) + .await; + Ok(Json(AttachmentResponse { status: attachment.status.as_str().to_string(), attachment, @@ -419,6 +496,32 @@ pub async fn scan_complete( ) .await; + // Quarantined outcomes are Warning severity: an admin should be able to + // pull just those out of the chain when triaging an incident. Clean + // outcomes stay Notice. + let severity = if matches!(current.status, FileStatus::Quarantined) { + AuditSeverity::Warning + } else { + AuditSeverity::Notice + }; + audit_file( + &state, + "forge.file.scan_complete", + severity, + claims.as_ref().map(|c| c.sub.as_str()), + FileTarget { + schema: ctx.schema.name.as_str(), + entity_id: ctx.entity.id.as_str(), + field: &field, + }, + serde_json::json!({ + "key": current.key, + "status": current.status.as_str(), + "reason": body.reason, + }), + ) + .await; + Ok(Json(AttachmentResponse { status: current.status.as_str().to_string(), attachment: current, @@ -455,6 +558,28 @@ pub async fn download_file( let client = resolve_backend(&ctx.registry, &ctx.constraints.bucket)?; + audit_file( + &state, + "forge.file.downloaded", + AuditSeverity::Notice, + claims.as_ref().map(|c| c.sub.as_str()), + FileTarget { + schema: ctx.schema.name.as_str(), + entity_id: ctx.entity.id.as_str(), + field: &field, + }, + serde_json::json!({ + "key": attachment.key, + "mime": attachment.mime, + "size": attachment.size, + "access": match ctx.constraints.access { + FileAccess::Presigned => "presigned", + FileAccess::Proxied => "proxied", + }, + }), + ) + .await; + match ctx.constraints.access { FileAccess::Presigned => { let url = client diff --git a/crates/schema-forge-acton/src/routes/users.rs b/crates/schema-forge-acton/src/routes/users.rs index 1119ee9..4dbed8a 100644 --- a/crates/schema-forge-acton/src/routes/users.rs +++ b/crates/schema-forge-acton/src/routes/users.rs @@ -24,6 +24,7 @@ use std::collections::BTreeMap; use std::sync::Arc; +use acton_service::audit::{AuditEventKind, AuditSeverity, AuditSource}; use acton_service::middleware::Claims; use acton_service::state::AppState; use axum::extract::{Path, State}; @@ -50,6 +51,69 @@ use crate::messages::{GetPolicyStore, GetSchema, ReplyChannel}; use crate::state::DynAuthStore; use acton_service::prelude::ActorHandleInterface; +// --------------------------------------------------------------------------- +// Audit helpers +// --------------------------------------------------------------------------- + +/// Emit a structured user-management audit event. +/// +/// User CRUD, role mutations, and password changes are all high-sensitivity +/// admin operations that need a durable chain entry for NIST 800-53 AU-2. +/// The acton-service audit chain owns sequencing and BLAKE3 hash linkage; +/// this helper centralises severity selection and the `actor / target / +/// action` metadata shape so every emission stays consistent. +async fn audit_user( + state: &AppState, + event: &'static str, + severity: AuditSeverity, + actor: &str, + target: &str, + extra: Option, +) { + let Some(logger) = state.audit_logger() else { return }; + let mut metadata = serde_json::json!({ + "actor": actor, + "target": target, + }); + if let (Some(obj), Some(extra)) = (metadata.as_object_mut(), extra) { + if let Some(extra_obj) = extra.as_object() { + for (k, v) in extra_obj { + obj.insert(k.clone(), v.clone()); + } + } + } + logger.log_custom(event, severity, Some(metadata)).await; +} + +/// Emit the built-in `auth.password.changed` event with the actor and target. +async fn audit_password_changed( + state: &AppState, + actor: &str, + target: &str, +) { + let Some(logger) = state.audit_logger() else { return }; + logger + .log_auth( + AuditEventKind::AuthPasswordChanged, + AuditSeverity::Notice, + AuditSource { + subject: Some(format!("user:{target}")), + ..AuditSource::default() + }, + ) + .await; + // Also drop a custom event carrying the *actor* so a self-service + // change and an admin-driven reset are distinguishable in the chain. + let metadata = serde_json::json!({ + "actor": actor, + "target": target, + "self_service": actor == target, + }); + logger + .log_custom("forge.user.password_changed", AuditSeverity::Notice, Some(metadata)) + .await; +} + // --------------------------------------------------------------------------- // Auth helpers // --------------------------------------------------------------------------- @@ -493,6 +557,19 @@ pub async fn create_user( message: format!("authz engine error during create_user: {e}"), })?; if !decision.is_allow() { + audit_user( + &state, + "forge.access.denied", + AuditSeverity::Warning, + &claims.sub, + &body.username, + Some(serde_json::json!({ + "action": "create_user", + "proposed_roles": body.roles, + "reason": "role_rank_guard", + })), + ) + .await; return Err(ForgeError::Forbidden { message: format!( "creating user with roles {:?} would exceed caller's role_rank", @@ -525,6 +602,19 @@ pub async fn create_user( message: format!("created user '{}' not found on readback", body.username), })?; + audit_user( + &state, + "forge.user.created", + AuditSeverity::Notice, + &claims.sub, + &body.username, + Some(serde_json::json!({ + "roles": body.roles, + "display_name": display_name, + })), + ) + .await; + Ok((StatusCode::CREATED, Json(user_to_response(&created)))) } @@ -579,6 +669,18 @@ pub async fn delete_user( message: format!("authz engine error during delete_user: {e}"), })?; if !decision.is_allow() { + audit_user( + &state, + "forge.access.denied", + AuditSeverity::Warning, + &claims.sub, + &username, + Some(serde_json::json!({ + "action": "delete_user", + "reason": "role_rank_guard", + })), + ) + .await; return Err(ForgeError::Forbidden { message: format!("not authorized to delete user '{username}'"), }); @@ -603,6 +705,18 @@ pub async fn delete_user( } auth_store.delete_user(&username).await?; + audit_user( + &state, + "forge.user.deleted", + AuditSeverity::Notice, + &claims.sub, + &username, + Some(serde_json::json!({ + "deleted_roles": target.roles, + "was_platform_admin": target_is_platform_admin, + })), + ) + .await; Ok(StatusCode::NO_CONTENT) } @@ -654,6 +768,19 @@ pub async fn update_user( message: format!("authz engine error during update_user (existing): {e}"), })?; if !decision_existing.is_allow() { + audit_user( + &state, + "forge.access.denied", + AuditSeverity::Warning, + &claims.sub, + &username, + Some(serde_json::json!({ + "action": "update_user", + "guard": "current_entity", + "reason": "role_rank_guard", + })), + ) + .await; return Err(ForgeError::Forbidden { message: format!("not authorized to edit user '{username}'"), }); @@ -696,6 +823,20 @@ pub async fn update_user( message: format!("authz engine error during update_user (proposed): {e}"), })?; if !decision_proposed.is_allow() { + audit_user( + &state, + "forge.access.denied", + AuditSeverity::Warning, + &claims.sub, + &username, + Some(serde_json::json!({ + "action": "update_user", + "guard": "proposed_entity", + "proposed_roles": new_roles, + "reason": "role_rank_guard", + })), + ) + .await; return Err(ForgeError::Forbidden { message: format!( "updating '{username}' to roles {new_roles:?} would exceed caller's role_rank" @@ -727,16 +868,48 @@ pub async fn update_user( // Persist. `update_user` covers roles + display_name; `toggle_user_active` // covers the active flag. We only call each store method when its - // governed field actually changed, so the audit trail stays clean. + // governed field actually changed, so the audit chain entries below + // distinguish roles/name updates from active-flag toggles. let roles_or_name_changed = body.roles.is_some() || body.display_name.is_some(); if roles_or_name_changed { auth_store .update_user(&username, &new_roles, &new_display_name) .await?; + let mut changed = Vec::new(); + if body.roles.is_some() && new_roles != current.roles { + changed.push("roles"); + } + if body.display_name.is_some() && Some(&new_display_name) != current.display_name.as_ref() { + changed.push("display_name"); + } + audit_user( + &state, + "forge.user.updated", + AuditSeverity::Notice, + &claims.sub, + &username, + Some(serde_json::json!({ + "changed_fields": changed, + "prev_roles": current.roles, + "new_roles": new_roles, + })), + ) + .await; } if body.active.is_some() && new_active != current.active { auth_store.toggle_user_active(&username).await?; + audit_user( + &state, + "forge.user.active_toggled", + AuditSeverity::Notice, + &claims.sub, + &username, + Some(serde_json::json!({ + "active": new_active, + })), + ) + .await; } let updated = auth_store @@ -793,6 +966,18 @@ pub async fn change_password( message: format!("authz engine error during change_password: {e}"), })?; if !decision.is_allow() { + audit_user( + &state, + "forge.access.denied", + AuditSeverity::Warning, + &claims.sub, + &username, + Some(serde_json::json!({ + "action": "change_password", + "reason": "role_rank_guard", + })), + ) + .await; return Err(ForgeError::Forbidden { message: format!("not authorized to change password for user '{username}'"), }); @@ -802,6 +987,7 @@ pub async fn change_password( auth_store .change_password(&username, &body.password) .await?; + audit_password_changed(&state, &claims.sub, &username).await; Ok(StatusCode::NO_CONTENT) }