Merge branch 'develop' into fix/MNT-17971_activiti_signalAndBoundaryEvent

.gitbugtraq

@@ -0,0 +1,4 @@
+# For SmartGit
+[bugtraq "jira"]
+ url = https://issues.alfresco.com/jira/browse/%BUGID%
+ logRegex = ([A-Z]+-\\d+)

pom.xml

@@ -43,6 +43,7 @@
         <dependency.alfresco-data-model.version>6.18</dependency.alfresco-data-model.version>
         <dependency.alfresco-jlan.version>6.3</dependency.alfresco-jlan.version>
         <dependency.alfresco-pdf-renderer.version>1.0</dependency.alfresco-pdf-renderer.version>
+        <dependency.alfresco-hb-data-sender.version>1.0.1</dependency.alfresco-hb-data-sender.version>
 
 
         <dependency.spring.version>3.2.17.RELEASE</dependency.spring.version>
@@ -99,7 +100,7 @@
         <dependency>
             <groupId>org.alfresco</groupId>
             <artifactId>alfresco-heartbeat-data-sender</artifactId>
-            <version>1.0.0</version>
+            <version>${dependency.alfresco-hb-data-sender.version}</version>
         </dependency>
         <dependency>
             <groupId>com.sun.mail</groupId>

src/main/java/org/alfresco/opencmis/CMISConnector.java

@@ -3098,12 +3098,13 @@ public class CMISConnector implements ApplicationContextAware, ApplicationListen
                 }
             }
 
+            Boolean isOnWorkingCopy = checkOutCheckInService.isWorkingCopy(nodeRef);
             Updatability updatability = propDef.getPropertyDefinition().getUpdatability();
-            if ((updatability == Updatability.READONLY)
-                    || (updatability == Updatability.WHENCHECKEDOUT && !checkOutCheckInService.isWorkingCopy(nodeRef)))
+            if (!isUpdatable(updatability, isOnWorkingCopy))
             {
-                throw new CmisInvalidArgumentException("Property " + property.getId() + " is read-only!");
+                throw new CmisInvalidArgumentException("Property " + propertyId + " is read-only!");
             }
+
             TypeDefinitionWrapper propType = propDef.getOwningType();
             Serializable value = getValue(property, propDef.getPropertyDefinition().getCardinality() == Cardinality.MULTI);
             Pair<TypeDefinitionWrapper, Serializable> pair = new Pair<TypeDefinitionWrapper, Serializable>(propType, value);
@@ -3129,7 +3130,7 @@ public class CMISConnector implements ApplicationContextAware, ApplicationListen
 
         for (String propertyId : propsMap.keySet())
         {
-            if(propertyId.equals(PropertyIds.SECONDARY_OBJECT_TYPE_IDS))
+            if (propertyId.equals(PropertyIds.SECONDARY_OBJECT_TYPE_IDS))
             {
                 // already handled above
                 continue;
@@ -3213,39 +3214,12 @@ public class CMISConnector implements ApplicationContextAware, ApplicationListen
         for(QName aspectQName : aspectsToRemove)
         {
             nodeService.removeAspect(nodeRef, aspectQName);
-            // aspect is being removed so remove all of its properties from the propsToAdd map
-            TypeDefinitionWrapper w = getOpenCMISDictionaryService().findNodeType(aspectQName);
-            for(PropertyDefinitionWrapper wr : w.getProperties())
-            {
-                String propertyId = wr.getPropertyId();
-                propsToAdd.remove(propertyId);
-            }
         }
 
         // add aspects and properties
         for(QName aspectQName : toAdd)
         {
             nodeService.addAspect(nodeRef, aspectQName, null);
-            
-            // get aspect properties
-            AspectDefinition aspectDef = dictionaryService.getAspect(aspectQName);
-            Map<QName, org.alfresco.service.cmr.dictionary.PropertyDefinition> aspectPropDefs = aspectDef.getProperties();
-            TypeDefinitionWrapper w = getOpenCMISDictionaryService().findNodeType(aspectQName);
-            // for each aspect property...
-            for(QName propQName : aspectPropDefs.keySet())
-            {
-                // find CMIS property id
-                PropertyDefinitionWrapper property = w.getPropertyByQName(propQName);
-                String propertyId = property.getPropertyId();
-                if(!propsToAdd.containsKey(propertyId))
-                {
-                    TypeDefinitionWrapper propType = property.getOwningType();
-                    // CMIS 1.1 secondary types specification requires that all secondary type properties are set
-                    // property not included in propsToAdd, add it with null value
-                    Pair<TypeDefinitionWrapper, Serializable> pair = new Pair<TypeDefinitionWrapper, Serializable>(propType, null);
-                    propsToAdd.put(propertyId, pair);
-                }
-            }
         }
     }
 
@@ -3580,9 +3554,9 @@ public class CMISConnector implements ApplicationContextAware, ApplicationListen
             throw new CmisInvalidArgumentException("Property " + propertyId + " is unknown!");
         }
 
