mirror of
https://github.com/Alfresco/alfresco-community-repo.git
synced 2025-08-07 17:49:17 +00:00
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@86049 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261
This commit is contained in:
@@ -429,6 +429,7 @@
|
||||
<property name="searchService" ref="admSearchService" />
|
||||
<property name="behaviourFilter" ref="policyBehaviourFilter" />
|
||||
<property name="nodeIndexer" ref="nodeIndexer"/>
|
||||
<property name="authorityService" ref="authorityService"/>
|
||||
</bean>
|
||||
|
||||
<!-- -->
|
||||
|
@@ -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
|
||||
@@ -657,6 +668,39 @@ public class LockServiceImpl implements LockService,
|
||||
}
|
||||
}
|
||||
|
||||
private void checkNodeBeforeUnlock(NodeRef nodeRef)
|
||||
{
|
||||
String userName = getUserName();
|
||||
Set<String> 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.
|
||||
*
|
||||
|
@@ -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;
|
||||
@@ -63,8 +68,19 @@ public abstract class AbstractLockStore<T extends ConcurrentMap<NodeRef, LockSta
|
||||
return lockState;
|
||||
}
|
||||
|
||||
@Override
|
||||
public void forceUnlock(NodeRef nodeRef)
|
||||
{
|
||||
set(nodeRef, LockState.createUnlocked(nodeRef), true);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void set(NodeRef nodeRef, LockState lockState)
|
||||
{
|
||||
set(nodeRef, lockState, false);
|
||||
}
|
||||
|
||||
private void set(NodeRef nodeRef, LockState lockState, boolean ignoreOwnerCheck)
|
||||
{
|
||||
Map<NodeRef, LockState> txMap = getTxMap();
|
||||
LockState previousLockState = null;
|
||||
@@ -94,6 +110,13 @@ public abstract class AbstractLockStore<T extends ConcurrentMap<NodeRef, LockSta
|
||||
|
||||
if (previousLockState != null)
|
||||
{
|
||||
String userName = AuthenticationUtil.getFullyAuthenticatedUser();
|
||||
String owner = previousLockState.getOwner();
|
||||
Date expires = previousLockState.getExpires();
|
||||
if (!ignoreOwnerCheck && LockUtils.lockStatus(userName, owner, expires) == LockStatus.LOCKED)
|
||||
{
|
||||
throw new UnableToAquireLockException(nodeRef);
|
||||
}
|
||||
// Use ConcurrentMap.replace(key, old, new) so that we can ensure we don't encounter a
|
||||
// 'lost update' (i.e. someone else has locked a node while we were thinking about it).
|
||||
updated = map.replace(nodeRef, previousLockState, lockState);
|
||||
|
@@ -35,6 +35,13 @@ public interface LockStore
|
||||
void set(NodeRef nodeRef, LockState lockState);
|
||||
public Set<NodeRef> 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.
|
||||
*
|
||||
|
@@ -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);
|
||||
this.lockService.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);
|
||||
TestWithUserUtils.authenticateUser(GOOD_USER_NAME, PWD, rootNodeRef, this.authenticationService);
|
||||
|
||||
this.nodeService.deleteNode(testNode);
|
||||
}
|
||||
|
Reference in New Issue
Block a user