Consolidate request context handling

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

@@ -83,6 +83,7 @@
     </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

@@ -72,7 +72,6 @@ import org.keycloak.adapters.OAuthRequestAuthenticator;
 import org.keycloak.adapters.OIDCAuthenticationError;
 import org.keycloak.adapters.OidcKeycloakAccount;
 import org.keycloak.adapters.PreAuthActionsHandler;
-import org.keycloak.adapters.ServerRequest;
 import org.keycloak.adapters.authentication.ClientCredentialsProviderUtils;
 import org.keycloak.adapters.rotation.AdapterTokenVerifier;
 import org.keycloak.adapters.rotation.AdapterTokenVerifier.VerifiedTokens;
@@ -95,18 +94,16 @@ 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;
@@ -118,6 +115,9 @@ import org.springframework.extensions.webscripts.connector.ConnectorContext;
 import org.springframework.extensions.webscripts.connector.ConnectorService;
 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 +131,7 @@ import de.acosix.alfresco.keycloak.share.util.RefreshableAccessTokenHolder;
  *
  * @author Axel Faust
  */
-public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, InitializingBean, ApplicationContextAware
+public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, InitializingBean
 {
 
     public static final String KEYCLOAK_AUTHENTICATED_COOKIE = "Acosix." + KeycloakAuthenticationFilter.class.getSimpleName();
@@ -176,10 +176,10 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I
 
     private static final ThreadLocal<String> LOGIN_REDIRECT_URL = new ThreadLocal<>();
 
-    protected ApplicationContext applicationContext;
-
     protected DependencyInjectedFilter defaultSsoFilter;
 
+    protected ServletRequestContextFactory requestContextFactory;
+
     protected ConfigService configService;
 
     protected ConnectorService connectorService;
@@ -236,15 +236,6 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I
         return authenticatedByKeycloak;
     }
 
-    /**
-     * {@inheritDoc}
-     */
-    @Override
-    public void setApplicationContext(final ApplicationContext applicationContext)
-    {
-        this.applicationContext = applicationContext;
-    }
-
     /**
      *
      * {@inheritDoc}
@@ -253,6 +244,7 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I
     public void afterPropertiesSet()
     {
         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);
@@ -361,6 +353,15 @@ 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,18 +439,23 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I
             }
 
             // Alfresco handling of RequestContext / ServletUtil / any other context holder is so immensely broken, it isn't even funny
-            // Request context is indirectly required for some framework page lookups (which lack null guards) as part of our handling
+            RequestContext requestContext = ThreadLocalRequestContext.getRequestContext();
-            // Our context is almost guaranteed to be overridden by some Surf component, which repeats the whole initialisation dance
+            if (requestContext == null)
-            RequestContext requestContext;
-            try
             {
-                requestContext = RequestContextUtil.initRequestContext(this.applicationContext, req, true);
+                try
-            }
+                {
-            catch (final Exception ex)
+                    requestContext = this.requestContextFactory.newInstance(new ServletWebRequest(req));
-            {
+                    request.setAttribute(RequestContext.ATTR_REQUEST_CONTEXT, context);
-                LOGGER.error("Error calling initRequestContext", ex);
+                }
-                throw new ServletException(ex);
+                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);
 
             try
             {
@@ -900,7 +906,7 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I
             session.setAttribute(UserFactory.SESSION_ATTRIBUTE_EXTERNAL_AUTH, Boolean.TRUE);
             session.setAttribute(UserFactory.SESSION_ATTRIBUTE_KEY_USER_ID, userId);
 
-            this.handleAlfrescoResourceAccessToken(session, false);
+            this.handleAlfrescoResourceAccessToken(session);
         }
 
         if (facade.isEnded())
@@ -1334,7 +1340,7 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I
         {
             LOGGER.debug("Skipping processKeycloakAuthenticationAndActions as Keycloak-authentication session is still valid");
             this.ensureKeycloakCookieSet(req, res);
-            this.handleAlfrescoResourceAccessToken(currentSession, false);
+            this.handleAlfrescoResourceAccessToken(currentSession);
             skip = true;
         }
         else
@@ -1555,11 +1561,8 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I
      *
      * @param session
      *            the active session managing any persistent access token state
-     * @param retry
-     *            {@code true} if the invocation of the operation is a retry as the result of a (hopefully temporary) verification failure
-     *            in the current thread
      */
