You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@activemq.apache.org by cl...@apache.org on 2020/03/06 02:38:24 UTC

[activemq-artemis] branch master updated: ARTEMIS-2643 allow masked password when resetting via mgmnt

This is an automated email from the ASF dual-hosted git repository.

clebertsuconic pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/activemq-artemis.git


The following commit(s) were added to refs/heads/master by this push:
     new ed7fee6  ARTEMIS-2643 allow masked password when resetting via mgmnt
     new 2b72653  This closes #2999
ed7fee6 is described below

commit ed7fee6d9cd94177dae30f2440e8bd07ddb5522a
Author: Justin Bertram <jb...@apache.org>
AuthorDate: Wed Mar 4 13:09:24 2020 -0600

    ARTEMIS-2643 allow masked password when resetting via mgmnt
---
 .../org/apache/activemq/cli/test/ArtemisTest.java  | 56 ++++++++++++++++------
 .../api/core/management/ActiveMQServerControl.java | 16 +++++++
 .../management/impl/ActiveMQServerControlImpl.java | 19 ++++++--
 .../ActiveMQServerControlUsingCoreTest.java        |  5 ++
 4 files changed, 77 insertions(+), 19 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
index 71b0f41..42bc468 100644
--- 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
@@ -483,7 +483,16 @@ public class ArtemisTest extends CliTestBase {
    }
 
    @Test
