[vharseko]

Summary

Enable HttpOnly session cookies by default and adapt the in-repository authentication clients to receive the session through Set-Cookie when /json/authenticate omits tokenId from its JSON body.

New installations and deployments without an explicit value get com.sun.identity.cookie.httponly=true. Existing installations retain their persisted setting. The XUI already supports HttpOnly cookies through #1036.

Changes

Cookie defaults and configuration

• Shared and IdP discovery CookieUtils enable HttpOnly unless the configured value is false (case-insensitive, surrounding whitespace ignored). Unset, empty, and unrecognized values leave it enabled; both readers have value-table tests.
• serverdefaults.properties ships httponly=true, and the standalone IdP discovery configurator selects True by default.
• Documentation and configuration comments explain the new default, the token-in-body opt-in, and the restart requirement. The existing warning about the OIDC session-management iframe is preserved.

Authentication consumers

• MCP server: both username/password and OAuth authentication prefer a non-empty body tokenId, otherwise read the last non-empty session cookie. The configured openam.tokenHeader supplies the cookie name, and URL-encoded values are decoded. Missing sessions fail explicitly. The README documents the behavior and OPENAM_TOKEN_HEADER for renamed cookies.
• REST and SOAP STS: ConnectionResult carries Set-Cookie headers through the authentication dispatchers to AMTokenParserImpl, including SOAP STS agent bootstrap. The parser prefers the body token and otherwise reads the last non-empty cookie named by the existing AM_SESSION_COOKIE_NAME binding, decoding encoded values. Headers are read by index to preserve wire order on JDK 11/17; a real loopback HttpURLConnection test covers repeated headers and HttpOnly visibility.
• Agent installer (openam-installtools): RESTEndpoint.RESTResponse retains Set-Cookie headers in wire order. CreateProfileTask uses the cookie name obtained from serverinfo, prefers a nonblank body token, and otherwise takes the last non-empty matching cookie, decoding encoded values. Automatic creation of a missing agent profile therefore works without enabling allowTokenInBody. Authentication failures, missing sessions, and malformed cookie encoding stop profile creation.
• js-sdk UI (/extui): a string successUrl identifies completed authentication; SuccessfulAuth.tokenId is optional because the browser receives the session through the cookie.
• Self-registration auto-login: under HttpOnly, AutoLoginStage sets the session cookie on the registration response and omits additions.tokenId unless allowTokenInBody is enabled. The XUI only stores a tokenId when one is returned.

CI and browser coverage

• Run the XUI Playwright specs against the new HttpOnly-enabled default first, verifying serverinfo.cookieHttpOnly=true.
• Restart the IDP with HttpOnly explicitly disabled, then run the full suite. The OAuth2, SAML, and Kerberos helpers that read body tokenId run in this second phase.
• Docker setup reads the administrator session from the last non-empty Set-Cookie instead of the JSON body; the IDP administrator login pipeline uses pipefail. Authentication smoke checks use successUrl.
• The self-registration spec checks registration, automatic login, cookie flags, omission of tokenId under HttpOnly, and session survival after reload in both modes. Existing XUI specs cover login, logout, reload, and step-up.
• The rebase preserves the Kerberos installation steps and subsequent cookie-handling fixes from master.

Compatibility and operator guidance

• Existing installations keep their persisted HttpOnly value; the new default applies to fresh installations and unset values.
• Under HttpOnly, successful /json/authenticate responses omit tokenId by default. Non-browser integrations should read the session Set-Cookie. Integrations that still require the JSON field can set org.openidentityplatform.openam.httponly.allowTokenInBody=true, which exposes the token to scripts again, or disable HttpOnly with com.sun.identity.cookie.httponly=false.
• Both properties are read at server startup; changes require a restart.
• The OIDC session-management iframe (/oauth2/connect/checkSession) reads the session cookie from JavaScript and cannot observe the session while HttpOnly is enabled. This remains a documented limitation.

Validation

Latest installer fix, 056cb216c6:

