From 8d589f48a1606975efcf187c90498bce0ceef9f1 Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Tue, 28 Jul 2026 23:32:21 +0200 Subject: [PATCH 01/13] docs(bff): make BffRuntime javadoc mode-neutral across server and cookie The BffRuntime and BffRuntimeProducer javadoc still described the runtime as server-mode-only, contradicting BffRuntimeProducer.isBffMode(), which activates the runtime for either recognised session.mode (server or cookie) provided a redirect_uri is configured. Rewrite the eight server-mode-only sites in BffRuntime.java (class javadoc, public constructor, inert(), isActive() and the three inert @throws clauses) to mode-neutral wording naming both modes, and drop the stale server-mode qualifier from BffRuntimeProducer's class javadoc, lazy-discovery paragraph and tokenValidator @param so they agree with its own activation paragraph. The literal token server-mode no longer occurs in either file. Javadoc only: no signature, import or behaviour change. dispatch()'s already mode-neutral @throws is left untouched. D5 hypothesis (re-source the user-info endpoint from the sealed cookie) is recorded as ALREADY-SATISFIED, shipped by PLAN-07A: BffRuntimeProducer.java :215-218/:274-275 builds UserInfoEndpoint over the mode-selected SessionBinding, and BffCookieStatelessnessIT.bothInstancesResolveTheSameIdentity() (:106-123) covers the cookie-mode user-info path end-to-end across two instances. No re-implementation and no new cookie-mode user-info IT were added. Co-Authored-By: Claude --- .../gateway/bff/runtime/BffRuntime.java | 24 +++++++++++-------- .../gateway/quarkus/BffRuntimeProducer.java | 11 +++++---- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/runtime/BffRuntime.java b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/runtime/BffRuntime.java index 5a74494b..371495db 100644 --- a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/runtime/BffRuntime.java +++ b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/runtime/BffRuntime.java @@ -35,12 +35,13 @@ import org.jspecify.annotations.Nullable; /** - * The assembled server-mode BFF runtime (D16 edge wiring) — the single collaborator the gateway edge + * The assembled BFF runtime (D16 edge wiring) — the single collaborator the gateway edge * consults to serve the {@code require: session} stage-4 runtime and to dispatch a matched reserved * OIDC path to its handler. *

* The runtime is built once at boot by {@code BffRuntimeProducer} only when a global - * {@code oidc} block with {@code session.mode=server} is configured; a gateway without such + * {@code oidc} block with a {@code redirect_uri} and a recognised {@code session.mode} — + * {@code server} or {@code cookie} — is configured; a gateway without such * a block gets the {@linkplain #inert() inert} instance, which reports {@link #isActive()} * {@code false} and dispatches nothing (the bearer-only proxy path is unchanged, and the * {@code ReservedPathRegistry} carries no reserved paths to dispatch anyway). @@ -102,8 +103,9 @@ private BffRuntime(boolean active, @Nullable SessionAuthenticationStage sessionS } /** - * Assembles an active server-mode runtime with every reserved-endpoint handler and the session - * stage-4 runtime wired. + * Assembles an active runtime with every reserved-endpoint handler and the session + * stage-4 runtime wired. The same wiring serves both session modes — {@code server} and + * {@code cookie} — which differ only in the {@code SessionBinding} the producer supplies. * * @param sessionStage the {@code require: session} stage-4 runtime * @param csrfDefence the fixed CSRF defence for unsafe-method session requests @@ -132,7 +134,7 @@ public BffRuntime(SessionAuthenticationStage sessionStage, CsrfDefence csrfDefen } /** - * @return the inert runtime — a gateway serving no server-mode BFF variant. It reports + * @return the inert runtime — a gateway serving no BFF variant in either session mode. It reports * {@link #isActive()} {@code false}, exposes no session stage, and dispatches nothing. */ public static BffRuntime inert() { @@ -142,7 +144,7 @@ public static BffRuntime inert() { /** * @return the RFC 9470 step-up coordinator (D7) that handles an upstream * {@code insufficient_user_authentication} challenge for a {@code require: session} route - * @throws IllegalStateException when this runtime is inert (no server-mode BFF variant) + * @throws IllegalStateException when this runtime is inert (no BFF variant is configured) */ public StepUpCoordinator stepUpCoordinator() { if (stepUpCoordinator == null) { @@ -152,8 +154,10 @@ public StepUpCoordinator stepUpCoordinator() { } /** - * @return {@code true} when the gateway serves a server-mode BFF variant (an {@code oidc} block - * with {@code session.mode=server} was configured); {@code false} for a bearer-only gateway + * @return {@code true} when the gateway serves a BFF variant in either session mode (an + * {@code oidc} block with a {@code redirect_uri} and a recognised {@code session.mode} — + * {@code server} or {@code cookie} — was configured); {@code false} for a bearer-only + * gateway */ public boolean isActive() { return active; @@ -162,7 +166,7 @@ public boolean isActive() { /** * @return the {@code require: session} stage-4 runtime the edge wires into the session-aware * {@code AuthenticationStage} - * @throws IllegalStateException when this runtime is inert (no server-mode BFF variant) + * @throws IllegalStateException when this runtime is inert (no BFF variant is configured) */ public SessionAuthenticationStage sessionStage() { if (sessionStage == null) { @@ -174,7 +178,7 @@ public SessionAuthenticationStage sessionStage() { /** * @return the fixed CSRF defence the edge enforces on unsafe-method {@code require: session} * requests - * @throws IllegalStateException when this runtime is inert (no server-mode BFF variant) + * @throws IllegalStateException when this runtime is inert (no BFF variant is configured) */ public CsrfDefence csrfDefence() { if (csrfDefence == null) { diff --git a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/quarkus/BffRuntimeProducer.java b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/quarkus/BffRuntimeProducer.java index 63dfc047..731fa81c 100644 --- a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/quarkus/BffRuntimeProducer.java +++ b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/quarkus/BffRuntimeProducer.java @@ -80,7 +80,7 @@ import jakarta.inject.Singleton; /** - * CDI producer of the server-mode {@link BffRuntime} — the D16 edge wiring that makes the D1–D12 BFF + * CDI producer of the {@link BffRuntime} — the D16 edge wiring that makes the D1–D12 BFF * components reachable at the live gateway edge. *

* The producer builds the active runtime only when a global {@code oidc} block with a @@ -101,9 +101,9 @@ * {@code StepUpHandler#initiate} for RFC 9470 re-drive — so the engine is reached at runtime. *

* Lazy discovery. The OIDC provider metadata is resolved through a memoized supplier - * on first engine use, not at boot: a server-mode gateway therefore boots (and is unit-testable) - * without a live IdP, and the discovery-dependent {@code end_session_endpoint} the logout leg needs is - * materialized only when the first logout arrives. + * on first engine use, not at boot: a BFF gateway in either session mode therefore boots (and is + * unit-testable) without a live IdP, and the discovery-dependent {@code end_session_endpoint} the + * logout leg needs is materialized only when the first logout arrives. * * @author API Sheriff Team * @since 1.0 @@ -127,7 +127,8 @@ public class BffRuntimeProducer { /** * @param gatewayConfig the bound global gateway document carrying the {@code oidc} block * @param tokenValidator a lazy handle to the gateway's shared offline validator, resolved only on - * the active server-mode path (a bearer-only gateway never triggers it) + * the active BFF path in either session mode (a bearer-only gateway never + * triggers it) */ public BffRuntimeProducer(GatewayConfig gatewayConfig, @GatewayValidator Instance tokenValidator) { From 8f348590d0343053f99d55aa549059a9d698e31c Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Tue, 28 Jul 2026 23:55:37 +0200 Subject: [PATCH 02/13] fix(bff): make cookie-mode session identity unique with a per-session nonce MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CookieSessionBinding derived the cookie-mode sessionId from a salted digest over the login instant and sub only, so two logins by the same subject inside one clock second collided. A collision cross-wires TokenRefreshCoordinator's single-flight inFlight keying between two genuinely distinct sessions. Mint a per-session random nonce at login and fold it into the digest: - SealedSessionPayload gains a ninth mandatory component sessionNonce, appended in declaration order and read back in decode(). FIELD_COUNT rises to 9 with a strict guard — no legacy eight-field acceptance path, since admitting one would mean synthesizing a nonce and silently changing the derived identity. - SealedSessionCookieCodec.FORMAT_VERSION rises to 2. The version byte is bound into the GCM associated data, so a v1 cookie fails authentication outright rather than being mis-parsed against the nine-field shape. - SessionRecord gains a tenth component Optional sessionNonce, normalized to empty in the canonical constructor and redacted from toString(). Server mode always leaves it empty; only the cookie binding populates it. - CookieSessionBinding mints the nonce once in bind() (32 secure-random bytes, base64url, matching newSessionId()'s width), re-seals it verbatim in persist(), and folds it into derivedIdentity(). persist() fails loud rather than re-minting when a record carries no nonce. - TokenRefreshCoordinator.rotate() copies the previous record's nonce onto the rotated one; rotate() rebuilds field-by-field, so an uncopied component is silently dropped and a dropped nonce would change the identity mid-session. SessionBinding.persist()'s signature is unchanged and the identity remains a digest — the raw nonce is never used as, or substituted for, the sessionId. Consumer sweep for the SessionRecord canonical-constructor break: a module-wide word-boundary identifier search across all four modules returned 178 hits (api-sheriff main and test; integration-tests and benchmarks empty as expected). Classified: 3 direct new SessionRecord(...) sites, all in InMemorySessionStoreTest and updated to the 10-arg form; 0 destructuring or pattern-match sites; the remainder builder() calls or plain type references, all source-compatible. Test contract updated for the new shape, including regressions that an eight-field payload and a v1-stamped cookie are both rejected, that two logins by one subject in a single second yield distinct identities, that the identity is byte-identical across a re-seal, and that rotate() propagates the nonce. CookieKeyMaterialTest also constructed the payload directly and was updated although the plan did not enumerate it. Both cookie ITs pass unmodified. Co-Authored-By: Claude --- .../bff/cookie/CookieSessionBinding.java | 62 +++++++++++++--- .../bff/cookie/SealedSessionCookieCodec.java | 12 +++- .../bff/cookie/SealedSessionPayload.java | 26 +++++-- .../bff/refresh/TokenRefreshCoordinator.java | 12 ++++ .../gateway/bff/session/SessionRecord.java | 25 +++++-- .../bff/cookie/CookieKeyMaterialTest.java | 2 +- .../bff/cookie/CookieSessionBindingTest.java | 72 +++++++++++++++++-- .../cookie/SealedSessionCookieCodecTest.java | 26 ++++++- .../bff/cookie/SealedSessionPayloadTest.java | 51 +++++++++---- .../refresh/TokenRefreshCoordinatorTest.java | 25 +++++++ .../bff/session/InMemorySessionStoreTest.java | 7 +- 11 files changed, 272 insertions(+), 48 deletions(-) diff --git a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/cookie/CookieSessionBinding.java b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/cookie/CookieSessionBinding.java index cfbad951..f647bf33 100644 --- a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/cookie/CookieSessionBinding.java +++ b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/cookie/CookieSessionBinding.java @@ -18,6 +18,7 @@ import java.nio.charset.StandardCharsets; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; +import java.security.SecureRandom; import java.time.Instant; import java.util.Base64; import java.util.List; @@ -63,8 +64,9 @@ * construction no previous key, so no cookie can carry a retired key id and this path never fires. *

* Session identity. Per the seam's single identity model, the reconstructed - * {@link SessionRecord#sessionId()} is a keyed digest over the sealed payload's login instant and - * {@code sub}. It is stable for the life of the session — so the refresh coordinator's single-flight + * {@link SessionRecord#sessionId()} is a keyed digest over the sealed payload's login instant, + * {@code sub} and the per-session nonce minted once at login. It is stable for the life of the + * session — so the refresh coordinator's single-flight * coalescing behaves exactly as in server mode — and is never emitted to the browser: * only the sealed value crosses. Single-flight is necessarily per instance here; a * cross-instance duplicate refresh cannot be prevented without shared state and is this variant's @@ -80,6 +82,14 @@ public final class CookieSessionBinding implements SessionBinding { private static final String DIGEST_ALGORITHM = "SHA-256"; private static final int IDENTITY_BYTES = 16; + /** + * The per-session nonce width, matched to {@code SessionRecord.newSessionId()}'s 32 bytes so the + * cookie-mode nonce carries the same entropy as the server-mode session id it stands in for. + */ + private static final int SESSION_NONCE_BYTES = 32; + + private static final SecureRandom SECURE_RANDOM = new SecureRandom(); + /** The bounded, non-sensitive disposition recorded when a retired-key session is rolled over. */ private static final String RESEAL_DISPOSITION = "previous-key rollover"; @@ -110,8 +120,10 @@ public CookieSessionBinding(SealedSessionCookieCodec codec, byte[] identitySalt) public BoundSession bind(SessionRecord session, Instant now) { Objects.requireNonNull(session, "session"); Objects.requireNonNull(now, "now"); - // A fresh login anchors the absolute lifetime at this instant. - return seal(payloadOf(session, now), now); + // A fresh login anchors the absolute lifetime at this instant and mints the session's one + // nonce. Minting here — once, on the login path only — is what makes two logins by the same + // subject within a single clock second resolve to distinct identities. + return seal(payloadOf(session, now, newSessionNonce()), now); } @Override @@ -131,6 +143,11 @@ public Optional resolve(@Nullable String cookieHeader, Instant no * the new sealed value. The original login instant is re-derived from the record's absolute * deadline rather than taken from {@code now}, so an endlessly-refreshed session still dies at * its original absolute deadline: a refresh rotates the tokens, never the session's lifetime. + *

+ * The session nonce is re-sealed verbatim and never re-minted, for the same + * reason the login instant is re-derived rather than refreshed: both are identity inputs, so + * minting a new nonce here would change the derived {@link SessionRecord#sessionId()} mid-session + * and break the refresh coordinator's single-flight coalescing. * * @param rotated the session carrying the rotated token material * @param now the reference instant, used only for the cookie's remaining {@code Max-Age} @@ -141,7 +158,13 @@ public BoundSession persist(SessionRecord rotated, Instant now) { Objects.requireNonNull(rotated, "rotated"); Objects.requireNonNull(now, "now"); Instant loginInstant = rotated.expiresAt().minus(codec.sessionTtl()); - return seal(payloadOf(rotated, loginInstant), now); + // Fail loud rather than mint a replacement: a cookie-mode record always carries the nonce + // sealed at login, so an absent one means a caller reconstructed the record and dropped it — + // silently re-minting would change the session's identity mid-flight. + String sessionNonce = rotated.sessionNonce().orElseThrow(() -> new IllegalStateException( + "cookie-mode session record carries no sessionNonce — cannot re-seal without changing " + + "the derived session identity")); + return seal(payloadOf(rotated, loginInstant, sessionNonce), now); } @Override @@ -217,14 +240,26 @@ private SessionRecord markForReseal(SealedSessionCookieCodec.Unsealed unsealed) return toSessionRecord(unsealed.payload()); } - private static SealedSessionPayload payloadOf(SessionRecord session, Instant loginInstant) { + private static SealedSessionPayload payloadOf(SessionRecord session, Instant loginInstant, + String sessionNonce) { return new SealedSessionPayload(session.accessToken(), session.refreshToken(), session.idToken(), - session.sub(), session.sid(), session.acr(), session.authTime(), loginInstant); + session.sub(), session.sid(), session.acr(), session.authTime(), loginInstant, sessionNonce); + } + + /** + * Mints one session nonce: {@link #SESSION_NONCE_BYTES} secure-random bytes, base64url-encoded so + * it survives the payload's base64 field encoding unchanged. + */ + private static String newSessionNonce() { + byte[] bytes = new byte[SESSION_NONCE_BYTES]; + SECURE_RANDOM.nextBytes(bytes); + return Base64.getUrlEncoder().withoutPadding().encodeToString(bytes); } private SessionRecord toSessionRecord(SealedSessionPayload payload) { return SessionRecord.builder() .sessionId(derivedIdentity(payload)) + .sessionNonce(Optional.of(payload.sessionNonce())) .accessToken(payload.accessToken()) .refreshToken(payload.refreshToken()) .idToken(payload.idToken()) @@ -238,9 +273,15 @@ private SessionRecord toSessionRecord(SealedSessionPayload payload) { /** * Derives this binding's stable per-session identity: a salted digest over the sealed payload's - * login instant and {@code sub}. Both inputs are fixed for the life of the session, so the - * identity is stable across re-seals; the salt keeps it un-recomputable outside this gateway; - * and it is never emitted to the browser. + * login instant, {@code sub} and per-session nonce. All three inputs are fixed for the life of + * the session, so the identity is stable across re-seals; the salt keeps it un-recomputable + * outside this gateway; and it is never emitted to the browser. + *

+ * The nonce is what makes the identity unique rather than merely stable: login instant + * and {@code sub} alone collide for two logins by the same subject inside one clock second, which + * would cross-wire the refresh coordinator's single-flight keying between two distinct sessions. + * The identity remains a digest — the raw nonce is never used as, or substituted for, the + * session id. */ private String derivedIdentity(SealedSessionPayload payload) { MessageDigest digest; @@ -253,6 +294,7 @@ private String derivedIdentity(SealedSessionPayload payload) { digest.update(identitySalt); digest.update(Long.toString(payload.loginInstant().getEpochSecond()).getBytes(StandardCharsets.UTF_8)); digest.update(payload.sub().getBytes(StandardCharsets.UTF_8)); + digest.update(payload.sessionNonce().getBytes(StandardCharsets.UTF_8)); byte[] identity = new byte[IDENTITY_BYTES]; System.arraycopy(digest.digest(), 0, identity, 0, IDENTITY_BYTES); return Base64.getUrlEncoder().withoutPadding().encodeToString(identity); diff --git a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionCookieCodec.java b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionCookieCodec.java index 40f4d604..95b8c67c 100644 --- a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionCookieCodec.java +++ b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionCookieCodec.java @@ -94,8 +94,16 @@ public final class SealedSessionCookieCodec { private static final CuiLogger LOGGER = new CuiLogger(SealedSessionCookieCodec.class); - /** The current sealed-cookie format version, bound into the GCM associated data. */ - public static final byte FORMAT_VERSION = 1; + /** + * The current sealed-cookie format version, bound into the GCM associated data. + *

+ * Version {@code 2} carries the nine-field payload that added the per-session nonce keying the + * derived session identity. The bump is a clean break with no migration path: because the + * version byte is bound into the associated data, a version-1 cookie fails GCM authentication + * outright rather than being mis-parsed against the nine-field shape, so pre-existing cookie + * sessions are refused fail-closed and the browser simply re-logs in. + */ + public static final byte FORMAT_VERSION = 2; private static final String TRANSFORMATION = "AES/GCM/NoPadding"; private static final int NONCE_BYTES = 12; diff --git a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionPayload.java b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionPayload.java index b52e0459..3030bd81 100644 --- a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionPayload.java +++ b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionPayload.java @@ -51,15 +51,20 @@ * @param acr the authentication context class, empty when absent * @param authTime the IdP authentication instant, empty when absent * @param loginInstant the absolute login instant anchoring the server-enforced session TTL + * @param sessionNonce the per-session random nonce minted once at login, folded into the derived + * session identity so two logins by the same subject within one clock second + * cannot collide. Re-sealed verbatim — never re-minted — so the identity is + * stable for the life of the session * @author API Sheriff Team * @since 1.0 */ public record SealedSessionPayload(String accessToken, Optional refreshToken, String idToken, -String sub, Optional sid, Optional acr, Optional authTime, Instant loginInstant) { +String sub, Optional sid, Optional acr, Optional authTime, Instant loginInstant, +String sessionNonce) { private static final String REDACTED = "***REDACTED***"; private static final char FIELD_SEPARATOR = '\n'; - private static final int FIELD_COUNT = 8; + private static final int FIELD_COUNT = 9; /** * Canonical constructor rejecting absent mandatory components and normalizing absent optionals @@ -70,6 +75,7 @@ public record SealedSessionPayload(String accessToken, Optional refreshT Objects.requireNonNull(idToken, "idToken"); Objects.requireNonNull(sub, "sub"); Objects.requireNonNull(loginInstant, "loginInstant"); + Objects.requireNonNull(sessionNonce, "sessionNonce"); refreshToken = Objects.requireNonNullElse(refreshToken, Optional.empty()); sid = Objects.requireNonNullElse(sid, Optional.empty()); acr = Objects.requireNonNullElse(acr, Optional.empty()); @@ -91,6 +97,7 @@ public byte[] encode() { appendField(encoded, acr.orElse("")); appendField(encoded, authTime.map(instant -> Long.toString(instant.getEpochSecond())).orElse("")); appendField(encoded, Long.toString(loginInstant.getEpochSecond())); + appendField(encoded, sessionNonce); return encoded.toString().getBytes(StandardCharsets.UTF_8); } @@ -98,6 +105,11 @@ public byte[] encode() { * Reads a payload back from the wire form. The input MUST already have been authenticated by the * codec's GCM tag — this method is not a parser for untrusted input. * + * The field-count guard is strict: only the current nine-field shape is accepted. There is no + * legacy eight-field (pre-nonce) acceptance path — a clean break, so a payload predating the + * session nonce is rejected outright rather than admitted with a synthesized nonce that would + * silently change the derived session identity. + * * @param encoded the UTF-8 bytes produced by {@link #encode()} * @return the decoded payload; empty when the bytes do not carry the expected field shape * (a defensive guard against a key that authenticates a foreign format) @@ -117,7 +129,8 @@ public static Optional decode(byte[] encoded) { optionalField(fields[4]), optionalField(fields[5]), optionalField(fields[6]).map(seconds -> Instant.ofEpochSecond(Long.parseLong(seconds))), - Instant.ofEpochSecond(Long.parseLong(decodeField(fields[7]))))); + Instant.ofEpochSecond(Long.parseLong(decodeField(fields[7]))), + decodeField(fields[8]))); } catch (IllegalArgumentException | DateTimeException _) { // Base64 decode failure, epoch-second parse failure, or an epoch second that parses as a // long but lies outside Instant's supported range, on bytes that authenticated: a @@ -143,7 +156,8 @@ public boolean isExpired(Duration ttl, Instant now) { } /** - * Overridden to redact every credential — the three tokens. The default record + * Overridden to redact every credential — the three tokens — plus the session nonce, which keys + * the derived session identity and so is treated as secret material. The default record * {@code toString()} would otherwise print the raw token material into any log line, exception * message, or debugger view. * @@ -151,9 +165,9 @@ public boolean isExpired(Duration ttl, Instant now) { */ @Override public String toString() { - return "SealedSessionPayload[accessToken=%s, refreshToken=%s, idToken=%s, sub=%s, sid=%s, acr=%s, authTime=%s, loginInstant=%s]" + return "SealedSessionPayload[accessToken=%s, refreshToken=%s, idToken=%s, sub=%s, sid=%s, acr=%s, authTime=%s, loginInstant=%s, sessionNonce=%s]" .formatted(REDACTED, refreshToken.isPresent() ? "Optional[" + REDACTED + "]" : "Optional.empty", - REDACTED, sub, sid, acr, authTime, loginInstant); + REDACTED, sub, sid, acr, authTime, loginInstant, REDACTED); } private static void appendField(StringBuilder target, String value) { diff --git a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/refresh/TokenRefreshCoordinator.java b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/refresh/TokenRefreshCoordinator.java index 2b0ee01b..64ea6567 100644 --- a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/refresh/TokenRefreshCoordinator.java +++ b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/refresh/TokenRefreshCoordinator.java @@ -185,6 +185,17 @@ private boolean nearExpiry(SessionRecord session, Instant now) { return !now.isBefore(expiry.minus(leeway)); } + /** + * Rebuilds the session with the rotated token material, carrying every non-token component over + * from {@code previous} unchanged. + *

+ * Because this reconstructs the record component-by-component, any component NOT copied here is + * silently dropped from the rotated session. That is load-bearing for + * {@link SessionRecord#sessionNonce()}: dropping it would make the cookie-mode binding re-seal + * without a nonce — changing the derived session identity mid-session and breaking the + * single-flight coalescing this coordinator depends on. Add a copy line here for every component + * added to {@link SessionRecord}. + */ private static SessionRecord rotate(SessionRecord previous, RotationResult rotation) { String rotatedIdToken = rotation.idToken(); return SessionRecord.builder() @@ -197,6 +208,7 @@ private static SessionRecord rotate(SessionRecord previous, RotationResult rotat .expiresAt(previous.expiresAt()) .acr(previous.acr()) .authTime(previous.authTime()) + .sessionNonce(previous.sessionNonce()) .build(); } diff --git a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/session/SessionRecord.java b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/session/SessionRecord.java index 1a22c4ca..0ed2fd64 100644 --- a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/session/SessionRecord.java +++ b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/session/SessionRecord.java @@ -44,12 +44,21 @@ * is itself a bearer credential; *

  • cookie mode: the minted id is discarded — it is not among the * fields sealed into the cookie. The binding replaces it with an identity derived - * from the sealed payload (a salted digest over the login instant and {@code sub}), so it is + * from the sealed payload (a salted digest over the login instant, {@code sub} and the + * per-session {@link #sessionNonce()}), so it is * stable across re-seals, un-recomputable outside this gateway, and never emitted to the * browser.
    • * * It is redacted from {@link #toString()} either way. *

    + * The session nonce is cookie-mode-only. {@link #sessionNonce()} is + * {@link Optional#empty()} for every server-mode record — that mode's identity is the minted opaque + * id, which is already unique, so it needs no additional entropy. Only the cookie-mode binding + * populates it, minting it once at login and re-sealing it verbatim on every refresh; without it, + * two logins by the same subject inside one clock second would derive the same identity and + * cross-wire the refresh coordinator's single-flight keying. It is redacted from + * {@link #toString()} because it keys the derived identity. + *

    * {@link #sid()} and {@link #sub()} back a server-mode store's secondary index for O(1) * back-channel logout destruction. * @@ -62,12 +71,15 @@ * @param expiresAt the absolute session expiry (from login), independent of activity * @param acr the authentication context class, empty when absent * @param authTime the IdP authentication instant, empty when absent + * @param sessionNonce the per-session nonce keying the cookie-mode derived identity; always empty in + * server mode (see the mode split above) * @author API Sheriff Team * @since 1.0 */ @Builder public record SessionRecord(String sessionId, String accessToken, Optional refreshToken, String idToken, -String sub, Optional sid, Instant expiresAt, Optional acr, Optional authTime) { +String sub, Optional sid, Instant expiresAt, Optional acr, Optional authTime, +Optional sessionNonce) { private static final String REDACTED = "***REDACTED***"; private static final SecureRandom SECURE_RANDOM = new SecureRandom(); @@ -87,6 +99,7 @@ public record SessionRecord(String sessionId, String accessToken, Optional binding.persist(nonceless, LOGIN), + "re-minting here would change the derived identity mid-session"); } @Test diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionCookieCodecTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionCookieCodecTest.java index a34149d4..bf75dd3c 100644 --- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionCookieCodecTest.java +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionCookieCodecTest.java @@ -65,6 +65,7 @@ class SealedSessionCookieCodecTest { private static final String REFRESH_TOKEN = "raw-refresh-token-SECRET-material"; private static final String ID_TOKEN = "raw-id-token-SECRET-material"; private static final String SUB = "user-sub-1"; + private static final String SESSION_NONCE = "session-nonce-SECRET-material"; private SecretKey key; private SealedSessionCookieCodec codec; @@ -84,7 +85,7 @@ private static SecretKey aesKey(byte fill) { private static SealedSessionPayload payload() { return new SealedSessionPayload(ACCESS_TOKEN, Optional.of(REFRESH_TOKEN), ID_TOKEN, SUB, Optional.of("idp-sid-9"), Optional.of("urn:acr:silver"), - Optional.of(Instant.parse("2026-07-27T09:59:00Z")), LOGIN); + Optional.of(Instant.parse("2026-07-27T09:59:00Z")), LOGIN, SESSION_NONCE); } private static String flipByteAt(String sealedValue, int index) { @@ -130,6 +131,13 @@ void shouldCarryVersionAndKeyIdHeader() throws Exception { assertEquals(SealedSessionCookieCodec.FORMAT_VERSION, raw[0]); assertEquals(KEY_ID, raw[1]); } + + @Test + @DisplayName("Should stamp format version 2 — the nine-field payload shape") + void shouldStampFormatVersionTwo() { + assertEquals((byte) 2, SealedSessionCookieCodec.FORMAT_VERSION, + "the per-session nonce raised the payload to nine fields, which is a wire-format break"); + } } @Nested @@ -184,6 +192,18 @@ void shouldRejectUnknownVersion() throws Exception { "an unknown format version is refused before any decrypt attempt"); } + @Test + @DisplayName("Should reject a cookie stamped with the retired version 1 format") + void shouldRejectLegacyFormatVersionOne() throws Exception { + String sealed = codec.seal(payload()); + byte[] raw = Base64.getUrlDecoder().decode(sealed); + raw[0] = 1; + + assertTrue(codec.unseal(Base64.getUrlEncoder().withoutPadding().encodeToString(raw)).isEmpty(), + "the version bump to 2 is a clean break: a v1 cookie is refused outright rather than " + + "parsed against the nine-field payload shape"); + } + @Test @DisplayName("Should reject an unknown key id without trying the key it does hold") void shouldRejectUnknownKeyId() throws Exception { @@ -290,7 +310,7 @@ class SizeBudget { void shouldRefuseOversizedPayload() { String huge = "x".repeat(BUDGET * 2); SealedSessionPayload oversized = new SealedSessionPayload(huge, Optional.empty(), ID_TOKEN, SUB, - Optional.empty(), Optional.empty(), Optional.empty(), LOGIN); + Optional.empty(), Optional.empty(), Optional.empty(), LOGIN, SESSION_NONCE); CookieSizeBudgetExceededException thrown = assertThrows(CookieSizeBudgetExceededException.class, () -> codec.seal(oversized)); @@ -396,7 +416,7 @@ void shouldNotLeakTokensIntoTheHeader() throws Exception { void shouldNotLeakKeyMaterialIntoTheOverBudgetMessage() { String huge = "x".repeat(BUDGET * 2); SealedSessionPayload oversized = new SealedSessionPayload(huge, Optional.empty(), ID_TOKEN, SUB, - Optional.empty(), Optional.empty(), Optional.empty(), LOGIN); + Optional.empty(), Optional.empty(), Optional.empty(), LOGIN, SESSION_NONCE); CookieSizeBudgetExceededException thrown = assertThrows(CookieSizeBudgetExceededException.class, () -> codec.seal(oversized)); diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionPayloadTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionPayloadTest.java index 194b3211..b2321a5a 100644 --- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionPayloadTest.java +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionPayloadTest.java @@ -46,16 +46,17 @@ class SealedSessionPayloadTest { private static final String REFRESH_TOKEN = "raw-refresh-token-SECRET-material"; private static final String ID_TOKEN = "raw-id-token-SECRET-material"; private static final String SUB = "user-sub-1"; + private static final String SESSION_NONCE = "session-nonce-SECRET-material"; private static SealedSessionPayload full() { return new SealedSessionPayload(ACCESS_TOKEN, Optional.of(REFRESH_TOKEN), ID_TOKEN, SUB, Optional.of("idp-sid-9"), Optional.of("urn:acr:silver"), - Optional.of(Instant.parse("2026-07-27T09:59:00Z")), LOGIN); + Optional.of(Instant.parse("2026-07-27T09:59:00Z")), LOGIN, SESSION_NONCE); } private static SealedSessionPayload minimal() { return new SealedSessionPayload(ACCESS_TOKEN, Optional.empty(), ID_TOKEN, SUB, - Optional.empty(), Optional.empty(), Optional.empty(), LOGIN); + Optional.empty(), Optional.empty(), Optional.empty(), LOGIN, SESSION_NONCE); } /** @@ -95,13 +96,16 @@ void shouldRoundTripAbsentOptionals() { assertTrue(decoded.sid().isEmpty()); assertTrue(decoded.acr().isEmpty()); assertTrue(decoded.authTime().isEmpty()); + assertEquals(SESSION_NONCE, decoded.sessionNonce(), + "the session nonce is mandatory and survives the round trip verbatim"); } @Test @DisplayName("Should round-trip values containing the field separator and other structural characters") void shouldRoundTripSeparatorBearingValues() { SealedSessionPayload original = new SealedSessionPayload("token\nwith\nnewlines", Optional.of("a=b;c"), - ID_TOKEN, "sub\nwith\nnewline", Optional.of("sid\n1"), Optional.empty(), Optional.empty(), LOGIN); + ID_TOKEN, "sub\nwith\nnewline", Optional.of("sid\n1"), Optional.empty(), Optional.empty(), LOGIN, + "nonce\nwith\nnewline"); assertEquals(Optional.of(original), SealedSessionPayload.decode(original.encode()), "each field is base64-encoded, so no value can collide with the separator"); @@ -117,12 +121,25 @@ void shouldDecodeNothingFromForeignShape() { @Test @DisplayName("Should decode nothing when a field is not valid base64") void shouldDecodeNothingFromMalformedFields() { - byte[] malformed = "~~~\n~~~\n~~~\n~~~\n\n\n\n~~~".getBytes(StandardCharsets.UTF_8); + // Nine fields, so the field-count guard passes and the base64 failure is genuinely what + // this test exercises rather than the shape check short-circuiting ahead of it. + byte[] malformed = "~~~\n~~~\n~~~\n~~~\n\n\n\n~~~\n~~~".getBytes(StandardCharsets.UTF_8); assertTrue(SealedSessionPayload.decode(malformed).isEmpty(), "a base64 failure on authenticated-but-foreign bytes is no session, never an exception"); } + @Test + @DisplayName("Should decode nothing from a legacy eight-field payload — no backward-compatible path") + void shouldDecodeNothingFromLegacyEightFieldPayload() { + byte[] legacy = wireForm(ACCESS_TOKEN, "", ID_TOKEN, SUB, "", "", "", + Long.toString(LOGIN.getEpochSecond())); + + assertTrue(SealedSessionPayload.decode(legacy).isEmpty(), + "the pre-nonce eight-field shape is rejected outright: admitting it would require " + + "synthesizing a nonce and would silently change the derived session identity"); + } + @Test @DisplayName("Should decode nothing when an epoch second parses as a long but exceeds Instant's range") void shouldDecodeNothingFromOutOfRangeEpochSecond() { @@ -131,13 +148,16 @@ void shouldDecodeNothingFromOutOfRangeEpochSecond() { String login = Long.toString(LOGIN.getEpochSecond()); assertTrue(SealedSessionPayload - .decode(wireForm(ACCESS_TOKEN, "", ID_TOKEN, SUB, "", "", "", beyondInstantMax)).isEmpty(), + .decode(wireForm(ACCESS_TOKEN, "", ID_TOKEN, SUB, "", "", "", beyondInstantMax, + SESSION_NONCE)).isEmpty(), "DateTimeException is not an IllegalArgumentException, so an out-of-range login instant " + "must still decode to no session rather than escaping unseal()"); assertTrue(SealedSessionPayload - .decode(wireForm(ACCESS_TOKEN, "", ID_TOKEN, SUB, "", "", "", beyondInstantMin)).isEmpty()); + .decode(wireForm(ACCESS_TOKEN, "", ID_TOKEN, SUB, "", "", "", beyondInstantMin, SESSION_NONCE)) + .isEmpty()); assertTrue(SealedSessionPayload - .decode(wireForm(ACCESS_TOKEN, "", ID_TOKEN, SUB, "", "", beyondInstantMax, login)).isEmpty(), + .decode(wireForm(ACCESS_TOKEN, "", ID_TOKEN, SUB, "", "", beyondInstantMax, login, + SESSION_NONCE)).isEmpty(), "the optional authTime field carries the same overflow risk as the login instant"); } } @@ -163,7 +183,7 @@ void shouldBeExpiredAtAndAfterDeadline() { @DisplayName("Should anchor the deadline on the login instant, so it cannot drift") void shouldAnchorDeadlineOnLoginInstant() { SealedSessionPayload resealed = new SealedSessionPayload("rotated-access", Optional.of("rotated-refresh"), - ID_TOKEN, SUB, Optional.empty(), Optional.empty(), Optional.empty(), LOGIN); + ID_TOKEN, SUB, Optional.empty(), Optional.empty(), Optional.empty(), LOGIN, SESSION_NONCE); assertTrue(resealed.isExpired(TTL, LOGIN.plus(TTL)), "re-sealing rotated material does not move the absolute deadline"); @@ -178,20 +198,23 @@ class Contract { @DisplayName("Should reject an absent mandatory component") void shouldRejectAbsentMandatoryComponents() { assertThrows(NullPointerException.class, () -> new SealedSessionPayload(null, Optional.empty(), ID_TOKEN, - SUB, Optional.empty(), Optional.empty(), Optional.empty(), LOGIN)); + SUB, Optional.empty(), Optional.empty(), Optional.empty(), LOGIN, SESSION_NONCE)); + assertThrows(NullPointerException.class, () -> new SealedSessionPayload(ACCESS_TOKEN, Optional.empty(), + null, SUB, Optional.empty(), Optional.empty(), Optional.empty(), LOGIN, SESSION_NONCE)); assertThrows(NullPointerException.class, () -> new SealedSessionPayload(ACCESS_TOKEN, Optional.empty(), - null, SUB, Optional.empty(), Optional.empty(), Optional.empty(), LOGIN)); + ID_TOKEN, null, Optional.empty(), Optional.empty(), Optional.empty(), LOGIN, SESSION_NONCE)); assertThrows(NullPointerException.class, () -> new SealedSessionPayload(ACCESS_TOKEN, Optional.empty(), - ID_TOKEN, null, Optional.empty(), Optional.empty(), Optional.empty(), LOGIN)); + ID_TOKEN, SUB, Optional.empty(), Optional.empty(), Optional.empty(), null, SESSION_NONCE)); assertThrows(NullPointerException.class, () -> new SealedSessionPayload(ACCESS_TOKEN, Optional.empty(), - ID_TOKEN, SUB, Optional.empty(), Optional.empty(), Optional.empty(), null)); + ID_TOKEN, SUB, Optional.empty(), Optional.empty(), Optional.empty(), LOGIN, null), + "the session nonce is mandatory — it keys the derived session identity"); } @Test @DisplayName("Should normalize a null optional to empty") void shouldNormalizeNullOptionals() { SealedSessionPayload payload = new SealedSessionPayload(ACCESS_TOKEN, null, ID_TOKEN, SUB, - null, null, null, LOGIN); + null, null, null, LOGIN, SESSION_NONCE); assertTrue(payload.refreshToken().isEmpty()); assertTrue(payload.sid().isEmpty()); @@ -212,6 +235,8 @@ void shouldRedactCredentials() { assertFalse(rendered.contains(ACCESS_TOKEN), rendered); assertFalse(rendered.contains(REFRESH_TOKEN), rendered); assertFalse(rendered.contains(ID_TOKEN), rendered); + assertFalse(rendered.contains(SESSION_NONCE), + "the session nonce keys the derived identity and must not reach a log line"); assertTrue(rendered.contains("***REDACTED***"), rendered); } diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/refresh/TokenRefreshCoordinatorTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/refresh/TokenRefreshCoordinatorTest.java index 63d76e69..4d9c6af8 100644 --- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/refresh/TokenRefreshCoordinatorTest.java +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/refresh/TokenRefreshCoordinatorTest.java @@ -367,6 +367,31 @@ void shouldServeTheRotatedMaterialFromTheReSealedCookie() { "the re-seal preserves the original absolute deadline — a refresh never extends the session"); } + @Test + @DisplayName("Should propagate the session nonce from the previous record onto the rotated one") + void shouldPropagateSessionNonceAcrossRotate() { + // rotate() rebuilds the record component-by-component, so an uncopied component is + // silently dropped. Dropping the nonce would make persist() refuse the re-seal outright — + // this asserts the copy directly rather than inferring it from identity stability. + assertTrue(cookieSession.sessionNonce().isPresent(), + "a resolved cookie-mode record always carries the nonce sealed at login"); + TokenRefreshCoordinator coordinator = cookieCoordinator(rt -> rotation()); + + RefreshOutcome outcome = coordinator.refresh(cookieSession, sealedCookieHeader, NOW); + + assertEquals(cookieSession.sessionNonce(), outcome.session().orElseThrow().sessionNonce(), + "the rotated record carries the previous record's nonce verbatim — never a fresh one"); + } + + @Test + @DisplayName("Should leave a server-mode record's session nonce empty") + void shouldLeaveServerModeNonceEmpty() { + SessionRecord serverModeRecord = session(CURRENT_REFRESH); + + assertTrue(serverModeRecord.sessionNonce().isEmpty(), + "the nonce is cookie-mode-only; server mode's minted opaque id is already unique"); + } + @Test @DisplayName("Should keep the derived identity stable across the re-seal, so single-flight keys the same") void shouldKeepTheSingleFlightKeyStable() { diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/session/InMemorySessionStoreTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/session/InMemorySessionStoreTest.java index bb0a27c4..e832603f 100644 --- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/session/InMemorySessionStoreTest.java +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/session/InMemorySessionStoreTest.java @@ -238,18 +238,19 @@ void shouldRedactCredentialsInToString() { @Test @DisplayName("Should normalize absent optionals and reject null mandatory components") void shouldNormalizeAndReject() { - SessionRecord sparse = new SessionRecord("s", "at", null, "it", "sub", null, FUTURE, null, null); + SessionRecord sparse = new SessionRecord("s", "at", null, "it", "sub", null, FUTURE, null, null, null); assertTrue(sparse.refreshToken().isEmpty()); assertTrue(sparse.sid().isEmpty()); assertTrue(sparse.acr().isEmpty()); assertTrue(sparse.authTime().isEmpty()); + assertTrue(sparse.sessionNonce().isEmpty(), "an absent session nonce normalizes to empty"); assertThrows(NullPointerException.class, () -> new SessionRecord(null, "at", Optional.empty(), "it", "sub", Optional.empty(), FUTURE, - Optional.empty(), Optional.empty())); + Optional.empty(), Optional.empty(), Optional.empty())); assertThrows(NullPointerException.class, () -> new SessionRecord("s", "at", Optional.empty(), "it", null, Optional.empty(), FUTURE, - Optional.empty(), Optional.empty())); + Optional.empty(), Optional.empty(), Optional.empty())); } } } From 440c3d0ffa04764ce0ed450517186ab97aa1578f Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Wed, 29 Jul 2026 00:12:46 +0200 Subject: [PATCH 03/13] docs(bff): complete the Variant 3 documentation layers and record ADR-0018/0019 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Variant 3 (cookie-based BFF) had only a design section, while Variant 2 carried all three documentation layers. This lands the missing layers, fixes the design section's factual defects, and records the architectural decisions. Design layer (doc/variants/03-bff-cookie.adoc): - session.encryption_key was described as required. It is not: omitting it selects the generate-on-startup key mode. Rewritten to describe the key as optional, naming what declaring it buys (sharing sessions across replicas, surviving restart) and the cost of omitting it. The YAML sample now carries the optionality inline, matching previous_key's existing annotation. - Documented that the variant is stateless after login but not during it: the pending authorization is per-instance, so a login must complete on the instance that began it. Session affinity is required for the login leg only. - Documented the sealed-cookie format bump and its forced-re-login consequence. Operator layer (doc/user/bff-cookie.adoc, new): client registration, mode selection, the two key-material modes and their trade-off, decrypt-only rotation, the 4 KB cookie budget with remediation guidance, what IdP-driven destruction cannot do here, login-leg affinity, and the upgrade consequence. Contributor layer (doc/development/bff-cookie.adoc, new): where the cookie code lives, the SessionBinding seam and why the reserved endpoints stay mode-neutral, the sealed-cookie wire format with the rule that a format change requires a FORMAT_VERSION bump, the derived identity and its stability-and-uniqueness constraints, and the unit/IT entry points including the two-instance topology. ADRs: - ADR-0018 records the BFF session-mode architecture: reserved-path precedence, the fixed CSRF model, the one SessionBinding seam, the cookie key-material two-mode decision, and the per-session identity nonce. The first four originate in earlier BFF work and are marked retroactive in the context. - ADR-0019 records the two inbound-validation relaxations with their bounding constraints: reserved paths bypass the url-parameter value pipeline (they terminate at the gateway and their handlers validate their own parameters), and the cookie-header length raise is necessarily gateway-wide because inbound validation runs before route selection. Indexes: added the new guides to doc/user/README.adoc, doc/development/ README.adoc and doc/README.adoc's Deployment Variants table. The Architecture Decisions table gained ADR-0018/0019 and, as a Boy-Scout fix, the four rows that were missing entirely (ADR-0010, 0011, 0012, 0017) — it now enumerates all 19 ADRs in the corpus. doc/architecture.adoc references both new ADRs and drops the same stale server-mode-only framing corrected earlier in BffRuntime: the producer activates for either recognised session.mode, and both modes drive the same wiring. Co-Authored-By: Claude --- doc/README.adoc | 43 ++++ ..._a_fixed_reserved-path_and_CSRF_model.adoc | 221 ++++++++++++++++++ ...der_length_relaxation_is_gateway-wide.adoc | 183 +++++++++++++++ doc/architecture.adoc | 46 ++-- doc/development/README.adoc | 7 + doc/development/bff-cookie.adoc | 165 +++++++++++++ doc/user/README.adoc | 7 + doc/user/bff-cookie.adoc | 168 +++++++++++++ doc/variants/03-bff-cookie.adoc | 43 +++- 9 files changed, 865 insertions(+), 18 deletions(-) create mode 100644 doc/adr/0018-BFF_session_mode_is_one_SessionBinding_seam_behind_a_fixed_reserved-path_and_CSRF_model.adoc create mode 100644 doc/adr/0019-Reserved_BFF_paths_bypass_the_url-parameter_value_pipeline_and_the_cookie-header_length_relaxation_is_gateway-wide.adoc create mode 100644 doc/development/bff-cookie.adoc create mode 100644 doc/user/bff-cookie.adoc diff --git a/doc/README.adoc b/doc/README.adoc index e7796e1e..84db177a 100644 --- a/doc/README.adoc +++ b/doc/README.adoc @@ -105,6 +105,11 @@ configuration. Each has its own design section: | Task-oriented companions to the Variant 2 design section above: link:user/bff-session.adoc[operator setup] (configure and run the server-session BFF) and link:development/bff-session.adoc[contributor guide] (build, test, and extend it). + +| Variant 3 -- three-layer guides +| Task-oriented companions to the Variant 3 design section above: + link:user/bff-cookie.adoc[operator setup] (configure and run the cookie-based BFF) and + link:development/bff-cookie.adoc[contributor guide] (build, test, and extend it). |=== == Implementation Plans @@ -170,6 +175,26 @@ implementing it. of alias-resolvability violations. *(Status: accepted.)* +| link:adr/0010-YAML_expansion_limits_enforced_in_a_compose-only_pre-pass_before_bind.adoc[ADR-0010] +| YAML alias-expansion and nesting-bomb caps are enforced in a dedicated SnakeYAML compose-only + pre-pass before Jackson binds a config document, because Jackson's `YAMLParser` consumes the + event stream directly and never runs the Composer that counts collection aliases. + *(Status: proposed.)* + +| link:adr/0011-gatewayyaml_exposes_JWKS_trust_and_egress_as_neutral_names_bound_by_the_deployment.adoc[ADR-0011] +| `gateway.yaml` is API Sheriff's own configuration language, so the JWKS issuer surface exposes + egress and TLS trust as framework-neutral names (`allowed_egress_hosts`, `tls_profile`) that carry + no trust material; exactly one mapping component binds a logical trust-profile name to concrete + runtime trust anchors, and config stays neutral while startup diagnostics name the concrete + runtime key. *(Status: accepted.)* + +| link:adr/0012-Comparative_cross-gateway_benchmarking_runs_on-demand_and_stays_segregated_from_the_CI_baseline.adoc[ADR-0012] +| Cross-gateway comparison benchmarking is a target-neutral, on-demand-only lane whose results stay + strictly segregated from the authoritative CI baseline and its published pages; one target + parameter switches the whole matrix between gateways, and fairness parity (identical upstreams, + auth realm, TLS material and resource limits) is a precondition of any comparison. + *(Status: proposed.)* + | link:adr/0013-anchor-type-access-axes.adoc[ADR-0013] | Anchor type/access axes: two required, orthogonal classification axes on every anchor -- `type` (`proxy`/`bff`/`asset`) and `access` (`public`/`authenticated`) -- validated @@ -194,6 +219,24 @@ implementing it. | gRPC gateway rejections are emitted as trailers-only gRPC responses (a gRPC client cannot consume `application/problem+json`), with a canonical status mapping and h2-negotiation-failure to `UNAVAILABLE`. *(Status: accepted.)* + +| link:adr/0017-Accept-time_SNI_split_at_a_dedicated_front_listener_passthrough_relays_opaquely_at_L4_everything_else_terminates_internally.adoc[ADR-0017] +| A dedicated raw-TCP front listener owns the public TLS port, reassembles the ClientHello, and by + SNI either opaquely L4-relays a passthrough match to its resolved backend or hands the + still-encrypted connection to an internal terminating HTTPS listener that solely owns TLS + termination and mTLS. *(Status: proposed.)* + +| link:adr/0018-BFF_session_mode_is_one_SessionBinding_seam_behind_a_fixed_reserved-path_and_CSRF_model.adoc[ADR-0018] +| Session mode selects only which `SessionBinding` is assembled; reserved paths, CSRF and session + identity are fixed, mode-neutral models behind that one seam -- including the cookie-mode + two-mode key material and the per-session identity nonce whose format bump forces a re-login. + *(Status: proposed.)* + +| link:adr/0019-Reserved_BFF_paths_bypass_the_url-parameter_value_pipeline_and_the_cookie-header_length_relaxation_is_gateway-wide.adoc[ADR-0019] +| Two deliberate inbound-validation relaxations the BFF requires: the url-parameter value pipeline + is not applied to matched reserved paths (they terminate at the gateway and their handlers + validate their own parameters), and the cookie-header value-length raise is necessarily + gateway-wide because inbound validation precedes route selection. *(Status: proposed.)* |=== == Reading Order diff --git a/doc/adr/0018-BFF_session_mode_is_one_SessionBinding_seam_behind_a_fixed_reserved-path_and_CSRF_model.adoc b/doc/adr/0018-BFF_session_mode_is_one_SessionBinding_seam_behind_a_fixed_reserved-path_and_CSRF_model.adoc new file mode 100644 index 00000000..771a7d1d --- /dev/null +++ b/doc/adr/0018-BFF_session_mode_is_one_SessionBinding_seam_behind_a_fixed_reserved-path_and_CSRF_model.adoc @@ -0,0 +1,221 @@ += ADR-0018: BFF session mode is one SessionBinding seam behind a fixed reserved-path and CSRF model +:toc: left +:toclevels: 2 +:sectnums: + +// adr-metadata +// Progressive-disclosure metadata block (see manage-adr SKILL.md → "ADR Template +// Structure"). Read by `manage-adr.py scan` so a caller can assess an ADR's +// relevance without reading the full file. List fields are comma-separated. +// summary: Session mode selects only which SessionBinding is assembled; reserved paths, CSRF and session identity are fixed, mode-neutral models behind that one seam +// tags: bff, session-mode, csrf, reserved-paths, cookie-session, key-material, session-identity +// affects: api-sheriff +// supersedes: +// end-adr-metadata + +== Status + +Proposed + +== Context + +A Backend-For-Frontend gateway that acts as an OIDC confidential client has to answer four +structural questions at once, and each of them admits a tempting per-deployment or per-route answer +that fragments the security model. + +*Gateway-owned paths versus proxied namespaces.* The BFF must serve its own OIDC endpoints — the +auth-code callback, login initiation, logout and its return leg, the back-channel logout receiver, +and a curated user-info view. Those paths live on the same host as proxied traffic. If they are +resolved by the ordinary route table, a proxy prefix can swallow them: a route matching a broad +prefix silently captures the callback, and a login that appears to be misconfigured is in fact +being forwarded upstream. The failure is silent and configuration-dependent, which is the worst +combination for a security-critical path. + +*CSRF as a knob versus a fixed property.* A cookie-authenticated session is inherently exposed to +cross-site request forgery. Making the defence configurable creates a deployment in which it is +off, and the class of gateway most likely to turn it off is the one that least understands the +exposure. + +*Two session modes with one feature surface.* Token custody has two legitimate shapes: hold tokens +server-side and give the browser an opaque id, or seal the tokens into the cookie itself and hold +nothing. These differ in state, in failure modes, and in which capabilities are achievable — but +they do *not* differ in what the login flow, the refresh path, the CSRF defence, or the reserved +endpoints have to do. Implementing them as two parallel stacks would double the surface on which a +security defect can appear, and guarantee the two drift. + +*Key material for the stateless mode.* Sealing requires a key. Demanding one unconditionally makes +the simplest deployment impossible without key management; generating one silently makes a +multi-replica deployment fail in a way that looks like an unrelated session bug. + +*Session identity under a derived-identity scheme.* Any session needs a stable identity to key +per-session coordination — most importantly single-flight coalescing of token refresh, so +concurrent requests for one session do not each drive a refresh. A stateless mode has no store to +mint an id in, so identity must be *derived* from the sealed material. A derived identity creates +a constraint that a minted one does not: the derivation inputs must be simultaneously stable across +re-seals and unique across sessions. Choosing inputs that are merely stable produces collisions +between distinct sessions, which cross-wires precisely the coordination the identity exists to key. + +Decisions (i) through (iv) below originate in the earlier BFF implementation work and are recorded +here *retroactively* — they were made and implemented before this record was written, and this ADR +captures them rather than proposing them. Decision (v) is recorded as it was made. + +== Decision + +*Session mode selects only which `SessionBinding` is assembled. Everything else — reserved-path +resolution, CSRF, and the session-identity contract — is a fixed, mode-neutral model behind that one +seam.* + +=== (i) Reserved paths resolve before the proxy route table + +The gateway carves out its own exact paths on the OIDC host and resolves them *ahead of* the proxy +route table, via a `ReservedPathRegistry` the edge consults first. A matched reserved path is +terminated at the gateway and dispatched to its handler; it can never be captured by a proxy prefix, +whatever the route configuration says. + +The set is closed and each entry is configuration-driven, so operators can move a reserved path out +of a namespace they need — but they cannot make a reserved path proxyable, and no route ordering +can shadow one. + +=== (ii) The CSRF model is fixed, not a knob + +Origin verification on unsafe methods, plus a `SameSite=Lax` cookie, applies to every +`require: session` route as a fixed property. There is no switch to disable it. The only +configuration is the *allowlist* of trusted origins a cross-origin SPA is served from — that is, the +operator declares who may call, never whether the check happens. + +=== (iii) Server and cookie mode sit behind one `SessionBinding` seam + +The whole BFF foundation binds to a `SessionBinding` abstraction, never to a store. The configured +mode selects only which implementation is assembled: a store-backed binding, or a stateless binding +over the sealed-cookie codec. The login flow, CSRF defence, refresh and step-up coordinators, the +reserved endpoints, and the session-authentication runtime are written against the seam and are +unaware of which binding is in force. + +Where the modes genuinely differ, they differ by *declared capability*, not by mode name. A binding +reports whether it supports IdP-driven destruction; the back-channel logout endpoint is gated on +that signal and answers a deliberate `404` when unsupported, rather than claiming a destruction that +did not happen or letting the reserved path fall through to the proxy route table. + +=== (iv) Cookie-mode key material has two modes, and the key is optional + +Cookie mode accepts an explicitly supplied sealing key or, when none is configured, generates one at +startup. Boot does *not* require a configured key — the relaxed rule is deliberate, so the simplest +deployment needs no key management at all. + +The trade-off is made explicit rather than hidden: a generated key cannot be shared with another +replica and does not survive a restart, so sessions are dropped on restart and cannot be served by a +peer. A decrypt-only previous key supports rotation and requires a current key to roll onto; +declaring it alone is rejected at boot, because a rotation key with nothing to roll onto is +meaningless. + +=== (v) Cookie-mode session identity is a keyed digest including a per-session nonce + +The derived cookie-mode identity is a salted digest over the session's fixed material *plus a +per-session random nonce* minted once at login. The nonce is sealed with the session, re-sealed +verbatim on every refresh, and never re-minted; the identity remains a digest, and the raw nonce is +never used as or substituted for the session id. The identity is never emitted to the browser. + +This satisfies both halves of the derived-identity constraint: stable, because every input is fixed +for the session's life, so a re-seal reproduces the identity exactly; and unique, because the nonce +separates two sessions that would otherwise share every other input — notably two logins by the same +subject within one clock granule. + +Adding the nonce changed the sealed payload's field set and the session record's shape. Because the +sealed-cookie format version is bound into the cookie's authenticated data, the version was bumped +and there is deliberately no acceptance path for the previous format: admitting an older cookie +would require synthesizing the missing nonce, which would silently change the derived identity — +exactly the defect the nonce exists to prevent. + +== Consequences + +=== Positive + +* A security-critical path cannot be shadowed by configuration: reserved-path precedence is + structural, not an ordering convention. +* There is no deployment of this gateway with CSRF defence disabled, because there is no way to + express one. +* A capability added to the BFF is written once, against the seam, and is available in both session + modes without a second implementation — so the two modes cannot drift apart in behaviour. +* Mode-specific limitations surface as an explicit capability signal with a defined response, rather + than as a silently missing behaviour. +* The stateless mode is usable with no key management at all, while the multi-replica case remains + fully supported by supplying a key. +* Single-flight refresh coalescing keys correctly in both modes, including for concurrent logins by + one subject. + +=== Negative + +* Reserved paths permanently occupy their configured paths on the OIDC host; an operator who wants + those namespaces proxied must relocate the reserved paths instead. +* A truly cross-site SPA cannot be supported, because the fixed cookie policy will not attach the + session cookie cross-site — such an SPA must be hosted under the gateway's site. +* Any change to the sealed payload's field set forces a format-version bump, which invalidates every + existing cookie-mode session and forces re-login on deploy. There is no rolling migration. +* The session record carries a component that is meaningful in only one mode, and it must be + explicitly propagated wherever the record is rebuilt field-by-field. + +=== Risks + +* *Silent identity drift.* Any code path that reconstructs the session record component-by-component + and omits the nonce would change the derived identity mid-session, breaking refresh coalescing. + The re-seal path fails loud rather than re-minting a nonce, which converts this from a silent + corruption into an immediate error. +* *Operator misreading of the key default.* Omitting the key is easy and works perfectly on one + replica, so a deployment can scale out and only then discover its sessions are not portable. The + trade-off is therefore documented at the point of configuration rather than only in reference + material. +* *Cookie size ceiling.* Sealing tokens into the cookie makes session size a function of token size, + which is bounded by what browsers accept. The gateway refuses to seal an over-budget session + rather than emitting a cookie the browser would silently drop. + +== Alternatives Considered + +*Resolve reserved paths through the ordinary route table.* This would need no separate registry and +no precedence rule, letting one mechanism resolve every path. Rejected because it makes the +correctness of a security-critical endpoint depend on route ordering and prefix breadth: a +legitimate broad proxy prefix silently captures the callback, and the resulting failure is +configuration-dependent and hard to attribute. Precedence must be structural to be trustworthy. + +*Make CSRF defence configurable, with a secure default.* This would accommodate deployments that +believe they do not need it, while defaulting to protection. Rejected because the option's existence +is the vulnerability: a defence that can be switched off will be switched off in the deployment that +understands the exposure least, and a secure default provides no protection against a deliberate +override. + +*Implement the two session modes as independent stacks.* Each mode could then be optimised for its +own shape without abstraction overhead. Rejected because it doubles the surface where a +security-relevant defect can occur and guarantees divergence: a fix or hardening applied to one +stack does not reach the other, and the two slowly acquire different behaviour for what operators +reasonably read as one feature. + +*Require a sealing key unconditionally in cookie mode.* This would remove the weaker generated-key +mode and the possibility of an operator being surprised by non-portable sessions. Rejected because +it imposes key management on every deployment including single-node and development ones, which +pushes operators toward checked-in or trivially-derived keys — a worse security outcome than an +explicitly documented, deliberately-scoped generated key. + +*Derive the cookie-mode identity from the session's fixed material alone, without a nonce.* This +would need no extra sealed field and no format bump. Rejected because stability and uniqueness are +independent requirements and this satisfies only the first: two sessions sharing the same subject +and login granule derive the same identity, which cross-wires the per-session coordination the +identity exists to key. The defect is silent, load-dependent, and appears only under concurrency. + +*Accept the previous sealed-cookie format alongside the new one.* This would avoid forcing every +user to log in again on upgrade. Rejected because accepting an older cookie means supplying the +absent identity input, and any value chosen changes the derived identity for that session — +reintroducing exactly the collision class the nonce removes. A clean break with a forced re-login is +the honest cost. + +The alternatives fall into one class: each trades a structural guarantee for local convenience — +configurability, less abstraction, an easier upgrade — and each pays for it with a failure mode that +is silent and surfaces only in the deployments least equipped to diagnose it. + +== References + +* link:0019-Reserved_BFF_paths_bypass_the_url-parameter_value_pipeline_and_the_cookie-header_length_relaxation_is_gateway-wide.adoc[ADR-0019 -- Reserved-path and cookie-header relaxations] -- the inbound-validation relaxations this architecture requires. +* link:../variants/02-bff-session.adoc[Variant 2 -- BFF, Session-based] -- the server-side-custody realisation. +* link:../variants/03-bff-cookie.adoc[Variant 3 -- BFF, Cookie-based] -- the stateless realisation. +* link:../architecture.adoc[Architecture] -- the seam statement in context. +* link:../configuration.adoc#_oidc[Configuration Reference -- `oidc`] -- the authoritative key contract, including the relaxed cookie-mode key rule. +* link:../user/bff-cookie.adoc[Operator setup -- cookie-based BFF] -- the key-material trade-off and upgrade consequence as operators meet them. +* link:../development/bff-cookie.adoc[Contributor guide -- cookie-based BFF] -- the wire format and identity constraints as implementers meet them. diff --git a/doc/adr/0019-Reserved_BFF_paths_bypass_the_url-parameter_value_pipeline_and_the_cookie-header_length_relaxation_is_gateway-wide.adoc b/doc/adr/0019-Reserved_BFF_paths_bypass_the_url-parameter_value_pipeline_and_the_cookie-header_length_relaxation_is_gateway-wide.adoc new file mode 100644 index 00000000..44825d38 --- /dev/null +++ b/doc/adr/0019-Reserved_BFF_paths_bypass_the_url-parameter_value_pipeline_and_the_cookie-header_length_relaxation_is_gateway-wide.adoc @@ -0,0 +1,183 @@ += ADR-0019: Reserved BFF paths bypass the url-parameter value pipeline and the cookie-header length relaxation is gateway-wide +:toc: left +:toclevels: 2 +:sectnums: + +// adr-metadata +// Progressive-disclosure metadata block (see manage-adr SKILL.md → "ADR Template +// Structure"). Read by `manage-adr.py scan` so a caller can assess an ADR's +// relevance without reading the full file. List fields are comma-separated. +// summary: Two deliberate inbound-validation relaxations the BFF requires — the url-parameter value pipeline is not applied to matched reserved paths, and the cookie-header value-length raise is necessarily gateway-wide +// tags: security, bff, reserved-paths, input-validation, cookie-header, relaxation, blast-radius +// affects: api-sheriff +// supersedes: +// end-adr-metadata + +== Status + +Proposed + +== Context + +The gateway applies an inbound security-validation pipeline to every request before any routing +decision: path canonicalisation, url-parameter validation, and header validation, each fail-closed +and each refusing to echo an offending value. The pipeline is tuned for its primary job — defending +an *upstream* against traffic the gateway forwards. + +The BFF introduces two request shapes that pipeline does not fit, and both create the same kind of +choice point: relax the default policy for a narrow case, or leave a legitimate request permanently +rejected at the edge. + +*Gateway-terminated requests are not forwarded traffic.* The url-parameter value pipeline is an +anti-injection defence for parameters the gateway passes to an upstream — its threat model is a +value that means one thing to the gateway and another to the backend. A reserved BFF path is not +forwarded anywhere: it terminates at the gateway and is consumed by a handler the gateway itself +owns. Worse, some of its parameters legitimately carry structure the pipeline is designed to reject. +A same-origin post-login return target is a path, so it contains separators the anti-injection +policy treats as suspicious in a value destined for an upstream. Applying an upstream-oriented +policy to a request that has no upstream rejects correct traffic while defending nothing. + +*Validation runs before route selection.* The cookie-mode BFF's sealed session cookie is designed to +a multi-kilobyte budget, well past the default header-value length cap, so an authenticated request +in that mode fails the header check every single time. The obvious containment — raise the cap only +for the routes that need it — is not available at this position in the pipeline: the stage runs +*before* any route or anchor is resolved, so at the moment the header must be validated there is no +per-anchor context in existence to scope the relaxation to. This is a structural property of where +inbound validation has to sit (it must precede routing so routing decides on canonicalised, +validated input), not an implementation shortcut that could be tightened later. + +Both cases pose the same question: when a defence does not fit a legitimate request class, is the +correct answer to weaken the defence globally, to carve out the narrowest possible exception, or to +reject the request class? Answering it once, explicitly, and bounding what each relaxation gives up +is what this record exists to do. + +== Decision + +*Both relaxations are granted, each scoped as narrowly as its position in the pipeline permits, and +each leaves every other validator untouched.* + +=== (a) The url-parameter value pipeline is not applied to matched reserved paths + +The inbound stage takes a reserved-path predicate over host and canonical path. When a request +matches a reserved gateway path, the url-parameter *value* pipeline is not applied to it. + +The justification is that a matched reserved path is *terminated at the gateway and never proxied*, +so the threat the value pipeline addresses — a parameter reinterpreted by an upstream — does not +exist for it. Validation of those parameters is not skipped; it *moves to the reserved handler*, +which is the component that actually knows each parameter's grammar. The handler validates a return +target as a same-origin path, a state parameter as an opaque correlator, and so on — checks that are +strictly more precise than a generic value policy, because they are type-aware. + +What still constrains a reserved path is everything else: path canonicalisation still runs and still +yields the single canonical path every later stage consumes, the parameter- and header-*count* caps +still apply, and every other header validator still applies. The relaxation is confined to +url-parameter *values*, on *matched reserved paths only*. A gateway with no BFF configured matches +no reserved path and is byte-for-byte unaffected. + +=== (b) The cookie-header value-length relaxation is gateway-wide + +The stage accepts an optional cookie-header policy whose *only* difference from the default is a +raised maximum header-value length, and applies it to the `Cookie` and `Set-Cookie` header values +only. It is supplied only by a cookie-mode BFF gateway; a bearer-only or server-mode gateway +supplies nothing and is byte-for-byte unaffected. + +The relaxation is gateway-wide rather than per-anchor because, as established above, no anchor is +resolved at this pipeline position. The configuration key that supplies the budget sits on the +global session block for exactly that reason, and its placement must not be read as per-anchor +enforcement — it is global because the enforcement point is global. + +The blast radius is bounded on three independent axes: + +* *By header.* Only `Cookie` and `Set-Cookie` values are affected. Every other header keeps the + default cap. +* *By validator.* Only the length cap changes. The null-byte, control-character, and + injection-pattern validators apply to the cookie header unchanged, so the relaxation admits a + *longer* value, never a *differently-shaped* one. +* *By deployment.* Only a cookie-mode gateway supplies the policy at all. + +The residual exposure is therefore precisely: on a cookie-mode gateway, a client may send a longer +`Cookie` header value than the default policy would accept, subject to every other content check and +to the gateway's own transport limits. A value that is not a resolvable sealed session is refused by +the session binding fail-closed as "no session". + +== Consequences + +=== Positive + +* Legitimate BFF traffic is not rejected at the edge: a login return leg carrying a path-shaped + target, and an authenticated cookie-mode request carrying a multi-kilobyte sealed cookie, both + pass validation. +* Reserved-path parameters end up *better* validated, not less: a type-aware handler check is + stricter than a generic value policy. +* Each relaxation is inert on deployments that do not use the feature — a bearer-only gateway's + inbound validation is unchanged in both respects. +* The residual exposure of each relaxation is explicitly enumerable rather than emergent, which is + what makes it reviewable. + +=== Negative + +* Reserved-path parameter validation is now the reserved handlers' responsibility, so a new reserved + handler that neglects to validate its own parameters has no generic backstop behind it. +* The cookie-header length relaxation cannot be scoped to the routes that need it, so on a + cookie-mode gateway it applies to every request including ones that will never carry a session + cookie. +* Two documented exceptions now exist to what was a uniform inbound policy, which is a cost in + comprehensibility for anyone reasoning about the pipeline. + +=== Risks + +* *A reserved handler forgetting its own validation.* The relaxation moves a check rather than + removing it, so the guarantee holds only while every reserved handler upholds its half. Adding a + reserved endpoint therefore carries an obligation to validate its parameters explicitly. +* *Relaxation creep.* Each carve-out makes the next easier to justify. The bounding described above + — one header pair, one validator, one deployment mode — is the limit, not a starting point; a + future request to relax a *different* validator for the same convenience is a separate decision + requiring its own record. +* *Misreading the configuration key's placement.* The budget key sitting on the global session block + can be misread as per-anchor enforcement; it is global because the enforcement point precedes + routing. + +== Alternatives Considered + +*Apply the url-parameter value pipeline to reserved paths unchanged.* This would keep one uniform +inbound policy with no exceptions to reason about. Rejected because it rejects correct traffic while +protecting nothing: a legitimately path-shaped return target fails a policy whose threat model +(reinterpretation by an upstream) does not apply to a request that is never forwarded. The result +would be a BFF whose login return leg cannot function. + +*Loosen the url-parameter value policy globally so reserved paths pass.* This would need no +predicate and no per-request branch. Rejected because it weakens the defence exactly where it is +load-bearing — on parameters actually forwarded to an upstream — in order to accommodate requests +that are not forwarded at all. It inverts the scoping: the narrow case should be the exception, not +the new global baseline. + +*Raise the header-value cap globally for all headers.* This is the simplest change and needs no +optional policy. Rejected because it discards the cap's protection on every other header to solve a +problem confined to one. The relaxation must name the header it applies to, or it is not a +relaxation but a removal. + +*Make the cookie-header cap per-anchor.* This is the natural containment and would confine the +raised cap to routes that need it. Rejected because it is not achievable at this pipeline position: +inbound validation must precede route selection so routing decides on canonicalised, validated +input, and therefore no anchor exists to scope against at the moment of the check. Deferring the +header check until after routing would mean routing on unvalidated input — a strictly worse exposure +than the raised cap it would contain. + +*Reject the cookie-mode variant instead of relaxing anything.* This would preserve a uniform policy +absolutely. Rejected because the stateless variant answers a real deployment need, and the +relaxation it requires is bounded and reviewable — the trade is a longer permitted cookie value +against a gateway that scales out without shared session state. + +The alternatives fall into two classes, and both are rejected for the same reason: one class widens +a relaxation past the case that motivates it, and the other refuses a bounded relaxation at the cost +of a legitimate capability. The chosen position is the narrowest scoping each relaxation's +enforcement point actually permits — and where that scoping is coarser than one would like, the +limit is structural and is stated as such rather than left to be discovered. + +== References + +* link:0018-BFF_session_mode_is_one_SessionBinding_seam_behind_a_fixed_reserved-path_and_CSRF_model.adoc[ADR-0018 -- BFF session mode and the reserved-path model] -- the architecture these relaxations serve. +* link:../architecture.adoc[Architecture] -- the inbound pipeline's position ahead of routing. +* link:../configuration.adoc#_oidc[Configuration Reference -- `oidc`] -- the session block carrying the cookie-size budget. +* link:../variants/03-bff-cookie.adoc[Variant 3 -- BFF, Cookie-based] -- the variant whose sealed cookie motivates the length relaxation. +* link:../development/bff-cookie.adoc[Contributor guide -- cookie-based BFF] -- the sealed-cookie format the budget bounds. diff --git a/doc/architecture.adoc b/doc/architecture.adoc index 2b55b351..3ba7f35c 100644 --- a/doc/architecture.adoc +++ b/doc/architecture.adoc @@ -172,40 +172,54 @@ the engine wires it internally. The engine's API surface is specified in ==== [[_bff_session_runtime]] -==== Server-session BFF runtime (edge wiring) +==== BFF runtime (edge wiring) -The *server-session* variant realizes the BFF side of that seam as four structural pieces, all +The BFF realizes its side of that seam as four structural pieces, all framework-agnostic (raw request pieces in, a rendered response out -- no JAX-RS/Vert.x coupling) -so each is unit-testable without a container: - -* *Session-resolution seam.* The opaque session-id cookie is the only browser-held state. A - `SessionCookieCodec` reads the id out of the request `Cookie` header and a `SessionStore` - (the in-memory `InMemorySessionStore`, bounded by `oidc.session.max_sessions`) resolves it to - the server-held `SessionRecord` carrying the tokens. A `require: session` route is served by a +so each is unit-testable without a container. The decisions behind this shape -- the reserved-path +model, the fixed CSRF model, the one `SessionBinding` seam the two session modes sit behind, the +cookie-mode key material, and the session-identity contract -- are recorded in +link:adr/0018-BFF_session_mode_is_one_SessionBinding_seam_behind_a_fixed_reserved-path_and_CSRF_model.adoc[ADR-0018]; +the two inbound-validation relaxations the BFF requires are recorded in +link:adr/0019-Reserved_BFF_paths_bypass_the_url-parameter_value_pipeline_and_the_cookie-header_length_relaxation_is_gateway-wide.adoc[ADR-0019]. + +* *Session-resolution seam.* Every collaborator binds to `SessionBinding`, never to a store; the + configured `session.mode` selects only which implementation is assembled (ADR-0018). In + *server* mode a `SessionCookieCodec` reads an opaque session id out of the request `Cookie` + header and a `SessionStore` (the in-memory `InMemorySessionStore`, bounded by + `oidc.session.max_sessions`) resolves it to the server-held `SessionRecord` carrying the tokens; + in *cookie* mode the sealed cookie itself carries that state and is unsealed per request, with no + store at all. Either way a `require: session` route is served by a `SessionAuthenticationStage` injected into the session-aware authentication stage -- so the - route is *mediated* (the stored access token injected upstream) rather than rejected. + route is *mediated* (the resolved access token injected upstream) rather than rejected. * *Reserved-path registry + live-edge dispatch.* The `ReservedPathRegistry` classifies each of the up-to-six gateway-owned OIDC paths (callback, logout, logout-return, back-channel logout, user-info, login-initiation) by *exact* host+path match. The gateway edge consults it *before* the proxy route table, so a matched reserved path is dispatched to its handler and a proxy route - such as `path_prefix: /auth` can never swallow `/auth/callback`. The - link:configuration.adoc#_oidc[`oidc`] block is the single source of the registered paths. + such as `path_prefix: /auth` can never swallow `/auth/callback` (ADR-0018). The + link:configuration.adoc#_oidc[`oidc`] block is the single source of the registered paths. Because + a matched reserved path terminates at the gateway rather than being proxied, the inbound + url-parameter *value* pipeline is deliberately not applied to it -- its parameters are validated + by the reserved handler itself, which knows their grammar (ADR-0019). * *Engine boundary.* The gateway re-implements no OAuth leg. Every reserved handler (login initiation, callback code-exchange, RP-initiated + back-channel logout, transparent refresh, RFC 9470 step-up) drives the `token-sheriff-client` engine across a small set of seams and adds only the browser-facing concern on top (binding/session cookies, CSRF, curated user-info view, - RFC 9457 rendering). The BFF never holds token material outside the `SessionStore`. + RFC 9457 rendering). The BFF never holds token material outside the binding in force -- the + `SessionStore` in server mode, the sealed cookie in cookie mode. * *`BffRuntime` / `BffRuntimeProducer` CDI wiring.* `BffRuntime` is the single collaborator the edge consults: it carries the `SessionAuthenticationStage` + fixed `CsrfDefence` and a `dispatch(...)` fan-out that routes each matched reserved kind to its wired handler and normalizes the outcomes into one response the edge renders verbatim. `BffRuntimeProducer` builds - it once at boot *only when a global `oidc` block with `session.mode=server` and a `redirect_uri` - is configured*; otherwise it produces the *inert* runtime, so a bearer-only gateway (or a - cookie-mode BFF) is unchanged and never touches the confidential-client engine. Provider-metadata - discovery is resolved lazily on first engine use, so a server-mode gateway boots without a live IdP. + it once at boot *only when a global `oidc` block with a `redirect_uri` and a recognised + `session.mode` -- `server` or `cookie` -- is configured*; otherwise it produces the *inert* + runtime, so a bearer-only gateway is unchanged and never touches the confidential-client engine. + Both modes drive the same wiring and both reach the engine; the mode selects only which + `SessionBinding` is assembled. Provider-metadata + discovery is resolved lazily on first engine use, so a BFF gateway boots without a live IdP. === Forward Policy (Zero-Trust) diff --git a/doc/development/README.adoc b/doc/development/README.adoc index 00f13d12..0a7a8a4e 100644 --- a/doc/development/README.adoc +++ b/doc/development/README.adoc @@ -59,6 +59,13 @@ This tree is seeded here and grows as contributor-facing material lands. how to build and test it, the engine-boundary rule (drive the `token-sheriff-client` engine, never re-implement an OAuth leg), the integration-test and benchmark entry points, and how to add a new reserved endpoint. + +| link:bff-cookie.adoc[Cookie-Based BFF -- Contributor Guide] +| Where the cookie-mode code lives (the `bff.cookie` types), the `SessionBinding` seam that keeps + every reserved endpoint mode-neutral, the sealed-cookie wire format and the rule that a format + change requires a `FORMAT_VERSION` bump, the derived session identity with its + stability-and-uniqueness constraints, the engine-boundary rule, and the unit/integration test + entry points. |=== == Scope of This Layer diff --git a/doc/development/bff-cookie.adoc b/doc/development/bff-cookie.adoc new file mode 100644 index 00000000..0986ac07 --- /dev/null +++ b/doc/development/bff-cookie.adoc @@ -0,0 +1,165 @@ += Cookie-Based BFF -- Contributor Guide +:toc: +:toclevels: 3 +:sectnums: + +A contributor-facing guide to the *cookie-based BFF* implementation (Variant 3): where its code +lives, how the sealed cookie is put together, and the rules that keep it correct. It describes how to +*work in* the code; the design rationale is in +link:../variants/03-bff-cookie.adoc[Variant 3 -- BFF, Cookie-based] and the architectural decision in +link:../adr/0018-BFF_session_mode_is_one_SessionBinding_seam_behind_a_fixed_reserved-path_and_CSRF_model.adoc[ADR-0018]. + +== The engine boundary rule (read first) + +*The gateway re-implements no OAuth leg.* The confidential-client *engine* (`token-sheriff-client`) +owns retrieval and flow -- auth-code + PKCE, `state`/`nonce`/exact `redirect_uri`, mix-up defence, +refresh-with-rotation, RP-initiated logout, RFC 9470 step-up. The *BFF* (this code) owns only the +browser-facing part: the binding/session cookies, CSRF, request proxying, the curated user-info view, +logout receivers, and RFC 9457 rendering. + +This rule is identical in both BFF variants -- cookie mode reaches the engine exactly as server mode +does. When you add or change a reserved handler, drive an engine seam; never hand-roll an OAuth +request, a token exchange, or a JWKS fetch inside the gateway. + +== Where the code lives + +The cookie-specific code is in the `api-sheriff` module under +`de.cuioss.sheriff.gateway.bff.cookie`: + +[cols="1,2"] +|=== +| Type | Responsibility + +| `SealedSessionPayload` +| The plaintext that gets sealed: the mediated tokens, the identity/session claims, the login instant + anchoring the TTL, and the per-session identity nonce. Owns the inner wire format (`encode()` / + `decode(...)`) and redacts every credential from `toString()`. + +| `SealedSessionCookieCodec` +| The AES-256-GCM seal/unseal, the outer value layout, the `FORMAT_VERSION` stamp, deterministic + key selection by key id, the `Set-Cookie` hardening, and the seal-time size budget. + +| `CookieSessionBinding` +| The `SessionBinding` implementation: `bind` (seal on login, mint the nonce), `resolve` (unseal, + enforce the absolute TTL server-side), `persist` (re-seal a refresh), and the derived session + identity. Reports `IdpDestruction.UNSUPPORTED`. + +| `CookieKeyMaterial` +| Resolves the key material -- passed `encryption_key` versus generate-on-startup -- plus the + decrypt-only `previous_key` and the derived per-gateway identity salt. +|=== + +Everything else is shared with the server-session variant; see +link:bff-session.adoc#_where_the_code_lives[the server-session guide] for the `bff.*` package map. + +== The SessionBinding seam + +`SessionBinding` is the seam the whole BFF foundation binds to -- never a store directly. The +configured `session.mode` selects only *which implementation* `BffRuntimeProducer` assembles: +`ServerSessionBinding` over the in-memory store, or `CookieSessionBinding` over the sealing codec. + +Every reserved endpoint, the CSRF defence, the refresh and step-up coordinators, and the +`require: session` stage-4 runtime are consequently *mode-neutral*: they are written against the +seam and are unaware of which binding is in force. That is why adding a cookie-mode capability +almost never means touching a reserved endpoint. + +The one place the modes genuinely differ is capability: `idpDestruction()` reports `UNSUPPORTED` +for the cookie binding, and the back-channel logout endpoint is *gated on that signal* rather than +on the mode name -- it answers a deliberate `404` instead of claiming a destruction that never +happened. + +== The sealed-cookie wire format + +The cookie value is a base64url-encoded *outer envelope*: + +[source] +---- +version(1) | key-id(1) | GCM nonce(12) | ciphertext | GCM tag(16) +---- + +The `version` and `key-id` bytes are bound into the GCM *associated data*, so tampering with either +fails the tag check rather than silently selecting a different parse. Key selection is deterministic +by the stamped key id -- never a try-every-key decrypt. + +The *inner* plaintext (what `SealedSessionPayload.encode()` produces) is nine newline-separated +fields in record-declaration order, each URL-safe base64 of its UTF-8 bytes so no value can collide +with the separator; an empty field stands for an absent optional. It is read only *after* the GCM +tag has authenticated the bytes -- `decode(...)` is never a parser for untrusted input. + +[IMPORTANT] +==== +*Any change to the inner field set requires a `FORMAT_VERSION` bump.* The field-count guard is +strict and there is no legacy-shape acceptance path: a cookie in the previous format must fail to +unseal, not be admitted with a synthesized value. Bumping the version is what makes that failure +clean, since the version byte is authenticated. A bump forces every existing cookie-mode session to +re-login -- call that out in the operator documentation when you make one. +==== + +== The derived session identity + +`SessionRecord.sessionId()` is the one identity model the seam defines, and it is the key the refresh +coordinator uses for single-flight coalescing. Cookie mode *discards* the id minted at login and +derives its own instead: a salted digest over the login instant, `sub`, and the per-session nonce. + +Two constraints hold it together: + +* *Stable across re-seals.* All three digest inputs are fixed for the life of the session, so a + refresh reproduces the identity byte-identically. `persist` re-seals the nonce *verbatim* and + fails loud rather than re-minting; `TokenRefreshCoordinator.rotate()` must copy the nonce onto the + rotated record. `rotate()` rebuilds the record component-by-component, so *any component you add to + `SessionRecord` needs a copy line there* or it is silently dropped. +* *Unique.* The nonce is what makes two logins by the same subject inside one clock second resolve to + distinct identities; without it they collided and cross-wired the single-flight keying. + +The identity is never emitted to the browser -- only the sealed value crosses -- and the raw nonce is +never used as, or substituted for, the session id. Single-flight coalescing is necessarily *per +instance* here; a cross-instance duplicate refresh is this variant's documented, accepted trade-off. + +== Build and test + +The canonical build commands are the ones in the repository `CLAUDE.md` (invoked through the build +executor, never a raw `mvn`). Both the quality gate and full verify must pass with zero findings +before a commit. + +Unit-test entry points: + +* `SealedSessionPayloadTest` -- the inner wire format: round trips, absent optionals, + separator-bearing values, the strict field-count guard, and credential redaction. +* `SealedSessionCookieCodecTest` -- seal/unseal, the version and key-id header, fail-closed rejection + of a tampered nonce/ciphertext/tag, unknown version or key id, `previous_key` rotation, and the + size budget. +* `CookieSessionBindingTest` -- bind/resolve/persist, the server-side TTL enforcement, rotation + re-seal, and the identity properties (stability across a re-seal, uniqueness within a clock + second). +* `TokenRefreshCoordinatorTest` -- its `CookieMode` group covers the stateless refresh: re-seal into + a new `Set-Cookie` and nonce propagation across `rotate()`. + +Integration tests live in the `integration-tests` module and run against the native image on the +Docker compose stack under the integration-tests Maven profile: + +* `BffCookieSessionIT` -- the full login -> mediated call -> logout round trip through a real IdP. +* `BffCookieStatelessnessIT` -- the two-instance topology (ports `10445` and `10446`) sharing only + the sealing key. It logs in against the *first* instance only, then exercises the second, which + is what makes "the peer held no record of this session" the evidence rather than an assumption. + That asymmetry is deliberate: it also documents the login-leg affinity boundary. + +Both IT suites are behaviour-level and are *not* bound to the inner wire form, so a format change +should leave them untouched and green. + +== How to extend + +* *A new sealed field* -- add the record component, append it in `encode()` in declaration order, + read it back in `decode(...)`, raise the field count, and *bump `FORMAT_VERSION`*. If it + participates in identity, mind the stability constraint above. +* *A new session-runtime capability* -- write it against the `SessionBinding` seam so it stays + mode-neutral; if it genuinely cannot work statelessly, gate it on a capability signal (as + back-channel logout does) rather than on the mode name. +* *Anything touching an OAuth leg* -- drive an engine seam, per the boundary rule. + +== See also + +* link:../variants/03-bff-cookie.adoc[Variant 3 -- BFF, Cookie-based] -- the design. +* link:../adr/0018-BFF_session_mode_is_one_SessionBinding_seam_behind_a_fixed_reserved-path_and_CSRF_model.adoc[ADR-0018 -- BFF architecture] -- the decision record. +* link:../adr/0019-Reserved_BFF_paths_bypass_the_url-parameter_value_pipeline_and_the_cookie-header_length_relaxation_is_gateway-wide.adoc[ADR-0019 -- BFF security relaxations] -- the accepted trade-offs. +* link:../user/bff-cookie.adoc[Operator setup -- cookie-based BFF] -- how it is configured and run. +* link:bff-session.adoc[Contributor guide -- server-session BFF] -- the shared package map. diff --git a/doc/user/README.adoc b/doc/user/README.adoc index a900340d..2e59f351 100644 --- a/doc/user/README.adoc +++ b/doc/user/README.adoc @@ -44,6 +44,13 @@ layer. confidential client, protecting a route with `require: session`, the gateway-owned reserved paths, serving a curated user-info view, CSRF for a cross-origin SPA, and operating the in-memory session store. Links to the Configuration Reference for the `oidc` key contract. + +| link:bff-cookie.adoc[Cookie-Based BFF -- Operator Setup] +| A task-oriented guide for standing up the cookie-based BFF (Variant 3): registering the OIDC + confidential client, selecting `session.mode: cookie`, the two key-material modes and their + trade-off, decrypt-only key rotation, the cookie size budget, what IdP-driven destruction cannot + do in this variant, the login-leg session affinity, and the forced re-login on a sealed-cookie + format bump. Links to the Configuration Reference for the `oidc` key contract. |=== == Scope of This Layer diff --git a/doc/user/bff-cookie.adoc b/doc/user/bff-cookie.adoc new file mode 100644 index 00000000..57e435b4 --- /dev/null +++ b/doc/user/bff-cookie.adoc @@ -0,0 +1,168 @@ += Cookie-Based BFF -- Operator Setup +:toc: +:toclevels: 3 +:sectnums: + +A task-oriented guide for operators standing up the *cookie-based BFF* (Variant 3): API Sheriff as +an OIDC confidential client that keeps no server-side session store and hands the browser a sealed +cookie that *is* the session. It tells you *how* to configure and run the variant; the field-by-field +contract lives in the link:../configuration.adoc#_oidc[Configuration Reference (`oidc`)] and the +design rationale in link:../variants/03-bff-cookie.adoc[Variant 3 -- BFF, Cookie-based]. + +== When to choose this variant + +Choose the cookie-based BFF when you need a *stateless* gateway: multiple replicas with no shared +session store and no sticky routing for ordinary traffic. Tokens still never reach the browser in +readable form -- they are sealed with AES-256-GCM and the cookie is `HttpOnly`. + +Choose the link:../variants/02-bff-session.adoc[server-session variant] instead when server-side-only +token custody is a hard requirement (the token must never leave the gateway, not even encrypted), or +when you need IdP-driven back-channel logout. + +== Prerequisites + +* An OIDC provider (IdP) where you can register a *confidential client* and obtain a + `client_id` / `client_secret`. +* A registered *redirect URI* pointing at the gateway's own callback path (`oidc.redirect_uri`), + matched exactly at the IdP. +* The client secret and any session keys available as environment variables (never written inline -- + secrets in `oidc` must be `${ENV_VAR}` references). + +== Configure the confidential client + +Add an `oidc` block to `gateway.yaml` and set at least one route's effective auth to +`require: session`. A minimal cookie-based configuration: + +[source,yaml] +---- +# gateway.yaml +oidc: + issuer: https://idp.example.com/realms/main + client_id: api-sheriff + client_secret: ${SHERIFF_CLIENT_SECRET} # must be an ${ENV_VAR} reference + redirect_uri: https://gw.example.com/auth/callback # registered exactly at the IdP + scopes: [openid, profile, orders.read] # include every scope a route requires + session: + mode: cookie # sealed cookie, no store (this variant) + cookie_name: __Host-sheriff + encryption_key: ${SHERIFF_SESSION_KEY} # optional -- see "Key material" below + previous_key: ${SHERIFF_SESSION_KEY_PREVIOUS} # optional; decrypt-only (rotation) + max_cookie_size: 4096 # seal-time budget; see "Cookie size budget" + ttl_seconds: 3600 # absolute lifetime from login + csrf: + trusted_origins: [https://app.example.com] + refresh: { enabled: true, leeway_seconds: 30, on_failure: reauthenticate } +---- + +The gateway ignores the `oidc` block entirely unless a route's effective auth is `require: session`, +so a bearer-only deployment is unaffected by its presence. + +== Key material + +The sealing key has *two modes*, selected by whether you declare `session.encryption_key`. + +[cols="1,2,2"] +|=== +| Mode | How to select | Trade-off + +| Passed key +| Declare `session.encryption_key` as an `${ENV_VAR}` reference holding an AES-256 key. +| Sessions survive a gateway restart and are shareable across replicas. Requires you to generate, + distribute and safeguard the key. + +| Generate on startup +| Omit `session.encryption_key` entirely. +| No operator input at all -- the gateway mints a key at boot. But *sessions are dropped on every + restart* (users re-authenticate) and the key *cannot be shared* with another replica, so each + replica can only unseal its own cookies. +|=== + +Use generate-on-startup for a single-replica deployment or a development environment. Use a passed +key for anything multi-replica or restart-sensitive. The gateway raises a startup diagnostic naming +the active key mode and rotation state -- it never logs key bytes. + +=== Rotating the key + +`session.previous_key` is *decrypt-only*: a cookie still sealed under it keeps resolving, and the +session's next write re-seals it under the current key. Sealing is always done with +`encryption_key`, never with `previous_key`. + +`previous_key` *requires* an `encryption_key` -- declaring it alone is rejected at boot, because a +decrypt-only rotation key with no current key to roll onto is nonsensical. Rotation therefore +composes only with the passed-key mode; generate-on-startup has no previous key by construction. + +Withdraw `previous_key` once the rotation window has passed; cookies still carrying the retired key +id are then refused as "no session" and those users log in again. + +== Cookie size budget + +Because the session travels in the cookie, its sealed value has a size budget -- +`session.max_cookie_size`, defaulting to 4096 bytes. Browsers are only required to accept 4 KB per +cookie, so a larger value risks being *silently dropped* by the browser, leaving a session that can +never be resolved. + +The gateway therefore refuses to seal an over-budget session rather than truncating it, and logs a +warning naming the offending length. If you hit the budget, *reduce the token set* -- trim `scopes` +so the IdP issues a smaller access token, and cut `user_info.allowed_claims` -- or *run the +link:../variants/02-bff-session.adoc[server-session variant]*, where token size is irrelevant +because nothing crosses to the browser. + +== What is unavailable in this variant + +A stateless gateway holds no session index and cannot reach another browser's cookie, so +IdP-driven destruction is *unsupported*: + +* *Back-channel logout answers `404`.* The reserved path stays wired and deliberately returns `404` + rather than falling through to the proxy route table -- it never claims a destruction that did not + happen. +* *No destroy-by-`sid` / destroy-by-`sub`.* Nothing can be revoked centrally mid-session. + +RP-initiated logout (the user logging themselves out) works normally: the browser's cookie is +cleared, and an expired cookie the browser keeps anyway is still refused server-side by the absolute +TTL check. Where instant central revocation is a hard requirement, use the server-session variant. +Mitigate in this variant with a short `ttl_seconds` and short access-token lifetimes. + +== Session affinity for the login leg + +The variant is stateless once a session is *established*, but not during the login exchange. The +pending authorization held between the login redirect and the OIDC callback is per-instance and +never shared, so a login *begun* on one replica must *complete* on that replica. + +*Configure session affinity for the login leg only* -- the redirect through to the callback. Once +the session cookie is issued, requests may be balanced freely across replicas with no affinity. + +== Upgrading: forced re-login on a format bump + +The sealed-cookie format carries a version that is bound into the cookie's authenticated data. When +that version is bumped, cookies in the previous format fail to unseal rather than being mis-parsed, +and there is deliberately no backward-compatible acceptance path. + +*Plan for it*: on an upgrade that bumps the format version, all existing cookie-mode sessions are +invalidated at deploy time and users must log in again. There is no rolling migration -- schedule +the deploy accordingly. The most recent bump added the per-session identity nonce. + +== Operate + +* *Absolute TTL.* `ttl_seconds` is counted from login and is never extended by a refresh; the + deadline is recomputed from the original login on every re-seal, so an endlessly-refreshed session + still dies on time. +* *Refresh re-seals the cookie.* A transparent token refresh emits a new `Set-Cookie` -- that is the + whole of "persisting" in this variant. +* *IdP reachability.* Provider metadata is discovered lazily on first use, so the gateway boots + without the IdP -- but logins fail until the issuer is reachable. + +== Private-CA / self-signed IdP + +If your IdP presents a certificate from a *private CA* (or a self-signed cert), add that CA to the +*JVM default truststore* -- the confidential-client engine uses it for outbound OIDC calls +(discovery, token, refresh, logout), and the per-issuer `tls_profile` that validates JWKS does not +cover those calls. See link:bff-session.adoc#_private_ca_self_signed_idp[the server-session guide] +for the exact system properties; the requirement is identical in both variants. + +== See also + +* link:../configuration.adoc#_oidc[Configuration Reference -- `oidc`] -- the authoritative key contract. +* link:../variants/03-bff-cookie.adoc[Variant 3 -- BFF, Cookie-based] -- design and flow rationale. +* link:../adr/0018-BFF_session_mode_is_one_SessionBinding_seam_behind_a_fixed_reserved-path_and_CSRF_model.adoc[ADR-0018 -- BFF architecture] -- why the variant is shaped this way. +* link:../adr/0019-Reserved_BFF_paths_bypass_the_url-parameter_value_pipeline_and_the_cookie-header_length_relaxation_is_gateway-wide.adoc[ADR-0019 -- BFF security relaxations] -- the accepted trade-offs. +* link:../development/bff-cookie.adoc[Contributor guide -- cookie-based BFF] -- building and extending it. diff --git a/doc/variants/03-bff-cookie.adoc b/doc/variants/03-bff-cookie.adoc index 9c3df561..1ca9c9e7 100644 --- a/doc/variants/03-bff-cookie.adoc +++ b/doc/variants/03-bff-cookie.adoc @@ -151,7 +151,7 @@ oidc: session: mode: cookie # encrypted cookie, no store (this variant) cookie_name: __Host-sheriff - encryption_key: ${SHERIFF_SESSION_KEY} # AES-256-GCM key + encryption_key: ${SHERIFF_SESSION_KEY} # AES-256-GCM key; optional -- omit to generate on startup previous_key: ${SHERIFF_SESSION_KEY_PREVIOUS} # optional; decrypt-only (key rotation) ttl_seconds: 3600 refresh: { enabled: true, leeway_seconds: 30, on_failure: reauthenticate } @@ -174,7 +174,14 @@ endpoint: ---- The only material difference from the link:02-bff-session.adoc[session-based] configuration is -`session.mode: cookie` and the required `session.encryption_key`. +`session.mode: cookie`. + +`session.encryption_key` is *optional*. Declare it when sessions must be shared across replicas +and survive a restart -- that is the only thing it buys. Omitting it selects the +generate-on-startup key mode: the gateway mints a key at boot, so sessions are dropped on every +restart and the key cannot be shared with another replica. `previous_key` is optional and +decrypt-only, and requires an `encryption_key` to roll onto. See +link:../configuration.adoc[the configuration reference] for the authoritative validation rules. == Properties @@ -184,3 +191,35 @@ The only material difference from the link:02-bff-session.adoc[session-based] co defence and short token lifetimes). * *Self-contained* -- instances share only the encryption key. * *Full OIDC RP* -- confidential-client login, transparent refresh, RFC 9470 step-up. + +=== Stateless after login, not during it + +The variant is stateless once a session is *established*, not for the whole login exchange. The +pending authorization -- the state the gateway holds between issuing the login redirect and +receiving the OIDC callback -- lives in an in-memory `PendingAuthorizationStore`, which is +per-instance and never shared. A login *begun* on one instance must therefore *complete* on that +same instance; a callback landing on a different replica finds no pending authorization and the +login fails. + +An *established* session is fully portable: any replica holding the sealing key can unseal the +cookie and serve the request. That is exactly the boundary the cookie-statelessness integration +test documents -- it logs in only against the first instance, then exercises the second instance +with the resulting cookie. + +*Deployment consequence*: session affinity is required for the *login leg only* (the redirect +through to the callback). Once the session cookie is issued, requests may be balanced freely +across replicas with no affinity at all. + +=== Sealed-cookie format version + +The sealed-cookie format version is bound into the cookie's authenticated data, so a cookie +carrying a retired version fails to unseal rather than being mis-parsed. The version was bumped +when the per-session identity nonce was added to the sealed payload. + +*Upgrade consequence*: existing cookie-mode sessions are invalidated on deploy and users must log +in again. There is deliberately no backward-compatible acceptance path for the previous format -- +accepting it would mean synthesizing the missing nonce, which would silently change the derived +session identity. See +link:../adr/0018-BFF_session_mode_is_one_SessionBinding_seam_behind_a_fixed_reserved-path_and_CSRF_model.adoc[ADR-0018] +for the architectural +decision behind the identity model. From 772e7ab1c9da653ac18dc629a9224725dcb1450c Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Wed, 29 Jul 2026 00:17:03 +0200 Subject: [PATCH 04/13] docs(claude): name all three PR reviewers, add post-merge step --- CLAUDE.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index bd37f4b0..28a93256 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -132,14 +132,18 @@ All cuioss repositories have branch protection on `main`. Direct pushes to `main 2. Commit changes: `git add && git commit -m ""` 3. Push the branch: `git push -u origin ` 4. Create a PR: `gh pr create --repo cuioss/API-Sheriff --head --base main --title "" --body "<body>"` -5. Wait for CI + Gemini review (waits until checks complete): `gh pr checks --watch` -6. **Handle Gemini review comments** — fetch with `gh api repos/cuioss/API-Sheriff/pulls/<pr-number>/comments` and for each: +5. Wait for CI and the three automated reviewers — CodeRabbit, Sourcery and PR-Agent (waits until checks complete): `gh pr checks --watch` +6. **Handle the automated review comments from all three reviewers** — CodeRabbit, Sourcery and PR-Agent each comment independently, so treat the union of their comments as the work list. Fetch with `gh api repos/cuioss/API-Sheriff/pulls/<pr-number>/comments` and for each comment, whichever bot authored it: - If clearly valid and fixable: fix it, commit, push, then reply explaining the fix and resolve the comment - If disagree or out of scope: reply explaining why, then resolve the comment - If uncertain (not 100% confident): **ask the user** before acting - Every comment MUST get a reply (reason for fix or reason for not fixing) and MUST be resolved + - **Re-review after pushing fixes is not uniform**: CodeRabbit and Sourcery re-review automatically on push; PR-Agent deliberately does **not** (`.github/workflows/pr-agent.yml` triggers only on `opened`/`reopened`/`ready_for_review` plus on-demand `issue_comment` commands). Re-request a PR-Agent pass explicitly by posting a `/review` comment on the PR after the fix push. 7. Do **NOT** enable auto-merge unless explicitly instructed. Wait for user approval. -8. Return to main: `git checkout main && git pull` +8. **After the merge lands, verify the post-merge state**: + - Re-check the PR's post-merge checks. The only genuinely post-merge workflow in this repository is `.github/workflows/benchmark.yml` (`pull_request: types: [closed]` gated on `merged == true`, plus tag pushes and `workflow_dispatch`); because it is `pull_request`-triggered, its run stays attached to the PR and remains visible through the CI abstraction after the merge. + - Assert zero unresolved review threads: `python3 .plan/execute-script.py plan-marshall:tools-integration-ci:ci pr comments --pr-number <n> --unresolved-only`. Route any straggler into a follow-up issue or plan rather than leaving it unresolved. +9. Return to main: `git checkout main && git pull` ## Temporary Files From cc8f60ab0441c9bfb9b013b6cf1bb51a2cf43ce2 Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Wed, 29 Jul 2026 00:22:30 +0200 Subject: [PATCH 05/13] style(bff): apply pre-commit formatter to cookie session tests --- .../gateway/bff/cookie/SealedSessionCookieCodecTest.java | 2 +- .../sheriff/gateway/bff/cookie/SealedSessionPayloadTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionCookieCodecTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionCookieCodecTest.java index bf75dd3c..c810bc92 100644 --- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionCookieCodecTest.java +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionCookieCodecTest.java @@ -135,7 +135,7 @@ void shouldCarryVersionAndKeyIdHeader() throws Exception { @Test @DisplayName("Should stamp format version 2 — the nine-field payload shape") void shouldStampFormatVersionTwo() { - assertEquals((byte) 2, SealedSessionCookieCodec.FORMAT_VERSION, + assertEquals(SealedSessionCookieCodec.FORMAT_VERSION, (byte) 2, "the per-session nonce raised the payload to nine fields, which is a wire-format break"); } } diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionPayloadTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionPayloadTest.java index b2321a5a..7c278e50 100644 --- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionPayloadTest.java +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionPayloadTest.java @@ -206,7 +206,7 @@ void shouldRejectAbsentMandatoryComponents() { assertThrows(NullPointerException.class, () -> new SealedSessionPayload(ACCESS_TOKEN, Optional.empty(), ID_TOKEN, SUB, Optional.empty(), Optional.empty(), Optional.empty(), null, SESSION_NONCE)); assertThrows(NullPointerException.class, () -> new SealedSessionPayload(ACCESS_TOKEN, Optional.empty(), - ID_TOKEN, SUB, Optional.empty(), Optional.empty(), Optional.empty(), LOGIN, null), + ID_TOKEN, SUB, Optional.empty(), Optional.empty(), Optional.empty(), LOGIN, null), "the session nonce is mandatory — it keys the derived session identity"); } From f25cd347f1a689551ff733a60da71a039d556a2b Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Wed, 29 Jul 2026 07:45:31 +0200 Subject: [PATCH 06/13] chore(simplify): collapse accidental complexity in plan-07b-bff-cookie-docs-adr Collapse two near-identical SessionRecord-rebuild helpers in CookieSessionBindingTest into one shared withRotatedAccessToken helper. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013EfShpgB9HbUSf9n6zhP9r --- .../bff/cookie/CookieSessionBindingTest.java | 52 +++++++++---------- 1 file changed, 24 insertions(+), 28 deletions(-) diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/CookieSessionBindingTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/CookieSessionBindingTest.java index 703634f5..142a525e 100644 --- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/CookieSessionBindingTest.java +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/CookieSessionBindingTest.java @@ -106,26 +106,35 @@ private static SessionRecord session(String accessToken, Instant expiresAt) { .build(); } + /** + * Rebuilds {@code original} with a rotated access token, carrying every other component — + * notably the session nonce — over verbatim, exactly as the refresh coordinator's + * {@code rotate()} does. A re-seal test must carry the nonce forward this way: {@code persist} + * refuses a record without one rather than re-minting it. + */ + private static SessionRecord withRotatedAccessToken(SessionRecord original, String rotatedAccessToken) { + return SessionRecord.builder() + .sessionId(original.sessionId()) + .accessToken(rotatedAccessToken) + .refreshToken(original.refreshToken()) + .idToken(original.idToken()) + .sub(original.sub()) + .sid(original.sid()) + .expiresAt(original.expiresAt()) + .acr(original.acr()) + .authTime(original.authTime()) + .sessionNonce(original.sessionNonce()) + .build(); + } + /** * A record that is legal to re-seal: bound then resolved, so it carries the session nonce minted - * at login. {@code persist} refuses a record without one rather than re-minting, so a re-seal - * test must start from a resolved record exactly as the refresh coordinator does. + * at login. */ private SessionRecord resealable(String rotatedAccessToken) { BoundSession bound = binding.bind(session(ACCESS_TOKEN, LOGIN.plus(TTL)), LOGIN); SessionRecord resolved = binding.resolve(cookieHeaderOf(bound), LOGIN).orElseThrow(); - return SessionRecord.builder() - .sessionId(resolved.sessionId()) - .accessToken(rotatedAccessToken) - .refreshToken(resolved.refreshToken()) - .idToken(resolved.idToken()) - .sub(resolved.sub()) - .sid(resolved.sid()) - .expiresAt(resolved.expiresAt()) - .acr(resolved.acr()) - .authTime(resolved.authTime()) - .sessionNonce(resolved.sessionNonce()) - .build(); + return withRotatedAccessToken(resolved, rotatedAccessToken); } private static String cookieHeaderOf(BoundSession bound) { @@ -342,20 +351,7 @@ void shouldDeriveStableIdentity() { BoundSession bound = binding.bind(session(ACCESS_TOKEN, LOGIN.plus(TTL)), LOGIN); SessionRecord first = binding.resolve(cookieHeaderOf(bound), LOGIN).orElseThrow(); - // The re-seal carries the resolved record's nonce forward, exactly as the refresh - // coordinator's rotate() does — the nonce is an identity input, never re-minted. - SessionRecord rotated = SessionRecord.builder() - .sessionId(first.sessionId()) - .accessToken("rotated-access-token") - .refreshToken(first.refreshToken()) - .idToken(first.idToken()) - .sub(first.sub()) - .sid(first.sid()) - .expiresAt(first.expiresAt()) - .acr(first.acr()) - .authTime(first.authTime()) - .sessionNonce(first.sessionNonce()) - .build(); + SessionRecord rotated = withRotatedAccessToken(first, "rotated-access-token"); BoundSession reBound = binding.persist(rotated, LOGIN.plusSeconds(60)); SessionRecord second = binding.resolve(cookieHeaderOf(reBound), LOGIN.plusSeconds(60)).orElseThrow(); From b6762c5280eb805df22b6b76ab57ee403c91939a Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Wed, 29 Jul 2026 08:28:44 +0200 Subject: [PATCH 07/13] fix(native): initialize CookieSessionBinding at run time The cookie-mode per-session nonce is minted from a static final SecureRandom on the login path. GraalVM initializes static state at build time by default, so the class initializer baked a seeded SecureRandom into the image heap and native-image failed with 'Detected an instance of Random/SplittableRandom class in the image heap'. Register the class with --initialize-at-run-time, matching the three sibling BFF classes that already hold a static SecureRandom for the same reason. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013EfShpgB9HbUSf9n6zhP9r --- api-sheriff/src/main/resources/application.properties | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/api-sheriff/src/main/resources/application.properties b/api-sheriff/src/main/resources/application.properties index 6390b6a7..e7020767 100644 --- a/api-sheriff/src/main/resources/application.properties +++ b/api-sheriff/src/main/resources/application.properties @@ -46,7 +46,7 @@ quarkus.tls.cipher-suites=TLS_AES_256_GCM_SHA384,TLS_CHACHA20_POLY1305_SHA256,TL quarkus.tls.alpn=true # Native Image Configuration -# The three BFF records below each hold a `static final SecureRandom`. GraalVM initializes static +# The four BFF classes below each hold a `static final SecureRandom`. GraalVM initializes static # state at BUILD time by default, which would bake a seeded SecureRandom into the image heap # (a build failure — "instance of Random/SplittableRandom in the image heap" — AND a real security # defect: predictable randomness from a cached seed). Force runtime initialization for each so the @@ -60,7 +60,10 @@ quarkus.tls.alpn=true # package-prefix --initialize-at-run-time=de.cuioss.sheriff.token.client.flow covers the whole flow # package (GraalVM treats a bare package name as a prefix over that package and its classes), so # every current and future sibling flow class is forced to runtime initialization in one entry. -quarkus.native.additional-build-args=--enable-url-protocols=https,--enable-http,--enable-https,-O2,--initialize-at-run-time=de.cuioss.sheriff.gateway.bff.pending.PendingAuthorizationRecord,--initialize-at-run-time=de.cuioss.sheriff.gateway.bff.session.SessionRecord,--initialize-at-run-time=de.cuioss.sheriff.gateway.bff.logout.RpInitiatedLogout,--initialize-at-run-time=de.cuioss.sheriff.token.client.flow +# CookieSessionBinding joins the list for the same reason: the cookie-mode per-session nonce is +# minted from a `static final SecureRandom` on the login path, so its class initializer would +# otherwise bake a seeded instance into the image heap. +quarkus.native.additional-build-args=--enable-url-protocols=https,--enable-http,--enable-https,-O2,--initialize-at-run-time=de.cuioss.sheriff.gateway.bff.pending.PendingAuthorizationRecord,--initialize-at-run-time=de.cuioss.sheriff.gateway.bff.session.SessionRecord,--initialize-at-run-time=de.cuioss.sheriff.gateway.bff.logout.RpInitiatedLogout,--initialize-at-run-time=de.cuioss.sheriff.gateway.bff.cookie.CookieSessionBinding,--initialize-at-run-time=de.cuioss.sheriff.token.client.flow quarkus.native.resources.includes=**/*.p12,**/*.crt,**/*.key,schema/*.json quarkus.native.monitoring=jfr # Java 25 Mandrel builder image for JEP 491 (virtual threads without pinning) — see ADR-0001 From 0d386eaf4a4c29f053d189ef0a0083a533623000 Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Wed, 29 Jul 2026 09:06:38 +0200 Subject: [PATCH 08/13] fix(it): adapt BffCookieSessionIT to the version-2 sealed layout and fix its tamper helper Two fixes in the cookie-session IT: 1. sealedCookieValueCarriesNoReadableToken asserted the sealed cookie's leading version byte is 1. FORMAT_VERSION is now 2, so assert 2. 2. tamperedCookieIsUnauthenticatedNeverServerError used the base64-padding-bit tamper helper that PR #121 replaced in BffCookieStatelessnessIT but left in place here. Flipping the final base64url character is not reliably a mutation: when the sealed length is not a multiple of three that character carries unused padding bits which the decoder discards, so the value re-decodes identically and the peer correctly answers 200. Adding the 9th sealed-payload field shifted the length so the degenerate case became deterministic rather than the ~4% flake #121 described. Apply the same decoded-GCM-tag mutation the sibling IT already uses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013EfShpgB9HbUSf9n6zhP9r --- .../integration/BffCookieSessionIT.java | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/integration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/BffCookieSessionIT.java b/integration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/BffCookieSessionIT.java index eb5c53ce..99903372 100644 --- a/integration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/BffCookieSessionIT.java +++ b/integration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/BffCookieSessionIT.java @@ -131,8 +131,8 @@ void sealedCookieValueCarriesNoReadableToken() { "the sealed cookie must carry no readable segment of the mediated access token"); } byte[] raw = Base64.getUrlDecoder().decode(sealed); - assertEquals((byte) 1, raw[0], - "the cookie value must be the version-1 sealed layout, not an opaque server-side handle"); + assertEquals((byte) 2, raw[0], + "the cookie value must be the version-2 sealed layout, not an opaque server-side handle"); } @Test @@ -202,15 +202,23 @@ void tamperedCookieIsUnauthenticatedNeverServerError() { } /** - * Flips the final character of the sealed value, which sits inside the authentication tag. + * Flips a bit inside the trailing 16-byte GCM authentication tag. + * <p> + * The mutation is applied to the <em>decoded</em> bytes rather than to the final base64url + * character, because editing that character is not reliably a mutation at all: when the sealed + * length is not a multiple of three, the final character carries 2 or 4 unused padding bits that + * {@link Base64#getUrlDecoder()} discards without validating. Flipping only those bits — which is + * what {@code 'A' -> 'B'} does — re-decodes to the identical ciphertext and tag, so the peer + * unseals it successfully and answers 200. Mutating a decoded tag byte is unambiguous. * * @param sealed the sealed cookie value * @return the tampered value, still in the base64url alphabet */ private static String tamper(String sealed) { assertNotNull(sealed, "the login must establish a sealed session cookie"); - char last = sealed.charAt(sealed.length() - 1); - return sealed.substring(0, sealed.length() - 1) + (last == 'A' ? 'B' : 'A'); + byte[] raw = Base64.getUrlDecoder().decode(sealed); + raw[raw.length - 1] ^= 0x01; + return Base64.getUrlEncoder().withoutPadding().encodeToString(raw); } /** From ba77195bc1a71428959350dcc0cb9fc935a51a09 Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Wed, 29 Jul 2026 09:36:43 +0200 Subject: [PATCH 09/13] =?UTF-8?q?fix(docs):=20correct=20the=20post-merge?= =?UTF-8?q?=20check=20step=20=E2=80=94=20deploy-snapshot=20does=20exist?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The earlier version of step 8 claimed benchmark.yml is the only genuinely post-merge workflow. That was derived from grepping .github/workflows/ and finding no deploy-snapshot, but deploy-snapshot is a job of the reusable cuioss-organization/reusable-maven-build.yml that maven.yml delegates to, so it never appears in this repo's workflow files. maven.yml triggers on push to main, and deploy-snapshot is skipped on pull requests but runs on that push. It is therefore a real post-merge job — and unlike benchmark.yml it is NOT attached to the PR, so it has to be looked up by merge commit. Step 8 now covers both, and names the distinction that makes the second one easy to miss. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013EfShpgB9HbUSf9n6zhP9r --- CLAUDE.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 28a93256..cf3a90fe 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -141,7 +141,8 @@ All cuioss repositories have branch protection on `main`. Direct pushes to `main - **Re-review after pushing fixes is not uniform**: CodeRabbit and Sourcery re-review automatically on push; PR-Agent deliberately does **not** (`.github/workflows/pr-agent.yml` triggers only on `opened`/`reopened`/`ready_for_review` plus on-demand `issue_comment` commands). Re-request a PR-Agent pass explicitly by posting a `/review` comment on the PR after the fix push. 7. Do **NOT** enable auto-merge unless explicitly instructed. Wait for user approval. 8. **After the merge lands, verify the post-merge state**: - - Re-check the PR's post-merge checks. The only genuinely post-merge workflow in this repository is `.github/workflows/benchmark.yml` (`pull_request: types: [closed]` gated on `merged == true`, plus tag pushes and `workflow_dispatch`); because it is `pull_request`-triggered, its run stays attached to the PR and remains visible through the CI abstraction after the merge. + - Re-check the **PR-attached** post-merge run. `.github/workflows/benchmark.yml` is `pull_request: types: [closed]` gated on `merged == true`, so its run stays attached to the PR and remains visible through the CI abstraction after the merge. + - Also check the **main-branch** post-merge run, which is *not* attached to the PR. `maven.yml` triggers on `push: branches: [main]`, and its `deploy-snapshot` job is skipped on pull requests but runs on that push — so a snapshot-deployment failure appears only in the Maven Build run for the merge commit on `main`, never on the PR. Look it up by the merge commit rather than by PR number. - Assert zero unresolved review threads: `python3 .plan/execute-script.py plan-marshall:tools-integration-ci:ci pr comments --pr-number <n> --unresolved-only`. Route any straggler into a follow-up issue or plan rather than leaving it unresolved. 9. Return to main: `git checkout main && git pull` From 9e2f88381e93130e73bb72763f3268c4bb80a27e Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Wed, 29 Jul 2026 10:19:10 +0200 Subject: [PATCH 10/13] docs(bff): correct six doc-vs-code accuracy defects in the cookie variant set Fixes six CodeRabbit findings on PR #125 where the Variant 3 documentation contradicted the shipped implementation: - architecture.adoc: cookie mode has no *session* store, but PendingAuthorizationStore.InMemory is assembled in both modes - development/bff-cookie.adoc: wire-format sizes now use the codec's own nB notation, so version(1B) no longer reads as format version 1 - user/bff-cookie.adoc: BFF activation is (recognised session.mode AND redirect_uri), never route effective auth - variants/03-bff-cookie.adoc: key-management row reconciled with the optional encryption_key contract - variants/03-bff-cookie.adoc: removed the non-existent cookie-splitting claim; over-budget seals fail - variants/03-bff-cookie.adoc: statelessness claim narrowed to established sessions, cross-referencing the login-affinity subsection --- doc/architecture.adoc | 5 ++++- doc/development/bff-cookie.adoc | 2 +- doc/user/bff-cookie.adoc | 6 ++++-- doc/variants/03-bff-cookie.adoc | 22 +++++++++++++++------- 4 files changed, 24 insertions(+), 11 deletions(-) diff --git a/doc/architecture.adoc b/doc/architecture.adoc index 3ba7f35c..666697c2 100644 --- a/doc/architecture.adoc +++ b/doc/architecture.adoc @@ -189,7 +189,10 @@ link:adr/0019-Reserved_BFF_paths_bypass_the_url-parameter_value_pipeline_and_the header and a `SessionStore` (the in-memory `InMemorySessionStore`, bounded by `oidc.session.max_sessions`) resolves it to the server-held `SessionRecord` carrying the tokens; in *cookie* mode the sealed cookie itself carries that state and is unsealed per request, with no - store at all. Either way a `require: session` route is served by a + *session* store at all -- the in-memory `PendingAuthorizationStore` that holds the login leg + between redirect and callback is assembled in *both* modes + (link:variants/03-bff-cookie.adoc#_stateless_after_login_not_during_it[stateless after login, not + during it]). Either way a `require: session` route is served by a `SessionAuthenticationStage` injected into the session-aware authentication stage -- so the route is *mediated* (the resolved access token injected upstream) rather than rejected. diff --git a/doc/development/bff-cookie.adoc b/doc/development/bff-cookie.adoc index 0986ac07..3d017e3c 100644 --- a/doc/development/bff-cookie.adoc +++ b/doc/development/bff-cookie.adoc @@ -74,7 +74,7 @@ The cookie value is a base64url-encoded *outer envelope*: [source] ---- -version(1) | key-id(1) | GCM nonce(12) | ciphertext | GCM tag(16) +version(1B) | key-id(1B) | GCM nonce(12B) | ciphertext | GCM tag(16B) ---- The `version` and `key-id` bytes are bound into the GCM *associated data*, so tampering with either diff --git a/doc/user/bff-cookie.adoc b/doc/user/bff-cookie.adoc index 57e435b4..ef88cba7 100644 --- a/doc/user/bff-cookie.adoc +++ b/doc/user/bff-cookie.adoc @@ -54,8 +54,10 @@ oidc: refresh: { enabled: true, leeway_seconds: 30, on_failure: reauthenticate } ---- -The gateway ignores the `oidc` block entirely unless a route's effective auth is `require: session`, -so a bearer-only deployment is unaffected by its presence. +The BFF runtime is built from exactly two inputs: a recognised `session.mode` (`server` or `cookie`) +*and* a configured `redirect_uri`. Route auth is not part of that decision -- `require: session` +routes are what *use* the runtime once it exists, not what turns it on. With no recognised +`session.mode` the runtime stays inert, so a bearer-only deployment is unaffected. == Key material diff --git a/doc/variants/03-bff-cookie.adoc b/doc/variants/03-bff-cookie.adoc index 1ca9c9e7..1b807e9d 100644 --- a/doc/variants/03-bff-cookie.adoc +++ b/doc/variants/03-bff-cookie.adoc @@ -67,15 +67,20 @@ and the gateway holds no per-user state. `__Host-` prefix. | Key management -| A single server-side symmetric key, provided by environment indirection - (`${SHERIFF_SESSION_KEY}`), shared across all gateway instances. Key rotation is supported by +| A single server-side symmetric key. `session.encryption_key` is *optional*: declared as an + environment indirection (`${SHERIFF_SESSION_KEY}`) it is shared across all gateway instances and + survives a restart; omitted, the gateway mints a key at boot, so sessions are dropped on every + restart and the key cannot be shared with another replica. Key rotation is supported by the optional, decrypt-only `session.previous_key`: during the grace window cookies sealed with the old key are still accepted and re-sealed with the new key on the next write. | Size -| The cookie must stay within browser limits (~4 KB). If the token set exceeds the budget, - the refresh and/or ID token may be split into their own cookies; oversized IdPs are a - deployment consideration. +| The cookie must stay within browser limits (~4 KB), enforced as a seal-time budget: an + over-budget seal *fails* rather than being silently truncated. Splitting the token set across + several cookies is a deliberate non-goal, so the two available remedies are to reduce the token + set (fewer scopes, a leaner IdP token shape) or to run the + link:02-bff-session.adoc[session-based variant]. Oversized IdP tokens are a deployment + consideration. |=== == Statelessness @@ -84,8 +89,10 @@ Because the encrypted cookie carries all session state: * No session store, no Redis, no database -- the manifest's self-contained requirement holds fully. -* Any gateway instance can serve any request as long as it has the encryption key -- no sticky - sessions, no coordination. +* Any gateway instance can serve any request *of an established session* as long as it has the + encryption key -- no sticky sessions, no coordination. The login leg is the one exception: it + holds per-instance pending state and requires affinity until the callback lands (see + <<_stateless_after_login_not_during_it,Stateless after login, not during it>>). * Horizontal scaling is linear; a node can be added or removed with no session migration. This is the property that distinguishes Variant A from Variant B and the reason it is the @@ -192,6 +199,7 @@ link:../configuration.adoc[the configuration reference] for the authoritative va * *Self-contained* -- instances share only the encryption key. * *Full OIDC RP* -- confidential-client login, transparent refresh, RFC 9470 step-up. +[#_stateless_after_login_not_during_it] === Stateless after login, not during it The variant is stateless once a session is *established*, not for the whole login exchange. The From e6f517613677f1c3279db2546e0099d927e4dcba Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Wed, 29 Jul 2026 10:19:56 +0200 Subject: [PATCH 11/13] docs(claude): point the bot-fix commit step at the mandatory pre-commit gates CodeRabbit finding e23e61 on PR #125: Git Workflow step 6 told the agent to fix-and-commit for a valid bot comment without naming the repository's own commit precondition, so the bot-fix path could read as exempt from the CRITICAL Pre-Commit Process gates. Adds a single cross-referencing clause; the canonical commands stay single-sourced in that section. --- CLAUDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index cf3a90fe..b7f3e75d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -134,7 +134,7 @@ All cuioss repositories have branch protection on `main`. Direct pushes to `main 4. Create a PR: `gh pr create --repo cuioss/API-Sheriff --head <branch-name> --base main --title "<title>" --body "<body>"` 5. Wait for CI and the three automated reviewers — CodeRabbit, Sourcery and PR-Agent (waits until checks complete): `gh pr checks --watch` 6. **Handle the automated review comments from all three reviewers** — CodeRabbit, Sourcery and PR-Agent each comment independently, so treat the union of their comments as the work list. Fetch with `gh api repos/cuioss/API-Sheriff/pulls/<pr-number>/comments` and for each comment, whichever bot authored it: - - If clearly valid and fixable: fix it, commit, push, then reply explaining the fix and resolve the comment + - If clearly valid and fixable: fix it, commit, push, then reply explaining the fix and resolve the comment — a bot-fix commit is a commit, so the **Pre-Commit Process** section above applies unchanged - If disagree or out of scope: reply explaining why, then resolve the comment - If uncertain (not 100% confident): **ask the user** before acting - Every comment MUST get a reply (reason for fix or reason for not fixing) and MUST be resolved From 8483687ad4072e208eebfa7d707d4ba56cbf9540 Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Wed, 29 Jul 2026 10:33:25 +0200 Subject: [PATCH 12/13] fix(bff): reject a blank session nonce in both record contracts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two CodeRabbit findings on PR #125: - 997d43: the FORMAT_VERSION javadoc named GCM authentication as the v1-rejection mechanism. unseal() rejects a foreign version at an explicit guard before the Cipher is constructed, so decryption is never attempted; the associated-data binding is an additional property, not the gate. Javadoc-only correction — unseal()'s control flow and guard order are unchanged. - aae09b: both canonical constructors guarded sessionNonce with a null check only, so the empty string was accepted and would silently degrade the derived session identity back to the colliding pre-nonce shape. SealedSessionPayload now rejects null and blank; SessionRecord keeps the nonce Optional (absent stays the valid server-mode shape) and rejects only a present-but-blank value. Neither rejection message carries the nonce. --- .../bff/cookie/SealedSessionCookieCodec.java | 11 ++-- .../bff/cookie/SealedSessionPayload.java | 21 ++++-- .../gateway/bff/session/SessionRecord.java | 14 +++- .../bff/cookie/CookieSessionBindingTest.java | 65 ++++++++++++++++++- .../bff/cookie/SealedSessionPayloadTest.java | 32 +++++++++ 5 files changed, 133 insertions(+), 10 deletions(-) diff --git a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionCookieCodec.java b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionCookieCodec.java index 95b8c67c..37291c06 100644 --- a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionCookieCodec.java +++ b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionCookieCodec.java @@ -98,10 +98,13 @@ public final class SealedSessionCookieCodec { * The current sealed-cookie format version, bound into the GCM associated data. * <p> * Version {@code 2} carries the nine-field payload that added the per-session nonce keying the - * derived session identity. The bump is a clean break with no migration path: because the - * version byte is bound into the associated data, a version-1 cookie fails GCM authentication - * outright rather than being mis-parsed against the nine-field shape, so pre-existing cookie - * sessions are refused fail-closed and the browser simply re-logs in. + * derived session identity. The bump is a clean break with no migration path: {@link #unseal} + * reads the leading version byte and rejects anything other than {@code FORMAT_VERSION} at an + * explicit gate, before a {@link Cipher} is even constructed — so a version-1 cookie is refused + * outright rather than being mis-parsed against the nine-field shape, and decryption is never + * attempted for it. The version byte is additionally bound into the GCM associated data, so it + * cannot be forged onto a value sealed under a different version either. Pre-existing cookie + * sessions are therefore refused fail-closed and the browser simply re-logs in. */ public static final byte FORMAT_VERSION = 2; diff --git a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionPayload.java b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionPayload.java index 3030bd81..99a72c85 100644 --- a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionPayload.java +++ b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionPayload.java @@ -69,6 +69,14 @@ public record SealedSessionPayload(String accessToken, Optional<String> refreshT /** * Canonical constructor rejecting absent mandatory components and normalizing absent optionals * to {@link Optional#empty()}. + * <p> + * {@code sessionNonce} is additionally rejected when blank: it keys the derived session + * identity, so an empty value would silently degrade that identity back to the colliding + * pre-nonce shape instead of failing. The nonce value itself never reaches the exception + * message. + * + * @throws NullPointerException when a mandatory component is {@code null} + * @throws IllegalArgumentException when {@code sessionNonce} is blank */ public SealedSessionPayload { Objects.requireNonNull(accessToken, "accessToken"); @@ -76,6 +84,9 @@ public record SealedSessionPayload(String accessToken, Optional<String> refreshT Objects.requireNonNull(sub, "sub"); Objects.requireNonNull(loginInstant, "loginInstant"); Objects.requireNonNull(sessionNonce, "sessionNonce"); + if (sessionNonce.isBlank()) { + throw new IllegalArgumentException("sessionNonce must not be blank"); + } refreshToken = Objects.requireNonNullElse(refreshToken, Optional.empty()); sid = Objects.requireNonNullElse(sid, Optional.empty()); acr = Objects.requireNonNullElse(acr, Optional.empty()); @@ -111,8 +122,9 @@ public byte[] encode() { * silently change the derived session identity. * * @param encoded the UTF-8 bytes produced by {@link #encode()} - * @return the decoded payload; empty when the bytes do not carry the expected field shape - * (a defensive guard against a key that authenticates a foreign format) + * @return the decoded payload; empty when the bytes do not carry the expected field shape or + * carry a blank session nonce (a defensive guard against a key that authenticates a + * foreign format) */ public static Optional<SealedSessionPayload> decode(byte[] encoded) { Objects.requireNonNull(encoded, "encoded"); @@ -132,8 +144,9 @@ public static Optional<SealedSessionPayload> decode(byte[] encoded) { Instant.ofEpochSecond(Long.parseLong(decodeField(fields[7]))), decodeField(fields[8]))); } catch (IllegalArgumentException | DateTimeException _) { - // Base64 decode failure, epoch-second parse failure, or an epoch second that parses as a - // long but lies outside Instant's supported range, on bytes that authenticated: a + // Base64 decode failure, epoch-second parse failure, an epoch second that parses as a + // long but lies outside Instant's supported range, or a blank session nonce rejected by + // the canonical constructor, on bytes that authenticated: a // foreign payload format under the same key. DateTimeException is NOT an // IllegalArgumentException, so it must be caught explicitly or it escapes unseal() as an // unhandled request error. Report "no session" rather than propagating. diff --git a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/session/SessionRecord.java b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/session/SessionRecord.java index 0ed2fd64..dc87e5d7 100644 --- a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/session/SessionRecord.java +++ b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/session/SessionRecord.java @@ -72,7 +72,7 @@ * @param acr the authentication context class, empty when absent * @param authTime the IdP authentication instant, empty when absent * @param sessionNonce the per-session nonce keying the cookie-mode derived identity; always empty in - * server mode (see the mode split above) + * server mode (see the mode split above), and never blank when present * @author API Sheriff Team * @since 1.0 */ @@ -88,6 +88,15 @@ public record SessionRecord(String sessionId, String accessToken, Optional<Strin /** * Canonical constructor rejecting absent mandatory components and normalizing absent * optionals to {@link Optional#empty()}. + * <p> + * {@code sessionNonce} stays optional — an <em>absent</em> nonce is valid and is the normal + * server-mode shape. Only a <em>present but blank</em> value is rejected: it keys the + * cookie-mode derived identity, so an empty string would silently degrade that identity back to + * the colliding pre-nonce shape instead of failing. The nonce value itself never reaches the + * exception message. + * + * @throws NullPointerException when a mandatory component is {@code null} + * @throws IllegalArgumentException when {@code sessionNonce} is present but blank */ public SessionRecord { Objects.requireNonNull(sessionId, "sessionId"); @@ -100,6 +109,9 @@ public record SessionRecord(String sessionId, String accessToken, Optional<Strin acr = Objects.requireNonNullElse(acr, Optional.empty()); authTime = Objects.requireNonNullElse(authTime, Optional.empty()); sessionNonce = Objects.requireNonNullElse(sessionNonce, Optional.empty()); + if (sessionNonce.filter(String::isBlank).isPresent()) { + throw new IllegalArgumentException("sessionNonce must not be blank when present"); + } } /** diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/CookieSessionBindingTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/CookieSessionBindingTest.java index 142a525e..9eec0e12 100644 --- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/CookieSessionBindingTest.java +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/CookieSessionBindingTest.java @@ -41,6 +41,8 @@ import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; /** * Tests for {@link CookieSessionBinding} — the stateless seam implementation. Covers the @@ -49,7 +51,9 @@ * than the reset lifetime on a re-seal, the {@code previous_key} rotation (a retired-key cookie * resolves and its next write is sealed under the current key, while withdrawing the retired key * makes it no session), the stable-but-never-emitted session identity, and the - * {@code UNSUPPORTED} IdP-driven destruction capability, and the once-per-process rotation record. + * {@code UNSUPPORTED} IdP-driven destruction capability, the once-per-process rotation record, and + * the {@link SessionRecord} session-nonce contract (absent is the valid server-mode shape; a + * present-but-blank value is refused because it would degrade the derived identity). */ @EnableTestLogger class CookieSessionBindingTest { @@ -410,6 +414,65 @@ void shouldSeparateIdentitiesBySubject() { } } + @Nested + @DisplayName("Session-record nonce contract") + class SessionRecordNonceContract { + + private static SessionRecord.SessionRecordBuilder record() { + return SessionRecord.builder() + .sessionId("ignored-on-bind") + .accessToken(ACCESS_TOKEN) + .idToken(ID_TOKEN) + .sub(SUB) + .expiresAt(LOGIN.plus(TTL)); + } + + @ParameterizedTest + @ValueSource(strings = {"", " ", "\t", "\n", " "}) + @DisplayName("Should reject a present-but-blank session nonce") + void shouldRejectPresentButBlankNonce(String blank) { + SessionRecord.SessionRecordBuilder builder = record().sessionNonce(Optional.of(blank)); + + assertThrows(IllegalArgumentException.class, builder::build, + "a blank nonce would silently degrade the derived identity to the colliding pre-nonce shape"); + } + + @Test + @DisplayName("Should keep the rejected nonce out of the exception message") + void shouldNotLeakNonceIntoRejectionMessage() { + SessionRecord.SessionRecordBuilder builder = record().sessionNonce(Optional.of(" ")); + + IllegalArgumentException rejection = assertThrows(IllegalArgumentException.class, builder::build); + + assertEquals("sessionNonce must not be blank when present", rejection.getMessage()); + } + + @Test + @DisplayName("Should accept an absent session nonce — that is the server-mode shape") + void shouldAcceptAbsentNonce() { + SessionRecord serverMode = record().build(); + + assertTrue(serverMode.sessionNonce().isEmpty(), + "server-mode records carry no nonce and must stay constructible"); + } + + @Test + @DisplayName("Should normalize a null session nonce to empty rather than reject it") + void shouldNormalizeNullNonceToEmpty() { + SessionRecord serverMode = record().sessionNonce(null).build(); + + assertTrue(serverMode.sessionNonce().isEmpty()); + } + + @Test + @DisplayName("Should accept a non-blank session nonce") + void shouldAcceptNonBlankNonce() { + SessionRecord cookieMode = record().sessionNonce(Optional.of("session-nonce-material")).build(); + + assertEquals(Optional.of("session-nonce-material"), cookieMode.sessionNonce()); + } + } + @Nested @DisplayName("Destruction") class Destruction { diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionPayloadTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionPayloadTest.java index 7c278e50..ce100079 100644 --- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionPayloadTest.java +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/SealedSessionPayloadTest.java @@ -31,6 +31,8 @@ import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; /** * Tests for {@link SealedSessionPayload} — the plaintext the cookie-mode codec seals. Covers the @@ -210,6 +212,36 @@ void shouldRejectAbsentMandatoryComponents() { "the session nonce is mandatory — it keys the derived session identity"); } + @ParameterizedTest + @ValueSource(strings = {"", " ", "\t", "\n", " "}) + @DisplayName("Should reject a blank session nonce") + void shouldRejectBlankSessionNonce(String blank) { + assertThrows(IllegalArgumentException.class, () -> new SealedSessionPayload(ACCESS_TOKEN, + Optional.empty(), ID_TOKEN, SUB, Optional.empty(), Optional.empty(), Optional.empty(), + LOGIN, blank), + "a blank nonce would silently degrade the derived identity to the colliding pre-nonce shape"); + } + + @Test + @DisplayName("Should keep the rejected nonce out of the exception message") + void shouldNotLeakNonceIntoRejectionMessage() { + IllegalArgumentException rejection = assertThrows(IllegalArgumentException.class, + () -> new SealedSessionPayload(ACCESS_TOKEN, Optional.empty(), ID_TOKEN, SUB, Optional.empty(), + Optional.empty(), Optional.empty(), LOGIN, " ")); + + assertEquals("sessionNonce must not be blank", rejection.getMessage()); + } + + @Test + @DisplayName("Should decode nothing from authenticated bytes carrying a blank session nonce") + void shouldDecodeNothingFromBlankNonce() { + byte[] blankNonce = wireForm(ACCESS_TOKEN, "", ID_TOKEN, SUB, "", "", "", + Long.toString(LOGIN.getEpochSecond()), ""); + + assertTrue(SealedSessionPayload.decode(blankNonce).isEmpty(), + "the constructor rejection surfaces as no session, never as an escaping exception"); + } + @Test @DisplayName("Should normalize a null optional to empty") void shouldNormalizeNullOptionals() { From 365b30eede7d230836614884966d26a2f6e0cc42 Mon Sep 17 00:00:00 2001 From: cuioss oliver <23139298+cuioss@users.noreply.github.com> Date: Wed, 29 Jul 2026 11:14:10 +0200 Subject: [PATCH 13/13] fix(test): rename the record() helper to baseRecord() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sonar java:S6213 (MAJOR) — 'record' is a Java restricted identifier, so a method named record() trips the restricted-identifier naming rule. The helper was introduced by the SessionRecordNonceContract tests in 8483687. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013EfShpgB9HbUSf9n6zhP9r --- .../gateway/bff/cookie/CookieSessionBindingTest.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/CookieSessionBindingTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/CookieSessionBindingTest.java index 9eec0e12..44e1f403 100644 --- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/CookieSessionBindingTest.java +++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/cookie/CookieSessionBindingTest.java @@ -418,7 +418,7 @@ void shouldSeparateIdentitiesBySubject() { @DisplayName("Session-record nonce contract") class SessionRecordNonceContract { - private static SessionRecord.SessionRecordBuilder record() { + private static SessionRecord.SessionRecordBuilder baseRecord() { return SessionRecord.builder() .sessionId("ignored-on-bind") .accessToken(ACCESS_TOKEN) @@ -431,7 +431,7 @@ private static SessionRecord.SessionRecordBuilder record() { @ValueSource(strings = {"", " ", "\t", "\n", " "}) @DisplayName("Should reject a present-but-blank session nonce") void shouldRejectPresentButBlankNonce(String blank) { - SessionRecord.SessionRecordBuilder builder = record().sessionNonce(Optional.of(blank)); + SessionRecord.SessionRecordBuilder builder = baseRecord().sessionNonce(Optional.of(blank)); assertThrows(IllegalArgumentException.class, builder::build, "a blank nonce would silently degrade the derived identity to the colliding pre-nonce shape"); @@ -440,7 +440,7 @@ void shouldRejectPresentButBlankNonce(String blank) { @Test @DisplayName("Should keep the rejected nonce out of the exception message") void shouldNotLeakNonceIntoRejectionMessage() { - SessionRecord.SessionRecordBuilder builder = record().sessionNonce(Optional.of(" ")); + SessionRecord.SessionRecordBuilder builder = baseRecord().sessionNonce(Optional.of(" ")); IllegalArgumentException rejection = assertThrows(IllegalArgumentException.class, builder::build); @@ -450,7 +450,7 @@ void shouldNotLeakNonceIntoRejectionMessage() { @Test @DisplayName("Should accept an absent session nonce — that is the server-mode shape") void shouldAcceptAbsentNonce() { - SessionRecord serverMode = record().build(); + SessionRecord serverMode = baseRecord().build(); assertTrue(serverMode.sessionNonce().isEmpty(), "server-mode records carry no nonce and must stay constructible"); @@ -459,7 +459,7 @@ void shouldAcceptAbsentNonce() { @Test @DisplayName("Should normalize a null session nonce to empty rather than reject it") void shouldNormalizeNullNonceToEmpty() { - SessionRecord serverMode = record().sessionNonce(null).build(); + SessionRecord serverMode = baseRecord().sessionNonce(null).build(); assertTrue(serverMode.sessionNonce().isEmpty()); } @@ -467,7 +467,7 @@ void shouldNormalizeNullNonceToEmpty() { @Test @DisplayName("Should accept a non-blank session nonce") void shouldAcceptNonBlankNonce() { - SessionRecord cookieMode = record().sessionNonce(Optional.of("session-nonce-material")).build(); + SessionRecord cookieMode = baseRecord().sessionNonce(Optional.of("session-nonce-material")).build(); assertEquals(Optional.of("session-nonce-material"), cookieMode.sessionNonce()); }