Skip to content

Commit 5e06da2

Browse files
rmartincstianst
authored andcommitted
Honor turnOffChangeSessionIdOnLogin in SAML adapter (#186)
Closes keycloak/keycloak-private#183 Signed-off-by: rmartinc <rmartinc@redhat.com>
1 parent 83f6f1f commit 5e06da2

File tree

3 files changed

+10
-6
lines changed

3 files changed

+10
-6
lines changed

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

+6-6
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,9 @@
3131
import org.keycloak.adapters.spi.SessionIdMapper;
3232
import org.keycloak.adapters.spi.SessionIdMapperUpdater;
3333
import org.keycloak.common.util.KeycloakUriBuilder;
34-
import org.keycloak.saml.processing.core.saml.v2.util.XMLTimeUtil;
3534
import org.wildfly.security.http.HttpScope;
3635
import org.wildfly.security.http.Scope;
3736

38-
import javax.xml.datatype.DatatypeConstants;
39-
import javax.xml.datatype.XMLGregorianCalendar;
40-
4137
/**
4238
* @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
4339
* @version $Revision: 1 $
@@ -174,8 +170,12 @@ public void saveAccount(SamlSession account) {
174170
}
175171

176172
protected String changeSessionId(HttpScope session) {
177-
if (!deployment.turnOffChangeSessionIdOnLogin()) return session.getID();
178-
else return session.getID();
173+
if (!deployment.turnOffChangeSessionIdOnLogin()) {
174+
if (!session.supportsChangeID() || !session.changeID()) {
175+
log.debug("Session ID cannot be changed although turnOffChangeSessionIdOnLogin is set to false");
176+
}
177+
}
178+
return session.getID();
179179
}
180180

181181
@Override

testsuite/integration-arquillian/test-apps/servlets/src/main/java/org/keycloak/testsuite/adapter/servlet/InputServlet.java

+2
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se
4040
String appBase = ServletTestUtils.getUrlBase();
4141
String actionUrl = appBase + "/input-portal/secured/post";
4242

43+
req.getSession(true);
4344
if (req.getRequestURI().endsWith("insecure")) {
4445
if (System.getProperty("insecure.user.principal.unsupported") == null) Assert.assertNotNull(req.getUserPrincipal());
4546
resp.setContentType("text/html");
@@ -65,6 +66,7 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se
6566

6667
@Override
6768
protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
69+
req.getSession(true);
6870
if (!FORM_URLENCODED.equals(req.getContentType())) {
6971
resp.setStatus(HttpServletResponse.SC_BAD_REQUEST);
7072
PrintWriter pw = resp.getWriter();

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

+2
Original file line numberDiff line numberDiff line change
@@ -1127,6 +1127,7 @@ public void idpInitiatedUnauthorizedLoginTest() {
11271127
public void testSavedPostRequest() {
11281128
inputPortalPage.navigateTo();
11291129
assertCurrentUrlStartsWith(inputPortalPage);
1130+
String sessionId = driver.manage().getCookieNamed("JSESSIONID").getValue();
11301131
inputPortalPage.execute("hello");
11311132

11321133
assertCurrentUrlStartsWith(testRealmSAMLPostLoginPage);
@@ -1137,6 +1138,7 @@ public void testSavedPostRequest() {
11371138
// test that user principal and KeycloakSecurityContext available
11381139
driver.navigate().to(inputPortalPage + "/insecure");
11391140
waitUntilElement(By.xpath("//body")).text().contains("Insecure Page");
1141+
Assert.assertNotEquals("SessionID has not been changed at login", sessionId, driver.manage().getCookieNamed("JSESSIONID").getValue());
11401142

11411143
if (System.getProperty("insecure.user.principal.unsupported") == null) waitUntilElement(By.xpath("//body")).text().contains("UserPrincipal");
11421144

0 commit comments

Comments
 (0)