ARTEMIS-3530 space in role list breaks user listing

(cherry picked from commit 6d52f20)
apache · Jan 24, 2022 · 01d7cb6 · 01d7cb6
1 parent 9863fed
commit 01d7cb6
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 3 deletions.
diff --git a/artemis-cli/src/test/java/org/apache/activemq/cli/test/ArtemisTest.java b/artemis-cli/src/test/java/org/apache/activemq/cli/test/ArtemisTest.java
@@ -34,6 +34,7 @@
 import javax.xml.transform.dom.DOMSource;
 import javax.xml.transform.stream.StreamResult;
 import java.io.BufferedReader;
+import java.io.BufferedWriter;
 import java.io.ByteArrayInputStream;
 import java.io.File;
 import java.io.IOException;
@@ -704,6 +705,46 @@ private void internalTestUserCommandViaManagement(boolean plaintext, boolean bas
       stopServer();
    }
 
+   @Test
+   public void testListUserWithMultipleRolesWithSpaces() throws Exception {
+      try {
+         Run.setEmbedded(true);
+         File instance1 = new File(temporaryFolder.getRoot(), "instance_user");
+         System.setProperty("java.security.auth.login.config", instance1.getAbsolutePath() + "/etc/login.config");
+         Artemis.main("create", instance1.getAbsolutePath(), "--silent", "--no-autotune", "--no-web", "--no-amqp-acceptor", "--no-mqtt-acceptor", "--no-stomp-acceptor", "--no-hornetq-acceptor", "--security-manager", "jaas");
+         System.setProperty("artemis.instance", instance1.getAbsolutePath());
+         Object result = Artemis.internalExecute("run");
+         server = ((Pair<ManagementContext, ActiveMQServer>) result).getB();
+         ActiveMQServerControl activeMQServerControl = server.getActiveMQServerControl();
+
+         File userFile = new File(instance1.getAbsolutePath() + "/etc/artemis-users.properties");
+         BufferedWriter writer = Files.newBufferedWriter(Paths.get(userFile.getPath()));
+         writer.write("");
+         writer.write("user1 = pass1");
+         writer.newLine();
+         writer.write("user2 = pass2");
+         writer.flush();
+         writer.close();
+         File roleFile = new File(instance1.getAbsolutePath() + "/etc/artemis-roles.properties");
+         writer = Files.newBufferedWriter(Paths.get(roleFile.getPath()));
+         writer.write("");
+         writer.write("role1 = user1, user2"); // the space here is what breaks the parsing
+         writer.newLine();
+         writer.write("role2 = user2");
+         writer.flush();
+         writer.close();
+
+         String jsonResult = activeMQServerControl.listUser("");
+         contains(JsonUtil.readJsonArray(jsonResult), "user2", "role1");
+         contains(JsonUtil.readJsonArray(jsonResult), "user2", "role2");
+         checkRole("user2", roleFile, false, "role1", "role2");
+         assertTrue(checkPassword("user1", "pass1", userFile, false));
+         assertTrue(checkPassword("user2", "pass2", userFile, false));
+      } finally {
+         stopServer();
+      }
+   }
+
    @Test
    public void testProperReloadWhenAddingUserViaManagementJAAS() throws Exception {
       testProperReloadWhenAddingUserViaManagement(false);

diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/utils/StringUtil.java b/artemis-core-client/src/main/java/org/apache/activemq/artemis/utils/StringUtil.java
@@ -17,7 +17,6 @@
 package org.apache.activemq.artemis.utils;
 
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Iterator;
 import java.util.List;
@@ -54,7 +53,9 @@ public static String joinStringList(Collection<String> strList, String delimit)
    public static List<String> splitStringList(String strList, String delimit) {
       ArrayList<String> list = new ArrayList<>();
       if (strList != null && !strList.isEmpty()) {
-         list.addAll(Arrays.asList(strList.split(delimit)));
+         for (String string : strList.split(delimit)) {
+            list.add(string.trim());
+         }
       }
       return list;
    }

diff --git a/artemis-core-client/src/test/java/org/apache/activemq/artemis/util/StringUtilTest.java b/artemis-core-client/src/test/java/org/apache/activemq/artemis/util/StringUtilTest.java
@@ -53,4 +53,15 @@ public void testSplitStringList() throws Exception {
       String result2 = StringUtil.joinStringList(result, ",");
       assertEquals(listStr, result2);
    }
+
+   @Test
+   public void testSplitStringListWithSpaces() throws Exception {
+      String listStr = "white, blue, yellow, green";
+      List<String> result = StringUtil.splitStringList(listStr, ",");
+      assertEquals(4, result.size());
+      assertEquals("white", result.get(0));
+      assertEquals("blue", result.get(1));
+      assertEquals("yellow", result.get(2));
+      assertEquals("green", result.get(3));
+   }
 }
diff --git a/...org/apache/activemq/artemis/spi/core/security/jaas/PropertiesLoginModuleConfigurator.java b/...org/apache/activemq/artemis/spi/core/security/jaas/PropertiesLoginModuleConfigurator.java
@@ -197,7 +197,7 @@ private Set<String> findRoles(String username) {
             //each roleList may be a comma separated list
             String[] items = roleList.split(",");
             for (String item : items) {
-               if (item.equals(username)) {
+               if (item.trim().equals(username)) {
                   roles.add(role);
                }
             }