You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-dev@jackrabbit.apache.org by Angela Schreiber <an...@adobe.com> on 2014/01/14 19:23:34 UTC
Re: svn commit: r1552419 - in
/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol:
AccessControlValidator.java AccessControlValidatorProvider.java
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) {
>
>
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