feat(authenticator): nginx+auth step 10 — full auth surface completion (all 9 items)

[cyberantonz]

Completes step 10 (auth surface completion, #1593) of the nginx+auth EPIC — all 9 items of NGINX_BFF_10.md in the authenticator service. Single PR (supersedes the earlier stacked #1848/#1849/#1850/#1851, now closed): the items all touch the same files, so they're one cohesive change with one commit per item.

Pairs with insight-front#212 (SPA sends X-CSRF-Token).

Items

1. POST /auth/refresh — rotation with grace (G10). New CSPRNG token mapping; old mapping demoted to refresh_grace_ms (250 ms) — the expiring old mapping is the grace window (no swap keys). expires_at = min(now + session_ttl, absolute cap) across record, key TTL, and per-user ZSET score; stable session_id + linked JWT untouched. Stale-in-grace → same session, no re-rotation; past grace/cap → 401 + clear cookie. Response {expires_at, refresh_at}, refresh_at = expires_at − 90 s ± 60 s (G8), shared with /auth/me.
2. Session management. GET /auth/sessions; DELETE /auth/sessions/{id} (no-existence-oracle 404); DELETE /auth/sessions; admin DELETE /auth/admin/users/{person_id}/sessions (.authenticated(), role-gated by admin_revoke_roles, via the SDK). Every revoke = tokens + session + linked JWT + indexes in one pipeline.
3. OIDC back-channel logout. Validates the logout_token (IdP JWKS, iss/aud, iat window, events, sub-or-sid, no nonce); (iss,jti) replay guard; resolves by sid index or the sub-only fallback via the new asm:sub_index.
4. IdP background refresher (G5). Leader-elected; per-session lock; invalid_grant → revoke owning session, transient → backoff never revoke; no_refresh_token_policy. Metrics: idp_refresh_total{result}, consecutive-failure gauge, invalid_grant counter.
5. CSRF. Middleware on state-changing /auth/*: constant-time X-CSRF-Token vs the session record, Origin-allowlist fallback (csrf_origins, empty = fail closed), back-channel exempt. GET /auth/csrf; /auth/me echoes it.
6. Rate limit, layer 2. Redis Lua token buckets keyed by session (/auth/refresh) / OIDC state (/auth/callback), plus a global live login-state cap; fails open on a Redis error (auth stays fail-closed).
7. Janitor. Leader-elected trim of expired ZSET index members + refresh-schedule orphans; metrics.
8. Audit events. Every auth action → insight.audit.events (Redpanda) with the platform envelope, via a non-blocking bounded channel + rdkafka; empty brokers = disabled (log only).
9. Key-rotation runbook + PRD/DESIGN step-10 updates (cfs green).

Security + QA review fixes (folded into the relevant commits)

Two independent reviews (security: SHIP; QA: BLOCK-until-fixed) → all must-fixes applied: H1 OIDC HTTP timeout (hung IdP conn otherwise outlives the per-session lock → burns the one-time grant → false logout + wedges the refresher), H2 existence-guarded Lua for store_idp_refresh/bump (revoke-mid-grant otherwise resurrects a TTL-less hash holding a live refresh token), M1 back-channel releases the replay guard on revoke failure, M2 rotation compare-and-swap (no orphan credential from multi-tab double-rotate), M3 post-grant store retry, M4 janitor trims a refresh-due orphan only if its session is gone, M5 CSRF fail-closed deploy note, L2 try_lead CAS, L3 next_due floor, L4 hashed rate-limit keys, L5 clock-backstep clamp.

Testing

Unit tests + clippy clean; services/authenticator/tests/run-e2e.sh runs 8 e2e loops (login, refresh, sessions, back-channel, rate-limit, refresher w/ fast lifecycle, service-token) against fakeidp + Redis — all green on the final tree.

Deploy / follow-up notes

• ⚠️ CSRF is fail-closed. Roll insight-front#212 (sends the header) first, or set csrfOrigins, or logout/refresh 403 during the transition (documented in the chart values).
• Known FE gap → feat(auth): SPA-side /auth/refresh driver — session renewal timer (nginx+auth #1583) #1854: the SPA doesn't yet drive /auth/refresh (no timer). Until that lands, sessions hard-expire at the 10-min TTL. Backend side is done here; the FE driver is tracked separately.

EPIC: #1583 · closes #1593

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added session management to list, revoke individual, or revoke all active sessions.
  • Added CSRF protection and a dedicated endpoint for retrieving CSRF tokens.
  • Added OIDC back-channel logout support across sessions.
  • Added automatic identity-provider token refresh with retry handling.
  • Added authentication audit event publishing and configurable rate limiting.

• Security

  • Enabled authenticated administrative endpoints with OIDC JWT verification.
  • Added service-token and session-revocation authorization controls.

• Documentation

  • Added key-rotation guidance and updated authenticator design and requirements documentation.

[coderabbitai]

📝 Walkthrough

Walkthrough

The authenticator gains CSRF-protected session APIs, refresh rotation, back-channel logout, IdP token refreshing, Redis rate limiting and maintenance, audit publishing, authenticated gateway configuration, deployment updates, and corresponding documentation and end-to-end tests.

Changes

Authenticator auth-surface completion

Layer / File(s)	Summary
Configuration and platform wiring .github/workflows/ci.yml, src/backend/services/authenticator/{Cargo.toml,Dockerfile}, deploy/compose/*, docker-compose.yml, src/backend/services/authenticator/helm/*, src/backend/services/authenticator/src/{config.rs,gear.rs,main.rs,api/mod.rs}	Build tooling, OIDC gateway verification, audit and CSRF settings, TLS mounts, shared application state, and background-worker startup are added or updated.
Session storage, CSRF, and rate limiting src/backend/services/authenticator/src/{session.rs,ratelimit.rs,csrf.rs}, src/backend/services/authenticator/src/api/error.rs, src/backend/services/authenticator/src/api/mod.rs	Redis indexes, token buckets, CSRF middleware, canonical session errors, and protected route wiring are introduced.
Browser login, refresh, and session APIs src/backend/services/authenticator/src/api/handlers.rs, src/backend/services/authenticator/tests/e2e_login_loop.rs	Login and callback limits, client attribution, CSRF issuance, refresh rotation with grace, session listing/revocation, and admin revoke handlers are implemented.
Back-channel logout validation src/backend/services/authenticator/src/{backchannel.rs,session.rs}, src/backend/services/authenticator/src/api/handlers.rs, src/backend/services/authenticator/tests/e2e_backchannel.rs	Logout tokens are validated against IdP JWKS, replay guarded, resolved through session indexes, and applied through session revocation.
IdP refresher and index janitor src/backend/services/authenticator/src/{oidc.rs,refresher.rs,janitor.rs,session.rs}, src/backend/services/authenticator/tests/{e2e_refresher.rs,run-e2e.sh}	Leader-coordinated refresh and janitor workers process scheduled sessions, distinguish transient failures from invalid grants, and clean Redis indexes.
Audit event delivery src/backend/services/authenticator/src/{audit.rs,service_token.rs}, src/backend/services/authenticator/src/api/handlers.rs, src/backend/services/authenticator/src/csrf.rs	Authentication events are enveloped and published through a bounded best-effort Kafka producer with retention setup and drop metrics.
Documentation and end-to-end validation docs/components/backend/authenticator/*, src/backend/services/authenticator/tests/*	Design, contract, key-rotation, service-token, refresh, session, logout, rate-limit, and refresher documentation and test scenarios are updated.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related issues

• PLATFORM 1: EPIC: nginx + authorization — replace the API Gateway with nginx auth_request + authenticator gear #1583 — Covers the broader authenticator auth-surface work implemented by this change.

Possibly related PRs

• constructorfabric/insight#1686 — Earlier authenticator session and HTTP plumbing that this change extends.
• constructorfabric/insight#1751 — Introduces the service-token issuance flow that now emits audit events in the same module.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: completing authenticator step 10 with the full auth surface.
Linked Issues check	✅ Passed	The reviewed changes cover the listed step-10 requirements: refresh, sessions, back-channel logout, CSRF, rate limiting, janitor, audit, and runbook/docs.
Out of Scope Changes check	✅ Passed	No clear unrelated changes stand out; the added docs, tests, and build/config updates support the step-10 authenticator work.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (7)

src/backend/services/authenticator/src/refresher.rs (2)

107-163: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicated leader-elected tick-loop boilerplate across the two new background workers. Both refresher.rs::run and janitor.rs::run independently implement the same cancel/tick-select → lease-sizing → try_lead → skip-if-not-leader shape; the only difference is the per-tick work performed once leadership is held.

• src/backend/services/authenticator/src/refresher.rs#L107-L163: extract the cancel/tick/lease/try_lead scaffolding into a shared helper (e.g., leader_elected_loop(state, cancel, tick, leader_key, work_fn)) and pass the due-session draining logic as the work closure.
• src/backend/services/authenticator/src/janitor.rs#L28-L83: reuse the same shared helper, passing the janitor_pass invocation as its work closure.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/services/authenticator/src/refresher.rs` around lines 107 - 163,
Extract the duplicated cancellation, ticking, lease sizing, and try_lead control
flow from refresher.rs lines 107-163 into a shared leader_elected_loop helper,
passing the refresher due-session draining logic as its work closure; update
janitor.rs lines 28-83 to reuse the same helper with janitor_pass as its work
closure, preserving each worker’s existing per-tick behavior.

---

121-161: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Permit acquisition inside the batch loop can stall cancellation and lease renewal.

acquire_owned().await (line 152) blocks the leader's main loop before it returns to tokio::select! (cancellation check) or re-runs try_lead (lease renewal). With a large due-batch (up to BATCH_LIMIT = 512) and modest refresh_concurrency, this loop can run well past the lease TTL (tick * 3), delaying shutdown responsiveness and letting the lease lapse mid-batch (a second instance can then start draining the same batch — harmless here only because lock_session_refresh de-dupes per session, but it's wasted leader-election churn).

♻️ Spawn tasks without blocking on the permit in the leader loop

         for session_id in due {
-            let Ok(permit) = semaphore.clone().acquire_owned().await else {
-                return; // semaphore closed — only on shutdown
-            };
+            let semaphore = semaphore.clone();
             let state = state.clone();
             let metrics = metrics.clone();
             tokio::spawn(async move {
-                let _permit = permit;
+                let Ok(_permit) = semaphore.acquire_owned().await else {
+                    return;
+                };
                 refresh_one(&state, &metrics, &session_id).await;
             });
         }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/services/authenticator/src/refresher.rs` around lines 121 - 161,
Update the batch spawning flow around the leader loop and refresh_one so permit
acquisition does not await inside the leader task. Spawn each refresh task
immediately, acquire the semaphore within that task before calling refresh_one,
and preserve shutdown handling and permit lifetime there so the leader loop can
promptly return to cancellation checks and lease renewal.

src/backend/services/authenticator/src/oidc.rs (2)

296-340: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider consolidating repeated discovery-document fetching.

idp_jwks() (lines 315-330) fetches /.well-known/openid-configuration itself to read jwks_uri, duplicating the same endpoint fetch already done in metadata() and in rp_logout_url() below (each with its own local Disco struct). Also, neither the discovery GET nor the JWKS GET call .error_for_status() before .json(), so a non-2xx response surfaces as an opaque decode error instead of a clear HTTP-status error.

♻️ Add explicit status checks

         let disco: Disco = self
             .http
             .get(format!(
                 "{}/.well-known/openid-configuration",
                 self.issuer_url
             ))
             .send()
             .await
             .context("fetch IdP discovery")?
+            .error_for_status()
+            .context("IdP discovery returned an error status")?
             .json()
             .await
             .context("decode IdP discovery")?;
         self.http
             .get(&disco.jwks_uri)
             .send()
             .await
             .context("fetch IdP JWKS")?
+            .error_for_status()
+            .context("IdP JWKS endpoint returned an error status")?
             .json()
             .await
             .context("decode IdP JWKS")

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/services/authenticator/src/oidc.rs` around lines 296 - 340,
Consolidate discovery-document retrieval into a shared helper used by
metadata(), rp_logout_url(), and idp_jwks(), removing their repeated local Disco
definitions and endpoint requests. In that helper, call error_for_status() after
the discovery GET before decoding JSON. Also apply error_for_status() to the
JWKS GET in idp_jwks() so non-2xx responses surface as HTTP errors.

---

250-262: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Cache discovery metadata for the background refresh path.

refresh_grant fetches OIDC discovery on every call, and refresher.rs invokes it for each due refresh under a worker semaphore. Add a short TTL/metadata-cache with periodic refresh, or rely on cache-aware HTTP client behavior for discovery metadata to reduce IdP discovery traffic without keeping metadata cached indefinitely.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/services/authenticator/src/oidc.rs` around lines 250 - 262,
Update refresh_grant and its metadata retrieval path to reuse OIDC discovery
metadata through a short-lived TTL cache or cache-aware HTTP behavior,
refreshing it periodically rather than fetching on every refresh. Ensure
concurrent background refreshes share the cached metadata and retain the
existing transient error handling when discovery fails.

src/backend/services/authenticator/tests/e2e_refresh.rs (1)

22-97: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract shared e2e test helpers into a common module. Five new e2e test files each reimplement identical env/client/rewrite_host/cookie_from/login (and, in three files, get_csrf) helpers — one root cause: no shared test-support module for this suite.

• src/backend/services/authenticator/tests/e2e_refresh.rs#L22-L97: move env, client, rewrite_host, cookie_from, login, get_csrf into a shared tests/common/mod.rs (or #[path]-included file) and import from there.
• src/backend/services/authenticator/tests/e2e_backchannel.rs#L21-L79: replace the local copies of env, client, rewrite_host, cookie_from, login with the shared module.
• src/backend/services/authenticator/tests/e2e_ratelimit.rs#L19-L92: replace the local copies of env, client, rewrite_host, cookie_from, login, get_csrf with the shared module.
• src/backend/services/authenticator/tests/e2e_refresher.rs#L23-L81: replace the local copies of env, client, rewrite_host, cookie_from, login with the shared module.
• src/backend/services/authenticator/tests/e2e_sessions.rs#L22-L103: replace the local copies of env, client, rewrite_host, cookie_from, get_csrf with the shared module, keeping this file's extra USER_AGENT header as a parameter/override on the shared login.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/services/authenticator/tests/e2e_refresh.rs` around lines 22 -
97, Create a shared tests/common/mod.rs containing env, client, rewrite_host,
cookie_from, login, and get_csrf, then import and use those helpers instead of
local copies. Update
src/backend/services/authenticator/tests/e2e_refresh.rs#L22-97,
e2e_backchannel.rs#L21-79, e2e_ratelimit.rs#L19-92, and e2e_refresher.rs#L23-81
accordingly; update e2e_sessions.rs#L22-103 to share all applicable helpers
while preserving its extra USER_AGENT behavior through a login parameter or
override.

src/backend/services/authenticator/src/gear.rs (1)

142-143: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Stale comment now contradicts the code immediately below it.

This comment says the refresher/janitor "land in step 10 and will spawn here the same way" — but this very PR (step 10) already spawns both a few lines down. Update or remove it to avoid confusing future readers.

✏️ Proposed comment update

-        // `start` must return promptly — the host awaits it before starting the
-        // next gear (including the api-gateway HTTP server). We bind the
-        // service-token listener here (surfacing a bad bind at boot) and spawn
-        // its server, holding `cancel` for graceful shutdown. The IdP refresher
-        // (G5) and janitor land in step 10 and will spawn here the same way.
+        // `start` must return promptly — the host awaits it before starting the
+        // next gear (including the api-gateway HTTP server). We bind the
+        // service-token listener here (surfacing a bad bind at boot), then spawn
+        // it plus the IdP refresher (G5) and index janitor, holding `cancel` for
+        // graceful shutdown of all three.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/services/authenticator/src/gear.rs` around lines 142 - 143,
Update the stale comment immediately before the step-10 spawning logic to remove
the claim that the IdP refresher and janitor will spawn there in the future,
since they already do so below. Keep only wording that accurately describes the
current shutdown and spawning behavior.

src/backend/services/authenticator/src/audit.rs (1)

85-101: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value

Bound the topic-creation admin call with an explicit timeout.

AdminOptions::new() does not set request_timeout, so this best-effort create falls back to the rdkafka socket.timeout.ms configuration instead of documenting a timeout boundary here. Since this is not required for auth availability, tie it to a small constant or the existing configured timeout.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/services/authenticator/src/audit.rs` around lines 85 - 101,
Update ensure_topic_retention to configure an explicit request timeout on the
AdminOptions used by admin.create_topics, using a small constant or the existing
configured timeout rather than relying on socket.timeout.ms. Preserve the
current best-effort behavior and error handling.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/backend/services/authenticator/src/audit.rs`:
- Around line 209-227: Update the serde_json::to_vec failure branch in the audit
delivery task to increment dropped_in_task and emit a tracing warning containing
the serialization error and relevant event context, matching the existing
producer delivery-failure drop path so serialization losses are counted and
visible.

In `@src/backend/services/authenticator/src/refresher.rs`:
- Around line 234-263: Extend the retry handling around store_idp_refresh to use
a longer, backed-off retry window suitable for Redis failovers instead of only
three attempts with fixed 200ms delays. When retries are exhausted in the final
Err branch, increment a dedicated idp_refresh_store_exhausted_total metric in
addition to the existing error log, keeping the Ok(true) and Ok(false) outcomes
unchanged.

---

Nitpick comments:
In `@src/backend/services/authenticator/src/audit.rs`:
- Around line 85-101: Update ensure_topic_retention to configure an explicit
request timeout on the AdminOptions used by admin.create_topics, using a small
constant or the existing configured timeout rather than relying on
socket.timeout.ms. Preserve the current best-effort behavior and error handling.

In `@src/backend/services/authenticator/src/gear.rs`:
- Around line 142-143: Update the stale comment immediately before the step-10
spawning logic to remove the claim that the IdP refresher and janitor will spawn
there in the future, since they already do so below. Keep only wording that
accurately describes the current shutdown and spawning behavior.

In `@src/backend/services/authenticator/src/oidc.rs`:
- Around line 296-340: Consolidate discovery-document retrieval into a shared
helper used by metadata(), rp_logout_url(), and idp_jwks(), removing their
repeated local Disco definitions and endpoint requests. In that helper, call
error_for_status() after the discovery GET before decoding JSON. Also apply
error_for_status() to the JWKS GET in idp_jwks() so non-2xx responses surface as
HTTP errors.
- Around line 250-262: Update refresh_grant and its metadata retrieval path to
reuse OIDC discovery metadata through a short-lived TTL cache or cache-aware
HTTP behavior, refreshing it periodically rather than fetching on every refresh.
Ensure concurrent background refreshes share the cached metadata and retain the
existing transient error handling when discovery fails.

In `@src/backend/services/authenticator/src/refresher.rs`:
- Around line 107-163: Extract the duplicated cancellation, ticking, lease
sizing, and try_lead control flow from refresher.rs lines 107-163 into a shared
leader_elected_loop helper, passing the refresher due-session draining logic as
its work closure; update janitor.rs lines 28-83 to reuse the same helper with
janitor_pass as its work closure, preserving each worker’s existing per-tick
behavior.
- Around line 121-161: Update the batch spawning flow around the leader loop and
refresh_one so permit acquisition does not await inside the leader task. Spawn
each refresh task immediately, acquire the semaphore within that task before
calling refresh_one, and preserve shutdown handling and permit lifetime there so
the leader loop can promptly return to cancellation checks and lease renewal.

In `@src/backend/services/authenticator/tests/e2e_refresh.rs`:
- Around line 22-97: Create a shared tests/common/mod.rs containing env, client,
rewrite_host, cookie_from, login, and get_csrf, then import and use those
helpers instead of local copies. Update
src/backend/services/authenticator/tests/e2e_refresh.rs#L22-97,
e2e_backchannel.rs#L21-79, e2e_ratelimit.rs#L19-92, and e2e_refresher.rs#L23-81
accordingly; update e2e_sessions.rs#L22-103 to share all applicable helpers
while preserving its extra USER_AGENT behavior through a login parameter or
override.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 181b0d9b-89ed-44e6-8c5c-fea377866d98

📥 Commits

Reviewing files that changed from the base of the PR and between 8c2a020 and 18b6d11.

⛔ Files ignored due to path filters (1)

• src/backend/Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (36)

• .github/workflows/ci.yml
• charts/insight/templates/secrets.yaml
• deploy/compose/authenticator-fullauth.yaml
• docker-compose.yml
• docs/components/backend/authenticator/DESIGN.md
• docs/components/backend/authenticator/KEY-ROTATION.md
• docs/components/backend/authenticator/PRD.md
• src/backend/services/authenticator/Cargo.toml
• src/backend/services/authenticator/Dockerfile
• src/backend/services/authenticator/config/insight.yaml
• src/backend/services/authenticator/helm/templates/configmap.yaml
• src/backend/services/authenticator/helm/templates/deployment.yaml
• src/backend/services/authenticator/helm/values.yaml
• src/backend/services/authenticator/src/api/error.rs
• src/backend/services/authenticator/src/api/handlers.rs
• src/backend/services/authenticator/src/api/mod.rs
• src/backend/services/authenticator/src/audit.rs
• src/backend/services/authenticator/src/backchannel.rs
• src/backend/services/authenticator/src/config.rs
• src/backend/services/authenticator/src/csrf.rs
• src/backend/services/authenticator/src/gear.rs
• src/backend/services/authenticator/src/janitor.rs
• src/backend/services/authenticator/src/main.rs
• src/backend/services/authenticator/src/oidc.rs
• src/backend/services/authenticator/src/ratelimit.rs
• src/backend/services/authenticator/src/refresher.rs
• src/backend/services/authenticator/src/service_token.rs
• src/backend/services/authenticator/src/session.rs
• src/backend/services/authenticator/tests/e2e_backchannel.rs
• src/backend/services/authenticator/tests/e2e_login_loop.rs
• src/backend/services/authenticator/tests/e2e_ratelimit.rs
• src/backend/services/authenticator/tests/e2e_refresh.rs
• src/backend/services/authenticator/tests/e2e_refresher.rs
• src/backend/services/authenticator/tests/e2e_service_token.rs
• src/backend/services/authenticator/tests/e2e_sessions.rs
• src/backend/services/authenticator/tests/run-e2e.sh

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Serialization failures silently bypass the dropped-events metric.

Every other drop path here (emit's queue-full, the producer's delivery failure) increments dropped/dropped_in_task and logs a warning. The serde_json::to_vec failure branch does neither, so a malformed event vanishes without a trace and without moving auth_audit_dropped_total — undercounting the very metric meant to surface audit data loss.

🩹 Proposed fix

                 let Ok(payload) = serde_json::to_vec(&env) else {
-                    continue;
+                    dropped_in_task.add(1, &[]);
+                    tracing::warn!(action = env.action, "audit event serialization failed (dropped)");
+                    continue;
                 };

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	tokio::spawn(async move {
	while let Some(event) = rx.recv().await {
	let env = envelope(
	&event,
	uuid::Uuid::now_v7().to_string(),
	chrono::Utc::now().to_rfc3339_opts(chrono::SecondsFormat::Millis, true),
	);
	let Ok(payload) = serde_json::to_vec(&env) else {
	continue;
	};
	// Key by tenant: per-tenant ordering, balanced partitions.
	let record = FutureRecord::to(&topic)
	.key(&env.tenant_id)
	.payload(&payload);
	if let Err((e, _)) = producer.send(record, SEND_TIMEOUT).await {
	dropped_in_task.add(1, &[]);
	tracing::warn!(error = %e, action = env.action, "audit event delivery failed (dropped)");
	}
	}
	tokio::spawn(async move {
	while let Some(event) = rx.recv().await {
	let env = envelope(
	&event,
	uuid::Uuid::now_v7().to_string(),
	chrono::Utc::now().to_rfc3339_opts(chrono::SecondsFormat::Millis, true),
	);
	let Ok(payload) = serde_json::to_vec(&env) else {
	dropped_in_task.add(1, &[]);
	tracing::warn!(action = env.action, "audit event serialization failed (dropped)");
	continue;
	};
	// Key by tenant: per-tenant ordering, balanced partitions.
	let record = FutureRecord::to(&topic)
	.key(&env.tenant_id)
	.payload(&payload);
	if let Err((e, _)) = producer.send(record, SEND_TIMEOUT).await {
	dropped_in_task.add(1, &[]);
	tracing::warn!(error = %e, action = env.action, "audit event delivery failed (dropped)");
	}
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/services/authenticator/src/audit.rs` around lines 209 - 227,
Update the serde_json::to_vec failure branch in the audit delivery task to
increment dropped_in_task and emit a tracing warning containing the
serialization error and relevant event context, matching the existing producer
delivery-failure drop path so serialization losses are counted and visible.

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Store-retry window may be too short to protect against realistic Redis blips.

The comment correctly identifies the false-logout risk if store_idp_refresh fails after the IdP has already rotated the grant, but the mitigation is only ~400ms of retry budget (3 attempts, 200ms apart). A Redis failover/blip commonly lasts longer than that, and on exhaustion there's no dedicated metric — only a log line — to distinguish "rotated token lost, next refresh will falsely log the user out" from an ordinary transient IdP failure tracked by idp_refresh_consecutive_failures.

Consider a longer/backed-off retry budget and a dedicated counter (e.g., idp_refresh_store_exhausted_total) so this specific failure mode is separately alertable before it manifests as user-visible logouts.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/services/authenticator/src/refresher.rs` around lines 234 - 263,
Extend the retry handling around store_idp_refresh to use a longer, backed-off
retry window suitable for Redis failovers instead of only three attempts with
fixed 200ms delays. When retries are exhausted in the final Err branch,
increment a dedicated idp_refresh_store_exhausted_total metric in addition to
the existing error log, keeping the Ok(true) and Ok(false) outcomes unchanged.

[cyberantonz]

The two CodeRabbit findings that landed here after merge (audit.rs serialization-drop metric; refresher.rs store-retry window) are addressed in follow-up PR #1882.