You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by re...@apache.org on 2012/01/12 19:15:28 UTC

svn commit: r1230681 - in /jackrabbit/trunk: jackrabbit-core/src/main/java/org/apache/jackrabbit/core/lock/ jackrabbit-core/src/test/java/org/apache/jackrabbit/core/ jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/ jackr...

Author: reschke
Date: Thu Jan 12 18:15:28 2012
New Revision: 1230681

URL: http://svn.apache.org/viewvc?rev=1230681&view=rev
Log:
JCR-2859: Make open scoped locks recoverable

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

Modified:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/lock/LockImpl.java
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/XATest.java
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractLockManagementTest.java
    jackrabbit/trunk/jackrabbit-jcr2spi/src/test/java/org/apache/jackrabbit/jcr2spi/lock/OpenScopedLockTest.java
    jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2dav/LockInfoImpl.java
    jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2dav/RepositoryServiceImpl.java
    jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2dav/SessionInfoImpl.java

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/lock/LockImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/lock/LockImpl.java?rev=1230681&r1=1230680&r2=1230681&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/lock/LockImpl.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/lock/LockImpl.java Thu Jan 12 18:15:28 2012
@@ -22,6 +22,7 @@ import org.apache.jackrabbit.core.securi
 
 import javax.jcr.Node;
 import javax.jcr.RepositoryException;
+import javax.jcr.Session;
 import javax.jcr.lock.LockException;
 
 /**
@@ -78,7 +79,7 @@ class LockImpl implements javax.jcr.lock
      * {@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;
@@ -151,4 +152,15 @@ class LockImpl implements javax.jcr.lock
         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());
+        }
+    }
 }

Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/XATest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/XATest.java?rev=1230681&r1=1230680&r2=1230681&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/XATest.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/XATest.java Thu Jan 12 18:15:28 2012
@@ -941,14 +941,21 @@ public class XATest extends AbstractJCRT
         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);
@@ -1141,7 +1148,10 @@ public class XATest extends AbstractJCRT
         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();
@@ -1149,7 +1159,9 @@ public class XATest extends AbstractJCRT
         // 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 {
@@ -1910,15 +1922,20 @@ public class XATest extends AbstractJCRT
         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

Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractLockManagementTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractLockManagementTest.java?rev=1230681&r1=1230680&r2=1230681&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractLockManagementTest.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractLockManagementTest.java Thu Jan 12 18:15:28 2012
@@ -22,7 +22,9 @@ import org.apache.jackrabbit.test.NotExe
 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 {
@@ -129,4 +131,49 @@ public abstract class AbstractLockManage
         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);
+            }
+        }
+    }
 }

Modified: jackrabbit/trunk/jackrabbit-jcr2spi/src/test/java/org/apache/jackrabbit/jcr2spi/lock/OpenScopedLockTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-jcr2spi/src/test/java/org/apache/jackrabbit/jcr2spi/lock/OpenScopedLockTest.java?rev=1230681&r1=1230680&r2=1230681&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-jcr2spi/src/test/java/org/apache/jackrabbit/jcr2spi/lock/OpenScopedLockTest.java (original)
+++ jackrabbit/trunk/jackrabbit-jcr2spi/src/test/java/org/apache/jackrabbit/jcr2spi/lock/OpenScopedLockTest.java Thu Jan 12 18:15:28 2012
@@ -89,7 +89,10 @@ public class OpenScopedLockTest extends 
         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);

Modified: jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2dav/LockInfoImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2dav/LockInfoImpl.java?rev=1230681&r1=1230680&r2=1230681&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2dav/LockInfoImpl.java (original)
+++ jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2dav/LockInfoImpl.java Thu Jan 12 18:15:28 2012
@@ -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>...
@@ -31,10 +33,12 @@ public class LockInfoImpl implements Loc
 
     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() {
@@ -64,7 +68,12 @@ public class LockInfoImpl implements Loc
     }
 
     public boolean isLockOwner() {
-        return activeLock.getToken() != null;
+        String lt = activeLock.getToken();
+        if (lt == null) {
+            return false;
+        } else {
+            return sessionLockTokens.contains(lt);
+        }
     }
 
     public NodeId getNodeId() {

Modified: jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2dav/RepositoryServiceImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2dav/RepositoryServiceImpl.java?rev=1230681&r1=1230680&r2=1230681&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2dav/RepositoryServiceImpl.java (original)
+++ jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2dav/RepositoryServiceImpl.java Thu Jan 12 18:15:28 2012
@@ -374,9 +374,10 @@ public class RepositoryServiceImpl imple
     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());
             }
@@ -1571,7 +1572,9 @@ public class RepositoryServiceImpl imple
         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);
     }
 
@@ -1592,6 +1595,10 @@ public class RepositoryServiceImpl imple
         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);
 
@@ -1600,6 +1607,7 @@ public class RepositoryServiceImpl imple
 
     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) {
@@ -1625,7 +1633,7 @@ public class RepositoryServiceImpl imple
         }
         // 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());
     }
 
     /**

Modified: jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2dav/SessionInfoImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2dav/SessionInfoImpl.java?rev=1230681&r1=1230680&r2=1230681&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2dav/SessionInfoImpl.java (original)
+++ jackrabbit/trunk/jackrabbit-spi2dav/src/main/java/org/apache/jackrabbit/spi2dav/SessionInfoImpl.java Thu Jan 12 18:15:28 2012
@@ -18,6 +18,7 @@ package org.apache.jackrabbit.spi2dav;
 
 import org.apache.jackrabbit.spi.commons.conversion.NamePathResolver;
 
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.Set;
 import java.util.Arrays;
@@ -98,10 +99,10 @@ public class SessionInfoImpl extends org
      * 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) {