ALF-3604: Correct case-sensitivity issues in LDAP sync

- User names are now brought in line with the case of the LDAP directory during sync (in case the UID attribute is case sensitive)
- User names are now compared according to Alfresco's case sensitivity setting
- Group name comparisions are still case sensitive
- Added unit test to ensure correct behaviour


git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@20873 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

source/java/org/alfresco/repo/security/authority/AuthorityDAOImpl.java

@@ -187,10 +187,18 @@ public class AuthorityDAOImpl implements AuthorityDAO, NodeServicePolicies.Befor
             parentRefs.add(parentRef);
         }
         NodeRef childRef = getAuthorityOrNull(childName);
+
         if (childRef == null)
         {
             throw new UnknownAuthorityException("An authority was not found for " + childName);
         }
+
+        // Normalize the user name if necessary
+        if (isUser)
+        {
+            childName = (String) nodeService.getProperty(childRef, ContentModel.PROP_USERNAME);
+        }
+
         nodeService.addChild(parentRefs, childRef, ContentModel.ASSOC_MEMBER, QName.createQName("cm", childName,
                 namespacePrefixResolver));
         if (isUser)
@@ -830,6 +838,12 @@ public class AuthorityDAOImpl implements AuthorityDAO, NodeServicePolicies.Befor
             NodeRef authRef = getAuthorityOrNull(authorityName);
             if (authRef != null)
             {
+                // Normalize the user name if necessary
+                if (AuthorityType.getAuthorityType(authorityName) == AuthorityType.USER)
+                {
+                    authorityName = (String) nodeService.getProperty(authRef, ContentModel.PROP_USERNAME);
+                }
+                
                 nodeService.addChild(zoneRefs, authRef, ContentModel.ASSOC_IN_ZONE, QName.createQName("cm", authorityName, namespacePrefixResolver));
             }
         }

source/java/org/alfresco/repo/security/person/PersonServiceImpl.java

@@ -616,7 +616,15 @@ public class PersonServiceImpl extends TransactionListenerAdapter implements Per
                 makeHomeFolderIfRequired(personNode);                
             }
             String realUserName = DefaultTypeConverter.INSTANCE.convert(String.class, nodeService.getProperty(personNode, ContentModel.PROP_USERNAME));
-            properties.put(ContentModel.PROP_USERNAME, realUserName);
+            String suggestedUserName;
+
+            // LDAP sync: allow change of case if we have case insensitive user names and the same name in a different case
+            if (getUserNamesAreCaseSensitive()
+                    || (suggestedUserName = (String) properties.get(ContentModel.PROP_USERNAME)) == null
+                    || !suggestedUserName.equalsIgnoreCase(realUserName))
+            {
+                properties.put(ContentModel.PROP_USERNAME, realUserName);
+            }
         }
         Map<QName, Serializable> update = nodeService.getProperties(personNode);
         update.putAll(properties);

source/java/org/alfresco/repo/security/sync/ChainingUserRegistrySynchronizer.java

@@ -29,6 +29,7 @@ import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -99,7 +100,6 @@ import org.springframework.extensions.surf.util.AbstractLifecycleBean;
 public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean implements UserRegistrySynchronizer,
         ApplicationEventPublisherAware
 {
-
     /** The logger. */
     private static final Log logger = LogFactory.getLog(ChainingUserRegistrySynchronizer.class);
 
@@ -337,7 +337,7 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean impl
         threadFactory.setNamePrefix("ChainingUserRegistrySynchronizer lock refresh");
         threadFactory.setThreadDaemon(true);
         ScheduledExecutorService lockRefresher = new ScheduledThreadPoolExecutor(1, threadFactory);
-		
+
         // Let's ensure all exceptions get logged
         try
         {
@@ -621,11 +621,13 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean impl
                 ChainingUserRegistrySynchronizer.logger, this.loggingInterval);
         class Analyzer implements BatchProcessWorker<NodeDescription>
         {
-            private final Set<String> allZoneAuthorities = new TreeSet<String>();
             private final Map<String, String> groupsToCreate = new TreeMap<String, String>();
-            private final Map<String, Set<String>> groupAssocsToCreate = new TreeMap<String, Set<String>>();
+            private final Map<String, Set<String>> personParentAssocsToCreate = newPersonMap();
-            private final Map<String, Set<String>> groupAssocsToDelete = new TreeMap<String, Set<String>>();
+            private final Map<String, Set<String>> personParentAssocsToDelete = newPersonMap();
-            private final Set<String> authoritiesMaintained = new TreeSet<String>();
+            private final Map<String, Set<String>> groupParentAssocsToCreate = new TreeMap<String, Set<String>>();
+            private final Map<String, Set<String>> groupParentAssocsToDelete = new TreeMap<String, Set<String>>();
+            private List<String> personsProcessed = new LinkedList<String>();
+            private Set<String> allZonePersons;
             private Set<String> deletionCandidates;
 
             private long latestTime;
@@ -677,7 +679,7 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean impl
                 if (groupZones == null)
                 {
                     // The group did not exist at all
-                    addGroup(group);
+                    updateGroup(group, false);
                 }
                 else
                 {
@@ -701,7 +703,7 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean impl
                     if (groupZones.contains(zoneId) || intersection.isEmpty())
                     {
                         // The group already existed in this zone or no valid zone: update the group
-                        updateGroup(group);
+                        updateGroup(group, true);
                     }
                     else
                     {
@@ -723,7 +725,7 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean impl
                         ChainingUserRegistrySynchronizer.this.authorityService.deleteAuthority(groupName);
 
                         // create the group
-                        addGroup(group);
+                        updateGroup(group, false);
                     }
                 }
 
@@ -738,35 +740,7 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean impl
                 }
             }
 
