epugh commented on code in PR #4688:
URL: https://github.com/apache/solr/pull/4688#discussion_r3690702846
##########
solr/core/src/java/org/apache/solr/cli/AuthTool.java:
##########
@@ -147,201 +165,178 @@ public Options getOptions() {
.addOptionGroup(getConnectionOptions());
}
- private void ensureArgumentIsValidBooleanIfPresent(CommandLine cli, Option
option) {
- if (cli.hasOption(option)) {
- final String value = cli.getOptionValue(option);
- if (!"true".equalsIgnoreCase(value) && !"false".equalsIgnoreCase(value))
{
- echo(
- "Argument ["
- + option.getLongOpt()
- + "] must be either true or false, but was ["
- + value
- + "]");
- runtime.exit(1);
- }
+ private void ensureArgumentIsValidBooleanIfPresent(String optionName, String
value) {
+ if (value != null && !"true".equalsIgnoreCase(value) &&
!"false".equalsIgnoreCase(value)) {
+ echo("Argument [" + optionName + "] must be either true or false, but
was [" + value + "]");
+ runtime.exit(1);
}
}
- private void handleBasicAuth(CommandLine cli) throws Exception {
- String cmd = cli.getArgs()[0];
- boolean prompt = Boolean.parseBoolean(cli.getOptionValue(PROMPT_OPTION,
"false"));
- boolean updateIncludeFileOnly =
- Boolean.parseBoolean(cli.getOptionValue(UPDATE_INCLUDE_FILE_OPTION,
"false"));
+ private void handleCommand(String cmd, AuthParams params) throws Exception {
switch (cmd) {
- case "enable":
- {
- if (!prompt && !cli.hasOption(CommonCLIOptions.CREDENTIALS_OPTION)) {
- CLIO.out("Option --credentials or --prompt is required with
enable.");
- runtime.exit(1);
- } else if (!prompt
- && (cli.getOptionValue(CommonCLIOptions.CREDENTIALS_OPTION) ==
null
- ||
!cli.getOptionValue(CommonCLIOptions.CREDENTIALS_OPTION).contains(":"))) {
- CLIO.out("Option --credentials is not in correct format.");
- runtime.exit(1);
- }
-
- String zkHost = null;
-
- if (!updateIncludeFileOnly) {
- try {
- zkHost = CLIUtils.getZkHost(cli);
- } catch (Exception ex) {
- if (cli.hasOption(CommonCLIOptions.ZK_HOST_OPTION)) {
- CLIO.out(
- "Couldn't get ZooKeeper host. Please make sure that
ZooKeeper is running and the correct zk-host has been passed in.");
- } else {
- CLIO.out(
- "Couldn't get ZooKeeper host. Please make sure Solr is
running in cloud mode, or a zk-host has been passed in.");
- }
- runtime.exit(1);
- }
- if (zkHost == null) {
- if (cli.hasOption(CommonCLIOptions.ZK_HOST_OPTION)) {
- CLIO.out(
- "Couldn't get ZooKeeper host. Please make sure that
ZooKeeper is running and the correct zk-host has been passed in.");
- } else {
- CLIO.out(
- "Couldn't get ZooKeeper host. Please make sure Solr is
running in cloud mode, or a zk-host has been passed in.");
- }
- runtime.exit(1);
- }
-
- // check if security is already enabled or not
- try (SolrZkClient zkClient = CLIUtils.getSolrZkClient(cli,
zkHost)) {
- checkSecurityJsonExists(zkClient);
- }
- }
-
- String username, password;
- if (cli.hasOption(CommonCLIOptions.CREDENTIALS_OPTION)) {
- String credentials =
cli.getOptionValue(CommonCLIOptions.CREDENTIALS_OPTION);
- username = credentials.split(":")[0];
- password = credentials.split(":")[1];
- } else {
- Console console = System.console();
- // keep prompting until they've entered a non-empty username &
password
- do {
- username = console.readLine("Enter username: ");
- } while (username == null || username.trim().isEmpty());
- username = username.trim();
-
- do {
- password = new String(console.readPassword("Enter password: "));
- } while (password.isEmpty());
- }
-
- if (username.equals(password)
- && !EnvUtils.getPropertyAsBool(
- Sha256AuthenticationProvider.ALLOW_USER_AS_PASSWORD_PROP,
false)) {
- CLIO.err(
- "Error: username and password must not be identical."
- + " This credential would never authenticate.");
- runtime.exit(1);
- }
-
- String resourceName = "security.json";
- final URL resource =
SolrCore.class.getClassLoader().getResource(resourceName);
- if (null == resource) {
- throw new IllegalArgumentException("invalid resource name: " +
resourceName);
- }
-
- ObjectMapper mapper = new ObjectMapper();
- JsonNode securityJson1 = mapper.readTree(resource.openStream());
- // Only override blockUnknown if explicitly passed; otherwise let
the template decide
- if (cli.hasOption(BLOCK_UNKNOWN_OPTION)) {
- boolean blockUnknown =
Boolean.parseBoolean(cli.getOptionValue(BLOCK_UNKNOWN_OPTION));
- ((ObjectNode)
securityJson1.get("authentication")).put("blockUnknown", blockUnknown);
- }
- JsonNode credentialsNode =
securityJson1.get("authentication").get("credentials");
- ((ObjectNode) credentialsNode)
- .put(username,
Sha256AuthenticationProvider.getSaltedHashedValue(password));
- JsonNode userRoleNode =
securityJson1.get("authorization").get("user-role");
- String[] predefinedRoles = {"superadmin", "admin", "search",
"index"};
- ArrayNode rolesNode = mapper.createArrayNode();
- for (String role : predefinedRoles) {
- rolesNode.add(role);
- }
- ((ObjectNode) userRoleNode).set(username, rolesNode);
- String securityJson = securityJson1.toPrettyString();
-
- if (!updateIncludeFileOnly) {
- echoIfVerbose("Uploading following security.json: " +
securityJson);
- try (SolrZkClient zkClient = CLIUtils.getSolrZkClient(cli,
zkHost)) {
- zkClient.makePath(
- "/security.json",
securityJson.getBytes(StandardCharsets.UTF_8), false);
- }
- }
-
- String solrIncludeFilename =
cli.getOptionValue(SOLR_INCLUDE_FILE_OPTION);
- Path includeFile = Path.of(solrIncludeFilename);
- if (Files.notExists(includeFile) || !Files.isWritable(includeFile)) {
- CLIO.out(
- "Solr include file " + solrIncludeFilename + " doesn't exist
or is not writeable.");
- printAuthEnablingInstructions(username, password);
- runtime.exit(0);
- }
- String authConfDir = cli.getOptionValue(AUTH_CONF_DIR_OPTION);
- Path basicAuthConfFile = Path.of(authConfDir, "basicAuth.conf");
-
- if (!Files.isWritable(basicAuthConfFile.getParent())) {
- CLIO.out("Cannot write to file: " +
basicAuthConfFile.toAbsolutePath());
- printAuthEnablingInstructions(username, password);
- runtime.exit(0);
- }
-
- Files.writeString(
- basicAuthConfFile,
- "httpBasicAuthUser=" + username + "\nhttpBasicAuthPassword=" +
password,
- StandardCharsets.UTF_8);
-
- // update the solr.in.sh file to contain the necessary
authentication lines
- updateIncludeFileEnableAuth(includeFile, basicAuthConfFile);
- final String successMessage =
- String.format(
- Locale.ROOT,
- "Successfully enabled basic auth with username [%s] assigned
to all roles (superadmin, admin, index, search).",
- username);
- echo(successMessage);
- if (!updateIncludeFileOnly) {
- Map<String, String> templateUsers = new LinkedHashMap<>();
- templateUsers.put("admin", "admin, index, search");
- templateUsers.put("index", "index, search");
- templateUsers.put("search", "search");
- templateUsers.remove(username);
- CLIO.out(
- "\nIMPORTANT: The following template users have been created
with NO password set"
- + " and cannot log in until passwords are assigned:");
- templateUsers.forEach((u, roles) -> CLIO.out(" - " + u + "
(roles: " + roles + ")"));
- CLIO.out(
- "Set their passwords using the Admin UI Security page or the
authentication API.");
- }
- return;
- }
- case "disable":
- {
- clearSecurityJson(cli, updateIncludeFileOnly);
-
- String solrIncludeFilename =
cli.getOptionValue(SOLR_INCLUDE_FILE_OPTION);
- Path includeFile = Path.of(solrIncludeFilename);
- if (Files.notExists(includeFile) || !Files.isWritable(includeFile)) {
- CLIO.out(
- "Solr include file " + solrIncludeFilename + " doesn't exist
or is not writeable.");
- CLIO.out(
- "Security has been disabled. Please remove any SOLR_AUTH_TYPE
or SOLR_AUTHENTICATION_OPTS configuration from solr.in.sh/solr.in.cmd.\n");
- runtime.exit(0);
- }
-
- // update the solr.in.sh file to comment out the necessary
authentication lines
- updateIncludeFileDisableAuth(includeFile);
- return;
- }
- default:
+ case "enable" -> enableBasicAuth(params);
Review Comment:
nice!
##########
solr/core/src/java/org/apache/solr/cli/AuthTool.java:
##########
@@ -103,6 +105,22 @@ public class AuthTool extends ToolBase {
"This is where any authentication related configuration files,
if any, would be placed. Defaults to $SOLR_HOME.")
.get();
+ /**
+ * Parameters for the auth sub-commands, independent of the command line
parser.
+ *
+ * @param zkHost resolved ZooKeeper connection string, or null if {@code
updateIncludeFileOnly} is
+ * set or the connection string could not be resolved
+ */
+ record AuthParams(
Review Comment:
nice!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]