Skip to content

Commit

Permalink
ARTEMIS-2643 allow masked password when resetting via mgmnt
Browse files Browse the repository at this point in the history
  • Loading branch information
@jbertram @clebertsuconic
jbertram authored and clebertsuconic committed Mar 6, 2020
1 parent b156b7d commit ed7fee6
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 19 deletions.
56 changes: 42 additions & 14 deletions artemis-cli/src/test/java/org/apache/activemq/cli/test/ArtemisTest.java
View file
Expand Up @@ -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");
Expand All @@ -502,26 +511,28 @@ 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("");
contains(JsonUtil.readJsonArray(jsonResult), "scott", "admin");
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) {
}
Expand Down Expand Up @@ -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");
Expand All @@ -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
Expand All @@ -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();
}

Expand Down Expand Up @@ -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<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);
}
Expand Down
16 changes: 16 additions & 0 deletions .../src/main/java/org/apache/activemq/artemis/api/core/management/ActiveMQServerControl.java
View file
Expand Up @@ -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).
*
Expand All @@ -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;
}

19 changes: 14 additions & 5 deletions ...main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java
View file
Expand Up @@ -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<String, Set<String>> info = config.listUser(username);
Expand All @@ -3915,22 +3916,30 @@ 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);
config.save();
}

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

Expand Down
5 changes: 5 additions & 0 deletions ...che/activemq/artemis/tests/integration/management/ActiveMQServerControlUsingCoreTest.java
View file
Expand Up @@ -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;
Expand Down

0 comments on commit ed7fee6

Please sign in to comment.