• Before the fix, six of the eleven new CreateProfileTaskTest scenarios failed, including cookie-only authentication, renamed cookies, repeated cookies, URL encoding, and empty body-token fallback.
• After the fix, all 48 openam-installtools tests pass on JDK 11 and JDK 27, with no failures, errors, or skips. The tests exercise the real CreateProfileTask.execute() and HTTP client against a loopback HTTP server, asserting the session header sent in the profile-creation request and that failure cases send no such request.
• git diff --check passes. A complete agent installation against a deployed OpenAM server was not run in this round.

Earlier validation:

• Targeted tests for both CookieUtils implementations, RestAuthenticationHandler, the STS token parser and HTTP wrapper, self-registration, MCP authentication, and both js-sdk login cases passed on the preceding head.
• Previous rounds ran the affected module suites and js-sdk type checking, checked the MCP precedence and STS header-order tests by mutation, and exercised XUI login/reload and /extui against a server built from this branch. Self-registration passed in both cookie modes and failed against the earlier implementation with HttpOnly enabled.
• The full browser e2e suite was not rerun locally for the installer-only fix; current CI status is shown in the PR checks.

References

• HttpOnly session-cookie support in XUI (#1036)
• CVE-2026-53660: Insecure SSO Cookie Initialization, reported by @wodzen

[tsujiguchitky]

LGTM - enabling HttpOnly by default is the right call, and the change is consistent across code, defaults, CI, tests, and docs. Approving.

[vharseko]

Rebased/merged master into this branch to pick up recent changes — in particular #1084, which had independently reworded the same HttpOnly documentation/comment block (still defaulting to false) while this PR was open. Resolved the conflict by keeping this PR's "default true" rationale and escape hatches (allowTokenInBody, disabling via false), while preserving the two caveats #1084 added that this PR didn't have yet: the property is read once at server startup (needs a restart to take effect), and the OIDC session-management iframe (/oauth2/connect/checkSession) can't observe the session while HttpOnly is enabled. Same treatment in serverdefaults.properties, chap-securing.adoc, and chap-deployments.adoc. No functional changes beyond the doc/comment text — re-requesting review since the branch moved.

[tsujiguchitky]

Thanks for all the work on this! I have to apologize for coming back with something new. In my earlier review, I didn't find any blockers, but I noticed an issue that I think should be addressed before merging.

openam-mcp-server gets its SSO token only from the /json/authenticate response body. AuthInterceptor#getUserNamePasswordToken and #getTokenIdFromAccessToken both do tokenResponse.get("tokenId"), and the RestClient has no cookie handling. RestAuthenticationHandler only puts tokenId in the body when !isCookieHttpOnly() || isHttpOnlyAllowTokenInBody(). So with the new defaults (HttpOnly on, allowTokenInBody off), a freshly installed OpenAM no longer returns it, and the MCP server ends up with a null token.

Could we have the MCP server read iPlanetDirectoryPro from the Set-Cookie header, the same way the CI step now does, with a test for the HttpOnly-mode response? If you'd rather handle that separately, then at least documenting in the MCP server README that allowTokenInBody=true is required would help. Thanks again!

[vharseko]

@tsujiguchitky Thanks — good catch, confirmed: with HttpOnly on and allowTokenInBody off, both AuthInterceptor paths got a null token (and actually hit an NPE in the Caffeine cache on refresh). Fixed in this PR rather than separately, since it is this change that breaks it (1700002):

• /json/authenticate responses are now read as a ResponseEntity; the token comes from tokenId in the body when present, otherwise from the last non-empty iPlanetDirectoryPro (openam.tokenHeader) Set-Cookie, with a clear error when neither is there.
• Tests cover both the username/password and OAuth paths in HttpOnly mode via MockRestServiceServer, plus a clearing cookie and no token at all.
• The MCP server README notes that allowTokenInBody isn't needed.

I also rebased the branch onto current master (no functional change to the rest of the PR). Re-requesting review.

[tsujiguchitky]

LGTM! Thanks for the quick fix — the MCP server now picks up the SSO token from the session cookie under the new HttpOnly default, and the tests cover both auth paths nicely.

[maximthomas]

praise: The default flip is small, and the one in-repo consumer it adapts is handled carefully.

• The flip is a single expression, getAsBoolean(Constants.AM_COOKIE_HTTPONLY, true) in openam-shared/.../CookieUtils.java:68-69, plus the shipped value in serverdefaults.properties.
• AuthInterceptor.extractSessionToken (openam-mcp-server/.../security/AuthInterceptor.java:152-173) prefers the body token, takes the last non-empty session cookie, and throws an IllegalStateException naming the cookie. A null token no longer reaches the token cache.

---

issue (blocking): Under the new default, the js-sdk login UI shipped at /extui never finishes a successful login.

openam-ui/openam-ui-js-sdk/src/lib/Login.tsx:47-49, openam-ui/openam-ui-js-sdk/src/lib/types.ts:43

isSuccessfulAuth() counts a response as success only when it has a string tokenId. RestAuthenticationHandler.java:339 now leaves tokenId out of the body whenever the cookie is HttpOnly and allowTokenInBody is off. A successful login therefore returns {successUrl, realm}, and handleAuthResponse falls through to console.error("Unknown response format"): no redirect, no navigate('/'). The bundle is unpacked into the server WAR at /extui (openam-server-only/pom.xml:111-118), so every new install is affected. The session already travels in the cookie, so the SDK does not need the token.

// Login.tsx
function isSuccessfulAuth(response: unknown): response is SuccessfulAuth {
    return typeof response === 'object' && response !== null
        && 'successUrl' in response && typeof response.successUrl === 'string';
}
// types.ts:43
  tokenId?: string;

---

issue (blocking): Under the new default, REST and SOAP STS fail every validation that goes through REST authN.

openam-sts/openam-common-sts/src/main/java/org/forgerock/openam/sts/token/AMTokenParserImpl.java:51-56, openam-sts/openam-common-sts/src/main/java/org/forgerock/openam/sts/token/validator/AuthenticationHandlerImpl.java:61, openam-sts/openam-soap-sts/openam-soap-sts-server/src/main/java/org/forgerock/openam/sts/soap/bootstrap/SoapSTSAccessTokenProviderImpl.java:143

getSessionFromAuthNResponse reads the session id only from tokenId in the /json/authenticate body. If the field is missing it throws TokenValidationException(INTERNAL_ERROR, "REST authN response does not contain tokenId string entry…"). On a new install the body no longer carries it, so these all fail:

• REST STS username, OIDC and X.509 token validation;
• SOAP STS agent bootstrap.

They keep failing until an operator sets allowTokenInBody=true or httponly=false. The STS HTTP client's HttpURLConnectionWrapper.ConnectionResult holds only the body, so the cookie is lost before the parser runs. No CI step exercises the STS.

Suggested fix: give the STS the fallback the MCP server got. Carry the last non-empty session Set-Cookie value (cookie name from com.iplanet.am.cookie.name) in ConnectionResult, and use it in AMTokenParserImpl when the body has no string tokenId.

Pin: a unit test in which getSessionFromAuthNResponse gets a {successUrl, realm} body plus a session cookie and returns the cookie value.

---

issue (non-blocking): Under the new default, a user logged in by self-registration auto-login gets no session cookie.

openam-ui/openam-ui-ria/src/main/js/org/forgerock/openam/ui/user/anonymousProcess/SelfRegistrationView.js:49-52, openam-selfservice/src/main/java/org/forgerock/openam/selfservice/config/flows/AutoLoginStage.java:94, openam-ui/openam-ui-ria/src/main/js/org/forgerock/openam/ui/user/login/tokens/SessionToken.jsm:104-110

AutoLoginStage returns the new session only as additions.tokenId and sets no cookie. In HttpOnly mode SessionToken.set() keeps that value in page memory instead of writing the cookie. RESTLoginView.handleExistingSession then sends the user to the goto URL with window.location.href, and on that page, or on the next reload, the user is logged out. The registration response also puts the session id in front of JavaScript, which HttpOnly is meant to prevent. Not run: this needs a browser against a realm with self-registration and auto-login enabled. With that confirmed, it would be blocking.

Suggested fix: when CookieUtils.isCookieHttpOnly(), have the registration response set the session cookie the way /json/authenticate does, and leave tokenId out of the additions.

---

issue (non-blocking): The two HttpOnly readers this PR changes disagree on any value other than true or false.

openam-federation/openam-idpdiscovery/src/main/java/com/sun/identity/saml2/idpdiscovery/CookieUtils.java:66-67, openam-shared/src/main/java/com/sun/identity/shared/encode/CookieUtils.java:68-69

The IDP discovery reader treats everything except false as on. The shared reader uses Boolean.parseBoolean, so everything except true is off. The server WAR maps the IDP discovery servlets (openam-server-only web.xml:479-491), so both readers see the same value in one JVM. With -Dcom.sun.identity.cookie.httponly= (or yes, or 1), _saml_idp is HttpOnly and iPlanetDirectoryPro is not. The console and ssoadm accept only true/false, so only -D values or hand-edited files reach this.

// openam-shared CookieUtils.java:68 — same semantics as idpdiscovery: only "false" turns it off
static boolean cookieHttpOnly = !"false".equalsIgnoreCase(
    StringUtils.trimToEmpty(SystemPropertiesManager.get(Constants.AM_COOKIE_HTTPONLY)));

---

issue (non-blocking): The MCP server's cookie fallback returns the URL-encoded cookie value when cookie encoding is on.

openam-mcp-server/src/main/java/org/openidentityplatform/openam/mcp/server/security/AuthInterceptor.java:158-166

With com.iplanet.am.cookie.encode=true and com.iplanet.am.cookie.c66Encode=false, the session id contains =@#. The cookie then carries %3D%40%23, and SessionResource reads the token header without decoding it. Session validation fails, and the retry loop ends in IllegalStateException. With the default c66Encode=true the id survives encoding unchanged, so default installs are not affected. A raw session id never contains %, so decoding only values that contain it is safe.

String value = pair.substring(eq + 1).trim();
if (value.indexOf('%') >= 0) {
    value = URLDecoder.decode(value, StandardCharsets.UTF_8);
}

---

suggestion (non-blocking): extractSessionToken_prefersTokenIdInBody cannot fail against a cookie-first implementation.

openam-mcp-server/src/test/java/org/openidentityplatform/openam/mcp/server/security/AuthInterceptorTest.java:393-400

openAMConfig.tokenHeader() is not stubbed, so the mock returns null and no cookie ever matches. A version that reads the cookie first and falls back to the body still returns the body token, so the body precedence described in the PR is not pinned. Stub the name with lenient(): the correct code never reads it, so a strict stub would raise UnnecessaryStubbingException. With that stub, the cookie-first version fails.

// import static org.mockito.Mockito.lenient;
@Test
void extractSessionToken_prefersTokenIdInBody() {
    lenient().when(openAMConfig.tokenHeader()).thenReturn("iPlanetDirectoryPro");
    ResponseEntity<Map<String, String>> response = ResponseEntity.ok()
            .headers(setCookies("iPlanetDirectoryPro=AQIC5wM2LY4Sfczn-cookie-token; Path=/"))
            .body(Map.of("tokenId", "AQIC5wM2LY4Sfczn-body-token"));

    assertThat(interceptor.extractSessionToken(response)).isEqualTo("AQIC5wM2LY4Sfczn-body-token");
}

---

suggestion (non-blocking): No test pins the "last non-empty cookie wins" rule.

openam-mcp-server/src/test/java/org/openidentityplatform/openam/mcp/server/security/AuthInterceptorTest.java:402-414, openam-mcp-server/src/main/java/org/openidentityplatform/openam/mcp/server/security/AuthInterceptor.java:148-150

Every fixture has at most one non-empty iPlanetDirectoryPro cookie. A version that stops at the first non-empty match passes every test, although the javadoc says the last one wins, and so does the tail -n1 in build.yml.

@Test
void extractSessionToken_lastNonEmptyCookieWins() {
    when(openAMConfig.tokenHeader()).thenReturn("iPlanetDirectoryPro");
    ResponseEntity<Map<String, String>> response = ResponseEntity.ok()
            .headers(setCookies(
                    "iPlanetDirectoryPro=AQIC5wM2LY4Sfczn-old-token; Path=/",
                    "iPlanetDirectoryPro=AQIC5wM2LY4Sfczn-new-token; Path=/; HttpOnly",
                    "iPlanetDirectoryPro=; Expires=Thu, 01-Jan-1970 00:00:10 GMT; Path=/"))
            .body(Map.of("successUrl", "/openam/console"));

    assertThat(interceptor.extractSessionToken(response)).isEqualTo("AQIC5wM2LY4Sfczn-new-token");
}

Pin: this test fails against both a first-match version and a last-match-including-empty version.

---

suggestion (non-blocking): A failed IDP admin login no longer fails the step where it happens.

.github/workflows/build.yml:185-193

The whole curl -sf … | grep . | tail -n1 pipeline now runs inside bash -c without pipefail, so its status is that of tail, which is always 0. A 401 or an unmatched Set-Cookie leaves ADMIN_TOKEN empty. The user-create call at :199 swallows the error, and the job fails later at "Test demo user Auth", which points at the wrong step.

          ADMIN_TOKEN=$(docker exec openam-idp bash -o pipefail -c \

[vharseko]

@maximthomas Thanks for the thorough round. All eight points are addressed in 225d262 (branch rebased onto current master first; no conflicts).

1. js-sdk /extui login (blocking) — isSuccessfulAuth() now requires only a string successUrl, and SuccessfulAuth.tokenId is optional. New Login.test.tsx covers a response with and without tokenId; the no-tokenId case failed before the fix. Also checked in a real browser against a server built from this branch in HttpOnly mode: /extui login lands on the user profile.

2. REST/SOAP STS (blocking) — done as you suggested, with one change to where the cookie name comes from:

• ConnectionResult now carries the Set-Cookie headers, and the dispatchers return the whole ConnectionResult instead of the body string.
• AMTokenParserImpl takes tokenId from the body when present. Otherwise it takes the last non-empty session cookie, URL-decoded when it contains %.
• The cookie name comes from the STS's existing @Named(AMSTSConstants.AM_SESSION_COOKIE_NAME) binding rather than com.iplanet.am.cookie.name: the SOAP STS is a separate WAR without the server's SystemProperties, and that binding is already available in all three injectors.
• Headers are read by index, not via getHeaderFields(), which returns repeated headers in reverse order. HttpURLConnectionWrapperFactoryTest pins that.
• AMTokenParserTest is rewritten; the old one never asserted its equals result. It covers the body token, the cookie fallback, body precedence, last-non-empty-wins, the URL-encoded cookie and the no-session case.

3. Self-registration auto-login — confirmed in a browser, and it was indeed blocking: on the previous code with HttpOnly on, the user stays on the registration form after "Save". With HttpOnly on, AutoLoginStage now sets the session cookie on the response the way CoreServicesWrapper#setAuthCookie does for /json/authenticate, and leaves tokenId out of the additions unless allowTokenInBody is set. The XUI only calls SessionToken.set when a tokenId came back. Unit tests cover the three modes. The new e2e spec e2e/xui/xui-self-registration.spec.mjs registers a user through the XUI and checks that the user is logged in and stays logged in after a reload, the cookie's HttpOnly flag, and that tokenId is absent from the registration response in HttpOnly mode. It passes in both cookie modes against a build of this branch and fails on the previous code. It sits under e2e/xui, so both CI phases run it.

4. HttpOnly value parsing — both readers now use the same rule: only false (any case, surrounding whitespace ignored) turns HttpOnly off. I kept a small isHttpOnlyEnabled(String) in each CookieUtils rather than calling the shared one from IDP discovery: that would pull the shared class's static initialisation (Debug, SystemPropertiesManager) into the standalone IDP discovery WAR. Both modules test the same value table. chap-config-ref.adoc states the rule.

5. MCP URL-encoded cookie — values containing % are decoded, as you suggested; test added.

6. prefersTokenIdInBody — the cookie name is stubbed with lenient(). 7. Last non-empty cookie wins — test added as you proposed. I checked both by mutation: a cookie-first implementation fails 6, and a first-match implementation fails 7.

8. CI pipefail — the IDP admin login pipeline runs under bash -o pipefail -c, so a failed login fails that step.

The failed build-docker job on the previous head was unrelated: the "latest release" GitHub API call returned nothing, so the image tag was empty. It runs again with this push.

[maximthomas]

praise: The follow-up commit fixes every consumer of the new default at the place where it broke.

• AMTokenParserImpl.getSessionFromAuthNResponse now falls back to the last non-empty session Set-Cookie, which HttpURLConnectionWrapperFactory reads by index. The new @Named(AM_SESSION_COOKIE_NAME) argument is already bound in all three injectors (RestSTSInstanceModule, SoapSTSModule, SoapSTSInstanceModule).
• Under HttpOnly, AutoLoginStage.putSessionInSuccessAdditions sets the session cookie itself and keeps tokenId out of the body. xui-self-registration.spec.mjs covers this in both CI phases.
• Both CookieUtils classes now share isHttpOnlyEnabled. With bash -o pipefail in the IDP admin-token step, a broken admin login fails that step again.

---

suggestion (non-blocking): The STS Set-Cookie fallback depends on how a real HttpURLConnection returns headers, and only a mock tests it.

openam-sts/openam-common-sts/src/test/java/org/forgerock/openam/sts/HttpURLConnectionWrapperFactoryTest.java:35-63, openam-sts/openam-common-sts/src/main/java/org/forgerock/openam/sts/HttpURLConnectionWrapperFactory.java:128-141

The test scripts getHeaderFieldKey(i)/getHeaderField(i) on a mock. That leaves the two JDK facts in the comment untested:

• repeated Set-Cookie headers come back by index, in wire order;
• an HttpOnly one is visible only while no JVM-wide CookieHandler is installed. With one installed, the value comes back as "".

No CI step runs the REST or SOAP STS, so this test is the only guard on this path. If the code switched to getHeaderFields(), or a fixture or library installed a CookieHandler, AMTokenParserImpl would throw "contains neither a tokenId … nor a non-empty … cookie" and the suite would stay green. The code is correct today: nothing in OpenAM or in the server WAR's jars calls CookieHandler.setDefault.

// imports: com.sun.net.httpserver.HttpServer, java.net.InetAddress, java.net.InetSocketAddress
@Test
public void seesHttpOnlySetCookieOnARealConnectionInWireOrder() throws Exception {
    HttpServer server = HttpServer.create(new InetSocketAddress(InetAddress.getLoopbackAddress(), 0), 0);
    server.createContext("/openam/json/authenticate", exchange -> {
        exchange.getResponseHeaders().add("Set-Cookie", "iPlanetDirectoryPro=AQIC5wM2LY4Sfczn-old-token; Path=/");
        exchange.getResponseHeaders().add("Set-Cookie", "iPlanetDirectoryPro=AQIC5wM2LY4Sfczn-new-token; Path=/; HttpOnly");
        byte[] body = "{\"successUrl\":\"/openam/console\"}".getBytes(StandardCharsets.UTF_8);
        exchange.sendResponseHeaders(HttpURLConnection.HTTP_OK, body.length);
        exchange.getResponseBody().write(body);
        exchange.close();
    });
    server.start();
    try {
        HttpURLConnectionWrapper.ConnectionResult result =
                new HttpURLConnectionWrapperFactory(new DefaultHttpURLConnectionFactory())
                        .httpURLConnectionWrapper(new URL("http://127.0.0.1:" + server.getAddress().getPort()
                                + "/openam/json/authenticate"))
                        .withoutAuditTransactionIdHeader()
                        .makeInvocation();
        assertEquals(result.getSetCookieHeaders(), Arrays.asList(
                "iPlanetDirectoryPro=AQIC5wM2LY4Sfczn-old-token; Path=/",
                "iPlanetDirectoryPro=AQIC5wM2LY4Sfczn-new-token; Path=/; HttpOnly"));
    } finally {
        server.stop(0);
    }
}

Pin: this test fails if the reader switches to getHeaderFields() (wrong order) or if the test JVM has a JVM-wide CookieHandler (empty value). It runs on every JDK in the CI matrix.

---

note (non-blocking): The PR description is out of date for 225d262:

• openam-shared/src/main/java/com/sun/identity/shared/encode/CookieUtils.java:69: the description says getAsBoolean(AM_COOKIE_HTTPONLY, true). The code uses isHttpOnlyEnabled, where only false turns HttpOnly off.
• openam-sts/openam-common-sts/src/main/java/org/forgerock/openam/sts/token/AMTokenParser.java, .../validator/disp/TokenAuthenticationRequestDispatcher.java, .../HttpURLConnectionWrapper.java: not described — they now pass ConnectionResult, and the session is read from Set-Cookie.
• openam-selfservice/src/main/java/org/forgerock/openam/selfservice/config/flows/AutoLoginStage.java: not described — under HttpOnly it sets the session cookie and leaves out tokenId.
• openam-ui/openam-ui-js-sdk/src/lib/Login.tsx, types.ts: not described — success now depends on successUrl alone.
• openam-ui/openam-ui-ria/src/main/js/org/forgerock/openam/ui/user/anonymousProcess/SelfRegistrationView.js: not described.

[vharseko]

@maximthomas The real-connection test is in (5957a6b): HttpURLConnectionWrapperFactoryTest.seesHttpOnlySetCookieOnARealConnectionInWireOrder, as proposed, with one change — the server binds to the literal 127.0.0.1 and the URL uses the same literal. Binding to InetAddress.getLoopbackAddress() can yield ::1 under -Djava.net.preferIPv6Addresses=true, which would no longer match the hard-coded 127.0.0.1 in the URL.

Verifying the pin by mutation refined one fact: getHeaderFields() returns the values of a repeated header in reverse order on JDK 11 and 17, but in wire order on JDK 21+ (checked empirically on 11.0.32, 17.0.20, 21.0.12 and 26.0.2). So:

• A getHeaderFields() rewrite fails the new test on the JDK 11/17 CI legs — confirmed by running the mutation on JDK 11, which fails with exactly the reversed order. On JDK 21+ the order no longer differs; there a naive rewrite is caught by the map key instead (the header arrives as Set-cookie, so an exact-spelling get("Set-Cookie") returns null).
• The unqualified "reverse order" wording in the HttpURLConnectionWrapperFactory comment was out of date for newer JDKs; it and the test comment now say "on JDK 11 and 17".

The CookieHandler half of the pin holds as described: with a JVM-wide CookieManager installed, the HttpOnly cookie's value comes back empty and the test fails.

The branch was rebased onto current master (81d8304) before the change; the openam-common-sts suite passes on JDK 11 and 26.

On the description note: the description was updated on 2026-09-28, right after the round-2 reply, and already covers the five items (isHttpOnlyEnabled, the STS ConnectionResult path, AutoLoginStage, Login.tsx/types.ts, SelfRegistrationView) — the note appears to have been drafted against the earlier text. It now also describes the new real-connection test.

[maximthomas]

praise: The STS HttpOnly fallback is now pinned against a real HttpURLConnection, not only a mock.

• HttpURLConnectionWrapperFactoryTest.seesHttpOnlySetCookieOnARealConnectionInWireOrder (openam-sts/openam-common-sts/src/test/java/org/forgerock/openam/sts/HttpURLConnectionWrapperFactoryTest.java:75-103) drives the wrapper through an in-JVM com.sun.net.httpserver.HttpServer and asserts both Set-Cookie lines in wire order, the HttpOnly one included, so a getHeaderFields()-based reader or a JVM-wide CookieHandler in the test JVM turns it red.
• It also asserts the response body, so the success road through makeInvocation() is covered along with the headers.
• All 11 build-maven legs (JDK 11 to 26 on Linux, macOS and Windows) are green at 5957a6bc.

[tsujiguchitky]

LGTM! Thanks for the thorough follow-up — the STS, self-registration, and js-sdk paths now all work with HttpOnly by default, and the new tests pin the behavior nicely.

[vharseko]

Rebased onto current master (082e236) to clear the merge conflict with #1136 and #1146.

The only conflicts were in the two CookieUtilsTest classes (openam-shared and openam-idpdiscovery), where master and this PR both appended tests at the end of the class. Both sides are kept: the withCookieSettings / Secure / HttpOnly / SameSite / RFC 6265 tests from master plus httpOnlyValues / onlyFalseTurnsHttpOnlyOff from this PR; the idpdiscovery class javadoc now names both. The main CookieUtils code merged cleanly — cookieHttpOnly is still initialised through isHttpOnlyEnabled(...), and the flag is now set at cookie creation as introduced in #1136.

One consequence of combining with #1146: in idpdiscovery an HttpOnly cookie is written as a hand-built Set-Cookie header rather than via response.addCookie, so with HttpOnly on by default the _saml_idp cookie now always takes that path.

mvn test -pl openam-shared,openam-federation/openam-idpdiscovery: 1261 + 29 tests, 0 failures.

[vharseko]

Rebased onto current master (4d5cca0, after the 16.1.4 release) to clear the merge conflict with #1172 and GHSA-2gwf. The head is now e3c9175.

Conflicts:

• .github/workflows/build.yml: WindowsDesktopSSO (Kerberos/SPNEGO) tests without Windows: Playwright e2e and Kerby module test #1172 added the Install Playwright and Install MIT Kerberos client steps in front of the Playwright phases, and this PR reorders those phases. Both are kept: master's install steps, then this PR's order (HttpOnly enabled, the default, first; then HttpOnly disabled with the full set).
• serverdefaults.properties: GHSA-2gwf added auth.goto.allowAnyUrlWithoutWhitelist and federation.allowAnyUrlWithoutWhitelist right after com.sun.identity.cookie.httponly. Both are kept, below this PR's comment and httponly=true.

One change outside the conflicts: the header comment of e2e/kerberos/kerberos.spec.mjs (#1172) said the spec runs before HttpOnly mode is enabled. With the phases reversed it runs in the HttpOnly-disabled phase, so the comment now says that. It is folded into the first commit; the other four commits are unchanged.

I also checked the code that came in from master for anything else that reads tokenId from /json/authenticate under the default configuration (workflow, e2e specs, Java, XUI, device flow) and found nothing.

RestAuthenticationHandlerTest (21), both CookieUtilsTest classes (15 + 28), AutoLoginStageTest (3), the STS token parser tests (8) and the MCP AuthInterceptorTest (21) pass.

[vharseko]

@maximthomas @tsujiguchitky The final review uncovered one more authentication-response consumer: automatic agent-profile creation in openam-installtools. Fixed in 056cb216c6.

CreateProfileTask still required tokenId in the JSON body, while RESTEndpoint discarded response cookies. With the new defaults, successful administrator authentication therefore ended in JSONObject["tokenId"] not found, and no profile-creation request was sent.

The fix retains Set-Cookie headers in wire order and uses the session cookie name already obtained from serverinfo. A nonblank body token takes precedence; otherwise the last non-empty matching cookie supplies the session, with URL decoding when needed. Authentication failures or a missing/invalid session stop profile creation.

The new test exercises the real installer task and HTTP client against a loopback server. Six scenarios failed before the fix. All 48 module tests now pass on JDK 11 and JDK 27, including renamed cookies, body precedence, repeated/clearing cookies, encoded and raw values, and failure cases. A complete agent installation against a deployed server was not run in this round.

The branch was checked against freshly fetched master before the change and was already current. I have updated the PR description to include this consumer and the latest validation. Re-requesting review.

[maximthomas]

praise: 056cb21 adapts the last in-repo /json/authenticate reader, agent-profile creation in openam-installtools, with the same rule as the MCP and STS clients.

• CreateProfileTask.getSSOToken (CreateProfileTask.java:220-244) uses a non-blank body tokenId first. Otherwise it takes the last non-empty Set-Cookie named by the serverinfo cookie name, and URL-decodes it only when the value has a %. When neither is present it throws an IllegalStateException naming the cookie, instead of the opaque JSONObject["tokenId"] not found.
• RESTEndpoint.call reads Set-Cookie by index in wire order (RESTEndpoint.java:116-120). RESTResponse.getSetCookieHeaders (:426) keeps the headers out of toString(), which the existing debug log prints.
• CreateProfileTaskTest drives the real task and HTTP client against a loopback server, so the cookie path goes through a real HttpURLConnection rather than a mock.

[tsujiguchitky]

Thanks for covering the installer as well!

LGTM. CreateProfileTask now takes the agent admin's session from the configured session cookie when tokenId is absent, using the same rules as MCP and STS (body first, last non-empty cookie, decode only when encoded).
RESTEndpoint keeps the Set-Cookie headers in wire order in a separate field, so toString() (used for the debug log and JSON parsing) still returns only the body and never writes the session cookie to the log.
The end-to-end tests against a local HTTP server cover HttpOnly, renamed-cookie and failure cases nicely.