Handle user and group name synchronisation consistently, and fix resource role exposure

- user names can now also be custom mapped from attributes
- introduced priority in user/group processors
- introduced operation dedicated to mapping authority name for use in user/group name collection operations

repository/src/main/java/de/acosix/alfresco/keycloak/repo/roles/RoleServiceImpl.java

@@ -466,7 +466,7 @@ public class RoleServiceImpl implements RoleService, InitializingBean
     {
         List<Role> roles;
 
-        if (this.enabled && !this.processResourceRoles)
+        if (this.enabled && this.processResourceRoles)
         {
             final RoleNameFilter roleNameFilter = this.resourceRoleNameFilter.get(resourceName);
             final RoleNameMapper roleNameMapper = this.resourceRoleNameMapper.get(resourceName);

repository/src/main/java/de/acosix/alfresco/keycloak/repo/sync/BaseAttributeProcessor.java

@@ -20,6 +20,7 @@ import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.function.Predicate;
 
 import org.alfresco.repo.security.sync.NodeDescription;
@@ -38,6 +39,8 @@ public abstract class BaseAttributeProcessor implements InitializingBean
 
     protected NamespaceService namespaceService;
 
+    protected int priority = 50;
+
     protected boolean mapBlankString;
 
     protected boolean mapNull;
@@ -71,6 +74,15 @@ public abstract class BaseAttributeProcessor implements InitializingBean
         this.namespaceService = namespaceService;
     }
 
+    /**
+     * @param priority
+     *     the priority to set
+     */
+    public void setPriority(final int priority)
+    {
+        this.priority = priority;
+    }
+
     /**
      * @param attributes
      *     the attributes to set
@@ -169,4 +181,34 @@ public abstract class BaseAttributeProcessor implements InitializingBean
             nodeDescription.getProperties().put(propertyQName, null);
         }
     }
+
+    protected Optional<String> mapAuthorityName(final QName authorityNameProperty, final Map<String, List<String>> attributes)
+    {
+        final Optional<String> result;
+        final String attribute = this.attributePropertyQNameMappings.entrySet().stream()
+                .filter((final Map.Entry<String, QName> e) -> authorityNameProperty.equals(e.getValue())).findFirst().map(Map.Entry::getKey)
+                .orElse(null);
+        if (attribute != null)
+        {
+            List<String> attrValues = attributes.get(attribute);
+            if (attrValues != null && !this.mapBlankString)
+            {
+                attrValues = attrValues.stream().filter(Predicate.not(String::isBlank)).toList();
+            }
+
+            if (attrValues != null && attrValues.size() == 1)
+            {
+                result = Optional.of(attrValues.get(0));
+            }
+            else
+            {
+                result = Optional.empty();
+            }
+        }
+        else
+        {
+            result = Optional.empty();
+        }
+        return result;
+    }
 }

repository/src/main/java/de/acosix/alfresco/keycloak/repo/sync/DefaultGroupProcessor.java

@@ -15,9 +15,10 @@
  */
 package de.acosix.alfresco.keycloak.repo.sync;
 
+import java.util.Optional;
+
 import org.alfresco.model.ContentModel;
 import org.alfresco.repo.security.sync.NodeDescription;
-import org.alfresco.service.cmr.repository.datatype.DefaultTypeConverter;
 import org.alfresco.util.PropertyMap;
 import org.keycloak.representations.idm.GroupRepresentation;
 
@@ -40,6 +41,16 @@ public class DefaultGroupProcessor implements GroupProcessor
         this.enabled = enabled;
     }
 
