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();
+        }
     }
 }


Reply via email to