Merge branch 'develop' into feature/REPO-2626_HB2.0

src/main/java/org/alfresco/repo/action/executer/MailActionExecuter.java

@@ -614,11 +614,13 @@ public class MailActionExecuter extends ActionExecuterAbstractBase
                 }
                 
                 // set recipient
-                String to = (String)ruleAction.getParameterValue(PARAM_TO);
+                String to = (String)ruleAction.getParameterValue(PARAM_TO);
+                String toRecipients = null;
                 if (to != null && to.length() != 0)
                 {
                     messageRef[0].setTo(to);
-                    
+                    toRecipients = to;
+
                     // Note: there is no validation on the username to check that it actually is an email address.
                     // TODO Fix this.
 
@@ -777,7 +779,8 @@ public class MailActionExecuter extends ActionExecuterAbstractBase
                         
                         if(recipients.size() > 0)
                         {
-                            messageRef[0].setTo(recipients.toArray(new String[recipients.size()]));
+                            messageRef[0].setTo(recipients.toArray(new String[recipients.size()]));
+                            toRecipients = String.join(",", recipients);
                         }
                         else
                         {
@@ -925,7 +928,7 @@ public class MailActionExecuter extends ActionExecuterAbstractBase
                     }
                     
                     // build the email template model
-                    Map<String, Object> model = createEmailTemplateModel(actionedUponNodeRef, suppliedModel, fromPerson);
+                    Map<String, Object> model = createEmailTemplateModel(actionedUponNodeRef, suppliedModel, fromPerson, toRecipients);
 
                     // Determine the locale to use to send the email.
                     Locale locale = recipient.getSecond();
@@ -1044,8 +1047,8 @@ public class MailActionExecuter extends ActionExecuterAbstractBase
         {
             // Send the message unless we are in "testMode"
             if (!testMode)
-            {
+            {	
-                mailService.send(preparedMessage.getMimeMessage());
+            	mailService.send(preparedMessage.getMimeMessage());
                 onSend();
             }
             else
@@ -1054,7 +1057,7 @@ public class MailActionExecuter extends ActionExecuterAbstractBase
                 testSentCount++;
             }
         }
-        catch (MailException e)
+        catch (NullPointerException | MailException e)
         {
             onFail();
             String to = (String)ruleAction.getParameterValue(PARAM_TO);
@@ -1500,15 +1503,20 @@ public class MailActionExecuter extends ActionExecuterAbstractBase
     * 
     * @return Model map for email templates
     */
-   private Map<String, Object> createEmailTemplateModel(NodeRef ref, Map<String, Object> suppliedModel, NodeRef fromPerson)
+   private Map<String, Object> createEmailTemplateModel(NodeRef ref, Map<String, Object> suppliedModel, NodeRef fromPerson, String toRecipents)
    {
       Map<String, Object> model = new HashMap<String, Object>(8, 1.0f);
       
       if (fromPerson != null)
       {
           model.put("person", new TemplateNode(fromPerson, serviceRegistry, null));
-      }      
+      }
-      
+
+      if (toRecipents != null)
+      {
+          model.put("to", toRecipents);
+      }
+
       if (ref != null)
       {
           model.put("document", new TemplateNode(ref, serviceRegistry, null));

src/main/java/org/alfresco/repo/site/SitesPermissionCleaner.java

@@ -1,28 +1,28 @@
-/*
+/*
- * #%L
+ * #%L
- * Alfresco Repository
+ * Alfresco Repository
- * %%
+ * %%
- * Copyright (C) 2005 - 2016 Alfresco Software Limited
+ * Copyright (C) 2005 - 2016 Alfresco Software Limited
- * %%
+ * %%
- * This file is part of the Alfresco software. 
+ * This file is part of the Alfresco software. 
- * If the software was purchased under a paid Alfresco license, the terms of 
+ * If the software was purchased under a paid Alfresco license, the terms of 
- * the paid license agreement will prevail.  Otherwise, the software is 
+ * the paid license agreement will prevail.  Otherwise, the software is 
- * provided under the following open source license terms:
+ * provided under the following open source license terms:
- * 
+ * 
- * Alfresco is free software: you can redistribute it and/or modify
+ * Alfresco is free software: you can redistribute it and/or modify
- * it under the terms of the GNU Lesser General Public License as published by
+ * it under the terms of the GNU Lesser General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
+ * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
+ * (at your option) any later version.
- * 
+ * 
- * Alfresco is distributed in the hope that it will be useful,
+ * Alfresco is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU Lesser General Public License for more details.
+ * GNU Lesser General Public License for more details.
- * 
+ * 
- * You should have received a copy of the GNU Lesser General Public License
+ * You should have received a copy of the GNU Lesser General Public License
- * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
+ * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
- * #L%
+ * #L%
- */
+ */
 package org.alfresco.repo.site;
 
 import java.util.List;
@@ -31,6 +31,7 @@ import org.alfresco.repo.domain.node.NodeDAO;
 import org.alfresco.repo.domain.node.NodeIdAndAclId;
 import org.alfresco.repo.domain.permissions.Acl;
 import org.alfresco.repo.domain.permissions.AclDAO;
+import org.alfresco.repo.security.authentication.AuthenticationUtil;
 import org.alfresco.repo.security.permissions.ACLType;
 import org.alfresco.repo.security.permissions.AccessControlEntry;
 import org.alfresco.repo.security.permissions.AccessControlList;
@@ -99,62 +100,70 @@ public class SitesPermissionCleaner
     
     public void cleanSitePermissions(final NodeRef targetNode, SiteInfo containingSite)
     {
-        if (nodeDAO.exists(targetNode))
+        if (!nodeDAO.exists(targetNode))
         {
-            // We can calculate the containing site at the start of a recursive call & then reuse it on subsequent calls.
+            return;
-            if (containingSite == null)
+        }
-            {
+        // We can calculate the containing site at the start of a recursive call & then reuse it on subsequent calls.
-                containingSite = siteServiceImpl.getSite(targetNode);
+        if (containingSite == null)
-            }
+        {
+            containingSite = siteServiceImpl.getSite(targetNode);
+        }
 
-            // Short-circuit at this point if the node is not in a Site.
+        // Short-circuit at this point if the node is not in a Site.
-            if (containingSite != null)
+        if (containingSite == null)
+        {
+            return;
+        }
+        // For performance reasons we navigate down the containment hierarchy using the DAOs
+        // rather than the NodeService. Note: direct use of NodeDAO requires tenantService (ALF-12732).
+        final Long targetNodeID = nodeDAO.getNodePair(tenantService.getName(targetNode)).getFirst();
+        final Long targetNodeAclID = nodeDAO.getNodeAclId(targetNodeID);
+        Acl targetNodeAcl = aclDAO.getAcl(targetNodeAclID);
+
+        // Nodes that don't have defining ACLs do not need to be considered.
+        if (targetNodeAcl.getAclType() == ACLType.DEFINING)
+        {
+            AccessControlList targetNodeAccessControlList = aclDAO.getAccessControlList(targetNodeAclID);
+            List<AccessControlEntry> targetNodeAclEntries = targetNodeAccessControlList.getEntries();
+            for (AccessControlEntry entry : targetNodeAclEntries)
             {
-                // For performance reasons we navigate down the containment hierarchy using the DAOs
+                String authority = entry.getAuthority();
-                // rather than the NodeService. Note: direct use of NodeDAO requires tenantService (ALF-12732).
+
-                final Long targetNodeID = nodeDAO.getNodePair(tenantService.getName(targetNode)).getFirst();
+                String thisSiteGroupPrefix = siteServiceImpl.getSiteGroup(containingSite.getShortName(), true);
-                final Long targetNodeAclID = nodeDAO.getNodeAclId(targetNodeID);
+
-                Acl targetNodeAcl = aclDAO.getAcl(targetNodeAclID);
+                // If it's a group site permission for a site other than the current site
-                
+                if (authority.startsWith(PermissionService.GROUP_PREFIX) &&
-                // Nodes that don't have defining ACLs do not need to be considered.
+                        // And it's not GROUP_EVERYONE
-                if (targetNodeAcl.getAclType() == ACLType.DEFINING)
+                        !authority.startsWith(PermissionService.ALL_AUTHORITIES) && !authority.startsWith(thisSiteGroupPrefix) &&
+                        //  And if the current user has permissions to do it
+                        publicServiceAccessService.hasAccess("PermissionService", "clearPermission", targetNode, authority) == AccessStatus.ALLOWED)
                 {
-                    AccessControlList targetNodeAccessControlList = aclDAO.getAccessControlList(targetNodeAclID);
+                    // Then remove it.
-                    List<AccessControlEntry> targetNodeAclEntries = targetNodeAccessControlList.getEntries();
+                    permissionService.clearPermission(targetNode, authority);
-                    for (AccessControlEntry entry : targetNodeAclEntries)
-                    {
-                        String authority = entry.getAuthority();
-                        
-                        String thisSiteGroupPrefix = siteServiceImpl.getSiteGroup(containingSite.getShortName(), true);
-                        
-                        // If it's a group site permission for a site other than the current site
-                        if (authority.startsWith(PermissionService.GROUP_PREFIX) &&
-                                !authority.startsWith(PermissionService.ALL_AUTHORITIES) && // And it's not GROUP_EVERYONE
-                                !authority.startsWith(thisSiteGroupPrefix))
-                        {
-                            // And if the current user has permissions to do it
-                            if (publicServiceAccessService.hasAccess("PermissionService", "clearPermission", targetNode, authority) == AccessStatus.ALLOWED)
-                            {
-                                // Then remove it.
-                                permissionService.clearPermission(targetNode, authority);
-                            }
-                            if (publicServiceAccessService.hasAccess("PermissionService", "setInheritParentPermissions", targetNode, true) == AccessStatus.ALLOWED)
-                            {
-                                // And reenable permission inheritance.
-                                permissionService.setInheritParentPermissions(targetNode, true);
-                            }
-                        }
-                        
-                    }
                 }
-                
+
-                // Recurse
+                if (!permissionService.getInheritParentPermissions(targetNode))
-                List<NodeIdAndAclId> childNodeIds = nodeDAO.getPrimaryChildrenAcls(targetNodeID);
-                for (NodeIdAndAclId nextChild : childNodeIds)
                 {
-                    cleanSitePermissions(nodeDAO.getNodePair(nextChild.getId()).getSecond(), containingSite);
+                    // The site manager from the new site, where this node was moved to, has to have permission to this node
+                    String siteManagerAuthority = thisSiteGroupPrefix + "_" + SiteModel.SITE_MANAGER;
+                    AuthenticationUtil.runAs(new AuthenticationUtil.RunAsWork<Void>()
+                    {
+                        public Void doWork() throws Exception
+                        {
+                            permissionService.setPermission(targetNode, siteManagerAuthority, SiteModel.SITE_MANAGER, true);
+                            return null;
+                        }
+                    }, AuthenticationUtil.getSystemUserName());
                 }
             }
         }
+
+        // Recurse
+        List<NodeIdAndAclId> childNodeIds = nodeDAO.getPrimaryChildrenAcls(targetNodeID);
+        for (NodeIdAndAclId nextChild : childNodeIds)
+        {
+            cleanSitePermissions(nodeDAO.getNodePair(nextChild.getId()).getSecond(), containingSite);
+        }
     }
 }

src/test/java/org/alfresco/repo/action/executer/AbstractMailActionExecuterTest.java

@@ -772,6 +772,60 @@ public abstract class AbstractMailActionExecuterTest
         }
     }
 