+    /**
+     *
+     * {@inheritDoc}
+     */
+    @Override
+    public int getPriority()
+    {
+        return Integer.MAX_VALUE;
+    }
+
     /**
      *
      * {@inheritDoc}
@@ -50,20 +61,18 @@ public class DefaultGroupProcessor implements GroupProcessor
         if (this.enabled)
         {
             final PropertyMap properties = groupNode.getProperties();
-
+            properties.put(ContentModel.PROP_AUTHORITY_NAME, group.getId());
-            final String existingName = DefaultTypeConverter.INSTANCE.convert(String.class,
+            properties.put(ContentModel.PROP_AUTHORITY_DISPLAY_NAME, group.getName());
-                    properties.get(ContentModel.PROP_AUTHORITY_NAME));
-            final String existingDisplayName = DefaultTypeConverter.INSTANCE.convert(String.class,
-                    properties.get(ContentModel.PROP_AUTHORITY_DISPLAY_NAME));
-
-            if (existingName == null || existingName.isBlank())
-            {
-                properties.put(ContentModel.PROP_AUTHORITY_NAME, group.getId());
-            }
-            if (existingDisplayName == null || existingDisplayName.isBlank())
-            {
-                properties.put(ContentModel.PROP_AUTHORITY_DISPLAY_NAME, group.getName());
-            }
         }
     }
+
+    /**
+     *
+     * {@inheritDoc}
+     */
+    @Override
+    public Optional<String> mapGroupName(final GroupRepresentation group)
+    {
+        return this.enabled ? Optional.of(group.getId()) : Optional.empty();
+    }
 }

repository/src/main/java/de/acosix/alfresco/keycloak/repo/sync/DefaultPersonProcessor.java

@@ -18,6 +18,7 @@ package de.acosix.alfresco.keycloak.repo.sync;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Optional;
 
 import org.alfresco.model.ContentModel;
 import org.alfresco.repo.security.sync.NodeDescription;
