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 md...@apache.org on 2013/07/25 17:51:21 UTC
svn commit: r1507027 - in /jackrabbit/oak/trunk:
oak-core/src/main/java/org/apache/jackrabbit/oak/core/
oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/
Author: mduerig
Date: Thu Jul 25 15:51:20 2013
New Revision: 1507027
URL: http://svn.apache.org/r1507027
Log:
OAK-929: Permission changes not visible on root after refresh
re-acquire security context in SecureNodeBuilder after reset
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/RootImpl.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeBuilder.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeState.java
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/ReadTest.java
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/RootImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/RootImpl.java?rev=1507027&r1=1507026&r2=1507027&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/RootImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/RootImpl.java Thu Jul 25 15:51:20 2013
@@ -18,12 +18,17 @@
*/
package org.apache.jackrabbit.oak.core;
+import static com.google.common.base.Preconditions.checkNotNull;
+import static org.apache.jackrabbit.oak.commons.PathUtils.getName;
+import static org.apache.jackrabbit.oak.commons.PathUtils.getParentPath;
+
import java.io.IOException;
import java.io.InputStream;
import java.security.PrivilegedAction;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
+
import javax.annotation.Nonnull;
import javax.security.auth.Subject;
@@ -60,10 +65,6 @@ import org.apache.jackrabbit.oak.spi.sta
import org.apache.jackrabbit.oak.spi.state.NodeStore;
import org.apache.jackrabbit.oak.spi.state.NodeStoreBranch;
-import static com.google.common.base.Preconditions.checkNotNull;
-import static org.apache.jackrabbit.oak.commons.PathUtils.getName;
-import static org.apache.jackrabbit.oak.commons.PathUtils.getParentPath;
-
public class RootImpl implements Root {
/**
@@ -104,6 +105,11 @@ public class RootImpl implements Root {
private NodeBuilder builder;
/**
+ * Secured builder for the root tree
+ */
+ private SecureNodeBuilder secureBuilder;
+
+ /**
* Sentinel for the next move operation to take place on the this root
*/
private Move lastMove = new Move();
@@ -144,7 +150,7 @@ public class RootImpl implements Root {
branch = this.store.branch();
NodeState root = branch.getHead();
builder = root.builder();
- NodeBuilder secureBuilder = new SecureNodeBuilder(builder, getRootContext(root));
+ secureBuilder = new SecureNodeBuilder(builder, getPermissionProvider(), getAcContext());
rootTree = new MutableTree(this, secureBuilder, lastMove);
}
@@ -377,7 +383,7 @@ public class RootImpl implements Root {
*/
NodeState getSecureBase() {
NodeState root = branch.getBase();
- return new SecureNodeState(root, getRootContext(root));
+ return new SecureNodeState(root, getPermissionProvider(), getAcContext());
}
// TODO better way to determine purge limit. See OAK-175
@@ -400,22 +406,6 @@ public class RootImpl implements Root {
return builder.getNodeState();
}
- /**
- * Secure view of the root node state of the tree including all transient changes
- * at the time of this call.
- *
- * @return secure root node state
- */
- @Nonnull
- private NodeState getSecureRootState() {
- return rootTree.getNodeState();
- }
-
- @Nonnull
- private SecurityContext getRootContext(NodeState root) {
- return new SecurityContext(root, getPermissionProvider(), getAcContext());
- }
-
@Nonnull
private PermissionProvider getPermissionProvider() {
if (permissionProvider == null) {
@@ -437,7 +427,7 @@ public class RootImpl implements Root {
*/
private void reset() {
NodeState root = branch.getHead();
- builder.reset(root);
+ secureBuilder.reset(root);
}
@Nonnull
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeBuilder.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeBuilder.java?rev=1507027&r1=1507026&r2=1507027&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeBuilder.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeBuilder.java Thu Jul 25 15:51:20 2013
@@ -32,48 +32,99 @@ import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import com.google.common.base.Predicate;
-
import org.apache.jackrabbit.oak.api.Blob;
import org.apache.jackrabbit.oak.api.PropertyState;
import org.apache.jackrabbit.oak.api.Type;
+import org.apache.jackrabbit.oak.spi.security.Context;
+import org.apache.jackrabbit.oak.spi.security.authorization.permission.PermissionProvider;
import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
import org.apache.jackrabbit.oak.spi.state.NodeState;
class SecureNodeBuilder implements NodeBuilder {
/**
+ * Root builder, or {@code this} for the root builder itself.
+ */
+ private final SecureNodeBuilder rootBuilder;
+
+ /**
+ * Parent builder, or {@code null} for a root builder.
+ */
+ private final SecureNodeBuilder parent;
+
+ /**
+ * Name of this child node within the parent builder,
+ * or {@code null} for a root builder.
+ */
+ private final String name;
+
+ /**
+ * Permissions provider for evaluating access rights to the underlying raw builder
+ */
+ private final PermissionProvider permissionProvider;
+
+ /**
+ * Access control context for evaluating access rights to the underlying raw builder
+ */
+ private final Context acContext;
+
+ /**
* Underlying node builder.
*/
private final NodeBuilder builder;
/**
- * Security context of this subtree.
+ * Internal revision counter for the base state of this builder. The counter
+ * is incremented in the root builder whenever its base state is reset.
+ * Each builder instance has its own copy of this revision counter for
+ * quickly checking whether its security context needs updating
+ * @see #reset(org.apache.jackrabbit.oak.spi.state.NodeState)
+ * @see #securityContext
*/
- private final SecurityContext context;
+ private long baseRevision;
- SecureNodeBuilder(
- @Nonnull NodeBuilder builder, @Nonnull SecurityContext context) {
+ /**
+ * Security context of this subtree. Use {@link #getSecurityContext()} for obtaining
+ * an up to date security context.
+ */
+ private SecurityContext securityContext;
+
+ SecureNodeBuilder(@Nonnull NodeBuilder builder, @Nonnull PermissionProvider permissionProvider,
+ @Nonnull Context acContext) {
+ this.rootBuilder = this;
+ this.parent = null;
+ this.name = null;
+ this.permissionProvider = checkNotNull(permissionProvider);
+ this.acContext = checkNotNull(acContext);
this.builder = checkNotNull(builder);
- this.context = checkNotNull(context);
+ }
+
+ private SecureNodeBuilder(SecureNodeBuilder parent, String name) {
+ this.rootBuilder = parent.rootBuilder;
+ this.parent = parent;
+ this.name = name;
+ this.permissionProvider = parent.permissionProvider;
+ this.acContext = parent.acContext;
+ this.builder = parent.builder.getChildNode(name);
}
@Override @CheckForNull
public NodeState getBaseState() {
NodeState base = builder.getBaseState();
if (base != null) { // TODO: should use a missing state instead of null
- base = new SecureNodeState(base, context); // TODO: baseContext?
+ base = new SecureNodeState(base, getSecurityContext()); // TODO: baseContext?
}
return base;
}
@Override @Nonnull
public NodeState getNodeState() {
- return new SecureNodeState(builder.getNodeState(), context);
+ return new SecureNodeState(builder.getNodeState(), getSecurityContext());
}
@Override
public boolean exists() {
- return builder.exists() && context.canReadThisNode(); // TODO: isNew()?
+ return builder.exists() && getSecurityContext().canReadThisNode(); // TODO: isNew()?
}
@Override
@@ -89,6 +140,8 @@ class SecureNodeBuilder implements NodeB
@Override
public void reset(@Nonnull NodeState state) throws IllegalStateException {
builder.reset(state); // NOTE: can be dangerous with SecureNodeState
+ baseRevision++;
+ securityContext = null;
}
@Override
@@ -99,7 +152,7 @@ class SecureNodeBuilder implements NodeB
@Override @CheckForNull
public PropertyState getProperty(String name) {
PropertyState property = builder.getProperty(name);
- if (property != null && context.canReadProperty(property)) {
+ if (property != null && getSecurityContext().canReadProperty(property)) {
return property;
} else {
return null;
@@ -113,7 +166,7 @@ class SecureNodeBuilder implements NodeB
@Override
public synchronized long getPropertyCount() {
- if (context.canReadAll()) {
+ if (getSecurityContext().canReadAll()) {
return builder.getPropertyCount();
} else {
return size(filter(
@@ -124,9 +177,9 @@ class SecureNodeBuilder implements NodeB
@Override @Nonnull
public Iterable<? extends PropertyState> getProperties() {
- if (context.canReadAll()) {
+ if (getSecurityContext().canReadAll()) {
return builder.getProperties();
- } else if (context.canReadThisNode()) { // TODO: check DENY_PROPERTIES?
+ } else if (getSecurityContext().canReadThisNode()) { // TODO: check DENY_PROPERTIES?
return filter(
builder.getProperties(),
new ReadablePropertyPredicate());
@@ -219,23 +272,20 @@ class SecureNodeBuilder implements NodeB
@Override @Nonnull
public NodeBuilder setChildNode(@Nonnull String name) {
NodeBuilder child = builder.setChildNode(name);
- return new SecureNodeBuilder(
- child, context.getChildContext(name, child.getBaseState()));
+ return new SecureNodeBuilder(this, name);
}
@Override @Nonnull
public NodeBuilder setChildNode(String name, @Nonnull NodeState nodeState) {
NodeBuilder child = builder.setChildNode(name, nodeState);
- return new SecureNodeBuilder(
- child, context.getChildContext(name, child.getBaseState()));
+ return new SecureNodeBuilder(this, name);
}
@Override
public NodeBuilder getChildNode(@Nonnull String name) {
NodeBuilder child = builder.getChildNode(checkNotNull(name));
- if (child.exists() && !context.canReadAll()) {
- return new SecureNodeBuilder(
- child, context.getChildContext(name, child.getBaseState()));
+ if (child.exists() && !getSecurityContext().canReadAll()) {
+ return new SecureNodeBuilder(this, name);
} else {
return child;
}
@@ -243,7 +293,7 @@ class SecureNodeBuilder implements NodeB
@Override
public synchronized long getChildNodeCount() {
- if (context.canReadAll()) {
+ if (getSecurityContext().canReadAll()) {
return builder.getChildNodeCount();
} else {
return size(getChildNodeNames());
@@ -255,6 +305,22 @@ class SecureNodeBuilder implements NodeB
return builder.createBlob(stream);
}
+ /**
+ * Security context of this subtree. This accessor memoizes the security context
+ * as long as {@link #reset(NodeState)} has not been called.
+ */
+ private SecurityContext getSecurityContext() {
+ if (securityContext == null || rootBuilder.baseRevision != baseRevision) {
+ if (parent == null) {
+ securityContext = new SecurityContext(builder.getNodeState(), permissionProvider, acContext);
+ } else {
+ securityContext = parent.getSecurityContext().getChildContext(name, parent.builder.getChildNode(name).getBaseState());
+ }
+ baseRevision = rootBuilder.baseRevision;
+ }
+ return securityContext;
+ }
+
//------------------------------------------------------< inner classes >---
/**
@@ -263,7 +329,7 @@ class SecureNodeBuilder implements NodeB
private class ReadablePropertyPredicate implements Predicate<PropertyState> {
@Override
public boolean apply(@Nonnull PropertyState property) {
- return context.canReadProperty(property);
+ return getSecurityContext().canReadProperty(property);
}
}
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeState.java?rev=1507027&r1=1507026&r2=1507027&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeState.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeState.java Thu Jul 25 15:51:20 2013
@@ -29,6 +29,8 @@ import com.google.common.base.Predicate;
import org.apache.jackrabbit.oak.api.PropertyState;
import org.apache.jackrabbit.oak.plugins.memory.MemoryChildNodeEntry;
import org.apache.jackrabbit.oak.plugins.memory.MemoryNodeBuilder;
+import org.apache.jackrabbit.oak.spi.security.Context;
+import org.apache.jackrabbit.oak.spi.security.authorization.permission.PermissionProvider;
import org.apache.jackrabbit.oak.spi.state.AbstractNodeState;
import org.apache.jackrabbit.oak.spi.state.ChildNodeEntry;
import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
@@ -62,6 +64,13 @@ class SecureNodeState extends AbstractNo
this.context = checkNotNull(context);
}
+ SecureNodeState(
+ @Nonnull NodeState root, @Nonnull PermissionProvider permissionProvider,
+ @Nonnull Context acContext) {
+ this(root, new SecurityContext(
+ root, checkNotNull(permissionProvider), checkNotNull(acContext)));
+ }
+
@Override
public boolean exists() {
return context.canReadThisNode();
Modified: jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/ReadTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/ReadTest.java?rev=1507027&r1=1507026&r2=1507027&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/ReadTest.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/ReadTest.java Thu Jul 25 15:51:20 2013
@@ -139,6 +139,32 @@ public class ReadTest extends AbstractEv
}
@Test
+ public void testDenyRoot() throws Exception {
+ try {
+ deny("/", readPrivileges);
+ testSession.getRootNode();
+ fail("root should not be accessible");
+ } catch (Exception e) {
+ // expected exception
+ } finally {
+ allow("/", readPrivileges);
+ }
+ }
+
+ @Test
+ public void testDenyPath() throws Exception {
+ try {
+ deny(path, readPrivileges);
+ testSession.getNode(path);
+ fail("nodet should not be accessible");
+ } catch (Exception e) {
+ // expected exception
+ } finally {
+ allow(path, readPrivileges);
+ }
+ }
+
+ @Test
public void testReadDenied() throws Exception {
/* deny READ privilege for testUser at 'path' */
deny(path, readPrivileges);