From 3238ca154a5a8136a9a0a141b0d7a4f6f1480b05 Mon Sep 17 00:00:00 2001 From: Dave Ward Date: Wed, 30 Jun 2010 13:03:27 +0000 Subject: [PATCH] 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 --- .../security/authority/AuthorityDAOImpl.java | 14 + .../security/person/PersonServiceImpl.java | 10 +- .../ChainingUserRegistrySynchronizer.java | 354 ++++++++++-------- .../ChainingUserRegistrySynchronizerTest.java | 120 +++++- .../repo/security/sync/UserRegistry.java | 18 +- .../security/sync/ldap/LDAPUserRegistry.java | 26 +- 6 files changed, 363 insertions(+), 179 deletions(-) diff --git a/source/java/org/alfresco/repo/security/authority/AuthorityDAOImpl.java b/source/java/org/alfresco/repo/security/authority/AuthorityDAOImpl.java index a3ced88020..2c075262da 100644 --- a/source/java/org/alfresco/repo/security/authority/AuthorityDAOImpl.java +++ b/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)); } } diff --git a/source/java/org/alfresco/repo/security/person/PersonServiceImpl.java b/source/java/org/alfresco/repo/security/person/PersonServiceImpl.java index 202c36e6ac..4f64e7368d 100644 --- a/source/java/org/alfresco/repo/security/person/PersonServiceImpl.java +++ b/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 update = nodeService.getProperties(personNode); update.putAll(properties); diff --git a/source/java/org/alfresco/repo/security/sync/ChainingUserRegistrySynchronizer.java b/source/java/org/alfresco/repo/security/sync/ChainingUserRegistrySynchronizer.java index 5221fc3a62..0fff6e152b 100644 --- a/source/java/org/alfresco/repo/security/sync/ChainingUserRegistrySynchronizer.java +++ b/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 { - private final Set allZoneAuthorities = new TreeSet(); private final Map groupsToCreate = new TreeMap(); - private final Map> groupAssocsToCreate = new TreeMap>(); - private final Map> groupAssocsToDelete = new TreeMap>(); - private final Set authoritiesMaintained = new TreeSet(); + private final Map> personParentAssocsToCreate = newPersonMap(); + private final Map> personParentAssocsToDelete = newPersonMap(); + private final Map> groupParentAssocsToCreate = new TreeMap>(); + private final Map> groupParentAssocsToDelete = new TreeMap>(); + private List personsProcessed = new LinkedList(); + private Set allZonePersons; private Set 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) - { - 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 oldChildren = ChainingUserRegistrySynchronizer.this.authorityService - .getContainedAuthorities(null, groupName, true); - Set newChildren = group.getChildAssociations(); - Set toDelete = new TreeSet(oldChildren); - Set toAdd = new TreeSet(newChildren); - toDelete.removeAll(newChildren); - toAdd.removeAll(oldChildren); - synchronized (this) - { - addAssociations(groupName, toAdd); - deleteAssociations(groupName, toDelete); - } - } - - private void addGroup(NodeDescription group) + private synchronized void updateGroup(NodeDescription group, boolean existed) { 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 newChildPersons = newPersonSet(); + Set newChildGroups = new TreeSet(); + + 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 children) + private void recordParentAssociation(Map> parentAssocs, String child, String parent) { - this.allZoneAuthorities.add(groupName); - // Add an entry for the parent itself, in case it is a root group - Set parents = this.groupAssocsToCreate.get(groupName); + Set parents = parentAssocs.get(child); if (parents == null) { parents = new TreeSet(); - this.groupAssocsToCreate.put(groupName, parents); + parentAssocs.put(child, parents); } - for (String child : children) + if (parent != null) { - parents = this.groupAssocsToCreate.get(child); - if (parents == null) - { - parents = new TreeSet(); - this.groupAssocsToCreate.put(child, parents); - } - parents.add(groupName); + parents.add(parent); } } - private synchronized void deleteAssociations(String groupName, Set children) + private Set newPersonSet() { - for (String child : children) - { - // Make sure each child features as a key in the creation map - addAssociations(child, Collections. emptySet()); + return ChainingUserRegistrySynchronizer.this.personService.getUserNamesAreCaseSensitive() ? new TreeSet() + : new TreeSet(String.CASE_INSENSITIVE_ORDER); + } - Set parents = this.groupAssocsToDelete.get(child); - if (parents == null) + private Map> newPersonMap() + { + return ChainingUserRegistrySynchronizer.this.personService.getUserNamesAreCaseSensitive() ? new TreeMap>() + : new TreeMap>(String.CASE_INSENSITIVE_ORDER); + } + + private void logRetainParentAssociations(Map> parentAssocs, Set toRetain) + { + Iterator>> i = parentAssocs.entrySet().iterator(); + StringBuilder groupList = null; + while (i.hasNext()) + { + Map.Entry> entry = i.next(); + String child = entry.getKey(); + if (!toRetain.contains(child)) { - parents = new TreeSet(); - this.groupAssocsToDelete.put(child, parents); + 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('\''); + + } + 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 allZonePersons = newPersonSet(); + final Set allZoneGroups = new TreeSet(); + // Add in current set of known authorities - this.allZoneAuthorities.addAll(ChainingUserRegistrySynchronizer.this.transactionService - .getRetryingTransactionHelper().doInTransaction( - new RetryingTransactionCallback>() - { - public Set execute() throws Throwable - { - return ChainingUserRegistrySynchronizer.this.authorityService - .getAllAuthoritiesInZone(zoneId, null); - } - }, true, splitTxns)); + ChainingUserRegistrySynchronizer.this.transactionService.getRetryingTransactionHelper() + .doInTransaction(new RetryingTransactionCallback() + { + public Void execute() throws Throwable + { + allZonePersons.addAll(ChainingUserRegistrySynchronizer.this.authorityService + .getAllAuthoritiesInZone(zoneId, AuthorityType.USER)); + allZoneGroups.addAll(ChainingUserRegistrySynchronizer.this.authorityService + .getAllAuthoritiesInZone(zoneId, AuthorityType.GROUP)); + return null; + } + }, true, splitTxns); + + final Set personDeletionCandidates = newPersonSet(); + personDeletionCandidates.addAll(allZonePersons); + + final Set groupDeletionCandidates = new TreeSet(); + groupDeletionCandidates.addAll(allZoneGroups); + + allZoneGroups.addAll(this.groupsToCreate.keySet()); // Prune our set of authorities according to deletions if (allowDeletions) { - this.deletionCandidates = new TreeSet(this.allZoneAuthorities); - userRegistry.processDeletions(this.deletionCandidates); - this.allZoneAuthorities.removeAll(this.deletionCandidates); - this.groupAssocsToCreate.keySet().removeAll(this.deletionCandidates); - this.groupAssocsToDelete.keySet().removeAll(this.deletionCandidates); + for (String person : userRegistry.getPersonNames()) + { + personDeletionCandidates.remove(person); + } + + for (String group : userRegistry.getGroupNames()) + { + groupDeletionCandidates.remove(group); + } + + this.deletionCandidates = new TreeSet(); + 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> sortedGroupAssociations = new LinkedHashMap>( - this.groupAssocsToCreate.size() * 2); + this.groupParentAssocsToCreate.size() * 2); List authorityPath = new ArrayList(5); - for (String authority : this.groupAssocsToCreate.keySet()) + for (String authority : this.groupParentAssocsToCreate.keySet()) { - if (this.allZoneAuthorities.contains(authority)) - { - authorityPath.add(authority); - visitGroupAssociations(authorityPath, this.allZoneAuthorities, - this.groupAssocsToCreate, sortedGroupAssociations); - authorityPath.clear(); - } + authorityPath.add(authority); + visitGroupAssociations(authorityPath, this.groupParentAssocsToCreate, + 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>> i = this.groupAssocsToCreate.entrySet().iterator(); - StringBuilder groupList = null; - while (i.hasNext()) - { - Map.Entry> 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('\''); + logRetainParentAssociations(this.personParentAssocsToCreate, this.allZonePersons); - } - ChainingUserRegistrySynchronizer.logger.debug("Ignoring non-existent member '" - + ChainingUserRegistrySynchronizer.this.authorityService.getShortName(child) - + "' in groups {" + groupList.toString() + "}"); - } - i.remove(); - } - } - - if (!this.groupAssocsToCreate.isEmpty()) + if (!this.personParentAssocsToCreate.isEmpty()) { BatchProcessor>> groupCreator = new BatchProcessor>>( 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 parents = this.groupAssocsToCreate.get(authorityName); + boolean isPerson = AuthorityType.getAuthorityType(authorityName) == AuthorityType.USER; + Set 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 parentsToDelete = this.groupAssocsToDelete.get(authorityName); + Set 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 - synchronized (this) + // Remember that this person's associations have been maintained + 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 authorityPath, Set allAuthorities, - Map> associationsOld, Map> associationsNew) + private void visitGroupAssociations(List authorityPath, Map> associationsOld, + Map> 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 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( - ChainingUserRegistrySynchronizer.ROOT_ATTRIBUTE_PATH, - label, - zoneId); + Long updateTime = (Long) ChainingUserRegistrySynchronizer.this.attributeService.getAttribute( + ChainingUserRegistrySynchronizer.ROOT_ATTRIBUTE_PATH, label, zoneId); return updateTime == null ? -1 : updateTime; } }, true, splitTxns); @@ -1420,16 +1475,17 @@ public class ChainingUserRegistrySynchronizer extends AbstractLifecycleBean impl * true, 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() { public Object execute() throws Throwable { - attributeService.setAttribute( - Long.valueOf(lastModifiedMillis), - ChainingUserRegistrySynchronizer.ROOT_ATTRIBUTE_PATH, label, zoneId); + ChainingUserRegistrySynchronizer.this.attributeService.setAttribute(Long + .valueOf(lastModifiedMillis), ChainingUserRegistrySynchronizer.ROOT_ATTRIBUTE_PATH, + label, zoneId); return null; } }, false, splitTxns); diff --git a/source/java/org/alfresco/repo/security/sync/ChainingUserRegistrySynchronizerTest.java b/source/java/org/alfresco/repo/security/sync/ChainingUserRegistrySynchronizerTest.java index 2d3db809e9..76c0f9a10d 100644 --- a/source/java/org/alfresco/repo/security/sync/ChainingUserRegistrySynchronizerTest.java +++ b/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() + { + + 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, - shortName); + return this.authorityService.getName(shortName.toLowerCase().startsWith("g") ? AuthorityType.GROUP + : AuthorityType.USER, shortName); } /** @@ -621,11 +684,11 @@ public class ChainingUserRegistrySynchronizerTest extends TestCase public void updateState(Collection persons, Collection groups) { List newPersons = new ArrayList(this.persons); - mergeNodeDescriptions(newPersons, persons, ContentModel.PROP_USERNAME); + mergeNodeDescriptions(newPersons, persons, ContentModel.PROP_USERNAME, false); this.persons = newPersons; List newGroups = new ArrayList(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 oldNodes, Collection newNodes, - QName idProp) + QName idProp, boolean caseSensitive) { Map nodeMap = new LinkedHashMap(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 candidateAuthoritiesForDeletion) + public Collection getGroupNames() { - for (NodeDescription person : this.persons) - { - candidateAuthoritiesForDeletion.remove(person.getProperties().get(ContentModel.PROP_USERNAME)); - } + List groupNames = new LinkedList(); 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 getPersonNames() + { + List personNames = new LinkedList(); + 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 getPersonMappedProperties() @@ -762,7 +849,8 @@ public class ChainingUserRegistrySynchronizerTest extends TestCase return new HashSet(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 })); } } diff --git a/source/java/org/alfresco/repo/security/sync/UserRegistry.java b/source/java/org/alfresco/repo/security/sync/UserRegistry.java index 984dc9b9d9..ace7cbe2cc 100644 --- a/source/java/org/alfresco/repo/security/sync/UserRegistry.java +++ b/source/java/org/alfresco/repo/security/sync/UserRegistry.java @@ -58,14 +58,20 @@ public interface UserRegistry public Collection getGroups(Date modifiedSince); /** - * Retrieves the complete set of known users and groups from the user registry and removes them from the set of - * candidate local authorities to be deleted. + * Gets the names of all persons in the registry. Used to detect local persons to be deleted. Note that the + * treatment of these names will depend on Alfresco's username case-sensitivity setting. * - * @param candidateAuthoritiesForDeletion - * the candidate authorities for deletion + * @return the person names */ - public void processDeletions(final Set candidateAuthoritiesForDeletion); - + public Collection getPersonNames(); + + /** + * Gets the names of all groups in the registry. Used to detect local groups to be deleted. + * + * @return the person names + */ + public Collection 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. diff --git a/source/java/org/alfresco/repo/security/sync/ldap/LDAPUserRegistry.java b/source/java/org/alfresco/repo/security/sync/ldap/LDAPUserRegistry.java index 94786fb0c9..fa00499a52 100644 --- a/source/java/org/alfresco/repo/security/sync/ldap/LDAPUserRegistry.java +++ b/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) - * @see org.alfresco.repo.security.sync.UserRegistry#processDeletions(java.util.Set) + + /* (non-Javadoc) + * @see org.alfresco.repo.security.sync.UserRegistry#getPersonNames() */ - public void processDeletions(final Set candidateAuthoritiesForDeletion) + public Collection getPersonNames() { + final List personNames = new LinkedList(); 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(); - candidateAuthoritiesForDeletion.remove(authority); + personNames.add((String) nameAttribute.get()); } } @@ -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 getGroupNames() + { + final List groupNames = new LinkedList(); 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; } /*