Skip to content

fix(audit): emit AuthPermissionDenied, HttpRequestDenied, AuthPasswordChanged#22

Merged
rrrodzilla merged 1 commit into
mainfrom
fix/audit-emission-coverage
May 28, 2026
Merged

fix(audit): emit AuthPermissionDenied, HttpRequestDenied, AuthPasswordChanged#22
rrrodzilla merged 1 commit into
mainfrom
fix/audit-emission-coverage

Conversation

@rrrodzilla

Copy link
Copy Markdown
Contributor

Summary

Closes Tier 1 of #16. Three AuditEventKind variants were declared as public API and listed in every storage parser but had no emission site in the framework. Downstream apps that rely on the framework for an audit trail got silent gaps for Cedar policy denials, rate-limit rejections, and password changes. This PR fills them in at the places where the framework already owns the code and has the AuditLogger in scope.

Changes

AuthPermissionDenied

  • HTTP (middleware/cedar.rs) — emit on Decision::Deny with a populated AuditSource (IP from X-Forwarded-For / X-Real-IP, user-agent, subject from JWT claims, request-id). Severity: Warning.
  • gRPC (middleware/cedar.rs) — emit on Decision::Deny with subject + UA + request-id from gRPC metadata. gRPC has no peer IP at this layer; ip stays None. Severity: Warning.

HttpRequestDenied

  • middleware/rate_limit.rs — emit when check_rate_limit_with_route returns Error::RateLimitExceeded. Other error variants (Redis down, etc.) are not audit-worthy and don't emit. Severity: Warning.

AuthPasswordChanged

  • accounts/mod.rsAuditAccountNotification was coarsely mapping AccountEvent::PasswordChanged to AuditEventKind::AccountUpdated with action: "password_changed" metadata. Remapped to the dedicated AuthPasswordChanged kind at Notice severity. Downstream consumers parsing the kind now get a distinct, security-relevant event instead of having to inspect metadata.

Breaking change

The AccountUpdated event with action: "password_changed" no longer fires. Downstream SIEM rules keyed on that combination need to switch to auth.password.changed. Justifies a minor-version bump.

Out of scope (tracked under #16)

  • Tier 2AuthApiKeyCreated, AuthApiKeyRevoked, AuthTokenRefresh — these live behind storage traits (ApiKeyStorage, RefreshTokenStorage) that don't carry an AuditLogger. Needs a notification-handler pattern like AuditAccountNotification for each, wired through all four backends.
  • Tier 3AuthLogout, AuthOAuthCallbackAuthSession::logout() is sync and has no path to the AuditLogger; the OAuth callback path isn't owned by the framework. Both need design decisions before implementation.

Test plan

  • cargo clippy -p acton-service --all-targets --features full -- -D warnings — clean
  • cargo clippy -p acton-service --all-targets --no-default-features --features "full,crypto-ring" -- -D warnings — clean
  • cargo nextest run -p acton-service --features full — 520/520 pass
  • Verify downstream (schemaforge) sees the three new event kinds reach storage and roll up correctly in SIEM dashboards.

…dChanged

Three audit variants were declared in AuditEventKind and listed in every
storage parser but had zero emission sites in the framework. Downstream
apps relying on the framework for an audit trail got silent gaps for
Cedar policy denials, rate-limit rejections, and password changes.

- AuthPermissionDenied: emit on Decision::Deny in both the Cedar HTTP
  middleware (with full AuditSource) and the Cedar gRPC tower service
  (with subject + UA + request-id from metadata; gRPC has no peer IP at
  this layer).
- HttpRequestDenied: emit in the rate_limit middleware when
  check_rate_limit_with_route returns Error::RateLimitExceeded. Other
  error variants (Redis down, etc.) are not audit-worthy and don't
  emit.
- AuthPasswordChanged: AuditAccountNotification was coarsely emitting
  AccountUpdated with action="password_changed" metadata. Remap to the
  dedicated AuthPasswordChanged kind at Notice severity; downstream
  consumers parsing the kind get a distinct, security-relevant event
  instead of having to inspect action metadata.

Tier 1 of #16. Tier 2 (AuthApiKey*, AuthTokenRefresh) and Tier 3
(AuthLogout, AuthOAuthCallback) need design decisions on how to thread
AuditLogger through storage traits and session types, tracked separately.

Refs #16
@rrrodzilla rrrodzilla merged commit 5aaf725 into main May 28, 2026
2 checks passed
rrrodzilla added a commit that referenced this pull request May 28, 2026
Reflects the audit-event work that landed in #14, #21, #22 and is
queued in #23, #25:

- audit/page.md: rewrite the "Auth Events (Automatic)" table to match
  the new emission set (AuthLoginSuccess at Notice; AuthTokenMissing /
  AuthTokenInvalid added; AuthTokenRevoked notes jti metadata;
  AuthPermissionDenied and HttpRequestDenied added); update the
  syslog and OTLP example severities to Notice; expand the "Event
  Kinds" reference table; add a migration callout for AuthLoginFailed.
- cedar-auth/page.md: add "Audit Integration" section describing
  automatic AuthPermissionDenied emission on Decision::Deny.
- rate-limiting/page.md: add "Audit Integration" section describing
  automatic HttpRequestDenied emission on RateLimitExceeded.
- token-auth/page.md: add "Audit Emission" section covering the four
  middleware-emitted kinds, the jti correlation field, and the
  AuthLoginFailed migration.

Refs #13 #15 #16 #18 #19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant