Share backend token handling + NPE fix

pom.xml

@@ -21,12 +21,12 @@
     <parent>
         <groupId>de.acosix.alfresco.maven</groupId>
         <artifactId>de.acosix.alfresco.maven.project.parent-6.0.7</artifactId>
-        <version>1.3.1</version>
+        <version>1.3.3</version>
     </parent>
 
     <groupId>de.acosix.alfresco.keycloak</groupId>
     <artifactId>de.acosix.alfresco.keycloak.parent</artifactId>
-    <version>1.1.0-rc3</version>
+    <version>1.1.0-rc4-SNAPSHOT</version>
     <packaging>pom</packaging>
 
     <name>Acosix Alfresco Keycloak - Parent</name>
@@ -79,7 +79,7 @@
         <apache.httpclient.version>4.5.1</apache.httpclient.version>
         <apache.httpcore.version>4.4.3</apache.httpcore.version>
 
-        <acosix.utility.version>1.2.1</acosix.utility.version>
+        <acosix.utility.version>1.2.3-SNAPSHOT</acosix.utility.version>
         <ootbee.support-tools.version>1.1.0.0</ootbee.support-tools.version>
     </properties>
 

repository-dependencies/pom.xml

@@ -21,7 +21,7 @@
     <parent>
         <groupId>de.acosix.alfresco.keycloak</groupId>
         <artifactId>de.acosix.alfresco.keycloak.parent</artifactId>
-        <version>1.1.0-rc3</version>
+        <version>1.1.0-rc4-SNAPSHOT</version>
     </parent>
 
     <artifactId>de.acosix.alfresco.keycloak.repo.deps</artifactId>

repository/module.properties

@@ -5,4 +5,4 @@ module.version=${noSnapshotVersion}
 
 module.repo.version.min=5
 
-module.depends.acosix-utility=1.1.0-*
+module.depends.acosix-utility=1.2.3-*

repository/pom.xml

@@ -21,7 +21,7 @@
     <parent>
         <groupId>de.acosix.alfresco.keycloak</groupId>
         <artifactId>de.acosix.alfresco.keycloak.parent</artifactId>
-        <version>1.1.0-rc3</version>
+        <version>1.1.0-rc4-SNAPSHOT</version>
     </parent>
 
     <artifactId>de.acosix.alfresco.keycloak.repo</artifactId>

repository/src/main/java/de/acosix/alfresco/keycloak/repo/authentication/KeycloakAuthenticationFilter.java

@@ -693,7 +693,14 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter
         }
         LOGGER.trace("Resetting session and state cookie before continueing with filter chain");
 
+        try
+        {
             req.getSession().invalidate();
+        }
+        catch (final IllegalStateException ignore)
+        {
+            // Keycloak authenticator may have already invalidated it - no way to check and avoid exception
+        }
 
         this.resetStateCookies(context, req, res);
 
@@ -723,13 +730,13 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter
         boolean skip = false;
 
         final String authHeader = req.getHeader(HEADER_AUTHORIZATION);
-        final String noKeycloakLoginRedirectHeader = req.getHeader(this.noKeycloakHandlingHeaderName);
+        final String noKeycloakHandlingRedirectHeader = req.getHeader(this.noKeycloakHandlingHeaderName);
 
         final String servletPath = req.getServletPath();
         final String pathInfo = req.getPathInfo();
         final String servletRequestUri = servletPath + (pathInfo != null ? pathInfo : "");
 
-        final SessionUser sessionUser = this.getSessionUser(context, req, res, true);
+        SessionUser sessionUser = this.getSessionUser(context, req, res, true);
         HttpSession session = req.getSession();
 
         final boolean publicRestApi = API_SERVLET_PATH.equals(servletPath);
@@ -743,6 +750,7 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter
             LOGGER.debug("Session {} for Keycloak-authenticated user {} was invalidated by back-channel logout", session.getId(),
                     AlfrescoCompatibilityUtil.maskUsername(sessionUser.getUserName()));
             this.invalidateSession(req);
+            sessionUser = null;
             session = req.getSession(false);
         }
 
@@ -908,7 +916,7 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter
                     "Skipping processKeycloakAuthenticationAndActions as filter higher up in chain determined authentication as not required");
             skip = true;
         }
