Apache
/
activemq-artemis
mirror of https://github.com/apache/activemq-artemis.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

ARTEMIS-2893 concurrent user admin actions can corrupt properties 

When performing concurrent user admin actions (e.g. resetUser, addUser,
removeUser on ActiveMQServerControl) when using the
PropertiesLoginModule with reload=true the underlying user and role
properties files can get corrupted.

This commit fixes the issue via the following changes:
 - Add synchronization to the management commands
 - Add concurrency controls to underlying file access
 - Change CLI user commands to use remote methods instead of modifying
   the files directly. This avoids potential concurrent changes. This
   change forced me to modify the names of some of the commands'
   parameters to disambiguate them from connection-related parameters.
This commit is contained in:
Justin Bertram 2020-09-02 10:53:12 -05:00 committed by Clebert Suconic
parent 054b14829e
commit 276a8bb029
10 changed files with 544 additions and 339 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

46
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/user/AddUser.java
View File

@ -18,45 +18,53 @@ package org.apache.activemq.artemis.cli.commands.user;
import io.airlift.airline.Command; import io.airlift.airline.Command;
import io.airlift.airline.Option; import io.airlift.airline.Option;
import org.apache.activemq.artemis.api.core.client.ClientMessage;
import org.apache.activemq.artemis.api.core.management.ManagementHelper;
import org.apache.activemq.artemis.cli.commands.AbstractAction;
import org.apache.activemq.artemis.cli.commands.ActionContext; import org.apache.activemq.artemis.cli.commands.ActionContext;
import org.apache.activemq.artemis.cli.commands.util.HashUtil;
import org.apache.activemq.artemis.spi.core.security.jaas.PropertiesLoginModuleConfigurator;
import org.apache.commons.lang3.StringUtils;
/** /**
* Adding a new user, example: * Adding a new user, example:
* ./artemis user add --user guest --role admin --password *** * ./artemis user add --user-command-user guest --role admin --user-command-password ***
*/ */
@Command(name = "add", description = "Add a new user") @Command(name = "add", description = "Add a new user")
public class AddUser extends PasswordAction { public class AddUser extends PasswordAction {
@Option(name = "--plaintext", description = "using plaintext (Default false)") @Option(name = "--plaintext", description = "store the password in plaintext (Default: false)")
boolean plaintext = false; boolean plaintext = false;
@Override @Override
public Object execute(ActionContext context) throws Exception { public Object execute(ActionContext context) throws Exception {
super.execute(context); super.execute(context);
checkInputUser(); checkInputUser();
checkInputPassword(); checkInputPassword();
checkInputRole(); checkInputRole();
add();
String hash = plaintext ? password : HashUtil.tryHash(context, password);
add(hash, StringUtils.split(role, ","));
return null; return null;
} }
/** /**
* Adding a new user * Add a new user
* @param hash the password *
* @param role the role * @throws Exception if communication with the broker fails
* @throws IllegalArgumentException if user exists
*/ */
private void add(String hash, String... role) throws Exception { private void add() throws Exception {
PropertiesLoginModuleConfigurator config = new PropertiesLoginModuleConfigurator(entry, getBrokerEtc()); performCoreManagement(new AbstractAction.ManagementCallback<ClientMessage>() {
config.addNewUser(username, hash, role); @Override
config.save(); public void setUpInvocation(ClientMessage message) throws Exception {
context.out.println("User added successfully."); ManagementHelper.putOperationInvocation(message, "broker", "addUser", userCommandUser, userCommandPassword, role, plaintext);
}
@Override
public void requestSuccessful(ClientMessage reply) throws Exception {
context.out.println(userCommandUser + " added successfully.");
}
@Override
public void requestFailed(ClientMessage reply) throws Exception {
String errMsg = (String) ManagementHelper.getResult(reply, String.class);
context.err.println("Failed to add user " + userCommandUser + ". Reason: " + errMsg);
}
});
} }
} }

50
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/user/ListUser.java
View File

