added concurrency support for new users

repository/src/main/config/alfresco-global.properties

@@ -1,3 +1,7 @@
+
+# Missing People are handled by this module in a thread-safe way
+create.missing.people=false
+
 ${moduleId}.authorityServiceEnhancement.enabled=true
 
 cache.${moduleId}.ssoToSessionCache.maxItems=10000

repository/src/main/globalConfig/subsystems/Authentication/keycloak/keycloak-authentication-context.xml

@@ -60,6 +60,8 @@
         <property name="allowGuestLogin" value="${keycloak.authentication.allowGuestLogin}" />
         <property name="failExpiredTicketTokens" value="${keycloak.authentication.failExpiredTicketTokens}" />
         <property name="deployment" ref="keycloakDeployment" />
+        
+        <property name="missingPersonService" ref="missingPersonService" />
     </bean>
     
     <!-- Wrapped version to be used within subsystem -->
@@ -154,6 +156,8 @@
         <property name="keycloakAuthenticationComponent" ref="authenticationComponent" />
         <property name="keycloakTicketTokenCache" ref="${moduleId}-ticketTokenCache" />
         <property name="publicApiRuntimeContainer" ref="publicapi.container" />
+        
+        <property name="missingPersonService" ref="missingPersonService" />
     </bean>
 
     <bean id="${moduleId}.keycloakAuthenticationListener" class="${project.artifactId}.authentication.KeycloakAuthenticationListener">
@@ -225,6 +229,12 @@
         <property name="authenticationService" ref="localAuthenticationService" />
     </bean>
     
+    <bean id="missingPersonService" class="${project.artifactId}.authentication.MissingPersonServiceImpl">
+        <property name="personService" ref="personService" />
+        <property name="transactionService" ref="TransactionService" />
+        <property name="jobLockService" ref="JobLockService" />
+    </bean>
+
     <bean id="userFilter.containedInGroup" class="${project.artifactId}.sync.GroupContainmentUserFilter">
         <property name="identitiesClient" ref="identitiesClient" />
     </bean>

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

@@ -15,13 +15,16 @@
  */
 package de.acosix.alfresco.keycloak.repo.authentication;
 
+import java.nio.file.AccessDeniedException;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
 import org.alfresco.repo.management.subsystems.ActivateableBean;
 import org.alfresco.repo.security.authentication.AbstractAuthenticationComponent;
 import org.alfresco.repo.security.authentication.AuthenticationException;
+import org.alfresco.repo.transaction.RetryingTransactionHelper;
 import org.alfresco.util.PropertyCheck;
 import org.keycloak.adapters.KeycloakDeployment;
 import org.keycloak.representations.AccessToken;
@@ -36,6 +39,7 @@ import de.acosix.alfresco.keycloak.repo.token.AccessTokenClient;
 import de.acosix.alfresco.keycloak.repo.token.AccessTokenException;
 import de.acosix.alfresco.keycloak.repo.token.AccessTokenRefreshException;
 import de.acosix.alfresco.keycloak.repo.util.RefreshableAccessTokenHolder;
