SAML IdMapperUpdaterSessionListener should be added always and must implement HttpSessionIdListener interface (#32234)

Closes #32084

Signed-off-by: rmartinc <rmartinc@redhat.com>

adapters/saml/wildfly-elytron/pom.xml

@@ -63,7 +63,7 @@
         </dependency>
         <dependency>
             <groupId>org.jboss.spec.javax.servlet</groupId>
-            <artifactId>jboss-servlet-api_3.0_spec</artifactId>
+            <artifactId>jboss-servlet-api_3.1_spec</artifactId>
             <scope>provided</scope>
         </dependency>
         <dependency>

adapters/saml/wildfly-elytron/src/main/java/org/keycloak/adapters/saml/elytron/IdMapperUpdaterSessionListener.java

@@ -24,6 +24,7 @@ import javax.servlet.http.HttpSession;
 import javax.servlet.http.HttpSessionAttributeListener;
 import javax.servlet.http.HttpSessionBindingEvent;
 import javax.servlet.http.HttpSessionEvent;
+import javax.servlet.http.HttpSessionIdListener;
 import javax.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();

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*")) {

pom.xml

@@ -102,6 +102,7 @@
         <jakarta.xml.soap.version>3.0.0</jakarta.xml.soap.version>
 
         <servlet.api.30.version>1.0.2.Final</servlet.api.30.version>
+        <servlet.api.31.version>1.0.2.Final</servlet.api.31.version>
         <jboss-jaxrs-api_2.1_spec>2.0.2.Final</jboss-jaxrs-api_2.1_spec>
         <jboss-servlet-api_4.0_spec>2.0.0.Final</jboss-servlet-api_4.0_spec>
         <jboss.spec.javax.xml.bind.jboss-jaxb-api_2.3_spec.version>2.0.1.Final</jboss.spec.javax.xml.bind.jboss-jaxb-api_2.3_spec.version>
@@ -150,7 +151,6 @@
         <osgi.version>6.0.0</osgi.version>
         <osgi.enterprise.version>5.0.0</osgi.enterprise.version>
         <pax.web.version>7.4.6</pax.web.version>
-        <servlet.api.30.version>1.0.2.Final</servlet.api.30.version>
         <servlet.api.40.version>2.0.0.Final</servlet.api.40.version>
         <twitter4j.version>4.1.2</twitter4j.version>
 
@@ -1435,6 +1435,11 @@
                 <artifactId>jboss-servlet-api_3.0_spec</artifactId>
                 <version>${servlet.api.30.version}</version>
             </dependency>
+            <dependency>
+                <groupId>org.jboss.spec.javax.servlet</groupId>
+                <artifactId>jboss-servlet-api_3.1_spec</artifactId>
+                <version>${servlet.api.31.version}</version>
+            </dependency>
             <dependency>
                 <groupId>org.jboss.spec.javax.ws.rs</groupId>
                 <artifactId>jboss-jaxrs-api_2.1_spec</artifactId>

testsuite/integration-arquillian/test-apps/servlets/pom.xml

@@ -15,7 +15,7 @@
     <dependencies>
         <dependency>
             <groupId>org.jboss.spec.javax.servlet</groupId>
-            <artifactId>jboss-servlet-api_3.0_spec</artifactId>
+            <artifactId>jboss-servlet-api_3.1_spec</artifactId>
             <!-- wherever this is being deployed, the Servlet API spec should already be there.
             Therefore, make this provided to avoid mixing APIs on the target -->
             <scope>provided</scope>

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 javax.servlet.RequestDispatcher;
 import javax.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) {

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletAdapterTest.java

@@ -1988,6 +1988,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();