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")); + }); + } }