feat(authenticator): back-channel logout + IdP refresher + janitor (nginx+auth step 10, PR 2/4) - #1849
Closed
cyberantonz wants to merge 7 commits into
Closed
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 <antonz@constructor.tech>
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 <antonz@constructor.tech>
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 <antonz@constructor.tech>
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 <antonz@constructor.tech>
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 <antonz@constructor.tech>
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 <antonz@constructor.tech>
…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 <antonz@constructor.tech>
|
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 (26)
✨ 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 |
aleksdotbar
approved these changes
Jul 22, 2026
Contributor
Author
|
Superseded — the four stacked step-10 PRs are collapsed into a single PR (#1851). Same commits, one review, one merge. Closing this subset. |
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
2 participants
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 2 of 4. Delivers items 10.3 (back-channel logout), 10.4 (IdP background refresher), 10.7 (janitor).
Stacked on #1848 (session surface). Until #1848 merges this diff includes its commits — review from
fix(authenticator): rotation CAS…onward, or diff againstfeat/auth-step10-session-surface.What's here
POST /auth/oidc/back-channel-logout: validates thelogout_tokenper OIDC BCL 1.0 (signature via the IdP JWKS fetched per call,iss/aud,iatskew+max-age window,eventsmember,sub-or-sid, nononce); replay-guards(iss, jti)withSET NX EX; resolves targets by(iss, sid)via the sid index or — the documented sub-only fallback — via a newasm:sub_index:{iss}:{sub}(maintained at login; Identity can't resolve a baresub, and the logout path must not depend on another service), revoking everything for that user with the operator-facing warn line. e2e via fakeidp/_control/backchannel.asm:idp_refresh_due, per-session refresh tasks behindSemaphore(idp.refresh_concurrency)under a per-session rotation lock. Success → store rotated token + reschedule with write-time jitter;invalid_grant→ revoke the owning session (peers die on their own grants); transient → exponential backoff, never revoke.no_refresh_token_policy=strictcaps the session at the IdP token lifetime. Metrics:idp_refresh_total{result}, consecutive-failure gauge,invalid_grantcounter. e2e via fakeidp/_control/outage+/_control/revoke.SCAN asm:user_sessions:*+ZREMRANGEBYSCORE, plus refresh-schedule orphan trim; metricsauth_janitor_removed_total/auth_janitor_backlog_size.Review-fix commit
Applies the security + QA review must-fixes for this branch: H1 (OIDC HTTP client timeout — a hung IdP connection otherwise outlives the 30 s per-session lock and burns the one-time refresh grant → false logout, and wedges the refresher's semaphore), H2 (existence-guarded Lua for
store_idp_refresh/bump— a revoke mid-grant otherwise resurrects a TTL-less session hash holding a live refresh token), M1 (release the back-channel replay guard if the revoke fails, so the IdP retry actually revokes), M3 (retry the post-grant store before giving up — the grant is already spent), M4 (janitor trims a refresh-schedule orphan only when its session hash is gone), L2 (try_leadis one compare-and-pexpire Lua), L3 (next_duefloor).Testing
Unit tests + clippy clean; full 8-loop e2e green (refresher loop exercises outage-survival and the
invalid_grantkill).EPIC: #1583 · #1593
🤖 Generated with Claude Code