From ef5954a1a4945ff5aa8d930f2dd4bfabe9358c16 Mon Sep 17 00:00:00 2001 From: Pavel Yurke Date: Thu, 2 Oct 2014 09:39:02 +0000 Subject: [PATCH] Reverse merged HEAD (5.0/Cloud) << Put the code back to HEAD >> 86049 : Reverse merged HEAD (5.0/Cloud) << Cause of 4 errors in https://bamboo.alfresco.com/bamboo/browse/ALF-ENT-141 >> 85880: ACE-2181 : Reverse Merge HEAD (5.0) << Implementing another approach for MNT-10946 and ACE-2181 >> Merged HEAD-BUG-FIX (5.0/Cloud) to HEAD (4.3/Cloud) 71600: Merged V4.2-BUG-FIX (4.2.3) to HEAD-BUG-FIX (4.3/Cloud) 70349: Merged DEV to V4.2-BUG-FIX (4.2.3) 70294 : MNT-10946 : Admin is no longer able to unlock files - Check if node is locked before unlock for non-admin or System users. Fix related test 85881: ACE-2181 : Merged DEV to HEAD (5.0) 76214 : MNT-10946 : Admin is no longer able to unlock files - Drop check for lockowner from AbstractLockStore.set. Fix related test git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@86238 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- config/alfresco/core-services-context.xml | 1 - .../alfresco/repo/lock/LockServiceImpl.java | 48 +------------------ .../repo/lock/mem/AbstractLockStore.java | 23 --------- .../org/alfresco/repo/lock/mem/LockStore.java | 7 --- .../repo/lock/LockServiceImplTest.java | 33 +++++++++---- 5 files changed, 27 insertions(+), 85 deletions(-) diff --git a/config/alfresco/core-services-context.xml b/config/alfresco/core-services-context.xml index d628b03752..ef6a591f95 100644 --- a/config/alfresco/core-services-context.xml +++ b/config/alfresco/core-services-context.xml @@ -429,7 +429,6 @@ - diff --git a/source/java/org/alfresco/repo/lock/LockServiceImpl.java b/source/java/org/alfresco/repo/lock/LockServiceImpl.java index 87d246b6ce..66094d8585 100644 --- a/source/java/org/alfresco/repo/lock/LockServiceImpl.java +++ b/source/java/org/alfresco/repo/lock/LockServiceImpl.java @@ -66,8 +66,6 @@ import org.alfresco.service.cmr.repository.StoreRef; import org.alfresco.service.cmr.search.ResultSet; import org.alfresco.service.cmr.search.SearchService; import org.alfresco.service.cmr.security.AuthenticationService; -import org.alfresco.service.cmr.security.AuthorityService; -import org.alfresco.service.cmr.security.PermissionService; import org.alfresco.service.namespace.QName; import org.alfresco.util.Pair; import org.alfresco.util.PropertyCheck; @@ -96,7 +94,6 @@ public class LockServiceImpl implements LockService, private TenantService tenantService; private AuthenticationService authenticationService; private SearchService searchService; - private AuthorityService authorityService; private BehaviourFilter behaviourFilter; private LockStore lockStore; private PolicyComponent policyComponent; @@ -142,11 +139,6 @@ public class LockServiceImpl implements LockService, this.searchService = searchService; } - public void setAuthorityService(AuthorityService authorityService) - { - this.authorityService = authorityService; - } - /** * Initialise methods called by Spring framework */ @@ -158,7 +150,6 @@ public class LockServiceImpl implements LockService, PropertyCheck.mandatory(this, "searchService", searchService); PropertyCheck.mandatory(this, "behaviourFilter", behaviourFilter); PropertyCheck.mandatory(this, "policyComponent", policyComponent); - PropertyCheck.mandatory(this, "authorityService", authorityService); // Register the policies beforeLock = policyComponent.registerClassPolicy(LockServicePolicies.BeforeLock.class); @@ -487,8 +478,6 @@ public class LockServiceImpl implements LockService, { throw new UnableToReleaseLockException(nodeRef, CAUSE.CHECKED_OUT); } - // check if the user able to unlock the node - checkNodeBeforeUnlock(nodeRef); // Remove the lock from persistent storage. Lifetime lifetime = lockState.getLifetime(); @@ -514,8 +503,8 @@ public class LockServiceImpl implements LockService, } else if (lifetime == Lifetime.EPHEMERAL) { - // force unlock the ephemeral lock. - lockStore.forceUnlock(nodeRef); + // Remove the ephemeral lock. + lockStore.set(nodeRef, LockState.createUnlocked(nodeRef)); nodeIndexer.indexUpdateNode(nodeRef); } else @@ -667,39 +656,6 @@ public class LockServiceImpl implements LockService, } } } - - private void checkNodeBeforeUnlock(NodeRef nodeRef) - { - String userName = getUserName(); - Set userAuthorities = authorityService.getAuthoritiesForUser(userName); - // ignore check for admins and system - if (userAuthorities.contains(PermissionService.ADMINISTRATOR_AUTHORITY) || - tenantService.getBaseNameUser(userName).equals(AuthenticationUtil.getSystemUserName())) - { - return; - } - - nodeRef = tenantService.getName(nodeRef); - - // Ensure we have found a node reference - if (nodeRef != null && userName != null) - { - try - { - // Get the current lock status on the node ref - LockStatus currentLockStatus = getLockStatus(nodeRef, userName); - - if (LockStatus.LOCKED.equals(currentLockStatus) == true) - { - throw new UnableToReleaseLockException(nodeRef); - } - } - catch (AspectMissingException exception) - { - // Ignore since this indicates that the node does not have the lock aspect applied - } - } - } /** * Ensures that the parent is not locked. diff --git a/source/java/org/alfresco/repo/lock/mem/AbstractLockStore.java b/source/java/org/alfresco/repo/lock/mem/AbstractLockStore.java index 894b0f3d39..97158681d9 100644 --- a/source/java/org/alfresco/repo/lock/mem/AbstractLockStore.java +++ b/source/java/org/alfresco/repo/lock/mem/AbstractLockStore.java @@ -18,16 +18,11 @@ */ package org.alfresco.repo.lock.mem; -import java.util.Date; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentMap; -import org.alfresco.repo.lock.LockUtils; -import org.alfresco.repo.security.authentication.AuthenticationUtil; import org.alfresco.repo.transaction.TransactionalResourceHelper; -import org.alfresco.service.cmr.lock.LockStatus; -import org.alfresco.service.cmr.lock.UnableToAquireLockException; import org.alfresco.service.cmr.repository.NodeRef; import org.springframework.dao.ConcurrencyFailureException; import org.springframework.transaction.support.TransactionSynchronizationManager; @@ -67,20 +62,9 @@ public abstract class AbstractLockStore txMap = getTxMap(); LockState previousLockState = null; @@ -110,13 +94,6 @@ public abstract class AbstractLockStore getNodes(); - /** - * WARNING: only use in lockService - unlocks node ignoring lockOwner - * - * @param nodeRef - */ - void forceUnlock(NodeRef nodeRef); - /** * WARNING: only use in test code - unsafe method for production use. * diff --git a/source/test-java/org/alfresco/repo/lock/LockServiceImplTest.java b/source/test-java/org/alfresco/repo/lock/LockServiceImplTest.java index ae35dda2c6..af742c5b06 100644 --- a/source/test-java/org/alfresco/repo/lock/LockServiceImplTest.java +++ b/source/test-java/org/alfresco/repo/lock/LockServiceImplTest.java @@ -34,6 +34,7 @@ import org.alfresco.repo.search.IndexerAndSearcher; import org.alfresco.repo.search.SearcherComponent; import org.alfresco.repo.security.authentication.AuthenticationComponent; import org.alfresco.repo.security.authentication.AuthenticationUtil; +import org.alfresco.repo.security.permissions.AccessDeniedException; import org.alfresco.service.cmr.coci.CheckOutCheckInService; import org.alfresco.service.cmr.lock.LockService; import org.alfresco.service.cmr.lock.LockStatus; @@ -47,6 +48,7 @@ import org.alfresco.service.cmr.repository.NodeService; import org.alfresco.service.cmr.repository.StoreRef; import org.alfresco.service.cmr.search.ResultSet; import org.alfresco.service.cmr.security.MutableAuthenticationService; +import org.alfresco.service.cmr.security.PermissionService; import org.alfresco.service.namespace.QName; import org.alfresco.test_category.BaseSpringTestsCategory; import org.alfresco.test_category.OwnJVMTestsCategory; @@ -71,6 +73,8 @@ public class LockServiceImplTest extends BaseSpringTest private MutableAuthenticationService authenticationService; private CheckOutCheckInService cociService; + private PermissionService permissionService; + private LockService securedLockService; /** * Data used in tests */ @@ -94,6 +98,10 @@ public class LockServiceImplTest extends BaseSpringTest { this.nodeService = (NodeService)applicationContext.getBean("dbNodeService"); this.lockService = (LockService)applicationContext.getBean("lockService"); + + this.securedLockService = (LockService)applicationContext.getBean("LockService"); + this.permissionService = (PermissionService)applicationContext.getBean("PermissionService"); + this.authenticationService = (MutableAuthenticationService)applicationContext.getBean("authenticationService"); this.cociService = (CheckOutCheckInService) applicationContext.getBean("checkOutCheckInService"); @@ -171,6 +179,11 @@ public class LockServiceImplTest extends BaseSpringTest TestWithUserUtils.createUser(GOOD_USER_NAME, PWD, rootNodeRef, this.nodeService, this.authenticationService); TestWithUserUtils.createUser(BAD_USER_NAME, PWD, rootNodeRef, this.nodeService, this.authenticationService); + this.permissionService.setPermission(rootNodeRef, GOOD_USER_NAME, PermissionService.ALL_PERMISSIONS, true); + this.permissionService.setPermission(rootNodeRef, BAD_USER_NAME, PermissionService.CHECK_OUT, true); + this.permissionService.setPermission(rootNodeRef, BAD_USER_NAME, PermissionService.WRITE, true); + this.permissionService.setPermission(rootNodeRef, BAD_USER_NAME, PermissionService.READ, true); + // Stash the user node ref's for later use TestWithUserUtils.authenticateUser(BAD_USER_NAME, PWD, rootNodeRef, this.authenticationService); TestWithUserUtils.authenticateUser(GOOD_USER_NAME, PWD, rootNodeRef, this.authenticationService); @@ -905,7 +918,8 @@ public class LockServiceImplTest extends BaseSpringTest } } - public void testUnlockEphemeralNodeWithAdminUser() + @SuppressWarnings("deprecation") + public void testUnlockNodeWithAdminUser() { for (Lifetime lt : new Lifetime[]{Lifetime.EPHEMERAL, Lifetime.PERSISTENT}) { @@ -916,27 +930,30 @@ public class LockServiceImplTest extends BaseSpringTest this.nodeService.createNode(parentNode, ContentModel.ASSOC_CONTAINS, QName.createQName("{}testNode"), ContentModel.TYPE_CONTAINER).getChildRef(); // lock it as GOOD user - this.lockService.lock(testNode, LockType.WRITE_LOCK, 2 * 86400, lt, null); + this.securedLockService.lock(testNode, LockType.WRITE_LOCK, 2 * 86400, lt, null); TestWithUserUtils.authenticateUser(BAD_USER_NAME, PWD, rootNodeRef, this.authenticationService); try { // try to unlock as bad user - this.lockService.unlock(testNode); + this.securedLockService.unlock(testNode); fail("BAD user shouldn't be able to unlock " + lt + " lock"); } - catch(UnableToReleaseLockException e) + catch(AccessDeniedException e) { - // it's expected + // expected expetion } TestWithUserUtils.authenticateUser(AuthenticationUtil.getAdminUserName(), "admin", rootNodeRef, this.authenticationService); // try to unlock as ADMIN user - this.lockService.unlock(testNode); - - TestWithUserUtils.authenticateUser(GOOD_USER_NAME, PWD, rootNodeRef, this.authenticationService); + this.securedLockService.unlock(testNode); + + // test that bad use able to lock/unlock node + TestWithUserUtils.authenticateUser(BAD_USER_NAME, PWD, rootNodeRef, this.authenticationService); + this.securedLockService.lock(testNode, LockType.WRITE_LOCK, 2 * 86400, lt, null); + this.securedLockService.unlock(testNode); this.nodeService.deleteNode(testNode); }