From a38d3b2f55bccc32239bfe8fb8b2449d0c93a59b Mon Sep 17 00:00:00 2001 From: rmartinc Date: Tue, 13 Aug 2024 12:46:57 +0200 Subject: [PATCH] SAML IdMapperUpdaterSessionListener should be added always and must implement HttpSessionIdListener interface Closes #32084 Signed-off-by: rmartinc --- .../IdMapperUpdaterSessionListener.java | 12 +++++++- .../KeycloakConfigurationServletListener.java | 4 +-- .../adapter/servlet/SendUsernameServlet.java | 13 ++++++--- .../servlet/SAMLServletAdapterTest.java | 28 +++++++++++++++++++ 4 files changed, 50 insertions(+), 7 deletions(-) diff --git a/adapters/saml/wildfly-elytron/src/main/java/org/keycloak/adapters/saml/elytron/IdMapperUpdaterSessionListener.java b/adapters/saml/wildfly-elytron/src/main/java/org/keycloak/adapters/saml/elytron/IdMapperUpdaterSessionListener.java index 475814dcc9..91abdb8514 100644 --- a/adapters/saml/wildfly-elytron/src/main/java/org/keycloak/adapters/saml/elytron/IdMapperUpdaterSessionListener.java +++ b/adapters/saml/wildfly-elytron/src/main/java/org/keycloak/adapters/saml/elytron/IdMapperUpdaterSessionListener.java @@ -24,6 +24,7 @@ import jakarta.servlet.http.HttpSession; import jakarta.servlet.http.HttpSessionAttributeListener; import jakarta.servlet.http.HttpSessionBindingEvent; import jakarta.servlet.http.HttpSessionEvent; +import jakarta.servlet.http.HttpSessionIdListener; import jakarta.servlet.http.HttpSessionListener; import org.jboss.logging.Logger; @@ -31,7 +32,7 @@ import org.jboss.logging.Logger; * * @author hmlnarik */ -public class IdMapperUpdaterSessionListener implements HttpSessionListener, HttpSessionAttributeListener { +public class IdMapperUpdaterSessionListener implements HttpSessionListener, HttpSessionAttributeListener, HttpSessionIdListener { private static final Logger LOG = Logger.getLogger(IdMapperUpdaterSessionListener.class); @@ -56,6 +57,15 @@ public class IdMapperUpdaterSessionListener implements HttpSessionListener, Http unmap(session.getId(), session.getAttribute(SamlSession.class.getName())); } + @Override + public void sessionIdChanged(HttpSessionEvent hse, String oldSessionId) { + LOG.debugf("Session changed ID from %s", oldSessionId); + HttpSession session = hse.getSession(); + Object value = session.getAttribute(SamlSession.class.getName()); + unmap(oldSessionId, value); + map(session.getId(), value); + } + @Override public void attributeAdded(HttpSessionBindingEvent hsbe) { HttpSession session = hsbe.getSession(); diff --git a/adapters/saml/wildfly-elytron/src/main/java/org/keycloak/adapters/saml/elytron/KeycloakConfigurationServletListener.java b/adapters/saml/wildfly-elytron/src/main/java/org/keycloak/adapters/saml/elytron/KeycloakConfigurationServletListener.java index 7c2fa22d4f..4fc84e8706 100644 --- a/adapters/saml/wildfly-elytron/src/main/java/org/keycloak/adapters/saml/elytron/KeycloakConfigurationServletListener.java +++ b/adapters/saml/wildfly-elytron/src/main/java/org/keycloak/adapters/saml/elytron/KeycloakConfigurationServletListener.java @@ -156,14 +156,14 @@ public class KeycloakConfigurationServletListener implements ServletContextListe public void addTokenStoreUpdaters(ServletContext servletContext) { SessionIdMapperUpdater updater = this.idMapperUpdater; + servletContext.addListener(new IdMapperUpdaterSessionListener(idMapper)); // This takes care of HTTP sessions manipulated locally + try { String idMapperSessionUpdaterClasses = servletContext.getInitParameter("keycloak.sessionIdMapperUpdater.classes"); if (idMapperSessionUpdaterClasses == null) { return; } - servletContext.addListener(new IdMapperUpdaterSessionListener(idMapper)); // This takes care of HTTP sessions manipulated locally - updater = SessionIdMapperUpdater.DIRECT; for (String clazz : idMapperSessionUpdaterClasses.split("\\s*,\\s*")) { diff --git a/testsuite/integration-arquillian/test-apps/servlets/src/main/java/org/keycloak/testsuite/adapter/servlet/SendUsernameServlet.java b/testsuite/integration-arquillian/test-apps/servlets/src/main/java/org/keycloak/testsuite/adapter/servlet/SendUsernameServlet.java index 283ce765cf..6b9f998a89 100755 --- a/testsuite/integration-arquillian/test-apps/servlets/src/main/java/org/keycloak/testsuite/adapter/servlet/SendUsernameServlet.java +++ b/testsuite/integration-arquillian/test-apps/servlets/src/main/java/org/keycloak/testsuite/adapter/servlet/SendUsernameServlet.java @@ -23,7 +23,6 @@ import org.keycloak.adapters.saml.SamlAuthenticationError; import org.keycloak.adapters.saml.SamlPrincipal; import org.keycloak.adapters.saml.SamlSession; import org.keycloak.adapters.spi.AuthenticationError; -import org.keycloak.saml.processing.core.saml.v2.constants.X500SAMLProfileConstants; import jakarta.servlet.RequestDispatcher; import jakarta.servlet.http.HttpServletRequest; @@ -45,8 +44,6 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Map.Entry; -import javax.xml.parsers.DocumentBuilderFactory; -import javax.xml.transform.OutputKeys; import javax.xml.transform.Transformer; import javax.xml.transform.TransformerException; import javax.xml.transform.TransformerFactory; @@ -104,11 +101,19 @@ public class SendUsernameServlet { } + @GET + @Path("change-session-id") + public Response changeSessionId() throws IOException { + System.out.println("In SendUsername Servlet changeSessionId()"); + final String sessionId = httpServletRequest.changeSessionId(); + + return Response.ok(sessionId).header(HttpHeaders.CONTENT_TYPE, MediaType.TEXT_PLAIN_TYPE + ";charset=UTF-8").build(); + } + @GET @Path("getAssertionFromDocument") public Response getAssertionFromDocument() throws IOException, TransformerException { sentPrincipal = httpServletRequest.getUserPrincipal(); - DocumentBuilderFactory domFact = DocumentBuilderFactory.newInstance(); Document doc = ((SamlPrincipal) sentPrincipal).getAssertionDocument(); String xml = ""; if (doc != null) { 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 f642223208..ec57ce8d35 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 @@ -1987,6 +1987,34 @@ public class SAMLServletAdapterTest extends AbstractSAMLServletAdapterTest { checkLoggedOut(salesPostSigEmailServletPage, testRealmSAMLPostLoginPage); } + @Test + public void testChangeSessionID() throws Exception { + // login in the employeeDom application + assertSuccessfulLogin(employeeDomServletPage, bburkeUser, testRealmSAMLPostLoginPage, "principal=bburke"); + assertSuccessfullyLoggedIn(employeeDomServletPage, "principal=bburke"); + String sessionId = driver.manage().getCookieNamed("JSESSIONID").getValue(); + + // retrieve the saml document + driver.navigate().to(employeeDomServletPage.getUriBuilder().clone().path("getAssertionFromDocument").build().toURL()); + waitForPageToLoad(); + String xml = getRawPageSource(); + Assert.assertNotEquals("", xml); + + // change the session id + driver.navigate().to(employeeDomServletPage.getUriBuilder().clone().path("change-session-id").build().toURL()); + waitForPageToLoad(); + Assert.assertNotEquals("SessionID has not been changed at login", sessionId, driver.manage().getCookieNamed("JSESSIONID").getValue()); + + // retrieve again the saml document and should be the same as login should be maintained + driver.navigate().to(employeeDomServletPage.getUriBuilder().clone().path("getAssertionFromDocument").build().toURL()); + waitForPageToLoad(); + Assert.assertEquals(xml, getRawPageSource()); + + // logout + employeeDomServletPage.logout(); + checkLoggedOut(employeeDomServletPage, testRealmSAMLPostLoginPage); + } + public static void printDocument(Source doc, OutputStream out) throws IOException, TransformerException { TransformerFactory tf = TransformerFactory.newInstance(); Transformer transformer = tf.newTransformer();