+import net.sf.acegisecurity.Authentication;
 
 /**
  * This component provides Keycloak-integrated user/password authentication support to an Alfresco instance.
@@ -68,6 +72,10 @@ public class KeycloakAuthenticationComponent extends AbstractAuthenticationCompo
 
     protected List<TokenProcessor> tokenProcessors;
     
+    private RetryingTransactionHelper rthelper;
+    
+    private MissingPersonServiceImpl missingPersonService;
+
     /**
      *
      * {@inheritDoc}
@@ -77,12 +85,22 @@ public class KeycloakAuthenticationComponent extends AbstractAuthenticationCompo
     {
         PropertyCheck.mandatory(this, "applicationContext", this.applicationContext);
         PropertyCheck.mandatory(this, "keycloakDeployment", this.deployment);
+        PropertyCheck.mandatory(this, "missingPersonService", this.missingPersonService);
 
         this.accessTokenClient = new AccessTokenClient(this.deployment);
 
         this.tokenProcessors = new ArrayList<>(this.applicationContext.getBeansOfType(TokenProcessor.class, false, true).values());
         Collections.sort(this.tokenProcessors);
         this.tokenProcessors = Collections.unmodifiableList(this.tokenProcessors);
+
+        this.rthelper = new RetryingTransactionHelper();
+        this.rthelper.setMaxRetries(3);
+        this.rthelper.setMinRetryWaitMs(2000);
+        this.rthelper.setTransactionService(this.getTransactionService());
+        this.rthelper.setExtraExceptions(Arrays.asList(
+        		AccessDeniedException.class // likely caused by a race condition, where multiple threads are authenticating at the same time
+        									// this is due to modern web apps and threading
+        		));
     }
 
     /**
@@ -159,6 +177,14 @@ public class KeycloakAuthenticationComponent extends AbstractAuthenticationCompo
         this.deployment = deployment;
     }
 
+    /**
+     * @param missingPersonService
+     *     the missingPersonService to set
+     */    
+    public void setMissingPersonService(MissingPersonServiceImpl missingPersonService) {
+		this.missingPersonService = missingPersonService;
+	}
+
     /**
      * Enables the thread-local storage of the last access token response and verified tokens beyond the internal needs of
      * {@link #authenticateImpl(String, char[]) authenticateImpl}.
@@ -294,4 +320,24 @@ public class KeycloakAuthenticationComponent extends AbstractAuthenticationCompo
     {
         return this.allowGuestLogin;
     }
+    
+    public Authentication setCurrentUser(final String realUserName) throws AuthenticationException {
+    	RuntimeException lastre = null;
+    	
+		for (int l = 0; l < 2; l++) {
+			LOGGER.trace("Setting current user: {}", realUserName);
+			
+	    	try {
+	    		return super.setCurrentUser(realUserName);
+			} catch (RuntimeException re) {
+				this.missingPersonService.handle(realUserName, re);
+				LOGGER.debug("Missing person handled; looping back: {}", realUserName);
+				lastre = re;
+        	}
+        }
+		
+		if (lastre != null)
+			throw lastre;
+		else throw new IllegalStateException("This should never happen");
+    }
 }

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

@@ -138,6 +138,8 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter
 
     protected RuntimeContainer publicApiRuntimeContainer;
     
+    private MissingPersonServiceImpl missingPersonService;
+
     /**
      * {@inheritDoc}
      */
@@ -159,6 +161,7 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter
         PropertyCheck.mandatory(this, "personService", this.personService);
         PropertyCheck.mandatory(this, "nodeService", this.nodeService);
         PropertyCheck.mandatory(this, "transactionService", this.transactionService);
+        PropertyCheck.mandatory(this, "missingPersonService", this.missingPersonService);
 
         // basic is handled ourselves
         this.keycloakDeployment.setEnableBasicAuth(false);
@@ -291,6 +294,14 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter
         this.publicApiRuntimeContainer = publicApiRuntimeContainer;
     }
 
