Skip to content

Commit

Permalink
JCR-2859: Make open scoped locks recoverable
Browse files Browse the repository at this point in the history
expose lock tokens to admin users, tune test cases that assumed lock tokens to be null, add lock breaking test case, tune spi2dav to properly deal with locks that expose their lock token to non-owners

git-svn-id: https://svn.apache.org/repos/asf/jackrabbit/trunk@1230681 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information
reschke committed Jan 12, 2012
1 parent 726743e commit 54f743f
Show file tree
Hide file tree
Showing 7 changed files with 117 additions and 20 deletions.
Expand Up @@ -22,6 +22,7 @@

import javax.jcr.Node;
import javax.jcr.RepositoryException;
import javax.jcr.Session;
import javax.jcr.lock.LockException;

/**
Expand Down Expand Up @@ -78,7 +79,7 @@ public Node getNode() {
* {@inheritDoc}
*/
public String getLockToken() {
if (!info.isSessionScoped() && info.isLockHolder(node.getSession())) {
if (!info.isSessionScoped() && (info.isLockHolder(node.getSession()) || isAdminUser(node.getSession()))) {
return info.getLockToken();
} else {
return null;
Expand Down Expand Up @@ -151,4 +152,15 @@ public boolean isLockOwningSession() {
return info.isLockHolder(node.getSession());
}

/**
* Check whether a session belongs to an administrative user.
*/
private boolean isAdminUser(Session session) {
if (session instanceof SessionImpl) {
return ((SessionImpl) session).isAdmin();
} else {
// fallback. use hardcoded default admin ID
return "admin".equals(session.getUserID());
}
}
}
Expand Up @@ -941,14 +941,21 @@ public void testCreateLockUnlockInDifferentTransactions() throws Exception {
n.save();

superuser.removeLockToken(lockToken);
assertNull("session must get a null lock token", lock.getLockToken());

String nlt = lock.getLockToken();
assertTrue("freshly obtained lock token must either be null or the same as the one returned earlier",
nlt == null || nlt.equals(lockToken));

assertFalse("session must not hold lock token", containsLockToken(superuser, lockToken));

// commit
utx.commit();

nlt = lock.getLockToken();
assertTrue("freshly obtained lock token must either be null or the same as the one returned earlier",
nlt == null || nlt.equals(lockToken));

assertFalse("session must not hold lock token", containsLockToken(superuser, lockToken));
assertNull("session must get a null lock token", lock.getLockToken());

// start new Transaction and try to unlock
utx = new UserTransactionImpl(superuser);
Expand Down Expand Up @@ -1141,15 +1148,20 @@ public void testAddRemoveLockToken() throws Exception {
assertTrue("session must hold lock token", containsLockToken(superuser, lockToken));

superuser.removeLockToken(lockToken);
assertNull("session must get a null lock token", lock.getLockToken());

String nlt = lock.getLockToken();
assertTrue("freshly obtained lock token must either be null or the same as the one returned earlier",
nlt == null || nlt.equals(lockToken));

// commit
utx.commit();

// refresh Lock Info
lock = n.getLock();

assertNull("session must get a null lock token", lock.getLockToken());
nlt = lock.getLockToken();
assertTrue("freshly obtained lock token must either be null or the same as the one returned earlier",
nlt == null || nlt.equals(lockToken));

Session other = getHelper().getSuperuserSession();
try {
Expand Down Expand Up @@ -1910,15 +1922,20 @@ public void testAddLockTokenRemoveNode() throws Exception {
assertTrue("session must hold lock token", containsLockToken(superuser, lockToken));

superuser.removeLockToken(lockToken);
assertNull("session must get a null lock token", lock.getLockToken());


String nlt = lock.getLockToken();
assertTrue("freshly obtained lock token must either be null or the same as the one returned earlier",
nlt == null || nlt.equals(lockToken));

// commit
utx.commit();

// refresh Lock Info
lock = n.getLock();

assertNull("session must get a null lock token", lock.getLockToken());
nlt = lock.getLockToken();
assertTrue("freshly obtained lock token must either be null or the same as the one returned earlier",
nlt == null || nlt.equals(lockToken));

Session other = getHelper().getSuperuserSession();
// start new Transaction and try to add lock token unlock the node and then remove it
Expand Down
Expand Up @@ -22,7 +22,9 @@
import javax.jcr.AccessDeniedException;
import javax.jcr.Node;
import javax.jcr.RepositoryException;
import javax.jcr.Session;
import javax.jcr.lock.Lock;
import javax.jcr.lock.LockManager;

/** <code>AbstractVersionAccessTest</code>... */
public abstract class AbstractLockManagementTest extends AbstractEvaluationTest {
Expand Down Expand Up @@ -129,4 +131,49 @@ public void testLock4() throws RepositoryException, NotExecutableException {
boolean isLocked = n.isLocked();
assertFalse(isLocked);
}

public void testLockBreaking() throws RepositoryException, NotExecutableException {
String locktoken = null;
LockManager sulm = superuser.getWorkspace().getLockManager();
String lockedpath = null;

try {
Node trn = getTestNode();
modifyPrivileges(trn.getPath(), Privilege.JCR_READ, true);
modifyPrivileges(trn.getPath(), PrivilegeRegistry.REP_WRITE, true);
modifyPrivileges(trn.getPath(), Privilege.JCR_LOCK_MANAGEMENT, true);

Session lockingSession = trn.getSession();

assertFalse("super user and test user should have different user ids: " + lockingSession.getUserID() + " vs " + superuser.getUserID(),
lockingSession.getUserID().equals(superuser.getUserID()));

trn.addNode("locktest", "nt:unstructured");
trn.addMixin("mix:lockable");
lockingSession.save();

// let the "other" user lock the node
LockManager oulm = lockingSession.getWorkspace().getLockManager();
Lock l = oulm.lock(trn.getPath(), true, false, Long.MAX_VALUE, null);
lockedpath = trn.getPath();
locktoken = l.getLockToken();
lockingSession.logout();

// transfer the lock token to the super user and try the unlock

Node lockednode = superuser.getNode(lockedpath);
assertTrue(lockednode.isLocked());
Lock sl = sulm.getLock(lockedpath);
assertNotNull(sl.getLockToken());
sulm.addLockToken(sl.getLockToken());
sulm.unlock(lockedpath);
locktoken = null;
}
finally {
if (locktoken != null && lockedpath != null) {
sulm.addLockToken(locktoken);
sulm.unlock(lockedpath);
}
}
}
}
Expand Up @@ -89,7 +89,10 @@ public void testTokenTransfer() throws Exception {
String lockToken = lock.getLockToken();
try {
superuser.removeLockToken(lockToken);
assertNull("After token transfer lock-token must not be visible", lock.getLockToken());

String nlt = lock.getLockToken();
assertTrue("freshly obtained lock token must either be null or the same as the one returned earlier",
nlt == null || nlt.equals(lockToken));
} finally {
// move lock token back in order to have lock removed properly
superuser.addLockToken(lockToken);
Expand Down
Expand Up @@ -15,12 +15,14 @@
*/
package org.apache.jackrabbit.spi2dav;

import org.apache.jackrabbit.webdav.lock.ActiveLock;
import org.apache.jackrabbit.webdav.DavConstants;
import java.util.Set;

import org.apache.jackrabbit.spi.LockInfo;
import org.apache.jackrabbit.spi.NodeId;
import org.slf4j.LoggerFactory;
import org.apache.jackrabbit.webdav.DavConstants;
import org.apache.jackrabbit.webdav.lock.ActiveLock;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* <code>LockInfoImpl</code>...
Expand All @@ -31,10 +33,12 @@ public class LockInfoImpl implements LockInfo {

private final ActiveLock activeLock;
private final NodeId nodeId;
private final Set<String> sessionLockTokens;

public LockInfoImpl(ActiveLock activeLock, NodeId nodeId) {
public LockInfoImpl(ActiveLock activeLock, NodeId nodeId, Set<String> sessionLockTokens) {
this.activeLock = activeLock;
this.nodeId = nodeId;
this.sessionLockTokens = sessionLockTokens;
}

ActiveLock getActiveLock() {
Expand Down Expand Up @@ -64,7 +68,12 @@ public long getSecondsRemaining() {
}

public boolean isLockOwner() {
return activeLock.getToken() != null;
String lt = activeLock.getToken();
if (lt == null) {
return false;
} else {
return sessionLockTokens.contains(lt);
}
}

public NodeId getNodeId() {
Expand Down
Expand Up @@ -374,9 +374,10 @@ private static boolean isUnLockMethod(DavMethod method) {
protected static void initMethod(HttpMethod method, SessionInfo sessionInfo, boolean addIfHeader) throws RepositoryException {
if (addIfHeader) {
checkSessionInfo(sessionInfo);
String[] locktokens = ((SessionInfoImpl) sessionInfo).getAllLockTokens();
Set<String> allLockTokens = ((SessionInfoImpl) sessionInfo).getAllLockTokens();
// TODO: ev. build tagged if header
if (locktokens != null && locktokens.length > 0) {
if (!allLockTokens.isEmpty()) {
String[] locktokens = allLockTokens.toArray(new String[allLockTokens.size()]);
IfHeader ifH = new IfHeader(locktokens);
method.setRequestHeader(ifH.getHeaderName(), ifH.getHeaderValue());
}
Expand Down Expand Up @@ -1571,7 +1572,9 @@ public void refreshLock(SessionInfo sessionInfo, NodeId nodeId) throws Repositor
String uri = getItemUri(nodeId, sessionInfo);
// since sessionInfo does not allow to retrieve token by NodeId,
// pass all available lock tokens to the LOCK method (TODO: correct?)
LockMethod method = new LockMethod(uri, INFINITE_TIMEOUT, ((SessionInfoImpl) sessionInfo).getAllLockTokens());
Set<String> allLockTokens = ((SessionInfoImpl) sessionInfo).getAllLockTokens();
String[] locktokens = allLockTokens.toArray(new String[allLockTokens.size()]);
LockMethod method = new LockMethod(uri, INFINITE_TIMEOUT, locktokens);
execute(method, sessionInfo);
}

Expand All @@ -1592,6 +1595,10 @@ public void unlock(SessionInfo sessionInfo, NodeId nodeId) throws RepositoryExce
String lockToken = lInfo.getActiveLock().getToken();
boolean isSessionScoped = lInfo.isSessionScoped();

if (!((SessionInfoImpl) sessionInfo).getAllLockTokens().contains(lockToken)) {
throw new LockException("Lock " + lockToken + " not owned by this session");
}

UnLockMethod method = new UnLockMethod(uri, lockToken);
execute(method, sessionInfo);

Expand All @@ -1600,6 +1607,7 @@ public void unlock(SessionInfo sessionInfo, NodeId nodeId) throws RepositoryExce

private LockInfo retrieveLockInfo(LockDiscovery lockDiscovery, SessionInfo sessionInfo,
NodeId nodeId, NodeId parentId) throws RepositoryException {
checkSessionInfo(sessionInfo);
List<ActiveLock> activeLocks = lockDiscovery.getValue();
ActiveLock activeLock = null;
for (ActiveLock l : activeLocks) {
Expand All @@ -1625,7 +1633,7 @@ private LockInfo retrieveLockInfo(LockDiscovery lockDiscovery, SessionInfo sessi
}
// no deep lock or parentID == null or lock is not present on parent
// -> nodeID is lockHolding Id.
return new LockInfoImpl(activeLock, nodeId);
return new LockInfoImpl(activeLock, nodeId, ((SessionInfoImpl)sessionInfo).getAllLockTokens());
}

/**
Expand Down
Expand Up @@ -18,6 +18,7 @@

import org.apache.jackrabbit.spi.commons.conversion.NamePathResolver;

import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
import java.util.Arrays;
Expand Down Expand Up @@ -98,10 +99,10 @@ void setNamePathResolver(NamePathResolver resolver) {
* communication with the DAV server and are never exposed through the
* JCR API for they belong to session-scoped locks.
*/
String[] getAllLockTokens() {
Set<String> getAllLockTokens() {
Set<String> s = new HashSet<String>(Arrays.asList(getLockTokens()));
s.addAll(sessionScopedTokens);
return s.toArray(new String[s.size()]);
return Collections.unmodifiableSet(s);
}

void addLockToken(String token, boolean sessionScoped) {
Expand Down

0 comments on commit 54f743f

Please sign in to comment.