@@ -47,7 +48,7 @@ public class DefaultPersonProcessor implements UserProcessor
 
     /**
      * @param enabled
-     *            the enabled to set
+     *     the enabled to set
      */
     public void setEnabled(final boolean enabled)
     {
@@ -56,7 +57,7 @@ public class DefaultPersonProcessor implements UserProcessor
 
     /**
      * @param mapNull
-     *            the mapNull to set
+     *     the mapNull to set
      */
     public void setMapNull(final boolean mapNull)
     {
@@ -65,7 +66,7 @@ public class DefaultPersonProcessor implements UserProcessor
 
     /**
      * @param mapFirstName
-     *            the mapFirstName to set
+     *     the mapFirstName to set
      */
     public void setMapFirstName(final boolean mapFirstName)
     {
@@ -74,7 +75,7 @@ public class DefaultPersonProcessor implements UserProcessor
 
     /**
      * @param mapLastName
-     *            the mapLastName to set
+     *     the mapLastName to set
      */
     public void setMapLastName(final boolean mapLastName)
     {
@@ -83,7 +84,7 @@ public class DefaultPersonProcessor implements UserProcessor
 
     /**
      * @param mapEmail
-     *            the mapEmail to set
+     *     the mapEmail to set
      */
     public void setMapEmail(final boolean mapEmail)
     {
@@ -92,13 +93,23 @@ public class DefaultPersonProcessor implements UserProcessor
 
     /**
      * @param mapEnabledState
-     *            the mapEnabledState to set
+     *     the mapEnabledState to set
      */
     public void setMapEnabledState(final boolean mapEnabledState)
     {
         this.mapEnabledState = mapEnabledState;
     }
 
+    /**
+     *
+     * {@inheritDoc}
+     */
+    @Override
+    public int getPriority()
+    {
+        return Integer.MAX_VALUE;
+    }
+
     /**
      *
      * {@inheritDoc}
@@ -110,6 +121,7 @@ public class DefaultPersonProcessor implements UserProcessor
         {
             final PropertyMap properties = person.getProperties();
 
+            properties.put(ContentModel.PROP_USERNAME, user.getUsername());
             if ((this.mapNull || user.getFirstName() != null) && this.mapFirstName)
             {
                 properties.put(ContentModel.PROP_FIRSTNAME, user.getFirstName());
@@ -140,6 +152,7 @@ public class DefaultPersonProcessor implements UserProcessor
         if (this.enabled)
         {
             mappedProperties = new ArrayList<>(4);
+            mappedProperties.add(ContentModel.PROP_USERNAME);
             if (this.mapFirstName)
             {
                 mappedProperties.add(ContentModel.PROP_FIRSTNAME);
@@ -164,4 +177,14 @@ public class DefaultPersonProcessor implements UserProcessor
 
         return mappedProperties;
     }
+
+    /**
+     *
+     * {@inheritDoc}
+     */
+    @Override
+    public Optional<String> mapUserName(final UserRepresentation user)
+    {
+        return this.enabled ? Optional.of(user.getUsername()) : Optional.empty();
+    }
 }

repository/src/main/java/de/acosix/alfresco/keycloak/repo/sync/GroupProcessor.java

@@ -15,6 +15,8 @@
  */
 package de.acosix.alfresco.keycloak.repo.sync;
 
+import java.util.Optional;
+
 import org.alfresco.repo.security.sync.NodeDescription;
 import org.keycloak.representations.idm.GroupRepresentation;
 
@@ -25,16 +27,53 @@ import org.keycloak.representations.idm.GroupRepresentation;
  *
  * @author Axel Faust
  */
-public interface GroupProcessor
+public interface GroupProcessor extends Comparable<GroupProcessor>
 {
 
+    /**
+     * Retrieves the priority of this processor. A lower value specifies a higher priority and the mapped properties / group name of
+     * processors with higher priorities may override those of lower priorities.
+     *
+     * @return the priority as an integer with {@code 50} as the default priority.
+     */
+    default int getPriority()
+    {
+        return 50;
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    default int compareTo(final GroupProcessor other)
+    {
+        int res = Integer.compare(this.getPriority(), other.getPriority());
+        if (res == 0)
+        {
+            res = this.getClass().getName().compareTo(other.getClass().getName());
+        }
+        return res;
+    }
+
     /**
      * Maps data from a Keycloak group representation to a description of an Alfresco node for the authority container.
      *
      * @param group
-     *            the Keycloak group representation
+     *     the Keycloak group representation
      * @param groupNodeDescription
-     *            the Alfresco node description
+     *     the Alfresco node description
      */
     void mapGroup(GroupRepresentation group, NodeDescription groupNodeDescription);
+
+    /**
+     * Maps a Keycloak group representation to the group name to use in Alfresco.
+     *
+     * @param group
+     *     the Keycloak group representation
+     * @return the Alfresco group name
+     */
+    default Optional<String> mapGroupName(final GroupRepresentation group)
+    {
+        return Optional.empty();
+    }
 }

repository/src/main/java/de/acosix/alfresco/keycloak/repo/sync/KeycloakUserRegistry.java

@@ -24,18 +24,19 @@ import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.NoSuchElementException;
+import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.Consumer;
 import java.util.function.Function;
 import java.util.function.IntConsumer;
+import java.util.function.Predicate;
 
 import org.alfresco.error.AlfrescoRuntimeException;
 import org.alfresco.model.ContentModel;
 import org.alfresco.repo.management.subsystems.ActivateableBean;
 import org.alfresco.repo.security.sync.NodeDescription;
 import org.alfresco.repo.security.sync.UserRegistry;
-import org.alfresco.service.cmr.repository.datatype.DefaultTypeConverter;
 import org.alfresco.service.cmr.security.AuthorityType;
 import org.alfresco.service.namespace.QName;
 import org.alfresco.util.PropertyCheck;
@@ -198,7 +199,7 @@ public class KeycloakUserRegistry implements UserRegistry, InitializingBean, Act
         if (this.active)
         {
             personNames = new UserCollection<>(this.personLoadBatchSize, this.identitiesClient.countUsers(),
-                    UserRepresentation::getUsername);
+                    this::determineEffectiveUserName);
         }
 
         return personNames;
@@ -215,7 +216,7 @@ public class KeycloakUserRegistry implements UserRegistry, InitializingBean, Act
         if (this.active)
         {
             groupNames = new GroupCollection<>(this.groupLoadBatchSize, this.identitiesClient.countGroups(),
-                    group -> AuthorityType.GROUP.getPrefixString() + group.getId());
+                    this::determineEffectiveGroupName);
         }
 
         return groupNames;
@@ -244,14 +245,17 @@ public class KeycloakUserRegistry implements UserRegistry, InitializingBean, Act
     protected NodeDescription mapUser(final UserRepresentation user)
     {
         final NodeDescription person = new NodeDescription(user.getId());
+
+        LOGGER.debug("Mapping user {} ({})", user.getUsername(), user.getId());
+
+        // reverse ordered so higher priority mappers may override properties of lower priority ones
+        this.userProcessors.stream().sorted((o1, o2) -> -o1.compareTo(o2)).forEach(processor -> processor.mapUser(user, person));
+
         final PropertyMap personProperties = person.getProperties();
+        final String userName = this.determineEffectiveUserName(user);
+        personProperties.put(ContentModel.PROP_USERNAME, userName);
 
-        LOGGER.debug("Mapping user {}", user.getUsername());
+        LOGGER.debug("Mapped user {} ({}) as {}", user.getUsername(), user.getId(), userName);
-
-        this.userProcessors.forEach(processor -> processor.mapUser(user, person));
-
-        // always wins against user-defined mappings for cm:userName
-        personProperties.put(ContentModel.PROP_USERNAME, user.getUsername());
 
         return person;
     }
@@ -266,28 +270,20 @@ public class KeycloakUserRegistry implements UserRegistry, InitializingBean, Act
     protected NodeDescription mapGroup(final GroupRepresentation group)
     {
         final NodeDescription groupD = new NodeDescription(group.getId());
-        final PropertyMap groupProperties = groupD.getProperties();
 
         LOGGER.debug("Mapping group {} ({})", group.getName(), group.getId());
 
         this.groupProcessors.forEach(processor -> processor.mapGroup(group, groupD));
 
-        // make sure groupName is mapped + prefixed
+        final PropertyMap groupProperties = groupD.getProperties();
-        String groupName = DefaultTypeConverter.INSTANCE.convert(String.class, groupProperties.get(ContentModel.PROP_AUTHORITY_NAME));
+        final String groupName = this.determineEffectiveGroupName(group);
-        if (groupName == null || groupName.isBlank())
-        {
-            // should never happen due to DefaultGroupProcessor
-            groupName = group.getId();
-        }
-        if (AuthorityType.getAuthorityType(groupName) != AuthorityType.GROUP)
-        {
-            groupName = AuthorityType.GROUP.getPrefixString() + group.getId();
-        }
         groupProperties.put(ContentModel.PROP_AUTHORITY_NAME, groupName);
 
+        LOGGER.debug("Mapped group {} ({}) as {}", group.getName(), group.getId(), groupName);
+
         final Set<String> childAssociations = groupD.getChildAssociations();
         group.getSubGroups().stream().filter(subGroup -> isGroupAllowed(this.groupFilters, subGroup))
-                .forEach(subGroup -> childAssociations.add(AuthorityType.GROUP.getPrefixString() + subGroup.getId()));
+                .forEach(subGroup -> childAssociations.add(this.determineEffectiveGroupName(subGroup)));
 
         int offset = 0;
         int processedMembers = 1;
@@ -296,7 +292,7 @@ public class KeycloakUserRegistry implements UserRegistry, InitializingBean, Act
             processedMembers = this.identitiesClient.processMembers(group.getId(), offset, this.personLoadBatchSize, user -> {
                 if (KeycloakUserRegistry.isUserAllowed(this.userFilters, user))
                 {
-                    childAssociations.add(user.getUsername());
+                    childAssociations.add(this.determineEffectiveUserName(user));
                 }
             });
             offset += processedMembers;
@@ -307,6 +303,40 @@ public class KeycloakUserRegistry implements UserRegistry, InitializingBean, Act
         return groupD;
     }
 
+    private String determineEffectiveUserName(final UserRepresentation user)
+    {
+        final List<String> userNameCandidates = this.userProcessors.stream().sorted().map(gp -> gp.mapUserName(user))
+                .filter(Predicate.not(Optional::isEmpty)).map(Optional::get).toList();
+
+        String userName = userNameCandidates.isEmpty() ? null : userNameCandidates.get(0);
+        if (userName == null || userName.isBlank())
+        {
+            // should never happen due to DefaultPersonProcessor
+            userName = user.getUsername();
+        }
+        return userName;
+    }
+
+    private String determineEffectiveGroupName(final GroupRepresentation group)
+    {
+        final List<String> groupNameCandidates = this.groupProcessors.stream().sorted().map(gp -> gp.mapGroupName(group))
+                .filter(Predicate.not(Optional::isEmpty)).map(Optional::get).toList();
+
+        String groupName = groupNameCandidates.isEmpty() ? null : groupNameCandidates.get(0);
+        if (groupName == null || groupName.isBlank())
+        {
+            // should never happen due to DefaultGroupProcessor
+            groupName = group.getId();
+        }
+
+        // make sure groupName is prefixed
+        if (AuthorityType.getAuthorityType(groupName) != AuthorityType.GROUP)
+        {
+            groupName = AuthorityType.GROUP.getPrefixString() + groupName;
+        }
+        return groupName;
+    }
+
     /**
      * This class provides common basic functionalities for a collection of Keycloak authority-based data elements, supporting basic batch
      * load-based pagination / iterative traversal.
@@ -558,9 +588,12 @@ public class KeycloakUserRegistry implements UserRegistry, InitializingBean, Act
             final List<GroupRepresentation> subGroups = group.getSubGroups();
             if (subGroups == null || subGroups.isEmpty())
             {
+                final List<GroupRepresentation> newSubGroups = new ArrayList<>();
+                group.setSubGroups(newSubGroups);
                 try
                 {
                     final int loadedChilren = KeycloakUserRegistry.this.identitiesClient.processSubGroups(group.getId(), subGroup -> {
+                        newSubGroups.add(subGroup);
                         this.processGroupsRecursively(subGroup, filteredHandler, authorityProcessor, count);
                     });
                     count.addAndGet(loadedChilren);

repository/src/main/java/de/acosix/alfresco/keycloak/repo/sync/SimpleGroupAttributeProcessor.java

@@ -15,6 +15,9 @@
  */
 package de.acosix.alfresco.keycloak.repo.sync;
 
+import java.util.Optional;
+
+import org.alfresco.model.ContentModel;
 import org.alfresco.repo.security.sync.NodeDescription;
 import org.keycloak.representations.idm.GroupRepresentation;
 
@@ -23,21 +26,30 @@ import org.keycloak.representations.idm.GroupRepresentation;
  *
  * @author Axel Faust
  */
-public class SimpleGroupAttributeProcessor extends BaseAttributeProcessor
+public class SimpleGroupAttributeProcessor extends BaseAttributeProcessor implements GroupProcessor
-        implements GroupProcessor
 {
 
     protected boolean enabled;
 
     /**
      * @param enabled
-     *            the enabled to set
+     *     the enabled to set
      */
     public void setEnabled(final boolean enabled)
     {
         this.enabled = enabled;
     }
 
+    /**
+     *
+     * {@inheritDoc}
+     */
+    @Override
+    public int getPriority()
+    {
+        return this.priority;
+    }
+
     /**
      *
      * {@inheritDoc}
@@ -51,4 +63,13 @@ public class SimpleGroupAttributeProcessor extends BaseAttributeProcessor
         }
     }
 
+    /**
+     *
+     * {@inheritDoc}
+     */
+    @Override
+    public Optional<String> mapGroupName(final GroupRepresentation group)
+    {
+        return this.enabled ? this.mapAuthorityName(ContentModel.PROP_AUTHORITY_NAME, group.getAttributes()) : Optional.empty();
+    }
 }

repository/src/main/java/de/acosix/alfresco/keycloak/repo/sync/SimpleUserAttributeProcessor.java

@@ -18,7 +18,9 @@ package de.acosix.alfresco.keycloak.repo.sync;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
+import java.util.Optional;
 
+import org.alfresco.model.ContentModel;
 import org.alfresco.repo.security.sync.NodeDescription;
 import org.alfresco.service.namespace.QName;
 import org.keycloak.representations.idm.UserRepresentation;
@@ -28,21 +30,30 @@ import org.keycloak.representations.idm.UserRepresentation;
  *
  * @author Axel Faust
  */
-public class SimpleUserAttributeProcessor extends BaseAttributeProcessor
+public class SimpleUserAttributeProcessor extends BaseAttributeProcessor implements UserProcessor
-        implements UserProcessor
 {
 
     protected boolean enabled;
 
     /**
      * @param enabled
-     *            the enabled to set
+     *     the enabled to set
      */
     public void setEnabled(final boolean enabled)
     {
         this.enabled = enabled;
     }
 
+    /**
+     *
+     * {@inheritDoc}
+     */
+    @Override
+    public int getPriority()
+    {
+        return this.priority;
+    }
+
     /**
      *
      * {@inheritDoc}
@@ -66,4 +77,14 @@ public class SimpleUserAttributeProcessor extends BaseAttributeProcessor
         return this.enabled ? new HashSet<>(this.attributePropertyQNameMappings.values()) : Collections.emptySet();
     }
 
+    /**
+     *
+     * {@inheritDoc}
+     */
+    @Override
+    public Optional<String> mapUserName(final UserRepresentation user)
+    {
+        return this.enabled ? this.mapAuthorityName(ContentModel.PROP_USERNAME, user.getAttributes()) : Optional.empty();
+    }
+
 }

repository/src/main/java/de/acosix/alfresco/keycloak/repo/sync/UserProcessor.java

@@ -16,6 +16,7 @@
 package de.acosix.alfresco.keycloak.repo.sync;
 
 import java.util.Collection;
+import java.util.Optional;
 
 import org.alfresco.repo.security.sync.NodeDescription;
 import org.alfresco.service.namespace.QName;
@@ -28,16 +29,41 @@ import org.keycloak.representations.idm.UserRepresentation;
  *
  * @author Axel Faust
  */
-public interface UserProcessor
+public interface UserProcessor extends Comparable<UserProcessor>
 {
 
+    /**
+     * Retrieves the priority of this processor. A lower value specifies a higher priority and the mapped properties / user name of
+     * processors with higher priorities may override those of lower priorities.
+     *
+     * @return the priority as an integer with {@code 50} as the default priority.
+     */
+    default int getPriority()
+    {
+        return 50;
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    default int compareTo(final UserProcessor other)
+    {
+        int res = Integer.compare(this.getPriority(), other.getPriority());
+        if (res == 0)
+        {
+            res = this.getClass().getName().compareTo(other.getClass().getName());
+        }
+        return res;
+    }
+
     /**
      * Maps data from a Keycloak user representation to a description of an Alfresco node for the person.
      *
      * @param user
-     *            the Keycloak user representation
+     *     the Keycloak user representation
      * @param personNodeDescription
-     *            the Alfresco node description
+     *     the Alfresco node description
      */
     void mapUser(UserRepresentation user, NodeDescription personNodeDescription);
 
@@ -47,4 +73,16 @@ public interface UserProcessor
      * @return the set of person node properties mapped by this instance
      */
     Collection<QName> getMappedProperties();
+
+    /**
+     * Maps a Keycloak user representation to the user name to use in Alfresco.
+     *
+     * @param group
+     *     the Keycloak user representation
+     * @return the Alfresco user name
+     */
+    default Optional<String> mapUserName(final UserRepresentation group)
+    {
+        return Optional.empty();
+    }
 }