Further refinement of request context handling

share/src/main/config/default-config.xml

@@ -28,13 +28,13 @@
     <!-- sensible default configuration -->
     <config evaluator="string-compare" condition="Keycloak">
         <keycloak-auth-config>
-            <enhance-login-form>true</enhance-login-form>
             <enable-sso-filter>true</enable-sso-filter>
+            <enhance-login-form>true</enhance-login-form>
             <force-keycloak-sso>false</force-keycloak-sso>
             <body-buffer-limit>10485760</body-buffer-limit>
-            <session-mapper-limit>1000</session-mapper-limit>
+            <session-mapper-limit>10000</session-mapper-limit>
             <ignore-default-filter>true</ignore-default-filter>
-            <perform-token-exchange>false</perform-token-exchange>
+            <perform-token-exchange>true</perform-token-exchange>
             <alfresco-resource-name>alfresco</alfresco-resource-name>
         </keycloak-auth-config>
         <keycloak-adapter-config>

share/src/main/config/module-context.xml

@@ -78,12 +78,19 @@
         <property name="enabled" value="true" />
     </bean>
 
+    <bean id="${moduleId}.populatingRequestContextInterecptorImplementationPatch"
+        class="de.acosix.alfresco.utility.common.spring.ImplementationClassReplacingBeanFactoryPostProcessor">
+        <property name="targetBeanName" value="requestContextInterceptor" />
+        <property name="originalClassName" value="org.springframework.extensions.surf.mvc.RequestContextInterceptor" />
+        <property name="replacementClassName" value="${project.artifactId}.web.PopulatingRequestContextInterceptor" />
+        <property name="enabled" value="true" />
+    </bean>
+
     <bean id="${moduleId}.SessionIdMapper" class="${project.artifactId}.web.DefaultSessionIdMapper">
         <property name="configService" ref="web.config" />
     </bean>
 
     <bean id="${moduleId}.KeycloakAuthenticationFilter" abstract="true" class="${project.artifactId}.web.KeycloakAuthenticationFilter">
-        <property name="requestContextFactory" ref="webframework.factory.requestcontext.servlet" />
         <property name="configService" ref="web.config" />
         <property name="connectorService" ref="connector.service" />
         <property name="pageViewResolver" ref="pageViewResolver" />

share/src/main/java/de/acosix/alfresco/keycloak/share/web/KeycloakAuthenticationFilter.java

@@ -94,16 +94,18 @@ import org.keycloak.util.JsonSerialization;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.InitializingBean;
+import org.springframework.context.ApplicationContext;
+import org.springframework.context.ApplicationContextAware;
 import org.springframework.extensions.config.ConfigService;
 import org.springframework.extensions.config.RemoteConfigElement;
 import org.springframework.extensions.config.RemoteConfigElement.EndpointDescriptor;
 import org.springframework.extensions.surf.RequestContext;
+import org.springframework.extensions.surf.RequestContextUtil;
 import org.springframework.extensions.surf.ServletUtil;
 import org.springframework.extensions.surf.UserFactory;
 import org.springframework.extensions.surf.exception.ConnectorServiceException;
 import org.springframework.extensions.surf.mvc.PageViewResolver;
 import org.springframework.extensions.surf.site.AuthenticationUtil;
-import org.springframework.extensions.surf.support.ServletRequestContextFactory;
 import org.springframework.extensions.surf.support.ThreadLocalRequestContext;
 import org.springframework.extensions.surf.types.Page;
 import org.springframework.extensions.surf.types.PageType;
@@ -117,7 +119,6 @@ import org.springframework.extensions.webscripts.connector.Response;
 import org.springframework.extensions.webscripts.servlet.DependencyInjectedFilter;
 import org.springframework.web.context.request.RequestContextHolder;
 import org.springframework.web.context.request.ServletRequestAttributes;
-import org.springframework.web.context.request.ServletWebRequest;
 
 import de.acosix.alfresco.keycloak.share.config.KeycloakAdapterConfigElement;
 import de.acosix.alfresco.keycloak.share.config.KeycloakAuthenticationConfigElement;
@@ -131,7 +132,7 @@ import de.acosix.alfresco.keycloak.share.util.RefreshableAccessTokenHolder;
  *
  * @author Axel Faust
  */