+    /**
+     * @param missingPersonService
+     *     the missingPersonService to set
+     */    
+    public void setMissingPersonService(MissingPersonServiceImpl missingPersonService) {
+		this.missingPersonService = missingPersonService;
+	}
+
     /**
      *
      * {@inheritDoc}
@@ -636,7 +647,16 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter
     @Override
     protected SessionUser createUserEnvironment(final HttpSession session, final String userName) throws IOException, ServletException
     {
-        final SessionUser sessionUser = super.createUserEnvironment(session, userName);
+    	SessionUser sessionUser;
+    	try {
+    		sessionUser = super.createUserEnvironment(session, userName);
+    	}
+    	catch (RuntimeException re)
+    	{
+    		LOGGER.warn("Failed to create user environemnt; trying to resolve: {}", re.getMessage());
+    		this.missingPersonService.handle(userName, re);
+    		sessionUser = super.createUserEnvironment(session, userName);
+    	}
 
         // ensure all common attribute names are mapped
         // Alfresco is really inconsistent with these attribute names
@@ -654,7 +674,16 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter
     protected SessionUser createUserEnvironment(final HttpSession session, final String userName, final String ticket,
             final boolean externalAuth) throws IOException, ServletException
     {
-        final SessionUser sessionUser = super.createUserEnvironment(session, userName, ticket, externalAuth);
+    	SessionUser sessionUser;
+    	try {
+    		sessionUser = super.createUserEnvironment(session, userName, ticket, externalAuth);
+    	}
+    	catch (RuntimeException re)
+    	{
+    		LOGGER.warn("Failed to create user environemnt; trying to resolve: {}", re.getMessage());
+    		this.missingPersonService.handle(userName, re);
+    		sessionUser = super.createUserEnvironment(session, userName, ticket, externalAuth);
+    	}
 
         // ensure all common attribute names are mapped
         // Alfresco is really inconsistent with these attribute names
@@ -986,7 +1015,7 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter
             if (sessionUser == null)
             {
                 LOGGER.debug("Propagating through the user identity: {}", AlfrescoCompatibilityUtil.maskUsername(userId));
-                this.authenticationComponent.setCurrentUser(userId);
+                this.keycloakAuthenticationComponent.setCurrentUser(userId);
                 session = httpServletRequest.getSession();
 
                 try

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

@@ -172,13 +172,8 @@ public class KeycloakTokenGroupSyncProcessor implements TokenProcessor, Initiali
             {
                 AuthenticationUtil.runAsSystem(() -> this.transactionService.getRetryingTransactionHelper().doInTransaction(() -> {
                     boolean changed = this.syncGroupMemberships(groups);
-                    if (changed) {
+                    if (changed)
-                    	String ticket = this.authenticationService.getCurrentTicket();
+                		LOGGER.debug("Group membership changed: {}", accessToken.getSubject());
-                    	if (ticket != null) {
-                    		LOGGER.debug("Invalidating Alflresco ticket as group membership changed: {}", ticket);
-                    		this.authenticationService.invalidateTicket(ticket);
-                    	}
-                    }
                     return null;
                 }, false, requiresNew));
             }
@@ -202,7 +197,7 @@ public class KeycloakTokenGroupSyncProcessor implements TokenProcessor, Initiali
         // in case some extractor mapped this pseudo-group
         groups.remove(PermissionService.ALL_AUTHORITIES);
 
-        LOGGER.debug("Mapped user group authorities from access token: {}", groups);
+        LOGGER.debug("Mapped user group authorities from access token: {} => {}", accessToken.getSubject(), groups);
 
         return groups;
     }

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

@@ -0,0 +1,123 @@
+package de.acosix.alfresco.keycloak.repo.authentication;
+
+import java.io.Serializable;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.alfresco.error.AlfrescoRuntimeException;
+import org.alfresco.model.ContentModel;
+import org.alfresco.repo.lock.JobLockService;
+import org.alfresco.repo.security.authentication.AuthenticationException;
+import org.alfresco.repo.security.authentication.AuthenticationUtil;
+import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork;
+import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback;
+import org.alfresco.service.cmr.repository.NodeRef;
+import org.alfresco.service.cmr.security.NoSuchPersonException;
+import org.alfresco.service.cmr.security.PersonService;
+import org.alfresco.service.namespace.NamespaceService;
+import org.alfresco.service.namespace.QName;
+import org.alfresco.service.transaction.TransactionService;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class MissingPersonServiceImpl {
+	
+	private static final Logger LOGGER = LoggerFactory.getLogger(MissingPersonServiceImpl.class);
+	
+	private PersonService personService;
+	
+	private TransactionService txService;
+	
+	private JobLockService jobLockService;
+	
+	public void setPersonService(PersonService personService) {
+		this.personService = personService;
+	}
+	
+	public void setTransactionService(TransactionService txService) {
+		this.txService = txService;
+	}
+	
+	public void setJobLockService(JobLockService jobLockService) {
+		this.jobLockService = jobLockService;
+	}
+	
+	public void handleAuthentication(final String username, AuthenticationException ae) throws AuthenticationException {
+		if (!ae.getMessage().contains("does not exist in Alfresco")) {
+			LOGGER.debug("Not supported by MissingPersonService: " + ae.getMessage(), ae);
+			throw ae;
+		}
+		
+		LOGGER.debug("AuthenticationException; user doesn't exist; creating person: {}", username);
+		this.createPerson(username);
+	}
+	
+	public void handleNoSuchPerson(final String username, NoSuchPersonException nspe) throws NoSuchPersonException {
+		LOGGER.debug("NoSuchPersonException; Creating person: {}", username);
+		this.createPerson(username);
+	}
+	
+	public void handle(String username, RuntimeException re) throws RuntimeException {
+		if (re instanceof AuthenticationException) {
+			this.handleAuthentication(username, (AuthenticationException) re);
+		} else if (re instanceof NoSuchPersonException) {
+			this.handleNoSuchPerson(username, (NoSuchPersonException) re);
+		} else if (re.getCause() instanceof AuthenticationException) {
+			this.handleAuthentication(username, (AuthenticationException) re.getCause());
+		} else if (re.getCause() instanceof NoSuchPersonException) {
+			this.handleNoSuchPerson(username, (NoSuchPersonException) re.getCause());
+		} else {
+			LOGGER.debug("Not supported by MissingPersonService: " + re.getMessage() + " => " + re.getCause(), re);
+			throw re;
+		}
+	}
+	
+	private void createPerson(final String username) {
+        final RetryingTransactionCallback<NodeRef> rtcallback = new RetryingTransactionCallback<NodeRef>() {
+        	@Override
+        	public NodeRef execute() {
+				if (personService.personExists(username)) {
+					LOGGER.debug("Person (now) already exists: {}", username);
+					return null;
+				} else {
+					LOGGER.info("Creating person: {}", username);
+					
+					Map<QName, Serializable> properties = new HashMap<>();
+					properties.put(ContentModel.PROP_USERNAME, username);
+					properties.put(ContentModel.PROP_FIRSTNAME, "Keycloak");
+					properties.put(ContentModel.PROP_LASTNAME, "Unknown");
+					return personService.createPerson(properties);
+				}
+			}
+		};
+		
+		try {
+			AuthenticationUtil.runAsSystem(new RunAsWork<NodeRef>() {
+				@Override
+				public NodeRef doWork() {
+					boolean readonly = txService.isReadOnly();
+					QName lockQname = QName.createQName(NamespaceService.CONTENT_MODEL_1_0_URI, QName.createValidLocalName(username));
+
+					LOGGER.trace("Obtaining exclusive lock on creation of person: {}", lockQname);
+					String lockId = jobLockService.getLock(lockQname, 2000L, 250L, 20);
+					try {
+						return txService.getRetryingTransactionHelper().doInTransaction(rtcallback, false, readonly);
+					} finally {
+						LOGGER.trace("Releasing exclusive lock: {}", lockQname);
+						jobLockService.releaseLock(lockId, lockQname);
+					}
+				}
+			});
+		} catch (RuntimeException re) {
+			LOGGER.warn("Failed to create person: {} => {}", username, re.getMessage());
+			if (re instanceof AlfrescoRuntimeException) {
+				if (re.getMessage().contains("already exists"))
+					LOGGER.info("Person already exists; likely thwarted race condition: {}", username);
+					return;
+			}
+			
+			throw re;
+		}
+	}
+
+}