+        Boolean isOnWorkingCopy = checkOutCheckInService.isWorkingCopy(nodeRef);
         Updatability updatability = propDef.getPropertyDefinition().getUpdatability();
-        if ((updatability == Updatability.READONLY)
-                || (updatability == Updatability.WHENCHECKEDOUT && !checkOutCheckInService.isWorkingCopy(nodeRef)))
+        if (!isUpdatable(updatability, isOnWorkingCopy))
         {
             throw new CmisInvalidArgumentException("Property " + propertyId + " is read-only!");
         }
@@ -3610,7 +3584,7 @@ public class CMISConnector implements ApplicationContextAware, ApplicationListen
                 {
                     String newName = value.toString();
                     // If the node is checked out and the name property is set on the working copy, make sure the new name has the working copy format
-                    if (checkOutCheckInService.isWorkingCopy(nodeRef))
+                    if (isOnWorkingCopy)
                     {
                         String wcLabel = (String)this.nodeService.getProperty(nodeRef, ContentModel.PROP_WORKING_COPY_LABEL);
                         if (wcLabel == null)
@@ -4110,4 +4084,23 @@ public class CMISConnector implements ApplicationContextAware, ApplicationListen
         }
         return renditionMapping;
     }
+
+    /**
+     * Verify if a property is updatable.
+     * @param updatability
+     * @param isOnWorkingCopy
+     * @return
+     */
+    private boolean isUpdatable(Updatability updatability, Boolean isOnWorkingCopy)
+    {
+        if ((updatability == Updatability.READONLY)
+                || (updatability == Updatability.WHENCHECKEDOUT && !isOnWorkingCopy))
+        {
+            return false;
+        }
+        else
+        {
+            return true;
+        }
+    }
 }

src/main/java/org/alfresco/repo/security/authentication/AuthenticationServiceImpl.java

@@ -33,6 +33,7 @@ import org.alfresco.repo.management.subsystems.ActivateableBean;
 import org.alfresco.repo.security.authentication.AuthenticationComponent.UserNameValidationMode;
 import org.alfresco.repo.tenant.TenantContextHolder;
 import org.alfresco.service.cmr.security.PersonService;
+import org.alfresco.util.GUID;
 import org.alfresco.util.Pair;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -43,6 +44,9 @@ public class AuthenticationServiceImpl extends AbstractAuthenticationService imp
     AuthenticationComponent authenticationComponent;
     TicketComponent ticketComponent;
 
+    /** a serviceInstanceId identifying this unique instance */
+    private String serviceInstanceId;
+
     private String domain;
     private boolean allowsUserCreation = true;
     private boolean allowsUserDeletion = true;
@@ -85,6 +89,8 @@ public class AuthenticationServiceImpl extends AbstractAuthenticationService imp
     public AuthenticationServiceImpl()
     {
         super();
+
+        this.serviceInstanceId = GUID.generate();
     }
     
     public void setTicketComponent(TicketComponent ticketComponent)
@@ -124,9 +130,10 @@ public class AuthenticationServiceImpl extends AbstractAuthenticationService imp
             TenantContextHolder.setTenantDomain(tenant);
             if (protectionEnabled)
             {
-                if (protectedUsersCache.get(userName) != null)
+                final String protectedUserKey = getProtectedUserKey(userName);
+                if (protectedUsersCache.get(protectedUserKey) != null)
                 {
-                     protectedUsersCache.remove(userName);
+                     protectedUsersCache.remove(protectedUserKey);
                 }
             }
         }
