diff --git a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/AccessControlManagerDelegator.java b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/AccessControlManagerDelegator.java index ef72502791..71105cbc63 100644 --- a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/AccessControlManagerDelegator.java +++ b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/AccessControlManagerDelegator.java @@ -19,12 +19,17 @@ package org.apache.jackrabbit.oak.jcr.delegate; +import static java.lang.String.format; + import javax.annotation.Nonnull; import javax.jcr.RepositoryException; +import javax.jcr.UnsupportedRepositoryOperationException; import javax.jcr.security.AccessControlManager; import javax.jcr.security.AccessControlPolicy; import javax.jcr.security.AccessControlPolicyIterator; import javax.jcr.security.Privilege; +import javax.jcr.version.VersionException; +import javax.jcr.version.VersionManager; import org.apache.jackrabbit.oak.jcr.session.operation.SessionOperation; @@ -37,10 +42,12 @@ import org.apache.jackrabbit.oak.jcr.session.operation.SessionOperation; public class AccessControlManagerDelegator implements AccessControlManager { private final SessionDelegate delegate; private final AccessControlManager acManager; + private final VersionManager vManager; - public AccessControlManagerDelegator(SessionDelegate delegate, AccessControlManager acManager) { + public AccessControlManagerDelegator(SessionDelegate delegate, AccessControlManager acManager, VersionManager vManager) { this.acManager = acManager; this.delegate = delegate; + this.vManager = vManager; } @Override @@ -131,6 +138,14 @@ public class AccessControlManagerDelegator implements AccessControlManager { public void performVoid() throws RepositoryException { acManager.setPolicy(absPath, policy); } + + @Override + public void checkPreconditions() throws RepositoryException { + super.checkPreconditions(); + if (!isCheckedOut(absPath)) { + throw new VersionException(format("Cannot set policy. Node [%s] is checked in.", absPath)); + } + } }); } @@ -142,6 +157,24 @@ public class AccessControlManagerDelegator implements AccessControlManager { public void performVoid() throws RepositoryException { acManager.removePolicy(absPath, policy); } + + @Override + public void checkPreconditions() throws RepositoryException { + super.checkPreconditions(); + if (!isCheckedOut(absPath)) { + throw new VersionException(format("Cannot remove policy. Node [%s] is checked in.", absPath)); + } + } }); } + + private boolean isCheckedOut(final String absPath) throws RepositoryException { + try { + return vManager.isCheckedOut(absPath); + } catch (UnsupportedRepositoryOperationException ex) { + // when versioning is not supported all nodes are considered to be + // checked out + return true; + } + } } diff --git a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/JackrabbitAccessControlManagerDelegator.java b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/JackrabbitAccessControlManagerDelegator.java index 2f162cc3ea..d497e610b2 100644 --- a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/JackrabbitAccessControlManagerDelegator.java +++ b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/JackrabbitAccessControlManagerDelegator.java @@ -27,6 +27,7 @@ import javax.jcr.RepositoryException; import javax.jcr.security.AccessControlPolicy; import javax.jcr.security.AccessControlPolicyIterator; import javax.jcr.security.Privilege; +import javax.jcr.version.VersionManager; import org.apache.jackrabbit.api.security.JackrabbitAccessControlManager; import org.apache.jackrabbit.api.security.JackrabbitAccessControlPolicy; @@ -44,10 +45,10 @@ public class JackrabbitAccessControlManagerDelegator implements JackrabbitAccess private final AccessControlManagerDelegator jcrACManager; public JackrabbitAccessControlManagerDelegator(SessionDelegate delegate, - JackrabbitAccessControlManager acManager) { + JackrabbitAccessControlManager acManager, VersionManager vManager) { this.jackrabbitACManager = acManager; this.delegate = delegate; - this.jcrACManager = new AccessControlManagerDelegator(delegate, acManager); + this.jcrACManager = new AccessControlManagerDelegator(delegate, acManager, vManager); } @Override diff --git a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/SessionContext.java b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/SessionContext.java index 24b6fb6022..8ff926aa32 100644 --- a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/SessionContext.java +++ b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/SessionContext.java @@ -239,10 +239,11 @@ public class SessionContext implements NamePathMapper { AccessControlManager acm = getConfig(AuthorizationConfiguration.class) .getAccessControlManager(delegate.getRoot(), namePathMapper); if (acm instanceof JackrabbitAccessControlManager) { - accessControlManager = new JackrabbitAccessControlManagerDelegator( - delegate, (JackrabbitAccessControlManager) acm); + accessControlManager = new JackrabbitAccessControlManagerDelegator(delegate, + (JackrabbitAccessControlManager) acm, getWorkspace().getVersionManager()); } else { - accessControlManager = new AccessControlManagerDelegator(delegate, acm); + accessControlManager = new AccessControlManagerDelegator(delegate, acm, + getWorkspace().getVersionManager()); } } return accessControlManager; diff --git a/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/VersionManagementTest.java b/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/VersionManagementTest.java index 0f156f7cfd..4bb2d74546 100644 --- a/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/VersionManagementTest.java +++ b/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/VersionManagementTest.java @@ -26,9 +26,12 @@ import javax.jcr.security.AccessControlEntry; import javax.jcr.security.AccessControlList; import javax.jcr.security.Privilege; import javax.jcr.version.Version; +import javax.jcr.version.VersionException; import javax.jcr.version.VersionHistory; import javax.jcr.version.VersionManager; +import org.apache.jackrabbit.commons.jackrabbit.authorization.AccessControlUtils; +import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal; import org.apache.jackrabbit.test.NotExecutableException; import org.junit.Before; import org.junit.Test; @@ -424,5 +427,40 @@ public class VersionManagementTest extends AbstractEvaluationTest { // create version versionManager.checkin(nodePath); versionManager.checkout(nodePath); - } + } + + @Test + public void testCheckInCheckoutAcls() throws Exception { + Node t = createVersionableNode(superuser.getNode(path)); + superuser.save(); + + VersionManager versionManager = superuser.getWorkspace().getVersionManager(); + String path = t.getPath(); + + versionManager.checkin(path); + try { + AccessControlUtils.addAccessControlEntry(superuser, path, EveryonePrincipal.getInstance(), + new String[] { Privilege.JCR_ALL }, true); + fail("Not allowed to #addAccessControlEntry on checked-in node [" + path + "]"); + } catch (VersionException ex) { + // expected + } finally { + versionManager.checkout(path); + } + + AccessControlUtils.addAccessControlEntry(superuser, path, EveryonePrincipal.getInstance(), + new String[] { Privilege.JCR_ALL }, true); + superuser.save(); + + versionManager.checkin(path); + try { + AccessControlUtils.clear(superuser, path, EveryonePrincipal.getInstance().getName()); + fail("Not allowed to #clear checked-in node [" + path + "]"); + } catch (VersionException ex) { + // expected + + } finally { + versionManager.checkout(path); + } + } }