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 341df0c010..3fded26cbe 100644 --- a/source/java/org/alfresco/repo/security/sync/ldap/LDAPUserRegistry.java +++ b/source/java/org/alfresco/repo/security/sync/ldap/LDAPUserRegistry.java @@ -1150,7 +1150,9 @@ public class LDAPUserRegistry implements UserRegistry, LDAPNameResolver, Initial String attributeName = attributeMapping.get(key); if (attributeName != null) { - Attribute attribute = ldapAttributes.get(attributeName); + Attribute attribute = ldapAttributes.get(attributeName); + String defaultAttribute = attributeDefaults.get(key); + if (attribute != null) { String value = (String) attribute.get(0); @@ -1159,13 +1161,14 @@ public class LDAPUserRegistry implements UserRegistry, LDAPNameResolver, Initial properties.put(keyQName, value); } } - else + else if (defaultAttribute != null) { - String defaultValue = attributeDefaults.get(key); - if (defaultValue != null) - { - properties.put(keyQName, defaultValue); - } + properties.put(keyQName, defaultAttribute); + } + else + { + // Make sure that a 2nd sync, updates deleted ldap attributes(MNT-14026) + properties.put(keyQName, null); } } else diff --git a/source/test-java/org/alfresco/repo/security/sync/ChainingUserRegistrySynchronizerTest.java b/source/test-java/org/alfresco/repo/security/sync/ChainingUserRegistrySynchronizerTest.java index ca0eedcd71..0201a5b797 100644 --- a/source/test-java/org/alfresco/repo/security/sync/ChainingUserRegistrySynchronizerTest.java +++ b/source/test-java/org/alfresco/repo/security/sync/ChainingUserRegistrySynchronizerTest.java @@ -25,8 +25,11 @@ */ package org.alfresco.repo.security.sync; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.mockito.Mockito.any; import junit.framework.TestCase; - + import org.alfresco.error.AlfrescoRuntimeException; import org.alfresco.model.ContentModel; import org.alfresco.repo.management.subsystems.ActivateableBean; @@ -35,10 +38,12 @@ import org.alfresco.repo.security.authentication.AuthenticationContext; import org.alfresco.repo.security.authentication.AuthenticationException; import org.alfresco.repo.security.authentication.AuthenticationUtil; import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork; +import org.alfresco.repo.security.authentication.ldap.LDAPInitialDirContextFactoryImpl; import org.alfresco.repo.security.person.PersonServiceImpl; import org.alfresco.repo.security.sync.ldap.AbstractDirectoryServiceUserAccountStatusInterpreter; import org.alfresco.repo.security.sync.ldap.LDAPUserAccountStatusInterpreter; import org.alfresco.repo.security.sync.ldap.LDAPUserRegistry; +import org.alfresco.repo.security.sync.ldap.LDAPUserRegistry.PersonCollection; import org.alfresco.repo.security.sync.ldap_ad.LDAPADUserAccountStatusInterpreter; import org.alfresco.repo.transaction.RetryingTransactionHelper; import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback; @@ -52,13 +57,23 @@ import org.alfresco.service.namespace.NamespaceService; import org.alfresco.service.namespace.QName; import org.alfresco.util.GUID; import org.alfresco.util.PropertyMap; +import org.mockito.Mockito; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationEvent; import org.springframework.context.ApplicationListener; import org.springframework.context.support.ClassPathXmlApplicationContext; import org.springframework.context.support.StaticApplicationContext; - + +import java.io.Serializable; import java.util.*; + +import javax.naming.NamingEnumeration; +import javax.naming.NamingException; +import javax.naming.directory.BasicAttribute; +import javax.naming.directory.BasicAttributes; +import javax.naming.directory.InitialDirContext; +import javax.naming.directory.SearchControls; +import javax.naming.directory.SearchResult; /** * Tests the {@link ChainingUserRegistrySynchronizer} using a simulated {@link UserRegistry}. @@ -82,7 +97,10 @@ public class ChainingUserRegistrySynchronizerTest extends TestCase private UserRegistrySynchronizer synchronizer; /** The application context manager. */ - private MockApplicationContextManager applicationContextManager; + private MockApplicationContextManager applicationContextManager; + + /** The namespace service. */ + private NamespaceService namespaceService; /** The person service. */ private PersonService personService; @@ -128,7 +146,10 @@ public class ChainingUserRegistrySynchronizerTest extends TestCase this.retryingTransactionHelper = (RetryingTransactionHelper) ChainingUserRegistrySynchronizerTest.context .getBean("retryingTransactionHelper"); - setHomeFolderCreationEager(false); // the normal default if using LDAP + setHomeFolderCreationEager(false); // the normal default if using LDAP + + this.namespaceService = (NamespaceService) ChainingUserRegistrySynchronizerTest.context + .getBean("namespaceService"); } /* @@ -441,8 +462,9 @@ public class ChainingUserRegistrySynchronizerTest extends TestCase @Override public Collection getPersons(Date modifiedSince) - { - return mockUserRegistry.getPersons(modifiedSince); + { + Collection persons = mockUserRegistry.getPersons(modifiedSince); + return !persons.isEmpty() ? persons : super.getPersons(modifiedSince); } @Override @@ -815,7 +837,125 @@ public class ChainingUserRegistrySynchronizerTest extends TestCase { tearDownTestUsersAndGroups(); } - } + } + + /** + *

Test that upon a first sync, the missing properties at the AD level are set as 'null' on the Alfresco Person object.

+ *

MNT-14026: LDAP sync fails to update attribute's value deletion.

+ */ + public void testSyncInexistentProperty() throws Exception + { + try + { + // Execute an LDAP sync where the AD server returns the attributes of a person without a certain property, in this case the 'mail'. + executeMockedLDAPSyncWithoutActiveDirectoryEmailProp(); + + Map userProperties = this.nodeService.getProperties(this.personService.getPerson("U1")); + assertTrue("User must have the email property even though it's null", userProperties.containsKey(ContentModel.PROP_EMAIL)); + assertTrue("User's email must be null on first sync.", userProperties.get(ContentModel.PROP_EMAIL) == null); + } + finally + { + tearDownTestUsersAndGroups(); + } + } + + /** + *

Test that an attribute is also removed on the Alfresco side, when it's removed at the AD level.

+ *

MNT-14026: LDAP sync fails to update attribute's value deletion.

+ */ + public void testSyncDeletedProperty() throws Exception + { + try + { + // Execute an LDAP sync where the AD server returns the attributes of a person, including the 'mail' property. + executeMockedLDAPSyncWithActiveDirectoryEmailProp(); + + Map userProperties = this.nodeService.getProperties(this.personService.getPerson("U1")); + assertTrue("User's email must be not null.", userProperties.get(ContentModel.PROP_EMAIL).equals("U1@alfresco.com")); + + // Execute an LDAP sync where the AD server returns the attributes + // of a person without the 'mail' property, because it was deleted. + executeMockedLDAPSyncWithoutActiveDirectoryEmailProp(); + + userProperties = this.nodeService.getProperties(this.personService.getPerson("U1")); + assertTrue("User must have the email property even though it's null", userProperties.containsKey(ContentModel.PROP_EMAIL)); + assertTrue("User's email must be null on a 2rd sync, since the email property was removed at the AD level.", userProperties.get(ContentModel.PROP_EMAIL) == null); + } + finally + { + tearDownTestUsersAndGroups(); + } + } + + private void executeMockedLDAPSyncWithActiveDirectoryEmailProp() throws Exception + { + executeMockedLDAPSync(true); + } + + private void executeMockedLDAPSyncWithoutActiveDirectoryEmailProp() throws Exception + { + executeMockedLDAPSync(false); + } + + private void executeMockedLDAPSync(boolean withEmail) throws NamingException, Exception + { + MockUserRegistry mockUserRegistry = new MockUserRegistry("Z0", new NodeDescription[] {}, new NodeDescription[] {}); + + MockLDAPUserRegistry mockLDAPUserRegistry = new MockLDAPUserRegistry(mockUserRegistry); + + LDAPInitialDirContextFactoryImpl mockedLdapInitialDirContextFactory = getMockedLDAPSearchResult(withEmail); + + mockLDAPUserRegistry.setLDAPInitialDirContextFactory(mockedLdapInitialDirContextFactory); + mockLDAPUserRegistry.setEnableProgressEstimation(false); + mockLDAPUserRegistry.setUserIdAttributeName("sAMAccountName"); + Map personAttributeMapping = getMockedLdapAttributeMapping(); + mockLDAPUserRegistry.setPersonAttributeMapping(personAttributeMapping); + mockLDAPUserRegistry.setNamespaceService(this.namespaceService); + + mockLDAPUserRegistry.afterPropertiesSet(); + + this.applicationContextManager.setUserRegistries(mockLDAPUserRegistry); + + ChainingUserRegistrySynchronizer chainingSynchronizer = (ChainingUserRegistrySynchronizer) this.synchronizer; + + chainingSynchronizer.synchronize(false, false); + } + + private LDAPInitialDirContextFactoryImpl getMockedLDAPSearchResult(boolean withEmail) throws NamingException + { + @SuppressWarnings("unchecked") + NamingEnumeration mockedNamingEnumeration = mock(NamingEnumeration.class); + when(mockedNamingEnumeration.hasMore()).thenReturn(true).thenReturn(false); + + BasicAttributes attributes = new BasicAttributes(); + attributes.put(new BasicAttribute("sAMAccountName", "U1")); + attributes.put(new BasicAttribute("givenName", "U1")); + if (withEmail) + { + attributes.put(new BasicAttribute("mail", "U1@alfresco.com")); + } + SearchResult mockedSearchResult = new SearchResult("CN:U1", null, attributes); + mockedSearchResult.setNameInNamespace("CN:U1"); + + when(mockedNamingEnumeration.next()).thenReturn(mockedSearchResult); + + InitialDirContext mockedInitialDirContext = mock(InitialDirContext.class); + when(mockedInitialDirContext.search(any(String.class), any(String.class), any(SearchControls.class))).thenReturn(mockedNamingEnumeration); + + LDAPInitialDirContextFactoryImpl mockedLdapInitialDirContextFactory = mock(LDAPInitialDirContextFactoryImpl.class); + when(mockedLdapInitialDirContextFactory.getDefaultIntialDirContext(0)).thenReturn(mockedInitialDirContext); + return mockedLdapInitialDirContextFactory; + } + + private Map getMockedLdapAttributeMapping() + { + Map personAttributeMapping = new HashMap<>(); + personAttributeMapping.put("cm:userName", "sAMAccountName"); + personAttributeMapping.put("cm:firstName", "givenName"); + personAttributeMapping.put("cm:email", "mail"); + return personAttributeMapping; + } /** * Tests synchronization of group associations in a zone with a larger volume of authorities.