-            private void updateGroup(NodeDescription group)
+            private synchronized void updateGroup(NodeDescription group, boolean existed)
-            {
-                PropertyMap groupProperties = group.getProperties();
-                String groupName = (String) groupProperties.get(ContentModel.PROP_AUTHORITY_NAME);
-                String groupDisplayName = (String) groupProperties.get(ContentModel.PROP_AUTHORITY_DISPLAY_NAME);
-                if (groupDisplayName == null)
-                {
-                    groupDisplayName = ChainingUserRegistrySynchronizer.this.authorityService.getShortName(groupName);
-                }
-                // Update the display name now
-                ChainingUserRegistrySynchronizer.this.authorityService.setAuthorityDisplayName(groupName,
-                        groupDisplayName);
-
-                // Work out the association differences
-                Set<String> oldChildren = ChainingUserRegistrySynchronizer.this.authorityService
-                        .getContainedAuthorities(null, groupName, true);
-                Set<String> newChildren = group.getChildAssociations();
-                Set<String> toDelete = new TreeSet<String>(oldChildren);
-                Set<String> toAdd = new TreeSet<String>(newChildren);
-                toDelete.removeAll(newChildren);
-                toAdd.removeAll(oldChildren);
-                synchronized (this)
-                {
-                    addAssociations(groupName, toAdd);
-                    deleteAssociations(groupName, toDelete);
-                }
-            }
-
-            private void addGroup(NodeDescription group)
             {
                 PropertyMap groupProperties = group.getProperties();
                 String groupName = (String) groupProperties.get(ContentModel.PROP_AUTHORITY_NAME);
@@ -776,95 +750,211 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean impl
                     groupDisplayName = ChainingUserRegistrySynchronizer.this.authorityService.getShortName(groupName);
                 }
 
-                synchronized (this)
+                // Add an entry for the parent itself, in case it is a root group
+                recordParentAssociation(this.groupParentAssocsToCreate, groupName, null);
+
+                // Divide the child associations into person and group associations, dealing with case sensitivity
+                Set<String> newChildPersons = newPersonSet();
+                Set<String> newChildGroups = new TreeSet<String>();
+
+                for (String child : group.getChildAssociations())
+                {
+                    if (AuthorityType.getAuthorityType(child) == AuthorityType.USER)
+                    {
+                        newChildPersons.add(child);
+                    }
+                    else
+                    {
+                        newChildGroups.add(child);
+                    }
+                }
+
+                // Account for differences if already existing
+                if (existed)
+                {
+                    // Update the display name now
+                    ChainingUserRegistrySynchronizer.this.authorityService.setAuthorityDisplayName(groupName,
+                            groupDisplayName);
+
+                    // Work out the association differences
+                    for (String child : ChainingUserRegistrySynchronizer.this.authorityService.getContainedAuthorities(
+                            null, groupName, true))
+                    {
+                        if (AuthorityType.getAuthorityType(child) == AuthorityType.USER)
+                        {
+                            if (!newChildPersons.remove(child))
+                            {
+                                // Make sure each child features as a key in the creation map
+                                recordParentAssociation(this.personParentAssocsToCreate, child, null);
+                                recordParentAssociation(this.personParentAssocsToDelete, child, groupName);
+                            }
+                        }
+                        else
+                        {
+                            if (!newChildGroups.remove(child))
+                            {
+                                // Make sure each child features as a key in the creation map
+                                recordParentAssociation(this.groupParentAssocsToCreate, child, null);
+                                recordParentAssociation(this.groupParentAssocsToDelete, child, groupName);
+                            }
+                        }
+                    }
+                }
+                // Mark as created if new
+                else
                 {
                     this.groupsToCreate.put(groupName, groupDisplayName);
-                    addAssociations(groupName, group.getChildAssociations());
+                }
+
+                // Create new associations
+                for (String child : newChildPersons)
+                {
+                    recordParentAssociation(this.personParentAssocsToCreate, child, groupName);
+                }
+                for (String child : newChildGroups)
+                {
+                    recordParentAssociation(this.groupParentAssocsToCreate, child, groupName);
                 }
             }
 
