feat(bff): add server-session BFF foundation and step-up (V2)#103
Open
cuioss-oliver wants to merge 32 commits into
Open
feat(bff): add server-session BFF foundation and step-up (V2)#103cuioss-oliver wants to merge 32 commits into
cuioss-oliver wants to merge 32 commits into
Conversation
… folds Add the two BFF operator-fold config keys to OidcConfig — a UserInfo block (path, allowedClaims allowlist, defaultView selector) and a Login block (path) — plus a server-mode maxSessions bound on the Session block. Add OIDC/session-section validation rules: user-info and login paths must be absolute gateway paths, default-view claims must lie within the operator allowlist (empty allowlist is the secure closed default), and max_sessions must be positive. Extend the value-object contract and validator test suites for the new records and rules. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Af6Ah5SR43Ug7eG9HN1Yyo
…ng cookie Introduce the D2b transient pre-login state under a new bff.pending package. PendingAuthorizationRecord wraps the engine's FlowContext (state/nonce/PKCE/ redirect_uri) and adds the gateway-only fields the engine leaves open: a short fixed TTL, single-use semantics, and a same-origin-validated post-login return URL. PendingAuthorizationStore is the pluggable storage seam with a bounded, single-use, TTL-expiring in-memory implementation (oldest-eviction DoS guard). BindingCookieCodec sets and reads the short-lived __Host- browser-binding cookie so a callback replayed in a different browser is rejected even with a valid state. Unit tests cover single-use consumption, TTL expiry, bounded eviction, return-URL same-origin validation, and cross-browser rejection. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Af6Ah5SR43Ug7eG9HN1Yyo
Introduce the D3 server-session package. SessionRecord holds the mediated access/refresh/ID tokens plus session metadata (expiry, acr, auth_time, sid, sub); token material never leaves the server and every credential — including the bearer session id — is redacted from toString(). SessionStore is the store contract; InMemorySessionStore is the only implementation, keyed by opaque id with secondary indexes by sid/sub for O(1) back-channel destruction, an absolute TTL enforced lazily plus by a periodic sweep (no per-session timer threads), and a documented max-session bound. SessionCookieCodec sets and reads the hardened __Host- session cookie carrying only the opaque id. Unit tests cover create/resolve/destroy, lazy and swept TTL eviction, O(1) back-channel lookup, the capacity bound, and cookie attribute assertions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Af6Ah5SR43Ug7eG9HN1Yyo
Carve the gateway's reserved OIDC paths out of the proxy route table (D2): ReservedPathRegistry does exact-match, OIDC-host-only resolution consulted ahead of route selection in GatewayEdgeRoute, so a proxy route such as path_prefix: /auth never swallows /auth/callback. CallbackEndpoint parses the raw query via CallbackParameters.parse (BFF-13 duplicate-parameter defence), resolves the browser-bound pending record via state + binding cookie, drives the engine code exchange through a CodeExchange seam, creates the server session, sets the opaque session cookie, clears the single-use binding cookie, and redirects to the recorded return URL. The seam keeps the endpoint decoupled from the deferred confidential-client producer wiring (session runtime) and unit-testable without a live token endpoint. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Af6Ah5SR43Ug7eG9HN1Yyo
LoginFlow orchestrates the browser-facing start of the OIDC auth-code flow (D1/D2/D2b): it drives the engine to build the authorization URL and transaction FlowContext (PKCE/state/nonce owned by the engine, no OAuth leg re-implemented), persists the context as a single-use pending-authorization record, sets the __Host- browser-binding cookie, and redirects to the IdP. The requested post-login return URL is same-origin-validated (falling back to / for a cross-origin or unparseable target) so the eventual redirect is never an open redirect. The engine authorization is reached through an AuthorizationInitiation seam, mirroring the callback endpoint, keeping the flow decoupled from the deferred discovery-metadata wiring and unit-testable without a live IdP. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Af6Ah5SR43Ug7eG9HN1Yyo
Add the SessionAuthenticationStage server-session runtime — the counterpart of the offline bearer validation — and remove the boot-time rejection that RouteRuntimeAssembler used to raise for require:session routes. - SessionAuthenticationStage resolves the opaque __Host- session cookie to a live session, offers it to the single-flight refresh seam (D9 hook), enforces required_scopes against the mediated token (403 SCOPE_MISSING), and records the mediated access token for automatic upstream Authorization: Bearer injection. Unauthenticated navigation is redirected 302 into the auth-code flow; anything else is challenged 401 application/problem+json. - AuthenticationStage dispatches require:session routes to the wired session runtime; an unwired route is a boot-configuration IllegalStateException. - RouteRuntimeAssembler compiles require:session routes like any other route. - ForwardPolicyStage renders the mediated bearer last so it wins over any allow-listed inbound Authorization; PipelineRequest carries the mediated bearer server-side. - Tests: new SessionAuthenticationStageTest; RouteRuntimeAssemblerTest, AuthenticationStageTest, and GatewayEdgeRouteTest migrated off the removed boot-time session rejection.
Fixed, fail-closed CSRF defence for require:session routes (D7) with no disable knob: unsafe methods (everything except GET/HEAD/OPTIONS) must present a trusted Origin (exact full-origin match, scheme+host+port) or, when Origin is absent, a Sec-Fetch-Site: same-origin header; everything else is rejected 403 with a CSRF_REJECTED event. Parameterized tests cover the positive/negative matrix. Co-Authored-By: Claude <noreply@anthropic.com>
Implement both OIDC logout channels for the BFF session mode: - RpInitiatedLogout orchestrates RP-initiated logout — best-effort RFC 7009 token revocation, session-bound state minted into the single-use __Host-sheriff-logout cookie, engine-owned end-session redirect (id_token_hint + exact-match post_logout_redirect_uri as the open-redirect defence), and constant-time state verification on the return leg before landing on final_redirect. - BackchannelLogoutReceiver + LogoutTokenValidator apply the full OIDC back-channel-logout-token claim residual (iss/aud, iat freshness replay guard, backchannel-logout event, nonce-absent, sub/sid present) on the engine-signature-verified token, then destroy affected sessions via the store's O(1) secondary index. - LogoutEndpoint and BackchannelLogoutEndpoint are the framework-agnostic request/response edges over the reserved logout paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Af6Ah5SR43Ug7eG9HN1Yyo
OpenRewrite import-order normalization applied by the pre-commit recipe during D8 verification, touching prior-deliverable bff sources and tests. No behavioral change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Af6Ah5SR43Ug7eG9HN1Yyo
- LogoutTokenValidatorTest exercises the full fail-closed check matrix (iss/aud wrong, iat stale/future/absent, events missing or lacking the backchannel-logout event, nonce present, sub+sid absent) plus the sub-only / sid-only success shapes and plain-string vs array aud. - RpInitiatedLogoutTest drives the real engine EndSessionFlow + PostLogoutRedirectValidator: end-session redirect assembly, the single-use __Host-sheriff-logout state cookie, the state round-trip on the return leg, and the exact post_logout_redirect_uri open-redirect rejection. api-sheriff quality gate + full test suite green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Af6Ah5SR43Ug7eG9HN1Yyo
TokenRefreshCoordinator refreshes the mediated token within its expiry leeway through the engine, single-flighted per session so concurrent requests on one session share one refresh; an IdP rejection or engine refresh-token-family reuse destroys the session so a revoked family can never be mediated from again. StepUpCoordinator parses an upstream RFC 9470 insufficient_user_authentication challenge, attempts silent satisfaction, and otherwise re-drives the auth-code flow with the challenge's elevated acr_values/max_age — persisting a single-use pending-authorization record with a same-origin-validated replay target and a browser-binding cookie. Both coordinators are framework-agnostic and reach the engine through functional-interface seams; no OAuth leg is re-implemented in gateway code. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Af6Ah5SR43Ug7eG9HN1Yyo
Deliverable 10 — cross-cutting BFF observability for require:session routes. - BffLogMessages: DSL-style LogRecord catalogue for the session lifecycle, transparent refresh, CSRF, and logout events on the shared ApiSheriff prefix with disjoint ids 10-14 (INFO) / 110-112 (WARN); records only non-sensitive dispositions, never session ids, subjects, sids, or token material. - EventType: SESSION_CREATED / SESSION_DESTROYED / SESSION_REFRESH_FAILED / BACKCHANNEL_LOGOUT (informational) and LOGOUT_TOKEN_INVALID (400, AUTHENTICATION) for the back-channel logout receiver. - SheriffMetrics: sheriff_session_events_total counter keyed by the EventType name (bounded cardinality), bridging GatewayEventCounter to Micrometer. - GatewayReadinessCheck: mode:server issuer reachability, reusing the JWKS-backed validation health check (reachable / unreachable / unverified). - doc/LogMessages.adoc: documents every new BFF record. Co-Authored-By: Claude <noreply@anthropic.com>
Add the D11 session/user-info reserved gateway endpoint above the session-resolution seam: - UserInfoEndpoint: exact-match reserved path serving a curated identity view (allowlisted claims via the operator default view) plus server-held session metadata (expires_at, auth_time, acr). Source is validated ID-token claims (via the ClaimSource seam) and session metadata only, never raw tokens. No live session yields 401 application/problem+json (never a redirect); 403 is reserved for CSRF / a disallowed-claim request; Cache-Control: no-store on every response. - ClaimAllowlistFilter: operator-owned claim allowlist with a secure closed default (empty allowlist discloses nothing); the curated default view is capped to the allowlist at construction. - gateway.schema.json: mirror the D1 OidcConfig additions (oidc.user_info, oidc.login, oidc.session.max_sessions) so downstream YAML binding validates cleanly under additionalProperties:false. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add the D11 test suite completing the session/user-info deliverable: - UserInfoEndpointTest: 401-not-redirect XHR probe (problem+json, no Location), curated default-view disclosure, allowlist enforcement (a non-allowlisted claim absent from the default view; an explicitly requested disallowed claim rejected 403), full-view capping, no raw token material in any response body, and Cache-Control: no-store on every outcome (200 / 401 / 403). Exercised with a real InMemorySessionStore, the real SessionCookieCodec, and a hand-built ClaimSource lambda — no container, no test double framework. - ClaimAllowlistFilterTest: secure-default-closed (empty allowlist discloses nothing), default-view capping to the allowlist, membership, order-preserving filtering, immutability, and the null-argument contract. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ted short-circuit The sixth reserved gateway path (oidc.login.path) folds onto the D5 LoginFlow and the D2b pending-authorization record: an unauthenticated caller is driven into the auth-code flow (binding cookie set) and returns to the same-origin- validated return URL; an already-authenticated caller short-circuits straight to the validated return URL without a fresh auth-code flow. The return URL is same-origin-validated on both paths, so the login-initiation URL is never an open redirect. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Close the D13 gap: the D1-D12 BFF components were unit-green but never invoked at the live gateway edge. This deliverable (D16) makes them reachable. - ReservedPathRegistry.from now also registers the user_info (D11) and login (D12) reserved endpoints alongside the existing four. - GatewayEdgeRoute dispatches a matched reserved path to its handler (callback/logout/logout-return/back-channel/user-info/login) instead of rendering NO_ROUTE_MATCHED, runs require:session routes through the session-aware AuthenticationStage/SessionAuthenticationStage, and enforces the fixed CSRF defence on unsafe-method session requests. - New BffRuntime holds the wired session stage, CSRF defence, step-up coordinator, and reserved-endpoint handlers, and normalizes the heterogeneous handler outcomes for the edge to render. - New BffRuntimeProducer instantiates the server-mode runtime only when a global oidc block with session.mode=server + redirect_uri is configured (inert bearer-only path otherwise) and binds the token-sheriff-client engine seams (authorize/exchange/refresh/step-up) with lazy OIDC discovery so boot needs no live IdP. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…line Add the k6 session-mediated benchmark script (session_mediated.js) modelled on bearer_proxied.js: it establishes one server session through the gateway's login-initiation reserved path in setup() and replays the opaque session cookie for the whole run, measuring only the steady-state mediation hot path (cookie -> store lookup -> bearer injection -> upstream), never the IdP-bound login handshake. Make K6BenchmarkConverter prefer the session-mediation aspect as the report overview headline when present, so a BFF run leads with the mediation figure that is directly comparable against the bearer (offline-validation) and proxiedStatic (plain-proxy) baselines measured in the same run. Runs without the mediation aspect keep the prior first-benchmark headline unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Grow the reserved-path list in configuration.adoc from four to six (add the oidc.user_info and oidc.login login-initiation fold paths) and document every new/changed OIDC/session key: oidc.login.path, oidc.user_info.path with its allowed_claims disclosure ceiling and default_view, and oidc.session.max_sessions. Record the server-session BFF structural additions in architecture.adoc's Authentication Layer (session-resolution seam, reserved-path registry + live-edge dispatch, engine boundary, and the BffRuntime / BffRuntimeProducer CDI wiring), and add the doc-first login-initiation path, pending-authorization binding cookie, and user-info endpoint to the Variant 2 design doc. Add the operator setup guide (doc/user/bff-session.adoc) and contributor guide (doc/development/bff-session.adoc), with append-only index lines in the user, development, and top-level documentation READMEs. Incidental: the pre-commit formatter normalized continuation-line indentation in BffRuntime.java (record component alignment); included so the tree stays clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Wire the server-mode BFF into the mounted IT gateway config and add the six Bff*IT suites that drive a scripted, browser-less OIDC auth-code flow against the compose Keycloak integration realm (TASK-25, deliverable 13). Config (integration-tests/src/main/docker/sheriff-config): - gateway.yaml: add the global server-mode `oidc` block (session.mode=server, memory store) wiring the confidential integration-client/integration-secret against the integration realm; reserved OIDC paths under /auth on the oidc host (localhost); add the require:session `bff-session` anchor; add the integration-keycloak token_validation issuer so the @GatewayValidator built from this block validates the realm's session tokens. - endpoints/bff-session.yaml: the require:session protected route to the go-httpbin echo upstream (Authorization allow-listed, Cookie stripped) so the mediation ITs have a target. Tests (integration-tests/src/test/java/.../integration): - BffKeycloakLoginFlow: shared cookie-jar flow helper with container-network authority rewrite (keycloak:8443 -> localhost:1443) and disjoint gateway / Keycloak cookie jars. - BffSessionLoginIT, BffSessionMediationIT, BffCsrfIT, BffLogoutIT, BffUserInfoIT, BffLoginInitiationIT. Engine-seam caveats verified as authored: (a) RP-initiated logout asserts local session destruction (the IdP TokenRevocation seam is a best-effort no-op); (b) the back-channel receiver is asserted wired + fail-closed (a real Keycloak-issued logout token round-trip is left to the native+Docker IT run). Verification handoff: `verify -Pintegration-tests -pl integration-tests -am` is a native-image + Docker (Keycloak/go-httpbin) orchestrator-tier build not run in this leaf; `test-compile -pl integration-tests -am` passed. The orchestrator must run the native+Docker IT build to close TASK-25 verification. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
GraalVM initializes static state at image build time by default, which
baked the three BFF records' static final SecureRandom into the image
heap. This both broke the native build ("instance of Random/SplittableRandom
in the image heap") and was a real security defect: a build-time-seeded
SecureRandom yields predictable randomness from a cached seed.
Force runtime initialization via per-class --initialize-at-run-time entries
in quarkus.native.additional-build-args for:
- de.cuioss.sheriff.gateway.bff.pending.PendingAuthorizationRecord
- de.cuioss.sheriff.gateway.bff.session.SessionRecord
- de.cuioss.sheriff.gateway.bff.logout.RpInitiatedLogout
The native+Docker integration stack refused to start with ApiSheriff-200:
oidc.client_secret in gateway.yaml carried a literal value, but
ConfigValidator requires a bare required ${VAR} reference. Change it to
${OIDC_CLIENT_SECRET} and inject the container-side value through the
api-sheriff service environment so EnvSecretResolver resolves it at boot.
The api-sheriff-mtls container overlays only gateway.yaml over the shared
sheriff-config dir, so it must resolve the shared endpoints/ (including the
D13 bff-session endpoint) identically to the primary gateway.yaml. Mirror
the three D13 additions the shared bff-session endpoint needs into the mtls
gateway.yaml: the bff-session anchor, the global oidc block, and the
integration-keycloak token_validation issuer. Add OIDC_CLIENT_SECRET to the
api-sheriff-mtls service so the ${OIDC_CLIENT_SECRET} reference resolves,
matching the primary service. The tls block (mtls) stays the one
intentional difference.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @cuioss-oliver, your pull request is larger than the review limit of 150000 diff characters
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: cuioss/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe pull request adds a server-session BFF runtime with opaque sessions, browser-bound login and callback flows, token mediation and refresh, CSRF enforcement, reserved OIDC endpoints, runtime wiring, configuration validation, integration coverage, benchmarking, and documentation. ChangesServer-session BFF runtime
Estimated code review effort: 5 (Critical) | ~120 minutes 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…OIDC discovery (token-sheriff-client#597) The token-sheriff-client OIDC engine performs its outbound OIDC discovery/token/refresh/logout calls with only the JVM default TrustManager and exposes no per-client TLS-trust seam (token-sheriff-client#597), so the per-issuer tls_profile that validates JWKS does not cover them. Against the self-signed IT Keycloak the BFF login ITs failed PKIX/500. Pass javax.net.ssl.trustStore* as runtime -D args on the native runner command line (a native runner does not honour JAVA_TOOL_OPTIONS) for both the api-sheriff and api-sheriff-mtls services, pointing the JVM default truststore at the already-mounted private-CA PKCS12. Document the mechanism in the operator guide. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api-sheriff/src/main/java/de/cuioss/sheriff/gateway/forward/ForwardPolicyStage.java (1)
93-125: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winMediated-bearer override is not guaranteed case-insensitively.
applyMediatedBearerwrites the fixed key"Authorization", butcopyAllowedHeadersinserts headers under whatever casing the route'sheaders_allowconfig uses — a case-sensitive JavaMap. If arequire: sessionroute'sforward.headers_allownames the header differently (e.g.authorization), the map ends up with two distinct entries for what is semantically one HTTP header, and this class documents (Lines 47-50) that the mediated bearer must deterministically win. Whether it actually does depends entirely on downstream iteration order overMap.copyOf(headers)(which does not preserve insertion order) and the HTTP client's case-insensitive header handling — neither of which this stage controls. That breaks the stated guarantee that "the upstream therefore sees only the mediated bearer" under a plausible config typo rather than an exotic edge case.🐛 Suggested fix
private static void applyMediatedBearer(PipelineRequest request, Map<String, String> headers) { - request.mediatedBearer().ifPresent(token -> headers.put("Authorization", "Bearer " + token)); + request.mediatedBearer().ifPresent(token -> { + headers.keySet().removeIf(name -> name.equalsIgnoreCase("Authorization")); + headers.put("Authorization", "Bearer " + token); + }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api-sheriff/src/main/java/de/cuioss/sheriff/gateway/forward/ForwardPolicyStage.java` around lines 93 - 125, Update applyMediatedBearer and the header-building flow in process so the mediated bearer removes or replaces any existing Authorization header regardless of casing before inserting the canonical Authorization entry. Preserve the documented ordering where the mediated bearer is applied last, ensuring the resulting headers contain only the mediated bearer for session routes.
🧹 Nitpick comments (4)
integration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/BffLoginInitiationIT.java (1)
73-76: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the validated return target.
The test passes for any non-IdP redirect, including an incorrect fallback. Assert that
Locationresolves to/home(and remains same-origin/relative) to cover the short-circuit contract.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@integration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/BffLoginInitiationIT.java` around lines 73 - 76, Update the assertions in BffLoginInitiationIT to validate that the Location header is same-origin or relative and resolves specifically to /home, while retaining the existing checks that it is present and does not redirect through the IdP.api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/reserved/ReservedPathRegistry.java (1)
104-116: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
toPathignores the host of absolute URIs, so an off-hostpost_logout_redirect_uristill reserves that path on the OIDC host.If
post_logout_redirect_uri(orbackchannel_path) is configured as an absolute URI on a different host, only its path component is registered — the gateway then carves that path out of the proxy route table on the OIDC host, shadowing a legitimate proxy route. Consider dropping absolute URIs whose host differs from the redirect-URI host.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/reserved/ReservedPathRegistry.java` around lines 104 - 116, Update ReservedPathRegistry’s toPath handling for absolute post_logout_redirect_uri and backchannel_path values: compare the URI host with the configured redirect-URI host and ignore entries whose hosts differ, while preserving relative paths and same-host absolute URIs. Keep the existing path registration behavior for accepted values.api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/reserved/package-info.java (1)
16-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPackage doc undercounts the reserved paths (says four, but six exist).
The Javadoc enumerates only the callback, logout, logout-return, and back-channel-logout paths as the carve-out set, but this same package (and cohort) also adds
UserInfoEndpointand a login-initiation endpoint, and theReservedEndpointdispatch inBffRuntimeswitches over six cases (CALLBACK,LOGOUT,LOGOUT_RETURN,BACKCHANNEL_LOGOUT,USER_INFO,LOGIN). Since this is the doc that describes the security-relevant carve-out ahead of the proxy route table, it should enumerate all six reserved paths, not four.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/reserved/package-info.java` around lines 16 - 38, Update the package-level Javadoc to describe six gateway-owned reserved paths instead of four, adding the UserInfoEndpoint and login-initiation endpoints to the existing callback, logout, logout-return, and back-channel logout enumeration. Keep the surrounding ReservedPathRegistry and CallbackEndpoint descriptions unchanged.api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/runtime/SessionAuthenticationStage.java (1)
142-155: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSilent cookie truncation risk: only the first
Set-Cookieis ever applied.
challenge.setCookieHeaders().stream().findFirst()discards every cookie beyond the first, and the sink itself (PipelineRequest.responseHeaders(), aMap<String,String>) cannot hold multiple values for the same header name anyway. TodayLoginChallengeonly ever carries the single binding cookie, so this is latent rather than active — but it silently diverges from the reserved-dispatch path (GatewayEdgeRoute.renderReserved), which correctly emits everySet-Cookieas a distinct header line via Vert.x's multi-value header API. If a future change toLoginInitiationneeds to emit a second cookie (e.g. clearing a stale one alongside the new binding cookie), it will be dropped without any error.♻️ Suggested direction
Either bound
LoginChallenge.setCookieHeaders()to a single value (matching current actual usage), or route the redirect challenge's cookies through the same multi-value-capable rendering path used for reserved endpoints instead ofPipelineRequest.responseHeaders().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/runtime/SessionAuthenticationStage.java` around lines 142 - 155, Update challengeUnauthenticated and its redirect response handling so every cookie from LoginChallenge.setCookieHeaders() is preserved, rather than selecting only the first value through the Map-backed responseHeaders sink. Either enforce a single-cookie contract on LoginChallenge.setCookieHeaders() or reuse the multi-value header rendering path used by GatewayEdgeRoute.renderReserved, ensuring future multiple Set-Cookie values are emitted distinctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/logout/LogoutTokenValidator.java`:
- Around line 111-114: Update the events-claim validation in
LogoutTokenValidator so it reads the claim as a JSON object rather than a plain
string, and require the BACKCHANNEL_LOGOUT_EVENT member under events before
accepting the token. Preserve the existing rejection and Optional.empty behavior
when the object claim or required member is absent, ensuring session destruction
only follows structurally valid claims.
In
`@api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/pending/PendingAuthorizationRecord.java`:
- Around line 122-145: Update PendingAuthorizationRecord.sameOrigin to reject
returnUrl values beginning with a backslash, including slash-backslash forms
such as `/\evil.com`, before the gateway-relative path check. Add coverage for
these cases in PendingAuthorizationStoreTest.ReturnUrlValidation and
LoginFlowTest.ReturnUrlGuard, preserving acceptance of valid same-origin paths.
In
`@api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/refresh/TokenRefreshCoordinator.java`:
- Around line 146-161: Remove the successful-path
sessionStore.destroyById(sessionId) call in the rotation flow of
TokenRefreshCoordinator, allowing sessionStore.create(rotated) to replace the
existing record directly. Keep session destruction in the TokenSheriffException
failure path unchanged.
In
`@api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/reserved/BackchannelLogoutEndpoint.java`:
- Around line 102-109: Update the form parsing logic in
BackchannelLogoutEndpoint’s logout-token extraction method to catch
IllegalArgumentException from decode when processing malformed percent escapes
and return Optional.empty(). Preserve the existing behavior for valid parameters
and blank values.
In
`@api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/reserved/LogoutEndpoint.java`:
- Around line 108-113: Update the logout flow around RpInitiatedLogout.initiate
so failures still destroy the local session via sessionStore.destroyById, add
the clearing cookie, and return a safe fallback redirect instead of propagating
the exception. Preserve the existing IdP redirect behavior when initiation
succeeds, and ensure cleanup occurs exactly once.
In
`@api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/runtime/JsonWriter.java`:
- Around line 53-62: Update the Number branch in JsonWriter.write to detect
non-finite Float and Double values before appending them, normalizing or
rejecting NaN and positive/negative infinity so the output remains valid JSON;
preserve existing serialization for finite numbers and other Number types.
In
`@api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRoute.java`:
- Around line 525-544: Update readFormBody in GatewayEdgeRoute to await the
request body with a bounded timeout instead of unconditionally calling
CompletableFuture.get(). Use the existing request/body future flow, catch
timeout failures alongside interrupted and execution failures, restore
interruption where applicable, and return null so stalled or slow reads are
rejected through the existing 400 path.
In
`@api-sheriff/src/main/java/de/cuioss/sheriff/gateway/quarkus/BffRuntimeProducer.java`:
- Around line 300-312: Update originOf to omit the URI’s default port when
constructing the origin: exclude 443 for https and 80 for http, while preserving
non-default ports and existing absolute-URI validation.
In
`@api-sheriff/src/main/java/de/cuioss/sheriff/gateway/quarkus/GatewayReadinessCheck.java`:
- Around line 134-142: Update the server-mode branch in GatewayReadinessCheck so
DATA_ISSUER_REACHABILITY is marked reachable only after probing the configured
BFF OIDC issuer used by BffRuntimeProducer for login discovery. Do not treat
gatewayValidator.get() alone as proof of issuer reachability; perform discovery
against that exact issuer, or report the status as unverified when the validator
is not demonstrably bound to it.
In `@benchmarks/src/main/resources/k6-scripts/session_mediated.js`:
- Around line 31-47: Update the session-mediated benchmark constants to target
the session-gated route via targetUrl('/bff-session/get') instead of
'/secure/get'. Change the default KEYCLOAK_USERNAME to the matching
integration/session OIDC user, while preserving the existing password and
environment-variable overrides.
In `@integration-tests/src/main/docker/sheriff-config-mtls/gateway.yaml`:
- Around line 140-144: Update the OIDC URL configuration in the mTLS gateway
overlay, including redirect_uri, logout.post_logout_redirect_uri, and
csrf.trusted_origins, to use the api-sheriff-mtls host port 10444 instead of the
primary gateway port 10443.
---
Outside diff comments:
In
`@api-sheriff/src/main/java/de/cuioss/sheriff/gateway/forward/ForwardPolicyStage.java`:
- Around line 93-125: Update applyMediatedBearer and the header-building flow in
process so the mediated bearer removes or replaces any existing Authorization
header regardless of casing before inserting the canonical Authorization entry.
Preserve the documented ordering where the mediated bearer is applied last,
ensuring the resulting headers contain only the mediated bearer for session
routes.
---
Nitpick comments:
In
`@api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/reserved/package-info.java`:
- Around line 16-38: Update the package-level Javadoc to describe six
gateway-owned reserved paths instead of four, adding the UserInfoEndpoint and
login-initiation endpoints to the existing callback, logout, logout-return, and
back-channel logout enumeration. Keep the surrounding ReservedPathRegistry and
CallbackEndpoint descriptions unchanged.
In
`@api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/reserved/ReservedPathRegistry.java`:
- Around line 104-116: Update ReservedPathRegistry’s toPath handling for
absolute post_logout_redirect_uri and backchannel_path values: compare the URI
host with the configured redirect-URI host and ignore entries whose hosts
differ, while preserving relative paths and same-host absolute URIs. Keep the
existing path registration behavior for accepted values.
In
`@api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/runtime/SessionAuthenticationStage.java`:
- Around line 142-155: Update challengeUnauthenticated and its redirect response
handling so every cookie from LoginChallenge.setCookieHeaders() is preserved,
rather than selecting only the first value through the Map-backed
responseHeaders sink. Either enforce a single-cookie contract on
LoginChallenge.setCookieHeaders() or reuse the multi-value header rendering path
used by GatewayEdgeRoute.renderReserved, ensuring future multiple Set-Cookie
values are emitted distinctly.
In
`@integration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/BffLoginInitiationIT.java`:
- Around line 73-76: Update the assertions in BffLoginInitiationIT to validate
that the Location header is same-origin or relative and resolves specifically to
/home, while retaining the existing checks that it is present and does not
redirect through the IdP.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: cuioss/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b484f4b3-fd89-4b70-aadb-faefc48f76b7
📒 Files selected for processing (97)
api-sheriff/src/main/java/de/cuioss/sheriff/gateway/auth/AuthenticationStage.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/BffLogMessages.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/csrf/CsrfDefence.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/csrf/package-info.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/login/LoginFlow.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/login/package-info.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/logout/BackchannelLogoutReceiver.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/logout/LogoutTokenValidator.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/logout/RpInitiatedLogout.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/logout/package-info.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/package-info.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/pending/BindingCookieCodec.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/pending/PendingAuthorizationRecord.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/pending/PendingAuthorizationStore.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/pending/package-info.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/refresh/StepUpCoordinator.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/refresh/TokenRefreshCoordinator.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/refresh/package-info.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/reserved/BackchannelLogoutEndpoint.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/reserved/CallbackEndpoint.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/reserved/ClaimAllowlistFilter.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/reserved/LoginInitiationEndpoint.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/reserved/LogoutEndpoint.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/reserved/ReservedPathRegistry.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/reserved/UserInfoEndpoint.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/reserved/package-info.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/runtime/BffRuntime.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/runtime/JsonWriter.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/runtime/SessionAuthenticationStage.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/runtime/package-info.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/session/InMemorySessionStore.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/session/SessionCookieCodec.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/session/SessionRecord.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/session/SessionStore.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/session/package-info.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/model/OidcConfig.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/validation/ConfigValidator.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRoute.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/RouteRuntimeAssembler.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/events/EventType.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/forward/ForwardPolicyStage.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/pipeline/PipelineRequest.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/quarkus/BffRuntimeProducer.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/quarkus/GatewayReadinessCheck.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/quarkus/SheriffMetrics.javaapi-sheriff/src/main/resources/application.propertiesapi-sheriff/src/main/resources/schema/gateway.schema.jsonapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/auth/AuthenticationStageTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/csrf/CsrfDefenceTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/login/LoginFlowTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/logout/LogoutTokenValidatorTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/logout/RpInitiatedLogoutTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/pending/BindingCookieCodecTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/pending/PendingAuthorizationStoreTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/refresh/StepUpCoordinatorTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/refresh/TokenRefreshCoordinatorTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/reserved/CallbackEndpointTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/reserved/ClaimAllowlistFilterTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/reserved/LoginInitiationEndpointTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/reserved/ReservedPathRegistryTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/reserved/UserInfoEndpointTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/runtime/SessionAuthenticationStageTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/session/InMemorySessionStoreTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/session/SessionCookieCodecTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/model/ConfigModelContractTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/config/validation/ConfigValidatorTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/GatewayEdgePipelineTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRouteBffWiringTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRouteTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/RouteRuntimeAssemblerTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/WebSocketRelayStageTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/events/EventTypeTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/quarkus/BffRuntimeProducerTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/quarkus/SheriffMetricsTest.javabenchmarks/src/main/java/de/cuioss/sheriff/gateway/k6/benchmark/K6BenchmarkConverter.javabenchmarks/src/main/resources/k6-scripts/session_mediated.jsbenchmarks/src/test/java/de/cuioss/sheriff/gateway/k6/benchmark/K6BenchmarkConverterTest.javadoc/LogMessages.adocdoc/README.adocdoc/architecture.adocdoc/configuration.adocdoc/development/README.adocdoc/development/bff-session.adocdoc/user/README.adocdoc/user/bff-session.adocdoc/variants/02-bff-session.adocintegration-tests/docker-compose.ymlintegration-tests/src/main/docker/sheriff-config-mtls/gateway.yamlintegration-tests/src/main/docker/sheriff-config/endpoints/bff-session.yamlintegration-tests/src/main/docker/sheriff-config/gateway.yamlintegration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/BffCsrfIT.javaintegration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/BffKeycloakLoginFlow.javaintegration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/BffLoginInitiationIT.javaintegration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/BffLogoutIT.javaintegration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/BffSessionLoginIT.javaintegration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/BffSessionMediationIT.javaintegration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/BffUserInfoIT.java
…ve DslJson deserialization The confidential-client engine parses every back-channel IdP response (OIDC discovery, token endpoint, PAR, userinfo) with a DSL-JSON instance. For each response type the DSL-JSON annotation processor ships a compile-time _<Type>_DslJsonConverter implementing com.dslplatform.json.Configuration, which DSL-JSON's ExternalConverterAnalyzer discovers by reflectively loading the <pkg>._<Name>_DslJsonConverter class, instantiating it, and calling configure(dslJson). That reflective loadClass+newInstance works on the JVM but fails in native (Unable to find reader for provided type ... fallback serialization is not registered) unless the converters are registered for reflection. Add TokenClientDslJsonReflection registering the four generated converters (ProviderMetadata, TokenResponse, ParResponse, UserInfoResponse) plus their DTOs for native reflection. Fixes native OIDC discovery against Keycloak. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…open-redirect, local logout on failure, CSRF port normalization, bounded logout body, benchmark route Applies the valid CodeRabbit findings on PR #103: - LogoutTokenValidator: validate events structurally as a JSON object carrying the back-channel-logout member as a key (reject scalar/array/value-position occurrences). - PendingAuthorizationRecord.sameOrigin: reject backslash-authority return URLs (/\evil.com etc.) that browsers normalize into protocol-relative open redirects. - TokenRefreshCoordinator: drop the redundant pre-create destroyById on the success path (create upserts by id), closing the resolve() miss window. - BackchannelLogoutEndpoint: fail closed to 400 on malformed percent-encoding instead of surfacing a 500. - LogoutEndpoint: always destroy the local session and clear the cookie even when the IdP end-session redirect construction throws. - JsonWriter: normalize non-finite doubles/floats to null so the user-info body is always valid JSON. - GatewayEdgeRoute.readFormBody: bound the back-channel body read with a 5s deadline, fail-closed on timeout. - BffRuntimeProducer.originOf: drop default ports (443/80) so the derived gateway origin matches the browser Origin for CSRF/same-origin checks. - session_mediated.js: target /bff-session/get with an integration-realm user. - sheriff-config-mtls/gateway.yaml: point redirect_uri/post_logout/trusted_origins at this instance's port 10444. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…cated require:session navigations redirect (302) instead of reaching the upstream Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ugs + smells) Fix the SonarCloud gate blocker for PR #103: two java:S3655 reliability BUGs (TokenRefreshCoordinator, LogoutEndpoint) that accessed an Optional value without a guard, plus the 56 new-code code smells, taking new-code findings to zero. Mechanical fixes (restricted-identifier renames, unnamed patterns, single-throw assertThrows lambdas, JSpecify null-safety guards) where a fix is correct; in-code suppressions with rationale where a fix fights the design (S107 wiring holder, S6206 behavior class, S1075 config pointers, S2589 load-bearing Map.get guard, S2925 thread-ordering sleep). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…parsed code preserving BFF-13) and adapt the Keycloak IT flow The engine hardcodes response_mode=form_post in the authorization request, so Keycloak returns a 200 auto-submit form that POSTs the code/state to /auth/callback as an application/x-www-form-urlencoded body rather than a 302 with the code in the query. The reserved callback now accepts POST: the edge reads the form body via the existing bounded read and carries the HTTP method, and BffRuntime.dispatch parses the code/state from the body for a POST callback (the query for a GET), reusing the same CallbackParameters.parse so the BFF-13 duplicate-parameter rejection holds identically. The reserved dispatch already runs before route selection, so the cross-site form_post is never subject to the require:session CSRF guard. The Keycloak IT harness scrapes the 200 form_post action + hidden inputs (tolerant of attribute ordering and &) and replays them to the gateway callback. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… surface CI login page in IT form-scrape failure Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ters null-return Two follow-ups surfaced by CI: - IT harness: the login flow replays authorization/callback URLs that the gateway/IdP already percent-encoded. REST Assured's default urlEncodingEnabled=true re-encodes their literal '+' to %2B, so the scope "openid+profile+email" reached Keycloak as one literal scope -> invalid_scope (form_post error page, "login form action not found"). Disable re-encoding on the gateway() and keycloak() request specs so the URLs go out as-is. - BffRuntime.callbackParameters: replace Objects.requireNonNullElse with an explicit null check so Sonar's dataflow recognizes the non-null return (clears the S2637 @NullMarked new-code reliability bug). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/runtime/SessionAuthenticationStage.java (1)
162-165: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
returnUrl()drops the query string on the post-login redirect target.
canonicalPath()/requestPath()appear to carry the path only, so a request like/report?year=2026redirected into the login challenge returns the user to/reportafter authentication, losing?year=2026. Consider appending the raw query string (mirroringGatewayEdgeRoute#renderQuery) when building the return URL.♻️ Proposed direction
private static String returnUrl(PipelineRequest request) { String canonicalPath = request.canonicalPath(); - return canonicalPath != null ? canonicalPath : request.requestPath(); + String path = canonicalPath != null ? canonicalPath : request.requestPath(); + String query = renderQuery(request.queryParameters()); // reuse/extract shared helper + return query.isEmpty() ? path : path + query; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/runtime/SessionAuthenticationStage.java` around lines 162 - 165, Update returnUrl() in SessionAuthenticationStage to preserve the request’s raw query string when constructing the post-login redirect target. Use the canonicalPath() value when available, fall back to requestPath(), and append the query using the existing GatewayEdgeRoute#renderQuery behavior without altering paths that have no query.api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRoute.java (1)
483-497: 🩺 Stability & Availability | 🔵 TrivialReserved-endpoint traffic is metered under the generic
no_routelabel.
handleReservedPathdispatches beforectx.put(ROUTE_KEY, ...)is ever set (that happens only afterrouteSelectionStage/requireSelectedRoutelater inprocess()), so every login/callback/logout/user-info/backchannel-logout request records againstSheriffMetrics.NO_ROUTEinrecordRequestMetrics/recordError. This conflates normal BFF traffic with genuinely-unmatched requests and removes per-endpoint observability into the new reserved paths. Consider stashing a distinct route label (e.g."reserved:" + kind) viactx.put(ROUTE_KEY, ...)inhandleReservedPath/dispatchReserved.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRoute.java` around lines 483 - 497, Update handleReservedPath and/or dispatchReserved to assign ctx.put(ROUTE_KEY, ...) before reserved-endpoint dispatch or error handling, using a distinct reserved label that includes the matched ReservedEndpoint kind (for example, a “reserved:” prefix). Ensure recordRequestMetrics and recordError use this label for all reserved paths instead of SheriffMetrics.NO_ROUTE, while leaving genuinely unmatched requests on the existing no_route label.api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRouteBffWiringTest.java (1)
256-354: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
formPostRuntime()duplicates the pre-existingactiveRuntime(SessionStore, SessionCookieCodec)wiring.The two helpers build an almost-identical
BffRuntime(sameSessionAuthenticationStage,StepUpCoordinator,LoginFlow,BackchannelLogoutEndpoint,UserInfoEndpoint,LoginInitiationEndpointwiring), differing only in theCallbackEndpointexchange seam and whetherpendingStore/bindingCodecare shared fields vs fresh instances. Consider extracting a shared private helper parameterized by the exchange seam (and optionally the pending store/binding codec) to avoid the ~35-line near-duplicate.♻️ Sketch of a shared helper
- private BffRuntime formPostRuntime() { - CallbackEndpoint callback = new CallbackEndpoint((context, params) -> { - Map<String, ClaimValue> accessClaims = new HashMap<>(); - accessClaims.put(ClaimName.SUBJECT.getName(), ClaimValue.forPlainString(SUBJECT)); - AccessTokenContent access = new AccessTokenContent(accessClaims, RAW_ACCESS_TOKEN); - Map<String, ClaimValue> idClaims = new HashMap<>(); - idClaims.put(ClaimName.SUBJECT.getName(), ClaimValue.forPlainString(SUBJECT)); - IdTokenContent id = new IdTokenContent(idClaims, RAW_ID_TOKEN); - return new AuthorizationCodeFlow.AuthenticationResult(access, id); - }, pendingStore, bindingCodec, sessionStore, codec, Duration.ofHours(1)); - ... (rest duplicated from activeRuntime) - } + private BffRuntime formPostRuntime() { + return wireRuntime(sessionStore, codec, pendingStore, bindingCodec, (context, params) -> { + Map<String, ClaimValue> accessClaims = Map.of(ClaimName.SUBJECT.getName(), ClaimValue.forPlainString(SUBJECT)); + AccessTokenContent access = new AccessTokenContent(accessClaims, RAW_ACCESS_TOKEN); + IdTokenContent id = new IdTokenContent(accessClaims, RAW_ID_TOKEN); + return new AuthorizationCodeFlow.AuthenticationResult(access, id); + }); + }Also applies to: 381-423
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRouteBffWiringTest.java` around lines 256 - 354, Refactor formPostRuntime() and the existing activeRuntime(SessionStore, SessionCookieCodec) wiring to use one shared private BffRuntime factory. Parameterize only the differing CallbackEndpoint exchange seam and, if needed, the pendingStore and bindingCodec instances; preserve each helper’s current stores, codecs, and behavior while removing the duplicated SessionAuthenticationStage, StepUpCoordinator, LoginFlow, logout, backchannel, user-info, and login wiring.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRoute.java`:
- Around line 529-543: Update dispatchReserved and its readFormBody flow to
enforce an explicit small maximum body size before buffering reserved POST
payloads. Apply the cap to BACKCHANNEL_LOGOUT and POST CALLBACK requests, reject
or handle oversized bodies before BffRuntime.ReservedHttpRequest is created, and
preserve the existing behavior for non-body reserved paths.
---
Nitpick comments:
In
`@api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/runtime/SessionAuthenticationStage.java`:
- Around line 162-165: Update returnUrl() in SessionAuthenticationStage to
preserve the request’s raw query string when constructing the post-login
redirect target. Use the canonicalPath() value when available, fall back to
requestPath(), and append the query using the existing
GatewayEdgeRoute#renderQuery behavior without altering paths that have no query.
In
`@api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRoute.java`:
- Around line 483-497: Update handleReservedPath and/or dispatchReserved to
assign ctx.put(ROUTE_KEY, ...) before reserved-endpoint dispatch or error
handling, using a distinct reserved label that includes the matched
ReservedEndpoint kind (for example, a “reserved:” prefix). Ensure
recordRequestMetrics and recordError use this label for all reserved paths
instead of SheriffMetrics.NO_ROUTE, while leaving genuinely unmatched requests
on the existing no_route label.
In
`@api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRouteBffWiringTest.java`:
- Around line 256-354: Refactor formPostRuntime() and the existing
activeRuntime(SessionStore, SessionCookieCodec) wiring to use one shared private
BffRuntime factory. Parameterize only the differing CallbackEndpoint exchange
seam and, if needed, the pendingStore and bindingCodec instances; preserve each
helper’s current stores, codecs, and behavior while removing the duplicated
SessionAuthenticationStage, StepUpCoordinator, LoginFlow, logout, backchannel,
user-info, and login wiring.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: cuioss/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 7f71d40d-2b53-41ea-a481-c94241576124
📒 Files selected for processing (39)
api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/login/LoginFlow.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/logout/LogoutTokenValidator.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/pending/PendingAuthorizationRecord.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/pending/PendingAuthorizationStore.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/refresh/StepUpCoordinator.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/refresh/TokenRefreshCoordinator.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/reserved/BackchannelLogoutEndpoint.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/reserved/CallbackEndpoint.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/reserved/ClaimAllowlistFilter.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/reserved/LoginInitiationEndpoint.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/reserved/LogoutEndpoint.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/reserved/ReservedPathRegistry.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/runtime/BffRuntime.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/runtime/JsonWriter.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/runtime/SessionAuthenticationStage.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/session/InMemorySessionStore.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/session/SessionStore.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/validation/ConfigValidator.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRoute.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/quarkus/BffRuntimeProducer.javaapi-sheriff/src/main/java/de/cuioss/sheriff/gateway/quarkus/TokenClientDslJsonReflection.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/login/LoginFlowTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/logout/LogoutTokenValidatorTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/pending/PendingAuthorizationStoreTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/refresh/StepUpCoordinatorTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/refresh/TokenRefreshCoordinatorTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/reserved/BackchannelLogoutEndpointTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/reserved/CallbackEndpointTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/reserved/ClaimAllowlistFilterTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/reserved/LogoutEndpointTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/runtime/JsonWriterTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/runtime/SessionAuthenticationStageTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/session/InMemorySessionStoreTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/session/SessionCookieCodecTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRouteBffWiringTest.javaapi-sheriff/src/test/java/de/cuioss/sheriff/gateway/quarkus/BffRuntimeProducerTest.javabenchmarks/src/main/resources/k6-scripts/session_mediated.jsintegration-tests/src/main/docker/sheriff-config-mtls/gateway.yamlintegration-tests/src/test/java/de/cuioss/sheriff/gateway/integration/BffKeycloakLoginFlow.java
🚧 Files skipped from review as they are similar to previous changes (28)
- api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/session/SessionCookieCodecTest.java
- api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/session/SessionStore.java
- integration-tests/src/main/docker/sheriff-config-mtls/gateway.yaml
- api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/login/LoginFlow.java
- api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/pending/PendingAuthorizationStore.java
- api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/runtime/JsonWriter.java
- api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/reserved/CallbackEndpointTest.java
- api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/login/LoginFlowTest.java
- api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/refresh/TokenRefreshCoordinatorTest.java
- api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/reserved/BackchannelLogoutEndpoint.java
- api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/session/InMemorySessionStore.java
- api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/logout/LogoutTokenValidator.java
- api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/pending/PendingAuthorizationRecord.java
- api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/reserved/ReservedPathRegistry.java
- api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/refresh/StepUpCoordinator.java
- api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/reserved/ClaimAllowlistFilterTest.java
- api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/reserved/LoginInitiationEndpoint.java
- api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/pending/PendingAuthorizationStoreTest.java
- api-sheriff/src/main/java/de/cuioss/sheriff/gateway/quarkus/BffRuntimeProducer.java
- benchmarks/src/main/resources/k6-scripts/session_mediated.js
- api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/session/InMemorySessionStoreTest.java
- api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/refresh/TokenRefreshCoordinator.java
- api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/reserved/ClaimAllowlistFilter.java
- api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/refresh/StepUpCoordinatorTest.java
- api-sheriff/src/main/java/de/cuioss/sheriff/gateway/config/validation/ConfigValidator.java
- api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/logout/LogoutTokenValidatorTest.java
- api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/reserved/LogoutEndpoint.java
- api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/reserved/CallbackEndpoint.java
Comment on lines
+529
to
+543
| private void dispatchReserved(RoutingContext ctx, PipelineRequest request, ReservedEndpoint kind) { | ||
| String cookieHeader = request.firstHeader(COOKIE_HEADER).orElse(null); | ||
| String method = ctx.request().method().name(); | ||
| // The reserved form body is read for two POST reserved paths: back-channel logout, and an OIDC | ||
| // response_mode=form_post callback (Keycloak POSTs the code/state to redirect_uri as an | ||
| // urlencoded body rather than returning a 302 with the code in the query). Both reuse the same | ||
| // bounded read; every other reserved path (and a GET callback) carries no body. | ||
| boolean callbackFormPost = kind == ReservedEndpoint.CALLBACK && "POST".equalsIgnoreCase(method); | ||
| String rawFormBody = kind == ReservedEndpoint.BACKCHANNEL_LOGOUT || callbackFormPost ? readFormBody(ctx) : null; | ||
| BffRuntime.ReservedHttpRequest reservedRequest = new BffRuntime.ReservedHttpRequest( | ||
| ctx.request().query(), cookieHeader, firstQueryParam(request, CLAIMS_PARAM), | ||
| firstQueryParam(request, RETURN_TO_PARAM), firstQueryParam(request, STATE_PARAM), rawFormBody, method); | ||
| BffRuntime.ReservedHttpResponse response = bffRuntime.dispatch(kind, reservedRequest, Instant.now()); | ||
| renderReserved(ctx, request, response); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "max-body-size|max_body_size|http.limits" api-sheriff/src/main/resources/application.propertiesRepository: cuioss/API-Sheriff
Length of output: 156
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant GatewayEdgeRoute section =="
file="api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRoute.java"
wc -l "$file"
sed -n '480,570p' "$file"
echo
echo "== readFormBody implementation/definition =="
rg -n "readFormBody|firstHeader|firstQueryParam|ReservedHttpRequest|dispatchReserved|backchannel" api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRoute.java
echo
echo "== max-body-size/http limits across repository =="
rg -n "max-body-size|max_body_size|max-body-size|limits/max-body-size|limit" . -g '!**/.git/**' | head -200
echo
echo "== Quarkus/Vert.x dependency references =="
rg -n "quarkus|Vertx|vertx|quarkus-vertx|quarkus-http" api-sheriff/pom.xml pom.xml api-sheriff/src -g '*.properties' -g '*.xml' | head -200
echo
echo "== application properties files =="
fd -a 'application.*properties|application.*yml|application.*yaml' api-sheriff/src/main/resources || true
for f in $(fd 'application.*properties|application.*yml|application.*yaml' api-sheriff/src/main/resources || true); do
echo "--- $f"
wc -l "$f"
sed -n '1,220p' "$f"
doneRepository: cuioss/API-Sheriff
Length of output: 47843
🌐 Web query:
Quarkus 3.37 quarkus.http.limits.max-body-size default BodyHandler vert.x Router buffer body size limit
💡 Result:
In Quarkus 3.37, the default value for the configuration property quarkus.http.limits.max-body-size is 10240K (10 MB) [1][2][3]. This setting serves as a global limit enforced by the Quarkus Vert.x HTTP layer to prevent resource exhaustion from excessively large request bodies [4][5]. When a request exceeds this limit, the server will typically respond with a 413 Request Entity Too Large status code [6]. Regarding the interaction with the Vert.x Router and BodyHandler: 1. Integration: Quarkus uses Vert.x as its underlying HTTP engine [6]. The quarkus.http.limits.max-body-size configuration is designed to be respected by the Quarkus Vert.x HTTP handlers [2][6]. 2. BodyHandler Behavior: While the upstream Vert.x BodyHandler also has a default limit (which is also 10 MB, or 10,485,760 bytes) [7], the Quarkus configuration provides a unified way to manage these limits across the application without needing to manually configure individual Vert.x handlers [1][2]. 3. Buffer Pre-allocation: You may also encounter settings related to how buffers are handled. For example, when using BodyHandler, there is a mechanism to pre-allocate buffers [8][7]. By default, if pre-allocation is not enabled or optimized, the buffer may start at 1 KB and grow dynamically as data is read [8][7]. If pre-allocation is enabled based on the Content-Length header, the buffer is allocated according to that size (often capped, for instance at 64 KB in some Vert.x versions) [8][7]. If you find that the limit is not being respected in a specific context (such as certain legacy Resteasy integrations), it is typically due to how the handler is registered or processed in the request lifecycle, but in standard Quarkus applications, the quarkus.http.limits.max-body-size property is the authoritative setting [1][6].
Citations:
- 1: https://quarkus.io/guides/http-reference
- 2: https://javadoc.io/static/io.quarkus/quarkus-vertx-http/3.3.2/io/quarkus/vertx/http/runtime/ServerLimitsConfig.html
- 3: https://quarkus.io/version/3.2/guides/http-reference
- 4: Set a safer default for quarkus.http.limits.max-body-size quarkusio/quarkus#10079
- 5: https://github.com/quarkusio/quarkus/wiki/Migration-Guide-1.6
- 6: Resteasy Vert.x does not respect max body size quarkusio/quarkus#6590
- 7: https://vertx.io/docs/apidocs/io/vertx/ext/web/handler/BodyHandler.html
- 8: https://javadoc.io/static/io.quarkus/quarkus-vertx-http/3.3.2/io/quarkus/vertx/http/runtime/BodyConfig.html
🌐 Web query:
Quarkus 3.37 vertx-web Router ctx.request().body() max-body-size limit BodyHandler default
💡 Result:
In Quarkus 3.37, the request body size limit is primarily governed by the Quarkus HTTP configuration, not the Vert.x BodyHandler default [1]. The default maximum size for a request body in Quarkus is 10240K (10 MB) [1]. This limit is configured via the property quarkus.http.limits.max-body-size [1]. It is important to distinguish between the two: 1. Quarkus Global Limit: Quarkus enforces a global limit on request body size using the configuration property quarkus.http.limits.max-body-size [1]. This setting applies to the application and defaults to 10240K [1]. If a request exceeds this size, the server typically rejects it before the request reaches your application-level handlers. 2. Vert.x BodyHandler Default: While the underlying Vert.x Web BodyHandler also has a default limit of 10 MB (defined as 10485760 bytes in its DEFAULT_BODY_LIMIT constant) [2][3][4], when you use Quarkus, the framework manages the HTTP server configuration [1][5]. If you are manually adding a BodyHandler to a Vert.x Router within Quarkus, you can override this by explicitly calling setBodyLimit(long) on your BodyHandler instance [2][6]. If you need to change the limit, you should update the Quarkus configuration property in your application.properties file: quarkus.http.limits.max-body-size=20M This will ensure the global limit is respected across your Quarkus application [1].
Citations:
- 1: https://quarkus.io/guides/http-reference
- 2: https://vertx.io/docs/apidocs/io/vertx/ext/web/handler/BodyHandler.html
- 3: https://github.com/vert-x3/vertx-web/blob/master/vertx-web/src/main/java/io/vertx/ext/web/handler/BodyHandler.java
- 4: https://vertx.io/docs/5.0.12/apidocs/io/vertx/ext/web/handler/BodyHandler.html
- 5: https://groups.google.com/g/quarkus-dev/c/6sUhF5ZD3ds
- 6: https://vertx.io/docs/vertx-web/java/
🌐 Web query:
Quarkus 3.37 vertx.io.vertx:vertx-web 4.x ctx.request().body() maxBodySize limit
💡 Result:
In Quarkus, the maximum request body size is primarily controlled by the configuration property quarkus.http.limits.max-body-size [1]. This property is part of the Quarkus HTTP configuration and defaults to 10240K (10 MB) [2][1]. Because Quarkus manages the HTTP server lifecycle and automatically configures underlying Vert.x components, you should generally use the Quarkus configuration property rather than manually setting the setBodyLimit on a Vert.x BodyHandler [3][1]. Key details regarding this limit: 1. Configuration: You can set the limit in your application.properties (or application.yaml) file. The value accepts standard memory size formats (e.g., 10M, 10240K, 100M) [2][1]. Example: quarkus.http.limits.max-body-size=20M 2. Underlying Behavior: When quarkus.http.limits.max-body-size is defined, Quarkus automatically enforces this limit for your application routes [3]. If a request exceeds this size, the server typically returns an HTTP 413 (Request Entity Too Large) status code [4][5]. 3. Vert.x Defaults vs. Quarkus: While the underlying Vert.x BodyHandler has its own DEFAULT_BODY_LIMIT of 10 MB [6][7], Quarkus overrides this behavior through its HTTP recorder infrastructure [3]. By configuring the property in Quarkus, you ensure the global limit is applied consistently across your routes [3]. 4. Practical Recommendation: Do not attempt to override the body limit programmatically within the Vert.x BodyHandler unless you have specific, non-standard requirements that bypass the standard Quarkus HTTP configuration. Relying on quarkus.http.limits.max-body-size is the standard, supported approach for Quarkus applications [1].
Citations:
- 1: https://quarkus.io/guides/http-reference
- 2: https://javadoc.io/static/io.quarkus/quarkus-vertx-http/3.3.2/io/quarkus/vertx/http/runtime/ServerLimitsConfig.html
- 3: https://github.com/quarkusio/quarkus/blob/main/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/VertxHttpRecorder.java
- 4: https://vertx.io/docs/4.4.9/vertx-web/java/
- 5: https://vertx.io/docs/vertx-web/java/
- 6: https://vertx.io/docs/apidocs/io/vertx/ext/web/handler/BodyHandler.html
- 7: https://github.com/vert-x3/vertx-web/blob/master/vertx-web/src/main/java/io/vertx/ext/web/handler/BodyHandler.java
Size-limit the reserved path POST bodies before buffering them.
readFormBody(ctx) only uses a 5s timeout while ctx.request().body() still buffers the payload first. The project does not set a global body limit, so this reserved logout/callback path should apply an explicit endpoint-specific cap consistent with the expected small logout_token or code/state payload before the BFF dispatch.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRoute.java`
around lines 529 - 543, Update dispatchReserved and its readFormBody flow to
enforce an explicit small maximum body size before buffering reserved POST
payloads. Apply the cap to BACKCHANNEL_LOGOUT and POST CALLBACK requests, reject
or handle oversized bodies before BffRuntime.ReservedHttpRequest is created, and
preserve the existing behavior for non-body reserved paths.
…uest The OIDC response_mode=form_post callback (and back-channel logout) POST their parameters as an x-www-form-urlencoded body. The edge pauses every request in handle() before dispatching to a virtual thread; readFormBody then called ctx.request().body() from that virtual thread. body() attaches its collector but does NOT re-arm an explicitly-paused stream, so the read stalled to the 5s deadline and returned null — the callback saw an empty body, reported "missing state", and rejected the login 400. Every full-login IT (BffSessionLoginIT, BffSessionMediationIT, BffUserInfoIT, BffLoginInitiationIT, BffLogoutIT) failed this way once the earlier scope-encoding blocker was cleared. Fix: capture the request's event-loop context in handle() before the pause, and in readFormBody marshal the body() collector + an explicit request.resume() back onto that context — mirroring what the proxy path's pipeTo(send) already does for a streamed upstream body. The buffered body is then delivered and drained. Verified end-to-end against the native stack + Keycloak: callback returns 302 (~18ms, was a 5s timeout) with the session cookie set, and a mediated require:session request with that cookie returns 200. Also revert the gateway() request spec to default URL encoding: it only issues a plain GET and the callback POST, whose code/state/iss body params must be percent-encoded by REST Assured (disabling it corrupted the body -> 400). Only keycloak() replays a pre-encoded URL and keeps urlEncodingEnabled(false). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
BFF Foundation + Variant 2 (Server Session)
Implements
doc/plan/07-bff-server-session.adoc— the BFF foundation, stateful (server-session) shape first, driving the already-wiredtoken-sheriff-clientengine as the OIDC confidential client. The gateway owns only the browser-facing side; the engine owns every OAuth leg.Deliverables (16)
OidcConfig+ConfigValidator(user-info + login fold keys,session.max_sessions),gateway.schema.jsonmirrored.__Host-cookie (single-use, TTL, same-origin return URL, cross-browser callback rejection).__Host-session cookie (in-memory, O(1) back-channelsid/subindex, no per-session timers; token material never in the cookie).CallbackParameters.parse(rawQuery)for BFF-13 duplicate-param defence).require: sessionstage-4 runtime + removal of the boot-time session rejection (asserting tests updated).BffLogMessages,doc/LogMessages.adoc).Cache-Control: no-store.BffRuntime+BffRuntimeProducerCDI producer so reserved paths dispatch to their handlers (not 404),require: sessionruns through the session-awareAuthenticationStage, and the engine seams bind with lazy, boot-safe OIDC discovery. (The ITs exposed that D1–D12 were unit-green but never invoked at the live edge; this deliverable closes that gap.)configuration.adocreserved-path list grown four → six, every new key documented) +architecture.adoc+ operator (doc/user/) + developer (doc/development/).oidcIT config) — see the CI note below.Two real security/config bugs the integration tests forced out
SecureRandomin both the BFF records and thetoken-sheriff-clientengineflowpackage — GraalVM refused to bake it into the native image heap, and it was a genuine defect (a cached seed = predictable randomness). Fixed by forcing those classes/packages to--initialize-at-run-time.oidc.client_secret— moved to a bare required${OIDC_CLIENT_SECRET}env reference perConfigValidator's secret-field rule.Rebase over PLAN-05
This branch is rebased over PLAN-05 (tls-edge, #100) per the plan's second-merger rebase obligation. Conflicts in
GatewayEdgeRoute,LogMessages.adoc, and both doc READMEs were resolved to keep both plans' additions; the mtls IT gateway config was brought back into lockstep with the shared BFF endpoint.MtlsHandshakeIT/TlsPassthroughIT— fails the handshake.git diff origin/main HEADproves the entire TLS-edge / config-loading path is byte-identical to main (this branch changed onlyOidcConfig,ConfigValidator, the schema, andapplication.properties), so this reads as an environment/native-image trait of the local SNI-passthrough stack, not a BFF regression. CI is the authoritative IT environment here.@Observes StartupEventregistration forTlsEdgeProducer, the--initialize-at-run-timepackage-prefix interaction, or topology-alias resolution timing in the merged stack.DiscoveryResolverPKIX,BffRuntimeProducerneeds to wire the engine HTTP client's trust to the gateway's configured truststore.🤖 Generated with Claude Code
Summary by CodeRabbit
require: sessionsupport with secure session cookies, token mediation, single-flight transparent refresh, required-scope enforcement, and CSRF protection.