@ -16,12 +16,15 @@
*/ */
package org.apache.activemq.artemis.cli.commands.user; package org.apache.activemq.artemis.cli.commands.user;
import java.util.Map; import javax.json.JsonArray;
import java.util.Set; import javax.json.JsonObject;
import io.airlift.airline.Command; import io.airlift.airline.Command;
import org.apache.activemq.artemis.api.core.JsonUtil;
import org.apache.activemq.artemis.api.core.client.ClientMessage;
import org.apache.activemq.artemis.api.core.management.ManagementHelper;
import org.apache.activemq.artemis.cli.commands.AbstractAction;
import org.apache.activemq.artemis.cli.commands.ActionContext; import org.apache.activemq.artemis.cli.commands.ActionContext;
import org.apache.activemq.artemis.spi.core.security.jaas.PropertiesLoginModuleConfigurator;
/** /**
* list existing users, example: * list existing users, example:
@ -40,20 +43,41 @@ public class ListUser extends UserAction {
} }
/** /**
* list a single user or all users * List a single user or all users if username is not specified
* if username is not specified *
* @throws Exception if communication with the broker fails
*/ */
private void list() throws Exception { private void list() throws Exception {
PropertiesLoginModuleConfigurator config = new PropertiesLoginModuleConfigurator(entry, getBrokerEtc());
Map<String, Set<String>> result = config.listUser(username);
StringBuilder logMessage = new StringBuilder("--- \"user\"(roles) ---\n"); StringBuilder logMessage = new StringBuilder("--- \"user\"(roles) ---\n");
int userCount = 0; int userCount = 0;
for (Map.Entry<String, Set<String>> entry : result.entrySet()) { final String[] result = new String[1];
logMessage.append("\"").append(entry.getKey()).append("\"(");
int roleCount = 0; performCoreManagement(new AbstractAction.ManagementCallback<ClientMessage>() {
for (String role : entry.getValue()) { @Override
logMessage.append(role); public void setUpInvocation(ClientMessage message) throws Exception {
if (++roleCount < entry.getValue().size()) { ManagementHelper.putOperationInvocation(message, "broker", "listUser", userCommandUser);
}
@Override
public void requestSuccessful(ClientMessage reply) throws Exception {
result[0] = (String) ManagementHelper.getResult(reply, String.class);
}
@Override
public void requestFailed(ClientMessage reply) throws Exception {
String errMsg = (String) ManagementHelper.getResult(reply, String.class);
context.err.println("Failed to list user " + userCommandUser + ". Reason: " + errMsg);
}
});
// process the JSON results from the broker
JsonArray array = JsonUtil.readJsonArray(result[0]);
for (JsonObject object : array.getValuesAs(JsonObject.class)) {
logMessage.append("\"").append(object.getString("username")).append("\"").append("(");
JsonArray roles = object.getJsonArray("roles");
for (int i = 0; i < roles.size(); i++) {
logMessage.append(roles.getString(i));
if ((i + 1) < roles.size()) {
logMessage.append(","); logMessage.append(",");
} }
} }

12
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/user/PasswordAction.java
View File

@ -21,17 +21,17 @@ import io.airlift.airline.Option;
public class PasswordAction extends UserAction { public class PasswordAction extends UserAction {
@Option(name = "--password", description = "the password (Default: input)") @Option(name = "--user-command-password", description = "The password to use for the chosen user command (Default: input)")
String password; String userCommandPassword;
void checkInputPassword() { void checkInputPassword() {
if (password == null) { if (userCommandPassword == null) {
password = inputPassword("--password", "Please provide the password:", null); userCommandPassword = inputPassword("--user-command-password", "Please provide the password to use for the chosen user command:", null);
} }
} }
public void setPassword(String password) { public void setUserCommandPassword(String userCommandPassword) {
this.password = password; this.userCommandPassword = userCommandPassword;
} }
} }

25
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/user/RemoveUser.java
View File

@ -17,8 +17,10 @@
package org.apache.activemq.artemis.cli.commands.user; package org.apache.activemq.artemis.cli.commands.user;
import io.airlift.airline.Command; import io.airlift.airline.Command;
import org.apache.activemq.artemis.api.core.client.ClientMessage;
import org.apache.activemq.artemis.api.core.management.ManagementHelper;
import org.apache.activemq.artemis.cli.commands.AbstractAction;
import org.apache.activemq.artemis.cli.commands.ActionContext; import org.apache.activemq.artemis.cli.commands.ActionContext;
import org.apache.activemq.artemis.spi.core.security.jaas.PropertiesLoginModuleConfigurator;
/** /**
* Remove a user, example: * Remove a user, example:
@ -36,10 +38,23 @@ public class RemoveUser extends UserAction {
} }
private void remove() throws Exception { private void remove() throws Exception {
PropertiesLoginModuleConfigurator config = new PropertiesLoginModuleConfigurator(entry, getBrokerEtc()); performCoreManagement(new AbstractAction.ManagementCallback<ClientMessage>() {
config.removeUser(username); @Override
config.save(); public void setUpInvocation(ClientMessage message) throws Exception {
context.out.println("User removed."); ManagementHelper.putOperationInvocation(message, "broker", "removeUser", userCommandUser);
}
@Override
public void requestSuccessful(ClientMessage reply) throws Exception {
context.out.println(userCommandUser + " removed successfully.");
}
@Override
public void requestFailed(ClientMessage reply) throws Exception {
String errMsg = (String) ManagementHelper.getResult(reply, String.class);
context.err.println("Failed to remove user " + userCommandUser + ". Reason: " + errMsg);
}
});
} }
} }

48
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/user/ResetUser.java
View File

@ -18,10 +18,10 @@ package org.apache.activemq.artemis.cli.commands.user;
import io.airlift.airline.Command; import io.airlift.airline.Command;
import io.airlift.airline.Option; import io.airlift.airline.Option;
import org.apache.activemq.artemis.api.core.client.ClientMessage;
import org.apache.activemq.artemis.api.core.management.ManagementHelper;
import org.apache.activemq.artemis.cli.commands.AbstractAction;
import org.apache.activemq.artemis.cli.commands.ActionContext; import org.apache.activemq.artemis.cli.commands.ActionContext;
import org.apache.activemq.artemis.cli.commands.util.HashUtil;
import org.apache.activemq.artemis.spi.core.security.jaas.PropertiesLoginModuleConfigurator;
import org.apache.commons.lang3.StringUtils;
/** /**
* Reset a user's password or roles, example: * Reset a user's password or roles, example:
@ -30,37 +30,35 @@ import org.apache.commons.lang3.StringUtils;
@Command(name = "reset", description = "Reset user's password or roles") @Command(name = "reset", description = "Reset user's password or roles")
public class ResetUser extends PasswordAction { public class ResetUser extends PasswordAction {
@Option(name = "--plaintext", description = "using plaintext (Default false)") @Option(name = "--plaintext", description = "store the password in plaintext (Default: false)")
boolean plaintext = false; boolean plaintext = false;
@Override @Override
public Object execute(ActionContext context) throws Exception { public Object execute(ActionContext context) throws Exception {
super.execute(context); super.execute(context);
checkInputUser(); checkInputUser();
checkInputPassword(); checkInputPassword();
reset();
if (password != null) {
password = plaintext ? password : HashUtil.tryHash(context, password);
}
String[] roles = null;
if (role != null) {
roles = StringUtils.split(role, ",");
}
reset(password, roles);
return null; return null;
} }
private void reset(String password, String[] roles) throws Exception { private void reset() throws Exception {
if (password == null && roles == null) { performCoreManagement(new AbstractAction.ManagementCallback<ClientMessage>() {
context.err.println("Nothing to update."); @Override
return; public void setUpInvocation(ClientMessage message) throws Exception {
} ManagementHelper.putOperationInvocation(message, "broker", "resetUser", userCommandUser, userCommandPassword, role, plaintext);
PropertiesLoginModuleConfigurator config = new PropertiesLoginModuleConfigurator(entry, getBrokerEtc()); }
config.updateUser(username, password, roles);
config.save(); @Override
context.out.println("User updated"); public void requestSuccessful(ClientMessage reply) throws Exception {
context.out.println(userCommandUser + " reset successfully.");
}
@Override
public void requestFailed(ClientMessage reply) throws Exception {
String errMsg = (String) ManagementHelper.getResult(reply, String.class);
context.err.println("Failed to reset user " + userCommandUser + ". Reason: " + errMsg);
}
});
} }
} }

19
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/user/UserAction.java
View File

@ -17,22 +17,19 @@
package org.apache.activemq.artemis.cli.commands.user; package org.apache.activemq.artemis.cli.commands.user;
import io.airlift.airline.Option; import io.airlift.airline.Option;
import org.apache.activemq.artemis.cli.commands.InputAbstract; import org.apache.activemq.artemis.cli.commands.AbstractAction;
public abstract class UserAction extends InputAbstract { public abstract class UserAction extends AbstractAction {
@Option(name = "--role", description = "user's role(s), comma separated") @Option(name = "--role", description = "user's role(s), comma separated")
String role; String role;
@Option(name = "--user", description = "The user name (Default: input)") @Option(name = "--user-command-user", description = "The username to use for the chosen user command (Default: input)")
String username = null; String userCommandUser = null;
@Option(name = "--entry", description = "The appConfigurationEntry (default: activemq)")
String entry = "activemq";
void checkInputUser() { void checkInputUser() {
if (username == null) { if (userCommandUser == null) {
username = input("--user", "Please provider the userName:", null); userCommandUser = input("--user-command-user", "Please provide the username to use for the chosen user command:", null);
} }
} }
@ -42,8 +39,8 @@ public abstract class UserAction extends InputAbstract {
} }
} }
public void setUsername(String username) { public void setUserCommandUser(String userCommandUser) {
this.username = username; this.userCommandUser = userCommandUser;
} }
public void setRole(String role) { public void setRole(String role) {

561
artemis-cli/src/test/java/org/apache/activemq/cli/test/ArtemisTest.java
View File

@ -344,144 +344,163 @@ public class ArtemisTest extends CliTestBase {
Run.setEmbedded(true); Run.setEmbedded(true);
File instance1 = new File(temporaryFolder.getRoot(), "instance_user"); File instance1 = new File(temporaryFolder.getRoot(), "instance_user");
System.setProperty("java.security.auth.login.config", instance1.getAbsolutePath() + "/etc/login.config"); System.setProperty("java.security.auth.login.config", instance1.getAbsolutePath() + "/etc/login.config");
Artemis.main("create", instance1.getAbsolutePath(), "--silent", "--no-autotune"); Artemis.main("create", instance1.getAbsolutePath(), "--silent", "--no-autotune", "--no-web", "--require-login");
System.setProperty("artemis.instance", instance1.getAbsolutePath()); System.setProperty("artemis.instance", instance1.getAbsolutePath());
Artemis.internalExecute("run");
File userFile = new File(instance1.getAbsolutePath() + "/etc/artemis-users.properties");
File roleFile = new File(instance1.getAbsolutePath() + "/etc/artemis-roles.properties");
ListUser listCmd = new ListUser();
TestActionContext context = new TestActionContext();
listCmd.execute(context);
String result = context.getStdout();
log.debug("output1:\n" + result);
//default only one user admin with role amq
assertTrue(result.contains("\"admin\"(amq)"));
checkRole("admin", roleFile, "amq");
//add a simple user
AddUser addCmd = new AddUser();
addCmd.setUsername("guest");
addCmd.setPassword("guest123");
addCmd.setRole("admin");
addCmd.execute(new TestActionContext());
//verify use list cmd
context = new TestActionContext();
listCmd.execute(context);
result = context.getStdout();
log.debug("output2:\n" + result);
assertTrue(result.contains("\"admin\"(amq)"));
assertTrue(result.contains("\"guest\"(admin)"));
checkRole("guest", roleFile, "admin");
assertTrue(checkPassword("guest", "guest123", userFile));
//add a user with 2 roles
addCmd = new AddUser();
addCmd.setUsername("scott");
addCmd.setPassword("tiger");
addCmd.setRole("admin,operator");
addCmd.execute(ActionContext.system());
//verify
context = new TestActionContext();
listCmd.execute(context);
result = context.getStdout();
log.debug("output3:\n" + result);
assertTrue(result.contains("\"admin\"(amq)"));
assertTrue(result.contains("\"guest\"(admin)"));
assertTrue(result.contains("\"scott\"(admin,operator)"));
checkRole("scott", roleFile, "admin", "operator");
assertTrue(checkPassword("scott", "tiger", userFile));
//add an existing user
addCmd = new AddUser();
addCmd.setUsername("scott");
addCmd.setPassword("password");
addCmd.setRole("visitor");
try { try {
File userFile = new File(instance1.getAbsolutePath() + "/etc/artemis-users.properties");
File roleFile = new File(instance1.getAbsolutePath() + "/etc/artemis-roles.properties");
ListUser listCmd = new ListUser();
TestActionContext context = new TestActionContext();
listCmd.setUser("admin");
listCmd.setPassword("admin");
listCmd.execute(context);
String result = context.getStdout();
log.debug("output1:\n" + result);
//default only one user admin with role amq
assertTrue(result.contains("\"admin\"(amq)"));
checkRole("admin", roleFile, "amq");
//add a simple user
AddUser addCmd = new AddUser();
addCmd.setUserCommandUser("guest");
addCmd.setUserCommandPassword("guest123");
addCmd.setRole("admin");
addCmd.setUser("admin");
addCmd.setPassword("admin");
addCmd.execute(new TestActionContext());
//verify use list cmd
context = new TestActionContext();
listCmd.execute(context);
result = context.getStdout();
log.debug("output2:\n" + result);
assertTrue(result.contains("\"admin\"(amq)"));
assertTrue(result.contains("\"guest\"(admin)"));
checkRole("guest", roleFile, "admin");
assertTrue(checkPassword("guest", "guest123", userFile));
//add a user with 2 roles
addCmd = new AddUser();
addCmd.setUserCommandUser("scott");
addCmd.setUserCommandPassword("tiger");
addCmd.setRole("admin,operator");
addCmd.setUser("admin");
addCmd.setPassword("admin");
addCmd.execute(ActionContext.system()); addCmd.execute(ActionContext.system());
fail("should throw an exception if adding a existing user");
} catch (IllegalArgumentException expected) {
}
//check existing users are intact //verify
context = new TestActionContext(); context = new TestActionContext();
listCmd.execute(context); listCmd.execute(context);
result = context.getStdout(); result = context.getStdout();
log.debug("output4:\n" + result); log.debug("output3:\n" + result);
assertTrue(result.contains("\"admin\"(amq)")); assertTrue(result.contains("\"admin\"(amq)"));
assertTrue(result.contains("\"guest\"(admin)")); assertTrue(result.contains("\"guest\"(admin)"));
assertTrue(result.contains("\"scott\"(admin,operator)")); assertTrue(result.contains("\"scott\"(admin,operator)"));
//remove a user checkRole("scott", roleFile, "admin", "operator");
RemoveUser rmCmd = new RemoveUser(); assertTrue(checkPassword("scott", "tiger", userFile));
rmCmd.setUsername("guest");
rmCmd.execute(ActionContext.system());
//check //add an existing user
context = new TestActionContext(); addCmd = new AddUser();
listCmd.execute(context); addCmd.setUserCommandUser("scott");
result = context.getStdout(); addCmd.setUserCommandPassword("password");
log.debug("output5:\n" + result); addCmd.setRole("visitor");
addCmd.setUser("admin");
addCmd.setPassword("admin");
context = new TestActionContext();
addCmd.execute(context);
result = context.getStderr();
assertTrue(result.contains("Failed to add user scott. Reason: AMQ229223: User scott already exists"));
assertTrue(result.contains("\"admin\"(amq)")); //check existing users are intact
assertFalse(result.contains("\"guest\"(admin)")); context = new TestActionContext();
assertTrue(result.contains("\"scott\"(admin,operator)") || result.contains("\"scott\"(operator,admin)")); listCmd.execute(context);
assertTrue(result.contains("Total: 2")); result = context.getStdout();
log.debug("output4:\n" + result);
//remove another assertTrue(result.contains("\"admin\"(amq)"));
rmCmd = new RemoveUser(); assertTrue(result.contains("\"guest\"(admin)"));
rmCmd.setUsername("scott"); assertTrue(result.contains("\"scott\"(admin,operator)"));
rmCmd.execute(ActionContext.system());
//check //remove a user
context = new TestActionContext(); RemoveUser rmCmd = new RemoveUser();
listCmd.execute(context); rmCmd.setUserCommandUser("guest");
result = context.getStdout(); rmCmd.setUser("admin");
log.debug("output6:\n" + result); rmCmd.setPassword("admin");
assertTrue(result.contains("\"admin\"(amq)"));
assertFalse(result.contains("\"guest\"(admin)"));
assertFalse(result.contains("\"scott\"(admin,operator)") || result.contains("\"scott\"(operator,admin)"));
assertTrue(result.contains("Total: 1"));
//remove non-exist
rmCmd = new RemoveUser();
rmCmd.setUsername("alien");
try {
rmCmd.execute(ActionContext.system()); rmCmd.execute(ActionContext.system());
fail("should throw exception when removing a non-existing user");
} catch (IllegalArgumentException expected) { //check
context = new TestActionContext();
listCmd.execute(context);
result = context.getStdout();
log.debug("output5:\n" + result);
assertTrue(result.contains("\"admin\"(amq)"));
assertFalse(result.contains("\"guest\"(admin)"));
assertTrue(result.contains("\"scott\"(admin,operator)") || result.contains("\"scott\"(operator,admin)"));
assertTrue(result.contains("Total: 2"));
//remove another
rmCmd = new RemoveUser();
rmCmd.setUserCommandUser("scott");
rmCmd.setUser("admin");
rmCmd.setPassword("admin");
rmCmd.execute(ActionContext.system());
//check
context = new TestActionContext();
listCmd.execute(context);
result = context.getStdout();
log.debug("output6:\n" + result);
assertTrue(result.contains("\"admin\"(amq)"));
assertFalse(result.contains("\"guest\"(admin)"));
assertFalse(result.contains("\"scott\"(admin,operator)") || result.contains("\"scott\"(operator,admin)"));
assertTrue(result.contains("Total: 1"));
//remove non-exist
rmCmd = new RemoveUser();
rmCmd.setUserCommandUser("alien");
rmCmd.setUser("admin");
rmCmd.setPassword("admin");
context = new TestActionContext();
rmCmd.execute(context);
result = context.getStderr();
assertTrue(result.contains("Failed to remove user alien. Reason: AMQ229224: User alien does not exist"));
//check
context = new TestActionContext();
listCmd.execute(context);
result = context.getStdout();
log.debug("output7:\n" + result);
assertTrue(result.contains("\"admin\"(amq)"));
assertTrue(result.contains("Total: 1"));
//now remove last
rmCmd = new RemoveUser();
rmCmd.setUserCommandUser("admin");
rmCmd.setUser("admin");
rmCmd.setPassword("admin");
rmCmd.execute(ActionContext.system());
//check
context = new TestActionContext();
listCmd.execute(context);
result = context.getStdout();
log.debug("output8:\n" + result);
assertTrue(result.contains("Total: 0"));
} finally {
stopServer();
} }
//check
context = new TestActionContext();
listCmd.execute(context);
result = context.getStdout();
log.debug("output7:\n" + result);
assertTrue(result.contains("\"admin\"(amq)"));
assertTrue(result.contains("Total: 1"));
//now remove last
rmCmd = new RemoveUser();
rmCmd.setUsername("admin");
rmCmd.execute(ActionContext.system());
//check
context = new TestActionContext();
listCmd.execute(context);
result = context.getStdout();
log.debug("output8:\n" + result);
assertTrue(result.contains("Total: 0"));
} }
@Test @Test
@ -687,98 +706,223 @@ public class ArtemisTest extends CliTestBase {
Run.setEmbedded(true); Run.setEmbedded(true);
File instance1 = new File(temporaryFolder.getRoot(), "instance_user"); File instance1 = new File(temporaryFolder.getRoot(), "instance_user");
System.setProperty("java.security.auth.login.config", instance1.getAbsolutePath() + "/etc/login.config"); System.setProperty("java.security.auth.login.config", instance1.getAbsolutePath() + "/etc/login.config");
Artemis.main("create", instance1.getAbsolutePath(), "--silent", "--no-autotune"); Artemis.main("create", instance1.getAbsolutePath(), "--silent", "--no-autotune", "--no-web", "--require-login");
System.setProperty("artemis.instance", instance1.getAbsolutePath()); System.setProperty("artemis.instance", instance1.getAbsolutePath());
Artemis.internalExecute("run");
File userFile = new File(instance1.getAbsolutePath() + "/etc/artemis-users.properties"); try {
File roleFile = new File(instance1.getAbsolutePath() + "/etc/artemis-roles.properties"); File userFile = new File(instance1.getAbsolutePath() + "/etc/artemis-users.properties");
File roleFile = new File(instance1.getAbsolutePath() + "/etc/artemis-roles.properties");
ListUser listCmd = new ListUser(); ListUser listCmd = new ListUser();
TestActionContext context = new TestActionContext(); listCmd.setUser("admin");
listCmd.execute(context); listCmd.setPassword("admin");
TestActionContext context = new TestActionContext();
listCmd.execute(context);
String result = context.getStdout(); String result = context.getStdout();
log.debug("output1:\n" + result); log.debug("output1:\n" + result);
//default only one user admin with role amq //default only one user admin with role amq
assertTrue(result.contains("\"admin\"(amq)")); assertTrue(result.contains("\"admin\"(amq)"));
//remove a user //remove a user
RemoveUser rmCmd = new RemoveUser(); RemoveUser rmCmd = new RemoveUser();
rmCmd.setUsername("admin"); rmCmd.setUserCommandUser("admin");
rmCmd.execute(ActionContext.system()); rmCmd.setUser("admin");
rmCmd.setPassword("admin");
rmCmd.execute(ActionContext.system());
//check //check
context = new TestActionContext(); context = new TestActionContext();
listCmd.execute(context); listCmd.execute(context);
result = context.getStdout(); result = context.getStdout();
log.debug("output8:\n" + result); log.debug("output8:\n" + result);
assertTrue(result.contains("Total: 0")); assertTrue(result.contains("Total: 0"));
//add some users //add some users
AddUser addCmd = new AddUser(); AddUser addCmd = new AddUser();
addCmd.setUsername("guest"); addCmd.setUserCommandUser("guest");
addCmd.setPassword("guest123"); addCmd.setUserCommandPassword("guest123");
addCmd.setRole("admin"); addCmd.setRole("admin");
addCmd.execute(new TestActionContext()); addCmd.setUser("admin");
addCmd.setPassword("admin");
addCmd.execute(new TestActionContext());
addCmd.setUsername("user1"); addCmd.setUserCommandUser("user1");
addCmd.setPassword("password1"); addCmd.setUserCommandPassword("password1");
addCmd.setRole("admin,manager"); addCmd.setRole("admin,manager");
addCmd.execute(new TestActionContext()); addCmd.execute(new TestActionContext());
assertTrue(checkPassword("user1", "password1", userFile)); assertTrue(checkPassword("user1", "password1", userFile));
addCmd.setUsername("user2"); addCmd.setUserCommandUser("user2");
addCmd.setPassword("password2"); addCmd.setUserCommandPassword("password2");
addCmd.setRole("admin,manager,master"); addCmd.setRole("admin,manager,master");
addCmd.execute(new TestActionContext()); addCmd.execute(new TestActionContext());
addCmd.setUsername("user3"); addCmd.setUserCommandUser("user3");
addCmd.setPassword("password3"); addCmd.setUserCommandPassword("password3");
addCmd.setRole("system,master"); addCmd.setRole("system,master");
addCmd.execute(new TestActionContext()); addCmd.execute(new TestActionContext());
//verify use list cmd //verify use list cmd
context = new TestActionContext(); context = new TestActionContext();
listCmd.execute(context); listCmd.execute(context);
result = context.getStdout(); result = context.getStdout();
log.debug("output2:\n" + result); log.debug("output2:\n" + result);
assertTrue(result.contains("Total: 4")); assertTrue(result.contains("Total: 4"));
assertTrue(result.contains("\"guest\"(admin)")); assertTrue(result.contains("\"guest\"(admin)"));
assertTrue(Pattern.compile("\"user1\"\\((admin|manager),(admin|manager)\\)").matcher(result).find()); assertTrue(Pattern
assertTrue(Pattern.compile("\"user2\"\\((admin|manager|master),(admin|manager|master),(admin|manager|master)\\)").matcher(result).find()); .compile("\"user1\"\\((admin|manager),(admin|manager)\\)")
assertTrue(Pattern.compile("\"user3\"\\((master|system),(master|system)\\)").matcher(result).find()); .matcher(result)
.find());
assertTrue(Pattern
.compile("\"user2\"\\((admin|manager|master),(admin|manager|master),(admin|manager|master)\\)")
.matcher(result)
.find());
assertTrue(Pattern
.compile("\"user3\"\\((master|system),(master|system)\\)")
.matcher(result)
.find());
checkRole("user1", roleFile, "admin", "manager"); checkRole("user1", roleFile, "admin", "manager");
//reset password //reset password
context = new TestActionContext(); context = new TestActionContext();
ResetUser resetCommand = new ResetUser(); ResetUser resetCommand = new ResetUser();
resetCommand.setUsername("user1"); resetCommand.setUserCommandUser("user1");
resetCommand.setPassword("newpassword1"); resetCommand.setUserCommandPassword("newpassword1");
resetCommand.execute(context); resetCommand.setUser("admin");
resetCommand.setPassword("admin");
resetCommand.execute(context);
checkRole("user1", roleFile, "admin", "manager"); checkRole("user1", roleFile, "admin", "manager");
assertFalse(checkPassword("user1", "password1", userFile)); assertFalse(checkPassword("user1", "password1", userFile));
assertTrue(checkPassword("user1", "newpassword1", userFile)); assertTrue(checkPassword("user1", "newpassword1", userFile));
//reset role //reset role
resetCommand.setUsername("user2"); resetCommand.setUserCommandUser("user2");
resetCommand.setRole("manager,master,operator"); resetCommand.setRole("manager,master,operator");
resetCommand.execute(new TestActionContext()); resetCommand.execute(new TestActionContext());
checkRole("user2", roleFile, "manager", "master", "operator"); checkRole("user2", roleFile, "manager", "master", "operator");
//reset both //reset both
resetCommand.setUsername("user3"); resetCommand.setUserCommandUser("user3");
resetCommand.setPassword("newpassword3"); resetCommand.setUserCommandPassword("newpassword3");
resetCommand.setRole("admin,system"); resetCommand.setRole("admin,system");
resetCommand.execute(new ActionContext()); resetCommand.execute(new ActionContext());
checkRole("user3", roleFile, "admin", "system"); checkRole("user3", roleFile, "admin", "system");
assertTrue(checkPassword("user3", "newpassword3", userFile)); assertTrue(checkPassword("user3", "newpassword3", userFile));
} finally {
stopServer();
}
}
@Test
public void testConcurrentUserAdministration() 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");
Artemis.main("create", instance1.getAbsolutePath(), "--silent", "--no-autotune", "--no-web", "--require-login");
System.setProperty("artemis.instance", instance1.getAbsolutePath());
Artemis.internalExecute("run");
try {
File userFile = new File(instance1.getAbsolutePath() + "/etc/artemis-users.properties");
File roleFile = new File(instance1.getAbsolutePath() + "/etc/artemis-roles.properties");
TestActionContext context = new TestActionContext();
ListUser listCmd = new ListUser();
listCmd.setUser("admin");
listCmd.setPassword("admin");
listCmd.execute(context);
String result = context.getStdout();
log.debug("output1:\n" + result);
assertTrue(result.contains("Total: 1"));
assertTrue(result.contains("\"admin\"(amq)"));
int nThreads = 4;
UserAdmin[] userAdminThreads = new UserAdmin[nThreads];
for (int j = 0; j < 25; j++) {
for (int i = 0; i < nThreads; i++) {
userAdminThreads[i] = new UserAdmin();
}
for (int i = 0; i < nThreads; i++) {
userAdminThreads[i].start();
}
for (UserAdmin userAdmin : userAdminThreads) {
userAdmin.join();
}
}
context = new TestActionContext();
listCmd = new ListUser();
listCmd.setUser("admin");
listCmd.setPassword("admin");
listCmd.execute(context);
result = context.getStdout();
log.debug("output2:\n" + result);
// make sure the admin user is still in tact (i.e. that the file wasn't corrupted via concurrent access)
assertTrue(result.contains("\"admin\"(amq)"));
checkRole("admin", roleFile, "amq");
checkPassword("admin", "admin", userFile);
} finally {
stopServer();
}
}
private class UserAdmin extends Thread {
@Override
public void run() {
//remove "myuser""
RemoveUser rmCmd = new RemoveUser();
rmCmd.setUserCommandUser("myuser");
rmCmd.setUser("admin");
rmCmd.setPassword("admin");
try {
rmCmd.execute(new TestActionContext());
} catch (Exception e) {
// this could fail if the user doesn't exist
}
//create user 'myuser' with password 'mypassword'
AddUser addCmd = new AddUser();
addCmd.setUserCommandUser("myuser");
addCmd.setUserCommandPassword("mypassword");
addCmd.setRole("foo");
addCmd.setUser("admin");
addCmd.setPassword("admin");
try {
addCmd.execute(new TestActionContext());
} catch (Exception e) {
// this could fail if the user already exists
}
//reset 'myuser' with role 'myrole'
ResetUser resetCmd = new ResetUser();
resetCmd.setUserCommandUser("myuser");
resetCmd.setUserCommandPassword("mypassword");
resetCmd.setRole("myrole");
resetCmd.setUser("admin");
resetCmd.setPassword("admin");
try {
resetCmd.execute(new TestActionContext());
} catch (Exception e) {
// this could fail if the user doesn't exist
}
}
} }
@Test @Test
@ -787,21 +931,28 @@ public class ArtemisTest extends CliTestBase {
Run.setEmbedded(true); Run.setEmbedded(true);
File instanceRole = new File(temporaryFolder.getRoot(), "instance_role"); File instanceRole = new File(temporaryFolder.getRoot(), "instance_role");
System.setProperty("java.security.auth.login.config", instanceRole.getAbsolutePath() + "/etc/login.config"); System.setProperty("java.security.auth.login.config", instanceRole.getAbsolutePath() + "/etc/login.config");
Artemis.main("create", instanceRole.getAbsolutePath(), "--silent", "--no-autotune", "--role", roleWithSpaces); Artemis.main("create", instanceRole.getAbsolutePath(), "--silent", "--no-autotune", "--no-web", "--require-login", "--role", roleWithSpaces);
System.setProperty("artemis.instance", instanceRole.getAbsolutePath()); System.setProperty("artemis.instance", instanceRole.getAbsolutePath());
Artemis.internalExecute("run");
File roleFile = new File(instanceRole.getAbsolutePath() + "/etc/artemis-roles.properties"); try {
File roleFile = new File(instanceRole.getAbsolutePath() + "/etc/artemis-roles.properties");
ListUser listCmd = new ListUser(); ListUser listCmd = new ListUser();
TestActionContext context = new TestActionContext(); listCmd.setUser("admin");
listCmd.execute(context); listCmd.setPassword("admin");
TestActionContext context = new TestActionContext();
listCmd.execute(context);
String result = context.getStdout(); String result = context.getStdout();
log.debug("output1:\n" + result); log.debug("output1:\n" + result);
assertTrue(result.contains("\"admin\"(" + roleWithSpaces + ")")); assertTrue(result.contains("\"admin\"(" + roleWithSpaces + ")"));
checkRole("admin", roleFile, roleWithSpaces); checkRole("admin", roleFile, roleWithSpaces);
} finally {
stopServer();
}
} }
@Test @Test

104
artemis-server/src/main/java/org/apache/activemq/artemis/core/management/impl/ActiveMQServerControlImpl.java
View File

@ -155,9 +155,8 @@ public class ActiveMQServerControlImpl extends AbstractControl implements Active
private final NotificationBroadcasterSupport broadcaster; private final NotificationBroadcasterSupport broadcaster;
private final AtomicLong notifSeq = new AtomicLong(0); private final AtomicLong notifSeq = new AtomicLong(0);
// Static --------------------------------------------------------
// Constructors -------------------------------------------------- private final Object userLock = new Object();
public ActiveMQServerControlImpl(final PostOffice postOffice, public ActiveMQServerControlImpl(final PostOffice postOffice,
final Configuration configuration, final Configuration configuration,
@ -4183,18 +4182,16 @@ public class ActiveMQServerControlImpl extends AbstractControl implements Active
@Override @Override
public void addUser(String username, String password, String roles, boolean plaintext) throws Exception { public void addUser(String username, String password, String roles, boolean plaintext) throws Exception {
if (AuditLogger.isEnabled()) { synchronized (userLock) {
AuditLogger.addUser(this.server, username, "****", roles, plaintext); if (AuditLogger.isEnabled()) {
AuditLogger.addUser(this.server, username, "****", roles, plaintext);
}
tcclInvoke(ActiveMQServerControlImpl.class.getClassLoader(), () -> {
PropertiesLoginModuleConfigurator config = getPropertiesLoginModuleConfigurator();
config.addNewUser(username, plaintext ? password : PasswordMaskingUtil.getHashProcessor().hash(password), roles.split(","));
config.save();
});
} }
tcclInvoke(ActiveMQServerControlImpl.class.getClassLoader(), () -> internalAddUser(username, password, roles, plaintext));
}
private void internalAddUser(String username, String password, String roles, boolean plaintext) throws Exception {
PropertiesLoginModuleConfigurator config = getPropertiesLoginModuleConfigurator();
config.addNewUser(username, plaintext ? password : PasswordMaskingUtil.getHashProcessor().hash(password), roles.split(","));
config.save();
} }
private String getSecurityDomain() { private String getSecurityDomain() {
@ -4203,64 +4200,67 @@ public class ActiveMQServerControlImpl extends AbstractControl implements Active
@Override @Override
public String listUser(String username) throws Exception { public String listUser(String username) throws Exception {
if (AuditLogger.isEnabled()) { synchronized (userLock) {
AuditLogger.listUser(this.server, username); if (AuditLogger.isEnabled()) {
} AuditLogger.listUser(this.server, username);
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);
JsonArrayBuilder users = JsonLoader.createArrayBuilder();
for (Entry<String, Set<String>> entry : info.entrySet()) {
JsonObjectBuilder user = JsonLoader.createObjectBuilder();
user.add("username", entry.getKey());
JsonArrayBuilder roles = JsonLoader.createArrayBuilder();
for (String role : entry.getValue()) {
roles.add(role);
} }
user.add("roles", roles);
users.add(user); return (String) tcclCall(ActiveMQServerControlImpl.class.getClassLoader(), () -> {
PropertiesLoginModuleConfigurator config = getPropertiesLoginModuleConfigurator();
Map<String, Set<String>> info = config.listUser(username);
JsonArrayBuilder users = JsonLoader.createArrayBuilder();
for (Entry<String, Set<String>> entry : info.entrySet()) {
JsonObjectBuilder user = JsonLoader.createObjectBuilder();
user.add("username", entry.getKey());
JsonArrayBuilder roles = JsonLoader.createArrayBuilder();
for (String role : entry.getValue()) {
roles.add(role);
}
user.add("roles", roles);
users.add(user);
}
return users.build().toString();
});
} }
return users.build().toString();
} }
@Override @Override
public void removeUser(String username) throws Exception { public void removeUser(String username) throws Exception {
if (AuditLogger.isEnabled()) { synchronized (userLock) {
AuditLogger.removeUser(this.server, username); if (AuditLogger.isEnabled()) {
AuditLogger.removeUser(this.server, username);
}
tcclInvoke(ActiveMQServerControlImpl.class.getClassLoader(), () -> {
PropertiesLoginModuleConfigurator config = getPropertiesLoginModuleConfigurator();
config.removeUser(username);
config.save();
});
} }
tcclInvoke(ActiveMQServerControlImpl.class.getClassLoader(), () -> internalRemoveUser(username));
}
private void internalRemoveUser(String username) throws Exception {
PropertiesLoginModuleConfigurator config = getPropertiesLoginModuleConfigurator();
config.removeUser(username);
config.save();
} }
@Override @Override
public void resetUser(String username, String password, String roles, boolean plaintext) throws Exception { public void resetUser(String username, String password, String roles, boolean plaintext) throws Exception {
if (AuditLogger.isEnabled()) { synchronized (userLock) {
AuditLogger.resetUser(this.server, username, "****", roles, plaintext); if (AuditLogger.isEnabled()) {
AuditLogger.resetUser(this.server, username, "****", roles, plaintext);
}
tcclInvoke(ActiveMQServerControlImpl.class.getClassLoader(), () -> {
PropertiesLoginModuleConfigurator config = getPropertiesLoginModuleConfigurator();
// 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();
});
} }
tcclInvoke(ActiveMQServerControlImpl.class.getClassLoader(), () -> internalresetUser(username, password, roles, plaintext));
} }
@Override @Override
public void resetUser(String username, String password, String roles) throws Exception { public void resetUser(String username, String password, String roles) throws Exception {
// no need to synchronize here because the method we call is synchronized
resetUser(username, password, roles, true); resetUser(username, password, roles, true);
} }
private void internalresetUser(String username, String password, String roles, boolean plaintext) throws Exception {
PropertiesLoginModuleConfigurator config = getPropertiesLoginModuleConfigurator();
// 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();
}
private PropertiesLoginModuleConfigurator getPropertiesLoginModuleConfigurator() throws Exception { private PropertiesLoginModuleConfigurator getPropertiesLoginModuleConfigurator() throws Exception {
URL configurationUrl = server.getConfiguration().getConfigurationUrl(); URL configurationUrl = server.getConfiguration().getConfigurationUrl();
if (configurationUrl == null) { if (configurationUrl == null) {

9
artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/PropertiesLoginModuleConfigurator.java
View File

@ -134,8 +134,13 @@ public class PropertiesLoginModuleConfigurator {
} }
public void save() throws Exception { public void save() throws Exception {
userBuilder.save(); ReloadableProperties.LOCK.writeLock().lock();
roleBuilder.save(); try {
userBuilder.save();
roleBuilder.save();
} finally {
ReloadableProperties.LOCK.writeLock().unlock();
}
} }
public void removeUser(String username) { public void removeUser(String username) {

9
artemis-server/src/main/java/org/apache/activemq/artemis/spi/core/security/jaas/ReloadableProperties.java
View File

@ -24,6 +24,8 @@ import java.util.HashSet;
import java.util.Map; import java.util.Map;
import java.util.Properties; import java.util.Properties;
import java.util.Set; import java.util.Set;
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
import java.util.regex.Pattern; import java.util.regex.Pattern;
import java.util.regex.PatternSyntaxException; import java.util.regex.PatternSyntaxException;
@ -34,6 +36,9 @@ public class ReloadableProperties {
private static final Logger logger = Logger.getLogger(ReloadableProperties.class); private static final Logger logger = Logger.getLogger(ReloadableProperties.class);
// use this whenever writing to the underlying properties files from another component
public static final ReadWriteLock LOCK = new ReentrantReadWriteLock();
private Properties props = new Properties(); private Properties props = new Properties();
private Map<String, String> invertedProps; private Map<String, String> invertedProps;
private Map<String, Set<String>> invertedValueProps; private Map<String, Set<String>> invertedValueProps;
@ -121,6 +126,7 @@ public class ReloadableProperties {
} }
private void load(final File source, Properties props) throws IOException { private void load(final File source, Properties props) throws IOException {
LOCK.readLock().lock();
try (FileInputStream in = new FileInputStream(source)) { try (FileInputStream in = new FileInputStream(source)) {
props.load(in); props.load(in);
// if (key.isDecrypt()) { // if (key.isDecrypt()) {
@ -132,7 +138,8 @@ public class ReloadableProperties {
// ActiveMQServerLogger.LOGGER.info("jasypt is not on the classpath: password decryption disabled."); // ActiveMQServerLogger.LOGGER.info("jasypt is not on the classpath: password decryption disabled.");
// } // }
// } // }
} finally {
LOCK.readLock().unlock();
} }
} }