ACE-2224: Merged DEV to HEAD (5.0/Cloud)

89023: ACE-2224: Could not re-applying ACL by cmis api request using Atom binding
      - Report only repo permissions if onlyBasicPermissions is 'false', but do revert conversion if cmis basic permission has exact matching. Remove MNT-4561 changes to avoid clever logic. Restore unit test for MNT-10165 scenario. Allow apply empty set of direct permissions if node inherits parent permissions. In onlyBasicPermissions mode report only one cmis permission if one was set.


git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@89123 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

source/java/org/alfresco/opencmis/CMISConnector.java

@@ -2495,7 +2495,7 @@ public class CMISConnector implements ApplicationContextAware, ApplicationListen
             AccessControlEntryImpl directAce = bothAces.get(true);
             if ((directAce != null) && (!directAce.getPermissions().isEmpty()))
             {
-            	List<String> permissions = translatePermmissionsToCMIS(directAce.getPermissions(), onlyBasicPermissions);
+            	List<String> permissions = translatePermissionsToCMIS(directAce.getPermissions(), onlyBasicPermissions);
             	if(permissions != null && !permissions.isEmpty())
             	{
                 	// tck doesn't like empty permissions list
@@ -2508,7 +2508,7 @@ public class CMISConnector implements ApplicationContextAware, ApplicationListen
             AccessControlEntryImpl indirectAce = bothAces.get(false);
             if ((indirectAce != null) && (!indirectAce.getPermissions().isEmpty()))
             {
-            	List<String> permissions = translatePermmissionsToCMIS(indirectAce.getPermissions(), onlyBasicPermissions);
+            	List<String> permissions = translatePermissionsToCMIS(indirectAce.getPermissions(), onlyBasicPermissions);
                 indirectAce.setPermissions(permissions);
 
                 // remove permissions that are already set in the direct ACE
@@ -2530,7 +2530,7 @@ public class CMISConnector implements ApplicationContextAware, ApplicationListen
         return result;
     }
 
-    private List<String> translatePermmissionsToCMIS(List<String> permissions, boolean onlyBasicPermissions)
+    private List<String> translatePermissionsToCMIS(List<String> permissions, boolean onlyBasicPermissions)
     {
         Set<String> result = new TreeSet<String>();
 
@@ -2538,64 +2538,74 @@ public class CMISConnector implements ApplicationContextAware, ApplicationListen
         {
             PermissionReference permissionReference = permissionModelDao.getPermissionReference(null, permission);
 
-            // check for full permissions
+            if (onlyBasicPermissions)
-            if (permissionModelDao.hasFull(permissionReference))
             {
-                result.add(BasicPermissions.READ);
-                result.add(BasicPermissions.WRITE);
-                result.add(BasicPermissions.ALL);
-            }
 
-            // check short forms
+                // check for full permissions
-            Set<PermissionReference> longForms = permissionModelDao.getGranteePermissions(permissionReference);
+                if (permissionModelDao.hasFull(permissionReference))
+                {
+                    result.add(BasicPermissions.ALL);
+                }
 
-            HashSet<String> shortForms = new HashSet<String>();
+                // check short forms
-            for (PermissionReference longForm : longForms)
+                Set<PermissionReference> longForms = permissionModelDao.getGranteePermissions(permissionReference);
-            {
-                shortForms.add(permissionModelDao.isUnique(longForm) ? longForm.getName() : longForm.toString());
-            }
 
-            for (String perm : shortForms)
+                HashSet<String> shortForms = new HashSet<String>();
-            {
+                for (PermissionReference longForm : longForms)
-                if (PermissionService.READ.equals(perm))
+                {
+                    shortForms.add(permissionModelDao.isUnique(longForm) ? longForm.getName() : longForm.toString());
+                }
+
+                for (String perm : shortForms)
+                {
+                    if (PermissionService.READ.equals(perm))
+                    {
+                        result.add(BasicPermissions.READ);
+                    }
+                    else if (PermissionService.WRITE.equals(perm))
+                    {
+                        result.add(BasicPermissions.WRITE);
+                    }
+                    else if (PermissionService.ALL_PERMISSIONS.equals(perm))
+                    {
+                        result.add(BasicPermissions.ALL);
+                    }
+                }
+
+                // check the permission
+                if (PermissionService.READ.equals(permission))
                 {
                     result.add(BasicPermissions.READ);
                 }
-                else if (PermissionService.WRITE.equals(perm))
+                else if (PermissionService.WRITE.equals(permission))
                 {
                     result.add(BasicPermissions.WRITE);
                 }
-                else if (PermissionService.ALL_PERMISSIONS.equals(perm))
+                else if (PermissionService.ALL_PERMISSIONS.equals(permission))
                 {
-                    result.add(BasicPermissions.READ);
-                    result.add(BasicPermissions.WRITE);
                     result.add(BasicPermissions.ALL);
                 }
             }
-
+            else
-            // check the permission
-            if (PermissionService.READ.equals(permission))
-            {
-                result.add(BasicPermissions.READ);
-            }
-            else if (PermissionService.WRITE.equals(permission))
-            {
-                result.add(BasicPermissions.WRITE);
-            }
-            else if (PermissionService.ALL_PERMISSIONS.equals(permission))
-            {
-                result.add(BasicPermissions.READ);
-                result.add(BasicPermissions.WRITE);
-                result.add(BasicPermissions.ALL);
-            }
-
-            // expand native permissions
-            if (!onlyBasicPermissions)
             {
+                // ACE-2224: only repository specific permissions should be reported
                 if (permission.startsWith("{"))
                 {
                     result.add(permission);
                 }
+                // do revert conversion for basic permissions that have exact matching
+                else if (PermissionService.READ.equals(permission))
+                {
+                    result.add(BasicPermissions.READ);
+                }
+                else if (PermissionService.WRITE.equals(permission))
+                {
+                    result.add(BasicPermissions.WRITE);
+                }
+                else if (PermissionService.ALL_PERMISSIONS.equals(permission))
+                {
+                    result.add(BasicPermissions.ALL);
+                }
                 else
                 {
                     result.add(permissionReference.toString());
@@ -2709,7 +2719,7 @@ public class CMISConnector implements ApplicationContextAware, ApplicationListen
     {
         boolean hasAces = (aces != null) && (aces.getAces() != null) && !aces.getAces().isEmpty();
 
-        if (!hasAces)
+        if (!hasAces && !permissionService.getInheritParentPermissions(nodeRef))
         {
             return;
         }
@@ -2719,8 +2729,6 @@ public class CMISConnector implements ApplicationContextAware, ApplicationListen
             throw new CmisConstraintException("Object is not ACL controllable!");
         }
 
-        Set<AccessPermission> currentAces = permissionService.getAllSetPermissions(nodeRef);
-
         // remove all permissions
         permissionService.deletePermissions(nodeRef);
 
@@ -2734,7 +2742,6 @@ public class CMISConnector implements ApplicationContextAware, ApplicationListen
             }
 
             List<String> permissions = translatePermissionsFromCMIS(ace.getPermissions());
-            normalisePermissions(currentAces, permissions);
             for (String permission : permissions)
             {
                 permissionService.setPermission(nodeRef, principalId, permission, true);
@@ -2742,50 +2749,6 @@ public class CMISConnector implements ApplicationContextAware, ApplicationListen
         }
     }
 
-    /*
-     * ALF-11868: the cmis client library may incorrectly send READ or WRITE permissions to applyAcl.
-     * This method works around this by "normalising" permissions:
-     * 
-     * <ul>
-     *    <li> the WRITE permission is removed from permissions if the cmis:write permission is being removed i.e. is in currentAccessPermissions but not in newPermissions
-     *    <li> the cmis:write permission is removed from permissions if the WRITE permission is being removed i.e. is in currentAccessPermissions but not in newPermissions
-     *    <li> the READ permission is removed from permissions if the cmis:read permission is being removed i.e. is in currentAccessPermissions but not in newPermissions
-     *    <li> the cmis:read permission is removed from permissions if the READ permission is being removed i.e. is in currentAccessPermissions but not in newPermissions
-     * </ul>
-     */
-    private void normalisePermissions(Set<AccessPermission> currentAccessPermissions, List<String> newPermissions)
-    {
-    	Set<String> currentPermissions = new HashSet<String>(currentAccessPermissions.size());
-    	for(AccessPermission accessPermission : currentAccessPermissions)
-    	{
-    		currentPermissions.add(accessPermission.getPermission());
-    	}
-
-    	if(currentPermissions.contains(PermissionService.WRITE) && !newPermissions.contains(BasicPermissions.WRITE) && newPermissions.contains(PermissionService.WRITE))
-    	{
-    		// cmis:write is being removed, so remove WRITE from permissions
-    		newPermissions.remove(PermissionService.WRITE);
-    	}
-    	
-    	if(currentPermissions.contains(PermissionService.WRITE) && !newPermissions.contains(PermissionService.WRITE) && newPermissions.contains(BasicPermissions.WRITE))
-    	{
-    		// WRITE is being removed, so remove cmis:write from permissions
-    		newPermissions.remove(BasicPermissions.WRITE);
-    	}
-    	
-    	if(currentPermissions.contains(PermissionService.READ) && !newPermissions.contains(BasicPermissions.READ) && newPermissions.contains(PermissionService.READ))
-    	{
-    		// cmis:read is being removed, so remove READ from permissions
-    		newPermissions.remove(PermissionService.READ);
-    	}
-    	
-    	if(currentPermissions.contains(PermissionService.READ) && !newPermissions.contains(PermissionService.READ) && newPermissions.contains(BasicPermissions.READ))
-    	{
-    		// READ is being removed, so remove cmis:read from permissions
-    		newPermissions.remove(BasicPermissions.READ);
-    	}
-    }
-
     private List<String> translatePermissionsFromCMIS(List<String> permissions)
     {
         List<String> result = new ArrayList<String>();

source/test-java/org/alfresco/opencmis/CMISTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2005-2013 Alfresco Software Limited.
+ * Copyright (C) 2005-2014 Alfresco Software Limited.
  *
  * This file is part of Alfresco
  *
@@ -33,6 +33,7 @@ import java.math.BigInteger;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.GregorianCalendar;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -75,7 +76,9 @@ import org.alfresco.service.cmr.repository.StoreRef;
 import org.alfresco.service.cmr.rule.Rule;
 import org.alfresco.service.cmr.rule.RuleService;
 import org.alfresco.service.cmr.rule.RuleType;
+import org.alfresco.service.cmr.security.AccessPermission;
 import org.alfresco.service.cmr.security.AuthorityService;
+import org.alfresco.service.cmr.security.AuthorityType;
 import org.alfresco.service.cmr.security.PermissionService;
 import org.alfresco.service.cmr.tagging.TaggingService;
 import org.alfresco.service.cmr.version.VersionService;
@@ -85,6 +88,7 @@ import org.alfresco.service.transaction.TransactionService;
 import org.alfresco.util.ApplicationContextHelper;
 import org.alfresco.util.Pair;
 import org.apache.chemistry.opencmis.commons.PropertyIds;
+import org.apache.chemistry.opencmis.commons.data.Ace;
 import org.apache.chemistry.opencmis.commons.data.AllowableActions;
 import org.apache.chemistry.opencmis.commons.data.CmisExtensionElement;
 import org.apache.chemistry.opencmis.commons.data.ObjectData;
@@ -95,6 +99,7 @@ import org.apache.chemistry.opencmis.commons.data.Properties;
 import org.apache.chemistry.opencmis.commons.data.PropertyData;
 import org.apache.chemistry.opencmis.commons.data.RepositoryInfo;
 import org.apache.chemistry.opencmis.commons.definitions.TypeDefinition;
+import org.apache.chemistry.opencmis.commons.enums.AclPropagation;
 import org.apache.chemistry.opencmis.commons.enums.Action;
 import org.apache.chemistry.opencmis.commons.enums.ChangeType;
 import org.apache.chemistry.opencmis.commons.enums.CmisVersion;
@@ -103,6 +108,7 @@ import org.apache.chemistry.opencmis.commons.enums.VersioningState;
 import org.apache.chemistry.opencmis.commons.exceptions.CmisConstraintException;
 import org.apache.chemistry.opencmis.commons.exceptions.CmisRuntimeException;
 import org.apache.chemistry.opencmis.commons.exceptions.CmisUpdateConflictException;
+import org.apache.chemistry.opencmis.commons.impl.dataobjects.AccessControlListImpl;
 import org.apache.chemistry.opencmis.commons.impl.dataobjects.CmisExtensionElementImpl;
 import org.apache.chemistry.opencmis.commons.impl.dataobjects.ContentStreamImpl;
 import org.apache.chemistry.opencmis.commons.impl.dataobjects.ExtensionDataImpl;
@@ -2579,4 +2585,115 @@ public class CMISTest
         withCmisService(callback, CmisVersion.CMIS_1_1);
         withCmisService(callback, CmisVersion.CMIS_1_0);
     }
+    
+    /**
+     * MNT-10165: Check that all concomitant basic CMIS permissions are deleted
+     * when permission is deleted vai CMIS 1.1 API. For Atom binding it applies
+     * new set of permissions instead of deleting the old ones.
+     */
+    @Test
+    public void testRemoveACL() throws Exception
+    {
+        AuthenticationUtil.pushAuthentication();
+        AuthenticationUtil.setFullyAuthenticatedUser(AuthenticationUtil.getAdminUserName());
+        final String groupName = "group" + GUID.generate();
+        final String testGroup = PermissionService.GROUP_PREFIX + groupName;
+        try
+        {
+            // preconditions: create test document
+            if (!authorityService.authorityExists(testGroup))
+            {
+                authorityService.createAuthority(AuthorityType.GROUP, groupName);
+            }
+            
+            final FileInfo document = transactionService.getRetryingTransactionHelper().doInTransaction(
+                    new RetryingTransactionCallback<FileInfo>()
+                    {
+                        @Override
+                        public FileInfo execute() throws Throwable
+                        {
+                            NodeRef companyHomeNodeRef = repositoryHelper.getCompanyHome();
+
+                            String folderName = GUID.generate();
+                            FileInfo folderInfo = fileFolderService.create(companyHomeNodeRef, folderName, ContentModel.TYPE_FOLDER);
+                            nodeService.setProperty(folderInfo.getNodeRef(), ContentModel.PROP_NAME, folderName);
+                            assertNotNull(folderInfo);
+
+                            String docName = GUID.generate();
+                            FileInfo document = fileFolderService.create(folderInfo.getNodeRef(), docName, ContentModel.TYPE_CONTENT);
+                            assertNotNull(document);
+                            nodeService.setProperty(document.getNodeRef(), ContentModel.PROP_NAME, docName);
+
+                            return document;
+                        }
+                    });
+
+            Set<AccessPermission> permissions = permissionService.getAllSetPermissions(document.getNodeRef());
+            assertEquals(permissions.size(), 1);
+            AccessPermission current = permissions.iterator().next();
+            assertEquals(current.getAuthority(), "GROUP_EVERYONE");
+            assertEquals(current.getPermission(), "Consumer");
+
+            // add group1 with Coordinator permissions
+            permissionService.setPermission(document.getNodeRef(), testGroup, PermissionService.COORDINATOR, true);
+            permissions = permissionService.getAllSetPermissions(document.getNodeRef());
+
+            Map<String , String> docPermissions = new HashMap<String, String>();
+            for (AccessPermission permission : permissions)
+            {
+                docPermissions.put(permission.getAuthority(), permission.getPermission());
+            }
+            assertTrue(docPermissions.keySet().contains(testGroup));
+            assertEquals(docPermissions.get(testGroup), PermissionService.COORDINATOR);
+
+            // update permissions for group1 via CMIS 1.1 API 
+            withCmisService(new CmisServiceCallback<Void>()
+            {
+                @Override
+                public Void execute(CmisService cmisService)
+                {
+                    List<RepositoryInfo> repositories = cmisService.getRepositoryInfos(null);
+                    assertNotNull(repositories);
+                    assertTrue(repositories.size() > 0);
+                    RepositoryInfo repo = repositories.iterator().next();
+                    String repositoryId = repo.getId();
+                    String docIdStr = document.getNodeRef().toString();
+
+                    // when removing Coordinator ACE there are only inherited permissions
+                    // so empty list of direct permissions is sent to be set
+                    AccessControlListImpl acesToPut = new AccessControlListImpl();
+                    List<Ace> acesList = Collections.emptyList();
+                    acesToPut.setAces(acesList);
+                    cmisService.applyAcl(repositoryId, docIdStr, acesToPut, AclPropagation.REPOSITORYDETERMINED);
+
+                    return null;
+                }
+            }, CmisVersion.CMIS_1_1);
+
+            // check that permissions are the same as they were before Coordinator was added
+            permissions = permissionService.getAllSetPermissions(document.getNodeRef());
+            docPermissions = new HashMap<String, String>();
+            for (AccessPermission permission : permissions)
+            {
+                docPermissions.put(permission.getAuthority(), permission.getPermission());
+            }
+            assertFalse(docPermissions.keySet().contains(testGroup));
+            assertEquals(permissions.size(), 1);
+            current = permissions.iterator().next();
+            assertEquals(current.getAuthority(), "GROUP_EVERYONE");
+            assertEquals(current.getPermission(), "Consumer");
+        }
+        catch (CmisConstraintException e)
+        {
+            fail(e.toString());
+        }
+        finally
+        {
+            if (authorityService.authorityExists(testGroup))
+            {
+                authorityService.deleteAuthority(testGroup);
+            }
+            AuthenticationUtil.popAuthentication();
+        }
+    }
 }