feat(authenticator): layer-2 rate limiting + audit events (nginx+auth step 10, PR 3/4)#1850
Open
cyberantonz wants to merge 10 commits into
Open
feat(authenticator): layer-2 rate limiting + audit events (nginx+auth step 10, PR 3/4)#1850cyberantonz wants to merge 10 commits into
cyberantonz wants to merge 10 commits into
Conversation
…ace (steps 10.1–10.2)
Item 1 — POST /auth/refresh (G10 rotation model, no swap keys):
a fresh CSPRNG token mapping is written and the superseded mapping's TTL
drops to refresh_grace_ms (default 250 ms) — the expiring old mapping IS the
grace window — while the session's expires_at advances to min(now +
session_ttl, absolute cap) across the record, its key TTL, and the per-user
index score, all in one pipeline. The stable session_id and the linked JWT
are untouched. A stale token inside the grace window resolves to the same
session and is answered with the current credential (no re-rotation); past
grace or past either cap → 401 + cleared cookie. Response is
{expires_at, refresh_at} with refresh_at = expires_at − 90 s ± uniform(60 s)
(big-jitter decision, G8), re-jittered per call and shared with GET /auth/me;
cookie Max-Age is the actual remaining session life.
Item 2 — session management:
GET /auth/sessions lists the caller's live sessions from the per-user ZSET
(created_at, expires_at, user_agent, ip, current flag; attribution captured
at login from User-Agent + first X-Forwarded-For hop, length-capped).
DELETE /auth/sessions/{id} revokes one owned session (absent and not-owned
are both 404 — no existence oracle); DELETE /auth/sessions revokes everything
for the current user. Every revoke runs the standard pipeline: token mappings
+ session + linked JWT + index entries in one MULTI/EXEC.
The admin/service variant DELETE /auth/admin/users/{person_id}/sessions is a
.authenticated() operation: the host authn pipeline (cf-gears-oidc-authn-plugin,
newly linked) verifies the ES256 gateway JWT against the authenticator's own
issuer, and the handler requires one of admin_revoke_roles (default
["session_admin"]) before delegating to the SDK contract
(AuthenticatorClientV1::revoke_user_sessions) — the lever the future
permissions service pulls (DD-AUTH-07).
Config wiring: committed host config flips to auth_disabled: false with a
fail-closed .invalid placeholder issuer; dev compose bind-mounts a full-auth
override (authn-tls issuer + self-signed CA) and grants the dev testclient
the session_admin role; the Helm configmap renders the plugin block off
tlsDiscovery (real issuer + in-pod CA when enabled, dark otherwise).
Also repairs the e2e harness: identity-stub readiness probed the old
/v1/persons path, and two stale ignored e2e asserts predated the
space-delimited roles claim and the UUIDv5 service sub. run-e2e.sh now also
runs the new refresh + sessions loops; all four e2e loops pass locally.
EPIC: constructorfabric#1583 (step 10, constructorfabric#1593)
Signed-off-by: Anton Zelenov <[email protected]>
Second line behind SameSite=Strict (PRD 5.11 / DESIGN 4.2, salvaged spec): middleware over the route table checks POST/PUT/PATCH/DELETE under /auth/*. X-CSRF-Token is compared in constant time (fixed-size SHA-256 digests) against the per-session token minted at login; with no header, the Origin allowlist (csrf_origins) is the fallback; empty allowlist (the default) is fail closed — token required. A presented-but-wrong token is never rescued by the Origin fallback. Requests without a resolvable session pass through (the handler answers 401 — nothing to forge); a Redis failure answers 503, never a bypass. The back-channel logout endpoint is exempt: IdP server-to-server, its credential is the signed logout_token. GET /auth/csrf issues the session's token; /auth/me echoes it so one call primes both the refresh timer and the CSRF header at page load. Config: csrf_origins in the committed config (empty = fail closed), the dev compose override (Vite + gateway origins keep browser flows working until the SPA header lands everywhere), and a new chart value csrfOrigins. The SPA side (store csrf_token from /auth/me, send X-CSRF-Token on logout) lands in insight-front (feat/auth-csrf-header). e2e now asserts 403-without / pass-with the header on refresh and log-out-everywhere; all loops green. EPIC: constructorfabric#1583 (step 10, constructorfabric#1593) Signed-off-by: Anton Zelenov <[email protected]>
M2 (QA/security review): make /auth/refresh rotation a compare-and-swap on the session's current_token (atomic Lua) instead of an unconditional pipeline. Two concurrent refreshes of the same cookie (multi-tab, within the refresh burst) could both pass the grace check and both rotate, leaving the first new token mapping written with the full session TTL and never demoted to grace — a live parallel credential that revoke never touches. The CAS lets only one rotate; the loser re-loads and answers the grace path with the winner's current credential. No orphan mapping. M5: document the fail-closed CSRF deploy coordination in the chart values — roll the header-sending insight-front first, or set csrfOrigins, or logout/ refresh 403 during the transition. EPIC: constructorfabric#1583 (step 10, constructorfabric#1593) Signed-off-by: Anton Zelenov <[email protected]>
POST /auth/oidc/back-channel-logout validates the logout_token per OIDC BCL
1.0: signature via the configured IdP's JWKS (fetched fresh per call — cold
path, picks up key rotation), iss against the one trusted issuer, aud
against our client_id, iat freshness inside a skew/max-age window
(backchannel_clock_skew_seconds 60 / backchannel_token_max_age_seconds 300),
the mandatory back-channel events member, sub-or-sid presence, and no nonce
(a nonce marks a replayed id_token). Rejections are 400 with a coarse
reason; JWKS unavailability is 503 (retryable by the IdP), never a bypass.
Replay guard: asm:logout_jti:{iss}:{jti} SET NX EX with TTL = iat + max_age
+ skew − now; a replayed delivery answers 200 idempotently without another
revoke. Success answers 200 no-store (BCL §2.7).
Target resolution: (iss, sid) via the existing asm:sid_index; a sub-only
token takes the documented fallback — a new asm:sub_index:{iss}:{sub} SET
(maintained in the create/revoke pipelines) resolves the user's sessions,
and EVERYTHING for that user is revoked through the standard pipeline with
the operator-facing warn line (blast radius visible, not silent). The sub
index replaces the PRD's 'resolve via Identity Service' sketch: Identity's
lookup is email-keyed (a logout_token carries no email), and an index keeps
the logout path free of a cross-service dependency; the spec text is updated
in the step-10 docs pass.
e2e via fakeidp /_control/backchannel: two live devices die on one signed
logout_token; garbage tokens are 400. All six e2e loops pass locally.
EPIC: constructorfabric#1583 (step 10, constructorfabric#1593)
Signed-off-by: Anton Zelenov <[email protected]>
RunnableCapability::start now spawns the leader-elected refresher: one
leader (Redis lock asm:leader:idp_refresher, SET NX PX + same-holder renew,
TTL = 3 ticks) polls asm:idp_refresh_due (ZRANGEBYSCORE 0 now, bounded) every
idp.refresher_tick_seconds (default 5) and spawns per-session refresh tasks
behind a Semaphore(idp.refresh_concurrency) — politeness toward the customer
IdP, not our capacity. Each task holds a per-session rotation lock (SET NX PX
30 s): refresh tokens are one-time-use at most IdPs, and two workers racing a
rotation would burn the grant and falsely kill the session.
Outcomes (fail open on transport, fail closed on verdict):
- success → store the rotated refresh token + new access expiry, reset the
failure counter, re-schedule margin-before-expiry with write-time jitter
(idp.refresh_due_jitter_seconds, G5 anti-herding);
- invalid_grant (definitive: revoked/expired/disabled) → revoke the owning
session through the standard pipeline (audited); the user's other sessions
hold their own grants and die at their own next refresh, so IdP-side
deactivation converges within about one access-token lifetime;
- transient (network, 5xx, 429) → exponential backoff min(15<<n, 300) s with
jitter, retry, NEVER revoke.
Login-side policy fixes: a session with no refresh token is no longer
scheduled (nothing to refresh), and no_refresh_token_policy=strict now
actually caps the session at the IdP access-token lifetime (login_only keeps
the absolute cap). Due-time jitter at login now comes from config.
Metrics (OTel global meter, exported by the toolkit host pipeline):
idp_refresh_total{result}, idp_refresh_consecutive_failures gauge (alert
before the mass logout), idp_refresh_invalid_grant_total.
e2e via the fakeidp control hooks at a fast lifecycle (token TTL 15 s,
margin 10 s, tick 1 s): /_control/outage 5xx logs nobody out;
/_control/revoke kills the victim's session on the next scheduled refresh
while another user's session survives. All seven e2e loops pass locally.
EPIC: constructorfabric#1583 (step 10, constructorfabric#1593)
Signed-off-by: Anton Zelenov <[email protected]>
Per-key TTLs remove session records and token mappings, but the per-user ZSET indexes and the refresh schedule keep dead members until trimmed. One leader (asm:leader:janitor, the same SET NX PX + same-holder-renew election as the refresher) runs a pass every janitor_interval_seconds (default 30 s): SCAN asm:user_sessions:* (bounded batches, never KEYS) + ZREMRANGEBYSCORE 0 now per index, plus dropping refresh-schedule entries overdue by more than 10 min (live sessions are rescheduled every attempt — a long-overdue entry has no owner). Metrics: auth_janitor_removed_total counter and auth_janitor_backlog_size gauge (expired-but-untrimmed members seen by the last pass — rises when no pod is running passes, per DESIGN 4.3). EPIC: constructorfabric#1583 (step 10, constructorfabric#1593) Signed-off-by: Anton Zelenov <[email protected]>
…H2, M1, M3, M4, L2, L3) H1: give the OIDC HTTP client a 10s total / 5s connect timeout (reqwest has none by default). A hung IdP connection otherwise outlives the refresher's 30s per-session lock — a second worker re-runs the grant with the same one-time-use refresh token, the IdP burns it → false logout — and holds a semaphore permit forever, wedging the whole refresher. H2: store_idp_refresh and bump_refresh_failures are now atomic Lua guarded on the session still existing. A revoke landing during the IdP round trip could otherwise HSET the deleted session key back into existence as a TTL-less hash holding the freshly-rotated, live IdP refresh token — a permanent, janitor-invisible secret for a logged-out user. store returns false on the revoked-mid-flight race; the caller unschedules. M3: the IdP rotates the grant before we store it, so a store failure loses the new token and the next attempt re-sends the spent one → false logout. Retry the store (3x, 200ms) before giving up. M4: the janitor no longer blind-ZREMRANGEBYSCOREs overdue refresh-due entries — it removes an entry only when its session hash is actually gone. Blind purge would silently kill IdP refresh for live-but-behind sessions after a Redis restore or while the refresher is disabled/wedged. M1: back-channel logout releases the (iss,jti) replay guard if the revoke that followed the claim fails, so the IdP's retry actually revokes instead of getting an idempotent 200 with nothing done. L2: try_lead is now a single compare-and-pexpire Lua (no GET-then-PEXPIRE race that briefly allowed two leaders). L3: next_due_at floored at now + margin/2 so an IdP with ≤margin access-token lifetimes can't be refreshed every tick. EPIC: constructorfabric#1583 (step 10, constructorfabric#1593) Signed-off-by: Anton Zelenov <[email protected]>
…gin-state cap (step 10.6) The gateway's per-IP limit_req zone stays the coarse flood guard (G8); this is the precise, multi-replica-correct layer in Redis (DESIGN 4.4 / §9.3): - an atomic Lua token bucket keyed by what identifies the caller — the STABLE session id on /auth/refresh (rotation doesn't reset it; default 5-burst, 6/min) and the OIDC state on /auth/callback (default 5-burst, 10/min). Never IP: corporate NAT makes per-IP keys wrong at this layer. - a global live login-state cap on /auth/login (default 1000): pre-auth there is no per-caller key, so the guarded resource is the store itself. A new asm:login_state_live ZSET (score = expiry, maintained in the put/take pipelines, trimmed by the janitor) counts live entries and excess logins get 429 before any state is written — the slow-trickle Redis-exhaustion attack the edge cannot see. Everything is tunable under rate_limit.* (burst 0 disables a bucket). Trips answer 429 problem+json with a quota violation + retry hint. The limiter fails OPEN on a Redis error (the coarse layer still guards; a Redis blip must not become a 429 storm) — auth itself keeps failing closed. e2e: one session's refresh bucket trips past the burst while a second session is unaffected; hammering one bogus callback state flips 400 → 429. All eight e2e loops pass locally. EPIC: constructorfabric#1583 (step 10, constructorfabric#1593) Signed-off-by: Anton Zelenov <[email protected]>
…p 10.8) New audit emitter (DESIGN 3.2 'Audit Emitter', PRD nfr-auth-audit): every auth-relevant action publishes to insight.audit.events with the platform envelope — schema tag insight.audit.event.v1, event_id (UUIDv7), RFC3339-ms timestamp, correlation_id (the gateway's X-Correlation-Id when the request carried one, else the event id), tenant_id, actor person/ip/user-agent, service=authenticator, category=auth, action, outcome, resource, details — field names mirroring the Audit Service's ClickHouse events schema. Covered actions: login success/failure (unknown person), session_refresh, logout, session_revoke (single / all / admin-by-user with the acting subject), back_channel_logout (with the sub-only-fallback marker), idp_refresh_invalid_grant kills, and service_token_issued. Publishing never touches auth latency or availability: emit() drops the event into a bounded channel; a background task owns the rdkafka FutureProducer (Kafka-compatible rdkafka API only — the backend PRD's Redpanda-to-Kafka migration constraint; vendored librdkafka via cmake-build, cmake added to the builder image). Queue-full or delivery failure drops the event and bumps auth_audit_dropped_total; with no audit.brokers configured the emitter is disabled and events remain in the structured log (the existing target:audit lines stay as the operator-facing trace). Wiring: audit.brokers/audit.topic config (empty = disabled), compose sets redpanda:9092, the subchart gains audit.* values, and the umbrella folds the global redpanda.brokers into the authenticator config Secret. EPIC: constructorfabric#1583 (step 10, constructorfabric#1593) Signed-off-by: Anton Zelenov <[email protected]>
…review: L4, L5) L4: SHA-256-hash the token-bucket key component before use, so an attacker- chosen key (the OIDC state on /auth/callback, created before state validation) is bounded to a fixed-width digest — it can't inflate Redis with long keys or smuggle control chars. L5: clamp the bucket's stored timestamp forward (ts = max(ts, now)) so a clock step-back (NTP correction) or multi-pod skew can't re-add the skipped window and over-refill. EPIC: constructorfabric#1583 (step 10, constructorfabric#1593) Signed-off-by: Anton Zelenov <[email protected]>
|
Warning Review limit reached
Next review available in: 10 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (32)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
1 participant
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.
Step 10 (auth surface completion) of the nginx+auth EPIC, PR 3 of 4. Delivers items 10.6 (layer-2 rate limiting) and 10.8 (audit events to Redpanda).
Stacked on #1848 → PR 2. Review from the ratelimit commit onward, or diff against
feat/auth-step10-idp-lifecycle.What's here
/auth/refresh(rotation doesn't reset it) and the OIDCstateon/auth/callback; never IP (corporate NAT). Global live login-state cap on/auth/login(asm:login_state_liveZSET) → 429 before any state is written. Trips → 429 problem+json with a retry hint; the limiter fails open on a Redis error (the gateway's coarse per-IP zone still guards) while auth itself stays fail-closed.invalid_grantkill, service-token issuance) publishes toinsight.audit.events(Redpanda) with the platform envelope (schema/event_id/timestamp/correlation_id/tenant/actor/service/action/outcome/resource/details). Non-blocking: a bounded channel feeds an rdkafkaFutureProducer(Kafka-compatible API only — the backend PRD's migration constraint; librdkafka via cmake-build, cmake added to the image); drops are counted (auth_audit_dropped_total), never coupled to auth availability; emptyaudit.brokers= disabled (log only).Review-fix commit
Applies L4 (SHA-256-hash the token-bucket key so an attacker-chosen
statecan't inflate Redis or smuggle control chars) and L5 (clamp the bucket timestamp forward so a clock step-back can't over-refill) from the review.Testing
Unit tests + clippy clean; full 8-loop e2e green (rate-limit loop asserts the refresh bucket trips per-session and the callback bucket trips per-state; audit emitter runs in disabled mode in the harness).
EPIC: #1583 · #1593
🤖 Generated with Claude Code