-    protected void handleAlfrescoResourceAccessToken(final HttpSession session, final boolean retry)
+    protected void handleAlfrescoResourceAccessToken(final HttpSession session)
     {
         final KeycloakAuthenticationConfigElement keycloakAuthConfig = (KeycloakAuthenticationConfigElement) this.configService
                 .getConfig(KeycloakConfigConstants.KEYCLOAK_CONFIG_SECTION_NAME).getConfigElement(KeycloakAuthenticationConfigElement.NAME);
@@ -1591,29 +1594,16 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I
                     AccessTokenResponse response;
                     try
                     {
-                        if (token != null && token.canRefresh())
+                        // Note: we tried to simply just refresh with the refresh the already exchanged token for the target resource
-                        {
+                        // but audience typically is not correct in the resulting token
-                            LOGGER.debug("Refreshing access token for Alfresco backend resource {}", alfrescoResourceName);
+                        LOGGER.debug("Retrieving / refreshing access token for Alfresco backend resource {}", alfrescoResourceName);
-                            response = ServerRequest.invokeRefresh(this.keycloakDeployment, token.getRefreshToken());
+                        response = this.getAccessToken(alfrescoResourceName, session);
-                        }
-                        else
-                        {
-                            LOGGER.debug("Retrieving initial / new access token for Alfresco backend resource {}", alfrescoResourceName);
-                            response = this.getAccessToken(alfrescoResourceName, session);
-                        }
                     }
                     catch (final IOException ioex)
                     {
                         LOGGER.error("Error retrieving / refreshing access token for Alfresco backend", ioex);
                         throw new AlfrescoRuntimeException("Error retrieving / refreshing access token for Alfresco backend", ioex);
                     }
-                    catch (final ServerRequest.HttpFailure httpFailure)
-                    {
-                        LOGGER.error("Refreshing access token for Alfresco backend failed: {} {}", httpFailure.getStatus(),
-                                httpFailure.getError());
-                        throw new AlfrescoRuntimeException("Failed to refresh access token for Alfresco backend: " + httpFailure.getStatus()
-                                + " " + httpFailure.getError());
-                    }
 
                     final String tokenString = response.getToken();
                     try
@@ -1639,20 +1629,9 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I
                     }
                     catch (final VerificationException vex)
                     {
-                        session.removeAttribute(BACKEND_ACCESS_TOKEN_SESSION_KEY);
+                        LOGGER.error("Verification of access token for Alfresco backend failed in retry", vex);
-                        if (!retry && token != null && token.canRefresh())
+                        throw new AlfrescoRuntimeException("Keycloak token exchange for access to backend yielded invalid access token",
-                        {
+                                vex);
-                            LOGGER.warn(
-                                    "Verification of refreshed access token for Alfresco backend failed - removed previous token from the session before retrying token exchange from scratch");
-
-                            this.handleAlfrescoResourceAccessToken(session, true);
-                        }
-                        else
-                        {
-                            LOGGER.error("Verification of access token for Alfresco backend failed in retry", vex);
-                            throw new AlfrescoRuntimeException("Keycloak token exchange for access to backend yielded invalid access token",
-                                    vex);
-                        }
                     }
                 }
             }

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

@@ -26,7 +26,6 @@ import javax.servlet.ServletResponse;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpSession;
 
-import org.alfresco.error.AlfrescoRuntimeException;
 import org.alfresco.util.PropertyCheck;
 import org.alfresco.web.site.servlet.SlingshotLoginController;
 import org.json.simple.JSONArray;
@@ -36,17 +35,11 @@ import org.json.simple.parser.ParseException;
 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.ConfigElement;
 import org.springframework.extensions.config.ConfigService;
-import org.springframework.extensions.surf.RequestContext;
-import org.springframework.extensions.surf.RequestContextUtil;
 import org.springframework.extensions.surf.exception.ConnectorServiceException;
-import org.springframework.extensions.surf.exception.RequestContextException;
 import org.springframework.extensions.surf.site.AuthenticationUtil;
 import org.springframework.extensions.surf.support.AlfrescoUserFactory;
-import org.springframework.extensions.surf.support.ThreadLocalRequestContext;
 import org.springframework.extensions.surf.util.URLEncoder;
 import org.springframework.extensions.webscripts.Status;
 import org.springframework.extensions.webscripts.connector.Connector;
@@ -65,7 +58,7 @@ import org.springframework.extensions.webscripts.servlet.DependencyInjectedFilte
  *
  * @author Axel Faust
  */