@@ -148,7 +155,8 @@ public class AuthenticationServiceImpl extends AbstractAuthenticationService imp
         boolean isProtected = false;
         if (protectionEnabled)
         {
-            ProtectedUser protectedUser = protectedUsersCache.get(userName);
+            final String protectedUserKey = getProtectedUserKey(userName);
+            ProtectedUser protectedUser = protectedUsersCache.get(protectedUserKey);
             if (protectedUser != null)
             {
                 long currentTimeStamp = System.currentTimeMillis();
@@ -168,7 +176,8 @@ public class AuthenticationServiceImpl extends AbstractAuthenticationService imp
     {
         if (protectionEnabled)
         {
-            ProtectedUser protectedUser = protectedUsersCache.get(userName);
+            final String protectedUserKey = getProtectedUserKey(userName);
+            ProtectedUser protectedUser = protectedUsersCache.get(protectedUserKey);
             if (protectedUser == null)
             {
                 protectedUser = new ProtectedUser(userName);
@@ -186,10 +195,18 @@ public class AuthenticationServiceImpl extends AbstractAuthenticationService imp
                     }
                 }
             }
-            protectedUsersCache.put(userName, protectedUser);
+            protectedUsersCache.put(protectedUserKey, protectedUser);
         }
     }
 
+    /**
+     * Creates a key by combining the service instance ID with the username. This are the type of keys maintained by protectedUsersCache map.
+     */
+    public String getProtectedUserKey(String userName)
+    {
+        return serviceInstanceId + "@@" + userName;
+    }
+
     public String getCurrentUserName() throws AuthenticationException
     {
         return authenticationComponent.getCurrentUserName();

src/main/resources/alfresco/caches.properties

@@ -729,7 +729,7 @@ cache.authorizationCache.readBackupData=false
 cache.protectedUsersCache.maxItems=1000
 cache.protectedUsersCache.timeToLiveSeconds=0
 cache.protectedUsersCache.maxIdleSeconds=0
-cache.protectedUsersCache.cluster.type=fully-distributed
+cache.protectedUsersCache.cluster.type=local
 cache.protectedUsersCache.backup-count=1
 cache.protectedUsersCache.eviction-policy=LRU
 cache.protectedUsersCache.eviction-percentage=25

src/test/java/org/alfresco/opencmis/CMISTest.java

@@ -50,6 +50,7 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.UUID;
 
 import org.alfresco.model.ContentModel;
 import org.alfresco.opencmis.dictionary.CMISDictionaryService;
@@ -1755,9 +1756,13 @@ public class CMISTest
 
         List secondaryTypeIds = currentProperties.getProperties().get(PropertyIds.SECONDARY_OBJECT_TYPE_IDS).getValues();
 
+        assertTrue(secondaryTypeIds.contains(aspectName));
+
         secondaryTypeIds.remove(aspectName);
         final PropertiesImpl newProperties = new PropertiesImpl();
         newProperties.addProperty(new PropertyStringImpl(PropertyIds.SECONDARY_OBJECT_TYPE_IDS, secondaryTypeIds));
+        final String updatedName = "My_new_name_"+UUID.randomUUID().toString();
+        newProperties.replaceProperty(new PropertyStringImpl(PropertyIds.NAME, updatedName));
 
         withCmisService(new CmisServiceCallback<Void>()
         {
@@ -1765,6 +1770,8 @@ public class CMISTest
             public Void execute(CmisService cmisService)
             {
                 Holder<String> latestObjectIdHolder = getHolderOfObjectOfLatestVersion(cmisService, repositoryId, objectIdHolder);
+                // This will result in aspectName being removed
+                // but that shouldn't mean that, for example, a cmis:name prop update gets ignored (MNT-18340)
                 cmisService.updateProperties(repositoryId, latestObjectIdHolder, null, newProperties, null);
                 return null;
             }
@@ -1775,12 +1782,15 @@ public class CMISTest
             @Override
             public Properties execute(CmisService cmisService)
             {
-                Properties properties = cmisService.getProperties(repositoryId, objectIdHolder.getValue(), null, null);
+                Holder<String> latestObjectIdHolder = getHolderOfObjectOfLatestVersion(cmisService, repositoryId, objectIdHolder);
+                Properties properties = cmisService.getProperties(repositoryId, latestObjectIdHolder.getValue(), null, null);
                 return properties;
             }
         }, CmisVersion.CMIS_1_1);
         secondaryTypeIds = currentProperties1.getProperties().get(PropertyIds.SECONDARY_OBJECT_TYPE_IDS).getValues();
 
+        assertFalse(secondaryTypeIds.contains(aspectName));
+        assertEquals(updatedName, currentProperties1.getProperties().get(PropertyIds.NAME).getFirstValue());
     }
 
     /**
@@ -3352,7 +3362,19 @@ public class CMISTest
                         
                         cmisService.updateProperties(repositoryId, new Holder<String>(fileInfo.getNodeRef().toString()), null, properties, null);
                     }
+                    //This extra check was added due to MNT-16641.
+                    {
+                        PropertiesImpl properties = new PropertiesImpl();
+                        properties.addProperty(new PropertyStringImpl(PropertyIds.SECONDARY_OBJECT_TYPE_IDS, "P:cm:lockable"));
 
+                        Set<QName> existingAspects = nodeService.getAspects(docs.get(0).getNodeRef());
+                        cmisService.updateProperties(repositoryId,new Holder<String>(docs.get(0).getNodeRef().toString()), null, properties, null);
+                        Set<QName> updatedAspects = nodeService.getAspects(docs.get(0).getNodeRef());
+                        updatedAspects.removeAll(existingAspects);
+
+                        assertEquals(ContentModel.ASPECT_LOCKABLE, updatedAspects.iterator().next());
+
+                    }
                     return repositoryId;
                 }
             }, CmisVersion.CMIS_1_1);

src/test/java/org/alfresco/repo/security/authentication/AuthenticationServiceImplTest.java

@@ -34,6 +34,7 @@ import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
 
+import static junit.framework.TestCase.assertNotNull;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNull;
@@ -57,6 +58,7 @@ public class AuthenticationServiceImplTest
     private SimpleCache<String, ProtectedUser> cache;
     private TicketComponent ticketComponent = mock(TicketComponent.class);
     private AuthenticationServiceImpl authService;
+    private AuthenticationServiceImpl authService2;
 
     private static final String USERNAME = "username";
     private static final char[] PASSWORD = "password".toCharArray();
@@ -69,6 +71,11 @@ public class AuthenticationServiceImplTest
         authService.setTicketComponent(ticketComponent);
         cache = new MockCache<>();
         authService.setProtectedUsersCache(cache);
+
+        authService2 = new AuthenticationServiceImpl();
+        authService2.setAuthenticationComponent(authenticationComponent);
+        authService2.setTicketComponent(ticketComponent);
+        authService2.setProtectedUsersCache(cache);
     }
 
     @Test
@@ -104,7 +111,9 @@ public class AuthenticationServiceImplTest
         }
         verify(authenticationComponent, times(limit)).authenticate(USERNAME, PASSWORD);
         assertTrue("The user should be protected.", authService.isUserProtected(USERNAME));
-        assertEquals("The number of recorded logins did not match.", attempts, cache.get(USERNAME).getNumLogins());
+
+        final String protectedUserKey = authService.getProtectedUserKey(USERNAME);
+        assertEquals("The number of recorded logins did not match.", attempts, cache.get(protectedUserKey).getNumLogins());
 
         // test that the protection is still in place even if the password is correct
         doNothing().when(authenticationComponent).authenticate(USERNAME, PASSWORD);
@@ -118,7 +127,7 @@ public class AuthenticationServiceImplTest
             // normal
         }
         verify(authenticationComponent, times(limit)).authenticate(USERNAME, PASSWORD);
-        assertEquals("The number of recorded logins did not match.", attempts + 1, cache.get(USERNAME).getNumLogins());
+        assertEquals("The number of recorded logins did not match.", attempts + 1, cache.get(protectedUserKey).getNumLogins());
     }
 
     @Test
@@ -145,11 +154,13 @@ public class AuthenticationServiceImplTest
             }
         }
         assertTrue("The user should be protected.", authService.isUserProtected(USERNAME));
-        assertEquals("The number of recorded logins did not match.", attempts, cache.get(USERNAME).getNumLogins());
+
+        final String protectedUserKey = authService.getProtectedUserKey(USERNAME);
+        assertEquals("The number of recorded logins did not match.", attempts, cache.get(protectedUserKey).getNumLogins());
         Thread.sleep(timeLimit*1000 + 1);
         assertFalse("The user should not be protected any more.", authService.isUserProtected(USERNAME));
         assertEquals("The number of recorded logins should stay the same after protection period ends.",
-                attempts, cache.get(USERNAME).getNumLogins());
+                attempts, cache.get(protectedUserKey).getNumLogins());
 
         doNothing().when(authenticationComponent).authenticate(USERNAME, PASSWORD);
         try
@@ -161,9 +172,78 @@ public class AuthenticationServiceImplTest
             fail("An " + AuthenticationException.class.getName() + " should not be thrown.");
         }
         assertNull("The user should be removed from the cache after successful login.",
-                cache.get(USERNAME));
+                cache.get(protectedUserKey));
     }
 
+    @Test
+    public void testAuthChainWorksIfFirstAuthFails() throws Exception
+    {
+        int timeLimit = 1;
+        int attempts = 2;
+        authService.setProtectionPeriodSeconds(timeLimit);
+        authService.setProtectionLimit(attempts);
+        authService.setProtectionEnabled(true);
+
+        authService2.setProtectionPeriodSeconds(timeLimit);
+        authService2.setProtectionLimit(attempts);
+        authService2.setProtectionEnabled(true);
+
+        AuthenticationServiceImpl[] authenticationChain = {authService, authService2};
+
+        doThrow(new AuthenticationException("Bad password"))
+                .when(authenticationComponent).authenticate(USERNAME, PASSWORD);
+
+        // Authentication fails on first run.
+        for (int i = 0; i < attempts; i++) {
+            for (AuthenticationServiceImpl authentication : authenticationChain) {
+                try {
+                    authentication.authenticate(USERNAME, PASSWORD);
+                    fail("An " + AuthenticationException.class.getName() + " should be thrown.");
+                } catch (AuthenticationException ae) {
+                    // normal
+                }
+            }
+        }
+
+        for (AuthenticationServiceImpl authentication : authenticationChain)
+        {
+            assertTrue("The user should be protected.", authentication.isUserProtected(USERNAME));
+        }
+
+        Thread.sleep(timeLimit*1000 + 1);
+
+        for (AuthenticationServiceImpl authentication : authenticationChain)
+        {
+            assertFalse("The user should not be protected any more.", authentication.isUserProtected(USERNAME));
+        }
+
+        // Authentication always fails on first authentication service in the chain.
+        try
+        {
+            authenticationChain[0].authenticate(USERNAME, PASSWORD);
+            fail("An " + AuthenticationException.class.getName() + " should be thrown.");
+        } catch (AuthenticationException ae) {
+            // normal
+        }
+
+        // Authentication should succeed on second authentication service in the chain.
+        doNothing().when(authenticationComponent).authenticate(USERNAME, PASSWORD);
+        try
+        {
+            authenticationChain[1].authenticate(USERNAME, PASSWORD);
+        }
+        catch (AuthenticationException ae)
+        {
+            fail("An " + AuthenticationException.class.getName() + " should not be thrown.");
+        }
+
+        assertNotNull("The user should not be removed from the cache for the corresponding authorization service after a failed login.",
+                cache.get(authenticationChain[0].getProtectedUserKey(USERNAME)));
+        assertNull("The user should be removed from the cache for the corresponding authorization service after successful login.",
+                cache.get(authenticationChain[1].getProtectedUserKey(USERNAME)));
+    }
+
+
     @Test
     public void testProtectionDisabledBadPassword()
     {