From 4b18c6a117885ac5eaf56adabd33651dcfb613bc Mon Sep 17 00:00:00 2001 From: mhajas Date: Thu, 27 Jun 2019 12:56:12 +0200 Subject: [PATCH] KEYCLOAK-7207 Check session expiration for SAML session --- .../keycloak/adapters/saml/SamlSession.java | 9 +- .../org/keycloak/adapters/saml/SamlUtil.java | 34 ++++ .../AbstractSamlAuthenticationHandler.java | 7 +- .../saml/jetty/JettySamlSessionStore.java | 6 +- .../saml/servlet/FilterSamlSessionStore.java | 10 +- .../adapters/saml/servlet/SamlFilter.java | 2 +- .../saml/CatalinaSamlSessionStore.java | 3 +- .../undertow/ServletSamlSessionStore.java | 7 +- .../saml/elytron/ElytronSamlSessionStore.java | 7 +- .../adapter/servlet/SendUsernameServlet.java | 28 +++- .../org/keycloak/testsuite/util/Matchers.java | 8 + .../testsuite/util/SamlClientBuilder.java | 8 + .../adapter/AbstractServletsAdapterTest.java | 5 +- .../AbstractSAMLServletAdapterTest.java | 62 ++++++++ .../servlet/SAMLClockSkewAdapterTest.java | 6 +- .../SAMLFilterServletSessionTimeoutTest.java | 27 ++++ .../servlet/SAMLServletAdapterTest.java | 22 +-- .../SAMLServletSessionTimeoutTest.java | 149 ++++++++++++++++++ .../keycloak-saml/web-with-action-filter.xml | 84 ++++++++++ 19 files changed, 440 insertions(+), 44 deletions(-) create mode 100644 testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/AbstractSAMLServletAdapterTest.java create mode 100644 testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLFilterServletSessionTimeoutTest.java create mode 100644 testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletSessionTimeoutTest.java create mode 100644 testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/keycloak-saml/web-with-action-filter.xml diff --git a/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/SamlSession.java b/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/SamlSession.java index ffeade6b4f..76024dddcf 100755 --- a/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/SamlSession.java +++ b/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/SamlSession.java @@ -19,6 +19,7 @@ package org.keycloak.adapters.saml; import org.keycloak.adapters.spi.KeycloakAccount; +import javax.xml.datatype.XMLGregorianCalendar; import java.io.Serializable; import java.util.Set; @@ -30,14 +31,16 @@ public class SamlSession implements Serializable, KeycloakAccount { private SamlPrincipal principal; private Set roles; private String sessionIndex; + private XMLGregorianCalendar sessionNotOnOrAfter; public SamlSession() { } - public SamlSession(SamlPrincipal principal, Set roles, String sessionIndex) { + public SamlSession(SamlPrincipal principal, Set roles, String sessionIndex, XMLGregorianCalendar sessionNotOnOrAfter) { this.principal = principal; this.roles = roles; this.sessionIndex = sessionIndex; + this.sessionNotOnOrAfter = sessionNotOnOrAfter; } public SamlPrincipal getPrincipal() { @@ -52,6 +55,10 @@ public class SamlSession implements Serializable, KeycloakAccount { return sessionIndex; } + public XMLGregorianCalendar getSessionNotOnOrAfter() { + return sessionNotOnOrAfter; + } + @Override public boolean equals(Object other) { if (this == other) diff --git a/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/SamlUtil.java b/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/SamlUtil.java index b551707873..656414ac3d 100755 --- a/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/SamlUtil.java +++ b/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/SamlUtil.java @@ -17,13 +17,17 @@ package org.keycloak.adapters.saml; +import org.jboss.logging.Logger; import org.keycloak.adapters.spi.HttpFacade; import org.keycloak.saml.BaseSAML2BindingBuilder; import org.keycloak.saml.common.constants.GeneralConstants; import org.keycloak.saml.common.exceptions.ConfigurationException; import org.keycloak.saml.common.exceptions.ProcessingException; +import org.keycloak.saml.processing.core.saml.v2.util.XMLTimeUtil; import org.w3c.dom.Document; +import javax.xml.datatype.DatatypeConstants; +import javax.xml.datatype.XMLGregorianCalendar; import java.io.IOException; /** @@ -31,6 +35,9 @@ import java.io.IOException; * @version $Revision: 1 $ */ public class SamlUtil { + + protected static Logger log = Logger.getLogger(SamlUtil.class); + public static void sendSaml(boolean asRequest, HttpFacade httpFacade, String actionUrl, BaseSAML2BindingBuilder binding, Document document, SamlDeployment.Binding samlBinding) throws ProcessingException, ConfigurationException, IOException { @@ -87,4 +94,31 @@ public class SamlUtil { redirectTo = baseUri + "/" + redirectTo; return redirectTo; } + + public static SamlSession validateSamlSession(Object potentialSamlSession, SamlDeployment deployment) { + if (potentialSamlSession == null) { + log.debug("SamlSession was not found in the session"); + return null; + } + + if (!(potentialSamlSession instanceof SamlSession)) { + log.debug("Provided samlSession was not SamlSession type"); + return null; + } + + SamlSession samlSession = (SamlSession) potentialSamlSession; + + XMLGregorianCalendar sessionNotOnOrAfter = samlSession.getSessionNotOnOrAfter(); + if (sessionNotOnOrAfter != null) { + XMLGregorianCalendar now = XMLTimeUtil.getIssueInstant(); + + XMLTimeUtil.add(sessionNotOnOrAfter, deployment.getIDP().getAllowedClockSkew()); // add clockSkew + + if (now.compare(sessionNotOnOrAfter) != DatatypeConstants.LESSER) { + return null; + } + } + + return samlSession; + } } diff --git a/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/profile/AbstractSamlAuthenticationHandler.java b/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/profile/AbstractSamlAuthenticationHandler.java index cb0ac60fdb..fa52cf91d3 100644 --- a/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/profile/AbstractSamlAuthenticationHandler.java +++ b/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/profile/AbstractSamlAuthenticationHandler.java @@ -77,6 +77,7 @@ import java.security.Signature; import java.security.SignatureException; import java.util.*; +import javax.xml.datatype.XMLGregorianCalendar; import javax.xml.namespace.QName; import org.keycloak.dom.saml.v2.SAML2Object; @@ -488,9 +489,9 @@ public abstract class AbstractSamlAuthenticationHandler implements SamlAuthentic URI nameFormat = subjectNameID == null ? null : subjectNameID.getFormat(); String nameFormatString = nameFormat == null ? JBossSAMLURIConstants.NAMEID_FORMAT_UNSPECIFIED.get() : nameFormat.toString(); final SamlPrincipal principal = new SamlPrincipal(assertion, principalName, principalName, nameFormatString, attributes, friendlyAttributes); - String index = authn == null ? null : authn.getSessionIndex(); - final String sessionIndex = index; - SamlSession account = new SamlSession(principal, roles, sessionIndex); + final String sessionIndex = authn == null ? null : authn.getSessionIndex(); + final XMLGregorianCalendar sessionNotOnOrAfter = authn == null ? null : authn.getSessionNotOnOrAfter(); + SamlSession account = new SamlSession(principal, roles, sessionIndex, sessionNotOnOrAfter); sessionStore.saveAccount(account); onCreateSession.onSessionCreated(account); diff --git a/adapters/saml/jetty/jetty-core/src/main/java/org/keycloak/adapters/saml/jetty/JettySamlSessionStore.java b/adapters/saml/jetty/jetty-core/src/main/java/org/keycloak/adapters/saml/jetty/JettySamlSessionStore.java index 0a96e6401e..ad5bd30b1b 100755 --- a/adapters/saml/jetty/jetty-core/src/main/java/org/keycloak/adapters/saml/jetty/JettySamlSessionStore.java +++ b/adapters/saml/jetty/jetty-core/src/main/java/org/keycloak/adapters/saml/jetty/JettySamlSessionStore.java @@ -132,14 +132,12 @@ public class JettySamlSessionStore implements SamlSessionStore { @Override public boolean isLoggedIn() { HttpSession session = request.getSession(false); - if (session == null) return false; if (session == null) { - log.debug("session was null, returning null"); + log.debug("session was null, returning false"); return false; } - SamlSession samlSession = (SamlSession)session.getAttribute(SamlSession.class.getName()); + SamlSession samlSession = SamlUtil.validateSamlSession(session.getAttribute(SamlSession.class.getName()), deployment); if (samlSession == null) { - log.debug("SamlSession was not in session, returning null"); return false; } diff --git a/adapters/saml/servlet-filter/src/main/java/org/keycloak/adapters/saml/servlet/FilterSamlSessionStore.java b/adapters/saml/servlet-filter/src/main/java/org/keycloak/adapters/saml/servlet/FilterSamlSessionStore.java index ee24c88cf5..731bdba26a 100755 --- a/adapters/saml/servlet-filter/src/main/java/org/keycloak/adapters/saml/servlet/FilterSamlSessionStore.java +++ b/adapters/saml/servlet-filter/src/main/java/org/keycloak/adapters/saml/servlet/FilterSamlSessionStore.java @@ -18,6 +18,7 @@ package org.keycloak.adapters.saml.servlet; import org.jboss.logging.Logger; +import org.keycloak.adapters.saml.SamlDeployment; import org.keycloak.adapters.saml.SamlSession; import org.keycloak.adapters.saml.SamlSessionStore; import org.keycloak.adapters.saml.SamlUtil; @@ -41,10 +42,12 @@ import java.util.Set; public class FilterSamlSessionStore extends FilterSessionStore implements SamlSessionStore { protected static Logger log = Logger.getLogger(SamlSessionStore.class); protected final SessionIdMapper idMapper; + private final SamlDeployment deployment; - public FilterSamlSessionStore(HttpServletRequest request, HttpFacade facade, int maxBuffer, SessionIdMapper idMapper) { + public FilterSamlSessionStore(HttpServletRequest request, HttpFacade facade, int maxBuffer, SessionIdMapper idMapper, SamlDeployment deployment) { super(request, facade, maxBuffer); this.idMapper = idMapper; + this.deployment = deployment; } @Override @@ -118,12 +121,11 @@ public class FilterSamlSessionStore extends FilterSessionStore implements SamlSe @Override public boolean isLoggedIn() { HttpSession session = request.getSession(false); - if (session == null) return false; if (session == null) { - log.debug("session was null, returning null"); + log.debug("session was null, returning false"); return false; } - final SamlSession samlSession = (SamlSession)session.getAttribute(SamlSession.class.getName()); + final SamlSession samlSession = SamlUtil.validateSamlSession(session.getAttribute(SamlSession.class.getName()), deployment); if (samlSession == null) { log.debug("SamlSession was not in session, returning null"); return false; diff --git a/adapters/saml/servlet-filter/src/main/java/org/keycloak/adapters/saml/servlet/SamlFilter.java b/adapters/saml/servlet-filter/src/main/java/org/keycloak/adapters/saml/servlet/SamlFilter.java index a06f1036e1..093f203d53 100755 --- a/adapters/saml/servlet-filter/src/main/java/org/keycloak/adapters/saml/servlet/SamlFilter.java +++ b/adapters/saml/servlet-filter/src/main/java/org/keycloak/adapters/saml/servlet/SamlFilter.java @@ -135,7 +135,7 @@ public class SamlFilter implements Filter { log.fine("deployment not configured"); return; } - FilterSamlSessionStore tokenStore = new FilterSamlSessionStore(request, facade, 100000, idMapper); + FilterSamlSessionStore tokenStore = new FilterSamlSessionStore(request, facade, 100000, idMapper, deployment); boolean isEndpoint = request.getRequestURI().substring(request.getContextPath().length()).endsWith("/saml"); SamlAuthenticator authenticator; if (isEndpoint) { diff --git a/adapters/saml/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/saml/CatalinaSamlSessionStore.java b/adapters/saml/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/saml/CatalinaSamlSessionStore.java index a74966b971..98d236965a 100755 --- a/adapters/saml/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/saml/CatalinaSamlSessionStore.java +++ b/adapters/saml/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/saml/CatalinaSamlSessionStore.java @@ -152,9 +152,8 @@ public class CatalinaSamlSessionStore implements SamlSessionStore { log.debug("session was null, returning null"); return false; } - final SamlSession samlSession = (SamlSession)session.getSession().getAttribute(SamlSession.class.getName()); + final SamlSession samlSession = SamlUtil.validateSamlSession(session.getSession().getAttribute(SamlSession.class.getName()), deployment); if (samlSession == null) { - log.debug("SamlSession was not in session, returning null"); return false; } diff --git a/adapters/saml/undertow/src/main/java/org/keycloak/adapters/saml/undertow/ServletSamlSessionStore.java b/adapters/saml/undertow/src/main/java/org/keycloak/adapters/saml/undertow/ServletSamlSessionStore.java index 7e8fb83456..3555b3ec3f 100755 --- a/adapters/saml/undertow/src/main/java/org/keycloak/adapters/saml/undertow/ServletSamlSessionStore.java +++ b/adapters/saml/undertow/src/main/java/org/keycloak/adapters/saml/undertow/ServletSamlSessionStore.java @@ -36,10 +36,13 @@ import org.keycloak.adapters.undertow.SavedRequest; import org.keycloak.adapters.undertow.ServletHttpFacade; import org.keycloak.adapters.undertow.UndertowUserSessionManagement; import org.keycloak.common.util.KeycloakUriBuilder; +import org.keycloak.saml.processing.core.saml.v2.util.XMLTimeUtil; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSession; +import javax.xml.datatype.DatatypeConstants; +import javax.xml.datatype.XMLGregorianCalendar; import java.security.Principal; import java.util.LinkedList; import java.util.List; @@ -162,9 +165,8 @@ public class ServletSamlSessionStore implements SamlSessionStore { return false; } - final SamlSession samlSession = (SamlSession)session.getAttribute(SamlSession.class.getName()); + final SamlSession samlSession = SamlUtil.validateSamlSession(session.getAttribute(SamlSession.class.getName()), deployment); if (samlSession == null) { - log.debug("SamlSession was not found in the session"); return false; } @@ -192,7 +194,6 @@ public class ServletSamlSessionStore implements SamlSessionStore { sessionManagement.login(servletRequestContext.getDeployment().getSessionManager()); String sessionId = changeSessionId(session); idMapperUpdater.map(idMapper, account.getSessionIndex(), account.getPrincipal().getSamlSubject(), sessionId); - } protected String changeSessionId(HttpSession session) { diff --git a/adapters/saml/wildfly-elytron/src/main/java/org/keycloak/adapters/saml/elytron/ElytronSamlSessionStore.java b/adapters/saml/wildfly-elytron/src/main/java/org/keycloak/adapters/saml/elytron/ElytronSamlSessionStore.java index ebe7376aa5..1516f4f311 100644 --- a/adapters/saml/wildfly-elytron/src/main/java/org/keycloak/adapters/saml/elytron/ElytronSamlSessionStore.java +++ b/adapters/saml/wildfly-elytron/src/main/java/org/keycloak/adapters/saml/elytron/ElytronSamlSessionStore.java @@ -31,9 +31,13 @@ import org.keycloak.adapters.saml.SamlUtil; import org.keycloak.adapters.spi.SessionIdMapper; import org.keycloak.adapters.spi.SessionIdMapperUpdater; import org.keycloak.common.util.KeycloakUriBuilder; +import org.keycloak.saml.processing.core.saml.v2.util.XMLTimeUtil; import org.wildfly.security.http.HttpScope; import org.wildfly.security.http.Scope; +import javax.xml.datatype.DatatypeConstants; +import javax.xml.datatype.XMLGregorianCalendar; + /** * @author Bill Burke * @version $Revision: 1 $ @@ -150,9 +154,8 @@ public class ElytronSamlSessionStore implements SamlSessionStore, ElytronTokeSto return false; } - final SamlSession samlSession = (SamlSession)session.getAttachment(SamlSession.class.getName()); + final SamlSession samlSession = SamlUtil.validateSamlSession(session.getAttachment(SamlSession.class.getName()), deployment); if (samlSession == null) { - log.debug("SamlSession was not in session, returning null"); return false; } 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 513787374d..b4794eac4e 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 @@ -21,11 +21,13 @@ package org.keycloak.testsuite.adapter.servlet; import org.jboss.resteasy.annotations.cache.NoCache; 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; +import javax.servlet.http.HttpSession; import javax.ws.rs.GET; import javax.ws.rs.POST; import javax.ws.rs.Path; @@ -35,6 +37,7 @@ import javax.ws.rs.core.Context; import javax.ws.rs.core.HttpHeaders; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; +import javax.xml.datatype.XMLGregorianCalendar; import java.io.IOException; import java.security.Principal; import java.util.Arrays; @@ -149,7 +152,6 @@ public class SendUsernameServlet { return "These roles will be checked: " + checkRolesList.toString(); } - private boolean checkRoles() { for (String role : checkRolesList) { System.out.println("In checkRoles() checking role " + role + " for user " + httpServletRequest.getUserPrincipal().getName()); @@ -175,7 +177,29 @@ public class SendUsernameServlet { sentPrincipal = principal; - return output + principal.getName(); + output += principal.getName() + "\n"; + output += getSessionInfo() + "\n"; + + return output; + } + + private String getSessionInfo() { + HttpSession session = httpServletRequest.getSession(false); + + if (session != null) { + final SamlSession samlSession = (SamlSession) httpServletRequest.getSession(false).getAttribute(SamlSession.class.getName()); + + if (samlSession != null) { + String output = "Session ID: " + samlSession.getSessionIndex() + "\n"; + XMLGregorianCalendar sessionNotOnOrAfter = samlSession.getSessionNotOnOrAfter(); + output += "SessionNotOnOrAfter: " + (sessionNotOnOrAfter == null ? "null" : sessionNotOnOrAfter.toString()); + return output; + } + + return "SamlSession doesn't exists"; + } + + return "Session doesn't exists"; } private String getErrorOutput(Integer statusCode) { diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/Matchers.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/Matchers.java index ffc2de4cbe..e0e8b38781 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/Matchers.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/Matchers.java @@ -17,6 +17,7 @@ package org.keycloak.testsuite.util; import org.keycloak.dom.saml.v2.SAML2Object; +import org.keycloak.dom.saml.v2.protocol.AuthnRequestType; import org.keycloak.dom.saml.v2.protocol.LogoutRequestType; import org.keycloak.dom.saml.v2.protocol.ResponseType; import org.keycloak.dom.saml.v2.protocol.StatusResponseType; @@ -142,6 +143,13 @@ public class Matchers { new SamlLogoutRequestTypeMatcher(URI.create(destination)) ); } + /** + * Matches when the type of a SAML object is instance of {@link AuthnRequestType}. + * @return + */ + public static Matcher isSamlAuthnRequest() { + return instanceOf(AuthnRequestType.class); + } /** * Matches when the SAML status of a {@link StatusResponseType} instance is equal to the given code. diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/SamlClientBuilder.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/SamlClientBuilder.java index c0c6431ed6..77fa65d193 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/SamlClientBuilder.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/SamlClientBuilder.java @@ -122,6 +122,14 @@ public class SamlClientBuilder { return this; } + public SamlClientBuilder assertResponse(Consumer consumer) { + steps.add((client, currentURI, currentResponse, context) -> { + consumer.accept(currentResponse); + return null; + }); + return this; + } + /** * When executing the {@link HttpUriRequest} obtained from the previous step, * do not to follow HTTP redirects but pass the first response immediately diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/AbstractServletsAdapterTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/AbstractServletsAdapterTest.java index 6d8708e09c..413b109754 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/AbstractServletsAdapterTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/AbstractServletsAdapterTest.java @@ -38,6 +38,7 @@ import org.jboss.shrinkwrap.api.asset.UrlAsset; import org.junit.Assert; import static org.keycloak.testsuite.auth.page.AuthRealm.DEMO; +import static org.keycloak.testsuite.util.WaitUtils.waitForPageToLoad; public abstract class AbstractServletsAdapterTest extends AbstractAdapterTest { @@ -132,7 +133,7 @@ public abstract class AbstractServletsAdapterTest extends AbstractAdapterTest { } else { deployment.addAsWebInfResource(keycloakSAMLConfig, "keycloak-saml.xml"); } - + } catch (IOException e) { throw new RuntimeException(e); } @@ -211,7 +212,7 @@ public abstract class AbstractServletsAdapterTest extends AbstractAdapterTest { .build().toString(); DroneUtils.getCurrentDriver().navigate().to(timeOffsetUri); - WaitUtils.waitUntilElement(By.tagName("body")).is().visible(); + waitForPageToLoad(); String pageSource = DroneUtils.getCurrentDriver().getPageSource(); System.out.println(pageSource); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/AbstractSAMLServletAdapterTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/AbstractSAMLServletAdapterTest.java new file mode 100644 index 0000000000..f81a0576af --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/AbstractSAMLServletAdapterTest.java @@ -0,0 +1,62 @@ +package org.keycloak.testsuite.adapter.servlet; + +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.client.methods.HttpUriRequest; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClientBuilder; +import org.apache.http.util.EntityUtils; +import org.keycloak.representations.idm.RealmRepresentation; +import org.keycloak.testsuite.adapter.AbstractServletsAdapterTest; +import org.keycloak.testsuite.adapter.filter.AdapterActionsFilter; +import org.keycloak.testsuite.utils.io.IOUtil; + +import javax.ws.rs.core.UriBuilder; +import java.io.IOException; +import java.util.Arrays; +import java.util.List; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import static org.keycloak.testsuite.auth.page.AuthRealm.SAMLSERVLETDEMO; + +public abstract class AbstractSAMLServletAdapterTest extends AbstractServletsAdapterTest { + + public static final String WEB_XML_WITH_ACTION_FILTER = "web-with-action-filter.xml"; + + @Override + public void setDefaultPageUriParameters() { + super.setDefaultPageUriParameters(); + testRealmPage.setAuthRealm(SAMLSERVLETDEMO); + testRealmSAMLRedirectLoginPage.setAuthRealm(SAMLSERVLETDEMO); + testRealmSAMLPostLoginPage.setAuthRealm(SAMLSERVLETDEMO); + } + + @Override + public void addAdapterTestRealms(List testRealms) { + testRealms.add(IOUtil.loadRealm("/adapter-test/keycloak-saml/testsaml.json")); + testRealms.add(IOUtil.loadRealm("/adapter-test/keycloak-saml/tenant1-realm.json")); + testRealms.add(IOUtil.loadRealm("/adapter-test/keycloak-saml/tenant2-realm.json")); + } + + protected void setAdapterAndServerTimeOffset(int timeOffset, String... servletUris) { + setTimeOffset(timeOffset); + + Arrays.stream(servletUris) + .map(url -> url += "unsecured") + .forEach(servletUri -> { + String url = UriBuilder.fromUri(servletUri) + .queryParam(AdapterActionsFilter.TIME_OFFSET_PARAM, timeOffset) + .build().toString(); + try (CloseableHttpClient client = HttpClientBuilder.create().build()) { + HttpUriRequest request = new HttpGet(url); + CloseableHttpResponse httpResponse = client.execute(request); + + System.out.println(EntityUtils.toString(httpResponse.getEntity())); + httpResponse.close(); + } catch (IOException e) { + throw new RuntimeException("Cannot change time on url " + url, e); + } + }); + } +} diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLClockSkewAdapterTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLClockSkewAdapterTest.java index 10eccc813b..21f130f184 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLClockSkewAdapterTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLClockSkewAdapterTest.java @@ -53,7 +53,7 @@ import static org.keycloak.testsuite.util.SamlClient.Binding.POST; @AppServerContainer(ContainerConstants.APP_SERVER_JETTY92) @AppServerContainer(ContainerConstants.APP_SERVER_JETTY93) @AppServerContainer(ContainerConstants.APP_SERVER_JETTY94) -public class SAMLClockSkewAdapterTest extends AbstractServletsAdapterTest { +public class SAMLClockSkewAdapterTest extends AbstractSAMLServletAdapterTest { @Page protected SalesPostClockSkewServlet salesPostClockSkewServletPage; private static final String DEPLOYMENT_NAME_3_SEC = SalesPostClockSkewServlet.DEPLOYMENT_NAME + "_3Sec"; @@ -88,13 +88,13 @@ public class SAMLClockSkewAdapterTest extends AbstractServletsAdapterTest { .login().user(bburkeUser).build() .processSamlResponse(POST) .transformDocument(doc -> { - setAdapterAndServerTimeOffset(timeOffset, salesPostClockSkewServletPage.toString() + "unsecured"); + setAdapterAndServerTimeOffset(timeOffset, salesPostClockSkewServletPage.toString()); return doc; }).build().executeAndTransform(resp -> EntityUtils.toString(resp.getEntity())); Assert.assertThat(resultPage, matcher); } finally { - setAdapterAndServerTimeOffset(0); + setAdapterAndServerTimeOffset(0, salesPostClockSkewServletPage.toString()); } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLFilterServletSessionTimeoutTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLFilterServletSessionTimeoutTest.java new file mode 100644 index 0000000000..83d83c6c99 --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLFilterServletSessionTimeoutTest.java @@ -0,0 +1,27 @@ +package org.keycloak.testsuite.adapter.servlet; + +import org.junit.Assume; +import org.junit.BeforeClass; +import org.keycloak.testsuite.arquillian.annotation.AppServerContainer; +import org.keycloak.testsuite.utils.annotation.UseServletFilter; +import org.keycloak.testsuite.utils.arquillian.ContainerConstants; + +/** + * @author mhajas + */ +@AppServerContainer(ContainerConstants.APP_SERVER_UNDERTOW) +@AppServerContainer(ContainerConstants.APP_SERVER_WILDFLY) +@AppServerContainer(ContainerConstants.APP_SERVER_WILDFLY_DEPRECATED) +@AppServerContainer(ContainerConstants.APP_SERVER_EAP) +@AppServerContainer(ContainerConstants.APP_SERVER_EAP6) +@AppServerContainer(ContainerConstants.APP_SERVER_EAP71) +@UseServletFilter(filterName = "saml-filter", filterClass = "org.keycloak.adapters.saml.servlet.SamlFilter", + filterDependency = "org.keycloak:keycloak-saml-servlet-filter-adapter") +public class SAMLFilterServletSessionTimeoutTest extends SAMLServletSessionTimeoutTest { + + @BeforeClass + public static void enabled() { + String appServerJavaHome = System.getProperty("app.server.java.home", ""); + Assume.assumeFalse(appServerJavaHome.contains("1.7") || appServerJavaHome.contains("ibm-java-70")); + } +} 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 94b6d56b8b..e8f957e090 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 @@ -18,6 +18,7 @@ package org.keycloak.testsuite.adapter.servlet; import static javax.ws.rs.core.Response.Status.OK; +import static org.assertj.core.api.Assertions.assertThat; import static org.hamcrest.Matchers.*; import static org.keycloak.OAuth2Constants.PASSWORD; import static org.keycloak.testsuite.admin.ApiUtil.createUserAndResetPasswordWithAdminClient; @@ -50,6 +51,8 @@ import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.stream.Collectors; import javax.ws.rs.client.Client; @@ -175,7 +178,7 @@ import org.xml.sax.SAXException; @AppServerContainer(ContainerConstants.APP_SERVER_TOMCAT7) @AppServerContainer(ContainerConstants.APP_SERVER_TOMCAT8) @AppServerContainer(ContainerConstants.APP_SERVER_TOMCAT9) -public class SAMLServletAdapterTest extends AbstractServletsAdapterTest { +public class SAMLServletAdapterTest extends AbstractSAMLServletAdapterTest { @Page protected BadClientSalesPostSigServlet badClientSalesPostSigServletPage; @@ -432,21 +435,6 @@ public class SAMLServletAdapterTest extends AbstractServletsAdapterTest { SendUsernameServlet.class, SamlMultiTenantResolver.class); } - @Override - public void addAdapterTestRealms(List testRealms) { - testRealms.add(IOUtil.loadRealm("/adapter-test/keycloak-saml/testsaml.json")); - testRealms.add(IOUtil.loadRealm("/adapter-test/keycloak-saml/tenant1-realm.json")); - testRealms.add(IOUtil.loadRealm("/adapter-test/keycloak-saml/tenant2-realm.json")); - } - - @Override - public void setDefaultPageUriParameters() { - super.setDefaultPageUriParameters(); - testRealmPage.setAuthRealm(SAMLSERVLETDEMO); - testRealmSAMLRedirectLoginPage.setAuthRealm(SAMLSERVLETDEMO); - testRealmSAMLPostLoginPage.setAuthRealm(SAMLSERVLETDEMO); - } - private void assertForbidden(AbstractPage page, String expectedNotContains) { page.navigateTo(); waitUntilElement(By.xpath("//body")).text().not().contains(expectedNotContains); @@ -1583,7 +1571,7 @@ public class SAMLServletAdapterTest extends AbstractServletsAdapterTest { .execute(r -> { Assert.assertThat(r, statusCodeIsHC(Response.Status.OK)); - Assert.assertThat(r, bodyHC(allOf(containsString("principal="), not(containsString("500"))))); + Assert.assertThat(r, bodyHC(containsString("principal="))); }); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletSessionTimeoutTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletSessionTimeoutTest.java new file mode 100644 index 0000000000..53184b7f09 --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLServletSessionTimeoutTest.java @@ -0,0 +1,149 @@ +package org.keycloak.testsuite.adapter.servlet; + +import org.apache.http.util.EntityUtils; +import org.jboss.arquillian.container.test.api.Deployment; +import org.jboss.arquillian.graphene.page.Page; +import org.jboss.shrinkwrap.api.spec.WebArchive; +import org.junit.Test; +import org.keycloak.adapters.rotation.PublicKeyLocator; +import org.keycloak.dom.saml.v2.SAML2Object; +import org.keycloak.dom.saml.v2.assertion.AuthnStatementType; +import org.keycloak.dom.saml.v2.assertion.StatementAbstractType; +import org.keycloak.dom.saml.v2.protocol.ResponseType; +import org.keycloak.models.utils.SessionTimeoutHelper; +import org.keycloak.saml.common.constants.JBossSAMLURIConstants; +import org.keycloak.saml.processing.core.saml.v2.util.XMLTimeUtil; +import org.keycloak.testsuite.adapter.filter.AdapterActionsFilter; +import org.keycloak.testsuite.adapter.page.Employee2Servlet; +import org.keycloak.testsuite.arquillian.annotation.AppServerContainer; +import org.keycloak.testsuite.util.Matchers; +import org.keycloak.testsuite.util.SamlClient; +import org.keycloak.testsuite.util.SamlClientBuilder; +import org.keycloak.testsuite.utils.arquillian.ContainerConstants; + +import javax.xml.datatype.XMLGregorianCalendar; +import java.util.Set; +import java.util.concurrent.atomic.AtomicReference; + +import static org.hamcrest.Matchers.allOf; +import static org.hamcrest.Matchers.containsString; +import static org.junit.Assert.assertThat; +import static org.keycloak.testsuite.util.Matchers.bodyHC; + + +@AppServerContainer(ContainerConstants.APP_SERVER_UNDERTOW) +@AppServerContainer(ContainerConstants.APP_SERVER_WILDFLY) +@AppServerContainer(ContainerConstants.APP_SERVER_WILDFLY_DEPRECATED) +@AppServerContainer(ContainerConstants.APP_SERVER_EAP) +@AppServerContainer(ContainerConstants.APP_SERVER_EAP6) +@AppServerContainer(ContainerConstants.APP_SERVER_EAP71) +@AppServerContainer(ContainerConstants.APP_SERVER_TOMCAT7) +@AppServerContainer(ContainerConstants.APP_SERVER_TOMCAT8) +@AppServerContainer(ContainerConstants.APP_SERVER_TOMCAT9) +@AppServerContainer(ContainerConstants.APP_SERVER_JETTY92) +@AppServerContainer(ContainerConstants.APP_SERVER_JETTY93) +@AppServerContainer(ContainerConstants.APP_SERVER_JETTY94) +public class SAMLServletSessionTimeoutTest extends AbstractSAMLServletAdapterTest { + + @Page + protected Employee2Servlet employee2ServletPage; + + @Deployment(name = Employee2Servlet.DEPLOYMENT_NAME) + protected static WebArchive employee2() { + return samlServletDeployment(Employee2Servlet.DEPLOYMENT_NAME, WEB_XML_WITH_ACTION_FILTER, SendUsernameServlet.class, AdapterActionsFilter.class, PublicKeyLocator.class); + } + + private static final int SESSION_LENGTH_IN_SECONDS = 120; + private static final int KEYCLOAK_SESSION_TIMEOUT = 1922; /** 1800 session max + 120 {@link SessionTimeoutHelper#IDLE_TIMEOUT_WINDOW_SECONDS} */ + + private AtomicReference sessionNotOnOrAfter = new AtomicReference<>(); + + private SAML2Object addSessionNotOnOrAfter(SAML2Object ob) { + assertThat(ob, Matchers.isSamlResponse(JBossSAMLURIConstants.STATUS_SUCCESS)); + ResponseType resp = (ResponseType) ob; + + Set statements = resp.getAssertions().get(0).getAssertion().getStatements(); + + AuthnStatementType authType = (AuthnStatementType) statements.stream() + .filter(statement -> statement instanceof AuthnStatementType) + .findFirst().orElse(new AuthnStatementType(XMLTimeUtil.getIssueInstant())); + XMLGregorianCalendar sessionTimeout = XMLTimeUtil.add(XMLTimeUtil.getIssueInstant(), SESSION_LENGTH_IN_SECONDS * 1000); + sessionNotOnOrAfter.set(sessionTimeout.toString()); + authType.setSessionNotOnOrAfter(sessionTimeout); + resp.getAssertions().get(0).getAssertion().addStatement(authType); + + return ob; + } + + private SamlClientBuilder beginAuthenticationAndLogin() { + return new SamlClientBuilder() + .navigateTo(employee2ServletPage.buildUri()) + .processSamlResponse(SamlClient.Binding.POST) // Process AuthnResponse + .build() + + .login().user(bburkeUser) + .build(); + } + + @Test + public void employee2TestSAMLRefreshingSession() { + sessionNotOnOrAfter.set(null); + + beginAuthenticationAndLogin() + .processSamlResponse(SamlClient.Binding.POST) // Update response with SessionNotOnOrAfter + .transformObject(this::addSessionNotOnOrAfter) + .build() + .addStep(() -> setAdapterAndServerTimeOffset(100, employee2ServletPage.toString())) // Move in time right before sessionNotOnOrAfter + .navigateTo(employee2ServletPage.buildUri()) + .assertResponse(response -> // Check that session is still valid within sessionTimeout limit + assertThat(response, // Cannot use matcher as sessionNotOnOrAfter variable is not set in time of creating matcher + bodyHC(allOf(containsString("principal=bburke"), + containsString("SessionNotOnOrAfter: " + sessionNotOnOrAfter.get()))))) + .addStep(() -> setAdapterAndServerTimeOffset(SESSION_LENGTH_IN_SECONDS, employee2ServletPage.toString())) // Move in time after sessionNotOnOrAfter + .navigateTo(employee2ServletPage.buildUri()) + .processSamlResponse(SamlClient.Binding.POST) // AuthnRequest should be send + .transformObject(ob -> { + assertThat(ob, Matchers.isSamlAuthnRequest()); + return ob; + }) + .build() + + .followOneRedirect() // There is a redirect on Keycloak side + .processSamlResponse(SamlClient.Binding.POST) // Process the response from keyclok, no login form should be present since session on keycloak side is still valid + .build() + .assertResponse(bodyHC(containsString("principal=bburke"))) + .execute(); + + setAdapterAndServerTimeOffset(0, employee2ServletPage.toString()); + } + + @Test + public void employee2TestSAMLSessionTimeoutOnBothSides() { + sessionNotOnOrAfter.set(null); + + beginAuthenticationAndLogin() + .processSamlResponse(SamlClient.Binding.POST) // Update response with SessionNotOnOrAfter + .transformObject(this::addSessionNotOnOrAfter) + .build() + + .navigateTo(employee2ServletPage.buildUri()) + .assertResponse(response -> // Check that session is still valid within sessionTimeout limit + assertThat(response, // Cannot use matcher as sessionNotOnOrAfter variable is not set in time of creating matcher + bodyHC(allOf(containsString("principal=bburke"), + containsString("SessionNotOnOrAfter: " + sessionNotOnOrAfter.get()))))) + .addStep(() -> setAdapterAndServerTimeOffset(KEYCLOAK_SESSION_TIMEOUT, employee2ServletPage.toString())) // Move in time after sessionNotOnOrAfter and keycloak session + .navigateTo(employee2ServletPage.buildUri()) + .processSamlResponse(SamlClient.Binding.POST) // AuthnRequest should be send + .transformObject(ob -> { + assertThat(ob, Matchers.isSamlAuthnRequest()); + return ob; + }) + .build() + + .followOneRedirect() // There is a redirect on Keycloak side + .assertResponse(Matchers.bodyHC(containsString("form id=\"kc-form-login\""))) + .execute(); + + setAdapterAndServerTimeOffset(0, employee2ServletPage.toString()); + } +} diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/keycloak-saml/web-with-action-filter.xml b/testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/keycloak-saml/web-with-action-filter.xml new file mode 100644 index 0000000000..de1b5adb3c --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/keycloak-saml/web-with-action-filter.xml @@ -0,0 +1,84 @@ + + + + + + %CONTEXT_PATH% + + + javax.ws.rs.core.Application + 1 + + + javax.ws.rs.core.Application + /* + + + + AdapterActionsFilter + org.keycloak.testsuite.adapter.filter.AdapterActionsFilter + + + AdapterActionsFilter + /* + + + + /error.html + + + + + Application + /* + + + manager + + + + + Unsecured-setCheckRoles + /setCheckRoles/* + + + + + Unsecured-uncheckRoles + /uncheckRoles/* + + + + + Unsecured + /unsecured/* + + + + + KEYCLOAK-SAML + demo + + + + manager + +