Author: angela
Date: Fri Jan 8 13:09:33 2021
New Revision: 1885272
URL: http://svn.apache.org/viewvc?rev=1885272&view=rev
Log:
OAK-9316 : Reduce complexity in ExternalGroupPrincipalProvider
Modified:
jackrabbit/oak/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/ExternalGroupPrincipalProvider.java
Modified:
jackrabbit/oak/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/ExternalGroupPrincipalProvider.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/ExternalGroupPrincipalProvider.java?rev=1885272&r1=1885271&r2=1885272&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/ExternalGroupPrincipalProvider.java
(original)
+++
jackrabbit/oak/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/ExternalGroupPrincipalProvider.java
Fri Jan 8 13:09:33 2021
@@ -16,34 +16,12 @@
*/
package
org.apache.jackrabbit.oak.spi.security.authentication.external.impl.principal;
-import java.security.Principal;
-import java.text.ParseException;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.Comparator;
-import java.util.Enumeration;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.Map;
-import java.util.Set;
-import java.util.Spliterator;
-import java.util.Spliterators;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.stream.Stream;
-import java.util.stream.StreamSupport;
-
-import javax.jcr.PropertyType;
-import javax.jcr.RepositoryException;
-import javax.jcr.Value;
-import javax.jcr.query.Query;
-
import com.google.common.base.Predicates;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Iterators;
import com.google.common.collect.Sets;
-
import org.apache.jackrabbit.api.security.principal.GroupPrincipal;
import org.apache.jackrabbit.api.security.principal.ItemBasedPrincipal;
import org.apache.jackrabbit.api.security.principal.PrincipalManager;
@@ -74,6 +52,26 @@ import org.jetbrains.annotations.Nullabl
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import javax.jcr.PropertyType;
+import javax.jcr.RepositoryException;
+import javax.jcr.Value;
+import javax.jcr.query.Query;
+import java.security.Principal;
+import java.text.ParseException;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.Enumeration;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Set;
+import java.util.Spliterator;
+import java.util.Spliterators;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.stream.Stream;
+import java.util.stream.StreamSupport;
+
/**
* Implementation of the {@code PrincipalProvider} interface that exposes
* 'external' principals of type {@link
org.apache.jackrabbit.api.security.principal.GroupPrincipal}. 'External'
@@ -311,28 +309,36 @@ class ExternalGroupPrincipalProvider imp
return false;
}
try {
- String name = getName();
- if (member instanceof ItemBasedPrincipal) {
- Tree tree = root.getTree(((ItemBasedPrincipal)
member).getPath());
- if (UserUtil.isType(tree, AuthorizableType.USER)) {
- PropertyState ps =
tree.getProperty(REP_EXTERNAL_PRINCIPAL_NAMES);
- return (ps != null &&
Iterables.contains(ps.getValue(Type.STRINGS), name));
- }
- } else {
- Authorizable a = userManager.getAuthorizable(member);
- if (a != null && !a.isGroup()) {
- Value[] vs =
a.getProperty(REP_EXTERNAL_PRINCIPAL_NAMES);
- if (vs != null) {
- for (Value v : vs) {
- if (name.equals(v.getString())) {
- return true;
- }
- }
- }
- }
- }
+ return isContainedInExternalPrincipalNames(member);
} catch (RepositoryException e) {
log.debug(e.getMessage());
+ return false;
+ }
+ }
+
+ private boolean isContainedInExternalPrincipalNames(@NotNull Principal
member) throws RepositoryException {
+ String name = getName();
+ if (member instanceof ItemBasedPrincipal) {
+ Tree tree = root.getTree(((ItemBasedPrincipal)
member).getPath());
+ if (UserUtil.isType(tree, AuthorizableType.USER)) {
+ PropertyState ps =
tree.getProperty(REP_EXTERNAL_PRINCIPAL_NAMES);
+ return (ps != null &&
Iterables.contains(ps.getValue(Type.STRINGS), name));
+ }
+ } else {
+ Authorizable a = userManager.getAuthorizable(member);
+ if (a == null || a.isGroup()) {
+ return false;
+ }
+
+ Value[] vs = a.getProperty(REP_EXTERNAL_PRINCIPAL_NAMES);
+ if (vs == null) {
+ return false;
+ }
+ for (Value v : vs) {
+ if (name.equals(v.getString())) {
+ return true;
+ }
+ }
}
return false;
}
@@ -464,35 +470,40 @@ class ExternalGroupPrincipalProvider imp
Set<Principal> principals;
if (!principalMap.containsKey(idpName)) {
- String[] vs = autoMembershipMapping.get(idpName);
- if (vs == null) {
- principals = ImmutableSet.of();
- } else {
- ImmutableSet.Builder<Principal> builder =
ImmutableSet.builder();
- for (String groupId : autoMembershipMapping.get(idpName)) {
- try {
- Authorizable gr =
userManager.getAuthorizable(groupId);
- if (gr != null && gr.isGroup()) {
- Principal grPrincipal = gr.getPrincipal();
- if (GroupPrincipals.isGroup(grPrincipal)) {
- builder.add(grPrincipal);
- } else {
- log.warn("Principal of group {} is not of
group type -> Ignoring", groupId);
- }
- } else {
- log.warn("Configured auto-membership group {}
does not exist -> Ignoring", groupId);
- }
- } catch (RepositoryException e) {
- log.debug("Failed to retrieved 'auto-membership'
group with id {}", groupId, e);
- }
- }
- principals = builder.build();
- }
+ principals = collectAutomembershipPrincipals(idpName);
principalMap.put(idpName, principals);
} else {
principals = principalMap.get(idpName);
}
return principals;
}
+
+ @NotNull
+ private Set<Principal> collectAutomembershipPrincipals(@NotNull String
idpName) {
+ String[] vs = autoMembershipMapping.get(idpName);
+ if (vs == null) {
+ return ImmutableSet.of();
+ }
+
+ ImmutableSet.Builder<Principal> builder = ImmutableSet.builder();
+ for (String groupId : vs) {
+ try {
+ Authorizable gr = userManager.getAuthorizable(groupId);
+ if (gr != null && gr.isGroup()) {
+ Principal grPrincipal = gr.getPrincipal();
+ if (GroupPrincipals.isGroup(grPrincipal)) {
+ builder.add(grPrincipal);
+ } else {
+ log.warn("Principal of group {} is not of group
type -> Ignoring", groupId);
+ }
+ } else {
+ log.warn("Configured auto-membership group {} does not
exist -> Ignoring", groupId);
+ }
+ } catch (RepositoryException e) {
+ log.debug("Failed to retrieved 'auto-membership' group
with id {}", groupId, e);
+ }
+ }
+ return builder.build();
+ }
}
}