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/04 09:06:29 UTC

svn commit: r1793756 - 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: Thu May  4 09:06:28 2017
New Revision: 1793756

URL: http://svn.apache.org/viewvc?rev=1793756&view=rev
Log:
OAK-5882 : Improve coverage for oak.security code in oak-core (wip)
minor improvements to user-query code

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

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/GroupPredicate.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/GroupPredicate.java?rev=1793756&r1=1793755&r2=1793756&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/GroupPredicate.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/query/GroupPredicate.java Thu May  4 09:06:28 2017
@@ -20,7 +20,6 @@ import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Set;
 import javax.annotation.CheckForNull;
-import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import javax.jcr.RepositoryException;
 
@@ -54,35 +53,35 @@ class GroupPredicate implements Predicat
 
     @Override
     public boolean apply(@Nullable Authorizable authorizable) {
-        if (authorizable != null) {
-            try {
-                String id = authorizable.getID();
-                if (memberIds.contains(id)) {
-                    return true;
-                } else {
-                    while (membersIterator.hasNext()) {
-                        String memberId = saveGetId(membersIterator.next());
-                        if (memberId != null) {
-                            memberIds.add(memberId);
-                            if (memberId.equals(id)) {
-                                return true;
-                            }
+        String id = saveGetId(authorizable);
+        if (id != null) {
+            if (memberIds.contains(id)) {
+                return true;
+            } else {
+                // not contained in ids that have already been processed => look
+                // for occurrence in the remaining iterator entries.
+                while (membersIterator.hasNext()) {
+                    String memberId = saveGetId(membersIterator.next());
+                    if (memberId != null) {
+                        memberIds.add(memberId);
+                        if (memberId.equals(id)) {
+                            return true;
                         }
                     }
                 }
-            } catch (RepositoryException e) {
-                log.debug("Cannot determine group membership for {}", authorizable, e);
             }
         }
         return false;
     }
 
     @CheckForNull
-    private String saveGetId(@Nonnull Authorizable authorizable) {
-        try {
-            return authorizable.getID();
-        } catch (RepositoryException e) {
-            log.debug("Error while retrieving ID for authorizable {}", authorizable, e);
+    private String saveGetId(@CheckForNull Authorizable authorizable) {
+        if (authorizable != null) {
+            try {
+                return authorizable.getID();
+            } catch (RepositoryException e) {
+                log.debug("Error while retrieving ID for authorizable {}", authorizable, e);
+            }
         }
         return null;
     }

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=1793756&r1=1793755&r2=1793756&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 Thu May  4 09:06:28 2017
@@ -117,7 +117,7 @@ public class UserQueryManager {
             // 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 groupFilter = new GroupPredicate(userManager,
+            Predicate<Authorizable> groupFilter = new GroupPredicate(userManager,
                     groupId,
                     builder.isDeclaredMembersOnly());
             return ResultIterator.create(builder.getOffset(), builder.getMaxCount(),
@@ -327,7 +327,7 @@ public class UserQueryManager {
      */
     private static final class UniqueResultPredicate implements Predicate<Authorizable> {
 
-        private final Set<String> authorizableIds = new HashSet<String>();
+        private final Set<String> authorizableIds = new HashSet();
 
         @Override
         public boolean apply(@Nullable Authorizable input) {

Added: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/GroupPredicateTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/GroupPredicateTest.java?rev=1793756&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/GroupPredicateTest.java (added)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/query/GroupPredicateTest.java Thu May  4 09:06:28 2017
@@ -0,0 +1,138 @@
+/*
+ * 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 javax.jcr.RepositoryException;
+
+import org.apache.jackrabbit.api.security.user.Authorizable;
+import org.apache.jackrabbit.api.security.user.Group;
+import org.apache.jackrabbit.api.security.user.User;
+import org.apache.jackrabbit.api.security.user.UserManager;
+import org.apache.jackrabbit.oak.AbstractSecurityTest;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.when;
+
+public class GroupPredicateTest extends AbstractSecurityTest {
+
+    private UserManager userManager;
+
+    private User testUser;
+    private Group testMember;
+    private Group testGroup;
+
+    @Override
+    public void before() throws Exception {
+        super.before();
+
+        userManager = getUserManager(root);
+
+        testUser = getTestUser();
+        testMember = userManager.createGroup("testMember");
+        testMember.addMember(testUser);
+
+        testGroup = userManager.createGroup("testGroup");
+        testGroup.addMember(testMember);
+
+        root.commit();
+    }
+
+    @Override
+    public void after() throws Exception {
+        try {
+            if (testMember != null) {
+                testMember.remove();
+                root.commit();
+            }
+            if (testGroup != null) {
+                testGroup.remove();
+                root.commit();
+            }
+            if (root.hasPendingChanges()) {
+                root.commit();
+            }
+        } finally {
+            super.after();
+        }
+    }
+
+    @Test
+    public void testUnknownGroupId() throws Exception {
+        String id = "unknownGroupId";
+        assertNull(userManager.getAuthorizable(id));
+
+        GroupPredicate gp = new GroupPredicate(userManager, id, false);
+        assertFalse(gp.apply(testUser));
+        assertFalse(gp.apply(testGroup));
+        assertFalse(gp.apply(null));
+    }
+
+    @Test
+    public void testUserId() throws Exception {
+        GroupPredicate gp = new GroupPredicate(userManager, testUser.getID(), false);
+        assertFalse(gp.apply(testUser));
+        assertFalse(gp.apply(testGroup));
+        assertFalse(gp.apply(null));
+    }
+
+    @Test
+    public void testDeclaredMembersOnly() throws Exception {
+        GroupPredicate gp = new GroupPredicate(userManager, testGroup.getID(), true);
+        assertTrue(gp.apply(testMember));
+
+        assertFalse(gp.apply(testUser));
+        assertFalse(gp.apply(testGroup));
+        assertFalse(gp.apply(null));
+    }
+
+    @Test
+    public void testInheritedMembers() throws Exception {
+        GroupPredicate gp = new GroupPredicate(userManager, testGroup.getID(), false);
+        assertTrue(gp.apply(testMember));
+        assertTrue(gp.apply(testUser));
+
+        assertFalse(gp.apply(testGroup));
+        assertFalse(gp.apply(null));
+    }
+
+    @Test
+    public void testApplyTwice() throws Exception {
+        GroupPredicate gp = new GroupPredicate(userManager, testGroup.getID(), true);
+        gp.apply(testMember);
+        assertTrue(gp.apply(testMember));
+    }
+
+    @Test
+    public void testApplyTwiceNotMember() throws Exception {
+        GroupPredicate gp = new GroupPredicate(userManager, testGroup.getID(), true);
+        gp.apply(testUser);
+        assertFalse(gp.apply(testUser));
+    }
+
+    @Test
+    public void testGetIdFails() throws Exception {
+        GroupPredicate gp = new GroupPredicate(userManager, testGroup.getID(), true);
+
+        Authorizable a = Mockito.mock(Authorizable.class);
+        when(a.getID()).thenThrow(new RepositoryException());
+        assertFalse(gp.apply(a));
+    }
+}
\ No newline at end of file

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