diff --git a/src/main/java/org/alfresco/repo/site/SitesPermissionCleaner.java b/src/main/java/org/alfresco/repo/site/SitesPermissionCleaner.java index 76d37475d3..9c655b43a7 100644 --- a/src/main/java/org/alfresco/repo/site/SitesPermissionCleaner.java +++ b/src/main/java/org/alfresco/repo/site/SitesPermissionCleaner.java @@ -1,28 +1,28 @@ -/* - * #%L - * Alfresco Repository - * %% - * Copyright (C) 2005 - 2016 Alfresco Software Limited - * %% - * This file is part of the Alfresco software. - * If the software was purchased under a paid Alfresco license, the terms of - * the paid license agreement will prevail. Otherwise, the software is - * provided under the following open source license terms: - * - * 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 - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * Alfresco is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with Alfresco. If not, see . - * #L% - */ +/* + * #%L + * Alfresco Repository + * %% + * Copyright (C) 2005 - 2016 Alfresco Software Limited + * %% + * This file is part of the Alfresco software. + * If the software was purchased under a paid Alfresco license, the terms of + * the paid license agreement will prevail. Otherwise, the software is + * provided under the following open source license terms: + * + * 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 + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Alfresco is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Alfresco. If not, see . + * #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. - if (containingSite == null) - { - containingSite = siteServiceImpl.getSite(targetNode); - } + return; + } + // We can calculate the containing site at the start of a recursive call & then reuse it on subsequent calls. + if (containingSite == null) + { + containingSite = siteServiceImpl.getSite(targetNode); + } - // Short-circuit at this point if the node is not in a Site. - if (containingSite != null) + // Short-circuit at this point if the node is not in a Site. + 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 targetNodeAclEntries = targetNodeAccessControlList.getEntries(); + for (AccessControlEntry entry : targetNodeAclEntries) { - // 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) + 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) && + // And it's not GROUP_EVERYONE + !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); - List targetNodeAclEntries = targetNodeAccessControlList.getEntries(); - 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); - } - } - - } + // Then remove it. + permissionService.clearPermission(targetNode, authority); } - - // Recurse - List childNodeIds = nodeDAO.getPrimaryChildrenAcls(targetNodeID); - for (NodeIdAndAclId nextChild : childNodeIds) + + if (!permissionService.getInheritParentPermissions(targetNode)) { - 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() + { + public Void doWork() throws Exception + { + permissionService.setPermission(targetNode, siteManagerAuthority, SiteModel.SITE_MANAGER, true); + return null; + } + }, AuthenticationUtil.getSystemUserName()); } } } + + // Recurse + List childNodeIds = nodeDAO.getPrimaryChildrenAcls(targetNodeID); + for (NodeIdAndAclId nextChild : childNodeIds) + { + cleanSitePermissions(nodeDAO.getNodePair(nextChild.getId()).getSecond(), containingSite); + } } } diff --git a/src/test/java/org/alfresco/repo/site/SiteServiceImplTest.java b/src/test/java/org/alfresco/repo/site/SiteServiceImplTest.java index 95bdf04954..43be6f9622 100644 --- a/src/test/java/org/alfresco/repo/site/SiteServiceImplTest.java +++ b/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 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 childAssocs) + { + return nodeService.getProperties(childAssocs.get(0).getChildRef()).get(ContentModel.PROP_NAME).toString(); + } + public void testDeleteSite() { @SuppressWarnings("deprecation")