From 8b1ecfaae7459451f6645088c3f9a660298249d5 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Fri, 18 Sep 2026 14:36:42 +0300 Subject: [PATCH] Set the Secure and HttpOnly cookie flags from creation CookieUtils.newCookie set Secure from com.iplanet.am.cookie.secure but left HttpOnly to addCookieToResponse, which applied it by reflection - a Servlet 2.5 leftover - so a cookie handed straight to response.addCookie, as every logout deletion is, went out without it. Both flags are now set on the cookie as it is created, from the same properties, and addCookieToResponse calls Cookie.setHttpOnly directly. The IdP discovery copy of CookieUtils follows suit. The remaining cookies built by hand go the same way: the LoginServlet and CDCServlet deletions are built with the name, path and domain the cookie was set with instead of re-adding the request's Cookie object, the AMTESTCOOKIE probe goes through CookieUtils, and the Oracle Access Manager and SiteMinder sample adapters mark their session cookie Secure on an HTTPS request and HttpOnly. --- .../identity/saml2/plugins/OAMAdapter.java | 4 +- .../sun/identity/saml2/plugins/SMAdapter.java | 4 +- .../com/iplanet/services/cdc/CDCServlet.java | 6 +- .../saml2/idpdiscovery/CookieUtils.java | 20 +++-- .../saml2/idpdiscovery/CookieUtilsTest.java | 70 +++++++++++++++ .../authentication/UI/LoginServlet.java | 15 ++-- .../identity/shared/encode/CookieUtils.java | 41 +++------ .../shared/encode/CookieUtilsTest.java | 85 ++++++++++++++++++- 8 files changed, 200 insertions(+), 45 deletions(-) diff --git a/openam-federation/OpenFM/src/main/integrations/oracle/source/com/sun/identity/saml2/plugins/OAMAdapter.java b/openam-federation/OpenFM/src/main/integrations/oracle/source/com/sun/identity/saml2/plugins/OAMAdapter.java index 9075bbf269..058b512f0a 100644 --- a/openam-federation/OpenFM/src/main/integrations/oracle/source/com/sun/identity/saml2/plugins/OAMAdapter.java +++ b/openam-federation/OpenFM/src/main/integrations/oracle/source/com/sun/identity/saml2/plugins/OAMAdapter.java @@ -25,7 +25,7 @@ * $Id: OAMAdapter.java,v 1.4 2008/08/19 19:11:39 veiming Exp $ * * Portions Copyrighted 2015 ForgeRock AS. - * Portions Copyrighted 2025 3A Systems LLC. + * Portions Copyrighted 2025-2026 3A Systems LLC. */ package com.sun.identity.saml2.plugins; @@ -275,6 +275,8 @@ public boolean postSingleSignOnSuccess( System.out.println("User session: " + sessionid); Cookie oamCookie = new Cookie(oamCookieName, sessionid); oamCookie.setDomain(cookieDomain); + oamCookie.setSecure(request.isSecure()); + oamCookie.setHttpOnly(true); response.addCookie(oamCookie); } } diff --git a/openam-federation/OpenFM/src/main/integrations/siteminder/source/com/sun/identity/saml2/plugins/SMAdapter.java b/openam-federation/OpenFM/src/main/integrations/siteminder/source/com/sun/identity/saml2/plugins/SMAdapter.java index 77f7557545..547f127c4f 100644 --- a/openam-federation/OpenFM/src/main/integrations/siteminder/source/com/sun/identity/saml2/plugins/SMAdapter.java +++ b/openam-federation/OpenFM/src/main/integrations/siteminder/source/com/sun/identity/saml2/plugins/SMAdapter.java @@ -25,7 +25,7 @@ * $Id: SMAdapter.java,v 1.4 2008/08/19 19:11:41 veiming Exp $ * * Portions Copyrighted 2015-2016 ForgeRock AS. - * Portions Copyrighted 2025 3A Systems LLC. + * Portions Copyrighted 2025-2026 3A Systems LLC. */ package com.sun.identity.saml2.plugins; @@ -356,6 +356,8 @@ public boolean postSingleSignOnSuccess( Cookie smCookie = new Cookie(smCookieName, sd.spec); smCookie.setDomain(cookieDomain); + smCookie.setSecure(request.isSecure()); + smCookie.setHttpOnly(true); response.addCookie(smCookie); return false; } catch (Exception ex) { diff --git a/openam-federation/OpenFM/src/main/java/com/iplanet/services/cdc/CDCServlet.java b/openam-federation/OpenFM/src/main/java/com/iplanet/services/cdc/CDCServlet.java index c131594733..c4a7487110 100644 --- a/openam-federation/OpenFM/src/main/java/com/iplanet/services/cdc/CDCServlet.java +++ b/openam-federation/OpenFM/src/main/java/com/iplanet/services/cdc/CDCServlet.java @@ -25,7 +25,7 @@ * $Id: CDCServlet.java,v 1.13 2009/11/13 23:43:17 dknab Exp $ * * Portions Copyrighted 2010-2016 ForgeRock AS. - * Portions Copyrighted 2025 3A Systems LLC. + * Portions Copyrighted 2025-2026 3A Systems LLC. */ package com.iplanet.services.cdc; @@ -672,8 +672,8 @@ private void redirectForAuthentication( * Reset the cookie value to null, to avoid continous loop * when a load balancer is used. */ - authCookie.setValue(""); - response.addCookie(authCookie); + CookieUtils.addCookieToResponse(response, CookieUtils.newCookie( + authURLCookieName, "", 0, "/", authURLCookieDomain)); response.sendRedirect(redirectURL.toString()); } diff --git a/openam-federation/openam-idpdiscovery/src/main/java/com/sun/identity/saml2/idpdiscovery/CookieUtils.java b/openam-federation/openam-idpdiscovery/src/main/java/com/sun/identity/saml2/idpdiscovery/CookieUtils.java index 21913da9db..a44efc76bb 100644 --- a/openam-federation/openam-idpdiscovery/src/main/java/com/sun/identity/saml2/idpdiscovery/CookieUtils.java +++ b/openam-federation/openam-idpdiscovery/src/main/java/com/sun/identity/saml2/idpdiscovery/CookieUtils.java @@ -368,8 +368,16 @@ public static Cookie newCookie(String name, String value, String path) { cookie.setDomain(domain); } - cookie.setSecure(isCookieSecure()); - + // The flags are set from creation so that a cookie handed straight to + // response.addCookie carries them as well as one that goes through + // addCookieToResponse. + if (isCookieSecure()) { + cookie.setSecure(true); + } + if (isCookieHttpOnly()) { + cookie.setHttpOnly(true); + } + return cookie; } @@ -484,13 +492,15 @@ public static void addCookieToResponse(HttpServletResponse response, if (cookie == null) { return; } - if (!isCookieHttpOnly() && getCookieSameSite() == null) { + if (getCookieSameSite() == null) { + if (isCookieHttpOnly()) { + cookie.setHttpOnly(true); + } response.addCookie(cookie); return; } - // Once JavaEE6 is available, the following code can be simplified - // to be one line response.addCookie(cookie) + // The servlet Cookie has no SameSite attribute: write the header by hand. StringBuffer sb = new StringBuffer(150); sb.append(cookie.getName()).append("=").append(cookie.getValue()); String path = cookie.getPath(); diff --git a/openam-federation/openam-idpdiscovery/src/test/java/com/sun/identity/saml2/idpdiscovery/CookieUtilsTest.java b/openam-federation/openam-idpdiscovery/src/test/java/com/sun/identity/saml2/idpdiscovery/CookieUtilsTest.java index cfc0854296..61a7b898f3 100644 --- a/openam-federation/openam-idpdiscovery/src/test/java/com/sun/identity/saml2/idpdiscovery/CookieUtilsTest.java +++ b/openam-federation/openam-idpdiscovery/src/test/java/com/sun/identity/saml2/idpdiscovery/CookieUtilsTest.java @@ -15,12 +15,19 @@ */ package com.sun.identity.saml2.idpdiscovery; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertTrue; +import jakarta.servlet.http.Cookie; import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; @@ -88,4 +95,67 @@ public void rejectsNullAndEmpty() { assertFalse(CookieUtils.isRedirectUrlValid(request, null)); assertFalse(CookieUtils.isRedirectUrlValid(request, " ")); } + + /** Runs {@code body} with the deployment's Secure/HttpOnly/SameSite cookie settings pinned. */ + private static void withCookieSettings(boolean secure, boolean httpOnly, String sameSite, Runnable body) { + boolean savedSecure = CookieUtils.secureCookie; + boolean savedHttpOnly = CookieUtils.cookieHttpOnly; + String savedSameSite = CookieUtils.cookieSameSite; + try { + CookieUtils.secureCookie = secure; + CookieUtils.cookieHttpOnly = httpOnly; + CookieUtils.cookieSameSite = sameSite; + body.run(); + } finally { + CookieUtils.secureCookie = savedSecure; + CookieUtils.cookieHttpOnly = savedHttpOnly; + CookieUtils.cookieSameSite = savedSameSite; + } + } + + @Test + public void newCookieCarriesTheSecureFlagTheDeploymentConfigures() { + withCookieSettings(true, false, null, () -> + assertTrue(CookieUtils.newCookie("_saml_idp", "aWRw").getSecure())); + withCookieSettings(false, false, null, () -> + assertFalse(CookieUtils.newCookie("_saml_idp", "aWRw").getSecure())); + } + + @Test + public void newCookieCarriesTheHttpOnlyFlagTheDeploymentConfigures() { + withCookieSettings(false, true, null, () -> + assertTrue(CookieUtils.newCookie("_saml_idp", "aWRw").isHttpOnly())); + withCookieSettings(false, false, null, () -> + assertFalse(CookieUtils.newCookie("_saml_idp", "aWRw").isHttpOnly())); + } + + /** Without SameSite the cookie goes through the servlet API, HttpOnly set on the cookie itself. */ + @Test + public void addCookieToResponseSetsHttpOnlyOnTheServletCookie() { + withCookieSettings(false, true, null, () -> { + HttpServletResponse response = mock(HttpServletResponse.class); + Cookie cookie = new Cookie("_saml_idp", "aWRw"); + + CookieUtils.addCookieToResponse(response, cookie); + + assertTrue(cookie.isHttpOnly()); + verify(response).addCookie(cookie); + verify(response, never()).addHeader(anyString(), anyString()); + }); + } + + /** With SameSite configured the cookie goes out as a hand-built header, flags included. */ + @Test + public void addCookieToResponseWritesTheHeaderItselfWhenSameSiteIsConfigured() { + withCookieSettings(true, true, "Lax", () -> { + HttpServletResponse response = mock(HttpServletResponse.class); + Cookie cookie = new Cookie("_saml_idp", "aWRw"); + cookie.setPath("/"); + + CookieUtils.addCookieToResponse(response, cookie); + + verify(response, never()).addCookie(any(Cookie.class)); + verify(response).addHeader(eq("SET-COOKIE"), eq("_saml_idp=aWRw;path=/;secure;httponly;SameSite=Lax")); + }); + } } diff --git a/openam-server-auth-ui/src/main/java/com/sun/identity/authentication/UI/LoginServlet.java b/openam-server-auth-ui/src/main/java/com/sun/identity/authentication/UI/LoginServlet.java index 068c3a5e9a..2936fe4675 100644 --- a/openam-server-auth-ui/src/main/java/com/sun/identity/authentication/UI/LoginServlet.java +++ b/openam-server-auth-ui/src/main/java/com/sun/identity/authentication/UI/LoginServlet.java @@ -25,7 +25,7 @@ * $Id: LoginServlet.java,v 1.9 2009/02/18 03:38:42 222713 Exp $ * * Portions Copyrighted 2011-2016 ForgeRock AS. - * Portions Copyrighted 2025 3A Systems LLC. + * Portions Copyrighted 2025-2026 3A Systems LLC. */ package com.sun.identity.authentication.UI; @@ -42,6 +42,7 @@ import com.sun.identity.common.RequestUtils; import com.sun.identity.shared.Constants; import com.sun.identity.shared.debug.Debug; +import com.sun.identity.shared.encode.CookieUtils; import com.sun.identity.shared.locale.L10NMessageImpl; import java.net.URL; import java.util.Map; @@ -272,9 +273,11 @@ private void rerouteRequest(HttpServletRequest request, HttpServletResponse resp + "initializeRequestContext removing" + "cookie " + cookies[i].getName()); } - cookies[i].setValue(""); - cookies[i].setMaxAge(0); - response.addCookie(cookies[i]); + // A request cookie carries no path or flags: a deletion built + // the way the cookie was set is the one the browser matches. + Cookie hostCookie = AuthUtils.createCookie(cookies[i].getName(), "", null); + hostCookie.setMaxAge(0); + response.addCookie(hostCookie); for (String domain : domains) { if (debug.messageEnabled()) { debug.message("LoginServlet:initializeRequestContext removing cookie " + domain); @@ -339,8 +342,8 @@ private boolean checkForCookiesInBrowser(HttpServletRequest request, } if (numCookies == 0 && redirectFlag == null) { - Cookie dummyCookie = new Cookie("AMTESTCOOKIE", "amtestcookie"); - response.addCookie(dummyCookie); + CookieUtils.addCookieToResponse(response, + CookieUtils.newCookie("AMTESTCOOKIE", "amtestcookie")); String queryStr = request.getQueryString(); try { if (queryStr == null || queryStr.length() == 0) { diff --git a/openam-shared/src/main/java/com/sun/identity/shared/encode/CookieUtils.java b/openam-shared/src/main/java/com/sun/identity/shared/encode/CookieUtils.java index 13d6c97f75..0aa4711fd7 100644 --- a/openam-shared/src/main/java/com/sun/identity/shared/encode/CookieUtils.java +++ b/openam-shared/src/main/java/com/sun/identity/shared/encode/CookieUtils.java @@ -36,8 +36,6 @@ import com.sun.identity.shared.configuration.SystemPropertiesManager; import com.sun.identity.shared.debug.Debug; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; import java.text.SimpleDateFormat; import java.util.Collection; import java.util.Collections; @@ -94,17 +92,6 @@ public class CookieUtils { private static int defAge = -1; static Debug debug = Debug.getInstance("amCookieUtils"); - private static final Method setHttpOnlyMethod; - - static { - Method method = null; - try { - method = Cookie.class.getMethod("setHttpOnly", boolean.class); - } catch (NoSuchMethodException nsme) { - debug.message("This is not a Java EE 6+ container, Cookie#setHttpOnly(boolean) is not available"); - } - setHttpOnlyMethod = method; - } /** * Gets property value of "com.iplanet.am.cookie.name" @@ -409,7 +396,15 @@ public static Cookie newCookie( cookie.setDomain(domain); } - cookie.setSecure(isCookieSecure()); + // The flags are set from creation so that a cookie handed straight to + // response.addCookie carries them as well as one that goes through + // addCookieToResponse. + if (isCookieSecure()) { + cookie.setSecure(true); + } + if (isCookieHttpOnly()) { + cookie.setHttpOnly(true); + } return cookie; } @@ -441,22 +436,14 @@ public static void addCookieToResponse(HttpServletResponse response, Cookie cook if (response==null || cookie == null) { return; } - if (!isCookieHttpOnly() && getCookieSameSite() == null) { + if (getCookieSameSite() == null) { + if (isCookieHttpOnly()) { + cookie.setHttpOnly(true); + } response.addCookie(cookie); return; } - - if (setHttpOnlyMethod != null && getCookieSameSite() == null) { - try { - setHttpOnlyMethod.invoke(cookie, true); - response.addCookie(cookie); - return; - } catch (IllegalAccessException iae) { - debug.warning("IllegalAccessException while trying to add HttpOnly cookie: " + iae.getMessage()); - } catch (InvocationTargetException ite) { - debug.error("An error occurred while trying to add HttpOnly cookie", ite); - } - } + // The servlet Cookie has no SameSite attribute: write the header by hand. StringBuilder sb = new StringBuilder(150); sb.append(cookie.getName()).append("=").append(cookie.getValue()); diff --git a/openam-shared/src/test/java/com/sun/identity/shared/encode/CookieUtilsTest.java b/openam-shared/src/test/java/com/sun/identity/shared/encode/CookieUtilsTest.java index f458680b78..c63023b3a7 100644 --- a/openam-shared/src/test/java/com/sun/identity/shared/encode/CookieUtilsTest.java +++ b/openam-shared/src/test/java/com/sun/identity/shared/encode/CookieUtilsTest.java @@ -12,7 +12,7 @@ * information: "Portions copyright [year] [name of copyright owner]". * * Copyright 2014-2015 ForgeRock AS. - * Portions copyright 2025 3A Systems LLC. + * Portions copyright 2025-2026 3A Systems LLC. */ package com.sun.identity.shared.encode; @@ -24,7 +24,9 @@ import java.util.HashSet; import java.util.Set; +import jakarta.servlet.http.Cookie; import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; import org.testng.annotations.Test; import com.sun.identity.shared.Constants; @@ -52,5 +54,84 @@ public void getMatchingCookieDomains() { })) ); } - + + /** Runs {@code body} with the deployment's Secure/HttpOnly/SameSite cookie settings pinned. */ + private static void withCookieSettings(boolean secure, boolean httpOnly, String sameSite, Runnable body) { + boolean savedSecure = CookieUtils.secureCookie; + boolean savedHttpOnly = CookieUtils.cookieHttpOnly; + String savedSameSite = CookieUtils.cookieSameSite; + try { + CookieUtils.secureCookie = secure; + CookieUtils.cookieHttpOnly = httpOnly; + CookieUtils.cookieSameSite = sameSite; + body.run(); + } finally { + CookieUtils.secureCookie = savedSecure; + CookieUtils.cookieHttpOnly = savedHttpOnly; + CookieUtils.cookieSameSite = savedSameSite; + } + } + + @Test + public void newCookieCarriesTheSecureFlagTheDeploymentConfigures() { + withCookieSettings(true, false, null, () -> + assertTrue(CookieUtils.newCookie("iPlanetDirectoryPro", "AQIC").getSecure())); + withCookieSettings(false, false, null, () -> + assertFalse(CookieUtils.newCookie("iPlanetDirectoryPro", "AQIC").getSecure())); + } + + /** + * HttpOnly used to be applied in addCookieToResponse only, so a cookie handed straight to + * response.addCookie went out without it; the cookie now carries it from creation. + */ + @Test + public void newCookieCarriesTheHttpOnlyFlagTheDeploymentConfigures() { + withCookieSettings(false, true, null, () -> + assertTrue(CookieUtils.newCookie("iPlanetDirectoryPro", "AQIC").isHttpOnly())); + withCookieSettings(false, false, null, () -> + assertFalse(CookieUtils.newCookie("iPlanetDirectoryPro", "AQIC").isHttpOnly())); + } + + @Test + public void addCookieToResponseSetsHttpOnlyOnTheServletCookie() { + withCookieSettings(false, true, null, () -> { + HttpServletResponse response = mock(HttpServletResponse.class); + Cookie cookie = new Cookie("iPlanetDirectoryPro", "AQIC"); + + CookieUtils.addCookieToResponse(response, cookie); + + assertTrue(cookie.isHttpOnly()); + verify(response).addCookie(cookie); + verify(response, never()).addHeader(anyString(), anyString()); + }); + } + + @Test + public void addCookieToResponseLeavesHttpOnlyAloneWhenNotConfigured() { + withCookieSettings(false, false, null, () -> { + HttpServletResponse response = mock(HttpServletResponse.class); + Cookie cookie = new Cookie("iPlanetDirectoryPro", "AQIC"); + + CookieUtils.addCookieToResponse(response, cookie); + + assertFalse(cookie.isHttpOnly()); + verify(response).addCookie(cookie); + }); + } + + /** With SameSite configured the cookie goes out as a hand-built header, flags included. */ + @Test + public void addCookieToResponseWritesTheHeaderItselfWhenSameSiteIsConfigured() { + withCookieSettings(true, true, "Strict", () -> { + HttpServletResponse response = mock(HttpServletResponse.class); + Cookie cookie = new Cookie("iPlanetDirectoryPro", "AQIC"); + cookie.setPath("/"); + + CookieUtils.addCookieToResponse(response, cookie); + + verify(response, never()).addCookie(any(Cookie.class)); + verify(response).addHeader(eq("Set-Cookie"), + eq("iPlanetDirectoryPro=AQIC;path=/;secure;httponly;SameSite=Strict")); + }); + } }