-
+    /**
-
+     * Test for MNT-17970
+     * @throws IOException
+     * @throws MessagingException
+     */
+    @Test
+    public void testGetToUsersWhenSendingToGroup() throws IOException, MessagingException
+    {
+        String groupName = null;
+        final String USER1 = "test_user1";
+        final String USER2 = "test_user2";
+        try
+        {
+            // Create users and add them to a group
+            createUser(USER1, null);
+            createUser(USER2, null);
+            groupName = AUTHORITY_SERVICE.createAuthority(AuthorityType.GROUP, "testgroup1");
+            AUTHORITY_SERVICE.addAuthority(groupName, USER1);
+            AUTHORITY_SERVICE.addAuthority(groupName, USER2);
+
+            // Create mail
+            final Action mailAction = ACTION_SERVICE.createAction(MailActionExecuter.NAME);
+            mailAction.setParameterValue(MailActionExecuter.PARAM_FROM, "some.body@example.com");
+            mailAction.setParameterValue(MailActionExecuter.PARAM_TO_MANY, groupName);
+            mailAction.setParameterValue(MailActionExecuter.PARAM_SUBJECT, "Testing");
+            mailAction.setParameterValue(MailActionExecuter.PARAM_TEXT, "Testing");
+            mailAction.setParameterValue(MailActionExecuter.PARAM_TEMPLATE, "alfresco/templates/mail/testSentTo.txt.ftl");
+
+            RetryingTransactionHelper txHelper = APP_CONTEXT_INIT.getApplicationContext().getBean("retryingTransactionHelper", RetryingTransactionHelper.class);
+
+            // Send mail
+            MimeMessage message = txHelper.doInTransaction(new RetryingTransactionCallback<MimeMessage>()
+            {
+                @Override
+                public MimeMessage execute() throws Throwable
+                {
+                    ACTION_EXECUTER.executeImpl(mailAction, null);
+                    return ACTION_EXECUTER.retrieveLastTestMessage();
+                }
+            }, true);
+
+            // Check that both users are displayed in message body
+            String recipients = USER1 + "@email.com" + "," + USER2 + "@email.com";
+            Assert.assertNotNull(message);
+            Assert.assertEquals("This email was sent to " + recipients, (String) message.getContent());
+        }
+        finally
+        {
+            if (groupName != null)
+            {
+                AUTHORITY_SERVICE.deleteAuthority(groupName, true);
+            }
+            PERSON_SERVICE.deletePerson(USER1);
+            PERSON_SERVICE.deletePerson(USER2);
+        }
+    }
 }

