Skip to content

docs(spec): audit and throttle authentication failures - #24

Draft
antstanley wants to merge 1 commit into
spec/fail-closed-config-and-adaptersfrom
spec/audit-and-throttle-auth-failures
Draft

docs(spec): audit and throttle authentication failures#24
antstanley wants to merge 1 commit into
spec/fail-closed-config-and-adaptersfrom
spec/audit-and-throttle-auth-failures

Conversation

@antstanley

@antstanley antstanley commented Aug 5, 2026

Copy link
Copy Markdown
Owner

Note

Stacked PR 4 of 16 — part of stack #35, which targets main.
Base: #25 (spec/fail-closed-config-and-adapters) · Followed by: #19

This PR's diff shows only its own change. Merge the stack bottom-up; do not merge this before its base.

Why it sits at position 4: Introduces the mandatory SecurityEvent channel, the RateLimiter port, ClientAddr and the TooManyRequests mapping. #19, #26 and #31 all build on this machinery.

Scope of this PR

This PR currently carries the change spec. It will also carry the implementation plan and the build work for that spec — plan and code land as later commits on this same branch, so the spec, its plan, and its implementation review and merge as one unit.

Review order within the PR: spec first, then plan, then implementation.


Change spec proposing that authentication failures be recorded and throttled.

Problem. Four defects that combine so that a brute-force campaign against /token is invisible:

  1. ValidationFailed is emitted at Debug (7) against a default Info (6) floor, so the shipped defaults discard every failed-authentication event before any adapter sees it.
  2. Failed validation and unknown-provider requests propagate out of exchange via ? before any event is emitted, so those paths record nothing at any level. There is no access log either.
  3. When the audit sink fails the request still succeeds, silently dropping five record types (UserCreated, TokenExchange, TokenRefresh, AllSessionsRevoked, TokenRevocation).
  4. There is no rate limiting on the public routes, so guessing is unlimited as well as unrecorded.

Proposed delta. Split audit emission into a mandatory SecurityEvent channel whose severity derives from a closed enum and which no configured threshold can filter, leaving emit_threshold/blocking_threshold to govern a best-effort channel only. Move terminal-outcome emission to a single exit point per flow so no failure can leave exchange/refresh unrecorded.

Make a mandatory-channel write failure fail the operation (audit.durability, shipping observe then flipping to enforce) and flip the shipped audit.adapter default from noop to stdout. The cost is named plainly: a remote-sink outage becomes an availability event for /token and /revoke.

Add a RateLimiter port with per-IP / per-IP-failures / per-subject / per-provider limits plus a global concurrency bound on the public routes, returning 429 slow_down with Retry-After, keyed only on a ClientAddr the server established via into_make_service_with_connect_info and server.trusted_proxies/trusted_proxy_hops — never on an unvalidated X-Forwarded-For.

Notes.

  • crates/server/src/main.rs calls axum::serve(listener, app) with no ConnectInfo, so no peer address exists today — the per-IP fix cannot be middleware-only. This is a structural prerequisite the finding did not name.
  • The spec explicitly reverses a Decision from changes/merged/2026-07-01-wire_audit_event_emission.md (which put ValidationFailed at debug behind emit_threshold) and argues why that reasoning was wrong, rather than quietly overriding it.
  • Two further hazards specified: the terminal success event fires after the session write, so an enforced audit failure must revoke that session or leave a live refresh token the caller never received; and the failure reason must be a fixed classification string, because ProviderError's Display embeds the upstream response body verbatim.

Drafted with the spec-creator plugin from a deep security scan of the repository at 53cbdec9. Spec only — no source changes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@antstanley
antstanley force-pushed the spec/audit-and-throttle-auth-failures branch from 2f9ace8 to aee02c0 Compare August 5, 2026 12:01
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