-            private synchronized void addAssociations(String groupName, Set<String> children)
+            private void recordParentAssociation(Map<String, Set<String>> parentAssocs, String child, String parent)
             {
-                this.allZoneAuthorities.add(groupName);
+                Set<String> parents = parentAssocs.get(child);
-                // Add an entry for the parent itself, in case it is a root group
-                Set<String> parents = this.groupAssocsToCreate.get(groupName);
                 if (parents == null)
                 {
                     parents = new TreeSet<String>();
-                    this.groupAssocsToCreate.put(groupName, parents);
+                    parentAssocs.put(child, parents);
                 }
-                for (String child : children)
+                if (parent != null)
                 {
-                    parents = this.groupAssocsToCreate.get(child);
+                    parents.add(parent);
-                    if (parents == null)
-                    {
-                        parents = new TreeSet<String>();
-                        this.groupAssocsToCreate.put(child, parents);
-                    }
-                    parents.add(groupName);
                 }
             }
 
-            private synchronized void deleteAssociations(String groupName, Set<String> children)
+            private Set<String> newPersonSet()
             {
-                for (String child : children)
+                return ChainingUserRegistrySynchronizer.this.personService.getUserNamesAreCaseSensitive() ? new TreeSet<String>()
-                {
+                        : new TreeSet<String>(String.CASE_INSENSITIVE_ORDER);
-                    // Make sure each child features as a key in the creation map
+            }
-                    addAssociations(child, Collections.<String> emptySet());
 
-                    Set<String> parents = this.groupAssocsToDelete.get(child);
+            private Map<String, Set<String>> newPersonMap()
-                    if (parents == null)
+            {
+                return ChainingUserRegistrySynchronizer.this.personService.getUserNamesAreCaseSensitive() ? new TreeMap<String, Set<String>>()
+                        : new TreeMap<String, Set<String>>(String.CASE_INSENSITIVE_ORDER);
+            }
+
+            private void logRetainParentAssociations(Map<String, Set<String>> parentAssocs, Set<String> toRetain)
+            {
+                Iterator<Map.Entry<String, Set<String>>> i = parentAssocs.entrySet().iterator();
+                StringBuilder groupList = null;
+                while (i.hasNext())
+                {
+                    Map.Entry<String, Set<String>> entry = i.next();
+                    String child = entry.getKey();
+                    if (!toRetain.contains(child))
                     {
-                        parents = new TreeSet<String>();
+                        if (ChainingUserRegistrySynchronizer.logger.isDebugEnabled())
-                        this.groupAssocsToDelete.put(child, parents);
+                        {
+                            if (groupList == null)
+                            {
+                                groupList = new StringBuilder(1024);
+                            }
+                            else
+                            {
+                                groupList.setLength(0);
+                            }
+                            for (String parent : entry.getValue())
+                            {
+                                if (groupList.length() > 0)
+                                {
+                                    groupList.append(", ");
+                                }
+                                groupList.append('\'').append(
+                                        ChainingUserRegistrySynchronizer.this.authorityService.getShortName(parent))
+                                        .append('\'');
+
+                            }
+                            ChainingUserRegistrySynchronizer.logger.debug("Ignoring non-existent member '"
+                                    + ChainingUserRegistrySynchronizer.this.authorityService.getShortName(child)
+                                    + "' in groups {" + groupList.toString() + "}");
+                        }
+                        i.remove();
                     }
-                    parents.add(groupName);
                 }
             }
 
             public void processGroups(UserRegistry userRegistry, boolean allowDeletions, boolean splitTxns)
             {
                 // If we got back some groups, we have to cross reference them with the set of known authorities
-                if (allowDeletions || !this.groupAssocsToCreate.isEmpty())
+                if (allowDeletions || !this.groupParentAssocsToCreate.isEmpty())
                 {
+                    final Set<String> allZonePersons = newPersonSet();
+                    final Set<String> allZoneGroups = new TreeSet<String>();
+
                     // Add in current set of known authorities
-                    this.allZoneAuthorities.addAll(ChainingUserRegistrySynchronizer.this.transactionService
+                    ChainingUserRegistrySynchronizer.this.transactionService.getRetryingTransactionHelper()
-                            .getRetryingTransactionHelper().doInTransaction(
+                            .doInTransaction(new RetryingTransactionCallback<Void>()
-                                    new RetryingTransactionCallback<Set<String>>()
+                            {
-                                    {
+                                public Void execute() throws Throwable
-                                        public Set<String> execute() throws Throwable
+                                {
-                                        {
+                                    allZonePersons.addAll(ChainingUserRegistrySynchronizer.this.authorityService
-                                            return ChainingUserRegistrySynchronizer.this.authorityService
+                                            .getAllAuthoritiesInZone(zoneId, AuthorityType.USER));
-                                                    .getAllAuthoritiesInZone(zoneId, null);
+                                    allZoneGroups.addAll(ChainingUserRegistrySynchronizer.this.authorityService
-                                        }
+                                            .getAllAuthoritiesInZone(zoneId, AuthorityType.GROUP));
-                                    }, true, splitTxns));
+                                    return null;
+                                }
+                            }, true, splitTxns);
+
+                    final Set<String> personDeletionCandidates = newPersonSet();
+                    personDeletionCandidates.addAll(allZonePersons);
+
+                    final Set<String> groupDeletionCandidates = new TreeSet<String>();
+                    groupDeletionCandidates.addAll(allZoneGroups);
+
+                    allZoneGroups.addAll(this.groupsToCreate.keySet());
 
                     // Prune our set of authorities according to deletions
                     if (allowDeletions)
                     {
-                        this.deletionCandidates = new TreeSet<String>(this.allZoneAuthorities);
+                        for (String person : userRegistry.getPersonNames())
-                        userRegistry.processDeletions(this.deletionCandidates);
+                        {
-                        this.allZoneAuthorities.removeAll(this.deletionCandidates);
+                            personDeletionCandidates.remove(person);
-                        this.groupAssocsToCreate.keySet().removeAll(this.deletionCandidates);
+                        }
-                        this.groupAssocsToDelete.keySet().removeAll(this.deletionCandidates);
+
+                        for (String group : userRegistry.getGroupNames())
+                        {
+                            groupDeletionCandidates.remove(group);
+                        }
+
+                        this.deletionCandidates = new TreeSet<String>();
+                        this.deletionCandidates.addAll(personDeletionCandidates);
+                        this.deletionCandidates.addAll(groupDeletionCandidates);
+
+                        allZonePersons.removeAll(personDeletionCandidates);
+                        allZoneGroups.removeAll(groupDeletionCandidates);
                     }
 
-                    if (!this.groupAssocsToCreate.isEmpty())
+                    // Prune the group associations now that we have complete information
+                    logRetainParentAssociations(this.groupParentAssocsToCreate, allZoneGroups);
+                    this.groupParentAssocsToDelete.keySet().retainAll(allZoneGroups);
+
+                    // Pruning person associations will have to wait until we have passed over all persons and built up
+                    // this set
+                    this.allZonePersons = allZonePersons;
+
+                    if (!this.groupParentAssocsToCreate.isEmpty())
                     {
                         // Sort the group associations in depth-first order (root groups first) and filter out
-                        // non-existent parents
+                        // non-existent children
                         Map<String, Set<String>> sortedGroupAssociations = new LinkedHashMap<String, Set<String>>(
-                                this.groupAssocsToCreate.size() * 2);
+                                this.groupParentAssocsToCreate.size() * 2);
                         List<String> authorityPath = new ArrayList<String>(5);
-                        for (String authority : this.groupAssocsToCreate.keySet())
+                        for (String authority : this.groupParentAssocsToCreate.keySet())
                         {
-                            if (this.allZoneAuthorities.contains(authority))
+                            authorityPath.add(authority);
-                            {
+                            visitGroupAssociations(authorityPath, this.groupParentAssocsToCreate,
-                                authorityPath.add(authority);
+                                    sortedGroupAssociations);
-                                visitGroupAssociations(authorityPath, this.allZoneAuthorities,
+                            authorityPath.clear();
-                                        this.groupAssocsToCreate, sortedGroupAssociations);
-                                authorityPath.clear();
-                            }
                         }
 
                         // Add the groups and their parent associations in depth-first order
@@ -928,51 +1018,16 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean impl
             public void finalizeAssociations(UserRegistry userRegistry, boolean splitTxns)
             {
                 // Remove all the associations we have already dealt with
-                this.groupAssocsToCreate.keySet().removeAll(this.authoritiesMaintained);
+                this.personParentAssocsToCreate.keySet().removeAll(this.personsProcessed);
 
                 // Filter out associations to authorities that simply can't exist (and log if debugging is enabled)
-                Iterator<Map.Entry<String, Set<String>>> i = this.groupAssocsToCreate.entrySet().iterator();
+                logRetainParentAssociations(this.personParentAssocsToCreate, this.allZonePersons);
-                StringBuilder groupList = null;
-                while (i.hasNext())
-                {
-                    Map.Entry<String, Set<String>> entry = i.next();
-                    String child = entry.getKey();
-                    if (!this.allZoneAuthorities.contains(child))
-                    {
-                        if (ChainingUserRegistrySynchronizer.logger.isDebugEnabled())
-                        {
-                            if (groupList == null)
-                            {
-                                groupList = new StringBuilder(1024);
-                            }
-                            else
-                            {
-                                groupList.setLength(0);
-                            }
-                            for (String parent : entry.getValue())
-                            {
-                                if (groupList.length() > 0)
-                                {
-                                    groupList.append(", ");
-                                }
-                                groupList.append('\'').append(
-                                        ChainingUserRegistrySynchronizer.this.authorityService.getShortName(parent))
-                                        .append('\'');
 
-                            }
+                if (!this.personParentAssocsToCreate.isEmpty())
-                            ChainingUserRegistrySynchronizer.logger.debug("Ignoring non-existent member '"
-                                    + ChainingUserRegistrySynchronizer.this.authorityService.getShortName(child)
-                                    + "' in groups {" + groupList.toString() + "}");
-                        }
-                        i.remove();
-                    }
-                }
-
-                if (!this.groupAssocsToCreate.isEmpty())
                 {
                     BatchProcessor<Map.Entry<String, Set<String>>> groupCreator = new BatchProcessor<Map.Entry<String, Set<String>>>(
                             zone + " Authority Association", ChainingUserRegistrySynchronizer.this.transactionService
-                                    .getRetryingTransactionHelper(), this.groupAssocsToCreate.entrySet(),
+                                    .getRetryingTransactionHelper(), this.personParentAssocsToCreate.entrySet(),
                             ChainingUserRegistrySynchronizer.this.workerThreads, 20,
                             ChainingUserRegistrySynchronizer.this.applicationEventPublisher,
                             ChainingUserRegistrySynchronizer.logger,
@@ -1010,7 +1065,9 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean impl
 
             private void maintainAssociations(String authorityName) throws BatchUpdateException
             {
-                Set<String> parents = this.groupAssocsToCreate.get(authorityName);
+                boolean isPerson = AuthorityType.getAuthorityType(authorityName) == AuthorityType.USER;
+                Set<String> parents = isPerson ? this.personParentAssocsToCreate.get(authorityName)
+                        : this.groupParentAssocsToCreate.get(authorityName);
                 if (parents != null && !parents.isEmpty())
                 {
                     if (ChainingUserRegistrySynchronizer.logger.isDebugEnabled())
@@ -1037,7 +1094,8 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean impl
                         throw e1;
                     }
                 }
-                Set<String> parentsToDelete = this.groupAssocsToDelete.get(authorityName);
+                Set<String> parentsToDelete = isPerson ? this.personParentAssocsToDelete.get(authorityName)
+                        : this.groupParentAssocsToDelete.get(authorityName);
                 if (parentsToDelete != null && !parentsToDelete.isEmpty())
                 {
                     for (String parent : parentsToDelete)
@@ -1056,10 +1114,13 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean impl
                     }
                 }
 
-                // Remember that this authority's associations have been maintained
+                // Remember that this person's associations have been maintained
-                synchronized (this)
+                if (isPerson)
                 {
-                    this.authoritiesMaintained.add(authorityName);
+                    synchronized (this)
+                    {
+                        this.personsProcessed.add(authorityName);
+                    }
                 }
             }
         }
@@ -1328,15 +1389,13 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean impl
      * @param authorityPath
      *            The authority to visit, preceeded by all its descendants. Allows detection of cyclic child
      *            associations.
-     * @param allAuthorities
-     *            the set of all known authorities
      * @param associationsOld
      *            the association map to sort
      * @param associationsNew
      *            the association map to add to in depth first order
      */
-    private void visitGroupAssociations(List<String> authorityPath, Set<String> allAuthorities,
+    private void visitGroupAssociations(List<String> authorityPath, Map<String, Set<String>> associationsOld,
-            Map<String, Set<String>> associationsOld, Map<String, Set<String>> associationsNew)
+            Map<String, Set<String>> associationsNew)
     {
         String authorityName = authorityPath.get(authorityPath.size() - 1);
         if (!associationsNew.containsKey(authorityName))
@@ -1345,8 +1404,6 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean impl
 
             if (!associations.isEmpty())
             {
-                // Filter out associations to unknown parent authorities
-                associations.retainAll(allAuthorities);
                 int insertIndex = authorityPath.size();
                 Iterator<String> i = associations.iterator();
                 while (i.hasNext())
@@ -1367,7 +1424,7 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean impl
                     else
                     {
                         authorityPath.add(parentAuthority);
-                        visitGroupAssociations(authorityPath, allAuthorities, associationsOld, associationsNew);
+                        visitGroupAssociations(authorityPath, associationsOld, associationsNew);
                         authorityPath.remove(insertIndex);
                     }
                 }
@@ -1397,10 +1454,8 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean impl
                 {
                     public Long execute() throws Throwable
                     {
-                        Long updateTime = (Long) attributeService.getAttribute(
+                        Long updateTime = (Long) ChainingUserRegistrySynchronizer.this.attributeService.getAttribute(
-                                    ChainingUserRegistrySynchronizer.ROOT_ATTRIBUTE_PATH,
+                                ChainingUserRegistrySynchronizer.ROOT_ATTRIBUTE_PATH, label, zoneId);
-                                    label,
-                                    zoneId);
                         return updateTime == null ? -1 : updateTime;
                     }
                 }, true, splitTxns);
@@ -1420,16 +1475,17 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean impl
      *            <code>true</code>, the attribute is persisted in a new transaction for increased performance and
      *            reliability.
      */
-    private void setMostRecentUpdateTime(final String label, final String zoneId, final long lastModifiedMillis, boolean splitTxns)
+    private void setMostRecentUpdateTime(final String label, final String zoneId, final long lastModifiedMillis,
+            boolean splitTxns)
     {
         this.transactionService.getRetryingTransactionHelper().doInTransaction(
                 new RetryingTransactionHelper.RetryingTransactionCallback<Object>()
                 {
                     public Object execute() throws Throwable
                     {
-                        attributeService.setAttribute(
+                        ChainingUserRegistrySynchronizer.this.attributeService.setAttribute(Long
-                                Long.valueOf(lastModifiedMillis),
+                                .valueOf(lastModifiedMillis), ChainingUserRegistrySynchronizer.ROOT_ATTRIBUTE_PATH,
-                                ChainingUserRegistrySynchronizer.ROOT_ATTRIBUTE_PATH, label, zoneId);
+                                label, zoneId);
                         return null;
                     }
                 }, false, splitTxns);

source/java/org/alfresco/repo/security/sync/ChainingUserRegistrySynchronizerTest.java

@@ -256,7 +256,8 @@ public class ChainingUserRegistrySynchronizerTest extends TestCase
             newPerson("U1", "changeofemail@alfresco.com"), newPerson("U6"), newPerson("U7")
         }, new NodeDescription[]
         {
-            newGroup("G1", "U1", "U6", "UDangling"), newGroup("G2", "U1", "GDangling"), newGroupWithDisplayName("G5", "Amazing Group", "U6", "U7", "G4")
+            newGroup("G1", "U1", "U6", "UDangling"), newGroup("G2", "U1", "GDangling"),
+            newGroupWithDisplayName("G5", "Amazing Group", "U6", "U7", "G4")
         });
         this.applicationContextManager.updateZone("Z2", new NodeDescription[]
         {
@@ -362,6 +363,65 @@ public class ChainingUserRegistrySynchronizerTest extends TestCase
         tearDownTestUsersAndGroups();
     }
 
+    /**
+     * Tests a forced update of the test users and groups where some of the users change their case and some groups
+     * appear with different case.
+     */
+    public void testCaseChange() throws Exception
+    {
+        setUpTestUsersAndGroups();
+
+        // Get hold of the original person nodes so we can compare them later
+        NodeRef u1 = this.personService.getPerson("U1", false);
+        NodeRef u2 = this.personService.getPerson("U2", false);
+        NodeRef u6 = this.personService.getPerson("U6", false);
+
+        this.applicationContextManager.setUserRegistries(new MockUserRegistry("Z1", new NodeDescription[]
+        {
+            newPerson("u1"), newPerson("u2"), newPerson("u6"), newPerson("U7")
+        }, new NodeDescription[]
+        {
+            newGroup("g1", "u6"), newGroup("g2", "u1", "G3"), newGroup("G3", "u2", "g4", "g5"), newGroup("g4"),
+            newGroup("g5")
+        }), new MockUserRegistry("Z2", new NodeDescription[]
+        {
+            newPerson("U1"), newPerson("U3"), newPerson("U4"), newPerson("U5")
+        }, new NodeDescription[]
+        {
+            newGroup("G2", "U1", "U3", "U4"), newGroup("G6", "U3", "U4", "G7"), newGroup("G7", "U5")
+        }));
+        this.synchronizer.synchronize(true, true, true);
+        this.retryingTransactionHelper.doInTransaction(new RetryingTransactionCallback<Object>()
+        {
+
+            public Object execute() throws Throwable
+            {
+                assertExists("Z1", "u1");
+                assertExists("Z1", "u2");
+                assertExists("Z1", "u6");
+                assertExists("Z1", "g1", "u6");
+                assertExists("Z1", "g2", "u1", "G3");
+                assertExists("Z1", "G3", "u2", "g4", "g5");
+                assertExists("Z1", "g4");
+                assertExists("Z1", "g5");
+                assertExists("Z2", "U3");
+                assertExists("Z2", "U4");
+                assertExists("Z2", "U5");
+                assertExists("Z2", "G2", "U3", "U4");
+                assertExists("Z2", "G6", "U3", "U4", "G7");
+                assertExists("Z2", "G7", "U5");
+                return null;
+            }
+        }, false, true);
+
+        // Make sure the original people have been preserved
+        assertEquals(u1, this.personService.getPerson("U1", false));
+        assertEquals(u2, this.personService.getPerson("U2", false));
+        assertEquals(u6, this.personService.getPerson("U6", false));
+
+        tearDownTestUsersAndGroups();
+    }
+
     /**
      * Tests synchronization with a zone with a larger volume of authorities.
      * 
@@ -512,6 +572,9 @@ public class ChainingUserRegistrySynchronizerTest extends TestCase
         {
             // Check users exist as persons
             assertTrue(this.personService.personExists(name));
+
+            // Check case matches
+            assertEquals(this.personService.getUserIdentifier(name), name);
         }
     }
 
@@ -573,8 +636,8 @@ public class ChainingUserRegistrySynchronizerTest extends TestCase
      */
     private String longName(String shortName)
     {
-        return this.authorityService.getName(shortName.startsWith("G") ? AuthorityType.GROUP : AuthorityType.USER,
+        return this.authorityService.getName(shortName.toLowerCase().startsWith("g") ? AuthorityType.GROUP
-                shortName);
+                : AuthorityType.USER, shortName);
     }
 
     /**
@@ -621,11 +684,11 @@ public class ChainingUserRegistrySynchronizerTest extends TestCase
         public void updateState(Collection<NodeDescription> persons, Collection<NodeDescription> groups)
         {
             List<NodeDescription> newPersons = new ArrayList<NodeDescription>(this.persons);
-            mergeNodeDescriptions(newPersons, persons, ContentModel.PROP_USERNAME);
+            mergeNodeDescriptions(newPersons, persons, ContentModel.PROP_USERNAME, false);
             this.persons = newPersons;
 
             List<NodeDescription> newGroups = new ArrayList<NodeDescription>(this.groups);
-            mergeNodeDescriptions(newGroups, groups, ContentModel.PROP_AUTHORITY_NAME);
+            mergeNodeDescriptions(newGroups, groups, ContentModel.PROP_AUTHORITY_NAME, true);
             this.groups = newGroups;
         }
 
@@ -639,19 +702,30 @@ public class ChainingUserRegistrySynchronizerTest extends TestCase
          *            the new node list
          * @param idProp
          *            the name of the ID property
+         * @param caseSensitive
+         *            are IDs case sensitive?
          */
         private void mergeNodeDescriptions(List<NodeDescription> oldNodes, Collection<NodeDescription> newNodes,
-                QName idProp)
+                QName idProp, boolean caseSensitive)
         {
             Map<String, NodeDescription> nodeMap = new LinkedHashMap<String, NodeDescription>(newNodes.size() * 2);
             for (NodeDescription node : newNodes)
             {
-                nodeMap.put((String) node.getProperties().get(idProp), node);
+                String id = (String) node.getProperties().get(idProp);
+                if (!caseSensitive)
+                {
+                    id = id.toLowerCase();
+                }
+                nodeMap.put(id, node);
             }
             for (int i = 0; i < oldNodes.size(); i++)
             {
                 NodeDescription oldNode = oldNodes.get(i);
                 String id = (String) oldNode.getProperties().get(idProp);
+                if (!caseSensitive)
+                {
+                    id = id.toLowerCase();
+                }
                 NodeDescription newNode = nodeMap.remove(id);
                 if (newNode == null)
                 {
@@ -694,18 +768,30 @@ public class ChainingUserRegistrySynchronizerTest extends TestCase
 
         /*
          * (non-Javadoc)
-         * @see org.alfresco.repo.security.sync.UserRegistry#processDeletions(java.util.Set)
+         * @see org.alfresco.repo.security.sync.UserRegistry#getGroupNames()
          */
-        public void processDeletions(Set<String> candidateAuthoritiesForDeletion)
+        public Collection<String> getGroupNames()
         {
-            for (NodeDescription person : this.persons)
+            List<String> groupNames = new LinkedList<String>();
-            {
-                candidateAuthoritiesForDeletion.remove(person.getProperties().get(ContentModel.PROP_USERNAME));
-            }
             for (NodeDescription group : this.groups)
             {
-                candidateAuthoritiesForDeletion.remove(group.getProperties().get(ContentModel.PROP_AUTHORITY_NAME));
+                groupNames.add((String) group.getProperties().get(ContentModel.PROP_AUTHORITY_NAME));
             }
+            return groupNames;
+        }
+
+        /*
+         * (non-Javadoc)
+         * @see org.alfresco.repo.security.sync.UserRegistry#getPersonNames()
+         */
+        public Collection<String> getPersonNames()
+        {
+            List<String> personNames = new LinkedList<String>();
+            for (NodeDescription person : this.persons)
+            {
+                personNames.add((String) person.getProperties().get(ContentModel.PROP_USERNAME));
+            }
+            return personNames;
         }
 
         /*
@@ -754,7 +840,8 @@ public class ChainingUserRegistrySynchronizerTest extends TestCase
             return filterNodeDescriptions(this.persons, modifiedSince);
         }
 
-        /* (non-Javadoc)
+        /*
+         * (non-Javadoc)
          * @see org.alfresco.repo.security.sync.UserRegistry#getPersonMappedProperties()
          */
         public Set<QName> getPersonMappedProperties()
@@ -762,7 +849,8 @@ public class ChainingUserRegistrySynchronizerTest extends TestCase
             return new HashSet<QName>(Arrays.asList(new QName[]
             {
                 ContentModel.PROP_USERNAME, ContentModel.PROP_FIRSTNAME, ContentModel.PROP_LASTNAME,
-                ContentModel.PROP_EMAIL, ContentModel.PROP_ORGID, ContentModel.PROP_HOME_FOLDER_PROVIDER
+                ContentModel.PROP_EMAIL, ContentModel.PROP_ORGID, ContentModel.PROP_ORGANIZATION,
+                ContentModel.PROP_HOME_FOLDER_PROVIDER
             }));
         }
     }

source/java/org/alfresco/repo/security/sync/UserRegistry.java

@@ -58,14 +58,20 @@ public interface UserRegistry
     public Collection<NodeDescription> getGroups(Date modifiedSince);
 
     /**
-     * Retrieves the complete set of known users and groups from the user registry and removes them from the set of
+     * Gets the names of all persons in the registry. Used to detect local persons to be deleted. Note that the
-     * candidate local authorities to be deleted.
+     * treatment of these names will depend on Alfresco's username case-sensitivity setting.
      * 
-     * @param candidateAuthoritiesForDeletion
+     * @return the person names
-     *            the candidate authorities for deletion
      */
-    public void processDeletions(final Set<String> candidateAuthoritiesForDeletion);
+    public Collection<String> getPersonNames();
-    
+
+    /**
+     * Gets the names of all groups in the registry. Used to detect local groups to be deleted.
+     * 
+     * @return the person names
+     */
+    public Collection<String> getGroupNames();
+
     /**
      * Gets the set of property names that are auto-mapped by this user registry. These should remain read-only for this
      * registry's users in the UI.

source/java/org/alfresco/repo/security/sync/ldap/LDAPUserRegistry.java

@@ -30,6 +30,8 @@ import java.util.Date;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
@@ -542,12 +544,13 @@ public class LDAPUserRegistry implements UserRegistry, LDAPNameResolver, Initial
         return new PersonCollection(modifiedSince);
     }
 
-    /*
+    
-     * (non-Javadoc)
+    /* (non-Javadoc)
-     * @see org.alfresco.repo.security.sync.UserRegistry#processDeletions(java.util.Set)
+     * @see org.alfresco.repo.security.sync.UserRegistry#getPersonNames()
      */
-    public void processDeletions(final Set<String> candidateAuthoritiesForDeletion)
+    public Collection<String> getPersonNames()
     {
+        final List<String> personNames = new LinkedList<String>();
         processQuery(new SearchCallback()
         {
             public void process(SearchResult result) throws NamingException, ParseException
@@ -568,8 +571,7 @@ public class LDAPUserRegistry implements UserRegistry, LDAPNameResolver, Initial
                 }
                 else
                 {
-                    String authority = (String) nameAttribute.get();
+                    personNames.add((String) nameAttribute.get());
-                    candidateAuthoritiesForDeletion.remove(authority);
                 }
             }
 
@@ -581,6 +583,15 @@ public class LDAPUserRegistry implements UserRegistry, LDAPNameResolver, Initial
         {
             this.userIdAttributeName
         });
+        return personNames;
+    }
+
+    /* (non-Javadoc)
+     * @see org.alfresco.repo.security.sync.UserRegistry#getGroupNames()
+     */
+    public Collection<String> getGroupNames()
+    {
+        final List<String> groupNames = new LinkedList<String>();
         processQuery(new SearchCallback()
         {
 
@@ -603,7 +614,7 @@ public class LDAPUserRegistry implements UserRegistry, LDAPNameResolver, Initial
                 else
                 {
                     String authority = "GROUP_" + (String) nameAttribute.get();
-                    candidateAuthoritiesForDeletion.remove(authority);
+                    groupNames.add(authority);
                 }
             }
 
@@ -615,6 +626,7 @@ public class LDAPUserRegistry implements UserRegistry, LDAPNameResolver, Initial
         {
             this.groupIdAttributeName
         });
+        return groupNames;
     }
 
     /*