From 1ee76a126f5a4a7e15c5277cf47d0bad0efd78e5 Mon Sep 17 00:00:00 2001 From: Bill Burke Date: Thu, 21 Jan 2016 20:18:07 -0500 Subject: [PATCH] KEYCLOAK-2373 KEYCLOAK-2376 --- .../undertow/KeycloakServletExtension.java | 3 +- .../adapters/undertow/SavedRequest.java | 131 ------------------ .../saml/undertow/SamlServletExtension.java | 2 + .../undertow/ServletSamlSessionStore.java | 2 +- .../undertow/ChangeSessionIdOnLogin.java | 27 ++++ .../adapters/undertow/SavedRequest.java | 65 +++++++++ .../undertow/SessionManagementBridge.java | 3 + pom.xml | 2 +- .../adapter/AdapterTestStrategy.java | 4 +- 9 files changed, 104 insertions(+), 135 deletions(-) delete mode 100755 adapters/oidc/undertow/src/main/java/org/keycloak/adapters/undertow/SavedRequest.java create mode 100755 adapters/spi/undertow-adapter-spi/src/main/java/org/keycloak/adapters/undertow/ChangeSessionIdOnLogin.java create mode 100755 adapters/spi/undertow-adapter-spi/src/main/java/org/keycloak/adapters/undertow/SavedRequest.java diff --git a/adapters/oidc/undertow/src/main/java/org/keycloak/adapters/undertow/KeycloakServletExtension.java b/adapters/oidc/undertow/src/main/java/org/keycloak/adapters/undertow/KeycloakServletExtension.java index fba8b8fa1c..8696a04ddf 100755 --- a/adapters/oidc/undertow/src/main/java/org/keycloak/adapters/undertow/KeycloakServletExtension.java +++ b/adapters/oidc/undertow/src/main/java/org/keycloak/adapters/undertow/KeycloakServletExtension.java @@ -177,7 +177,7 @@ public class KeycloakServletExtension implements ServletExtension { ServletSessionConfig cookieConfig = new ServletSessionConfig(); cookieConfig.setPath(deploymentInfo.getContextPath()); deploymentInfo.setServletSessionConfig(cookieConfig); - + ChangeSessionIdOnLogin.turnOffChangeSessionIdOnLogin(deploymentInfo); deploymentInfo.addListener(new ListenerInfo(UndertowNodesRegistrationManagementWrapper.class, new InstanceFactory() { @Override @@ -204,4 +204,5 @@ public class KeycloakServletExtension implements ServletExtension { } return errorPage; } + } diff --git a/adapters/oidc/undertow/src/main/java/org/keycloak/adapters/undertow/SavedRequest.java b/adapters/oidc/undertow/src/main/java/org/keycloak/adapters/undertow/SavedRequest.java deleted file mode 100755 index bd56cde0b8..0000000000 --- a/adapters/oidc/undertow/src/main/java/org/keycloak/adapters/undertow/SavedRequest.java +++ /dev/null @@ -1,131 +0,0 @@ -package org.keycloak.adapters.undertow; -import io.undertow.UndertowLogger; -import io.undertow.UndertowOptions; -import io.undertow.server.Connectors; -import io.undertow.server.HttpServerExchange; -import io.undertow.server.session.Session; -import io.undertow.servlet.handlers.ServletRequestContext; -import io.undertow.servlet.spec.HttpSessionImpl; -import io.undertow.util.HeaderMap; -import io.undertow.util.HeaderValues; -import io.undertow.util.Headers; -import io.undertow.util.HttpString; -import io.undertow.util.ImmediatePooled; - -import javax.servlet.http.HttpSession; -import java.io.IOException; -import java.io.InputStream; -import java.io.Serializable; -import java.nio.ByteBuffer; -import java.security.AccessController; -import java.util.Iterator; - -/** - * Saved servlet request. - * - * Note bill burke: I had to fork this because Undertow was automatically restoring the request before the code could be processed and redirected. - * - * @author Stuart Douglas - */ -public class SavedRequest implements Serializable { - - private static final String SESSION_KEY = SavedRequest.class.getName(); - - private final byte[] data; - private final int dataLength; - private final HttpString method; - private final String requestUri; - private final HeaderMap headerMap; - - public SavedRequest(byte[] data, int dataLength, HttpString method, String requestUri, HeaderMap headerMap) { - this.data = data; - this.dataLength = dataLength; - this.method = method; - this.requestUri = requestUri; - this.headerMap = headerMap; - } - - public static void trySaveRequest(final HttpServerExchange exchange) { - int maxSize = exchange.getConnection().getUndertowOptions().get(UndertowOptions.MAX_BUFFERED_REQUEST_SIZE, 16384); - if (maxSize > 0) { - //if this request has a body try and cache the response - if (!exchange.isRequestComplete()) { - final long requestContentLength = exchange.getRequestContentLength(); - if (requestContentLength > maxSize) { - UndertowLogger.REQUEST_LOGGER.debugf("Request to %s was to large to save", exchange.getRequestURI()); - return;//failed to save the request, we just return - } - //TODO: we should really be used pooled buffers - //TODO: we should probably limit the number of saved requests at any given time - byte[] buffer = new byte[maxSize]; - int read = 0; - int res = 0; - InputStream in = exchange.getInputStream(); - try { - while ((res = in.read(buffer, read, buffer.length - read)) > 0) { - read += res; - if (read == maxSize) { - UndertowLogger.REQUEST_LOGGER.debugf("Request to %s was to large to save", exchange.getRequestURI()); - return;//failed to save the request, we just return - } - } - HeaderMap headers = new HeaderMap(); - for(HeaderValues entry : exchange.getRequestHeaders()) { - if(entry.getHeaderName().equals(Headers.CONTENT_LENGTH) || - entry.getHeaderName().equals(Headers.TRANSFER_ENCODING) || - entry.getHeaderName().equals(Headers.CONNECTION)) { - continue; - } - headers.putAll(entry.getHeaderName(), entry); - } - SavedRequest request = new SavedRequest(buffer, read, exchange.getRequestMethod(), exchange.getRequestURI(), exchange.getRequestHeaders()); - final ServletRequestContext sc = exchange.getAttachment(ServletRequestContext.ATTACHMENT_KEY); - HttpSessionImpl session = sc.getCurrentServletContext().getSession(exchange, true); - Session underlyingSession; - if(System.getSecurityManager() == null) { - underlyingSession = session.getSession(); - } else { - underlyingSession = AccessController.doPrivileged(new HttpSessionImpl.UnwrapSessionAction(session)); - } - underlyingSession.setAttribute(SESSION_KEY, request); - } catch (IOException e) { - UndertowLogger.REQUEST_IO_LOGGER.ioException(e); - } - } - } - } - - public static void tryRestoreRequest(final HttpServerExchange exchange, HttpSession session) { - if(session instanceof HttpSessionImpl) { - - Session underlyingSession; - if(System.getSecurityManager() == null) { - underlyingSession = ((HttpSessionImpl) session).getSession(); - } else { - underlyingSession = AccessController.doPrivileged(new HttpSessionImpl.UnwrapSessionAction(session)); - } - SavedRequest request = (SavedRequest) underlyingSession.getAttribute(SESSION_KEY); - if(request != null) { - if(request.requestUri.equals(exchange.getRequestURI()) && exchange.isRequestComplete()) { - UndertowLogger.REQUEST_LOGGER.debugf("restoring request body for request to %s", request.requestUri); - exchange.setRequestMethod(request.method); - Connectors.ungetRequestBytes(exchange, new ImmediatePooled(ByteBuffer.wrap(request.data, 0, request.dataLength))); - underlyingSession.removeAttribute(SESSION_KEY); - //clear the existing header map of everything except the connection header - //TODO: are there other headers we should preserve? - Iterator headerIterator = exchange.getRequestHeaders().iterator(); - while (headerIterator.hasNext()) { - HeaderValues header = headerIterator.next(); - if(!header.getHeaderName().equals(Headers.CONNECTION)) { - headerIterator.remove(); - } - } - for(HeaderValues header : request.headerMap) { - exchange.getRequestHeaders().putAll(header.getHeaderName(), header); - } - } - } - } - } - -} diff --git a/adapters/saml/undertow/src/main/java/org/keycloak/adapters/saml/undertow/SamlServletExtension.java b/adapters/saml/undertow/src/main/java/org/keycloak/adapters/saml/undertow/SamlServletExtension.java index ad22718658..589835dd9b 100755 --- a/adapters/saml/undertow/src/main/java/org/keycloak/adapters/saml/undertow/SamlServletExtension.java +++ b/adapters/saml/undertow/src/main/java/org/keycloak/adapters/saml/undertow/SamlServletExtension.java @@ -35,6 +35,7 @@ import org.keycloak.adapters.saml.SamlDeployment; import org.keycloak.adapters.saml.SamlDeploymentContext; import org.keycloak.adapters.saml.config.parsers.DeploymentBuilder; import org.keycloak.adapters.saml.config.parsers.ResourceLoader; +import org.keycloak.adapters.undertow.ChangeSessionIdOnLogin; import org.keycloak.adapters.undertow.UndertowUserSessionManagement; import org.keycloak.saml.common.exceptions.ParsingException; @@ -183,6 +184,7 @@ public class SamlServletExtension implements ServletExtension { ServletSessionConfig cookieConfig = new ServletSessionConfig(); cookieConfig.setPath(deploymentInfo.getContextPath()); deploymentInfo.setServletSessionConfig(cookieConfig); + ChangeSessionIdOnLogin.turnOffChangeSessionIdOnLogin(deploymentInfo); } 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 34d718b59d..d9885d83f7 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 @@ -6,11 +6,11 @@ import io.undertow.server.HttpServerExchange; import io.undertow.server.session.SessionManager; import io.undertow.servlet.handlers.ServletRequestContext; import io.undertow.servlet.spec.HttpSessionImpl; -import io.undertow.servlet.util.SavedRequest; import org.jboss.logging.Logger; import org.keycloak.adapters.spi.SessionIdMapper; import org.keycloak.adapters.saml.SamlSession; import org.keycloak.adapters.saml.SamlSessionStore; +import org.keycloak.adapters.undertow.SavedRequest; import org.keycloak.adapters.undertow.UndertowUserSessionManagement; import org.keycloak.common.util.KeycloakUriBuilder; import org.keycloak.dom.saml.v2.protocol.StatusType; diff --git a/adapters/spi/undertow-adapter-spi/src/main/java/org/keycloak/adapters/undertow/ChangeSessionIdOnLogin.java b/adapters/spi/undertow-adapter-spi/src/main/java/org/keycloak/adapters/undertow/ChangeSessionIdOnLogin.java new file mode 100755 index 0000000000..1e6869a301 --- /dev/null +++ b/adapters/spi/undertow-adapter-spi/src/main/java/org/keycloak/adapters/undertow/ChangeSessionIdOnLogin.java @@ -0,0 +1,27 @@ +package org.keycloak.adapters.undertow; + +import io.undertow.servlet.api.DeploymentInfo; + +import java.lang.reflect.Method; + +/** + * @author Bill Burke + * @version $Revision: 1 $ + */ +public class ChangeSessionIdOnLogin { + /** + * This is a hack to be backward compatible between Undertow 1.3+ and versions lower. In Undertow 1.3, a new + * switch was added setChangeSessionIdOnLogin, this screws up session management for keycloak as after the session id + * is uploaded to Keycloak, undertow changes the session id and it can't be invalidated. + * + * @param deploymentInfo + */ + public static void turnOffChangeSessionIdOnLogin(DeploymentInfo deploymentInfo) { + try { + Method method = DeploymentInfo.class.getMethod("setChangeSessionIdOnLogin", boolean.class); + method.invoke(deploymentInfo, false); + } catch (Exception ignore) { + + } + } +} diff --git a/adapters/spi/undertow-adapter-spi/src/main/java/org/keycloak/adapters/undertow/SavedRequest.java b/adapters/spi/undertow-adapter-spi/src/main/java/org/keycloak/adapters/undertow/SavedRequest.java new file mode 100755 index 0000000000..556f5bce02 --- /dev/null +++ b/adapters/spi/undertow-adapter-spi/src/main/java/org/keycloak/adapters/undertow/SavedRequest.java @@ -0,0 +1,65 @@ +package org.keycloak.adapters.undertow; + +import io.undertow.server.HttpServerExchange; +import io.undertow.server.session.Session; +import io.undertow.servlet.handlers.ServletRequestContext; +import io.undertow.servlet.spec.HttpSessionImpl; + +import javax.servlet.http.HttpSession; +import java.io.Serializable; +import java.security.AccessController; + +/** + * Saved servlet request. + * + * Note bill burke: I had to fork this because Undertow was automatically restoring the request before the code could be + * processed and redirected. + * + * CachedAuthenticatedSessionHandler was restoring the request before the authentication manager could read the code from the URI + * Originally, I copied SavedRequest as is, but there are type mismatches between Undertow 1.1.1 and 1.3.10. + * So, trySaveRequest calls the same undertow version, removes the saved request, stores it in a different session attribute, + * then restores the old attribute later + * + * + * @author Stuart Douglas + */ +public class SavedRequest implements Serializable { + + private static final String SESSION_KEY = SavedRequest.class.getName(); + + public static void trySaveRequest(final HttpServerExchange exchange) { + io.undertow.servlet.util.SavedRequest.trySaveRequest(exchange); + final ServletRequestContext sc = exchange.getAttachment(ServletRequestContext.ATTACHMENT_KEY); + HttpSessionImpl session = sc.getCurrentServletContext().getSession(exchange, true); + Session underlyingSession; + if(System.getSecurityManager() == null) { + underlyingSession = session.getSession(); + } else { + underlyingSession = AccessController.doPrivileged(new HttpSessionImpl.UnwrapSessionAction(session)); + } + io.undertow.servlet.util.SavedRequest request = (io.undertow.servlet.util.SavedRequest) underlyingSession.removeAttribute(io.undertow.servlet.util.SavedRequest.class.getName()); + if (request != null) underlyingSession.setAttribute(SESSION_KEY, request); + + + } + + public static void tryRestoreRequest(final HttpServerExchange exchange, HttpSession session) { + if(session instanceof HttpSessionImpl) { + + Session underlyingSession; + if(System.getSecurityManager() == null) { + underlyingSession = ((HttpSessionImpl) session).getSession(); + } else { + underlyingSession = AccessController.doPrivileged(new HttpSessionImpl.UnwrapSessionAction(session)); + } + io.undertow.servlet.util.SavedRequest request = (io.undertow.servlet.util.SavedRequest) underlyingSession.removeAttribute(SESSION_KEY); + if (request != null) { + underlyingSession.setAttribute(io.undertow.servlet.util.SavedRequest.class.getName(), request); + io.undertow.servlet.util.SavedRequest.tryRestoreRequest(exchange, session); + + } + + } + } + +} diff --git a/adapters/spi/undertow-adapter-spi/src/main/java/org/keycloak/adapters/undertow/SessionManagementBridge.java b/adapters/spi/undertow-adapter-spi/src/main/java/org/keycloak/adapters/undertow/SessionManagementBridge.java index f9ac9aed75..fec640e7c7 100755 --- a/adapters/spi/undertow-adapter-spi/src/main/java/org/keycloak/adapters/undertow/SessionManagementBridge.java +++ b/adapters/spi/undertow-adapter-spi/src/main/java/org/keycloak/adapters/undertow/SessionManagementBridge.java @@ -17,8 +17,10 @@ package org.keycloak.adapters.undertow; import io.undertow.server.session.SessionManager; +import io.undertow.servlet.api.DeploymentInfo; import org.keycloak.adapters.spi.UserSessionManagement; +import java.lang.reflect.Method; import java.util.List; /** @@ -44,4 +46,5 @@ public class SessionManagementBridge implements UserSessionManagement { public void logoutHttpSessions(List ids) { userSessionManagement.logoutHttpSessions(sessionManager, ids); } + } diff --git a/pom.xml b/pom.xml index a7cf76d9cd..c964a1ad35 100755 --- a/pom.xml +++ b/pom.xml @@ -32,7 +32,7 @@ 4.3.3 3.0.14.Final 4.2.1 - 1.1.1.Final + 1.3.10.Final 2.7.0.Final 3.2.0 3.1.4.GA diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/adapter/AdapterTestStrategy.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/adapter/AdapterTestStrategy.java index f93c4ebf87..dec73c6635 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/adapter/AdapterTestStrategy.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/adapter/AdapterTestStrategy.java @@ -489,7 +489,9 @@ public class AdapterTestStrategy extends ExternalResource { String logoutUri = OIDCLoginProtocolService.logoutUrl(UriBuilder.fromUri(AUTH_SERVER_URL)) .queryParam(OAuth2Constants.REDIRECT_URI, APP_SERVER_BASE_URL + "/secure-portal").build("demo").toString(); driver.navigate().to(logoutUri); - Assert.assertTrue(driver.getCurrentUrl().startsWith(LOGIN_URL)); + String currentUrl = driver.getCurrentUrl(); + pageSource = driver.getPageSource(); + Assert.assertTrue(currentUrl.startsWith(LOGIN_URL)); driver.navigate().to(APP_SERVER_BASE_URL + "/secure-portal"); Assert.assertTrue(driver.getCurrentUrl().startsWith(LOGIN_URL)); }