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 2021/05/19 14:22:44 UTC
svn commit: r1890029 - in /jackrabbit/oak/trunk/oak-core/src:
main/java/org/apache/jackrabbit/oak/security/principal/
main/java/org/apache/jackrabbit/oak/security/user/
test/java/org/apache/jackrabbit/oak/security/principal/
Author: angela
Date: Wed May 19 14:22:43 2021
New Revision: 1890029
URL: http://svn.apache.org/viewvc?rev=1890029&view=rev
Log:
OAK-9441 : Duplicate code wrt everyone handling in PrincipalProvider implementations
Added:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/principal/EveryoneFilter.java (with props)
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/principal/EveryoneFilterTest.java (with props)
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/principal/PrincipalProviderImpl.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserPrincipalProvider.java
Added: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/principal/EveryoneFilter.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/principal/EveryoneFilter.java?rev=1890029&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/principal/EveryoneFilter.java (added)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/principal/EveryoneFilter.java Wed May 19 14:22:43 2021
@@ -0,0 +1,84 @@
+/*
+ * 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.principal;
+
+import com.google.common.base.Predicate;
+import com.google.common.collect.Iterators;
+import org.apache.jackrabbit.api.security.principal.PrincipalManager;
+import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+
+import java.security.Principal;
+import java.util.Iterator;
+
+public final class EveryoneFilter {
+
+ private EveryoneFilter() {
+ }
+
+ /**
+ * Inject the everyone principal in the result if the given query (defined by name hint and search type) matched
+ * everyone and the search expects the complete set to be return (i.e. no offset and no limit defined).
+ *
+ * @param resultPrincipals The principals returned by the query.
+ * @param nameHint The original name hint as passed to the query.
+ * @param searchType The original search type as passed to the query.
+ * @param offset The offset as passed to the query.
+ * @param limit The limit as passed to the query.
+ * @return The principal iterator with additionally {@link EveryonePrincipal} attached if the given query matches the
+ * everyone principal name or search-type and doesn't specify a range (offset or limit). If the query doesn't match
+ * everyone or if a range was defined the original iterator is returned unmodified.
+ */
+ public static Iterator<Principal> filter(@NotNull Iterator<Principal> resultPrincipals, @Nullable String nameHint, int searchType, long offset, long limit) {
+ boolean noRange = offset == 0 && limit == Long.MAX_VALUE;
+ if (noRange && matchesEveryone(nameHint, searchType)) {
+ Iterator<Principal> principals = Iterators.concat(resultPrincipals, Iterators.singletonIterator(EveryonePrincipal.getInstance()));
+ return Iterators.filter(principals, new EveryonePredicate());
+ } else {
+ return resultPrincipals;
+ }
+ }
+
+ private static boolean matchesEveryone(@Nullable String nameHint, int searchType) {
+ return searchType != PrincipalManager.SEARCH_TYPE_NOT_GROUP &&
+ (nameHint == null || EveryonePrincipal.NAME.contains(nameHint));
+ }
+
+ /**
+ * Predicate to make sure the everyone principal is only included once in
+ * the result set.
+ */
+ private static final class EveryonePredicate implements Predicate<Principal> {
+ private boolean servedEveryone = false;
+
+ @Override
+ public boolean apply(@Nullable Principal principal) {
+ if (principal != null && EveryonePrincipal.NAME.equals(principal.getName())) {
+ if (servedEveryone) {
+ return false;
+ } else {
+ servedEveryone = true;
+ return true;
+ }
+ } else {
+ // not everyone
+ return true;
+ }
+ }
+ }
+}
\ No newline at end of file
Propchange: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/principal/EveryoneFilter.java
------------------------------------------------------------------------------
svn:eol-style = native
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/principal/PrincipalProviderImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/principal/PrincipalProviderImpl.java?rev=1890029&r1=1890028&r2=1890029&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/principal/PrincipalProviderImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/principal/PrincipalProviderImpl.java Wed May 19 14:22:43 2021
@@ -17,10 +17,8 @@
package org.apache.jackrabbit.oak.security.principal;
import com.google.common.base.Function;
-import com.google.common.base.Predicate;
import com.google.common.collect.Iterators;
import org.apache.jackrabbit.api.security.principal.ItemBasedPrincipal;
-import org.apache.jackrabbit.api.security.principal.PrincipalManager;
import org.apache.jackrabbit.api.security.user.Authorizable;
import org.apache.jackrabbit.api.security.user.Query;
import org.apache.jackrabbit.api.security.user.QueryBuilder;
@@ -147,15 +145,7 @@ class PrincipalProviderImpl implements P
try {
Iterator<Authorizable> authorizables = findAuthorizables(nameHint, searchType, offset, limit);
Iterator<Principal> principals = Iterators.filter(Iterators.transform(authorizables, new AuthorizableToPrincipal()), Objects::nonNull);
-
- // everyone is injected only in complete set, not on pages
- boolean noRange = offset == 0 && limit == Long.MAX_VALUE;
- if (noRange && matchesEveryone(nameHint, searchType)) {
- principals = Iterators.concat(principals, Iterators.singletonIterator(EveryonePrincipal.getInstance()));
- return Iterators.filter(principals, new EveryonePredicate());
- } else {
- return principals;
- }
+ return EveryoneFilter.filter(principals, nameHint, searchType, offset, limit);
} catch (RepositoryException e) {
log.debug(e.getMessage());
return Collections.emptyIterator();
@@ -169,7 +159,8 @@ class PrincipalProviderImpl implements P
}
//------------------------------------------------------------< private >---
- private Authorizable getAuthorizable(Principal principal) {
+ @Nullable
+ private Authorizable getAuthorizable(@NotNull Principal principal) {
try {
return userManager.getAuthorizable(principal);
} catch (RepositoryException e) {
@@ -178,7 +169,8 @@ class PrincipalProviderImpl implements P
}
}
- private Set<Principal> getGroupMembership(Authorizable authorizable) {
+ @NotNull
+ private static Set<Principal> getGroupMembership(@NotNull Authorizable authorizable) {
Set<Principal> groupPrincipals = new HashSet<>();
try {
Iterator<org.apache.jackrabbit.api.security.user.Group> groups = authorizable.memberOf();
@@ -195,6 +187,7 @@ class PrincipalProviderImpl implements P
return groupPrincipals;
}
+ @NotNull
private Iterator<Authorizable> findAuthorizables(@Nullable final String nameHint,
final int searchType, final long offset,
final long limit) throws RepositoryException {
@@ -210,7 +203,8 @@ class PrincipalProviderImpl implements P
return userManager.findAuthorizables(userQuery);
}
- private static String buildSearchPattern(String nameHint) {
+ @NotNull
+ private static String buildSearchPattern(@Nullable String nameHint) {
if (nameHint == null) {
return "%";
} else {
@@ -222,12 +216,6 @@ class PrincipalProviderImpl implements P
}
}
-
- private static boolean matchesEveryone(String nameHint, int searchType) {
- return searchType != PrincipalManager.SEARCH_TYPE_NOT_GROUP &&
- (nameHint == null || EveryonePrincipal.NAME.contains(nameHint));
- }
-
//--------------------------------------------------------------------------
/**
* Function to covert an authorizable tree to a principal.
@@ -245,27 +233,4 @@ class PrincipalProviderImpl implements P
return null;
}
}
-
- /**
- * Predicate to make sure the everyone principal is only included once in
- * the result set.
- */
- private static final class EveryonePredicate implements Predicate<Principal> {
- private boolean servedEveryone = false;
- @Override
- public boolean apply(Principal principal) {
- // principal must never be null as the result was already filtered for null.
- if (EveryonePrincipal.NAME.equals(principal.getName())) {
- if (servedEveryone) {
- return false;
- } else {
- servedEveryone = true;
- return true;
- }
- } else {
- // not everyone
- return true;
- }
- }
- }
}
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserPrincipalProvider.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserPrincipalProvider.java?rev=1890029&r1=1890028&r2=1890029&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserPrincipalProvider.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserPrincipalProvider.java Wed May 19 14:22:43 2021
@@ -18,13 +18,11 @@ package org.apache.jackrabbit.oak.securi
import com.google.common.base.Function;
import com.google.common.base.Joiner;
-import com.google.common.base.Predicate;
import com.google.common.base.Predicates;
import com.google.common.base.Strings;
import com.google.common.collect.Iterables;
import com.google.common.collect.Iterators;
import org.apache.jackrabbit.api.security.principal.ItemBasedPrincipal;
-import org.apache.jackrabbit.api.security.principal.PrincipalManager;
import org.apache.jackrabbit.api.security.user.Authorizable;
import org.apache.jackrabbit.api.security.user.UserManager;
import org.apache.jackrabbit.oak.api.CommitFailedException;
@@ -36,6 +34,7 @@ import org.apache.jackrabbit.oak.api.Tre
import org.apache.jackrabbit.oak.commons.LongUtils;
import org.apache.jackrabbit.oak.namepath.NamePathMapper;
import org.apache.jackrabbit.oak.plugins.tree.TreeUtil;
+import org.apache.jackrabbit.oak.security.principal.EveryoneFilter;
import org.apache.jackrabbit.oak.security.user.query.QueryUtil;
import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal;
import org.apache.jackrabbit.oak.spi.security.principal.PrincipalImpl;
@@ -164,8 +163,7 @@ class UserPrincipalProvider implements P
@NotNull
@Override
- public Iterator<? extends Principal> findPrincipals(@Nullable final String nameHint, final boolean fullText, final int searchType, long offset,
- long limit) {
+ public Iterator<? extends Principal> findPrincipals(@Nullable final String nameHint, final boolean fullText, final int searchType, long offset, long limit) {
if (offset < 0) {
offset = 0;
}
@@ -197,13 +195,7 @@ class UserPrincipalProvider implements P
Predicates.notNull());
// everyone is injected only in complete set, not on pages
- boolean noRange = offset == 0 && limit == Long.MAX_VALUE;
- if (noRange && matchesEveryone(nameHint, searchType)) {
- principals = Iterators.concat(principals, Iterators.singletonIterator(EveryonePrincipal.getInstance()));
- return Iterators.filter(principals, new EveryonePredicate());
- } else {
- return principals;
- }
+ return EveryoneFilter.filter(principals, nameHint, searchType, offset, limit);
} catch (ParseException e) {
log.debug(e.getMessage());
return Collections.emptyIterator();
@@ -381,11 +373,6 @@ class UserPrincipalProvider implements P
}
}
- private static boolean matchesEveryone(String nameHint, int searchType) {
- return searchType != PrincipalManager.SEARCH_TYPE_NOT_GROUP &&
- (nameHint == null || EveryonePrincipal.NAME.contains(nameHint));
- }
-
//--------------------------------------------------------------------------
/**
* Function to covert an authorizable tree (as obtained from the query result) to a principal.
@@ -397,28 +384,6 @@ class UserPrincipalProvider implements P
}
}
- /**
- * Predicate to make sure the everyone principal is only included once in
- * the result set.
- */
- private static final class EveryonePredicate implements Predicate<Principal> {
- private boolean servedEveryone = false;
- @Override
- public boolean apply(Principal principal) {
- if (EveryonePrincipal.NAME.equals(principal.getName())) {
- if (servedEveryone) {
- return false;
- } else {
- servedEveryone = true;
- return true;
- }
- } else {
- // not everyone
- return true;
- }
- }
- }
-
//--------------------------------------------------------------------------
// Group Principal implementations that retrieve member information on demand
//--------------------------------------------------------------------------
Added: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/principal/EveryoneFilterTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/principal/EveryoneFilterTest.java?rev=1890029&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/principal/EveryoneFilterTest.java (added)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/principal/EveryoneFilterTest.java Wed May 19 14:22:43 2021
@@ -0,0 +1,107 @@
+/*
+ * 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.principal;
+
+import com.google.common.collect.Iterators;
+import com.google.common.collect.Lists;
+import org.apache.jackrabbit.api.security.principal.PrincipalManager;
+import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal;
+import org.apache.jackrabbit.oak.spi.security.principal.PrincipalImpl;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import java.security.Principal;
+import java.util.Collection;
+import java.util.Iterator;
+
+import static org.junit.Assert.assertEquals;
+
+@RunWith(Parameterized.class)
+public class EveryoneFilterTest {
+
+ @Parameterized.Parameters(name = "searchType={1}")
+ public static Collection<Object[]> parameters() {
+ return Lists.newArrayList(
+ new Object[] { PrincipalManager.SEARCH_TYPE_GROUP , "Group"},
+ new Object[] { PrincipalManager.SEARCH_TYPE_ALL , "All"},
+ new Object[] { PrincipalManager.SEARCH_TYPE_NOT_GROUP , "Not_Group"}
+ );
+ }
+
+ private final int searchType;
+ private final Principal anotherPrincipal = new PrincipalImpl("another");
+
+ public EveryoneFilterTest(int searchType, String name) {
+ this.searchType = searchType;
+ }
+
+ private static int adjustExpectedSize (int searchType, int expectedSize) {
+ // for search-type 'users only' everyone will not get injected if missing
+ return (searchType != PrincipalManager.SEARCH_TYPE_NOT_GROUP) ? expectedSize+1 : expectedSize;
+ }
+
+ @Test
+ public void testEveryoneAlreadyIncluded() {
+ Iterator<Principal> principals = Iterators.forArray(EveryonePrincipal.getInstance(), anotherPrincipal);
+ Iterator<Principal> result = EveryoneFilter.filter(principals, EveryonePrincipal.NAME, searchType, 0, Long.MAX_VALUE);
+
+ assertEquals(2, Iterators.size(result));
+ }
+
+ @Test
+ public void testMissingEveryoneNoRange() {
+ Iterator<Principal> principals = Iterators.singletonIterator(anotherPrincipal);
+ Iterator<Principal> result = EveryoneFilter.filter(principals, EveryonePrincipal.NAME, searchType, 0, Long.MAX_VALUE);
+
+ int expectedSize = adjustExpectedSize(searchType, 1);
+ assertEquals(expectedSize, Iterators.size(result));
+ }
+
+ @Test
+ public void testMissingEveryoneNullHint() {
+ Iterator<Principal> principals = Iterators.forArray(anotherPrincipal);
+ Iterator<Principal> result = EveryoneFilter.filter(principals, null, searchType, 0, Long.MAX_VALUE);
+
+ int expectedSize = adjustExpectedSize(searchType, 1);
+ assertEquals(expectedSize, Iterators.size(result));
+ }
+
+ @Test
+ public void testMissingEveryoneOffset() {
+ Iterator<Principal> principals = Iterators.forArray(anotherPrincipal);
+ Iterator<Principal> result = EveryoneFilter.filter(principals, EveryonePrincipal.NAME, searchType, 1, Long.MAX_VALUE);
+
+ assertEquals(1, Iterators.size(result));
+ }
+
+ @Test
+ public void testMissingEveryoneLimit() {
+ Iterator<Principal> principals = Iterators.forArray();
+ Iterator<Principal> result = EveryoneFilter.filter(principals, EveryonePrincipal.NAME, searchType, 0, 10);
+
+ assertEquals(0, Iterators.size(result));
+ }
+
+ @Test
+ public void testResultContainsNull() {
+ Iterator<Principal> principals = Iterators.forArray(anotherPrincipal, null, EveryonePrincipal.getInstance());
+ Iterator<Principal> result = EveryoneFilter.filter(principals, EveryonePrincipal.NAME, searchType, 0, Long.MAX_VALUE);
+
+ assertEquals(3, Iterators.size(result));
+ }
+}
\ No newline at end of file
Propchange: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/principal/EveryoneFilterTest.java
------------------------------------------------------------------------------
svn:eol-style = native