You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by an...@apache.org on 2013/05/14 12:43:52 UTC
svn commit: r1482290 - in /jackrabbit/oak/trunk:
oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/
oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/
oak-core/src/main/java/org/apache/jackrabbit/o...
Author: angela
Date: Tue May 14 10:43:51 2013
New Revision: 1482290
URL: http://svn.apache.org/r1482290
Log:
OAK-809 : Review remove permissions
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/AccessControlConstants.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/AccessControlContext.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionValidator.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionValidatorProvider.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/permission/Permissions.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/evaluation/Jr2CompatibilityTest.java
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/WriteTest.java
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/AccessControlConstants.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/AccessControlConstants.java?rev=1482290&r1=1482289&r2=1482290&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/AccessControlConstants.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/AccessControlConstants.java Tue May 14 10:43:51 2013
@@ -22,6 +22,7 @@ import java.util.Set;
import com.google.common.collect.ImmutableSet;
import org.apache.jackrabbit.oak.plugins.name.NamespaceConstants;
import org.apache.jackrabbit.oak.plugins.nodetype.NodeTypeConstants;
+import org.apache.jackrabbit.oak.spi.security.authorization.permission.Permissions;
import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants;
/**
@@ -65,17 +66,45 @@ public interface AccessControlConstants
/**
* Configuration parameter to enforce backwards compatible permission
- * validation with respect to user/group creation, modification and removal.
- * As of OAK 1.0 those actions require
- * {@link org.apache.jackrabbit.oak.spi.security.authorization.permission.Permissions#USER_MANAGEMENT USER_MANAGEMENT}
- * permissions while in Jackrabbit 2.0 they were covered by regular item
- * write permissions.
+ * validation with respect to user management and node removal:
*
+ * <ul>
+ * <li>User Management: As of OAK 1.0 creation/removal of user and
+ * groups as well as modification of user/group specific protected properties
+ * requires {@link org.apache.jackrabbit.oak.spi.security.authorization.permission.Permissions#USER_MANAGEMENT USER_MANAGEMENT}
+ * permissions while in Jackrabbit 2.0 they were covered by regular item
+ * write permissions.</li>
+ * <li>Removing Nodes: As of OAK 1.0 removing a node will succeed if the
+ * removal is granted on that specific node irrespective of the permission
+ * granted or denied within the subtree. This contrasts to JR 2.0 where
+ * removal of a node only succeeded if all child items (nodes and properties)
+ * could be removed.</li>
+ * </ul>
+ *
+ * In order to enforce backwards compatible behavior of the listed permissions
+ * above the access control configuration setup needs to contain the
+ * {@code #PARAM_PERMISSIONS_JR2} configuration parameter whose value is
+ * expected to be a comma separated string of permission names for which
+ * backwards compatible behavior should be turned on.<p>
+ *
+ * Currently the following values are respected:
+ * <ul>
+ * <li>"USER_MANAGEMENT" : to avoid enforcing {@link Permissions#USER_MANAGEMENT}
+ * permission.</li>
+ * <li>"REMOVE_NODE" : to enforce permission checks for all items located
+ * in the subtree in case of removal.</li>
+ * </ul>
* @since OAK 1.0
*/
String PARAM_PERMISSIONS_JR2 = "permissionsJr2";
/**
+ * Value of the {@link #PARAM_PERMISSIONS_JR2} configuration parameter that
+ * contains all value entries.
+ */
+ String VALUE_PERMISSIONS_JR2 = Permissions.getString(Permissions.USER_MANAGEMENT | Permissions.REMOVE_NODE);
+
+ /**
* Configuration parameter to enable full read access to regular nodes and
* properties at the specified paths.
*/
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/AccessControlContext.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/AccessControlContext.java?rev=1482290&r1=1482289&r2=1482290&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/AccessControlContext.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/AccessControlContext.java Tue May 14 10:43:51 2013
@@ -24,9 +24,6 @@ import org.apache.jackrabbit.oak.spi.sec
import org.apache.jackrabbit.oak.util.TreeUtil;
import org.apache.jackrabbit.util.Text;
-/**
- * AccessControlContext... TODO
- */
final class AccessControlContext implements Context, AccessControlConstants, PermissionConstants {
private static final Context INSTANCE = new AccessControlContext();
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionValidator.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionValidator.java?rev=1482290&r1=1482289&r2=1482290&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionValidator.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionValidator.java Tue May 14 10:43:51 2013
@@ -16,9 +16,6 @@
*/
package org.apache.jackrabbit.oak.security.authorization.permission;
-import static com.google.common.base.Preconditions.checkNotNull;
-import static org.apache.jackrabbit.oak.api.CommitFailedException.ACCESS;
-
import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
@@ -38,16 +35,15 @@ import org.apache.jackrabbit.oak.spi.sta
import org.apache.jackrabbit.oak.spi.state.NodeStateUtils;
import org.apache.jackrabbit.oak.util.TreeUtil;
+import static com.google.common.base.Preconditions.checkNotNull;
+import static org.apache.jackrabbit.oak.api.CommitFailedException.ACCESS;
+
/**
* Validator implementation that checks for sufficient permission for all
* write operations executed by a given content session.
*/
class PermissionValidator extends DefaultValidator {
- /* TODO
- * - OAK-710: Renaming nodes or Move with same parent are reflected as remove+add -> needs special handling
- */
-
private final Tree parentBefore;
private final Tree parentAfter;
private final PermissionProvider permissionProvider;
@@ -201,7 +197,7 @@ class PermissionValidator extends Defaul
if (provider.getAccessControlContext().definesTree(tree)) {
perm = Permissions.MODIFY_ACCESS_CONTROL;
} else if (provider.getUserContext().definesTree(tree)
- && !provider.jr2Permissions()) {
+ && !provider.requiresJr2Permissions(Permissions.USER_MANAGEMENT)) {
perm = Permissions.USER_MANAGEMENT;
} else {
// FIXME: OAK-710 (identify renaming/move of nodes that only required MODIFY_CHILD_NODE_COLLECTION permission)
@@ -238,7 +234,7 @@ class PermissionValidator extends Defaul
} else if (provider.getAccessControlContext().definesProperty(parent, propertyState)) {
perm = Permissions.MODIFY_ACCESS_CONTROL;
} else if (provider.getUserContext().definesProperty(parent, propertyState)
- && !provider.jr2Permissions()) {
+ && !provider.requiresJr2Permissions(Permissions.USER_MANAGEMENT)) {
perm = Permissions.USER_MANAGEMENT;
} else {
perm = defaultPermission;
@@ -250,12 +246,15 @@ class PermissionValidator extends Defaul
return JcrConstants.JCR_LOCKISDEEP.equals(name) || JcrConstants.JCR_LOCKOWNER.equals(name);
}
- // TODO
- public static boolean noTraverse(long permission, long defaultPermission) {
- return permission == Permissions.MODIFY_ACCESS_CONTROL ||
- permission == Permissions.VERSION_MANAGEMENT ||
- permission == Permissions.REMOVE_NODE ||
- defaultPermission == Permissions.REMOVE_NODE;
+ public boolean noTraverse(long permission, long defaultPermission) {
+ if (defaultPermission == Permissions.REMOVE_NODE && provider.requiresJr2Permissions(Permissions.REMOVE_NODE)) {
+ return false;
+ } else {
+ return permission == Permissions.MODIFY_ACCESS_CONTROL ||
+ permission == Permissions.VERSION_MANAGEMENT ||
+ permission == Permissions.REMOVE_NODE ||
+ defaultPermission == Permissions.REMOVE_NODE;
+ }
}
// TODO
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionValidatorProvider.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionValidatorProvider.java?rev=1482290&r1=1482289&r2=1482290&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionValidatorProvider.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionValidatorProvider.java Tue May 14 10:43:51 2013
@@ -29,6 +29,7 @@ import org.apache.jackrabbit.oak.spi.sec
import org.apache.jackrabbit.oak.spi.security.Context;
import org.apache.jackrabbit.oak.spi.security.SecurityProvider;
import org.apache.jackrabbit.oak.spi.security.authorization.permission.PermissionProvider;
+import org.apache.jackrabbit.oak.spi.security.authorization.permission.Permissions;
import org.apache.jackrabbit.oak.spi.state.NodeState;
/**
@@ -38,7 +39,7 @@ import org.apache.jackrabbit.oak.spi.sta
public class PermissionValidatorProvider extends ValidatorProvider {
private final SecurityProvider securityProvider;
- private final boolean jr2Permissions;
+ private final long jr2Permissions;
private Context acCtx;
private Context userCtx;
@@ -47,7 +48,8 @@ public class PermissionValidatorProvider
this.securityProvider = securityProvider;
ConfigurationParameters params = securityProvider.getAccessControlConfiguration().getConfigurationParameters();
- jr2Permissions = params.getConfigValue(AccessControlConstants.PARAM_PERMISSIONS_JR2, false);
+ String compatValue = params.getConfigValue(AccessControlConstants.PARAM_PERMISSIONS_JR2, null);
+ jr2Permissions = Permissions.getPermissions(params.getConfigValue(AccessControlConstants.PARAM_PERMISSIONS_JR2, compatValue));
}
//--------------------------------------------------< ValidatorProvider >---
@@ -74,8 +76,8 @@ public class PermissionValidatorProvider
return userCtx;
}
- boolean jr2Permissions() {
- return jr2Permissions;
+ boolean requiresJr2Permissions(long permission) {
+ return Permissions.includes(jr2Permissions, permission);
}
private ImmutableTree createTree(NodeState root) {
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/permission/Permissions.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/permission/Permissions.java?rev=1482290&r1=1482289&r2=1482290&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/permission/Permissions.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/permission/Permissions.java Tue May 14 10:43:51 2013
@@ -18,18 +18,21 @@ package org.apache.jackrabbit.oak.spi.se
import java.util.Arrays;
import java.util.HashSet;
+import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Set;
+import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import javax.jcr.Session;
import com.google.common.collect.ImmutableSet;
-import org.apache.jackrabbit.oak.util.TreeLocation;
+import com.google.common.collect.Sets;
import org.apache.jackrabbit.oak.plugins.name.NamespaceConstants;
import org.apache.jackrabbit.oak.plugins.nodetype.NodeTypeConstants;
import org.apache.jackrabbit.oak.plugins.version.VersionConstants;
import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants;
+import org.apache.jackrabbit.oak.util.TreeLocation;
/**
* Provides constants for permissions used in the OAK access evaluation as well
@@ -136,6 +139,21 @@ public final class Permissions {
PERMISSION_NAMES.put(USER_MANAGEMENT, "USER_MANAGEMENT");
}
+ public static Set<String> getNames(long permissions) {
+ if (PERMISSION_NAMES.containsKey(permissions)) {
+ return ImmutableSet.of(PERMISSION_NAMES.get(permissions));
+ } else {
+ Set<String> names = new HashSet<String>();
+ for (Map.Entry<Long, String> entry : PERMISSION_NAMES.entrySet()) {
+ long key = entry.getKey();
+ if ((permissions & key) == key) {
+ names.add(entry.getValue());
+ }
+ }
+ return names;
+ }
+ }
+
public static String getString(long permissions) {
if (PERMISSION_NAMES.containsKey(permissions)) {
return PERMISSION_NAMES.get(permissions);
@@ -202,8 +220,8 @@ public final class Permissions {
*/
public static long getPermissions(String jcrActions, TreeLocation location,
boolean isAccessControlContent) {
- Set<String> actions = new HashSet<String>(Arrays.asList(jcrActions.split(",")));
- int permissions = 0;
+ Set<String> actions = Sets.newHashSet(Arrays.asList(jcrActions.split(",")));
+ long permissions = NO_PERMISSION;
if (actions.remove(Session.ACTION_READ)) {
if (isAccessControlContent) {
permissions |= READ_ACCESS_CONTROL;
@@ -246,11 +264,7 @@ public final class Permissions {
}
}
- for (Map.Entry<Long, String> entry : PERMISSION_NAMES.entrySet()) {
- if (actions.remove(entry.getValue())) {
- permissions |= entry.getKey();
- }
- }
+ permissions |= getPermissions(actions);
if (!actions.isEmpty()) {
throw new IllegalArgumentException("Unknown actions: " + actions);
@@ -258,6 +272,34 @@ public final class Permissions {
return permissions;
}
+ /**
+ * Returns the permissions that correspond the given permission names.
+ *
+ * @param permissionNames A comma separated string of permission names.
+ * @return The permissions.
+ * @throws IllegalArgumentException If the string contains unknown actions
+ * or permission names.
+ */
+ public static long getPermissions(@Nullable String permissionNames) {
+ if (permissionNames == null || permissionNames.isEmpty()) {
+ return NO_PERMISSION;
+ } else {
+ return getPermissions(Sets.newHashSet(Arrays.asList(permissionNames.split(","))));
+ }
+ }
+
+ private static long getPermissions(@Nonnull Set<String> permissionNames) {
+ long permissions = NO_PERMISSION;
+ Iterator<Map.Entry<Long, String>> entryItr = PERMISSION_NAMES.entrySet().iterator();
+ while (entryItr.hasNext() && !permissionNames.isEmpty()) {
+ Map.Entry<Long,String> entry = entryItr.next();
+ if (permissionNames.remove(entry.getValue())) {
+ permissions |= entry.getKey();
+ }
+ }
+ return permissions;
+ }
+
public static long getPermission(@Nullable String path, long defaultPermission) {
long permission;
if (NamespaceConstants.NAMESPACES_PATH.equals(path)) {
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/evaluation/Jr2CompatibilityTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/evaluation/Jr2CompatibilityTest.java?rev=1482290&r1=1482289&r2=1482290&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/evaluation/Jr2CompatibilityTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/evaluation/Jr2CompatibilityTest.java Tue May 14 10:43:51 2013
@@ -18,7 +18,6 @@ package org.apache.jackrabbit.oak.securi
import java.util.Collections;
import java.util.Map;
-
import javax.jcr.security.AccessControlEntry;
import javax.jcr.security.AccessControlManager;
@@ -28,6 +27,7 @@ import org.apache.jackrabbit.api.securit
import org.apache.jackrabbit.api.security.user.User;
import org.apache.jackrabbit.api.security.user.UserManager;
import org.apache.jackrabbit.commons.jackrabbit.authorization.AccessControlUtils;
+import org.apache.jackrabbit.oak.api.CommitFailedException;
import org.apache.jackrabbit.oak.api.Root;
import org.apache.jackrabbit.oak.namepath.NamePathMapper;
import org.apache.jackrabbit.oak.security.authorization.AccessControlConstants;
@@ -36,8 +36,13 @@ import org.apache.jackrabbit.oak.spi.sec
import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants;
import org.junit.After;
import org.junit.Before;
+import org.junit.Ignore;
import org.junit.Test;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
/**
* Test compatibility with Jackrabbit 2.x using the
* {@link AccessControlConstants#PARAM_PERMISSIONS_JR2} configuration parameter.
@@ -77,7 +82,7 @@ public class Jr2CompatibilityTest extend
@Override
protected ConfigurationParameters getSecurityConfigParameters() {
- Map<String, Boolean> map = Collections.singletonMap(AccessControlConstants.PARAM_PERMISSIONS_JR2, Boolean.TRUE);
+ Map<String, String> map = Collections.singletonMap(AccessControlConstants.PARAM_PERMISSIONS_JR2, AccessControlConstants.VALUE_PERMISSIONS_JR2);
ConfigurationParameters acConfig = new ConfigurationParameters(map);
return new ConfigurationParameters(ImmutableMap.of(AccessControlConfiguration.PARAM_ACCESS_CONTROL_OPTIONS, acConfig));
@@ -107,4 +112,52 @@ public class Jr2CompatibilityTest extend
}
}
}
+
+ @Ignore("OAK-781") // FIXME
+ @Test
+ public void testRemoveNodeWithJr2Flag() throws Exception {
+ /* allow READ/WRITE privilege for testUser at 'path' */
+ setupPermission("/a", testPrincipal, true, PrivilegeConstants.JCR_READ, PrivilegeConstants.REP_WRITE);
+ /* deny REMOVE_NODE privilege at subtree. */
+ setupPermission("/a/b", testPrincipal, false, PrivilegeConstants.JCR_REMOVE_NODE);
+
+ Root testRoot = getTestRoot();
+ AccessControlManager acMgr = getAccessControlManager(testRoot);
+ assertTrue(acMgr.hasPrivileges("/a", privilegesFromNames(PrivilegeConstants.REP_WRITE)));
+ assertFalse(acMgr.hasPrivileges("/a/b", privilegesFromNames(PrivilegeConstants.JCR_REMOVE_NODE)));
+
+ // removing the tree must fail
+ try {
+ testRoot.getTree("/a").remove();
+ testRoot.commit();
+ fail();
+ } catch (CommitFailedException e) {
+ // success
+ assertTrue(e.isAccessViolation());
+ }
+ }
+
+ @Ignore("OAK-781") // FIXME
+ @Test
+ public void testRemoveNodeWithJr2Flag2() throws Exception {
+ /* allow READ/WRITE privilege for testUser at 'path' */
+ setupPermission("/a", testPrincipal, true, PrivilegeConstants.JCR_READ, PrivilegeConstants.REP_WRITE);
+ /* deny REP_REMOVE_PROPERTIES privilege at subtree. */
+ setupPermission("/a/b", testPrincipal, false, PrivilegeConstants.REP_REMOVE_PROPERTIES);
+
+ Root testRoot = getTestRoot();
+ AccessControlManager acMgr = getAccessControlManager(testRoot);
+ assertTrue(acMgr.hasPrivileges("/a", privilegesFromNames(PrivilegeConstants.REP_WRITE)));
+ assertFalse(acMgr.hasPrivileges("/a/b", privilegesFromNames(PrivilegeConstants.REP_REMOVE_PROPERTIES)));
+
+ // removing the tree must fail
+ try {
+ testRoot.getTree("/a").remove();
+ testRoot.commit();
+ fail();
+ } catch (CommitFailedException e) {
+ // success
+ assertTrue(e.isAccessViolation());
+ }
+ }
}
\ No newline at end of file
Modified: jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/WriteTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/WriteTest.java?rev=1482290&r1=1482289&r2=1482290&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/WriteTest.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/WriteTest.java Tue May 14 10:43:51 2013
@@ -802,7 +802,7 @@ public class WriteTest extends AbstractE
assertTrue(testSession.nodeExists(subtreePath));
assertTrue(testSession.propertyExists(subtreePath + "/property"));
- // removing the child node succeed even if subtree cannot be removed.
+ // removing the child node succeed even if a property in the subtree cannot be removed.
testSession.getNode(childNPath).remove();
testSession.save();
}