src/test/java/org/alfresco/repo/site/SiteServiceImplTest.java

@@ -64,6 +64,7 @@ import org.alfresco.service.cmr.dictionary.DictionaryService;
 import org.alfresco.service.cmr.dictionary.TypeDefinition;
 import org.alfresco.service.cmr.model.FileFolderService;
 import org.alfresco.service.cmr.model.FileInfo;
+import org.alfresco.service.cmr.repository.ChildAssociationRef;
 import org.alfresco.service.cmr.repository.ContentService;
 import org.alfresco.service.cmr.repository.ContentWriter;
 import org.alfresco.service.cmr.repository.CopyService;
@@ -91,7 +92,6 @@ import org.alfresco.test_category.BaseSpringTestsCategory;
 import org.alfresco.util.ApplicationContextHelper;
 import org.alfresco.util.BaseAlfrescoSpringTest;
 import org.alfresco.util.GUID;
-import org.alfresco.util.PropertyMap;
 import org.junit.experimental.categories.Category;
 import org.springframework.extensions.surf.util.I18NUtil;
 
@@ -1167,7 +1167,115 @@ public class SiteServiceImplTest extends BaseAlfrescoSpringTest
             // Intentionally empty
         }
     }
-    
+
+    /**
+     * This is an integration test for MNT-18014
+     */
+    public void testMoveFolderStructureWithNonInheritedPermission()
+    {
+        //Login to share as the admin user
+        AuthenticationUtil.setAdminUserAsFullyAuthenticatedUser();
+
+        // Create 2 sites test1, test2 as admin
+        String test1SiteShortName = "test1" + GUID.generate();
+        String test2SiteShortName = "test2" + GUID.generate();
+        createSite(test1SiteShortName, SiteService.DOCUMENT_LIBRARY, SiteVisibility.PUBLIC);
+        createSite(test2SiteShortName, SiteService.DOCUMENT_LIBRARY, SiteVisibility.PUBLIC);
+
+        SiteInfo test1SiteInfo = this.siteService.getSite(test1SiteShortName);
+        assertNotNull(test1SiteInfo);
+        SiteInfo test2SiteInfo = this.siteService.getSite(test2SiteShortName);
+        assertNotNull(test2SiteInfo);
+
+        // add user1 (USER_ONE) and user2 (USER_TWO) as managers on test1 site (test1SiteInfo)
+        siteService.setMembership(test1SiteShortName, USER_ONE, SiteModel.SITE_MANAGER);
+        siteService.setMembership(test1SiteShortName, USER_TWO, SiteModel.SITE_MANAGER);
+
+        // Give manager role to user1 to test2
+        siteService.setMembership(test2SiteShortName, USER_ONE, SiteModel.SITE_MANAGER);
+
+        // Log in as user2
+        AuthenticationUtil.setFullyAuthenticatedUser(USER_TWO);
+
+        // In document library of test1, create fol1 containing fol2 containing fol3
+        NodeRef documentLibraryTest1Site = siteService.getContainer(test1SiteShortName, SiteService.DOCUMENT_LIBRARY);
+        assertNotNull(documentLibraryTest1Site);
+        NodeRef fol1 = this.fileFolderService.create(documentLibraryTest1Site, "fol1-" + GUID.generate(), ContentModel.TYPE_FOLDER).getNodeRef();
+        NodeRef fol2 = this.fileFolderService.create(fol1, "fol2-" + GUID.generate(), ContentModel.TYPE_FOLDER).getNodeRef();
+        NodeRef fol3 = this.fileFolderService.create(fol2, "fol3-" + GUID.generate(), ContentModel.TYPE_FOLDER).getNodeRef();
+
+        // Cut inheritance on fol2
+        permissionService.setInheritParentPermissions(fol2, false);
+
+        // this is what happens when called from Share : permissions.post:
+        // var siteManagerAuthority = "GROUP_site_" + location.site + "_SiteManager";
+        // // Insure Site Managers can still manage content.
+        // node.setPermission("SiteManager", siteManagerAuthority);
+        String test1SiteGroupPrefix = siteServiceImpl.getSiteGroup(test1SiteShortName, true);
+        String test1SiteManagerAuthority = test1SiteGroupPrefix + "_" + SiteModel.SITE_MANAGER;
+        permissionService.setPermission(fol2, test1SiteManagerAuthority, SiteModel.SITE_MANAGER, true);
+
+        // Log in as user1, go to site test1
+        AuthenticationUtil.setFullyAuthenticatedUser(USER_ONE);
+
+        // Check that user1 can see fol1 fol2 fol3
+        List<ChildAssociationRef> childAssocs = nodeService.getChildAssocs(documentLibraryTest1Site);
+        assertEquals("Size should be 1", 1, childAssocs.size());
+        assertTrue("Folder name should start with fol1", getFirstName(childAssocs).startsWith("fol1"));
+        childAssocs = nodeService.getChildAssocs(childAssocs.get(0).getChildRef());
+        assertEquals("Size should be 1", 1, childAssocs.size());
+        assertTrue("Folder name should start with fol2", getFirstName(childAssocs).startsWith("fol2"));
+        childAssocs = nodeService.getChildAssocs(childAssocs.get(0).getChildRef());
+        assertEquals("Size should be 1", 1, childAssocs.size());
+        assertTrue("Folder name should start with fol3", getFirstName(childAssocs).startsWith("fol3"));
+
+        NodeRef documentLibraryTest2Site = siteService.getContainer(test2SiteShortName, SiteService.DOCUMENT_LIBRARY);
+        assertNotNull(documentLibraryTest2Site);
+        childAssocs = nodeService.getChildAssocs(documentLibraryTest2Site);
+        assertTrue("Folder should be empty.", childAssocs.isEmpty());
+
+        // Move fol1 to site test2
+        ChildAssociationRef childAssociationRef = nodeService.moveNode(fol1, documentLibraryTest2Site, ContentModel.ASSOC_CONTAINS,
+                QName.createQName(NamespaceService.CONTENT_MODEL_1_0_URI, GUID.generate()));
+
+        // This is what Share does:
+        // move the node
+        //result.success = fileNode.move(parent, destNode);
+        //
+        //if (result.success)
+        //{
+        //    // If this was an inter-site move, we'll need to clean up the permissions on the node
+        //    if ((fromSite) && (String(fromSite) !== String(fileNode.siteShortName)))
+        //    {
+        //        siteService.cleanSitePermissions(fileNode);
+        //    }
+        //}
+        siteService.cleanSitePermissions(fol1, test2SiteInfo);
+
+        childAssocs = nodeService.getChildAssocs(documentLibraryTest1Site);
+        assertTrue("test1Site document library should be empty.", childAssocs.isEmpty());
+
+        assertFalse("After the move the folder should keep the inherit permission value(false).",
+                permissionService.getInheritParentPermissions(fol2));
+
+        // Go to the site test2's document library and click on fol1
+        // user1 is able to see the contents of fol1
+        childAssocs = nodeService.getChildAssocs(documentLibraryTest2Site);
+        assertEquals("Size should be 1", 1, childAssocs.size());
+        assertTrue("Folder name should start with fol1", getFirstName(childAssocs).startsWith("fol1"));
+        childAssocs = nodeService.getChildAssocs(childAssocs.get(0).getChildRef());
+        assertEquals("Size should be 1", 1, childAssocs.size());
+        assertTrue("Folder name should start with fol2", getFirstName(childAssocs).startsWith("fol2"));
+        childAssocs = nodeService.getChildAssocs(childAssocs.get(0).getChildRef());
+        assertEquals("Size should be 1", 1, childAssocs.size());
+        assertTrue("Folder name should start with fol3", getFirstName(childAssocs).startsWith("fol3"));
+    }
+
+    private String getFirstName(List<ChildAssociationRef> childAssocs)
+    {
+        return nodeService.getProperties(childAssocs.get(0).getChildRef()).get(ContentModel.PROP_NAME).toString();
+    }
+
     public void testDeleteSite()
     {
         @SuppressWarnings("deprecation")

src/test/resources/alfresco/templates/mail/testSentTo.txt.ftl

@@ -0,0 +1 @@
+This email was sent to ${to}