-public class UserGroupsLoadFilter implements DependencyInjectedFilter, InitializingBean, ApplicationContextAware
+public class UserGroupsLoadFilter implements DependencyInjectedFilter, InitializingBean
 {
 
     public static final String SESSION_ATTRIBUTE_KEY_USER_GROUPS_LAST_LOADED = SlingshotLoginController.SESSION_ATTRIBUTE_KEY_USER_GROUPS
@@ -75,21 +68,10 @@ public class UserGroupsLoadFilter implements DependencyInjectedFilter, Initializ
 
     private static final long DEFAULT_CACHED_USER_GROUPS_TIMEOUT = 60000;
 
-    protected ApplicationContext applicationContext;
-
     protected ConfigService configService;
 
     protected ConnectorService connectorService;
 
-    /**
-     * {@inheritDoc}
-     */
-    @Override
-    public void setApplicationContext(final ApplicationContext applicationContext)
-    {
-        this.applicationContext = applicationContext;
-    }
-
     /**
      *
      * {@inheritDoc}
@@ -97,7 +79,6 @@ public class UserGroupsLoadFilter implements DependencyInjectedFilter, Initializ
     @Override
     public void afterPropertiesSet()
     {
-        PropertyCheck.mandatory(this, "applicationContext", this.applicationContext);
         PropertyCheck.mandatory(this, "configService", this.configService);
         PropertyCheck.mandatory(this, "connectorService", this.connectorService);
     }
@@ -197,22 +178,6 @@ public class UserGroupsLoadFilter implements DependencyInjectedFilter, Initializ
             // logic nearly identical to SlingshotLoginController
             final Connector connector = this.connectorService.getConnector(AlfrescoUserFactory.ALFRESCO_ENDPOINT_ID, userId, session);
 
-            // bug in default Alfresco RequestCachingConnector: with ConnectorContext having HttpMethod.GET, null check of
-            // ThreadLocalRequestContext.getRequestContext() is short-circuited, causing NPE on access
-            final RequestContext requestContext = ThreadLocalRequestContext.getRequestContext();
-            if (requestContext == null)
-            {
-                try
-                {
-                    RequestContextUtil.initRequestContext(this.applicationContext, request, true);
-                }
-                catch (final RequestContextException e)
-                {
-                    LOGGER.error("Failed to initialise request context", e);
-                    throw new AlfrescoRuntimeException("Failed to initialise request context", e);
-                }
-            }
-
             final ConnectorContext c = new ConnectorContext(HttpMethod.GET);
             c.setContentType("application/json");
             final Response res = connector.call("/api/people/" + URLEncoder.encode(userId) + "?groups=true", c);
@@ -221,7 +186,7 @@ public class UserGroupsLoadFilter implements DependencyInjectedFilter, Initializ
             {
                 final String responseText = res.getResponse();
                 final JSONParser jsonParser = new JSONParser();
-                final Object userData = jsonParser.parse(responseText.toString());
+                final Object userData = jsonParser.parse(responseText);
 
                 final StringBuilder groups = new StringBuilder(512);
                 if (userData instanceof JSONObject)

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

@@ -23,7 +23,6 @@ import javax.servlet.http.HttpServletRequestWrapper;
 import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpSession;
 
-import org.alfresco.error.AlfrescoRuntimeException;
 import org.alfresco.util.PropertyCheck;
 import org.alfresco.web.site.servlet.SlingshotLoginController;
 import org.json.simple.JSONObject;
@@ -32,13 +31,9 @@ import org.json.simple.parser.ParseException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.InitializingBean;
-import org.springframework.extensions.surf.RequestContext;
-import org.springframework.extensions.surf.RequestContextUtil;
 import org.springframework.extensions.surf.UserFactory;
 import org.springframework.extensions.surf.exception.ConnectorServiceException;
-import org.springframework.extensions.surf.exception.RequestContextException;
 import org.springframework.extensions.surf.support.AlfrescoUserFactory;
-import org.springframework.extensions.surf.support.ThreadLocalRequestContext;
 import org.springframework.extensions.webscripts.Status;
 import org.springframework.extensions.webscripts.connector.Connector;
 import org.springframework.extensions.webscripts.connector.ConnectorContext;
@@ -183,22 +178,6 @@ public class UserNameCorrectingSlingshotLoginController extends SlingshotLoginCo
         {
             final Connector connector = this.connectorService.getConnector(AlfrescoUserFactory.ALFRESCO_ENDPOINT_ID, userId, session);
 
-            // bug in default Alfresco RequestCachingConnector: with ConnectorContext having HttpMethod.GET, null check of
-            // ThreadLocalRequestContext.getRequestContext() is short-circuited, causing NPE on access
-            final RequestContext requestContext = ThreadLocalRequestContext.getRequestContext();
-            if (requestContext == null)
-            {
-                try
-                {
-                    RequestContextUtil.initRequestContext(this.getApplicationContext(), request, true);
-                }
-                catch (final RequestContextException e)
-                {
-                    LOGGER.error("Failed to initialise request context", e);
-                    throw new AlfrescoRuntimeException("Failed to initialise request context", e);
-                }
-            }
-
             final ConnectorContext c = new ConnectorContext(HttpMethod.GET);
             c.setContentType("application/json");
             final Response res = connector.call("/acosix/api/keycloak/effectiveUserName", c);