diff --git a/adapters/saml/jetty/jetty-core/src/main/java/org/keycloak/adapters/saml/jetty/AbstractSamlAuthenticator.java b/adapters/saml/jetty/jetty-core/src/main/java/org/keycloak/adapters/saml/jetty/AbstractSamlAuthenticator.java index c0543e488a..a946c1f955 100755 --- a/adapters/saml/jetty/jetty-core/src/main/java/org/keycloak/adapters/saml/jetty/AbstractSamlAuthenticator.java +++ b/adapters/saml/jetty/jetty-core/src/main/java/org/keycloak/adapters/saml/jetty/AbstractSamlAuthenticator.java @@ -66,6 +66,7 @@ import java.io.IOException; import java.io.InputStream; import java.util.HashSet; import java.util.Set; +import java.util.regex.Pattern; /** * @author Bill Burke @@ -119,16 +120,27 @@ public abstract class AbstractSamlAuthenticator extends LoginAuthenticator { tokenStore.logoutAccount(); } - protected void forwardToLogoutPage(Request request, HttpServletResponse response, SamlDeployment deployment) { - RequestDispatcher disp = request.getRequestDispatcher(deployment.getLogoutPage()); - //make sure the login page is never cached - response.setHeader("Cache-Control", "no-cache, no-store, must-revalidate"); - response.setHeader("Pragma", "no-cache"); - response.setHeader("Expires", "0"); + private static final Pattern PROTOCOL_PATTERN = Pattern.compile("^[a-zA-Z][a-zA-Z0-9+.-]*:"); + protected void forwardToLogoutPage(Request request, HttpServletResponse response, SamlDeployment deployment) { + final String location = deployment.getLogoutPage(); try { - disp.forward(request, response); + //make sure the login page is never cached + response.setHeader("Cache-Control", "no-cache, no-store, must-revalidate"); + response.setHeader("Pragma", "no-cache"); + response.setHeader("Expires", "0"); + + if (location == null) { + log.warn("Logout page not set."); + response.sendError(HttpServletResponse.SC_NOT_FOUND); + } else if (PROTOCOL_PATTERN.matcher(location).find()) { + response.sendRedirect(response.encodeRedirectURL(location)); + } else { + RequestDispatcher disp = request.getRequestDispatcher(location); + + disp.forward(request, response); + } } catch (ServletException e) { throw new RuntimeException(e); } catch (IOException e) { diff --git a/adapters/saml/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/saml/AbstractSamlAuthenticatorValve.java b/adapters/saml/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/saml/AbstractSamlAuthenticatorValve.java index c356391af7..cc3abc2a17 100755 --- a/adapters/saml/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/saml/AbstractSamlAuthenticatorValve.java +++ b/adapters/saml/tomcat/tomcat-core/src/main/java/org/keycloak/adapters/saml/AbstractSamlAuthenticatorValve.java @@ -44,7 +44,7 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; import java.lang.reflect.*; -import java.util.Map; +import java.util.regex.Pattern; /** * Keycloak authentication valve @@ -189,16 +189,27 @@ public abstract class AbstractSamlAuthenticatorValve extends FormAuthenticator i protected abstract GenericPrincipalFactory createPrincipalFactory(); protected abstract boolean forwardToErrorPageInternal(Request request, HttpServletResponse response, Object loginConfig) throws IOException; - protected void forwardToLogoutPage(Request request, HttpServletResponse response,SamlDeployment deployment) { - RequestDispatcher disp = request.getRequestDispatcher(deployment.getLogoutPage()); - //make sure the login page is never cached - response.setHeader("Cache-Control", "no-cache, no-store, must-revalidate"); - response.setHeader("Pragma", "no-cache"); - response.setHeader("Expires", "0"); + private static final Pattern PROTOCOL_PATTERN = Pattern.compile("^[a-zA-Z][a-zA-Z0-9+.-]*:"); + protected void forwardToLogoutPage(Request request, HttpServletResponse response, SamlDeployment deployment) { + final String location = deployment.getLogoutPage(); try { - disp.forward(request.getRequest(), response); + //make sure the login page is never cached + response.setHeader("Cache-Control", "no-cache, no-store, must-revalidate"); + response.setHeader("Pragma", "no-cache"); + response.setHeader("Expires", "0"); + + if (location == null) { + log.warn("Logout page not set."); + response.sendError(HttpServletResponse.SC_NOT_FOUND); + } else if (PROTOCOL_PATTERN.matcher(location).find()) { + response.sendRedirect(response.encodeRedirectURL(location)); + } else { + RequestDispatcher disp = request.getRequestDispatcher(location); + + disp.forward(request.getRequest(), response); + } } catch (ServletException e) { throw new RuntimeException(e); } catch (IOException e) { diff --git a/adapters/saml/undertow/src/main/java/org/keycloak/adapters/saml/undertow/AbstractSamlAuthMech.java b/adapters/saml/undertow/src/main/java/org/keycloak/adapters/saml/undertow/AbstractSamlAuthMech.java index 614a83957e..090734368e 100755 --- a/adapters/saml/undertow/src/main/java/org/keycloak/adapters/saml/undertow/AbstractSamlAuthMech.java +++ b/adapters/saml/undertow/src/main/java/org/keycloak/adapters/saml/undertow/AbstractSamlAuthMech.java @@ -33,6 +33,9 @@ import org.keycloak.adapters.spi.AuthOutcome; import org.keycloak.adapters.spi.HttpFacade; import org.keycloak.adapters.undertow.UndertowHttpFacade; import org.keycloak.adapters.undertow.UndertowUserSessionManagement; +import java.util.logging.Level; +import java.util.logging.Logger; +import java.util.regex.Pattern; /** * Abstract base class for a Keycloak-enabled Undertow AuthenticationMechanism. @@ -40,6 +43,9 @@ import org.keycloak.adapters.undertow.UndertowUserSessionManagement; * @author Stan Silvert ssilvert@redhat.com (C) 2014 Red Hat Inc. */ public abstract class AbstractSamlAuthMech implements AuthenticationMechanism { + + private static final Logger LOG = Logger.getLogger(AbstractSamlAuthMech.class.getName()); + public static final AttachmentKey KEYCLOAK_CHALLENGE_ATTACHMENT_KEY = AttachmentKey.create(AuthChallenge.class); protected SamlDeploymentContext deploymentContext; protected UndertowUserSessionManagement sessionManagement; @@ -68,11 +74,22 @@ public abstract class AbstractSamlAuthMech implements AuthenticationMechanism { return StatusCodes.TEMPORARY_REDIRECT; } + private static final Pattern PROTOCOL_PATTERN = Pattern.compile("^[a-zA-Z][a-zA-Z0-9+.-]*:"); + static void sendRedirect(final HttpServerExchange exchange, final String location) { - // TODO - String concatenation to construct URLS is extremely error prone - switch to a URI which will better - // handle this. - String loc = exchange.getRequestScheme() + "://" + exchange.getHostAndPort() + location; - exchange.getResponseHeaders().put(Headers.LOCATION, loc); + if (location == null) { + LOG.log(Level.WARNING, "Logout page not set."); + exchange.setStatusCode(StatusCodes.NOT_FOUND); + exchange.endExchange(); + return; + } + + if (PROTOCOL_PATTERN.matcher(location).find()) { + exchange.getResponseHeaders().put(Headers.LOCATION, location); + } else { + String loc = exchange.getRequestScheme() + "://" + exchange.getHostAndPort() + location; + exchange.getResponseHeaders().put(Headers.LOCATION, loc); + } } protected void registerNotifications(final SecurityContext securityContext) { @@ -142,7 +159,7 @@ public abstract class AbstractSamlAuthMech implements AuthenticationMechanism { protected void redirectLogout(SamlDeployment deployment, HttpServerExchange exchange) { String page = deployment.getLogoutPage(); sendRedirect(exchange, page); - exchange.setResponseCode(302); + exchange.setStatusCode(StatusCodes.FOUND); exchange.endExchange(); } diff --git a/adapters/saml/undertow/src/main/java/org/keycloak/adapters/saml/undertow/ServletSamlAuthMech.java b/adapters/saml/undertow/src/main/java/org/keycloak/adapters/saml/undertow/ServletSamlAuthMech.java index b0a73391c3..df1a471b07 100755 --- a/adapters/saml/undertow/src/main/java/org/keycloak/adapters/saml/undertow/ServletSamlAuthMech.java +++ b/adapters/saml/undertow/src/main/java/org/keycloak/adapters/saml/undertow/ServletSamlAuthMech.java @@ -113,7 +113,11 @@ public class ServletSamlAuthMech extends AbstractSamlAuthMech { @Override protected void redirectLogout(SamlDeployment deployment, HttpServerExchange exchange) { - servePage(exchange, deployment.getLogoutPage()); + exchange.getResponseHeaders().add(Headers.CACHE_CONTROL, "no-cache, no-store, must-revalidate"); + exchange.getResponseHeaders().add(Headers.PRAGMA, "no-cache"); + exchange.getResponseHeaders().add(Headers.EXPIRES, "0"); + + super.redirectLogout(deployment, exchange); } @Override diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/adapter/page/AdapterLogoutPage.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/adapter/page/AdapterLogoutPage.java new file mode 100644 index 0000000000..baac09a950 --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/adapter/page/AdapterLogoutPage.java @@ -0,0 +1,62 @@ +/* + * Copyright 2016 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.keycloak.testsuite.adapter.page; + +import org.keycloak.testsuite.page.AbstractPageWithInjectedUrl; +import org.jboss.arquillian.container.test.api.OperateOnDeployment; +import org.jboss.arquillian.test.api.ArquillianResource; + +import java.net.URL; +import org.jboss.shrinkwrap.api.ShrinkWrap; +import org.jboss.shrinkwrap.api.asset.StringAsset; +import org.jboss.shrinkwrap.api.spec.WebArchive; + +/** + * @author mhajas + */ +public class AdapterLogoutPage extends AbstractPageWithInjectedUrl { + + public static final String DEPLOYMENT_NAME = "logout"; + + private static final String WEB_XML = + "" + + " " + DEPLOYMENT_NAME + "" + + ""; + + private static final String LOGOUT_PAGE_HTML = "Logged out"; + + public static final WebArchive createDeployment() { + return ShrinkWrap.create(WebArchive.class, AdapterLogoutPage.DEPLOYMENT_NAME + ".war") + .addAsWebInfResource(new StringAsset(WEB_XML), "web.xml") + .add(new StringAsset(LOGOUT_PAGE_HTML), "/index.html"); + } + + @ArquillianResource + @OperateOnDeployment(DEPLOYMENT_NAME) + private URL url; + + @Override + public URL getInjectedUrl() { + return url; + } + + @Override + public boolean isCurrent() { + return driver.getCurrentUrl().startsWith(url.toString()); + } +} diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/DeploymentArchiveProcessor.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/DeploymentArchiveProcessor.java index cd77c5b64b..f81cf56f7a 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/DeploymentArchiveProcessor.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/DeploymentArchiveProcessor.java @@ -170,11 +170,14 @@ public class DeploymentArchiveProcessor implements ApplicationArchiveProcessor { modifyDocElementAttribute(doc, "SingleLogoutService", "postBindingUrl", "http", "https"); modifyDocElementAttribute(doc, "SingleLogoutService", "redirectBindingUrl", "8080", System.getProperty("auth.server.https.port")); modifyDocElementAttribute(doc, "SingleLogoutService", "redirectBindingUrl", "http", "https"); + modifyDocElementAttribute(doc, "SP", "logoutPage", "8081", System.getProperty("app.server.https.port")); + modifyDocElementAttribute(doc, "SP", "logoutPage", "http", "https"); } else { modifyDocElementAttribute(doc, "SingleSignOnService", "bindingUrl", "8080", System.getProperty("auth.server.http.port")); modifyDocElementAttribute(doc, "SingleSignOnService", "assertionConsumerServiceUrl", "8081", System.getProperty("app.server.http.port")); modifyDocElementAttribute(doc, "SingleLogoutService", "postBindingUrl", "8080", System.getProperty("auth.server.http.port")); modifyDocElementAttribute(doc, "SingleLogoutService", "redirectBindingUrl", "8080", System.getProperty("auth.server.http.port")); + modifyDocElementAttribute(doc, "SP", "logoutPage", "8081", System.getProperty("app.server.http.port")); } archive.add(new StringAsset(IOUtil.documentToString(doc)), adapterConfigPath); @@ -244,8 +247,13 @@ public class DeploymentArchiveProcessor implements ApplicationArchiveProcessor { } protected void modifyWebXml(Archive archive, TestClass testClass) { - Document webXmlDoc = loadXML( - archive.get(WEBXML_PATH).getAsset().openStream()); + Document webXmlDoc; + try { + webXmlDoc = loadXML( + archive.get(WEBXML_PATH).getAsset().openStream()); + } catch (Exception ex) { + throw new RuntimeException("Error when processing " + archive.getName(), ex); + } if (isTomcatAppServer(testClass.getJavaClass())) { modifyDocElementValue(webXmlDoc, "auth-method", "KEYCLOAK", "BASIC"); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/AbstractSAMLServletsAdapterTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/AbstractSAMLServletsAdapterTest.java index 80868cfe00..aab94a13e6 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/AbstractSAMLServletsAdapterTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/AbstractSAMLServletsAdapterTest.java @@ -123,6 +123,8 @@ import javax.xml.xpath.XPath; import javax.xml.xpath.XPathConstants; import javax.xml.xpath.XPathExpression; import javax.xml.xpath.XPathFactory; +import org.jboss.shrinkwrap.api.ShrinkWrap; +import org.jboss.shrinkwrap.api.asset.StringAsset; import org.w3c.dom.Node; import org.w3c.dom.NodeList; @@ -234,6 +236,9 @@ public abstract class AbstractSAMLServletsAdapterTest extends AbstractServletsAd @Page protected SalesPostAutodetectServlet salesPostAutodetectServletPage; + @Page + protected AdapterLogoutPage adapterLogoutPage; + @Page protected EcpSP ecpSPPage; @@ -362,7 +367,13 @@ public abstract class AbstractSAMLServletsAdapterTest extends AbstractServletsAd @Deployment(name = EmployeeServlet.DEPLOYMENT_NAME) protected static WebArchive employeeServlet() { - return samlServletDeployment(EmployeeServlet.DEPLOYMENT_NAME, "employee/WEB-INF/web.xml", SamlSPFacade.class, ServletTestUtils.class); + return samlServletDeployment(EmployeeServlet.DEPLOYMENT_NAME, "employee/WEB-INF/web.xml", SamlSPFacade.class, ServletTestUtils.class) + .add(new StringAsset("Logged out"), "/logout.jsp"); + } + + @Deployment(name = AdapterLogoutPage.DEPLOYMENT_NAME) + protected static WebArchive logoutWar() { + return AdapterLogoutPage.createDeployment(); } @Deployment(name = SalesPostAutodetectServlet.DEPLOYMENT_NAME) @@ -641,6 +652,18 @@ public abstract class AbstractSAMLServletsAdapterTest extends AbstractServletsAd testSuccessfulAndUnauthorizedLogin(employeeSigFrontServletPage, testRealmSAMLRedirectLoginPage); } + @Test + public void testLogoutRedirectToExternalPage() throws Exception { + employeeServletPage.navigateTo(); + assertCurrentUrlStartsWith(testRealmSAMLPostLoginPage); + testRealmSAMLPostLoginPage.form().login("bburke", "password"); + assertCurrentUrlStartsWith(employeeServletPage); + WaitUtils.waitForPageToLoad(); + + employeeServletPage.logout(); + adapterLogoutPage.assertCurrent(); + } + @Test public void salesMetadataTest() throws Exception { Document doc = loadXML(AbstractSAMLServletsAdapterTest.class.getResourceAsStream("/adapter-test/keycloak-saml/sp-metadata.xml")); @@ -980,7 +1003,7 @@ public abstract class AbstractSAMLServletsAdapterTest extends AbstractServletsAd samlidpInitiatedLoginPage.form().login(bburkeUser); assertCurrentUrlStartsWith(salesPost2ServletPage); - assertTrue(driver.getCurrentUrl().endsWith("/foo")); + assertThat(driver.getCurrentUrl(), endsWith("/foo")); waitUntilElement(By.xpath("//body")).text().contains("principal=bburke"); salesPost2ServletPage.logout(); checkLoggedOut(salesPost2ServletPage, testRealmSAMLPostLoginPage); diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/keycloak-saml/employee/WEB-INF/keycloak-saml.xml b/testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/keycloak-saml/employee/WEB-INF/keycloak-saml.xml index 571d6b8823..a53c54f8fc 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/keycloak-saml/employee/WEB-INF/keycloak-saml.xml +++ b/testsuite/integration-arquillian/tests/base/src/test/resources/adapter-test/keycloak-saml/employee/WEB-INF/keycloak-saml.xml @@ -21,7 +21,7 @@