diff --git a/repository/src/main/config/alfresco-global.properties b/repository/src/main/config/alfresco-global.properties index ad180f0..213c125 100644 --- a/repository/src/main/config/alfresco-global.properties +++ b/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 diff --git a/repository/src/main/globalConfig/subsystems/Authentication/keycloak/keycloak-authentication-context.xml b/repository/src/main/globalConfig/subsystems/Authentication/keycloak/keycloak-authentication-context.xml index 40d3535..027a86d 100644 --- a/repository/src/main/globalConfig/subsystems/Authentication/keycloak/keycloak-authentication-context.xml +++ b/repository/src/main/globalConfig/subsystems/Authentication/keycloak/keycloak-authentication-context.xml @@ -60,6 +60,8 @@ + + @@ -154,6 +156,8 @@ + + @@ -224,6 +228,12 @@ + + + + + + diff --git a/repository/src/main/java/de/acosix/alfresco/keycloak/repo/authentication/KeycloakAuthenticationComponent.java b/repository/src/main/java/de/acosix/alfresco/keycloak/repo/authentication/KeycloakAuthenticationComponent.java index a504808..d02e6ba 100644 --- a/repository/src/main/java/de/acosix/alfresco/keycloak/repo/authentication/KeycloakAuthenticationComponent.java +++ b/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. @@ -67,6 +71,10 @@ public class KeycloakAuthenticationComponent extends AbstractAuthenticationCompo protected AccessTokenClient accessTokenClient; protected List tokenProcessors; + + private RetryingTransactionHelper rthelper; + + private MissingPersonServiceImpl missingPersonService; /** * @@ -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"); + } } diff --git a/repository/src/main/java/de/acosix/alfresco/keycloak/repo/authentication/KeycloakAuthenticationFilter.java b/repository/src/main/java/de/acosix/alfresco/keycloak/repo/authentication/KeycloakAuthenticationFilter.java index 03d1a18..9ff5b36 100644 --- a/repository/src/main/java/de/acosix/alfresco/keycloak/repo/authentication/KeycloakAuthenticationFilter.java +++ b/repository/src/main/java/de/acosix/alfresco/keycloak/repo/authentication/KeycloakAuthenticationFilter.java @@ -137,6 +137,8 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter protected SimpleCache keycloakTicketTokenCache; 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 diff --git a/repository/src/main/java/de/acosix/alfresco/keycloak/repo/authentication/KeycloakTokenGroupSyncProcessor.java b/repository/src/main/java/de/acosix/alfresco/keycloak/repo/authentication/KeycloakTokenGroupSyncProcessor.java index 4a2dcf1..d28eb28 100644 --- a/repository/src/main/java/de/acosix/alfresco/keycloak/repo/authentication/KeycloakTokenGroupSyncProcessor.java +++ b/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) { - String ticket = this.authenticationService.getCurrentTicket(); - if (ticket != null) { - LOGGER.debug("Invalidating Alflresco ticket as group membership changed: {}", ticket); - this.authenticationService.invalidateTicket(ticket); - } - } + if (changed) + LOGGER.debug("Group membership changed: {}", accessToken.getSubject()); 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; } diff --git a/repository/src/main/java/de/acosix/alfresco/keycloak/repo/authentication/MissingPersonServiceImpl.java b/repository/src/main/java/de/acosix/alfresco/keycloak/repo/authentication/MissingPersonServiceImpl.java new file mode 100644 index 0000000..5c5ba72 --- /dev/null +++ b/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 rtcallback = new RetryingTransactionCallback() { + @Override + public NodeRef execute() { + if (personService.personExists(username)) { + LOGGER.debug("Person (now) already exists: {}", username); + return null; + } else { + LOGGER.info("Creating person: {}", username); + + Map 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() { + @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; + } + } + +}