-   public void testUserCommandViaManagement() throws Exception {
+   public void testUserCommandViaManagementPlaintext() throws Exception {
+      internalTestUserCommandViaManagement(true);
+   }
+
+   @Test
+   public void testUserCommandViaManagementHashed() throws Exception {
+      internalTestUserCommandViaManagement(false);
+   }
+
+   private void internalTestUserCommandViaManagement(boolean plaintext) throws Exception {
       Run.setEmbedded(true);
       File instance1 = new File(temporaryFolder.getRoot(), "instance_user");
       System.setProperty("java.security.auth.login.config", instance1.getAbsolutePath() + "/etc/login.config");
@@ -502,16 +511,17 @@ public class ArtemisTest extends CliTestBase {
       checkRole("admin", roleFile, "amq");
 
       //add a simple user
-      activeMQServerControl.addUser("guest", "guest123", "admin", true);
+      activeMQServerControl.addUser("guest", "guest123", "admin", plaintext);
 
       //verify add
       jsonResult = activeMQServerControl.listUser("");
       contains(JsonUtil.readJsonArray(jsonResult), "guest", "admin");
       checkRole("guest", roleFile, "admin");
       assertTrue(checkPassword("guest", "guest123", userFile));
+      assertEquals(plaintext, !PasswordMaskingUtil.isEncMasked(getStoredPassword("guest", userFile)));
 
       //add a user with 2 roles
-      activeMQServerControl.addUser("scott", "tiger", "admin,operator", true);
+      activeMQServerControl.addUser("scott", "tiger", "admin,operator", plaintext);
 
       //verify add
       jsonResult = activeMQServerControl.listUser("");
@@ -519,9 +529,10 @@ public class ArtemisTest extends CliTestBase {
       contains(JsonUtil.readJsonArray(jsonResult), "scott", "operator");
       checkRole("scott", roleFile, "admin", "operator");
       assertTrue(checkPassword("scott", "tiger", userFile));
+      assertEquals(plaintext, !PasswordMaskingUtil.isEncMasked(getStoredPassword("scott", userFile)));
 
       try {
-         activeMQServerControl.addUser("scott", "password", "visitor", true);
+         activeMQServerControl.addUser("scott", "password", "visitor", plaintext);
          fail("should throw an exception if adding a existing user");
       } catch (IllegalArgumentException expected) {
       }
@@ -729,7 +740,16 @@ public class ArtemisTest extends CliTestBase {
    }
 
    @Test
-   public void testUserCommandResetViaManagement() throws Exception {
+   public void testUserCommandResetViaManagementPlaintext() throws Exception {
+      internalTestUserCommandResetViaManagement(true);
+   }
+
+   @Test
+   public void testUserCommandResetViaManagementHashed() throws Exception {
+      internalTestUserCommandResetViaManagement(false);
+   }
+
+   private void internalTestUserCommandResetViaManagement(boolean plaintext) throws Exception {
       Run.setEmbedded(true);
       File instance1 = new File(temporaryFolder.getRoot(), "instance_user");
       System.setProperty("java.security.auth.login.config", instance1.getAbsolutePath() + "/etc/login.config");
@@ -753,11 +773,12 @@ public class ArtemisTest extends CliTestBase {
       contains(JsonUtil.readJsonArray(jsonResult), "admin", "amq", false);
 
       //add some users
-      activeMQServerControl.addUser("guest", "guest123", "admin", true);
-      activeMQServerControl.addUser("user1", "password1", "admin,manager", true);
+      activeMQServerControl.addUser("guest", "guest123", "admin", plaintext);
+      activeMQServerControl.addUser("user1", "password1", "admin,manager", plaintext);
       assertTrue(checkPassword("user1", "password1", userFile));
-      activeMQServerControl.addUser("user2", "password2", "admin,manager,master", true);
-      activeMQServerControl.addUser("user3", "password3", "system,master", true);
+      assertEquals(plaintext, !PasswordMaskingUtil.isEncMasked(getStoredPassword("user1", userFile)));
+      activeMQServerControl.addUser("user2", "password2", "admin,manager,master", plaintext);
+      activeMQServerControl.addUser("user3", "password3", "system,master", plaintext);
 
 
       //verify use list cmd
@@ -774,23 +795,26 @@ public class ArtemisTest extends CliTestBase {
       checkRole("user1", roleFile, "admin", "manager");
 
       //reset password
-      activeMQServerControl.resetUser("user1", "newpassword1", null);
+      activeMQServerControl.resetUser("user1", "newpassword1", null, plaintext);
 
       checkRole("user1", roleFile, "admin", "manager");
       assertFalse(checkPassword("user1", "password1", userFile));
       assertTrue(checkPassword("user1", "newpassword1", userFile));
+      assertEquals(plaintext, !PasswordMaskingUtil.isEncMasked(getStoredPassword("user1", userFile)));
 
       //reset role
-      activeMQServerControl.resetUser("user2", null, "manager,master,operator");
+      activeMQServerControl.resetUser("user2", null, "manager,master,operator", plaintext);
 
       checkRole("user2", roleFile, "manager", "master", "operator");
       assertTrue(checkPassword("user2", "password2", userFile));
+      assertEquals(plaintext, !PasswordMaskingUtil.isEncMasked(getStoredPassword("user2", userFile)));
 
       //reset both
-      activeMQServerControl.resetUser("user3", "newpassword3", "admin,system");
+      activeMQServerControl.resetUser("user3", "newpassword3", "admin,system", plaintext);
 
       checkRole("user3", roleFile, "admin", "system");
       assertTrue(checkPassword("user3", "newpassword3", userFile));
+      assertEquals(plaintext, !PasswordMaskingUtil.isEncMasked(getStoredPassword("user3", userFile)));
       stopServer();
    }
 
@@ -1394,11 +1418,15 @@ public class ArtemisTest extends CliTestBase {
       }
    }
 
-   private boolean checkPassword(String user, String password, File userFile) throws Exception {
+   private String getStoredPassword(String user, File userFile) throws Exception {
       Configurations configs = new Configurations();
       FileBasedConfigurationBuilder<PropertiesConfiguration> userBuilder = configs.propertiesBuilder(userFile);
       PropertiesConfiguration userConfig = userBuilder.getConfiguration();
-      String storedPassword = (String) userConfig.getProperty(user);
+      return (String) userConfig.getProperty(user);
+   }
+
+   private boolean checkPassword(String user, String password, File userFile) throws Exception {
+      String storedPassword = getStoredPassword(user, userFile);
       HashProcessor processor = PasswordMaskingUtil.getHashProcessor(storedPassword);
       return processor.compare(password.toCharArray(), storedPassword);
    }
diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/management/ActiveMQServerControl.java b/artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/management/ActiveMQServerControl.java
index d559526..d0d9db1 100644
--- a/artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/management/ActiveMQServerControl.java
+++ b/artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/management/ActiveMQServerControl.java
@@ -1702,6 +1702,7 @@ public interface ActiveMQServerControl {
     */
    @Operation(desc = "remove a user (only applicable when using the JAAS PropertiesLoginModule)", impact = MBeanOperationInfo.ACTION)
    void removeUser(@Parameter(name = "username", desc = "Name of the user") String username) throws Exception;
+
    /**
     * Set new properties on an existing user (only applicable when using the JAAS PropertiesLoginModule).
     *
@@ -1714,5 +1715,20 @@ public interface ActiveMQServerControl {
    void resetUser(@Parameter(name = "username", desc = "Name of the user") String username,
                   @Parameter(name = "password", desc = "User's password") String password,
                   @Parameter(name = "roles", desc = "User's role (comma separated)") String roles) throws Exception;
+   /**
+    * Set new properties on an existing user (only applicable when using the JAAS PropertiesLoginModule).
+    *
+    * @param username
+    * @param password
+    * @param roles
+    * @param plaintext
+    * @throws Exception
+    */
+
+   @Operation(desc = "set new properties on an existing user (only applicable when using the JAAS PropertiesLoginModule)", impact = MBeanOperationInfo.ACTION)
+   void resetUser(@Parameter(name = "username", desc = "Name of the user") String username,
+                  @Parameter(name = "password", desc = "User's password") String password,
+                  @Parameter(name = "roles", desc = "User's role (comma separated)") String roles,
+                  @Parameter(name = "plaintext", desc = "whether or not to store the password in plaintext or hash it") boolean plaintext) throws Exception;
 }
 
diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java
index 2f6407c..793451d 100644
--- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java
+++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java
@@ -3891,6 +3891,7 @@ public class ActiveMQServerControlImpl extends AbstractControl implements Active
 
       return (String) tcclCall(ActiveMQServerControlImpl.class.getClassLoader(), () -> internaListUser(username));
    }
+
    private String internaListUser(String username) throws Exception {
       PropertiesLoginModuleConfigurator config = getPropertiesLoginModuleConfigurator();
       Map<String, Set<String>> info = config.listUser(username);
@@ -3915,6 +3916,7 @@ public class ActiveMQServerControlImpl extends AbstractControl implements Active
       }
       tcclInvoke(ActiveMQServerControlImpl.class.getClassLoader(), () -> internalRemoveUser(username));
    }
+
    private void internalRemoveUser(String username) throws Exception {
       PropertiesLoginModuleConfigurator config = getPropertiesLoginModuleConfigurator();
       config.removeUser(username);
@@ -3922,15 +3924,22 @@ public class ActiveMQServerControlImpl extends AbstractControl implements Active
    }
 
    @Override
-   public void resetUser(String username, String password, String roles) throws Exception {
+   public void resetUser(String username, String password, String roles, boolean plaintext) throws Exception {
       if (AuditLogger.isEnabled()) {
-         AuditLogger.resetUser(this.server, username, "****", roles);
+         AuditLogger.resetUser(this.server, username, "****", roles, plaintext);
       }
-      tcclInvoke(ActiveMQServerControlImpl.class.getClassLoader(), () -> internalresetUser(username, password, roles));
+      tcclInvoke(ActiveMQServerControlImpl.class.getClassLoader(), () -> internalresetUser(username, password, roles, plaintext));
+   }
+
+   @Override
+   public void resetUser(String username, String password, String roles) throws Exception {
+      resetUser(username, password, roles, true);
    }
-   private void internalresetUser(String username, String password, String roles) throws Exception {
+
+   private void internalresetUser(String username, String password, String roles, boolean plaintext) throws Exception {
       PropertiesLoginModuleConfigurator config = getPropertiesLoginModuleConfigurator();
-      config.updateUser(username, password, roles == null ? null : roles.split(","));
+      // don't hash a null password even if plaintext = false
+      config.updateUser(username, password == null ? password : plaintext ? password : PasswordMaskingUtil.getHashProcessor().hash(password), roles == null ? null : roles.split(","));
       config.save();
    }
 
diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ActiveMQServerControlUsingCoreTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ActiveMQServerControlUsingCoreTest.java
index 4bf0e8b..cc3dcb3 100644
--- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ActiveMQServerControlUsingCoreTest.java
+++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/management/ActiveMQServerControlUsingCoreTest.java
@@ -386,6 +386,11 @@ public class ActiveMQServerControlUsingCoreTest extends ActiveMQServerControlTes
          }
 
          @Override
+         public void resetUser(String username, String password, String roles, boolean plaintext) throws Exception {
+            proxy.invokeOperation("resetUser", username, password, roles, plaintext);
+         }
+
+         @Override
          public String getUptime() {
             return null;
          }