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 ju...@apache.org on 2013/12/19 21:34:00 UTC

svn commit: r1552419 - in /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol: AccessControlValidator.java AccessControlValidatorProvider.java

Author: jukka
Date: Thu Dec 19 20:33:59 2013
New Revision: 1552419

URL: http://svn.apache.org/r1552419
Log:
OAK-1296: Use TypePredicate instead of NodeType.isNodeType() for NodeState type checks

Use TypePredicate in AccessControlValidator

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidator.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidatorProvider.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidator.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidator.java?rev=1552419&r1=1552418&r2=1552419&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidator.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidator.java Thu Dec 19 20:33:59 2013
@@ -35,7 +35,8 @@ import org.apache.jackrabbit.oak.api.Pro
 import org.apache.jackrabbit.oak.api.Tree;
 import org.apache.jackrabbit.oak.api.Type;
 import org.apache.jackrabbit.oak.core.AbstractTree;
-import org.apache.jackrabbit.oak.plugins.nodetype.ReadOnlyNodeTypeManager;
+import org.apache.jackrabbit.oak.core.ImmutableTree;
+import org.apache.jackrabbit.oak.plugins.nodetype.TypePredicate;
 import org.apache.jackrabbit.oak.spi.commit.DefaultValidator;
 import org.apache.jackrabbit.oak.spi.commit.Validator;
 import org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AccessControlConstants;
