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 71b0f418d35..42bc468ce49 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 void testUserCommand() throws Exception { } @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 void testUserCommandViaManagement() throws Exception { 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 void testUserCommandViaManagement() throws Exception { 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 void testUserCommandReset() throws Exception { } @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 void testUserCommandResetViaManagement() throws Exception { 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 void testUserCommandResetViaManagement() throws Exception { 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 @@ private void checkRole(String user, File roleFile, String... roles) throws Excep } } - 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 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 d55952693b3..d0d9db1e34e 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 @@ void addUser(@Parameter(name = "username", desc = "Name of the user") String use */ @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 @@ void addUser(@Parameter(name = "username", desc = "Name of the user") String use 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 2f6407c3714..793451d41e6 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 String listUser(String username) throws Exception { return (String) tcclCall(ActiveMQServerControlImpl.class.getClassLoader(), () -> internaListUser(username)); } + private String internaListUser(String username) throws Exception { PropertiesLoginModuleConfigurator config = getPropertiesLoginModuleConfigurator(); Map> info = config.listUser(username); @@ -3915,6 +3916,7 @@ public void removeUser(String username) throws Exception { } tcclInvoke(ActiveMQServerControlImpl.class.getClassLoader(), () -> internalRemoveUser(username)); } + private void internalRemoveUser(String username) throws Exception { PropertiesLoginModuleConfigurator config = getPropertiesLoginModuleConfigurator(); config.removeUser(username); @@ -3922,15 +3924,22 @@ private void internalRemoveUser(String username) throws Exception { } @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 4bf0e8bcda8..cc3dcb3598f 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 @@ -385,6 +385,11 @@ public void resetUser(String username, String password, String roles) throws Exc proxy.invokeOperation("resetUser", username, password, roles); } + @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;