KEYCLOAK-2373 KEYCLOAK-2376

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<UndertowNodesRegistrationManagementWrapper>() {
 
             @Override
@@ -204,4 +204,5 @@ public class KeycloakServletExtension implements ServletExtension {
         }
         return errorPage;
     }
+
 }

adapters/oidc/undertow/src/main/java/org/keycloak/adapters/undertow/SavedRequest.java

@@ -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>(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<HeaderValues> 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);
-                    }
-                }
-            }
-        }
-    }
-
-}

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);
 
      }
 

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;

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 <a href="mailto:bill@burkecentral.com">Bill Burke</a>
+ * @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) {
+
+        }
+    }
+}

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);
+
+            }
+
+         }
+    }
+
+}

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<String> ids) {
         userSessionManagement.logoutHttpSessions(sessionManager, ids);
     }
+
 }

pom.xml

@@ -32,7 +32,7 @@
         <apache.httpcomponents.httpcore.version>4.3.3</apache.httpcomponents.httpcore.version>
         <resteasy.version>3.0.14.Final</resteasy.version>
         <keycloak.apache.httpcomponents.version>4.2.1</keycloak.apache.httpcomponents.version>
-        <undertow.version>1.1.1.Final</undertow.version>
+        <undertow.version>1.3.10.Final</undertow.version>
         <picketlink.version>2.7.0.Final</picketlink.version>
         <mongo.driver.version>3.2.0</mongo.driver.version>
         <jboss.logging.version>3.1.4.GA</jboss.logging.version>

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));
     }