-        else if (Boolean.parseBoolean(noKeycloakLoginRedirectHeader))
+        else if (Boolean.parseBoolean(noKeycloakHandlingRedirectHeader))
         {
             LOGGER.trace(
                     "Skipping processKeycloakAuthenticationAndActions as client provided custom 'no Keycloak handling' header {} with value that resolves to 'true'",

share-dependencies/pom.xml

@@ -21,7 +21,7 @@
     <parent>
         <groupId>de.acosix.alfresco.keycloak</groupId>
         <artifactId>de.acosix.alfresco.keycloak.parent</artifactId>
-        <version>1.1.0-rc3</version>
+        <version>1.1.0-rc4-SNAPSHOT</version>
     </parent>
 
     <artifactId>de.acosix.alfresco.keycloak.share.deps</artifactId>

share/pom.xml

@@ -21,7 +21,7 @@
     <parent>
         <groupId>de.acosix.alfresco.keycloak</groupId>
         <artifactId>de.acosix.alfresco.keycloak.parent</artifactId>
-        <version>1.1.0-rc3</version>
+        <version>1.1.0-rc4-SNAPSHOT</version>
     </parent>
 
     <artifactId>de.acosix.alfresco.keycloak.share</artifactId>

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

@@ -53,6 +53,7 @@
         <property name="propertyName" value="requestHeaders" />
         <property name="valueMap">
             <map>
+                <!-- unless we pro-actively provide a bearer token, repository should not handle Keycloak on remote API for requests from Share -->
                 <entry key="x-${moduleId}-no-keycloak-handling" value="true" />
             </map>
         </property>

share/src/main/java/de/acosix/alfresco/keycloak/share/remote/AccessTokenAwareSlingshotAlfrescoConnector.java

@@ -32,6 +32,7 @@ import de.acosix.alfresco.keycloak.share.deps.keycloak.adapters.OidcKeycloakAcco
 import de.acosix.alfresco.keycloak.share.deps.keycloak.adapters.spi.KeycloakAccount;
 import de.acosix.alfresco.keycloak.share.util.RefreshableAccessTokenHolder;
 import de.acosix.alfresco.keycloak.share.web.KeycloakAuthenticationFilter;
+import de.acosix.alfresco.utility.share.connector.MutableSlingshotRemoteClient;
 
 /**
  * @author Axel Faust
@@ -72,14 +73,27 @@ public class AccessTokenAwareSlingshotAlfrescoConnector extends SlingshotAlfresc
                 ? session.getAttribute(KeycloakAuthenticationFilter.BACKEND_ACCESS_TOKEN_SESSION_KEY)
                 : null);
 
+        final MutableSlingshotRemoteClient mrc = remoteClient instanceof MutableSlingshotRemoteClient
+                ? (MutableSlingshotRemoteClient) remoteClient
+                : null;
+
         if (endpointSpecificAccessToken != null)
         {
             if (endpointSpecificAccessToken.isActive())
             {
                 LOGGER.debug("Using access token for backend found in session for request");
                 final String tokenString = endpointSpecificAccessToken.getToken();
+                if (mrc != null)
+                {
+                    mrc.addRemoveResponseHeader("WWW-Authenticate");
+                    // must be request property to be a final override (other Alfresco components sets Authorization=null)
+                    mrc.addRequestProperty("Authorization", "Bearer " + tokenString);
+                }
+                else
+                {
                     remoteClient.setRequestProperties(Collections.singletonMap("Authorization", "Bearer " + tokenString));
                 }
+            }
             else
             {
                 LOGGER.warn("Acesss token for backend stored in session has expired");
@@ -91,15 +105,33 @@ public class AccessTokenAwareSlingshotAlfrescoConnector extends SlingshotAlfresc
                     "Did not find access token for backend in session - using regularly authenticated Keycloak account access token for request instead");
             final KeycloakSecurityContext keycloakSecurityContext = ((OidcKeycloakAccount) keycloakAccount).getKeycloakSecurityContext();
             final String tokenString = keycloakSecurityContext.getTokenString();
+            if (mrc != null)
+            {
+                mrc.addRemoveResponseHeader("WWW-Authenticate");
+                // must be request property to be a final override (other Alfresco components sets Authorization=null)
+                mrc.addRequestProperty("Authorization", "Bearer " + tokenString);
+            }
+            else
+            {
                 remoteClient.setRequestProperties(Collections.singletonMap("Authorization", "Bearer " + tokenString));
             }
+        }
         else if (accessToken != null)
         {
             LOGGER.debug(
                     "Did not find access token for backend in session - using Bearer access token provided in original authentication request for request instead");
             final String tokenString = accessToken.getToken();
+            if (mrc != null)
+            {
+                mrc.addRemoveResponseHeader("WWW-Authenticate");
+                // must be request property to be a final override (other Alfresco components sets Authorization=null)
+                mrc.addRequestProperty("Authorization", "Bearer " + tokenString);
+            }
+            else
+            {
                 remoteClient.setRequestProperties(Collections.singletonMap("Authorization", "Bearer " + tokenString));
             }
+        }
         else
         {
             LOGGER.debug("Did not find Keycloak-related authentication data in session - applying regular request authentication");

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

@@ -1025,7 +1025,14 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I
                 authError != null ? authError : "<missing AuthenticationError details in request context>");
         LOGGER.debug("Resetting session and state cookie before continueing with filter chain");
 
+        try
+        {
             req.getSession().invalidate();
+        }
+        catch (final IllegalStateException ignore)
+        {
+            // Keycloak authenticator may have already invalidated it - no way to check and avoid exception
+        }
 
         this.resetStateCookies(context, req, res);
 
@@ -1126,6 +1133,10 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I
         {
             // variant with request AND response is only available in Spring 5+
             RequestContextHolder.setRequestAttributes(new ServletRequestAttributes((HttpServletRequest) request));
+            // a bit redundant since request attributes already holds the request, but hey, that's Alfresco
+            // ServletUtil uses an attribute in the request attributes object separate from the main request
+            // see ServletUtil.VIEW_REQUEST_ATTRIBUTE_NAME
+            ServletUtil.setRequest((HttpServletRequest) request);
         }
 
         try
@@ -1589,7 +1600,8 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I
                     token = (RefreshableAccessTokenHolder) backendAccessTokenCandidate;
                 }
 
-                // not really feasible to synchronise / lock concurrent refresh on token
+                // not really feasible to synchronise / lock concurrent refresh on token, especially given that we cannot lock across
+                // potentially multiple Share instances
                 // not a big problem - apart from wasted CPU cycles / latency - since each concurrently refreshed token is valid
                 // independently
                 if (token == null || !token.isActive()