diff --git a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/SAML2LogoutAPIAuthenticatorCmd.java b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/SAML2LogoutAPIAuthenticatorCmd.java index ca46bef4b5a3..6c5d0242307f 100644 --- a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/SAML2LogoutAPIAuthenticatorCmd.java +++ b/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 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 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 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 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 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; diff --git a/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/api/command/SAML2LogoutAPIAuthenticatorCmdTest.java b/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/api/command/SAML2LogoutAPIAuthenticatorCmdTest.java index 2060d0baf311..d02120bd0e5a 100644 --- a/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/api/command/SAML2LogoutAPIAuthenticatorCmdTest.java +++ b/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()); + } }