Skip to content

[#1080] OAuth2 consent: serve the consent details as JSON - #1081

Closed
maximthomas wants to merge 1 commit into
OpenIdentityPlatform:masterfrom
maximthomas:issues/1080-oauth2-json-consent
Closed

[#1080] OAuth2 consent: serve the consent details as JSON#1081
maximthomas wants to merge 1 commit into
OpenIdentityPlatform:masterfrom
maximthomas:issues/1080-oauth2-json-consent

Conversation

@maximthomas

Copy link
Copy Markdown
Contributor

Fix #1080

16.1.1 (CVE-2026-53660) replaced the csrf consent parameter with a random per-request token that is only minted while rendering the HTML consent page, so a client that posts the consent decision directly always gets HTTP 400. There was no supported way for it to obtain a token.

Add a JSON representation of the consent details to GET /oauth2/authorize, selected with Accept: application/json. It carries the token along with the requested scopes and claims, so a non-browser client can post the decision back exactly as the consent form does. The strict per-request token is kept as it is; the legacy csrf=<session id> form is not restored.

  • negotiate strictly: JSON has to be preferred over HTML, so */* and the library defaults that weight the two equally keep getting the consent page
  • refuse the JSON representation to a foreign Origin, since the OAuth2 endpoints sit behind a CORS filter that can be configured to echo the caller's Origin with credentials
  • report errors as JSON to JSON clients, and turn the login redirect into 401 login_required, which a non-browser client can act on
  • e2e coverage of the two-step flow, and correct the dev guide and admin guide, which still described csrf as a copy of the iPlanetDirectoryPro cookie

…ails as JSON

16.1.1 (CVE-2026-53660) replaced the `csrf` consent parameter with a random
per-request token that is only minted while rendering the HTML consent page,
so a client that posts the consent decision directly always gets HTTP 400.
There was no supported way for it to obtain a token.

Add a JSON representation of the consent details to GET /oauth2/authorize,
selected with `Accept: application/json`. It carries the token along with the
requested scopes and claims, so a non-browser client can post the decision back
exactly as the consent form does. The strict per-request token is kept as it
is; the legacy `csrf=<session id>` form is not restored.

- negotiate strictly: JSON has to be preferred over HTML, so `*/*` and the
  library defaults that weight the two equally keep getting the consent page
- refuse the JSON representation to a foreign Origin, since the OAuth2
  endpoints sit behind a CORS filter that can be configured to echo the
  caller's Origin with credentials
- report errors as JSON to JSON clients, and turn the login redirect into
  401 login_required, which a non-browser client can act on
- e2e coverage of the two-step flow, and correct the dev guide and admin guide,
  which still described `csrf` as a copy of the iPlanetDirectoryPro cookie
@maximthomas
maximthomas requested a review from vharseko July 27, 2026 09:25
@vharseko

Copy link
Copy Markdown
Member

Thanks for the thorough PR — but I think we should solve #1080 differently. The regression has a ~5-line fix that restores the documented contract exactly, and once it is in place the JSON consent representation has no remaining problem to solve. I'd rather not take on a new public API for this.

A workaround exists already, with no code change at all. The double-submit check needs no server-side state: it simply compares the csrf parameter against the CSRF cookie, and that branch is evaluated as a fallback even for stateful sessions. Since a non-browser client controls its own Cookie header, it can mint the pair itself — no prior GET of the consent page needed:

$ curl --request POST \
    --header "Cookie: iPlanetDirectoryPro=<session>; oauth2_csrf=<any random value>" \
    --data "csrf=<the same value>" \
    --data "decision=allow" \
    ... \
    https://openam.example.com:8443/openam/oauth2/realms/root/authorize

(on HTTPS deployments the cookie name is __Host-oauth2_csrf). This is exactly the security model double-submit is built on — the protection comes from a cross-site attacker being unable to set cookies for the OpenAM host, not from the server knowing the value — so nothing is weakened. It could be documented today as the answer for affected clients, independent of whatever we decide below.

The proposal: accept the resource owner's session token ID as a valid csrf value again — a third branch in CsrfProtection.isCsrfAttack(), after the session-property and double-submit-cookie checks — but only while the SSO cookie is HttpOnly:

// Backward compatibility with the pre-16.1.1 documented contract: the resource owner's
// session token ID is accepted as proof - but only while the SSO cookie is HttpOnly,
// so the accepted value is never script-readable. See the security note below.
if (ssoToken != null && CookieUtils.isCookieHttpOnly()
        && constantTimeEquals(ssoToken.getTokenID().toString(), csrfValue)) {
    return false;
}

Why this does not reintroduce CVE-2026-53660. The vulnerability was never "the server accepts the session ID as the CSRF token". It was that the browser flow required the XUI JavaScript to read the iPlanetDirectoryPro cookie, which forced the SSO cookie to be shipped without HttpOnly. That stays fixed: the consent page and XUI keep using the dedicated random token, and the SSO cookie remains HttpOnly.

The CookieUtils.isCookieHttpOnly() condition makes the CSRF guarantee hold unconditionally: the legacy form is only ever accepted in configurations where the session ID is provably unreadable from any script — a cross-site attacker cannot read the cookie at all, and with HttpOnly on, neither can same-origin script. If an administrator reverts com.sun.identity.cookie.httponly to false — the exact exposure the CVE closed — the compat branch disarms itself and those deployments keep today's strict behaviour. The session ID is also a strictly stronger secret than the CSRF token: anyone who knows it can take over the session outright, at which point CSRF is moot.

Non-browser clients know their token ID legitimately: HttpOnly does not constrain an HTTP client that reads the Set-Cookie header itself, and raw-REST integrations that want it echoed in the /json/authenticate body can already opt in via org.openidentityplatform.openam.httponly.allowTokenInBody=true — the switch was introduced for precisely this kind of integration, and the consent compat branch follows the same philosophy. The comparison must be constant-time (MessageDigest.isEqual), same as the old code.

Why I'd drop the JSON representation rather than ship both:

  • It does not fix the reported breakage. Every existing client stays broken until it is rewritten against a new, OpenAM-specific two-step protocol — including keeping a cookie jar on stateless deployments. The compat fix makes those clients work again with zero changes; after that, the JSON flow serves no client that the restored contract doesn't already serve.
  • A client that is being written from scratch already has two supported options: the restored csrf=<tokenId> form, or the self-minted double-submit pair shown above — a single request, no HTML scraping, no new API. A third mechanism adds choice, not capability.
  • Content negotiation on a browser-facing endpoint is inherently fragile — the PR itself needs the strict q-value logic and a large test matrix precisely to keep curl's */* and the axios/RestTemplate defaults from silently switching formats. That risk surface is permanent, and it guards an API nobody needs once compatibility is restored.
  • The JSON branch also brings its own security surface that then has to be maintained: the Origin allow-list against credentialed CORS, cache-control of a token-plus-PII JSON body, the split error modes. All of it is well done here, but it is cost without benefit compared to the 5-line fix.
  • It is a public API: once released we have to document, support and version it. For a consent endpoint I'd keep the surface minimal.

The documentation corrections are valuable regardless — the dev guide and admin guide still describe csrf as a copy of the iPlanetDirectoryPro cookie, and with the compat fix restored that description becomes accurate again for HttpOnly deployments. I'd document the random token as the primary mechanism, the double-submit pair as the supported programmatic path, and mark the session-ID form as legacy.

Happy to open a PR with the isCsrfAttack() change, a unit test covering both HttpOnly states and the doc update if you agree with the direction.

@vharseko vharseko added bug oauth2 OAuth2 / OpenID Connect security Security fix or hardening (CVE, GHSA, XSS/CSRF/SSRF) documentation Documentation, README, or javadoc labels Jul 27, 2026
@maximthomas

Copy link
Copy Markdown
Contributor Author

closed in favor of #1083

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug documentation Documentation, README, or javadoc oauth2 OAuth2 / OpenID Connect security Security fix or hardening (CVE, GHSA, XSS/CSRF/SSRF)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OAuth2 Consent CSRF changes break documented POST consent flow

2 participants