From c9dbaa773aa665a3bc28916284586710f42c3710 Mon Sep 17 00:00:00 2001 From: Pavel Yurke Date: Fri, 24 Oct 2014 14:52:08 +0000 Subject: [PATCH] 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 --- .../org/alfresco/opencmis/CMISConnector.java | 143 +++++++----------- .../org/alfresco/opencmis/CMISTest.java | 119 ++++++++++++++- 2 files changed, 171 insertions(+), 91 deletions(-) diff --git a/source/java/org/alfresco/opencmis/CMISConnector.java b/source/java/org/alfresco/opencmis/CMISConnector.java index b33c849c47..1393bbf93a 100644 --- a/source/java/org/alfresco/opencmis/CMISConnector.java +++ b/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 permissions = translatePermmissionsToCMIS(directAce.getPermissions(), onlyBasicPermissions); + List 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 permissions = translatePermmissionsToCMIS(indirectAce.getPermissions(), onlyBasicPermissions); + List 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 translatePermmissionsToCMIS(List permissions, boolean onlyBasicPermissions) + private List translatePermissionsToCMIS(List permissions, boolean onlyBasicPermissions) { Set result = new TreeSet(); @@ -2538,64 +2538,74 @@ public class CMISConnector implements ApplicationContextAware, ApplicationListen { PermissionReference permissionReference = permissionModelDao.getPermissionReference(null, permission); - // check for full permissions - if (permissionModelDao.hasFull(permissionReference)) + if (onlyBasicPermissions) { - result.add(BasicPermissions.READ); - result.add(BasicPermissions.WRITE); - result.add(BasicPermissions.ALL); - } - // check short forms - Set longForms = permissionModelDao.getGranteePermissions(permissionReference); + // check for full permissions + if (permissionModelDao.hasFull(permissionReference)) + { + result.add(BasicPermissions.ALL); + } - HashSet shortForms = new HashSet(); - for (PermissionReference longForm : longForms) - { - shortForms.add(permissionModelDao.isUnique(longForm) ? longForm.getName() : longForm.toString()); - } + // check short forms + Set longForms = permissionModelDao.getGranteePermissions(permissionReference); - for (String perm : shortForms) - { - if (PermissionService.READ.equals(perm)) + HashSet shortForms = new HashSet(); + for (PermissionReference longForm : longForms) + { + 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); } } - - // 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) + else { + // 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 currentAces = permissionService.getAllSetPermissions(nodeRef); - // remove all permissions permissionService.deletePermissions(nodeRef); @@ -2734,7 +2742,6 @@ public class CMISConnector implements ApplicationContextAware, ApplicationListen } List 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: - * - *
    - *
  • the WRITE permission is removed from permissions if the cmis:write permission is being removed i.e. is in currentAccessPermissions but not in newPermissions - *
  • the cmis:write permission is removed from permissions if the WRITE permission is being removed i.e. is in currentAccessPermissions but not in newPermissions - *
  • the READ permission is removed from permissions if the cmis:read permission is being removed i.e. is in currentAccessPermissions but not in newPermissions - *
  • the cmis:read permission is removed from permissions if the READ permission is being removed i.e. is in currentAccessPermissions but not in newPermissions - *
- */ - private void normalisePermissions(Set currentAccessPermissions, List newPermissions) - { - Set currentPermissions = new HashSet(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 translatePermissionsFromCMIS(List permissions) { List result = new ArrayList(); diff --git a/source/test-java/org/alfresco/opencmis/CMISTest.java b/source/test-java/org/alfresco/opencmis/CMISTest.java index 14e38c8161..2703e9a5ab 100644 --- a/source/test-java/org/alfresco/opencmis/CMISTest.java +++ b/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() + { + @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 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 docPermissions = new HashMap(); + 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() + { + @Override + public Void execute(CmisService cmisService) + { + List 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 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(); + 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(); + } + } }