fix: clear session cookies on SAML SLO logout to prevent stale session key

Fixes: #13997
Signed-off-by: waterWang <672684719@qq.com>

Author: waterWang

plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/SAML2LogoutAPIAuthenticatorCmd.java

@@ -41,6 +41,7 @@
 import org.xml.sax.SAXException;
 
 import javax.inject.Inject;
+import javax.servlet.http.Cookie;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpSession;
@@ -89,6 +90,7 @@ public String authenticate(String command, Map<String, Object[]> params, HttpSes
         String responseString = ApiResponseSerializer.toSerializedString(response, responseType);
 
         if (session == null) {
+            clearSessionCookies(req, resp);
             try {
                 resp.sendRedirect(SAML2AuthManager.SAMLCloudStackRedirectionUrl.value());
             } catch (IOException ignored) {
@@ -119,6 +121,7 @@ public String authenticate(String command, Map<String, Object[]> params, HttpSes
             } catch (ConfigurationException | FactoryConfigurationError | ParserConfigurationException | SAXException | IOException | UnmarshallingException e) {
                 logger.error("SAMLResponse processing error: " + e.getMessage());
             }
+            clearSessionCookies(req, resp);
             try {
                 resp.sendRedirect(SAML2AuthManager.SAMLCloudStackRedirectionUrl.value());
             } catch (IOException ignored) {
@@ -131,6 +134,7 @@ public String authenticate(String command, Map<String, Object[]> params, HttpSes
         SAMLProviderMetadata idpMetadata = _samlAuthManager.getIdPMetadata(idpId);
         String nameId = (String) session.getAttribute(SAMLPluginConstants.SAML_NAMEID);
         if (idpMetadata == null || nameId == null || nameId.isEmpty()) {
+            clearSessionCookies(req, resp);
             try {
                 resp.sendRedirect(SAML2AuthManager.SAMLCloudStackRedirectionUrl.value());
             } catch (IOException ignored) {
@@ -142,6 +146,7 @@ public String authenticate(String command, Map<String, Object[]> params, HttpSes
 
         try {
             String redirectUrl = idpMetadata.getSloUrl() + "?SAMLRequest=" + SAMLUtils.encodeSAMLRequest(logoutRequest);
+            clearSessionCookies(req, resp);
             resp.sendRedirect(redirectUrl);
         } catch (MarshallingException | IOException e) {
             logger.error("SAML SLO error: " + e.getMessage());
@@ -152,6 +157,24 @@ public String authenticate(String command, Map<String, Object[]> params, HttpSes
         return responseString;
     }
 
+    /**
+     * Clears the session cookies (JSESSIONID, sessionkey, userid, ...) received from the browser so the
+     * SAML SLO redirect response actually instructs the browser to drop them. ApiServlet runs its cookie
+     * cleanup only after this authenticator returns, but {@code sendRedirect} commits the response first,
+     * so those Set-Cookie headers would be lost and the session key would survive the logout.
+     */
+    private void clearSessionCookies(final HttpServletRequest req, final HttpServletResponse resp) {
+        final Cookie[] cookies = req.getCookies();
+        if (cookies == null) {
+            return;
+        }
+        for (final Cookie cookie : cookies) {
+            cookie.setValue("");
+            cookie.setMaxAge(0);
+            resp.addCookie(cookie);
+        }
+    }
+
     @Override
     public APIAuthenticationType getAPIType() {
         return APIAuthenticationType.LOGOUT_API;

plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/api/command/SAML2LogoutAPIAuthenticatorCmdTest.java

@@ -23,6 +23,7 @@
 import java.net.InetAddress;
 import java.security.cert.X509Certificate;
 
+import javax.servlet.http.Cookie;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpSession;
@@ -85,4 +86,32 @@ public void testAuthenticate() throws Exception {
     public void testGetAPIType() throws Exception {
         Assert.assertTrue(new SAML2LogoutAPIAuthenticatorCmd().getAPIType() == APIAuthenticationType.LOGOUT_API);
     }
+
+    @Test
+    public void testAuthenticateClearsSessionCookiesBeforeRedirect() throws Exception {
+        SAML2LogoutAPIAuthenticatorCmd cmd = new SAML2LogoutAPIAuthenticatorCmd();
+
+        Field apiServerField = SAML2LogoutAPIAuthenticatorCmd.class.getDeclaredField("_apiServer");
+        apiServerField.setAccessible(true);
+        apiServerField.set(cmd, apiServer);
+
+        Field managerField = SAML2LogoutAPIAuthenticatorCmd.class.getDeclaredField("_samlAuthManager");
+        managerField.setAccessible(true);
+        managerField.set(cmd, samlAuthManager);
+
+        Cookie jsessionid = new Cookie("JSESSIONID", "dummy-session-id");
+        Cookie sessionkey = new Cookie("sessionkey", "dummy-session-key");
+        Cookie[] cookies = new Cookie[]{jsessionid, sessionkey};
+        Mockito.when(req.getCookies()).thenReturn(cookies);
+        Mockito.when(session.getAttribute(Mockito.anyString())).thenReturn(null);
+
+        cmd.authenticate("command", null, session, InetAddress.getByName("127.0.0.1"), HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), req, resp);
+
+        Mockito.verify(resp, Mockito.times(1)).sendRedirect(Mockito.any());
+        Mockito.verify(resp, Mockito.times(2)).addCookie(Mockito.any(Cookie.class));
+        Assert.assertEquals(0, jsessionid.getMaxAge());
+        Assert.assertEquals("", jsessionid.getValue());
+        Assert.assertEquals(0, sessionkey.getMaxAge());
+        Assert.assertEquals("", sessionkey.getValue());
+    }
 }