From 9c780e9190be0929e125fd1ce1ace50716702512 Mon Sep 17 00:00:00 2001 From: Ricardo Martin Date: Tue, 20 Aug 2024 09:35:52 +0200 Subject: [PATCH] Honor turnOffChangeSessionIdOnLogin in SAML adapter (#185) Closes keycloak/keycloak-private#183 Signed-off-by: rmartinc --- .../saml/elytron/ElytronSamlSessionStore.java | 12 ++++++------ .../testsuite/adapter/servlet/InputServlet.java | 2 ++ .../adapter/servlet/SAMLServletAdapterTest.java | 2 ++ 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/adapters/saml/wildfly-elytron/src/main/java/org/keycloak/adapters/saml/elytron/ElytronSamlSessionStore.java b/adapters/saml/wildfly-elytron/src/main/java/org/keycloak/adapters/saml/elytron/ElytronSamlSessionStore.java index 26dc328477..6dd6f0e7e3 100644 --- a/adapters/saml/wildfly-elytron/src/main/java/org/keycloak/adapters/saml/elytron/ElytronSamlSessionStore.java +++ b/adapters/saml/wildfly-elytron/src/main/java/org/keycloak/adapters/saml/elytron/ElytronSamlSessionStore.java @@ -31,13 +31,9 @@ import org.keycloak.adapters.saml.SamlUtil; import org.keycloak.adapters.spi.SessionIdMapper; import org.keycloak.adapters.spi.SessionIdMapperUpdater; import org.keycloak.common.util.KeycloakUriBuilder; -import org.keycloak.saml.processing.core.saml.v2.util.XMLTimeUtil; import org.wildfly.security.http.HttpScope; import org.wildfly.security.http.Scope; -import javax.xml.datatype.DatatypeConstants; -import javax.xml.datatype.XMLGregorianCalendar; - /** * @author Bill Burke * @version $Revision: 1 $ @@ -174,8 +170,12 @@ public class ElytronSamlSessionStore implements SamlSessionStore, ElytronTokeSto } protected String changeSessionId(HttpScope session) { - if (!deployment.turnOffChangeSessionIdOnLogin()) return session.getID(); - else return session.getID(); + if (!deployment.turnOffChangeSessionIdOnLogin()) { + if (!session.supportsChangeID() || !session.changeID()) { + log.debug("Session ID cannot be changed although turnOffChangeSessionIdOnLogin is set to false"); + } + } + return session.getID(); } @Override diff --git a/testsuite/integration-arquillian/test-apps/servlets/src/main/java/org/keycloak/testsuite/adapter/servlet/InputServlet.java b/testsuite/integration-arquillian/test-apps/servlets/src/main/java/org/keycloak/testsuite/adapter/servlet/InputServlet.java index 4cc5f08a19..f154ee4492 100644 --- a/testsuite/integration-arquillian/test-apps/servlets/src/main/java/org/keycloak/testsuite/adapter/servlet/InputServlet.java +++ b/testsuite/integration-arquillian/test-apps/servlets/src/main/java/org/keycloak/testsuite/adapter/servlet/InputServlet.java @@ -40,6 +40,7 @@ public class InputServlet extends HttpServlet { String appBase = ServletTestUtils.getUrlBase(); String actionUrl = appBase + "/input-portal/secured/post"; + req.getSession(true); if (req.getRequestURI().endsWith("insecure")) { if (System.getProperty("insecure.user.principal.unsupported") == null) Assert.assertNotNull(req.getUserPrincipal()); resp.setContentType("text/html"); @@ -65,6 +66,7 @@ public class InputServlet extends HttpServlet { @Override protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { + req.getSession(true); if (!FORM_URLENCODED.equals(req.getContentType())) { resp.setStatus(HttpServletResponse.SC_BAD_REQUEST); PrintWriter pw = resp.getWriter(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletAdapterTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletAdapterTest.java index 5920461356..8b1f276d53 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletAdapterTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletAdapterTest.java @@ -1133,6 +1133,7 @@ public class SAMLServletAdapterTest extends AbstractSAMLServletAdapterTest { public void testSavedPostRequest() { inputPortalPage.navigateTo(); assertCurrentUrlStartsWith(inputPortalPage); + String sessionId = driver.manage().getCookieNamed("JSESSIONID").getValue(); inputPortalPage.execute("hello"); assertCurrentUrlStartsWith(testRealmSAMLPostLoginPage); @@ -1143,6 +1144,7 @@ public class SAMLServletAdapterTest extends AbstractSAMLServletAdapterTest { // test that user principal and KeycloakSecurityContext available driver.navigate().to(inputPortalPage + "/insecure"); waitUntilElement(By.xpath("//body")).text().contains("Insecure Page"); + Assert.assertNotEquals("SessionID has not been changed at login", sessionId, driver.manage().getCookieNamed("JSESSIONID").getValue()); if (System.getProperty("insecure.user.principal.unsupported") == null) waitUntilElement(By.xpath("//body")).text().contains("UserPrincipal");