-public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, InitializingBean
+public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, InitializingBean, ApplicationContextAware
 {
 
     public static final String KEYCLOAK_AUTHENTICATED_COOKIE = "Acosix." + KeycloakAuthenticationFilter.class.getSimpleName();
@@ -176,9 +177,9 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I
 
     private static final ThreadLocal<String> LOGIN_REDIRECT_URL = new ThreadLocal<>();
 
-    protected DependencyInjectedFilter defaultSsoFilter;
+    protected ApplicationContext applicationContext;
 
-    protected ServletRequestContextFactory requestContextFactory;
+    protected DependencyInjectedFilter defaultSsoFilter;
 
     protected ConfigService configService;
 
@@ -236,6 +237,15 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I
         return authenticatedByKeycloak;
     }
 
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public void setApplicationContext(final ApplicationContext applicationContext)
+    {
+        this.applicationContext = applicationContext;
+    }
+
     /**
      *
      * {@inheritDoc}
@@ -243,8 +253,8 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I
     @Override
     public void afterPropertiesSet()
     {
+        PropertyCheck.mandatory(this, "applicationContext", this.applicationContext);
         PropertyCheck.mandatory(this, "primaryEndpoint", this.primaryEndpoint);
-        PropertyCheck.mandatory(this, "requestContextFactory", this.requestContextFactory);
         PropertyCheck.mandatory(this, "configService", this.configService);
         PropertyCheck.mandatory(this, "connectorService", this.connectorService);
         PropertyCheck.mandatory(this, "pageViewResolver", this.pageViewResolver);
@@ -353,15 +363,6 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I
         this.defaultSsoFilter = defaultSsoFilter;
     }
 
-    /**
-     * @param requestContextFactory
-     *            the requestContextFactory to set
-     */
-    public void setRequestContextFactory(final ServletRequestContextFactory requestContextFactory)
-    {
-        this.requestContextFactory = requestContextFactory;
-    }
-
     /**
      * @param configService
      *            the configService to set
@@ -438,24 +439,21 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I
                         this.keycloakDeployment.getAuthServerBaseUrl());
             }
 
-            // Alfresco handling of RequestContext / ServletUtil / any other context holder is so immensely broken, it isn't even funny
-            RequestContext requestContext = ThreadLocalRequestContext.getRequestContext();
-            if (requestContext == null)
-            {
-                try
-                {
-                    requestContext = this.requestContextFactory.newInstance(new ServletWebRequest(req));
-                    request.setAttribute(RequestContext.ATTR_REQUEST_CONTEXT, context);
-                }
-                catch (final Exception ex)
-                {
-                    LOGGER.error("Error calling initRequestContext", ex);
-                    throw new ServletException(ex);
-                }
-            }
             // TODO Figure out how to support Enteprise 6.2 / 7.x or 6.3+, which overload the constructor
             RequestContextHolder.setRequestAttributes(new ServletRequestAttributes(req));
-            ServletUtil.setRequest(req);
+            // Alfresco handling of RequestContext / ServletUtil / any other context holder is so immensely broken, it isn't even funny
+            // this request context is for any handling that needs it until it gets nuked / bulldozed by RequestContextInterceptor
+            // ...after which we will have to enhance that class' partially initialised context
+            RequestContext requestContext;
+            try
+            {
+                requestContext = RequestContextUtil.initRequestContext(this.applicationContext, req, true);
+            }
+            catch (final Exception ex)
+            {
+                LOGGER.error("Error calling initRequestContext", ex);
+                throw new ServletException(ex);
+            }
 
             try
             {
@@ -469,8 +467,12 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I
 
                     if (skip)
                     {
-                        if (!AuthenticationUtil.isAuthenticated(req) && keycloakDeploymentReady && this.loginFormEnhancementEnabled
+                        final boolean authenticated = AuthenticationUtil.isAuthenticated(req);
-                                && this.isLoginPage(req))
+                        if (authenticated)
+                        {
+                            this.completeRequestContext(req);
+                        }
+                        else if (keycloakDeploymentReady && this.loginFormEnhancementEnabled && this.isLoginPage(req))
                         {
                             this.prepareLoginFormEnhancement(context, req, res);
                         }
@@ -490,6 +492,7 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I
             finally
             {
                 requestContext.release();
+                RequestContextHolder.resetRequestAttributes();
             }
         }
         finally
@@ -929,6 +932,8 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I
             }
         }
 
+        this.completeRequestContext(req);
+
         LOGGER.debug("Continueing with filter chain processing");
         final HttpServletRequestWrapper requestWrapper = tokenStore.buildWrapper();
         this.continueFilterChain(context, requestWrapper, res, chain);
@@ -988,6 +993,8 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I
             }
         }
 
+        this.completeRequestContext(req);
+
         LOGGER.debug("Continueing with filter chain processing");
         this.continueFilterChain(context, req, res, chain);
     }
@@ -1126,6 +1133,27 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I
         }
     }
 
+    /**
+     * Completes the request context in the current thread by populating missing data, foremost any user details for the authenticated user.
+     *
+     * @param req
+     *            the servlet request
+     * @throws ServletException
+     *             if an error occurs populating the request context
+     */
+    protected void completeRequestContext(final HttpServletRequest req) throws ServletException
+    {
+        try
+        {
+            RequestContextUtil.populateRequestContext(ThreadLocalRequestContext.getRequestContext(), req);
+        }
+        catch (final Exception ex)
+        {
+            LOGGER.error("Error calling populateRequestContext", ex);
+            throw new ServletException(ex);
+        }
+    }
+
     /**
      * Continues processing the filter chain, either directly or by delegating to the facaded default SSO filter.
      *

share/src/main/java/de/acosix/alfresco/keycloak/share/web/PopulatingRequestContextInterceptor.java

@@ -0,0 +1,47 @@
+/*
+ * Copyright 2019 - 2020 Acosix GmbH
+ *
+ * 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 de.acosix.alfresco.keycloak.share.web;
+
+import org.springframework.extensions.surf.RequestContext;
+import org.springframework.extensions.surf.RequestContextUtil;
+import org.springframework.extensions.surf.mvc.RequestContextInterceptor;
+import org.springframework.extensions.surf.support.ThreadLocalRequestContext;
+import org.springframework.web.context.request.ServletWebRequest;
+import org.springframework.web.context.request.WebRequest;
+
+/**
+ * This specialisation of the request context interceptor exists only to ensure that a newly created request context is properly
+ * {@link RequestContextUtil#populateRequestContext(org.springframework.extensions.surf.RequestContext, javax.servlet.http.HttpServletRequest)
+ * populated} as to ensure that somewhat important data, such as the user object, is properly initialised.
+ *
+ * @author Axel Faust
+ */
+public class PopulatingRequestContextInterceptor extends RequestContextInterceptor
+{
+
+    /**
+     *
+     * {@inheritDoc}
+     */
+    @Override
+    public void preHandle(final WebRequest request) throws Exception
+    {
+        super.preHandle(request);
+
+        final RequestContext context = ThreadLocalRequestContext.getRequestContext();
+        RequestContextUtil.populateRequestContext(context, ((ServletWebRequest) request).getRequest());
+    }
+}