diff --git a/config/alfresco/core-services-context.xml b/config/alfresco/core-services-context.xml index ef6a591f95..d628b03752 100644 --- a/config/alfresco/core-services-context.xml +++ b/config/alfresco/core-services-context.xml @@ -429,6 +429,7 @@ + diff --git a/source/java/org/alfresco/repo/lock/LockServiceImpl.java b/source/java/org/alfresco/repo/lock/LockServiceImpl.java index 66094d8585..87d246b6ce 100644 --- a/source/java/org/alfresco/repo/lock/LockServiceImpl.java +++ b/source/java/org/alfresco/repo/lock/LockServiceImpl.java @@ -66,6 +66,8 @@ 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; @@ -94,6 +96,7 @@ 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; @@ -139,6 +142,11 @@ public class LockServiceImpl implements LockService, this.searchService = searchService; } + public void setAuthorityService(AuthorityService authorityService) + { + this.authorityService = authorityService; + } + /** * Initialise methods called by Spring framework */ @@ -150,6 +158,7 @@ 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); @@ -478,6 +487,8 @@ 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(); @@ -503,8 +514,8 @@ public class LockServiceImpl implements LockService, } else if (lifetime == Lifetime.EPHEMERAL) { - // Remove the ephemeral lock. - lockStore.set(nodeRef, LockState.createUnlocked(nodeRef)); + // force unlock the ephemeral lock. + lockStore.forceUnlock(nodeRef); nodeIndexer.indexUpdateNode(nodeRef); } else @@ -656,6 +667,39 @@ 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 97158681d9..894b0f3d39 100644 --- a/source/java/org/alfresco/repo/lock/mem/AbstractLockStore.java +++ b/source/java/org/alfresco/repo/lock/mem/AbstractLockStore.java @@ -18,11 +18,16 @@ */ 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; @@ -62,9 +67,20 @@ public abstract class AbstractLockStore txMap = getTxMap(); LockState previousLockState = null; @@ -94,6 +110,13 @@ 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 af742c5b06..ae35dda2c6 100644 --- a/source/test-java/org/alfresco/repo/lock/LockServiceImplTest.java +++ b/source/test-java/org/alfresco/repo/lock/LockServiceImplTest.java @@ -34,7 +34,6 @@ 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; @@ -48,7 +47,6 @@ 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; @@ -73,8 +71,6 @@ public class LockServiceImplTest extends BaseSpringTest private MutableAuthenticationService authenticationService; private CheckOutCheckInService cociService; - private PermissionService permissionService; - private LockService securedLockService; /** * Data used in tests */ @@ -98,10 +94,6 @@ 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"); @@ -179,11 +171,6 @@ 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); @@ -918,8 +905,7 @@ public class LockServiceImplTest extends BaseSpringTest } } - @SuppressWarnings("deprecation") - public void testUnlockNodeWithAdminUser() + public void testUnlockEphemeralNodeWithAdminUser() { for (Lifetime lt : new Lifetime[]{Lifetime.EPHEMERAL, Lifetime.PERSISTENT}) { @@ -930,30 +916,27 @@ 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.securedLockService.lock(testNode, LockType.WRITE_LOCK, 2 * 86400, lt, null); + this.lockService.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.securedLockService.unlock(testNode); + this.lockService.unlock(testNode); fail("BAD user shouldn't be able to unlock " + lt + " lock"); } - catch(AccessDeniedException e) + catch(UnableToReleaseLockException e) { - // expected expetion + // it's expected } TestWithUserUtils.authenticateUser(AuthenticationUtil.getAdminUserName(), "admin", rootNodeRef, this.authenticationService); // try to unlock as ADMIN user - 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.lockService.unlock(testNode); + + TestWithUserUtils.authenticateUser(GOOD_USER_NAME, PWD, rootNodeRef, this.authenticationService); this.nodeService.deleteNode(testNode); }