@@ -57,25 +58,35 @@ import static org.apache.jackrabbit.oak.
  */
 class AccessControlValidator extends DefaultValidator implements AccessControlConstants {
 
-    private final Tree parentBefore;
-    private final Tree parentAfter;
+    private final ImmutableTree parentAfter;
 
     private final PrivilegeBitsProvider privilegeBitsProvider;
     private final PrivilegeManager privilegeManager;
     private final RestrictionProvider restrictionProvider;
-    private final ReadOnlyNodeTypeManager ntMgr;
 
-    AccessControlValidator(Tree parentBefore, Tree parentAfter,
+    private final TypePredicate isRepoAccessControllable;
+    private final TypePredicate isAccessControllable;
+
+    AccessControlValidator(ImmutableTree parentAfter,
                            PrivilegeManager privilegeManager,
                            PrivilegeBitsProvider privilegeBitsProvider,
-                           RestrictionProvider restrictionProvider,
-                           ReadOnlyNodeTypeManager ntMgr) {
-        this.parentBefore = parentBefore;
+                           RestrictionProvider restrictionProvider) {
         this.parentAfter = parentAfter;
         this.privilegeBitsProvider = privilegeBitsProvider;
         this.privilegeManager = privilegeManager;
         this.restrictionProvider = restrictionProvider;
-        this.ntMgr = ntMgr;
+        this.isRepoAccessControllable = new TypePredicate(parentAfter.getNodeState(), MIX_REP_REPO_ACCESS_CONTROLLABLE);
+        this.isAccessControllable = new TypePredicate(parentAfter.getNodeState(), MIX_REP_ACCESS_CONTROLLABLE);
+    }
+
+    private AccessControlValidator(
+            AccessControlValidator parent, ImmutableTree parentAfter) {
+        this.parentAfter = parentAfter;
+        this.privilegeBitsProvider = parent.privilegeBitsProvider;
+        this.privilegeManager = parent.privilegeManager;
+        this.restrictionProvider = parent.restrictionProvider;
+        this.isRepoAccessControllable = parent.isRepoAccessControllable;
+        this.isAccessControllable = parent.isAccessControllable;
     }
 
     //----------------------------------------------------------< Validator >---
@@ -106,19 +117,18 @@ class AccessControlValidator extends Def
 
     @Override
     public Validator childNodeAdded(String name, NodeState after) throws CommitFailedException {
-        Tree treeAfter = checkNotNull(parentAfter.getChild(name));
+        ImmutableTree treeAfter = checkNotNull(parentAfter.getChild(name));
 
         checkValidTree(parentAfter, treeAfter, after);
-        return new AccessControlValidator(null, treeAfter, privilegeManager, privilegeBitsProvider, restrictionProvider, ntMgr);
+        return new AccessControlValidator(this, treeAfter);
     }
 
     @Override
     public Validator childNodeChanged(String name, NodeState before, NodeState after) throws CommitFailedException {
-        Tree treeBefore = checkNotNull(parentBefore.getChild(name));
-        Tree treeAfter = checkNotNull(parentAfter.getChild(name));
+        ImmutableTree treeAfter = checkNotNull(parentAfter.getChild(name));
 
         checkValidTree(parentAfter, treeAfter, after);
-        return new AccessControlValidator(treeBefore, treeAfter, privilegeManager, privilegeBitsProvider, restrictionProvider, ntMgr);
+        return new AccessControlValidator(this, treeAfter);
     }
 
     @Override
@@ -129,7 +139,7 @@ class AccessControlValidator extends Def
 
     //------------------------------------------------------------< private >---
 
-    private void checkValidTree(Tree parentAfter, Tree treeAfter, NodeState nodeAfter) throws CommitFailedException {
+    private void checkValidTree(ImmutableTree parentAfter, Tree treeAfter, NodeState nodeAfter) throws CommitFailedException {
         if (isPolicy(treeAfter)) {
             checkValidPolicy(parentAfter, treeAfter, nodeAfter);
         } else if (isAccessControlEntry(treeAfter)) {
@@ -155,11 +165,10 @@ class AccessControlValidator extends Def
         }
     }
 
-    private void checkValidPolicy(Tree parent, Tree policyTree, NodeState policyNode) throws CommitFailedException {
-        String mixinType = (REP_REPO_POLICY.equals(policyTree.getName())) ?
-                MIX_REP_REPO_ACCESS_CONTROLLABLE :
-                MIX_REP_ACCESS_CONTROLLABLE;
-        checkValidAccessControlledNode(parent, mixinType);
+    private void checkValidPolicy(ImmutableTree parent, Tree policyTree, NodeState policyNode) throws CommitFailedException {
+        TypePredicate requiredMixin = (REP_REPO_POLICY.equals(policyTree.getName())) ?
+                isRepoAccessControllable : isAccessControllable;
+        checkValidAccessControlledNode(parent, requiredMixin);
 
         Collection<String> validPolicyNames = (parent.isRoot()) ?
                 POLICY_NODE_NAMES :
@@ -182,13 +191,13 @@ class AccessControlValidator extends Def
         }
     }
 
-    private void checkValidAccessControlledNode(Tree accessControlledTree, String requiredMixin) throws CommitFailedException {
+    private void checkValidAccessControlledNode(ImmutableTree accessControlledTree, TypePredicate requiredMixin) throws CommitFailedException {
         if (AC_NODETYPE_NAMES.contains(TreeUtil.getPrimaryTypeName(accessControlledTree))) {
             throw accessViolation(5, "Access control policy within access control content (" + accessControlledTree.getPath() + ')');
         }
 
-        String msg = "Isolated policy node. Parent is not of type " + requiredMixin;
-        if (!ntMgr.isNodeType(accessControlledTree, requiredMixin)) {
+        if (!requiredMixin.apply(accessControlledTree.getNodeState())) {
+            String msg = "Isolated policy node. Parent is not of type " + requiredMixin;
             throw accessViolation(6, msg);
         }
 

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidatorProvider.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidatorProvider.java?rev=1552419&r1=1552418&r2=1552419&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidatorProvider.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidatorProvider.java Thu Dec 19 20:33:59 2013
@@ -20,11 +20,9 @@ import javax.annotation.Nonnull;
 
 import org.apache.jackrabbit.api.security.authorization.PrivilegeManager;
 import org.apache.jackrabbit.oak.api.Root;
-import org.apache.jackrabbit.oak.api.Tree;
 import org.apache.jackrabbit.oak.core.ImmutableRoot;
 import org.apache.jackrabbit.oak.core.ImmutableTree;
 import org.apache.jackrabbit.oak.namepath.NamePathMapper;
-import org.apache.jackrabbit.oak.plugins.nodetype.ReadOnlyNodeTypeManager;
 import org.apache.jackrabbit.oak.spi.commit.Validator;
 import org.apache.jackrabbit.oak.spi.commit.ValidatorProvider;
 import org.apache.jackrabbit.oak.spi.security.SecurityProvider;
@@ -52,17 +50,15 @@ public class AccessControlValidatorProvi
     @Nonnull
     @Override
     public Validator getRootValidator(NodeState before, NodeState after) {
-        Tree rootBefore = new ImmutableTree(before);
-        Tree rootAfter = new ImmutableTree(after);
+        ImmutableTree rootAfter = new ImmutableTree(after);
 
         RestrictionProvider restrictionProvider = getConfig(AuthorizationConfiguration.class).getRestrictionProvider();
 
         Root root = new ImmutableRoot(before);
         PrivilegeManager privilegeManager = getConfig(PrivilegeConfiguration.class).getPrivilegeManager(root, NamePathMapper.DEFAULT);
         PrivilegeBitsProvider privilegeBitsProvider = new PrivilegeBitsProvider(root);
-        ReadOnlyNodeTypeManager ntMgr = ReadOnlyNodeTypeManager.getInstance(before);
 
-        return new AccessControlValidator(rootBefore, rootAfter, privilegeManager, privilegeBitsProvider, restrictionProvider, ntMgr);
+        return new AccessControlValidator(rootAfter, privilegeManager, privilegeBitsProvider, restrictionProvider);
     }
 
     private <T> T getConfig(Class<T> configClass) {



Re: svn commit: r1552419 - in /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol: AccessControlValidator.java AccessControlValidatorProvider.java

Posted by Jukka Zitting <ju...@gmail.com>.
Hi,

On Tue, Jan 14, 2014 at 1:23 PM, Angela Schreiber <an...@adobe.com> wrote:
> if am not mistaken this change introduces causes the following line to
> compare
> different objects with equals:
>
> the following line used to compare String with String but now compares
> String.equals(TypePredicate) in the AccessControlValidator:
>
> line 204        if
> (MIX_REP_REPO_ACCESS_CONTROLLABLE.equals(requiredMixin)) {
>
> i don't know how to get the requiredMixin string out of the predicate.
> so, maybe you can fix that right away?

Good catch! I'll fix it in a moment.

BR,

Jukka Zitting

Re: svn commit: r1552419 - in /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol: AccessControlValidator.java AccessControlValidatorProvider.java

Posted by Angela Schreiber <an...@adobe.com>.
hi jukka

if am not mistaken this change introduces causes the following line to
compare
different objects with equals:

the following line used to compare String with String but now compares
String.equals(TypePredicate) in the AccessControlValidator:

line 204        if 
(MIX_REP_REPO_ACCESS_CONTROLLABLE.equals(requiredMixin)) {

i don't know how to get the requiredMixin string out of the predicate.
so, maybe you can fix that right away?

thanks
angela




On 19/12/13 21:34, "jukka@apache.org" <ju...@apache.org> wrote:

>Author: jukka
>Date: Thu Dec 19 20:33:59 2013
>New Revision: 1552419
>
>URL: http://svn.apache.org/r1552419
>Log:
>OAK-1296: Use TypePredicate instead of NodeType.isNodeType() for
>NodeState type checks
>
>Use TypePredicate in AccessControlValidator
>
>Modified:
>    
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu
>rity/authorization/accesscontrol/AccessControlValidator.java
>    
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu
>rity/authorization/accesscontrol/AccessControlValidatorProvider.java
>
>Modified: 
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu
>rity/authorization/accesscontrol/AccessControlValidator.java
>URL: 
>http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/o
>rg/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessContro
>lValidator.java?rev=1552419&r1=1552418&r2=1552419&view=diff
>==========================================================================
>====
>--- 
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu
>rity/authorization/accesscontrol/AccessControlValidator.java (original)
>+++ 
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu
>rity/authorization/accesscontrol/AccessControlValidator.java Thu Dec 19
>20:33:59 2013
>@@ -35,7 +35,8 @@ import org.apache.jackrabbit.oak.api.Pro
> import org.apache.jackrabbit.oak.api.Tree;
> import org.apache.jackrabbit.oak.api.Type;
> import org.apache.jackrabbit.oak.core.AbstractTree;
>-import 
>org.apache.jackrabbit.oak.plugins.nodetype.ReadOnlyNodeTypeManager;
>+import org.apache.jackrabbit.oak.core.ImmutableTree;
>+import org.apache.jackrabbit.oak.plugins.nodetype.TypePredicate;
> import org.apache.jackrabbit.oak.spi.commit.DefaultValidator;
> import org.apache.jackrabbit.oak.spi.commit.Validator;
> import 
>org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AccessC
>ontrolConstants;
>@@ -57,25 +58,35 @@ import static org.apache.jackrabbit.oak.
>  */
> class AccessControlValidator extends DefaultValidator implements
>AccessControlConstants {
> 
>-    private final Tree parentBefore;
>-    private final Tree parentAfter;
>+    private final ImmutableTree parentAfter;
> 
>     private final PrivilegeBitsProvider privilegeBitsProvider;
>     private final PrivilegeManager privilegeManager;
>     private final RestrictionProvider restrictionProvider;
>-    private final ReadOnlyNodeTypeManager ntMgr;
> 
>-    AccessControlValidator(Tree parentBefore, Tree parentAfter,
>+    private final TypePredicate isRepoAccessControllable;
>+    private final TypePredicate isAccessControllable;
>+
>+    AccessControlValidator(ImmutableTree parentAfter,
>                            PrivilegeManager privilegeManager,
>                            PrivilegeBitsProvider privilegeBitsProvider,
>-                           RestrictionProvider restrictionProvider,
>-                           ReadOnlyNodeTypeManager ntMgr) {
>-        this.parentBefore = parentBefore;
>+                           RestrictionProvider restrictionProvider) {
>         this.parentAfter = parentAfter;
>         this.privilegeBitsProvider = privilegeBitsProvider;
>         this.privilegeManager = privilegeManager;
>         this.restrictionProvider = restrictionProvider;
>-        this.ntMgr = ntMgr;
>+        this.isRepoAccessControllable = new
>TypePredicate(parentAfter.getNodeState(),
>MIX_REP_REPO_ACCESS_CONTROLLABLE);
>+        this.isAccessControllable = new
>TypePredicate(parentAfter.getNodeState(), MIX_REP_ACCESS_CONTROLLABLE);
>+    }
>+
>+    private AccessControlValidator(
>+            AccessControlValidator parent, ImmutableTree parentAfter) {
>+        this.parentAfter = parentAfter;
>+        this.privilegeBitsProvider = parent.privilegeBitsProvider;
>+        this.privilegeManager = parent.privilegeManager;
>+        this.restrictionProvider = parent.restrictionProvider;
>+        this.isRepoAccessControllable = parent.isRepoAccessControllable;
>+        this.isAccessControllable = parent.isAccessControllable;
>     }
> 
>     //----------------------------------------------------------<
>Validator >---
>@@ -106,19 +117,18 @@ class AccessControlValidator extends Def
> 
>     @Override
>     public Validator childNodeAdded(String name, NodeState after) throws
>CommitFailedException {
>-        Tree treeAfter = checkNotNull(parentAfter.getChild(name));
>+        ImmutableTree treeAfter =
>checkNotNull(parentAfter.getChild(name));
> 
>         checkValidTree(parentAfter, treeAfter, after);
>-        return new AccessControlValidator(null, treeAfter,
>privilegeManager, privilegeBitsProvider, restrictionProvider, ntMgr);
>+        return new AccessControlValidator(this, treeAfter);
>     }
> 
>     @Override
>     public Validator childNodeChanged(String name, NodeState before,
>NodeState after) throws CommitFailedException {
>-        Tree treeBefore = checkNotNull(parentBefore.getChild(name));
>-        Tree treeAfter = checkNotNull(parentAfter.getChild(name));
>+        ImmutableTree treeAfter =
>checkNotNull(parentAfter.getChild(name));
> 
>         checkValidTree(parentAfter, treeAfter, after);
>-        return new AccessControlValidator(treeBefore, treeAfter,
>privilegeManager, privilegeBitsProvider, restrictionProvider, ntMgr);
>+        return new AccessControlValidator(this, treeAfter);
>     }
> 
>     @Override
>@@ -129,7 +139,7 @@ class AccessControlValidator extends Def
> 
>     //------------------------------------------------------------<
>private >---
> 
>-    private void checkValidTree(Tree parentAfter, Tree treeAfter,
>NodeState nodeAfter) throws CommitFailedException {
>+    private void checkValidTree(ImmutableTree parentAfter, Tree
>treeAfter, NodeState nodeAfter) throws CommitFailedException {
>         if (isPolicy(treeAfter)) {
>             checkValidPolicy(parentAfter, treeAfter, nodeAfter);
>         } else if (isAccessControlEntry(treeAfter)) {
>@@ -155,11 +165,10 @@ class AccessControlValidator extends Def
>         }
>     }
> 
>-    private void checkValidPolicy(Tree parent, Tree policyTree,
>NodeState policyNode) throws CommitFailedException {
>-        String mixinType =
>(REP_REPO_POLICY.equals(policyTree.getName())) ?
>-                MIX_REP_REPO_ACCESS_CONTROLLABLE :
>-                MIX_REP_ACCESS_CONTROLLABLE;
>-        checkValidAccessControlledNode(parent, mixinType);
>+    private void checkValidPolicy(ImmutableTree parent, Tree policyTree,
>NodeState policyNode) throws CommitFailedException {
>+        TypePredicate requiredMixin =
>(REP_REPO_POLICY.equals(policyTree.getName())) ?
>+                isRepoAccessControllable : isAccessControllable;
>+        checkValidAccessControlledNode(parent, requiredMixin);
> 
>         Collection<String> validPolicyNames = (parent.isRoot()) ?
>                 POLICY_NODE_NAMES :
>@@ -182,13 +191,13 @@ class AccessControlValidator extends Def
>         }
>     }
> 
>-    private void checkValidAccessControlledNode(Tree
>accessControlledTree, String requiredMixin) throws CommitFailedException {
>+    private void checkValidAccessControlledNode(ImmutableTree
>accessControlledTree, TypePredicate requiredMixin) throws
>CommitFailedException {
>         if 
>(AC_NODETYPE_NAMES.contains(TreeUtil.getPrimaryTypeName(accessControlledTr
>ee))) {
>             throw accessViolation(5, "Access control policy within
>access control content (" + accessControlledTree.getPath() + ')');
>         }
> 
>-        String msg = "Isolated policy node. Parent is not of type " +
>requiredMixin;
>-        if (!ntMgr.isNodeType(accessControlledTree, requiredMixin)) {
>+        if (!requiredMixin.apply(accessControlledTree.getNodeState())) {
>+            String msg = "Isolated policy node. Parent is not of type "
>+ requiredMixin;
>             throw accessViolation(6, msg);
>         }
> 
>
>Modified: 
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu
>rity/authorization/accesscontrol/AccessControlValidatorProvider.java
>URL: 
>http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/o
>rg/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessContro
>lValidatorProvider.java?rev=1552419&r1=1552418&r2=1552419&view=diff
>==========================================================================
>====
>--- 
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu
>rity/authorization/accesscontrol/AccessControlValidatorProvider.java
>(original)
>+++ 
>jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/secu
>rity/authorization/accesscontrol/AccessControlValidatorProvider.java Thu
>Dec 19 20:33:59 2013
>@@ -20,11 +20,9 @@ import javax.annotation.Nonnull;
> 
> import org.apache.jackrabbit.api.security.authorization.PrivilegeManager;
> import org.apache.jackrabbit.oak.api.Root;
>-import org.apache.jackrabbit.oak.api.Tree;
> import org.apache.jackrabbit.oak.core.ImmutableRoot;
> import org.apache.jackrabbit.oak.core.ImmutableTree;
> import org.apache.jackrabbit.oak.namepath.NamePathMapper;
>-import 
>org.apache.jackrabbit.oak.plugins.nodetype.ReadOnlyNodeTypeManager;
> import org.apache.jackrabbit.oak.spi.commit.Validator;
> import org.apache.jackrabbit.oak.spi.commit.ValidatorProvider;
> import org.apache.jackrabbit.oak.spi.security.SecurityProvider;
>@@ -52,17 +50,15 @@ public class AccessControlValidatorProvi
>     @Nonnull
>     @Override
>     public Validator getRootValidator(NodeState before, NodeState after)
>{
>-        Tree rootBefore = new ImmutableTree(before);
>-        Tree rootAfter = new ImmutableTree(after);
>+        ImmutableTree rootAfter = new ImmutableTree(after);
> 
>         RestrictionProvider restrictionProvider =
>getConfig(AuthorizationConfiguration.class).getRestrictionProvider();
> 
>         Root root = new ImmutableRoot(before);
>         PrivilegeManager privilegeManager =
>getConfig(PrivilegeConfiguration.class).getPrivilegeManager(root,
>NamePathMapper.DEFAULT);
>         PrivilegeBitsProvider privilegeBitsProvider = new
>PrivilegeBitsProvider(root);
>-        ReadOnlyNodeTypeManager ntMgr =
>ReadOnlyNodeTypeManager.getInstance(before);
> 
>-        return new AccessControlValidator(rootBefore, rootAfter,
>privilegeManager, privilegeBitsProvider, restrictionProvider, ntMgr);
>+        return new AccessControlValidator(rootAfter, privilegeManager,
>privilegeBitsProvider, restrictionProvider);
>     }
> 
>     private <T> T getConfig(Class<T> configClass) {
>
>