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 2017/05/30 16:07:11 UTC

svn commit: r1796887 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/security/user/query/ test/java/org/apache/jackrabbit/oak/security/user/query/

Author: angela
Date: Tue May 30 16:07:11 2017
New Revision: 1796887

URL: http://svn.apache.org/viewvc?rev=1796887&view=rev
Log:
OAK-6277 : UserQueryManager: redundant check for colliding bound and offset 
OAK-6278 : UserQueryManager: scope filtering for everyone groupId compares to principal name
OAK-5882 : Improve coverage for oak.security code in oak-core (wip)

Added:
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/XPathConditionVisitorTest.java   (with props)
Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/ResultRowToAuthorizable.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/UserQueryManager.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/UserQueryManagerTest.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/ResultRowToAuthorizable.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/ResultRowToAuthorizable.java?rev=1796887&r1=1796886&r2=1796887&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/ResultRowToAuthorizable.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/ResultRowToAuthorizable.java Tue May 30 16:07:11 2017
@@ -51,30 +51,31 @@ class ResultRowToAuthorizable implements
         this.targetType = (targetType == null || AuthorizableType.AUTHORIZABLE == targetType) ? null : targetType;
     }
 
+    @Nullable
     @Override
-    public Authorizable apply(ResultRow row) {
-        if (row != null) {
-            return getAuthorizable(row.getPath());
-        }
-        return null;
+    public Authorizable apply(@Nullable ResultRow row) {
+        return getAuthorizable(row);
     }
 
     //------------------------------------------------------------< private >---
     @CheckForNull
-    private Authorizable getAuthorizable(String resultPath) {
+    private Authorizable getAuthorizable(@CheckForNull ResultRow row) {
         Authorizable authorizable = null;
-        try {
-            Tree tree = root.getTree(resultPath);
-            AuthorizableType type = UserUtil.getType(tree);
-            while (tree.exists() && !tree.isRoot() && type == null) {
-                tree = tree.getParent();
-                type = UserUtil.getType(tree);
-            }
-            if (tree.exists() && (targetType == null || targetType == type)) {
-                authorizable = userManager.getAuthorizable(tree);
+        if (row != null) {
+            String resultPath = row.getPath();
+            try {
+                Tree tree = root.getTree(resultPath);
+                AuthorizableType type = UserUtil.getType(tree);
+                while (tree.exists() && !tree.isRoot() && type == null) {
+                    tree = tree.getParent();
+                    type = UserUtil.getType(tree);
+                }
+                if (tree.exists() && (targetType == null || targetType == type)) {
+                    authorizable = userManager.getAuthorizable(tree);
+                }
+            } catch (RepositoryException e) {
+                log.debug("Failed to access authorizable " + resultPath);
             }
-        } catch (RepositoryException e) {
-            log.debug("Failed to access authorizable " + resultPath);
         }
         return authorizable;
     }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/UserQueryManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/UserQueryManager.java?rev=1796887&r1=1796886&r2=1796887&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/UserQueryManager.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/UserQueryManager.java Tue May 30 16:07:11 2017
@@ -34,6 +34,7 @@ import com.google.common.base.Strings;
 import com.google.common.collect.Iterators;
 
 import org.apache.jackrabbit.api.security.user.Authorizable;
+import org.apache.jackrabbit.api.security.user.Group;
 import org.apache.jackrabbit.api.security.user.Query;
 import org.apache.jackrabbit.api.security.user.QueryBuilder;
 import org.apache.jackrabbit.oak.api.Result;
@@ -90,38 +91,25 @@ public class UserQueryManager {
         }
 
         String statement = buildXPathStatement(builder);
-
         final String groupId = builder.getGroupID();
-        if (groupId == null || EveryonePrincipal.NAME.equals(groupId)) {
+        if (groupId == null || isEveryone(groupId)) {
             long offset = builder.getOffset();
-            Value bound = builder.getBound();
-
-            if (bound != null && offset > 0) {
-                log.warn("Found bound {} and offset {} in limit. Discarding offset.", builder.getBound(), offset);
-                offset = 0;
-            }
             Iterator<Authorizable> result = findAuthorizables(statement, builder.getMaxCount(), offset, null);
             if (groupId == null) {
                 return result;
             } else {
-                return Iterators.filter(result, new Predicate<Authorizable>() {
-                    @Override
-                    public boolean apply(@Nullable Authorizable authorizable) {
-                        try {
-                            return authorizable != null && !groupId.equals(authorizable.getID());
-                        } catch (RepositoryException e) {
-                            return false;
-                        }
+                return Iterators.filter(result, authorizable -> {
+                    try {
+                        return authorizable != null && !groupId.equals(authorizable.getID());
+                    } catch (RepositoryException e) {
+                        return false;
                     }
                 });
             }
         } else {
-            // filtering by group name included in query -> enforce offset
-            // and limit on the result set.
+            // filtering by group name included in query -> enforce offset and limit on the result set.
             Iterator<Authorizable> result = findAuthorizables(statement, Long.MAX_VALUE, 0, null);
-            Predicate<Authorizable> groupFilter = new GroupPredicate(userManager,
-                    groupId,
-                    builder.isDeclaredMembersOnly());
+            Predicate<Authorizable> groupFilter = new GroupPredicate(userManager, groupId, builder.isDeclaredMembersOnly());
             return ResultIterator.create(builder.getOffset(), builder.getMaxCount(),
                     Iterators.filter(result, groupFilter));
         }
@@ -323,6 +311,16 @@ public class UserQueryManager {
         return UserConstants.GROUP_PROPERTY_NAMES.contains(propName) || UserConstants.USER_PROPERTY_NAMES.contains(propName);
     }
 
+    private boolean isEveryone(@Nonnull String groupId) throws RepositoryException {
+        Group gr = userManager.getAuthorizable(groupId, Group.class);
+        if (gr == null) {
+            // compatibility with original code that didn't check for existence of the group
+            return EveryonePrincipal.NAME.equals(groupId);
+        } else {
+            return EveryonePrincipal.NAME.equals(gr.getPrincipal().getName());
+        }
+    }
+
     /**
      * Predicate asserting that a given user/group is only included once in the
      * result set.

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/UserQueryManagerTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/UserQueryManagerTest.java?rev=1796887&r1=1796886&r2=1796887&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/UserQueryManagerTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/UserQueryManagerTest.java Tue May 30 16:07:11 2017
@@ -16,21 +16,35 @@
  */
 package org.apache.jackrabbit.oak.security.user.query;
 
+import java.security.Principal;
+import java.util.ArrayList;
 import java.util.Iterator;
+import java.util.List;
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+import javax.jcr.RepositoryException;
 import javax.jcr.Value;
 import javax.jcr.ValueFactory;
 
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterators;
 import org.apache.jackrabbit.api.security.user.Authorizable;
+import org.apache.jackrabbit.api.security.user.Group;
+import org.apache.jackrabbit.api.security.user.Query;
+import org.apache.jackrabbit.api.security.user.QueryBuilder;
 import org.apache.jackrabbit.api.security.user.User;
 import org.apache.jackrabbit.oak.AbstractSecurityTest;
 import org.apache.jackrabbit.oak.plugins.value.jcr.ValueFactoryImpl;
 import org.apache.jackrabbit.oak.security.user.UserManagerImpl;
+import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal;
 import org.apache.jackrabbit.oak.spi.security.user.AuthorizableType;
 import org.junit.Before;
 import org.junit.Test;
 
+import static com.google.common.base.Preconditions.checkNotNull;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 
 /**
@@ -43,14 +57,18 @@ public class UserQueryManagerTest extend
     private ValueFactory valueFactory;
     private UserQueryManager queryMgr;
     private User user;
+    private String userId;
     private String propertyName;
 
+    private List<Group> groups = new ArrayList();
+
     @Before
     public void before() throws Exception {
         super.before();
 
         UserManagerImpl userMgr = (UserManagerImpl) getUserManager(root);
         user = getTestUser();
+        userId = user.getID();
         queryMgr = new UserQueryManager(userMgr, namePathMapper, getUserConfiguration().getParameters(), root);
 
         valueFactory = new ValueFactoryImpl(root, namePathMapper);
@@ -61,8 +79,48 @@ public class UserQueryManagerTest extend
 
     @Override
     public void after() throws Exception {
-        getQueryEngineSettings().setFailTraversal(true);
-        super.after();
+        try {
+            getQueryEngineSettings().setFailTraversal(true);
+            for (Group g : groups) {
+                g.remove();
+            }
+            if (root.hasPendingChanges()) {
+                root.commit();
+            }
+        } finally {
+            super.after();
+        }
+    }
+
+    private Group createGroup(@Nullable String id, @Nullable Principal principal) throws RepositoryException {
+        Group g;
+        if (id != null) {
+            if (principal != null) {
+                g = getUserManager(root).createGroup(id, principal, null);
+            } else {
+                g = getUserManager(root).createGroup(id);
+            }
+        } else {
+            checkNotNull(principal);
+            g = getUserManager(root).createGroup(principal);
+        }
+        groups.add(g);
+        return g;
+    }
+
+    private static void assertResultContainsAuthorizables(@Nonnull Iterator<Authorizable> result, Authorizable... expected) throws RepositoryException {
+        switch (expected.length) {
+            case 0:
+                assertFalse(result.hasNext());
+                break;
+            case 1:
+                assertTrue(result.hasNext());
+                assertEquals(expected[0].getID(), result.next().getID());
+                assertFalse(result.hasNext());
+                break;
+            default:
+                assertEquals(ImmutableSet.copyOf(expected), ImmutableSet.copyOf(result));
+        }
     }
 
     /**
@@ -136,4 +194,178 @@ public class UserQueryManagerTest extend
             root.commit();
         }
     }
+
+    @Test
+    public void testQueryMaxCountZero() throws Exception {
+        Query q = new Query() {
+            @Override
+            public <T> void build(QueryBuilder<T> queryBuilder) {
+                queryBuilder.setLimit(0, 0);
+
+            }
+        };
+        assertSame(Iterators.emptyIterator(), queryMgr.findAuthorizables(q));
+    }
+
+    @Test
+    public void testQueryScopeEveryoneNonExisting() throws Exception {
+        Query q = new Query() {
+            @Override
+            public <T> void build(QueryBuilder<T> builder) {
+                builder.setCondition(builder.nameMatches(userId));
+                builder.setScope(EveryonePrincipal.NAME, false);
+            }
+        };
+
+        Iterator<Authorizable> result = queryMgr.findAuthorizables(q);
+        assertResultContainsAuthorizables(result, user);
+    }
+
+    @Test
+    public void testQueryScopeEveryoneFiltersEveryone() throws Exception {
+        Value v = getValueFactory(root).createValue("value");
+
+        Group g = createGroup(null, EveryonePrincipal.getInstance());
+        g.setProperty(propertyName, v);
+        user.setProperty(propertyName, v);
+        root.commit();
+
+        Query q = new Query() {
+            @Override
+            public <T> void build(QueryBuilder<T> builder) {
+                builder.setCondition(builder.eq(propertyName, v));
+                builder.setScope(EveryonePrincipal.NAME, false);
+            }
+        };
+
+        Iterator<Authorizable> result = queryMgr.findAuthorizables(q);
+        assertResultContainsAuthorizables(result, user);
+    }
+
+    @Test
+    public void testQueryScopeEveryoneWithIdDiffersPrincipalName() throws Exception {
+        Value v = getValueFactory(root).createValue("value");
+
+        Group g = createGroup("eGroup", EveryonePrincipal.getInstance());
+        g.setProperty(propertyName, v);
+        user.setProperty(propertyName, v);
+        root.commit();
+
+        Query q = new Query() {
+            @Override
+            public <T> void build(QueryBuilder<T> builder) {
+                builder.setCondition(builder.eq(propertyName, v));
+                builder.setScope("eGroup", false);
+            }
+        };
+
+        Iterator<Authorizable> result = queryMgr.findAuthorizables(q);
+        assertResultContainsAuthorizables(result, user);
+    }
+
+    @Test
+    public void testQueryNoScope() throws Exception {
+        Value v = getValueFactory(root).createValue("value");
+
+        Group g = createGroup(null, EveryonePrincipal.getInstance());
+        g.setProperty(propertyName, v);
+        user.setProperty(propertyName, v);
+        root.commit();
+
+        Query q = new Query() {
+            @Override
+            public <T> void build(QueryBuilder<T> builder) {
+                builder.setCondition(builder.eq(propertyName, v));
+            }
+        };
+
+        Iterator<Authorizable> result = queryMgr.findAuthorizables(q);
+        assertResultContainsAuthorizables(result, user, g);
+    }
+
+    @Test
+    public void testQueryScopeNotMember() throws Exception {
+        Value v = getValueFactory(root).createValue("value");
+
+        Group g = createGroup("g1", null);
+        user.setProperty(propertyName, v);
+        root.commit();
+
+        Query q = new Query() {
+            @Override
+            public <T> void build(QueryBuilder<T> builder) {
+                builder.setCondition(builder.eq(propertyName, v));
+                builder.setScope("g1", false);
+            }
+        };
+
+        Iterator<Authorizable> result = queryMgr.findAuthorizables(q);
+        assertResultContainsAuthorizables(result);
+    }
+
+    @Test
+    public void testQueryScopeDeclaredMember() throws Exception {
+        Value v = getValueFactory(root).createValue("value");
+
+        Group g = createGroup("g1", null);
+        g.addMember(user);
+        user.setProperty(propertyName, v);
+        root.commit();
+
+        Query q = new Query() {
+            @Override
+            public <T> void build(QueryBuilder<T> builder) {
+                builder.setCondition(builder.eq(propertyName, v));
+                builder.setScope("g1", false);
+            }
+        };
+
+        Iterator<Authorizable> result = queryMgr.findAuthorizables(q);
+        assertResultContainsAuthorizables(result, user);
+    }
+
+    @Test
+    public void testQueryScopeDeclaredMembership() throws Exception {
+        Value v = getValueFactory(root).createValue("value");
+
+        Group g = createGroup("g1", null);
+        Group g2 = createGroup("g2", null);
+        g.addMember(g2);
+        g2.addMember(user);
+        user.setProperty(propertyName, v);
+        root.commit();
+
+        Query q = new Query() {
+            @Override
+            public <T> void build(QueryBuilder<T> builder) {
+                builder.setCondition(builder.eq(propertyName, v));
+                builder.setScope("g1", true);
+            }
+        };
+
+        Iterator<Authorizable> result = queryMgr.findAuthorizables(q);
+        assertResultContainsAuthorizables(result);
+    }
+
+    @Test
+    public void testQueryScopeInheritedMembership() throws Exception {
+        Value v = getValueFactory(root).createValue("value");
+
+        Group g = createGroup("g1", null);
+        Group g2 = createGroup("g2", null);
+        g.addMember(g2);
+        g2.addMember(user);
+        user.setProperty(propertyName, v);
+        root.commit();
+
+        Query q = new Query() {
+            @Override
+            public <T> void build(QueryBuilder<T> builder) {
+                builder.setCondition(builder.eq(propertyName, v));
+                builder.setScope("g1", false);
+            }
+        };
+        Iterator<Authorizable> result = queryMgr.findAuthorizables(q);
+        assertResultContainsAuthorizables(result, user);
+    }
 }
\ No newline at end of file

Added: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/XPathConditionVisitorTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/XPathConditionVisitorTest.java?rev=1796887&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/XPathConditionVisitorTest.java (added)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/XPathConditionVisitorTest.java Tue May 30 16:07:11 2017
@@ -0,0 +1,206 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.security.user.query;
+
+import java.util.Iterator;
+import java.util.Map;
+import javax.annotation.Nonnull;
+import javax.jcr.Value;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Iterators;
+import org.apache.jackrabbit.oak.AbstractSecurityTest;
+import org.apache.jackrabbit.oak.commons.QueryUtils;
+import org.apache.jackrabbit.oak.namepath.LocalNameMapper;
+import org.apache.jackrabbit.oak.namepath.NamePathMapper;
+import org.apache.jackrabbit.oak.namepath.NamePathMapperImpl;
+import org.apache.jackrabbit.oak.spi.security.user.UserConstants;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public class XPathConditionVisitorTest extends AbstractSecurityTest {
+
+    private static final Map<String, String> LOCAL = ImmutableMap.of("rcj", "http://www.jcp.org/jcr/1.0");
+
+    private static final String REL_PATH = "r'e/l/path";
+    private static final String SERACH_EXPR = "s%e\\%arch\\E[:]xpr";
+
+    private StringBuilder statement;
+    private XPathConditionVisitor visitor;
+    private Condition.Contains testCondition;
+
+    @Override
+    public void before() throws Exception {
+        super.before();
+
+        statement = new StringBuilder();
+        visitor = new XPathConditionVisitor(statement, getNamePathMapper(), getUserManager(root));
+        testCondition = new Condition.Contains(REL_PATH, SERACH_EXPR);
+    }
+
+    @Override
+    protected NamePathMapper getNamePathMapper() {
+        return new NamePathMapperImpl(new LocalNameMapper(root, LOCAL));
+    }
+
+    private static void reduceCompoundConditionToSingleTerm(@Nonnull Condition.Compound condition) {
+        Iterator<Condition> it = condition.iterator();
+        if (it.hasNext()) {
+            it.next();
+            it.remove();
+        }
+        assertEquals(1, Iterators.size(condition.iterator()));
+    }
+
+    @Test
+    public void testVisitNode() throws Exception {
+        visitor.visit(new Condition.Node(SERACH_EXPR));
+
+        String s = statement.toString();
+        assertFalse(s.contains(SERACH_EXPR));
+        assertTrue(s.contains(QueryUtils.escapeForQuery(SERACH_EXPR)));
+        assertTrue(s.contains(QueryUtils.escapeForQuery(QueryUtils.escapeNodeName(SERACH_EXPR))));
+
+    }
+
+    @Test
+    public void testVisitProperty() throws Exception {
+        Value v = getValueFactory(root).createValue(SERACH_EXPR);
+        for (RelationOp op : RelationOp.values()) {
+            if (op == RelationOp.EX || op == RelationOp.LIKE) {
+                continue;
+            }
+
+            visitor.visit(new Condition.Property(REL_PATH, op, v));
+
+            String s = statement.toString();
+            String expected = QueryUtils.escapeForQuery(REL_PATH) + op.getOp() + QueryUtil.format(v);
+
+            assertEquals(expected, s);
+
+            // reset statement for next operation
+            statement.delete(0, statement.length());
+        }
+    }
+
+    @Test
+    public void testVisitPropertyExists() throws Exception {
+        visitor.visit((Condition.Property) new XPathQueryBuilder().exists(REL_PATH));
+
+        assertEquals(QueryUtils.escapeForQuery(REL_PATH), statement.toString());
+    }
+
+    @Test
+    public void testVisitPropertyLike() throws Exception {
+        visitor.visit((Condition.Property) new XPathQueryBuilder().like(REL_PATH, SERACH_EXPR));
+
+        String s = statement.toString();
+        assertTrue(s.startsWith("jcr:like("));
+        assertTrue(s.endsWith(")"));
+
+        assertFalse(s.contains(REL_PATH));
+        assertTrue(s.contains(QueryUtils.escapeForQuery(REL_PATH)));
+
+        assertFalse(s.contains(SERACH_EXPR));
+        assertTrue(s.contains(QueryUtils.escapeForQuery(SERACH_EXPR)));
+    }
+
+    @Test
+    public void testVisitContains() {
+        visitor.visit(testCondition);
+
+        String s = statement.toString();
+        assertTrue(s.startsWith("jcr:contains("));
+        assertTrue(s.endsWith(")"));
+
+        assertFalse(s.contains(REL_PATH));
+        assertTrue(s.contains(QueryUtils.escapeForQuery(REL_PATH)));
+
+        assertFalse(s.contains(SERACH_EXPR));
+        assertTrue(s.contains(QueryUtils.escapeForQuery(SERACH_EXPR)));
+    }
+
+    @Test
+    public void testVisitImpersonation() throws Exception {
+        String principalName = getTestUser().getPrincipal().getName();;
+        Condition.Impersonation c = new Condition.Impersonation(principalName);
+        visitor.visit(c);
+
+        String s = statement.toString();
+        assertTrue(s.contains(UserConstants.REP_IMPERSONATORS));
+        assertFalse(s.contains("@rcj:primaryType='" + UserConstants.NT_REP_USER + "'"));
+    }
+
+    @Test
+    public void testVisitImpersonationAdmin() throws Exception {
+        String adminPrincipalName = getUserManager(root).getAuthorizable(getUserConfiguration().getParameters().getConfigValue(UserConstants.PARAM_ADMIN_ID, UserConstants.DEFAULT_ADMIN_ID)).getPrincipal().getName();
+        Condition.Impersonation c = new Condition.Impersonation(adminPrincipalName);
+        visitor.visit(c);
+
+        String s = statement.toString();
+        assertFalse(s.contains(UserConstants.REP_IMPERSONATORS));
+        assertTrue(s.contains("@rcj:primaryType='" + UserConstants.NT_REP_USER + "'"));
+    }
+
+    @Test
+    public void testVisitNot() throws Exception {
+        visitor.visit(new Condition.Not(testCondition));
+        assertTrue(statement.toString().startsWith("not("));
+        assertTrue(statement.toString().endsWith(")"));
+    }
+
+    @Test
+    public void testVisitAnd() throws Exception {
+        visitor.visit(new Condition.And(testCondition, testCondition));
+        assertTrue(statement.toString().contains(" and "));
+    }
+
+
+    @Test
+    public void testVisitAndSingle() throws Exception {
+        Condition.And c = new Condition.And(testCondition, testCondition);
+        reduceCompoundConditionToSingleTerm(c);
+
+        visitor.visit(c);
+        assertFalse(statement.toString().contains(" and "));
+    }
+
+    @Test
+    public void testVisitOr() throws Exception {
+        visitor.visit(new Condition.Or(testCondition, testCondition));
+
+        String s = statement.toString();
+        assertTrue(s.contains(" or "));
+        assertTrue(s.startsWith("("));
+        assertTrue(s.endsWith("))"));
+    }
+
+    @Test
+    public void testVisitOrSingle() throws Exception {
+        Condition.Or c = new Condition.Or(testCondition, testCondition);
+        reduceCompoundConditionToSingleTerm(c);
+        visitor.visit(c);
+
+        String s = statement.toString();
+        assertFalse(s.contains(" or "));
+        assertFalse(s.startsWith("("));
+        assertFalse(s.endsWith("))"));
+    }
+}
\ No newline at end of file

Propchange: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/XPathConditionVisitorTest.java
------------------------------------------------------------------------------
    svn:eol-style = native