diff --git a/.github/workflows/demo-client-e2e.yml b/.github/workflows/demo-client-e2e.yml
new file mode 100644
index 00000000..4795b22d
--- /dev/null
+++ b/.github/workflows/demo-client-e2e.yml
@@ -0,0 +1,116 @@
+# The demo SPA's browser end-to-end suite (demo-client), run against the real Keycloak stack.
+#
+# DEDICATED AND OPT-IN BY DESIGN. This workflow is triggered by workflow_dispatch and by pushes to
+# main — deliberately NOT by any pull_request trigger, and deliberately NOT folded into maven.yml or
+# integration-tests.yml. The suite builds a native image, starts containers and downloads a browser
+# toolchain; putting that on the default pull-request path would slow every PR for a signal that is
+# about the demo client, not about the gateway. Keep it that way: if this ever needs to gate a PR,
+# that is a decision to take explicitly, not by adding a trigger here in passing.
+name: Demo Client E2E
+
+on:
+ push:
+ branches: [main]
+ workflow_dispatch:
+
+permissions:
+ contents: read
+
+# One demo run at a time — the suite drives a shared container stack on fixed host ports, so two
+# concurrent runs on the same runner class would collide.
+concurrency:
+ group: ${{ github.workflow }}-${{ github.ref }}
+ cancel-in-progress: false
+
+jobs:
+ demo-client-e2e:
+ name: Demo SPA browser suite (both session modes)
+ runs-on: ubuntu-latest
+ # The native compile is the dominant and most variable term; the container bring-up and the two
+ # Playwright projects together are minutes, not tens of minutes.
+ timeout-minutes: 60
+
+ steps:
+ - name: Harden the runner (Audit all outbound calls)
+ uses: step-security/harden-runner@a90bcbc6539c36a85cdfeb73f7e2f433735f215b # v2.15.0
+ with:
+ egress-policy: audit
+
+ - name: Checkout code
+ uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
+ with:
+ persist-credentials: false
+
+ - name: Set up JDK 25
+ uses: actions/setup-java@be666c2fcd27ec809703dec50e508c2fdc7f6654 # v5.2.0
+ with:
+ java-version: '25'
+ distribution: 'temurin'
+ cache: maven
+
+ # Build the native executable explicitly rather than letting the bring-up script's
+ # build-native-if-needed.sh do it implicitly. Both produce the same artifact, but doing it as
+ # its own step means a native compilation failure is reported as a native compilation failure
+ # instead of surfacing as a container that never became ready.
+ - name: Build the native executable
+ run: |
+ ./mvnw --no-transfer-progress clean package -Pnative -pl api-sheriff -am -DskipTests
+
+ # The e2e-demo profile owns the whole lifecycle: it installs the pinned Node toolchain and the
+ # pinned browser, lints, runs start-dev-environment.sh (which rebuilds the api-sheriff image
+ # from the executable above and brings up the trimmed three-container stack), runs both
+ # Playwright projects, and tears the stack down again in post-integration-test.
+ - name: Run the demo E2E suite
+ run: |
+ ./mvnw --no-transfer-progress verify -Pe2e-demo -pl demo-client
+
+ # THE TEARDOWN SAFETY NET, and it is not redundant with the profile's own post-integration-test
+ # teardown. frontend-maven-plugin's npm goal fails the build IMMEDIATELY on a non-zero npm exit
+ # — unlike maven-failsafe-plugin it records nothing for a later phase and honours no
+ # testFailureIgnore — so a FAILING suite means Maven never reaches post-integration-test and
+ # keycloak, api-sheriff and api-sheriff-cookie are all left running. Nothing else here reclaims
+ # them: the job runs `verify`, not `clean verify`, so the profile's pre-clean teardown never
+ # fires either. `always()` covers the failing-suite path and a cancelled run alike.
+ #
+ # `|| true` is a deliberate guard, not sloppiness. This step runs on EVERY outcome, including
+ # after a green suite whose post-integration-test teardown already emptied the stack — so the
+ # nothing-left-to-stop path is the common one, and it must never turn a green job red.
+ # stop-dev-environment.sh is not contracted to exit 0 on that path: demo-client/pom.xml already
+ # declares successCodes 0 AND 1 on both of its teardown executions for exactly this reason.
+ # (Observed locally on docker compose v2, the nothing-to-stop path does exit 0 — the guard
+ # covers the tolerated 1, it does not assume it.)
+ - name: Tear down the demo stack
+ if: always()
+ run: |
+ ./demo-client/scripts/stop-dev-environment.sh || true
+
+ # Diagnostics, best-effort: the JUnit XML plus Playwright's failure-path traces, screenshots
+ # and videos all land under target/test-results. `always()` because a failing suite is exactly
+ # when they are worth having, and `warn` rather than `error` because an earlier step failing
+ # (a native compile that never produced an image, say) legitimately leaves nothing here — that
+ # is already reported by the step that actually failed, and re-reporting it as a missing
+ # artifact would only obscure the real cause.
+ - name: Upload JUnit results and failure diagnostics
+ if: always()
+ uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0
+ with:
+ name: demo-client-junit-results
+ path: demo-client/target/test-results/
+ retention-days: 30
+ if-no-files-found: warn
+
+ # The documentation screenshot set: one parallel set per session-mode project, captured on the
+ # SUCCESS path at the meaningful states (anonymous, authenticated, full allowlisted view,
+ # claim denied, logged out). This is the artifact the demo exists to produce, so the
+ # assertion is deliberately strict — but scoped to `success()`, where it is meaningful: a
+ # suite that passed and produced no screenshots has silently stopped documenting anything,
+ # and that MUST fail. On a failed suite the set is legitimately incomplete, so the step does
+ # not run at all rather than adding a second red mark to an already-diagnosed failure.
+ - name: Upload documentation screenshots
+ if: success()
+ uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0
+ with:
+ name: demo-client-screenshots
+ path: demo-client/target/screenshots/
+ retention-days: 30
+ if-no-files-found: error
diff --git a/.gitignore b/.gitignore
index e507a5b9..b3e3b441 100644
--- a/.gitignore
+++ b/.gitignore
@@ -20,3 +20,4 @@ benchmarking/doc/templates/data/
.plan/*
!.plan/marshal.json
!.plan/project-architecture/
+node_modules/
diff --git a/.plan/project-architecture/_project.json b/.plan/project-architecture/_project.json
index a39f9dd1..1023fabf 100644
--- a/.plan/project-architecture/_project.json
+++ b/.plan/project-architecture/_project.json
@@ -9,6 +9,8 @@
"api-sheriff": {},
"api-sheriff-parent": {},
"benchmarks": {},
+ "demo-client-maven": {},
+ "demo-client-npm": {},
"documentation": {},
"integration-tests": {}
},
diff --git a/.plan/project-architecture/demo-client-maven/enriched.json b/.plan/project-architecture/demo-client-maven/enriched.json
new file mode 100644
index 00000000..3348eda2
--- /dev/null
+++ b/.plan/project-architecture/demo-client-maven/enriched.json
@@ -0,0 +1,104 @@
+{
+ "best_practices": [],
+ "insights": [],
+ "internal_dependencies": [],
+ "key_dependencies": [],
+ "key_dependencies_reasoning": "",
+ "key_packages": {},
+ "purpose": "e2e-tests",
+ "purpose_reasoning": "packaging=pom with maven.compiler.skip and skipTests true and zero Java sources; the only executions are Playwright browser tests bound to integration-test behind -Pe2e-demo",
+ "responsibility": "Reactor-visible shell for the demo client: a Java-free pom-packaging module that owns the dependency-free demo SPA sources and the Playwright end-to-end suite, and wires the pinned Node/npm toolchain plus the stack bring-up and teardown scripts behind the opt-in e2e-demo profile. A default reactor build of this module is a deliberate no-op - no Node download, no npm install, no container, no artifact - and nothing here ever reaches the production image or the native build.",
+ "responsibility_reasoning": "demo-client/pom.xml description and comments, the e2e-demo profile executions (frontend-maven-plugin install-node-and-npm, npm ci, npm run lint:strict, npm run install-browser, npm run test) and the exec-maven-plugin start/stop-dev-environment bindings",
+ "skills_by_profile": {
+ "implementation": {
+ "defaults": [
+ {
+ "description": "Foundational agent behavior rules (user interaction, tool usage, research, dependency management)",
+ "skill": "plan-marshall:persona-plan-marshall-agent"
+ },
+ {
+ "description": "Language-agnostic code quality, refactoring, and documentation principles",
+ "skill": "plan-marshall:ref-code-quality"
+ },
+ {
+ "description": "marshalld build-server consumption client (submit/wait/ping/preflight)",
+ "skill": "plan-marshall:build-server-client"
+ },
+ {
+ "description": "Core JavaScript standards: ES modules, modern patterns, DOM trust boundaries and XSS prevention, code quality",
+ "skill": "pm-dev-frontend:javascript"
+ },
+ {
+ "description": "ESLint flat config, Prettier and Stylelint configuration and enforcement",
+ "skill": "pm-dev-frontend:lint-config"
+ },
+ {
+ "description": "Modern CSS standards covering responsive design, quality practices and tooling",
+ "skill": "pm-dev-frontend:css"
+ }
+ ]
+ },
+ "module_testing": {
+ "defaults": [
+ {
+ "description": "Foundational agent behavior rules (user interaction, tool usage, research, dependency management)",
+ "skill": "plan-marshall:persona-plan-marshall-agent"
+ },
+ {
+ "description": "Language-agnostic code quality principles (SRP, CQS, complexity, error handling)",
+ "skill": "plan-marshall:ref-code-quality"
+ },
+ {
+ "description": "Language-agnostic testing methodology (AAA, coverage, reliability, determinism)",
+ "skill": "plan-marshall:persona-module-tester"
+ },
+ {
+ "description": "Integration-testing persona for the cross-component browser suite (Playwright)",
+ "skill": "plan-marshall:persona-integration-tester"
+ },
+ {
+ "description": "marshalld build-server consumption client (submit/wait/ping/preflight)",
+ "skill": "plan-marshall:build-server-client"
+ },
+ {
+ "description": "Core JavaScript standards: ES modules, modern patterns, DOM trust boundaries and XSS prevention, code quality",
+ "skill": "pm-dev-frontend:javascript"
+ }
+ ]
+ },
+ "quality": {
+ "defaults": [
+ {
+ "description": "Foundational agent behavior rules (user interaction, tool usage, research, dependency management)",
+ "skill": "plan-marshall:persona-plan-marshall-agent"
+ },
+ {
+ "description": "Language-agnostic code quality, refactoring, and documentation principles",
+ "skill": "plan-marshall:ref-code-quality"
+ },
+ {
+ "description": "Core JavaScript standards: ES modules, modern patterns, DOM trust boundaries and XSS prevention, code quality",
+ "skill": "pm-dev-frontend:javascript"
+ },
+ {
+ "description": "ESLint flat config, Prettier and Stylelint configuration and enforcement",
+ "skill": "pm-dev-frontend:lint-config"
+ }
+ ]
+ },
+ "security": {
+ "defaults": [
+ {
+ "description": "JavaScript security: DOM trust boundaries, XSS sinks, sanitization and Trusted Types",
+ "skill": "pm-dev-frontend:javascript-security"
+ },
+ {
+ "description": "Core JavaScript standards: ES modules, modern patterns, DOM trust boundaries and XSS prevention, code quality",
+ "skill": "pm-dev-frontend:javascript"
+ }
+ ]
+ }
+ },
+ "skills_by_profile_reasoning": "Composed by hand rather than by add-domain because the domain detector keys off build_systems=maven and therefore offers only java, java-cui and general-dev for this module, while every file it owns (SPA app.js, Playwright specs, fixtures, utils, playwright.config.js, eslint.config.js) is JavaScript and it carries no Java at all. general-dev supplies the cross-cutting defaults; the pm-dev-frontend skills supply the surface that actually applies; persona-integration-tester is included under module_testing because the tests here are browser end-to-end specs, not unit tests, so jest-testing does not apply.",
+ "tips": []
+}
\ No newline at end of file
diff --git a/.plan/project-architecture/demo-client-npm/enriched.json b/.plan/project-architecture/demo-client-npm/enriched.json
new file mode 100644
index 00000000..07858f94
--- /dev/null
+++ b/.plan/project-architecture/demo-client-npm/enriched.json
@@ -0,0 +1,100 @@
+{
+ "best_practices": [],
+ "insights": [],
+ "internal_dependencies": [],
+ "key_dependencies": [],
+ "key_dependencies_reasoning": "",
+ "key_packages": {},
+ "purpose": "e2e-tests",
+ "purpose_reasoning": "the module contributes no shipped artifact; its only outputs are Playwright browser test results and lint findings",
+ "responsibility": "npm-side view of the same demo-client directory: owns package.json, the pinned Playwright and ESLint toolchain, the lint:strict script and the browser end-to-end specs that drive the demo SPA against the integration-tests stack in both session modes (server-side and cookie). Installed and executed only from the demo-client-maven e2e-demo profile, never from a default reactor build.",
+ "responsibility_reasoning": "demo-client/package.json scripts and devDependencies, playwright.config.js project matrix, tests/01-04 spec files, eslint.config.js",
+ "skills_by_profile": {
+ "implementation": {
+ "defaults": [
+ {
+ "description": "Core JavaScript development standards covering ES modules, modern patterns, web component patterns, DOM trust boundaries / XSS prevention, and code quality",
+ "skill": "pm-dev-frontend:javascript"
+ },
+ {
+ "description": "Language-agnostic code quality principles (SRP, CQS, complexity, error handling)",
+ "skill": "plan-marshall:ref-code-quality"
+ },
+ {
+ "description": "ESLint, Prettier, and Stylelint configuration and enforcement with systematic fixing",
+ "skill": "pm-dev-frontend:lint-config"
+ },
+ {
+ "description": "Modern CSS standards covering essentials, responsive design, quality practices, and tooling",
+ "skill": "pm-dev-frontend:css"
+ },
+ {
+ "description": "Foundational agent behavior rules (user interaction, tool usage, research, dependency management)",
+ "skill": "plan-marshall:persona-plan-marshall-agent"
+ },
+ {
+ "description": "marshalld build-server consumption client (submit/wait/ping/preflight) \u2014 build-dispatch routes builds through the daemon when the project is registered, else falls back in-process",
+ "skill": "plan-marshall:build-server-client"
+ }
+ ]
+ },
+ "module_testing": {
+ "defaults": [
+ {
+ "description": "Core JavaScript development standards covering ES modules, modern patterns, web component patterns, DOM trust boundaries / XSS prevention, and code quality",
+ "skill": "pm-dev-frontend:javascript"
+ },
+ {
+ "description": "Language-agnostic code quality principles (SRP, CQS, complexity, error handling)",
+ "skill": "plan-marshall:ref-code-quality"
+ },
+ {
+ "description": "Language-agnostic testing methodology (AAA, coverage, reliability, determinism)",
+ "skill": "plan-marshall:persona-module-tester"
+ },
+ {
+ "description": "JavaScript unit testing with Jest, DOM testing, mocking, async patterns",
+ "skill": "pm-dev-frontend:jest-testing"
+ },
+ {
+ "description": "Foundational agent behavior rules (user interaction, tool usage, research, dependency management)",
+ "skill": "plan-marshall:persona-plan-marshall-agent"
+ },
+ {
+ "description": "marshalld build-server consumption client (submit/wait/ping/preflight) \u2014 test-run dispatch routes through the daemon when the project is registered, else falls back in-process",
+ "skill": "plan-marshall:build-server-client"
+ }
+ ]
+ },
+ "quality": {
+ "defaults": [
+ {
+ "description": "Foundational agent behavior rules (user interaction, tool usage, research, dependency management)",
+ "skill": "plan-marshall:persona-plan-marshall-agent"
+ },
+ {
+ "description": "Language-agnostic code quality, refactoring, and documentation principles",
+ "skill": "plan-marshall:ref-code-quality"
+ }
+ ]
+ },
+ "security": {
+ "defaults": [
+ {
+ "description": "Core JavaScript development standards covering ES modules, modern patterns, web component patterns, DOM trust boundaries / XSS prevention, and code quality",
+ "skill": "pm-dev-frontend:javascript"
+ },
+ {
+ "description": "Language-agnostic code quality principles (SRP, CQS, complexity, error handling)",
+ "skill": "plan-marshall:ref-code-quality"
+ },
+ {
+ "description": "JavaScript security \u2014 DOM trust boundaries, XSS sinks, sanitization, and Trusted Types",
+ "skill": "pm-dev-frontend:javascript-security"
+ }
+ ]
+ }
+ },
+ "skills_by_profile_reasoning": "Every file this module owns is JavaScript: the Playwright specs, fixtures, utils, playwright.config.js and eslint.config.js. Optionals are included because lint-config (ESLint flat config behind npm run lint:strict) and css (the SPA stylesheet served from this directory) are both live surfaces.; Cross-cutting agent behaviour, code-quality and build-dispatch skills apply to every module.; The SPA renders identity and claim data into the DOM, so the JavaScript security surface (DOM trust boundaries, XSS sinks, sanitization) applies to this module under the security profile.",
+ "tips": []
+}
\ No newline at end of file
diff --git a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/login/LoginFlow.java b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/login/LoginFlow.java
index bcabc429..93cd3457 100644
--- a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/login/LoginFlow.java
+++ b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/login/LoginFlow.java
@@ -46,6 +46,12 @@
* so the post-login redirect is never an open redirect. The engine authorization is reached through
* the {@link AuthorizationInitiation} seam, keeping the flow decoupled from the confidential-client
* wiring (discovery metadata) and unit-testable without a live IdP.
+ *
+ * Why the gateway overrides the engine here. {@link AuthorizationRequestBuilder}
+ * unconditionally emits {@code response_mode=form_post}, so after a successful IdP login the browser
+ * performs a cross-site POST (the IdP's auto-submit form) to the {@code redirect_uri}. The
+ * short-lived browser-binding cookie minted by {@link BindingCookieCodec} carries
+ * {@code SameSite=Lax}, and a Lax cookie is not sent on a cross-site POST — only on a
+ * top-level GET navigation. The real browser therefore dropped the binding cookie on the callback
+ * leg and every login dead-ended on the
+ * {@code OIDC callback without a browser-binding cookie — rejected} {@code 403} branch. Driving the
+ * request with {@code response_mode=query} makes the callback a {@code 302} top-level GET
+ * navigation, for which {@code SameSite=Lax} is sent, so the binding cookie survives.
+ *
+ * The rejected alternative was {@code SameSite=None} on the binding cookie: that would have weakened
+ * the exact cross-site binding control the cookie exists to provide. No cookie attribute is changed
+ * by this component — {@code __Host-} prefix, {@code Secure}, {@code HttpOnly}, {@code Path=/} and
+ * {@code SameSite=Lax} all stay exactly as they were.
+ *
+ * Accepted tradeoff — the authorization code travels in the URL query string.
+ * {@code response_mode=query} places the {@code code} in the callback URL rather than in a POST
+ * body, which exposes it to the {@code Referer} header, to proxy / CDN and server access logs, and to
+ * browser history. That exposure is the standard reason {@code form_post} is preferred, and it is
+ * accepted here deliberately, by operator decision, because it is what makes the
+ * browser-facing flow work at all. The mitigations, each verified in this codebase rather than
+ * assumed:
+ *
+ *
PKCE is in force. {@link FlowContext} carries a non-optional
+ * {@code PkceChallenge} and the engine builder always emits {@code code_challenge} /
+ * {@code code_challenge_method}; it additionally refuses to start the flow when the provider
+ * does not advertise {@code S256}. A leaked code is therefore not redeemable without the
+ * verifier, which never leaves the gateway.
+ *
The code is single-use and short-lived. The authorization code is redeemed
+ * once at the token endpoint; a replay of the same code fails there. The integration realm
+ * ({@code integration-tests/src/main/docker/keycloak/integration-realm.json}) declares no
+ * {@code accessCodeLifespan} override, so Keycloak's own short default lifetime is what is in
+ * force — the gateway does not widen it.
+ *
The binding cookie plus the {@code state} double-check.
+ * {@code CallbackEndpoint.handle} resolves the pending record by the unguessable id in the
+ * binding cookie and constant-time-compares the returned {@code state}, so a code
+ * replayed from a different browser is rejected {@code 403} even when the code itself is still
+ * live.
+ *
+ * Not overstated: the exposure of the code to intermediaries is real and is not removed by any of
+ * the above — it is bounded by them.
+ *
+ * How the rewrite works. {@link #build} delegates to the engine and then rewrites
+ * only the {@code response_mode} parameter of the returned URL. The rewrite is
+ * parameter-aware (it splits the query into its {@code name=value} pairs rather than substring-
+ * replacing the literal {@code form_post}), it preserves every other authorization parameter
+ * byte-for-byte in its original order and encoding ({@code client_id}, {@code redirect_uri},
+ * {@code scope}, {@code state}, {@code nonce}, {@code code_challenge},
+ * {@code code_challenge_method}, {@code acr_values}, {@code max_age}), and it is idempotent — a URL
+ * that already carries {@code response_mode=query} comes back unchanged.
+ *
+ * The same instance is wired into both engine seams that build an authorization URL — the
+ * {@code AuthorizationCodeFlow} login leg and the {@code StepUpHandler} RFC 9470 re-drive leg — so
+ * the step-up leg cannot keep emitting the broken mode.
+ *
+ * @author API Sheriff Team
+ * @since 1.0
+ */
+public final class QueryResponseModeAuthorizationRequestBuilder extends AuthorizationRequestBuilder {
+
+ private static final CuiLogger LOGGER = new CuiLogger(QueryResponseModeAuthorizationRequestBuilder.class);
+
+ /** The authorization-request parameter this builder rewrites. */
+ private static final String PARAM_RESPONSE_MODE = "response_mode";
+
+ /** The response mode the gateway drives: a {@code 302} top-level GET callback. */
+ private static final String RESPONSE_MODE_QUERY = "query";
+
+ private static final String RESPONSE_MODE_PAIR = PARAM_RESPONSE_MODE + '=' + RESPONSE_MODE_QUERY;
+ private static final char QUERY_START = '?';
+ private static final String PAIR_SEPARATOR = "&";
+ private static final char NAME_VALUE_SEPARATOR = '=';
+
+ /**
+ * Builds the engine's authorization URL and rewrites its {@code response_mode} to {@code query}.
+ *
+ * @param configuration the confidential-client configuration
+ * @param metadata the resolved OIDC provider metadata
+ * @param context the transaction context (state / nonce / PKCE, owned by the engine)
+ * @return the engine's authorization URL with {@code response_mode=query}, every other parameter
+ * preserved verbatim
+ */
+ @Override
+ public String build(ClientConfiguration configuration, ProviderMetadata metadata, FlowContext context) {
+ return withQueryResponseMode(super.build(configuration, metadata, context));
+ }
+
+ /**
+ * Rewrites the {@code response_mode} parameter of an authorization URL to {@code query}, leaving
+ * every other parameter untouched.
+ *
+ * The comparison is against the literal parameter name because the engine form-encodes the query
+ * and {@code response_mode} contains no character that encoding alters. Values are never decoded
+ * and re-encoded: each untouched pair is copied through exactly as the engine emitted it, so no
+ * round-trip can corrupt an already-encoded {@code redirect_uri} or {@code scope}. The method is
+ * idempotent and total — a URL with no query at all, or with an empty query, simply gains the
+ * parameter.
+ *
+ * @param authorizationUrl the engine-built authorization URL
+ * @return the same URL with {@code response_mode=query}
+ */
+ public static String withQueryResponseMode(String authorizationUrl) {
+ Objects.requireNonNull(authorizationUrl, "authorizationUrl");
+ int queryStart = authorizationUrl.indexOf(QUERY_START);
+ if (queryStart < 0) {
+ return authorizationUrl + QUERY_START + RESPONSE_MODE_PAIR;
+ }
+ String prefix = authorizationUrl.substring(0, queryStart + 1);
+ String query = authorizationUrl.substring(queryStart + 1);
+ if (query.isEmpty()) {
+ return prefix + RESPONSE_MODE_PAIR;
+ }
+ List pairs = new ArrayList<>();
+ boolean rewritten = false;
+ for (String pair : query.split(PAIR_SEPARATOR, -1)) {
+ if (PARAM_RESPONSE_MODE.equals(nameOf(pair))) {
+ pairs.add(RESPONSE_MODE_PAIR);
+ rewritten = true;
+ } else {
+ pairs.add(pair);
+ }
+ }
+ if (!rewritten) {
+ pairs.add(RESPONSE_MODE_PAIR);
+ }
+ // Never log the URL itself: it carries state, nonce and the PKCE code_challenge.
+ LOGGER.debug("Authorization request driven with response_mode=%s (rewritten=%s)",
+ RESPONSE_MODE_QUERY, rewritten);
+ return prefix + String.join(PAIR_SEPARATOR, pairs);
+ }
+
+ private static String nameOf(String pair) {
+ int separator = pair.indexOf(NAME_VALUE_SEPARATOR);
+ return separator < 0 ? pair : pair.substring(0, separator);
+ }
+}
diff --git a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/pending/BindingCookieCodec.java b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/pending/BindingCookieCodec.java
index 76c3f8fa..20d09eb2 100644
--- a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/pending/BindingCookieCodec.java
+++ b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/pending/BindingCookieCodec.java
@@ -33,9 +33,19 @@
*
* The cookie is hardened by construction: the {@code __Host-} prefix (which the browser only
* honours with {@code Secure} + {@code Path=/} + no {@code Domain}), plus {@code HttpOnly} (no
- * script access) and {@code SameSite=Lax} (survives the top-level IdP redirect while blocking
- * cross-site sends). The codec is framework-agnostic: it produces and parses raw header values,
- * so it carries no JAX-RS/Vert.x coupling and is unit-testable without a container.
+ * script access) and {@code SameSite=Lax}. The codec is framework-agnostic: it produces and parses
+ * raw header values, so it carries no JAX-RS/Vert.x coupling and is unit-testable without a
+ * container.
+ *
+ * Why {@code SameSite=Lax} is both correct and sufficient here. The gateway drives
+ * the authorization request with {@code response_mode=query} (see
+ * {@link de.cuioss.sheriff.gateway.bff.login.QueryResponseModeAuthorizationRequestBuilder}), so the
+ * callback the IdP sends the browser to is a top-level GET navigation — precisely the
+ * request shape a Lax cookie is sent on, while every cross-site send is still blocked. That is
+ * exactly why the gateway does not need {@code SameSite=None} on this cookie:
+ * {@code None} would have to be paired with the cross-site sends this cookie exists to prevent, so
+ * it would weaken the browser-binding control itself. It was considered and rejected. The mode
+ * choice and this attribute are one design — do not change either in isolation.
*
* @author API Sheriff Team
* @since 1.0
diff --git a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/reserved/CallbackEndpoint.java b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/reserved/CallbackEndpoint.java
index 9eaa6d65..bd3542e0 100644
--- a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/reserved/CallbackEndpoint.java
+++ b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/bff/reserved/CallbackEndpoint.java
@@ -49,18 +49,42 @@
* string and a raw {@code Cookie} header and returns a {@link CallbackOutcome} the edge renders),
* so it carries no JAX-RS/Vert.x coupling and is unit-testable without a container.
*
- * form_post callback. The engine drives the authorization request with
- * {@code response_mode=form_post}, so after a successful login Keycloak returns a 200 auto-submit
- * form that POSTs the {@code code}/{@code state} to the {@code redirect_uri} as an
- * {@code application/x-www-form-urlencoded} body — not a 302 with the code in the query. The edge
- * therefore hands this endpoint the raw form body for a POST callback and the raw query for a GET
- * callback; both are the same urlencoded parameter shape, so a single {@code parse} handles either.
+ * Query-mode callback. The gateway drives the authorization request with
+ * {@code response_mode=query} (see
+ * {@link de.cuioss.sheriff.gateway.bff.login.QueryResponseModeAuthorizationRequestBuilder}), so
+ * after a successful login the IdP answers a {@code 302} that navigates the browser to the
+ * {@code redirect_uri} with the {@code code}/{@code state} in the query string. That top-level GET
+ * navigation is the only callback shape on which the browser sends the {@code SameSite=Lax}
+ * browser-binding cookie the {@code 403} branch below requires; the engine's built-in
+ * {@code response_mode=form_post} produced a cross-site POST instead, on which a Lax cookie is
+ * dropped, so every real-browser login dead-ended there. The edge therefore hands this endpoint the
+ * raw query, and no body is read for the callback path at all.
*
- * Callback HPP defence (BFF-13). The endpoint parses the raw parameter
- * string with {@link CallbackParameters#parse(String)} — never {@link CallbackParameters#of(java.util.Map)}:
- * only the raw parse lets the engine re-detect a duplicated {@code code}/{@code state} (the
- * Keycloak CVE-2026-9689 class), which a collapsed map cannot. A form_post body is parsed by the
- * same {@code parse}, so the duplicate-parameter defence holds identically for the POST body.
+ * Callback HPP defence (BFF-13) — verified on the query path. The endpoint parses
+ * the raw parameter string with {@link CallbackParameters#parse(String)} — never
+ * {@link CallbackParameters#of(java.util.Map)}: only the raw parse lets the engine re-detect a
+ * duplicated {@code code}/{@code state} (the Keycloak CVE-2026-9689 class), which a collapsed map
+ * cannot. The defence was re-verified end to end for the query shape rather than assumed to carry
+ * over: the edge populates {@code ReservedHttpRequest.rawQuery} from the genuinely raw Vert.x
+ * {@code HttpServerRequest.query()} — the untouched request-target query string, not a
+ * first-value-wins projection of the parsed parameter map — and
+ * {@code BffRuntime.callbackParameters} feeds exactly that string to the same {@code parse}. No
+ * stage between the edge and this endpoint collapses, reorders or de-duplicates the query, so a
+ * duplicated {@code code} or {@code state} still reaches {@code parse} and is still rejected
+ * {@code 400}.
+ *
+ * Accepted tradeoff — the code travels in the URL. {@code response_mode=query}
+ * places the authorization code in the callback URL, exposing it to the {@code Referer} header, to
+ * proxy / CDN and server access logs, and to browser history. That exposure is the standard reason
+ * {@code form_post} is preferred; it is accepted here deliberately, by operator decision,
+ * because it is what makes the browser-facing flow work at all. It is bounded — not removed — by
+ * PKCE (the engine always emits {@code code_challenge}/{@code code_challenge_method} and refuses a
+ * provider that does not advertise {@code S256}, so a leaked code is not redeemable without the
+ * gateway-held verifier), by the single-use, short-lived nature of the code at the token endpoint,
+ * and by the binding-cookie + {@code state} double-check this endpoint performs below, which
+ * rejects {@code 403} a code replayed from a different browser even while it is still live. The
+ * full statement of the tradeoff and of how each mitigation was verified lives on
+ * {@link de.cuioss.sheriff.gateway.bff.login.QueryResponseModeAuthorizationRequestBuilder}.
*
* Browser binding (D2b). The pending-authorization record is resolved by the
* unguessable id carried in the {@link BindingCookieCodec browser-binding cookie}, not by the
@@ -125,9 +149,8 @@ public CallbackEndpoint(CodeExchange codeExchange, PendingAuthorizationStore pen
* Handles one OIDC callback: validates the binding, drives the engine exchange, creates the
* session, and returns the browser response.
*
- * @param rawParameters the raw callback parameter string — the query for a GET callback or the
- * {@code application/x-www-form-urlencoded} body for a {@code response_mode=form_post}
- * POST callback (never map-collapsed — BFF-13)
+ * @param rawParameters the raw callback parameter string — the untouched query string of the
+ * {@code response_mode=query} GET callback, never map-collapsed (BFF-13)
* @param cookieHeader the raw request {@code Cookie} header value, may be absent
* @param now the reference instant (TTL anchor for pending resolution and session expiry)
* @return the redirect outcome on success, a {@code 400}/{@code 403} error outcome when the
@@ -275,7 +298,7 @@ public interface CodeExchange {
* {@code state}/{@code nonce}/{@code iss} + token validation, fail-closed.
*
* @param context the pending record's engine transaction context (owns state/nonce/PKCE)
- * @param params the parsed callback parameters (from the raw query — BFF-13)
+ * @param params the parsed callback parameters (from the raw, never map-collapsed query — BFF-13)
* @return the validated access + ID token result
* @throws de.cuioss.sheriff.token.commons.error.TokenSheriffException when the exchange or
* token validation fails (invalid state/nonce, IdP error, signature/claim failure)
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 371495db..a5b34d6c 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
@@ -218,14 +218,18 @@ public ReservedHttpResponse dispatch(ReservedEndpoint kind, ReservedHttpRequest
}
/**
- * Selects the raw parameter string the callback parses. Keycloak drives the OIDC auth-code
- * callback with {@code response_mode=form_post}: after a successful login it returns a 200
- * auto-submit form whose {@code code}/{@code state} are POSTed to the {@code redirect_uri} as an
- * {@code application/x-www-form-urlencoded} body — never a 302 with the code in the query. A POST
- * callback therefore parses the raw form body; a GET callback parses the raw query. Both are the
- * same urlencoded parameter shape, so the single {@code CallbackParameters.parse} the callback
- * uses handles either source and preserves the BFF-13 duplicate-parameter rejection (the collapsed
- * {@code of(Map)} form is never taken).
+ * Selects the raw parameter string the callback parses. The gateway drives the OIDC auth-code
+ * flow with {@code response_mode=query}, so the live callback is a {@code 302}-driven top-level
+ * GET whose {@code code}/{@code state} arrive in the query string — the only shape on which the
+ * browser sends the {@code SameSite=Lax} binding cookie the callback requires. The GET branch
+ * hands the callback the raw query verbatim, never a map-collapsed projection, so the BFF-13
+ * duplicate-parameter rejection holds (the collapsed {@code of(Map)} form is never taken).
+ *
+ * The POST branch is retained as a fail-closed path only. The edge no longer reads a
+ * body for the callback (only back-channel logout keeps that eager reserved-body read), so a
+ * stray {@code POST} to the callback path finds {@code rawFormBody} absent, normalizes to the
+ * empty string here, and is rejected {@code 400} for a missing {@code state} — an honest
+ * rejection, never a {@code 500}.
*/
private static String callbackParameters(ReservedHttpRequest req) {
final String raw = req.isFormPost() ? req.rawFormBody() : req.rawQuery();
@@ -273,19 +277,18 @@ private static T requireNonNull(@Nullable T value) {
* reads {@link #rawFormBody}, the user-info fold reads {@link #claimsParam}, and so on).
*
* @param rawQuery the raw query string (without the leading {@code ?}), never map-collapsed
- * — a GET callback re-parses it to re-detect a duplicated {@code code}/{@code
- * state} (BFF-13)
+ * — the {@code response_mode=query} GET callback re-parses it to re-detect a
+ * duplicated {@code code}/{@code state} (BFF-13)
* @param cookieHeader the raw request {@code Cookie} header value, may be absent
* @param claimsParam the raw {@code claims} selector for the user-info fold, may be absent
- * @param returnUrlParam the raw post-login {@code return_to} target for the login fold, may be absent
+ * @param returnUrlParam the raw post-login {@code returnUrl} target for the login fold, may be absent
* @param stateParam the {@code state} the IdP returned on a logout-return leg, may be absent
* @param rawFormBody the raw {@code application/x-www-form-urlencoded} body — carried for
- * back-channel logout and for a {@code response_mode=form_post} callback POST
- * (the {@code code}/{@code state} arrive here, not in {@link #rawQuery}), may
- * be absent
- * @param httpMethod the request HTTP method; a {@code POST} to the callback is a
- * {@code response_mode=form_post} submission. Absent normalizes to
- * {@code GET} (the CSRF-safe default)
+ * back-channel logout, the one reserved path that still consumes a body. The
+ * {@code response_mode=query} callback carries its {@code code}/{@code state}
+ * in {@link #rawQuery} and no body is read for it, so this is absent there
+ * @param httpMethod the request HTTP method. Absent normalizes to {@code GET} (the CSRF-safe
+ * default), which is also the method of the live query-mode callback
* @author API Sheriff Team
* @since 1.0
*/
@@ -313,9 +316,9 @@ public record ReservedHttpRequest(String rawQuery, @Nullable
}
/**
- * @return {@code true} when this is a {@code POST} — an OIDC {@code response_mode=form_post}
- * callback, whose {@code code}/{@code state} arrive in {@link #rawFormBody} rather than
- * {@link #rawQuery}
+ * @return {@code true} when this is a {@code POST}. The gateway drives
+ * {@code response_mode=query}, so the live callback is never a POST; this selects the
+ * fail-closed body branch for a stray POST to a reserved path instead
*/
public boolean isFormPost() {
return "POST".equalsIgnoreCase(httpMethod);
diff --git a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/EdgeHardeningOptions.java b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/EdgeHardeningOptions.java
index dd1bc2e7..a5ba6a9c 100644
--- a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/EdgeHardeningOptions.java
+++ b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/EdgeHardeningOptions.java
@@ -40,8 +40,8 @@
* the graceful-shutdown wait for in-flight requests to complete on {@code SIGTERM}, and the
* {@linkplain #reservedBodyMaxBytes() reserved-body ceiling} bounds the cumulative bytes the edge
* will buffer for a gateway-terminated reserved POST path (rejected with {@code 413}), so an
- * unauthenticated caller cannot exhaust heap through the pre-authentication callback / back-channel
- * logout paths that never reach the per-route body cap.
+ * unauthenticated caller cannot exhaust heap through the pre-authentication back-channel logout
+ * path that never reaches the per-route body cap.
*
* Transport bounds are fixed; the admission budget is operator-configurable. The
* codec limits above are deliberate secure defaults chosen to keep the abuse surface bounded while
@@ -71,13 +71,13 @@ public class EdgeHardeningOptions implements HttpServerOptionsCustomizer {
private static final int IDLE_TIMEOUT_SECONDS = 60;
/**
- * Ceiling in bytes for a gateway-terminated reserved-path request body (the OIDC
- * {@code response_mode=form_post} callback and the back-channel logout receiver).
+ * Ceiling in bytes for a gateway-terminated reserved-path request body — since the gateway drives
+ * {@code response_mode=query} and its OIDC callback is a bodyless top-level GET, the back-channel
+ * logout receiver is the one path this bounds.
*
- * Derivation — deliberately not a second independent number. Both payloads are
- * small, gateway-terminated inbound units of exactly the same class as the request header block
- * this file already bounds: a form_post callback carries a urlencoded {@code code}/{@code state}
- * pair (hundreds of bytes in practice) and a back-channel logout carries a single compact
+ * Derivation — deliberately not a second independent number. That payload is a
+ * small, gateway-terminated inbound unit of exactly the same class as the request header block
+ * this file already bounds: a back-channel logout carries a single compact
* {@code logout_token} JWT (low single-digit KiB even with a large signing certificate chain).
* The ceiling is therefore defined as {@link #MAX_HEADER_SIZE_BYTES} rather than
* restated as its own literal, so the gateway's single 16 KiB inbound-unit bound cannot drift
@@ -159,9 +159,8 @@ public long drainTimeoutMillis() {
/**
* @return the byte ceiling the edge enforces on a gateway-terminated reserved-path request body
- * (form_post callback / back-channel logout). A request declaring more, or actually
- * streaming more, is rejected {@code 413} — see {@link #RESERVED_BODY_MAX_BYTES} for the
- * derivation
+ * (the back-channel logout receiver). A request declaring more, or actually streaming
+ * more, is rejected {@code 413} — see {@link #RESERVED_BODY_MAX_BYTES} for the derivation
*/
public long reservedBodyMaxBytes() {
return RESERVED_BODY_MAX_BYTES;
diff --git a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRoute.java b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRoute.java
index eedba714..9ada13ef 100644
--- a/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRoute.java
+++ b/api-sheriff/src/main/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRoute.java
@@ -127,8 +127,8 @@
* last} so management / health routes keep working. Each request is admitted under a bounded
* {@linkplain EdgeHardeningOptions#admissionCap() admission cap} before a virtual thread is
* dispatched (a flood is rejected {@code 503} rather than spawning unbounded virtual threads), then
- * the request stream is paused and the whole pipeline runs on a virtual thread (a reserved POST path —
- * the form_post callback and back-channel logout — instead has its small body read on the event loop
+ * the request stream is paused and the whole pipeline runs on a virtual thread (the one body-carrying
+ * reserved POST path — back-channel logout — instead has its small body read on the event loop
* first, under the {@linkplain EdgeHardeningOptions#reservedBodyMaxBytes() reserved-body byte
* ceiling}, then dispatches, so a handler never has to drain a paused stream from a virtual thread):
*
@@ -173,14 +173,14 @@ public class GatewayEdgeRoute {
private static final String CONNECTION_HEADER = "Connection";
private static final String CONNECTION_CLOSE = "close";
private static final String CLAIMS_PARAM = "claims";
- private static final String RETURN_TO_PARAM = "return_to";
+ private static final String RETURN_URL_PARAM = "returnUrl";
private static final String STATE_PARAM = "state";
private static final int SERVICE_UNAVAILABLE = 503;
private static final int INTERNAL_ERROR = 500;
private static final int BAD_GATEWAY = 502;
private static final long DRAIN_POLL_INTERVAL_MILLIS = 50L;
- // Fail-closed deadline for reading a tiny reserved-POST form body (the form_post callback's
- // code/state, or a back-channel logout_token). It bounds a genuinely slow/stalled body so it cannot
+ // Fail-closed deadline for reading a tiny reserved-POST form body (a back-channel logout_token —
+ // the only such body left). It bounds a genuinely slow/stalled body so it cannot
// pin admission indefinitely; it is deliberately generous because the body is read on a shared event
// loop that can be scheduling-starved under CPU contention, and the deadline handler still honours a
// body that has already fully arrived (see readReservedBodyThenDispatch), so a legitimate body is
@@ -189,8 +189,8 @@ public class GatewayEdgeRoute {
/** Per-request {@link RoutingContext} data key holding the resolved metrics route label. */
private static final String ROUTE_KEY = "sheriff.route";
- /** Holds the fully-read {@code application/x-www-form-urlencoded} body of a reserved POST path
- * (form_post callback / back-channel logout), buffered on the event loop in {@link #handle} before
+ /** Holds the fully-read {@code application/x-www-form-urlencoded} body of the one body-carrying
+ * reserved POST path (back-channel logout), buffered on the event loop in {@link #handle} before
* the virtual-thread dispatch so the handler never has to re-arm a paused stream. Left unset on a
* read failure or timeout, so {@link #readFormBody} reads {@code null} and the receiver fails closed
* to {@code 400}. */
@@ -408,7 +408,7 @@ private void handle(RoutingContext ctx) {
recordRequestMetrics(ctx, startNanos);
});
if (needsReservedBodyRead(ctx)) {
- // A reserved POST (form_post callback / back-channel logout) is dispatched on a virtual
+ // The one body-carrying reserved POST (back-channel logout) is dispatched on a virtual
// thread that cannot reliably re-arm a paused request stream. Read the small, gateway-
// terminated body here on its own event loop — the natural Vert.x path — under a bounded
// deadline, stash it, then dispatch. This avoids any paused-stream / cross-thread resume.
@@ -421,9 +421,26 @@ private void handle(RoutingContext ctx) {
/**
* @return {@code true} when the request is a reserved POST path whose {@code x-www-form-urlencoded}
- * body a handler consumes (the form_post callback and back-channel logout). Matched on the
- * raw path against the reserved registry's exact-match set, so only an exact clean reserved
- * path (raw == canonical) triggers the eager body read; every other request pauses as before.
+ * body a handler consumes. Matched on the raw path against the reserved registry's
+ * exact-match set, so only an exact clean reserved path (raw == canonical) triggers the
+ * eager body read; every other request pauses as before.
+ *
+ * Allowlist decision — {@code CALLBACK} was removed, deliberately. The
+ * gateway now drives the authorization request with {@code response_mode=query}, so the
+ * OIDC callback is a top-level GET carrying its {@code code}/{@code state} in the query
+ * string and consuming no body at all. Leaving {@code ReservedEndpoint.CALLBACK} in this
+ * allowlist would not have been inert: it would keep granting any unauthenticated
+ * {@code POST} to the callback path a pre-authentication, pre-pipeline body read of up to
+ * {@link EdgeHardeningOptions#reservedBodyMaxBytes()} before the handler could reject it —
+ * a retained surface with no remaining purpose. The tighter posture was chosen. A stray
+ * {@code POST} to the callback path now takes the ordinary paused-stream path, reaches the
+ * callback with no body, and is rejected {@code 400} for a missing {@code state} (see
+ * {@code BffRuntime.callbackParameters}) — an honest rejection, never a {@code 500}. An
+ * IdP still configured to {@code form_post} to this gateway is therefore no longer
+ * supported, which is intended: the gateway itself selects the mode.
+ *
+ * Back-channel logout is unaffected. It remains a genuinely
+ * body-carrying reserved POST — it stays in this allowlist and its ceiling is unchanged.
*/
private boolean needsReservedBodyRead(RoutingContext ctx) {
if (!"POST".equalsIgnoreCase(ctx.request().method().name())) {
@@ -431,7 +448,7 @@ private boolean needsReservedBodyRead(RoutingContext ctx) {
}
String host = ctx.request().authority() != null ? ctx.request().authority().host() : ctx.request().host();
return reservedPathRegistry.match(host, ctx.request().path())
- .filter(kind -> kind == ReservedEndpoint.CALLBACK || kind == ReservedEndpoint.BACKCHANNEL_LOGOUT)
+ .filter(kind -> kind == ReservedEndpoint.BACKCHANNEL_LOGOUT)
.isPresent();
}
@@ -441,7 +458,7 @@ private boolean needsReservedBodyRead(RoutingContext ctx) {
* the stream drains on its own event loop, fully asynchronously — no virtual-thread {@code .get()}
* blocks on a contended event loop.
*
- * Two bounds, both mandatory. These two reserved paths are read
+ * Two bounds, both mandatory. The back-channel logout path is read
* pre-authentication and before {@code basicChecksStage} /
* {@code thoroughChecksStage} run, so the per-route {@link SecurityConfiguration#maxBodySize()} cap
* that bounds every ordinary proxied route can never apply here, and the transport's
@@ -782,15 +799,15 @@ private void handleGatewayRejection(RoutingContext ctx, @Nullable PipelineReques
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;
+ // Back-channel logout is the ONE reserved path that still consumes a request body, so it is the
+ // only kind that reads the eagerly buffered body here (and the only kind needsReservedBodyRead
+ // buffers one for). The OIDC callback carries its code/state in the query under
+ // response_mode=query and is handed ctx.request().query() below — the genuinely raw, never
+ // map-collapsed query string the BFF-13 duplicate-parameter defence re-parses.
+ String rawFormBody = kind == ReservedEndpoint.BACKCHANNEL_LOGOUT ? 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);
+ firstQueryParam(request, RETURN_URL_PARAM), firstQueryParam(request, STATE_PARAM), rawFormBody, method);
BffRuntime.ReservedHttpResponse response = bffRuntime.dispatch(kind, reservedRequest, Instant.now());
renderReserved(ctx, request, response);
}
@@ -831,8 +848,8 @@ private static void applyStageSetCookies(HttpServerResponse response, List
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 731fa81c..19e4be18 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
@@ -34,6 +34,7 @@
import de.cuioss.sheriff.gateway.bff.cookie.SealedSessionCookieCodec;
import de.cuioss.sheriff.gateway.bff.csrf.CsrfDefence;
import de.cuioss.sheriff.gateway.bff.login.LoginFlow;
+import de.cuioss.sheriff.gateway.bff.login.QueryResponseModeAuthorizationRequestBuilder;
import de.cuioss.sheriff.gateway.bff.logout.BackchannelLogoutReceiver;
import de.cuioss.sheriff.gateway.bff.logout.LogoutTokenValidator;
import de.cuioss.sheriff.gateway.bff.logout.RpInitiatedLogout;
@@ -63,6 +64,9 @@
import de.cuioss.sheriff.token.client.discovery.DiscoveryResolver;
import de.cuioss.sheriff.token.client.discovery.ProviderMetadata;
import de.cuioss.sheriff.token.client.flow.AuthorizationCodeFlow;
+import de.cuioss.sheriff.token.client.flow.AuthorizationRequestBuilder;
+import de.cuioss.sheriff.token.client.flow.CallbackHandler;
+import de.cuioss.sheriff.token.client.flow.IssValidator;
import de.cuioss.sheriff.token.client.flow.RefreshFlow;
import de.cuioss.sheriff.token.client.flow.StepUpHandler;
import de.cuioss.sheriff.token.client.flow.TokenEndpointClient;
@@ -100,6 +104,12 @@
* {@code #exchange} for login and callback, {@code RefreshFlow#refresh} for transparent refresh, and
* {@code StepUpHandler#initiate} for RFC 9470 re-drive — so the engine is reached at runtime.
*
+ * Response mode. Both authorization-URL seams are wired with the gateway-owned
+ * {@link QueryResponseModeAuthorizationRequestBuilder}, so the flow is driven with
+ * {@code response_mode=query} and the callback is a top-level GET the browser sends the
+ * {@code SameSite=Lax} binding cookie on. See that class for the reasoning and for the accepted
+ * code-in-the-URL tradeoff.
+ *
+ * The rewrite is the reason the OIDC callback is a top-level GET navigation, which is in turn the
+ * only request shape the browser sends the {@code SameSite=Lax} binding cookie on. Two properties
+ * therefore matter and are pinned here: the mode really does become {@code query}, and
+ * nothing else in the URL is disturbed — a rewrite that silently re-encoded
+ * {@code redirect_uri}, dropped {@code code_challenge} or reordered the query would break the flow
+ * in ways no response-mode assertion alone would catch.
+ *
+ * The static {@link QueryResponseModeAuthorizationRequestBuilder#withQueryResponseMode(String)} is
+ * exercised directly rather than through {@code build(..)}: the engine's superclass needs live
+ * client configuration and provider metadata, while the rewrite — the part this class actually owns
+ * — is a total function on the URL string. Testing it directly is what makes the parameter-survival
+ * assertions expressible against a URL carrying every parameter at once.
+ */
+@DisplayName("QueryResponseModeAuthorizationRequestBuilder — response_mode rewrite")
+class QueryResponseModeAuthorizationRequestBuilderTest {
+
+ private static final String AUTHORIZE = "https://idp.example.com/realms/integration/protocol/openid-connect/auth";
+
+ /**
+ * A representative engine-built authorization URL: every parameter the engine emits, in the
+ * engine's order, with {@code redirect_uri} and {@code scope} already form-encoded — the two
+ * values a careless decode/re-encode round-trip would corrupt.
+ */
+ private static String engineUrl(String responseModePair) {
+ List pairs = new ArrayList<>(List.of(
+ "response_type=code",
+ "client_id=api-sheriff",
+ "redirect_uri=https%3A%2F%2Fgw.example.com%2Fauth%2Fcallback",
+ "scope=openid+profile+email",
+ "state=Xy7-state_value",
+ "nonce=Nn9-nonce_value",
+ "code_challenge=E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM",
+ "code_challenge_method=S256"));
+ if (responseModePair != null) {
+ pairs.add(responseModePair);
+ }
+ return AUTHORIZE + "?" + String.join("&", pairs);
+ }
+
+ /** Splits a URL's query into ordered name → value pairs, without decoding either side. */
+ private static Map queryPairs(String url) {
+ Map pairs = new LinkedHashMap<>();
+ String query = URI.create(url).getRawQuery();
+ if (query == null || query.isEmpty()) {
+ return pairs;
+ }
+ for (String pair : query.split("&", -1)) {
+ int separator = pair.indexOf('=');
+ if (separator < 0) {
+ pairs.put(pair, "");
+ } else {
+ pairs.put(pair.substring(0, separator), pair.substring(separator + 1));
+ }
+ }
+ return pairs;
+ }
+
+ @Nested
+ @DisplayName("The response mode becomes query")
+ class ResponseMode {
+
+ @Test
+ @DisplayName("Should rewrite response_mode=form_post to response_mode=query")
+ void shouldRewriteFormPostToQuery() {
+ String rewritten = QueryResponseModeAuthorizationRequestBuilder
+ .withQueryResponseMode(engineUrl("response_mode=form_post"));
+
+ assertAll("the engine's built-in form_post is replaced, not merely appended to",
+ () -> assertEquals("query", queryPairs(rewritten).get("response_mode"),
+ "the gateway drives response_mode=query"),
+ () -> assertFalse(rewritten.contains("form_post"),
+ "no form_post value may survive anywhere in the URL: " + rewritten));
+ }
+
+ @Test
+ @DisplayName("Should add response_mode=query when the engine emitted no response_mode at all")
+ void shouldAddResponseModeWhenAbsent() {
+ String rewritten = QueryResponseModeAuthorizationRequestBuilder
+ .withQueryResponseMode(engineUrl(null));
+
+ assertEquals("query", queryPairs(rewritten).get("response_mode"),
+ "the mode is asserted explicitly rather than left to the response_type=code default");
+ }
+
+ @Test
+ @DisplayName("Should be idempotent — applying the rewrite twice yields the same URL")
+ void shouldBeIdempotent() {
+ String once = QueryResponseModeAuthorizationRequestBuilder
+ .withQueryResponseMode(engineUrl("response_mode=form_post"));
+
+ String twice = QueryResponseModeAuthorizationRequestBuilder.withQueryResponseMode(once);
+
+ assertEquals(once, twice, "a URL already carrying response_mode=query comes back unchanged");
+ }
+
+ @Test
+ @DisplayName("Should rewrite only the response_mode parameter, never a lookalike name")
+ void shouldNotRewriteALookalikeParameterName() {
+ String url = AUTHORIZE + "?response_mode_hint=form_post&response_mode=form_post";
+
+ String rewritten = QueryResponseModeAuthorizationRequestBuilder.withQueryResponseMode(url);
+
+ assertAll("the rewrite matches on the whole parameter NAME, not on a substring",
+ () -> assertEquals("form_post", queryPairs(rewritten).get("response_mode_hint"),
+ "a differently-named parameter that merely starts the same is untouched"),
+ () -> assertEquals("query", queryPairs(rewritten).get("response_mode")));
+ }
+ }
+
+ @Nested
+ @DisplayName("Every other authorization parameter survives verbatim")
+ class ParameterSurvival {
+
+ @ParameterizedTest(name = "{0} survives the rewrite byte-for-byte")
+ @ValueSource(strings = {"response_type", "client_id", "redirect_uri", "scope", "state", "nonce",
+ "code_challenge", "code_challenge_method"})
+ @DisplayName("Should preserve each authorization parameter's value exactly")
+ void shouldPreserveParameterValue(String parameter) {
+ String original = engineUrl("response_mode=form_post");
+
+ String rewritten = QueryResponseModeAuthorizationRequestBuilder.withQueryResponseMode(original);
+
+ assertEquals(queryPairs(original).get(parameter), queryPairs(rewritten).get(parameter),
+ parameter + " must be copied through exactly as the engine emitted it — values are never "
+ + "decoded and re-encoded, so no round-trip can corrupt an already-encoded value");
+ }
+
+ @Test
+ @DisplayName("Should preserve acr_values and max_age when the step-up leg supplies them")
+ void shouldPreserveStepUpParameters() {
+ String original = engineUrl("response_mode=form_post")
+ + "&acr_values=urn%3Amace%3Aincommon%3Aiap%3Asilver&max_age=0";
+
+ String rewritten = QueryResponseModeAuthorizationRequestBuilder.withQueryResponseMode(original);
+
+ assertAll("the RFC 9470 step-up re-drive shares this builder, so its parameters matter too",
+ () -> assertEquals("urn%3Amace%3Aincommon%3Aiap%3Asilver", queryPairs(rewritten).get("acr_values")),
+ () -> assertEquals("0", queryPairs(rewritten).get("max_age")),
+ () -> assertEquals("query", queryPairs(rewritten).get("response_mode")));
+ }
+
+ @Test
+ @DisplayName("Should preserve the parameter order and the authorization endpoint itself")
+ void shouldPreserveOrderAndEndpoint() {
+ String original = engineUrl("response_mode=form_post");
+
+ String rewritten = QueryResponseModeAuthorizationRequestBuilder.withQueryResponseMode(original);
+
+ assertAll("only the response_mode VALUE changes; the URL's shape does not",
+ () -> assertTrue(rewritten.startsWith(AUTHORIZE + "?"),
+ "the authorization endpoint is untouched: " + rewritten),
+ () -> assertEquals(List.copyOf(queryPairs(original).keySet()),
+ List.copyOf(queryPairs(rewritten).keySet()),
+ "the parameter names keep their original order — the rewrite is in-place"));
+ }
+ }
+
+ @Nested
+ @DisplayName("Total over degenerate URL shapes")
+ class DegenerateShapes {
+
+ @Test
+ @DisplayName("Should append the parameter to a URL carrying no query at all")
+ void shouldAppendToUrlWithoutQuery() {
+ String rewritten = QueryResponseModeAuthorizationRequestBuilder.withQueryResponseMode(AUTHORIZE);
+
+ assertEquals(AUTHORIZE + "?response_mode=query", rewritten);
+ }
+
+ @Test
+ @DisplayName("Should append the parameter to a URL whose query is empty")
+ void shouldAppendToUrlWithEmptyQuery() {
+ String rewritten = QueryResponseModeAuthorizationRequestBuilder.withQueryResponseMode(AUTHORIZE + "?");
+
+ assertEquals(AUTHORIZE + "?response_mode=query", rewritten);
+ }
+
+ @Test
+ @DisplayName("Should reject a null authorization URL")
+ void shouldRejectNullUrl() {
+ assertThrows(NullPointerException.class,
+ () -> QueryResponseModeAuthorizationRequestBuilder.withQueryResponseMode(null));
+ }
+ }
+}
diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/pending/BindingCookieCodecTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/pending/BindingCookieCodecTest.java
index e170b027..9e62c788 100644
--- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/pending/BindingCookieCodecTest.java
+++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/pending/BindingCookieCodecTest.java
@@ -15,7 +15,9 @@
*/
package de.cuioss.sheriff.gateway.bff.pending;
+import static org.junit.jupiter.api.Assertions.assertAll;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.time.Duration;
@@ -54,6 +56,46 @@ void shouldEmitHardenedCookie() {
assertTrue(header.contains("; Max-Age=300"), header);
}
+ /**
+ * A regression fence around the alternative remedy that was CONSIDERED AND REJECTED.
+ *
+ * The gateway drives {@code response_mode=query} precisely so the OIDC callback is a
+ * top-level GET navigation — the request shape a {@code SameSite=Lax} cookie IS sent on.
+ * The alternative was to leave the flow on {@code response_mode=form_post} (a cross-site
+ * POST, on which a Lax cookie is dropped) and relax this cookie to {@code SameSite=None}
+ * instead. That would have had to permit exactly the cross-site sends this cookie exists to
+ * prevent, weakening the browser-binding control itself, so it was rejected.
+ *
+ * This test asserts the rejection is still in force. It is deliberately expressed as an
+ * explicit absence check rather than left implicit in the positive assertions: a
+ * future edit that appended {@code SameSite=None} without removing {@code SameSite=Lax}
+ * would still satisfy a `contains("SameSite=Lax")` assertion while shipping the weaker
+ * attribute to the browser. The two directions are complementary and BOTH header forms carry
+ * both: an absence check cannot see a dropped attribute, and a positive check cannot see an
+ * appended one.
+ */
+ @Test
+ @DisplayName("Should never emit SameSite=None — the rejected alternative to response_mode=query")
+ void shouldNeverEmitSameSiteNone() {
+ String setCookie = codec.toSetCookieHeader("record-123");
+ String clearing = codec.toClearingSetCookieHeader();
+
+ assertAll("no cookie attribute is weakened to make the browser flow work",
+ () -> assertFalse(setCookie.contains("SameSite=None"),
+ "SameSite=None would permit the cross-site sends this cookie exists to block: "
+ + setCookie),
+ () -> assertFalse(clearing.contains("SameSite=None"),
+ "the clearing form must not weaken the attribute either: " + clearing),
+ () -> assertTrue(setCookie.contains("; SameSite=Lax"),
+ "Lax is correct AND sufficient because the callback is a top-level GET: " + setCookie),
+ // The absence checks above cannot see an attribute that was DROPPED, and absent is
+ // not None: a change that removed SameSite entirely from the clearing header would
+ // satisfy every other assertion here. The set form is fenced positively, so the
+ // clearing form must be too.
+ () -> assertTrue(clearing.contains("; SameSite=Lax"),
+ "the clearing form must carry Lax positively, not merely lack None: " + clearing));
+ }
+
@Test
@DisplayName("Should clear the cookie with Max-Age=0 while keeping the __Host- attributes")
void shouldClearCookie() {
diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/reserved/CallbackEndpointTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/reserved/CallbackEndpointTest.java
index 13b64eeb..d26a626a 100644
--- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/reserved/CallbackEndpointTest.java
+++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/bff/reserved/CallbackEndpointTest.java
@@ -60,6 +60,14 @@
* duplicate-parameter rejection (via {@code parse(rawQuery)}), the browser-binding checks (D2b),
* and the success path that drives the exchange seam, creates the session, and redirects.
*
+ * Every case below is a {@code response_mode=query} callback. The gateway drives
+ * the authorization request with {@code response_mode=query}, so the live callback is a top-level
+ * GET whose {@code code}/{@code state} arrive in the query string, and the string each test hands
+ * {@code handle(..)} IS that raw query. The endpoint itself stays source-neutral by design — it
+ * parses whatever raw parameter string it is given — which is exactly why the assertion that the
+ * runtime hands it the uncollapsed query lives one layer out, in
+ * {@code GatewayEdgeRouteBffWiringTest}'s query-mode dispatch coverage rather than here.
+ *
* The engine exchange is driven through the {@link CodeExchange} seam, so the success and
* exchange-failure paths are exercised with a hand-built {@link AuthorizationCodeFlow.AuthenticationResult}
* — no live token endpoint, no signed tokens, no test double framework.
@@ -147,8 +155,17 @@ SealedSessionCookieCodec.DEFAULT_COOKIE_VALUE_BUDGET, new SecretKeySpec(key, "AE
salt);
}
+ /**
+ * The BFF-13 duplicate-parameter defence — the Keycloak CVE-2026-9689 class — asserted on the
+ * raw query, which under {@code response_mode=query} is the live callback shape.
+ *
+ * The defence is structural: the endpoint parses the raw string with
+ * {@code CallbackParameters.parse(String)} and never {@code CallbackParameters.of(Map)}, because
+ * only the raw parse can still SEE a duplicated {@code code} or {@code state}. A collapsed map
+ * has already silently chosen one occurrence by the time the endpoint runs.
+ */
@Nested
- @DisplayName("BFF-13 duplicate-parameter rejection (raw parse)")
+ @DisplayName("BFF-13 duplicate-parameter rejection on the raw query")
class DuplicateParameterRejection {
@Test
@@ -163,12 +180,16 @@ void shouldRejectDuplicateCode() {
}
@Test
- @DisplayName("Should reject a duplicate-state callback 400")
+ @DisplayName("Should reject a duplicate-state callback 400 without consuming the pending record")
void shouldRejectDuplicateState() {
CallbackOutcome outcome = endpoint.handle("code=abc&state=" + state + "&state=other", bindingCookieHeader,
T0);
- assertEquals(400, outcome.status());
+ assertEquals(400, outcome.status(),
+ "state is the parameter the binding check compares, so a duplicate must be refused "
+ + "outright rather than resolved to whichever occurrence a parser happened to pick");
+ assertTrue(pendingStore.consume(recordId, T0).isPresent(),
+ "the record is untouched — parse fails before binding resolution");
}
}
diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRouteBffWiringTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRouteBffWiringTest.java
index b7f17c04..5975343f 100644
--- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRouteBffWiringTest.java
+++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/GatewayEdgeRouteBffWiringTest.java
@@ -86,6 +86,7 @@
import io.vertx.core.Vertx;
import io.vertx.core.http.HttpClient;
import io.vertx.core.http.HttpClientRequest;
+import io.vertx.core.http.HttpClientResponse;
import io.vertx.core.http.HttpServer;
import io.vertx.core.http.RequestOptions;
import io.vertx.core.net.SocketAddress;
@@ -241,7 +242,8 @@ void tearDown() throws Exception {
@DisplayName("a reserved path is served by its handler and never reaches the route's allowed_paths gate")
void reservedPathNeverReachesThoroughChecks() throws Exception {
// Act — a reserved user-info request under the same /auth prefix the proxy route claims,
- // carrying a return_to value the url-parameter pipeline would reject
+ // carrying an arbitrary query parameter the url-parameter pipeline would reject. The
+ // parameter NAME is incidental here — it is not a login parameter and carries no contract.
int status = statusOf(USER_INFO_PATH + "?return_to=%2Fhome");
// Assert — 401 is the user-info handler's own no-session answer. A 400 would mean the
@@ -286,6 +288,113 @@ private static ResolvedRoute rejectEverythingRoute() {
}
}
+ /**
+ * Pins the wire name of the login return-URL query parameter to {@code returnUrl}.
+ *
+ * The name is a browser-facing contract shared by the demo SPA, the Playwright helper and
+ * {@link LoginInitiationEndpoint}'s own documented surface, but the only place it is actually read
+ * is the edge's reserved dispatch — so a rename there silently breaks the whole login flow with no
+ * compile error anywhere: every internal identifier on the path is already {@code returnUrl}, and a
+ * value the edge fails to extract simply degrades to {@link LoginFlow#DEFAULT_RETURN_URL}. That
+ * degradation is invisible to a type checker and to every test that does not drive a real request
+ * through the edge, which is why these two run over a live Vert.x server rather than calling
+ * {@link BffRuntime#dispatch} directly.
+ *
+ * The pair is deliberately a matched positive/negative control: the accepted spelling must reach
+ * the login fold, and the retired {@code return_to} spelling must NOT. Asserting only the positive
+ * case would still pass if the edge accepted both.
+ */
+ @Nested
+ @DisplayName("login initiation reads the returnUrl wire parameter — and only that spelling")
+ class LoginReturnUrlWireName {
+
+ private static final String RETURN_TARGET = "/dashboard";
+ private static final String ENCODED_RETURN_TARGET = "%2Fdashboard";
+
+ private Vertx vertx;
+ private ExecutorService virtualThreadExecutor;
+ private HttpServer front;
+ private HttpClient client;
+ private String sessionCookie;
+
+ @BeforeEach
+ void setUp() throws Exception {
+ vertx = Vertx.vertx();
+ virtualThreadExecutor = Executors.newVirtualThreadPerTaskExecutor();
+ TokenValidator tokenValidator = TokenValidator.builder()
+ .issuerConfig(TestTokenGenerators.accessTokens().next().getIssuerConfig()).build();
+
+ // A live session makes login initiation take the already-authenticated short-circuit, whose
+ // redirect Location IS the return URL the edge extracted — the cleanest observable for the
+ // wire name, and one that never reaches the IdP engine.
+ SessionStore store = new InMemorySessionStore(16);
+ String sessionId = SessionRecord.newSessionId();
+ store.create(SessionRecord.builder().sessionId(sessionId).accessToken("a").idToken("i").sub("sub")
+ .expiresAt(Instant.now().plus(Duration.ofHours(1))).build());
+ sessionCookie = SessionCookieCodec.DEFAULT_COOKIE_NAME + "=" + sessionId;
+
+ GatewayConfig gatewayConfig = GatewayConfig.builder().version(1).oidc(Optional.of(fullOidc())).build();
+ GatewayEdgeRoute edge = new GatewayEdgeRoute(new RouteTable(List.of()), gatewayConfig,
+ new SingletonInstance<>(tokenValidator), vertx, virtualThreadExecutor,
+ new EdgeHardeningOptions(), new SheriffMetrics(new SimpleMeterRegistry()),
+ activeRuntime(serverBinding(store)));
+ Router router = Router.router(vertx);
+ edge.registerRoutes(router);
+ front = vertx.createHttpServer().requestHandler(router)
+ .listen(0).toCompletionStage().toCompletableFuture().get(15, TimeUnit.SECONDS);
+ client = vertx.createHttpClient();
+ }
+
+ @AfterEach
+ void tearDown() throws Exception {
+ client.close().toCompletionStage().toCompletableFuture().get(5, TimeUnit.SECONDS);
+ front.close().toCompletionStage().toCompletableFuture().get(5, TimeUnit.SECONDS);
+ virtualThreadExecutor.close();
+ vertx.close().toCompletionStage().toCompletableFuture().get(5, TimeUnit.SECONDS);
+ }
+
+ @Test
+ @DisplayName("returnUrl reaches the login fold and becomes the short-circuit redirect target")
+ void shouldReadReturnUrlParameter() throws Exception {
+ // Arrange — see setUp: a live session and a same-origin relative return target
+
+ // Act
+ HttpClientResponse response = login("?returnUrl=" + ENCODED_RETURN_TARGET);
+
+ // Assert
+ assertEquals(302, response.statusCode(), "the live-session login short-circuit is a 302");
+ assertEquals(RETURN_TARGET, response.getHeader("Location"),
+ "returnUrl is the login wire parameter, so its value must reach the login fold");
+ }
+
+ @Test
+ @DisplayName("the retired return_to spelling is ignored and degrades to the default return URL")
+ void shouldIgnoreRetiredReturnToSpelling() throws Exception {
+ // Arrange — see setUp: identical request except for the parameter spelling
+
+ // Act
+ HttpClientResponse response = login("?return_to=" + ENCODED_RETURN_TARGET);
+
+ // Assert — the regression pin. If the wire name ever reverts to return_to, this request
+ // would be honoured and the Location would be RETURN_TARGET instead of the default.
+ assertEquals(302, response.statusCode(), "the live-session login short-circuit is a 302");
+ assertEquals(LoginFlow.DEFAULT_RETURN_URL, response.getHeader("Location"),
+ "return_to is not the login wire parameter, so it must not reach the login fold");
+ }
+
+ private HttpClientResponse login(String query) throws Exception {
+ // Connect to the local front server but present the OIDC host in the authority: the
+ // reserved-path registry is keyed on (host, canonicalPath).
+ RequestOptions options = new RequestOptions()
+ .setServer(SocketAddress.inetSocketAddress(front.actualPort(), "localhost"))
+ .setHost(OIDC_HOST).setPort(front.actualPort())
+ .setMethod(io.vertx.core.http.HttpMethod.GET).setURI(LOGIN_PATH + query);
+ return client.request(options)
+ .compose(request -> request.putHeader("Cookie", sessionCookie).send())
+ .toCompletionStage().toCompletableFuture().get(15, TimeUnit.SECONDS);
+ }
+ }
+
@Nested
@DisplayName("BffRuntime.dispatch routes each reserved path to its handler (not NO_ROUTE_MATCHED)")
class ReservedDispatch {
@@ -353,9 +462,22 @@ private BffRuntime.ReservedHttpRequest request(String cookie, String claims) {
}
}
+ /**
+ * The callback leg one layer out from {@link CallbackEndpoint}: through
+ * {@link BffRuntime#dispatch}, whose {@code callbackParameters} selects WHICH raw string the
+ * endpoint parses.
+ *
+ * That selection is the reason this coverage exists separately from
+ * {@code CallbackEndpointTest}. The endpoint is source-neutral — it parses whatever string it is
+ * handed — so an endpoint-level duplicate-parameter test proves the parse rejects duplicates, but
+ * NOT that the runtime hands it the genuinely raw, uncollapsed query. Under
+ * {@code response_mode=query} the {@code code}/{@code state} arrive in the query string, so the
+ * rawQuery selection path is now the live one and its BFF-13 duplicate-parameter defence (the
+ * Keycloak CVE-2026-9689 class) is asserted HERE, at the seam that could silently collapse it.
+ */
@Nested
- @DisplayName("form_post callback dispatch (POST /auth/callback, body-parsed code/state)")
- class FormPostCallbackDispatch {
+ @DisplayName("Query-mode callback dispatch (GET /auth/callback, raw-query code/state)")
+ class QueryCallbackDispatch {
private static final String RETURN_URL = "/dashboard";
private static final String RAW_ACCESS_TOKEN = "raw-access-token";
@@ -382,36 +504,69 @@ void setUp() {
pendingStore.store(pending);
bindingCookieHeader = bindingCodec.toSetCookieHeader(pending.id()).split(";", 2)[0];
- runtime = formPostRuntime();
+ runtime = callbackRuntime();
+ }
+
+ /** A {@code response_mode=query} callback: GET, {@code code}/{@code state} in the raw query, no body. */
+ private BffRuntime.ReservedHttpRequest queryCallback(String rawQuery) {
+ return new BffRuntime.ReservedHttpRequest(rawQuery, bindingCookieHeader, null, null, null, null, "GET");
}
@Test
- @DisplayName("POST body with code+state resolves the pending record and creates the session (302 + session cookie)")
- void shouldCompleteFormPostLogin() {
+ @DisplayName("GET query with code+state resolves the pending record and creates the session (302 + session cookie)")
+ void shouldCompleteQueryModeLogin() {
BffRuntime.ReservedHttpResponse response = runtime.dispatch(ReservedEndpoint.CALLBACK,
- new BffRuntime.ReservedHttpRequest("", bindingCookieHeader, null, null, null,
- "code=auth-code&state=" + state, "POST"), now);
+ queryCallback("code=auth-code&state=" + state), now);
- assertEquals(302, response.status(), "form_post code exchange completes the login");
+ assertEquals(302, response.status(), "the query-mode code exchange completes the login");
assertEquals(Optional.of(RETURN_URL), response.locationOptional());
assertTrue(response.setCookieHeaders().stream()
.anyMatch(cookie -> cookie.startsWith(SessionCookieCodec.DEFAULT_COOKIE_NAME + "=")),
- "the session cookie is set from the form_post callback");
+ "the session cookie is set from the query-mode callback");
}
@Test
- @DisplayName("A duplicate code in the POST body is still rejected 400 (BFF-13 raw-parse defence)")
- void shouldRejectDuplicateCodeInFormBody() {
+ @DisplayName("A duplicate code in the RAW QUERY is rejected 400 (BFF-13 defence survives the mode switch)")
+ void shouldRejectDuplicateCodeInRawQuery() {
BffRuntime.ReservedHttpResponse response = runtime.dispatch(ReservedEndpoint.CALLBACK,
- new BffRuntime.ReservedHttpRequest("", bindingCookieHeader, null, null, null,
- "code=first&code=second&state=" + state, "POST"), now);
+ queryCallback("code=first&code=second&state=" + state), now);
- assertEquals(400, response.status(), "a duplicated code in the form body is rejected by parse()");
+ assertEquals(400, response.status(),
+ "a duplicated code reaches parse() uncollapsed and is rejected — a first-value-wins "
+ + "projection anywhere between the edge and the endpoint would have let it through");
assertTrue(pendingStore.consume(bindingCodec.readRecordId(bindingCookieHeader).orElseThrow(), now)
.isPresent(), "the record is untouched — parse fails before binding resolution");
}
- private BffRuntime formPostRuntime() {
+ @Test
+ @DisplayName("A duplicate state in the RAW QUERY is rejected 400 (BFF-13 defence survives the mode switch)")
+ void shouldRejectDuplicateStateInRawQuery() {
+ BffRuntime.ReservedHttpResponse response = runtime.dispatch(ReservedEndpoint.CALLBACK,
+ queryCallback("code=auth-code&state=" + state + "&state=attacker-supplied"), now);
+
+ assertEquals(400, response.status(),
+ "a duplicated state is rejected too — state is the parameter the binding check compares, "
+ + "so a collapsed map choosing either occurrence would be exploitable");
+ assertTrue(pendingStore.consume(bindingCodec.readRecordId(bindingCookieHeader).orElseThrow(), now)
+ .isPresent(), "the record is untouched — parse fails before binding resolution");
+ }
+
+ /**
+ * The fail-closed counterpart: the edge no longer buffers a body for the callback, so a stray
+ * POST to that path arrives with no body at all. It must be an honest {@code 400}, never a
+ * {@code 500} from a null body reaching the parse.
+ */
+ @Test
+ @DisplayName("A stray POST to the callback arrives bodyless and is rejected 400, never 500")
+ void shouldRejectStrayPostBodyless() {
+ BffRuntime.ReservedHttpResponse response = runtime.dispatch(ReservedEndpoint.CALLBACK,
+ new BffRuntime.ReservedHttpRequest("", bindingCookieHeader, null, null, null, null, "POST"), now);
+
+ assertEquals(400, response.status(),
+ "an absent form body normalizes to the empty string and fails for a missing state");
+ }
+
+ private BffRuntime callbackRuntime() {
CallbackEndpoint callback = new CallbackEndpoint((context, params) -> {
Map accessClaims = new HashMap<>();
accessClaims.put(ClaimName.SUBJECT.getName(), ClaimValue.forPlainString(SUBJECT));
diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/ReservedBodyCeilingTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/ReservedBodyCeilingTest.java
index 98831b02..8763bb90 100644
--- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/ReservedBodyCeilingTest.java
+++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/edge/ReservedBodyCeilingTest.java
@@ -58,11 +58,16 @@
import org.junit.jupiter.api.Test;
/**
- * Pins the byte ceiling the edge enforces on a gateway-terminated reserved POST body (the OIDC
- * {@code response_mode=form_post} callback and the back-channel logout receiver), driven over a live
- * Vert.x HTTP server — no Docker, no Quarkus.
+ * Pins the byte ceiling the edge enforces on a gateway-terminated reserved POST body, driven over a
+ * live Vert.x HTTP server — no Docker, no Quarkus.
*
- * Those two paths are read pre-authentication and before the pipeline's per-route
+ * The back-channel logout receiver is the one path this bounds. The gateway drives
+ * the OIDC flow with {@code response_mode=query}, so its callback is a bodyless top-level GET that
+ * consumes no request body at all and is deliberately no longer on the edge's reserved-body-read
+ * allowlist. Back-channel logout remains a genuinely body-carrying reserved POST — it carries a
+ * {@code logout_token} JWT — so it keeps the ceiling, and every assertion below drives it.
+ *
+ * That path is read pre-authentication and before the pipeline's per-route
* {@code maxBodySize} cap can apply, so without a ceiling at the read itself an unauthenticated caller
* could buffer an arbitrarily large body straight into the gateway heap. The ceiling is enforced twice
* — a {@code Content-Length} pre-check and a streaming cumulative counter — and both arms are pinned
@@ -79,6 +84,11 @@
class ReservedBodyCeilingTest {
private static final String CALLBACK_PATH = "/auth/callback";
+ /**
+ * The one reserved path that still consumes a request body, and therefore the path every
+ * ceiling assertion below drives.
+ */
+ private static final String BACKCHANNEL_LOGOUT_PATH = "/auth/backchannel-logout";
private static final String CRLF = "\r\n";
/** Bounded wait proving the pre-check refused BEFORE buffering: far below the 20s body deadline. */
private static final long NO_BUFFERING_TIMEOUT_SECONDS = 5L;
@@ -102,6 +112,9 @@ void setUp() throws Exception {
.version(1)
.oidc(Optional.of(OidcConfig.builder()
.redirectUri(Optional.of("https://localhost" + CALLBACK_PATH))
+ .logout(Optional.of(OidcConfig.Logout.builder()
+ .backchannelPath(Optional.of(BACKCHANNEL_LOGOUT_PATH))
+ .build()))
.build()))
.build();
EdgeHardeningOptions hardening = new EdgeHardeningOptions();
@@ -135,7 +148,7 @@ void tearDown() throws Exception {
void acceptsBodyExactlyAtTheCeiling() throws Exception {
String body = formBody(ceiling);
- int status = post(CALLBACK_PATH, body);
+ int status = post(BACKCHANNEL_LOGOUT_PATH, body);
assertEquals(ceiling, body.length(),
"The arranged body must sit exactly on the boundary for this to pin the ceiling");
@@ -148,19 +161,48 @@ void acceptsBodyExactlyAtTheCeiling() throws Exception {
void rejectsBodyOverTheCeiling() throws Exception {
String body = formBody(ceiling + 1);
- int status = post(CALLBACK_PATH, body);
+ int status = post(BACKCHANNEL_LOGOUT_PATH, body);
assertEquals(413, status,
"A body beyond the ceiling is refused 413 — the honest status for an over-large payload");
}
+ /**
+ * The counterpart assertion to every test in this class: the OIDC callback is NOT bounded here,
+ * because it no longer reads a body at all.
+ *
+ * The gateway drives {@code response_mode=query}, so the live callback is a top-level GET
+ * carrying {@code code}/{@code state} in the query string. {@code ReservedEndpoint.CALLBACK} was
+ * therefore removed from {@code GatewayEdgeRoute.needsReservedBodyRead}'s allowlist — and that
+ * removal was not cosmetic: leaving it there would have kept granting ANY unauthenticated POST
+ * to the callback path a pre-authentication, pre-pipeline body read of up to the ceiling, a
+ * retained attack surface with no remaining purpose.
+ *
+ * An over-ceiling POST to the callback path is consequently no longer refused {@code 413}. It
+ * takes the ordinary paused-stream path and reaches the reserved carve-out ({@code 404} under
+ * the inert runtime here; {@code 400} for a missing {@code state} against a live one). Asserting
+ * the {@code 413} is GONE is what pins the allowlist decision — without it, silently restoring
+ * {@code CALLBACK} to the allowlist would break no test.
+ */
+ @Test
+ @DisplayName("no longer applies the ceiling to the callback path — it reads no body under response_mode=query")
+ void doesNotApplyTheCeilingToTheBodylessCallback() throws Exception {
+ String body = formBody(ceiling + 1);
+
+ int status = post(CALLBACK_PATH, body);
+
+ assertEquals(404, status,
+ "The callback is not on the reserved-body-read allowlist, so no ceiling applies and the "
+ + "request reaches the reserved carve-out instead of being refused 413");
+ }
+
@Test
@DisplayName("rejects an over-ceiling declared Content-Length without buffering any body byte")
void rejectsOverCeilingContentLengthWithoutBuffering() {
// A raw request head declaring far more than the ceiling, with ZERO body bytes ever sent. A
// gateway that buffered first would still be blocked on its 20-second body deadline; only a
// gateway that refuses on the declared length alone can answer inside the bounded wait below.
- String head = "POST " + CALLBACK_PATH + " HTTP/1.1" + CRLF
+ String head = "POST " + BACKCHANNEL_LOGOUT_PATH + " HTTP/1.1" + CRLF
+ "Host: localhost:" + frontPort + CRLF
+ "Content-Length: " + (ceiling * 64) + CRLF
+ "Connection: close" + CRLF + CRLF;
@@ -188,7 +230,7 @@ void rejectsOverCeilingChunkedBody() throws Exception {
// altogether. That is what is exercised here.
int chunkSize = 1024;
long chunks = ceiling / chunkSize + 2;
- StringBuilder raw = new StringBuilder("POST " + CALLBACK_PATH + " HTTP/1.1" + CRLF
+ StringBuilder raw = new StringBuilder("POST " + BACKCHANNEL_LOGOUT_PATH + " HTTP/1.1" + CRLF
+ "Host: localhost:" + frontPort + CRLF
+ "Transfer-Encoding: chunked" + CRLF
+ "Connection: close" + CRLF + CRLF);
@@ -214,7 +256,7 @@ void retiresConnectionAfterDeclaredLengthRejection() {
// NOT hand the connection back for reuse: the request body was never consumed, so pending bytes
// would desync the next request framed on it — or an attacker could pin connections by
// repeatedly tripping the 413, which is exactly the DoS this ceiling exists to bound.
- String head = "POST " + CALLBACK_PATH + " HTTP/1.1" + CRLF
+ String head = "POST " + BACKCHANNEL_LOGOUT_PATH + " HTTP/1.1" + CRLF
+ "Host: localhost:" + frontPort + CRLF
+ "Content-Length: " + (ceiling * 64) + CRLF + CRLF;
@@ -234,7 +276,7 @@ void retiresConnectionAfterStreamedRejection() {
// same retirement applies — this path left the most unread bytes of the two.
int chunkSize = 1024;
long chunks = ceiling / chunkSize + 2;
- StringBuilder raw = new StringBuilder("POST " + CALLBACK_PATH + " HTTP/1.1" + CRLF
+ StringBuilder raw = new StringBuilder("POST " + BACKCHANNEL_LOGOUT_PATH + " HTTP/1.1" + CRLF
+ "Host: localhost:" + frontPort + CRLF
+ "Transfer-Encoding: chunked" + CRLF + CRLF);
String chunk = "x".repeat(chunkSize);
@@ -282,11 +324,11 @@ private CompletableFuture sendRawAwaitingClose(String rawRequest) {
/**
* Builds a urlencoded form body of exactly {@code length} characters, shaped like the
- * {@code state=…} a real form_post callback carries so the payload is representative rather than
- * arbitrary filler.
+ * {@code logout_token=…} a real back-channel logout carries so the payload is representative
+ * rather than arbitrary filler.
*/
private static String formBody(long length) {
- String prefix = "state=" + Generators.letterStrings(8, 16).next() + "&code=";
+ String prefix = "sid=" + Generators.letterStrings(8, 16).next() + "&logout_token=";
return prefix + "a".repeat((int) length - prefix.length());
}
diff --git a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/quarkus/BffRuntimeProducerTest.java b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/quarkus/BffRuntimeProducerTest.java
index ba35e9e2..90fe240c 100644
--- a/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/quarkus/BffRuntimeProducerTest.java
+++ b/api-sheriff/src/test/java/de/cuioss/sheriff/gateway/quarkus/BffRuntimeProducerTest.java
@@ -15,26 +15,39 @@
*/
package de.cuioss.sheriff.gateway.quarkus;
+import static org.junit.jupiter.api.Assertions.assertAll;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.lang.annotation.Annotation;
+import java.lang.reflect.Field;
+import java.lang.reflect.InaccessibleObjectException;
+import java.lang.reflect.Modifier;
import java.time.Instant;
+import java.util.ArrayDeque;
+import java.util.ArrayList;
import java.util.Arrays;
import java.util.Base64;
+import java.util.Collections;
+import java.util.Deque;
+import java.util.IdentityHashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Optional;
+import java.util.Set;
+import de.cuioss.sheriff.gateway.bff.login.QueryResponseModeAuthorizationRequestBuilder;
import de.cuioss.sheriff.gateway.bff.reserved.ReservedPathRegistry.ReservedEndpoint;
import de.cuioss.sheriff.gateway.bff.runtime.BffRuntime;
import de.cuioss.sheriff.gateway.config.model.GatewayConfig;
import de.cuioss.sheriff.gateway.config.model.OidcConfig;
+import de.cuioss.sheriff.token.client.flow.AuthorizationRequestBuilder;
import de.cuioss.sheriff.token.validation.TokenValidator;
import de.cuioss.sheriff.token.validation.test.generator.TestTokenGenerators;
import de.cuioss.test.generator.junit.EnableGeneratorController;
@@ -44,6 +57,7 @@
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.function.Executable;
/**
* Covers {@link BffRuntimeProducer}: the runtime is active (and its reserved handlers and session
@@ -103,6 +117,92 @@ void shouldWireUserInfo() {
Instant.parse("2026-07-25T10:00:00Z"));
assertEquals(401, response.status());
}
+
+ /**
+ * The wiring-level half of the response-mode assertion — the seam's own behaviour is pinned by
+ * {@code QueryResponseModeAuthorizationRequestBuilderTest}.
+ *
+ * This exists because the failure mode it guards is an omission, and an omission is
+ * invisible to a behavioural test of the seam. The engine's
+ * {@link AuthorizationRequestBuilder} emits {@code response_mode=form_post} unconditionally,
+ * and its shorter constructors silently install that default: a future refactor that rebuilt
+ * {@code AuthorizationCodeFlow} through the 4-argument constructor, or {@code StepUpHandler}
+ * through its no-argument one, would compile, pass every seam test, and quietly reintroduce
+ * the cross-site POST callback on which the {@code SameSite=Lax} binding cookie is dropped.
+ *
+ * The assertion is therefore made against the object graph the producer actually built, and
+ * it is type-directed rather than name-directed: it finds every
+ * {@code AuthorizationRequestBuilder} reachable from the assembled runtime and requires each
+ * one to be the gateway's query-mode subclass. Renaming an engine field does not break it;
+ * reverting a seam to the engine default does — which is exactly the intended sensitivity.
+ */
+ @Test
+ @DisplayName("Should wire the query-mode response builder into every engine authorization seam")
+ void shouldWireQueryResponseModeIntoEveryAuthorizationSeam() {
+ List wired = reachableAuthorizationRequestBuilders(runtime);
+
+ assertFalse(wired.isEmpty(),
+ "no AuthorizationRequestBuilder was reachable from the assembled runtime — this test "
+ + "must never pass vacuously; if the producer's wiring moved, retarget the walk");
+ assertAll("every engine seam that builds an authorization URL carries the query-mode builder",
+ wired.stream().map(builder -> (Executable) () ->
+ assertInstanceOf(QueryResponseModeAuthorizationRequestBuilder.class, builder,
+ "an engine seam is still on the default builder, which emits "
+ + "response_mode=form_post")));
+ }
+ }
+
+ /**
+ * Collects every {@link AuthorizationRequestBuilder} reachable from {@code root} by walking
+ * instance fields, following lambda captures so a seam held only inside a closure is still seen.
+ *
+ * The walk is bounded to the gateway's and the engine's own packages: it never descends into JDK
+ * or container types, which keeps it away from the strongly-encapsulated {@code java.*} modules
+ * and stops it wandering through collections and class loaders. A field the JVM refuses to open
+ * is skipped rather than failing the walk — the caller's non-empty assertion is what guarantees
+ * the result is still meaningful.
+ */
+ private static List reachableAuthorizationRequestBuilders(Object root) {
+ List found = new ArrayList<>();
+ Set