Return-Path: X-Original-To: apmail-jackrabbit-oak-commits-archive@minotaur.apache.org Delivered-To: apmail-jackrabbit-oak-commits-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 5748A10223 for ; Tue, 5 Nov 2013 16:28:19 +0000 (UTC) Received: (qmail 80330 invoked by uid 500); 5 Nov 2013 16:28:19 -0000 Delivered-To: apmail-jackrabbit-oak-commits-archive@jackrabbit.apache.org Received: (qmail 80260 invoked by uid 500); 5 Nov 2013 16:28:16 -0000 Mailing-List: contact oak-commits-help@jackrabbit.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: oak-dev@jackrabbit.apache.org Delivered-To: mailing list oak-commits@jackrabbit.apache.org Received: (qmail 80246 invoked by uid 99); 5 Nov 2013 16:28:15 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 05 Nov 2013 16:28:15 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=5.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 05 Nov 2013 16:28:13 +0000 Received: from eris.apache.org (localhost [127.0.0.1]) by eris.apache.org (Postfix) with ESMTP id 9DC3D23889E3; Tue, 5 Nov 2013 16:27:53 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r1539047 - in /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak: core/ security/authorization/permission/ spi/security/authorization/permission/ Date: Tue, 05 Nov 2013 16:27:53 -0000 To: oak-commits@jackrabbit.apache.org From: angela@apache.org X-Mailer: svnmailer-1.0.9 Message-Id: <20131105162753.9DC3D23889E3@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org Author: angela Date: Tue Nov 5 16:27:53 2013 New Revision: 1539047 URL: http://svn.apache.org/r1539047 Log: OAK-1144 : Avoid wrapping TreePermission into SecurityContext Removed: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecurityContext.java Modified: 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-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/permission/TreePermission.java 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=1539047&r1=1539046&r2=1539047&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 Tue Nov 5 16:27:53 2013 @@ -16,24 +16,13 @@ */ package org.apache.jackrabbit.oak.core; -import static com.google.common.base.Preconditions.checkNotNull; -import static com.google.common.base.Preconditions.checkState; -import static com.google.common.collect.Iterables.filter; -import static com.google.common.collect.Iterables.size; -import static java.util.Collections.emptyList; -import static org.apache.jackrabbit.oak.api.Type.BOOLEAN; -import static org.apache.jackrabbit.oak.api.Type.NAME; -import static org.apache.jackrabbit.oak.api.Type.NAMES; - import java.io.IOException; import java.io.InputStream; - import javax.annotation.CheckForNull; 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; @@ -41,10 +30,20 @@ import org.apache.jackrabbit.oak.kernel. import org.apache.jackrabbit.oak.kernel.KernelNodeBuilder; import org.apache.jackrabbit.oak.spi.security.Context; import org.apache.jackrabbit.oak.spi.security.authorization.permission.PermissionProvider; +import org.apache.jackrabbit.oak.spi.security.authorization.permission.TreePermission; import org.apache.jackrabbit.oak.spi.state.NodeBuilder; import org.apache.jackrabbit.oak.spi.state.NodeState; import org.apache.jackrabbit.oak.util.LazyValue; +import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkState; +import static com.google.common.collect.Iterables.filter; +import static com.google.common.collect.Iterables.size; +import static java.util.Collections.emptyList; +import static org.apache.jackrabbit.oak.api.Type.BOOLEAN; +import static org.apache.jackrabbit.oak.api.Type.NAME; +import static org.apache.jackrabbit.oak.api.Type.NAMES; + class SecureNodeBuilder implements NodeBuilder, FastCopyMove { /** @@ -82,17 +81,17 @@ class SecureNodeBuilder implements NodeB * Security context of this subtree, updated whenever the base revision * changes. */ - private SecurityContext securityContext = null; // initialized lazily + private TreePermission treePermission = null; // initialized lazily /** - * Possibly outdated reference to the security context of the root + * Possibly outdated reference to the tree permission of the root * builder. Used to detect when the base state (and thus the security * context) of the root builder has changed, and trigger an update of - * the security context of this builder. + * the tree permission of this builder. * - * @see #securityContext + * @see #treePermission */ - private SecurityContext rootContext = null; // initialized lazily + private TreePermission rootPermission = null; // initialized lazily SecureNodeBuilder( @Nonnull NodeBuilder builder, @@ -117,17 +116,17 @@ class SecureNodeBuilder implements NodeB @Override @Nonnull public NodeState getBaseState() { - return new SecureNodeState(builder.getBaseState(), getSecurityContext()); + return new SecureNodeState(builder.getBaseState(), getTreePermission()); } @Override @Nonnull public NodeState getNodeState() { - return new SecureNodeState(builder.getNodeState(), getSecurityContext()); + return new SecureNodeState(builder.getNodeState(), getTreePermission()); } @Override public boolean exists() { - return getSecurityContext().canReadThisNode() && builder.exists(); // TODO: isNew()? + return getTreePermission().canRead() && builder.exists(); // TODO: isNew()? } @Override @@ -142,9 +141,9 @@ class SecureNodeBuilder implements NodeB public void baseChanged() { checkState(parent == null); - securityContext = null; // trigger re-evaluation of the context - rootContext = null; - getSecurityContext(); // sets both securityContext and rootContext + treePermission = null; // trigger re-evaluation of the context + rootPermission = null; + getTreePermission(); // sets both tree permissions and root node permissions } @Override @@ -166,7 +165,7 @@ class SecureNodeBuilder implements NodeB @Override @CheckForNull public PropertyState getProperty(String name) { PropertyState property = builder.getProperty(name); - if (property != null && getSecurityContext().canReadProperty(property)) { + if (property != null && getTreePermission().canRead(property)) { return property; } else { return null; @@ -180,7 +179,7 @@ class SecureNodeBuilder implements NodeB @Override public synchronized long getPropertyCount() { - if (getSecurityContext().canReadAll()) { + if (getTreePermission().canReadAll()) { return builder.getPropertyCount(); } else { return size(filter( @@ -191,9 +190,9 @@ class SecureNodeBuilder implements NodeB @Override @Nonnull public Iterable getProperties() { - if (getSecurityContext().canReadAll()) { + if (getTreePermission().canReadAll()) { return builder.getProperties(); - } else if (getSecurityContext().canReadThisNode()) { // TODO: check DENY_PROPERTIES? + } else if (getTreePermission().canRead()) { // TODO: check DENY_PROPERTIES? return filter( builder.getProperties(), new ReadablePropertyPredicate()); @@ -302,7 +301,7 @@ class SecureNodeBuilder implements NodeB @Override public synchronized long getChildNodeCount(long max) { - if (getSecurityContext().canReadAll()) { + if (getTreePermission().canReadAll()) { return builder.getChildNodeCount(max); } else { return size(getChildNodeNames()); @@ -339,23 +338,24 @@ class SecureNodeBuilder implements NodeB } /** - * Security context of this subtree. + * Permissions of this tree. + * + * @return The permissions for this tree. */ - private SecurityContext getSecurityContext() { - if (securityContext == null - || rootContext != rootBuilder.securityContext) { + private TreePermission getTreePermission() { + if (treePermission == null + || rootPermission != rootBuilder.treePermission) { NodeState base = builder.getBaseState(); if (parent == null) { - securityContext = new SecurityContext( - base, permissionProvider.get(), acContext); - rootContext = securityContext; + ImmutableTree baseTree = new ImmutableTree(base, new TreeTypeProviderImpl(acContext)); + treePermission = permissionProvider.get().getTreePermission(baseTree, TreePermission.EMPTY); + rootPermission = treePermission; } else { - SecurityContext parentContext = parent.getSecurityContext(); - securityContext = parentContext.getChildContext(name, base); - rootContext = parent.rootContext; + treePermission = parent.getTreePermission().getChildPermission(name, base); + rootPermission = parent.rootPermission; } } - return securityContext; + return treePermission; } //------------------------------------------------------< inner classes >--- @@ -366,7 +366,7 @@ class SecureNodeBuilder implements NodeB private class ReadablePropertyPredicate implements Predicate { @Override public boolean apply(@Nonnull PropertyState property) { - return getSecurityContext().canReadProperty(property); + return getTreePermission().canRead(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=1539047&r1=1539046&r2=1539047&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 Tue Nov 5 16:27:53 2013 @@ -26,6 +26,7 @@ import org.apache.jackrabbit.oak.plugins 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.security.authorization.permission.TreePermission; import org.apache.jackrabbit.oak.spi.state.AbstractNodeState; import org.apache.jackrabbit.oak.spi.state.ChildNodeEntry; import org.apache.jackrabbit.oak.spi.state.NodeBuilder; @@ -50,36 +51,35 @@ class SecureNodeState extends AbstractNo private final NodeState state; /** - * Security context of this subtree. + * Tree permissions of this subtree. */ - private final SecurityContext context; + private final TreePermission treePermission; private long childNodeCount = -1; private long propertyCount = -1; - SecureNodeState( - @Nonnull NodeState state, @Nonnull SecurityContext context) { + SecureNodeState(@Nonnull NodeState state, @Nonnull TreePermission treePermission) { this.state = checkNotNull(state); - this.context = checkNotNull(context); + this.treePermission = checkNotNull(treePermission); } - SecureNodeState( - @Nonnull NodeState root, @Nonnull PermissionProvider permissionProvider, - @Nonnull Context acContext) { - this(root, new SecurityContext( - root, checkNotNull(permissionProvider), checkNotNull(acContext))); + SecureNodeState(@Nonnull NodeState root, @Nonnull PermissionProvider permissionProvider, + @Nonnull Context acContext) { + this(root, permissionProvider.getTreePermission( + new ImmutableTree(root, new TreeTypeProviderImpl(acContext)), + TreePermission.EMPTY)); } @Override public boolean exists() { - return context.canReadThisNode(); + return treePermission.canRead(); } @Override @CheckForNull public PropertyState getProperty(String name) { PropertyState property = state.getProperty(name); - if (property != null && context.canReadProperty(property)) { + if (property != null && treePermission.canRead(property)) { return property; } else { return null; @@ -89,7 +89,7 @@ class SecureNodeState extends AbstractNo @Override public synchronized long getPropertyCount() { if (propertyCount == -1) { - if (context.canReadAll()) { + if (treePermission.canReadAll()) { propertyCount = state.getPropertyCount(); } else { propertyCount = count(filter( @@ -102,9 +102,9 @@ class SecureNodeState extends AbstractNo @Override @Nonnull public Iterable getProperties() { - if (context.canReadAll()) { + if (treePermission.canReadAll()) { return state.getProperties(); - } else if (context.canReadThisNode()) { // TODO: check DENY_PROPERTIES? + } else if (treePermission.canRead()) { // TODO: check DENY_PROPERTIES? return filter( state.getProperties(), new ReadablePropertyPredicate()); @@ -116,7 +116,7 @@ class SecureNodeState extends AbstractNo @Override public NodeState getChildNode(@Nonnull String name) { NodeState child = state.getChildNode(checkNotNull(name)); - if (child.exists() && !context.canReadAll()) { + if (child.exists() && !treePermission.canReadAll()) { ChildNodeEntry entry = new MemoryChildNodeEntry(name, child); return new WrapChildEntryFunction().apply(entry).getNodeState(); } else { @@ -128,7 +128,7 @@ class SecureNodeState extends AbstractNo public synchronized long getChildNodeCount(long max) { if (childNodeCount == -1) { long count; - if (context.canReadAll()) { + if (treePermission.canReadAll()) { count = state.getChildNodeCount(max); } else { count = super.getChildNodeCount(max); @@ -143,10 +143,10 @@ class SecureNodeState extends AbstractNo @Override @Nonnull public Iterable getChildNodeEntries() { - if (context.canReadAll()) { + if (treePermission.canReadAll()) { // everything is readable including ac-content -> no secure wrapper needed return state.getChildNodeEntries(); - } else if (context.canReadThisNode()) {// TODO: check DENY_CHILDREN? + } else if (treePermission.canRead()) {// TODO: check DENY_CHILDREN? Iterable readable = transform( state.getChildNodeEntries(), new WrapChildEntryFunction()); @@ -173,7 +173,7 @@ class SecureNodeState extends AbstractNo // different revisions (root states) as long as the path, // the subtree, and any security-related areas like the // permission store are equal for both states. - if (state.equals(that.state) && context.equals(that.context)) { + if (state.equals(that.state) && treePermission.equals(that.treePermission)) { return true; } } @@ -188,7 +188,7 @@ class SecureNodeState extends AbstractNo private class ReadablePropertyPredicate implements Predicate { @Override public boolean apply(@Nonnull PropertyState property) { - return context.canReadProperty(property); + return treePermission.canRead(property); } } @@ -217,11 +217,11 @@ class SecureNodeState extends AbstractNo public ChildNodeEntry apply(@Nonnull ChildNodeEntry input) { String name = input.getName(); NodeState child = input.getNodeState(); - SecurityContext childContext = context.getChildContext(name, child); + TreePermission childContext = treePermission.getChildPermission(name, child); SecureNodeState secureChild = new SecureNodeState(child, childContext); if (child.getChildNodeCount(1) == 0 - && secureChild.context.canReadThisNode() - && secureChild.context.canReadAllProperties()) { + && secureChild.treePermission.canRead() + && secureChild.treePermission.canReadProperties()) { // Since this is an accessible leaf node whose all properties // are readable, we don't need the SecureNodeState wrapper // TODO: A further optimization would be to return the raw Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java?rev=1539047&r1=1539046&r2=1539047&view=diff ============================================================================== --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java (original) +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java Tue Nov 5 16:27:53 2013 @@ -45,11 +45,11 @@ import org.apache.jackrabbit.oak.spi.sec import org.apache.jackrabbit.oak.spi.security.authorization.permission.Permissions; import org.apache.jackrabbit.oak.spi.security.authorization.permission.RepositoryPermission; import org.apache.jackrabbit.oak.spi.security.authorization.permission.TreePermission; -import org.apache.jackrabbit.oak.spi.security.authorization.restriction.RestrictionPattern; import org.apache.jackrabbit.oak.spi.security.authorization.restriction.RestrictionProvider; import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeBits; import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeBitsProvider; import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants; +import org.apache.jackrabbit.oak.spi.state.NodeState; import org.apache.jackrabbit.oak.util.TreeUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -161,7 +161,7 @@ final class CompiledPermissionImpl imple if (VersionConstants.VERSION_STORE_NT_NAMES.contains(ntName) || VersionConstants.NT_ACTIVITY.equals(ntName)) { return new TreePermissionImpl(tree, TreeTypeProvider.TYPE_VERSION, parentPermission); } else { - Tree versionableTree = getVersionableTree(tree); + ImmutableTree versionableTree = getVersionableTree(tree); if (versionableTree == null) { log.warn("Cannot retrieve versionable node for " + tree.getPath()); return TreePermission.EMPTY; @@ -185,8 +185,8 @@ final class CompiledPermissionImpl imple } @Nonnull - private TreePermission getParentPermission(@Nonnull Tree tree, int type) { - List trees = new ArrayList(); + private TreePermission getParentPermission(@Nonnull ImmutableTree tree, int type) { + List trees = new ArrayList(); while (!tree.isRoot()) { tree = tree.getParent(); if (tree.exists()) { @@ -194,7 +194,7 @@ final class CompiledPermissionImpl imple } } TreePermission pp = TreePermission.EMPTY; - for (Tree tr : trees) { + for (ImmutableTree tr : trees) { pp = new TreePermissionImpl(tr, type, pp); } return pp; @@ -377,7 +377,7 @@ final class CompiledPermissionImpl imple } @CheckForNull - private Tree getVersionableTree(@Nonnull Tree versionStoreTree) { + private ImmutableTree getVersionableTree(@Nonnull ImmutableTree versionStoreTree) { String relPath = ""; String versionablePath = null; Tree t = versionStoreTree; @@ -413,7 +413,8 @@ final class CompiledPermissionImpl imple private final class TreePermissionImpl implements TreePermission { - private final Tree tree; + private final ImmutableTree rootTree; + private final ImmutableTree tree; private final TreePermissionImpl parent; private final boolean isAcTree; @@ -425,17 +426,30 @@ final class CompiledPermissionImpl imple private boolean skipped; private ReadStatus readStatus; - private TreePermissionImpl(Tree tree, int treeType, TreePermission parentPermission) { + private TreePermissionImpl(ImmutableTree tree, int treeType, TreePermission parentPermission) { this.tree = tree; if (parentPermission instanceof TreePermissionImpl) { parent = (TreePermissionImpl) parentPermission; + rootTree = parent.rootTree; } else { parent = null; + if (tree.isRoot()) { + rootTree = tree; + } else { + rootTree = root.getTree("/"); + } } readableTree = readPolicy.isReadableTree(tree, parent); isAcTree = (treeType == TreeTypeProvider.TYPE_AC); } + //-------------------------------------------------< TreePermission >--- + @Override + public TreePermission getChildPermission(String childName, NodeState childState) { + ImmutableTree childTree = new ImmutableTree(tree, childName, childState); + return getTreePermission(childTree, this); + } + @Override public boolean canRead() { if (!isAcTree && readableTree) { @@ -501,6 +515,30 @@ final class CompiledPermissionImpl imple return hasPermissions(getIterator(property), permissions, tree.getPath()); } + //---------------------------------------------------------< Object >--- + @Override + public boolean equals(Object object) { + if (object == this) { + return true; + } else if (object instanceof TreePermissionImpl) { + TreePermissionImpl that = (TreePermissionImpl) object; + // TODO: We should be able to do this optimization also across + // different revisions (root states) as long as the path, + // the subtree, and any security-related areas like the + // permission store are equal for both states. + return rootTree.equals(that.rootTree) + && tree.getPath().equals(that.tree.getPath()); + } else { + return false; + } + } + + @Override + public int hashCode() { + return 0; + } + + //--------------------------------------------------------< private >--- private Iterator getIterator(@Nullable PropertyState property) { EntryPredicate predicate = new EntryPredicate(tree, property); return concat(new LazyIterator(this, true, predicate), new LazyIterator(this, false, predicate)); @@ -657,6 +695,4 @@ final class CompiledPermissionImpl imple return false; } } - - } Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/permission/TreePermission.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/permission/TreePermission.java?rev=1539047&r1=1539046&r2=1539047&view=diff ============================================================================== --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/permission/TreePermission.java (original) +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/permission/TreePermission.java Tue Nov 5 16:27:53 2013 @@ -20,12 +20,15 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; import org.apache.jackrabbit.oak.api.PropertyState; +import org.apache.jackrabbit.oak.spi.state.NodeState; /** * TreePermission... TODO */ public interface TreePermission { + TreePermission getChildPermission(String childName, NodeState childState); + boolean canRead(); boolean canRead(@Nonnull PropertyState property); @@ -40,6 +43,11 @@ public interface TreePermission { TreePermission EMPTY = new TreePermission() { @Override + public TreePermission getChildPermission(String childName, NodeState childState) { + return EMPTY; + } + + @Override public boolean canRead() { return false; } @@ -72,6 +80,11 @@ public interface TreePermission { TreePermission ALL = new TreePermission() { @Override + public TreePermission getChildPermission(String childName, NodeState childState) { + return ALL; + } + + @Override public boolean canRead() { return true; }