https://issues.apache.org/jira/browse/AMQ-5876 - load groups properties into per user set once also to avoid parse per login attempt

activemq-jaas/src/main/java/org/apache/activemq/jaas/CertificateLoginModule.java

@@ -21,7 +21,6 @@ import java.io.IOException;
 import java.security.Principal;
 import java.security.cert.X509Certificate;
 import java.util.HashSet;
-import java.util.Iterator;
 import java.util.Map;
 import java.util.Set;
 
@@ -52,7 +51,6 @@ public abstract class CertificateLoginModule extends PropertiesLoader implements
 
     private X509Certificate certificates[];
     private String username;
-    private Set<String> groups;
     private Set<Principal> principals = new HashSet<Principal>();
 
     /**
@@ -87,8 +85,6 @@ public abstract class CertificateLoginModule extends PropertiesLoader implements
             throw new FailedLoginException("No user for client certificate: " + getDistinguishedName(certificates));
         }
 
-        groups = getUserGroups(username);
-
         if (debug) {
             LOG.debug("Certificate for user: " + username);
         }
@@ -102,7 +98,7 @@ public abstract class CertificateLoginModule extends PropertiesLoader implements
     public boolean commit() throws LoginException {
         principals.add(new UserPrincipal(username));
 
-        for (String group : groups) {
+        for (String group : getUserGroups(username)) {
              principals.add(new GroupPrincipal(group));
         }
 
@@ -147,8 +143,8 @@ public abstract class CertificateLoginModule extends PropertiesLoader implements
      * Helper method.
      */
     private void clear() {
-        groups.clear();
         certificates = null;
+        username = null;
     }
 
     /**

activemq-jaas/src/main/java/org/apache/activemq/jaas/PropertiesLoginModule.java

@@ -47,7 +47,7 @@ public class PropertiesLoginModule extends PropertiesLoader implements LoginModu
     private CallbackHandler callbackHandler;
 
     private Properties users;
-    private Properties groups;
+    private Map<String,Set<String>> groups;
     private String user;
     private final Set<Principal> principals = new HashSet<Principal>();
     private boolean loginSucceeded;
@@ -59,7 +59,7 @@ public class PropertiesLoginModule extends PropertiesLoader implements LoginModu
         loginSucceeded = false;
         init(options);
         users = load(USER_FILE_PROP_NAME, "user", options).getProps();
-        groups = load(GROUP_FILE_PROP_NAME, "group", options).getProps();
+        groups = load(GROUP_FILE_PROP_NAME, "group", options).invertedPropertiesValuesMap();
     }
 
     @Override
@@ -105,14 +105,10 @@ public class PropertiesLoginModule extends PropertiesLoader implements LoginModu
         if (result) {
             principals.add(new UserPrincipal(user));
 
-            for (Map.Entry<Object, Object> entry : groups.entrySet()) {
+            Set<String> matchedGroups = groups.get(user);
-                String name = (String) entry.getKey();
+            if (matchedGroups != null) {
-                String[] userList = ((String)entry.getValue()).split(",");
+                for (String entry : matchedGroups) {
-                for (int i = 0; i < userList.length; i++) {
+                    principals.add(new GroupPrincipal(entry));
-                    if (user.equals(userList[i])) {
-                        principals.add(new GroupPrincipal(name));
-                        break;
-                    }
                 }
             }
 

activemq-jaas/src/main/java/org/apache/activemq/jaas/ReloadableProperties.java

@@ -20,8 +20,10 @@ import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Map;
 import java.util.Properties;
+import java.util.Set;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -30,6 +32,7 @@ public class ReloadableProperties {
 
     private Properties props = new Properties();
     private Map<String, String> invertedProps;
+    private Map<String, Set<String>> invertedValueProps;
     private long reloadTime = -1;
     private final PropertiesLoader.FileNameKey key;
 
@@ -71,6 +74,24 @@ public class ReloadableProperties {
         return invertedProps;
     }
 
+    public synchronized Map<String, Set<String>> invertedPropertiesValuesMap() {
+        if (invertedValueProps == null) {
+            invertedValueProps = new HashMap<>(props.size());
+            for (Map.Entry<Object, Object> val : props.entrySet()) {
+                String[] userList = ((String)val.getValue()).split(",");
+                for (String user : userList) {
+                    Set<String> set = invertedValueProps.get(user);
+                    if (set == null) {
+                        set = new HashSet<>();
+                        invertedValueProps.put(user, set);
+                    }
+                    set.add((String)val.getKey());
+                }
+            }
+        }
+        return invertedValueProps;
+    }
+
     private void load(final File source, Properties props) throws IOException {
         FileInputStream in = new FileInputStream(source);
         try {

activemq-jaas/src/main/java/org/apache/activemq/jaas/TextFileCertificateLoginModule.java

@@ -18,6 +18,7 @@
 package org.apache.activemq.jaas;
 
 import java.security.cert.X509Certificate;
+import java.util.Collections;
 import java.util.Enumeration;
 import java.util.HashSet;
 import java.util.Map;
@@ -46,7 +47,7 @@ public class TextFileCertificateLoginModule extends CertificateLoginModule {
     private static final String USER_FILE_PROP_NAME = "org.apache.activemq.jaas.textfiledn.user";
     private static final String GROUP_FILE_PROP_NAME = "org.apache.activemq.jaas.textfiledn.group";
 
-    private Properties groups;
+    private Map<String, Set<String>> groupsByUser;
     private Map<String, String> usersByDn;
 
     /**
@@ -57,7 +58,7 @@ public class TextFileCertificateLoginModule extends CertificateLoginModule {
         super.initialize(subject, callbackHandler, sharedState, options);
 
         usersByDn = load(USER_FILE_PROP_NAME, "", options).invertedPropertiesMap();
-        groups = load(GROUP_FILE_PROP_NAME, "", options).getProps();
+        groupsByUser = load(GROUP_FILE_PROP_NAME, "", options).invertedPropertiesValuesMap();
      }
 
     /**
@@ -89,18 +90,10 @@ public class TextFileCertificateLoginModule extends CertificateLoginModule {
      */
     @Override
     protected Set<String> getUserGroups(String username) throws LoginException {
-        Set<String> userGroups = new HashSet<String>();
+        Set<String> userGroups = groupsByUser.get(username);
-        for (Enumeration<Object> enumeration = groups.keys(); enumeration.hasMoreElements();) {
+        if (userGroups == null) {
-            String groupName = (String)enumeration.nextElement();
+            userGroups = Collections.emptySet();
-            String[] userList = (groups.getProperty(groupName) + "").split(",");
-            for (int i = 0; i < userList.length; i++) {
-                if (username.equals(userList[i])) {
-                    userGroups.add(groupName);
-                    break;
         }
-            }
-        }
-
         return userGroups;
     }
 }