Skip to content

Commit 6684375

Browse files
authored
Restore behavior for force parameter (#44847)
Turns out that the behavior of `-f` for the add and add-file sub commands where it would also forcibly create the keystore if it didn't exist, was by design - although undocumented. This change restores that behavior auto-creating a keystore that is not password protected if the force flag is used. The force OptionSpec is moved to the BaseKeyStoreCommand as we will presumably want to maintain the same behavior in any other command that takes a force option.
1 parent c7882c3 commit 6684375

File tree

6 files changed

+23
-11
lines changed

6 files changed

+23
-11
lines changed

distribution/docker/docker-test-entrypoint.sh

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
#!/bin/bash
22
cd /usr/share/elasticsearch/bin/
33
./elasticsearch-users useradd x_pack_rest_user -p x-pack-test-password -r superuser || true
4-
./elasticsearch-keystore create
54
echo "testnode" > /tmp/password
65
cat /tmp/password | ./elasticsearch-keystore add -x -f -v 'xpack.security.transport.ssl.keystore.secure_password'
76
cat /tmp/password | ./elasticsearch-keystore add -x -f -v 'xpack.security.http.ssl.keystore.secure_password'

distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddFileKeyStoreCommand.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,12 @@
3838
*/
3939
class AddFileKeyStoreCommand extends BaseKeyStoreCommand {
4040

41-
private final OptionSpec<Void> forceOption;
4241
private final OptionSpec<String> arguments;
4342

4443
AddFileKeyStoreCommand() {
4544
super("Add a file setting to the keystore", false);
46-
this.forceOption = parser.acceptsAll(Arrays.asList("f", "force"), "Overwrite existing setting without prompting");
45+
this.forceOption = parser.acceptsAll(Arrays.asList("f", "force"),
46+
"Overwrite existing setting without prompting, creating keystore if necessary");
4747
// jopt simple has issue with multiple non options, so we just get one set of them here
4848
// and convert to File when necessary
4949
// see https://github.com/jopt-simple/jopt-simple/issues/103
@@ -52,7 +52,6 @@ class AddFileKeyStoreCommand extends BaseKeyStoreCommand {
5252

5353
@Override
5454
protected void executeCommand(Terminal terminal, OptionSet options, Environment env) throws Exception {
55-
5655
List<String> argumentValues = arguments.values(options);
5756
if (argumentValues.size() == 0) {
5857
throw new UserException(ExitCodes.USAGE, "Missing setting name");

distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/AddStringKeyStoreCommand.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,13 @@
3838
class AddStringKeyStoreCommand extends BaseKeyStoreCommand {
3939

4040
private final OptionSpec<Void> stdinOption;
41-
private final OptionSpec<Void> forceOption;
4241
private final OptionSpec<String> arguments;
4342

4443
AddStringKeyStoreCommand() {
4544
super("Add a string setting to the keystore", false);
4645
this.stdinOption = parser.acceptsAll(Arrays.asList("x", "stdin"), "Read setting value from stdin");
47-
this.forceOption = parser.acceptsAll(Arrays.asList("f", "force"), "Overwrite existing setting without prompting");
46+
this.forceOption = parser.acceptsAll(Arrays.asList("f", "force"),
47+
"Overwrite existing setting without prompting, creating keystore if necessary");
4848
this.arguments = parser.nonOptions("setting name");
4949
}
5050

distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/BaseKeyStoreCommand.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
package org.elasticsearch.common.settings;
2121

2222
import joptsimple.OptionSet;
23+
import joptsimple.OptionSpec;
2324
import org.elasticsearch.cli.EnvironmentAwareCommand;
2425
import org.elasticsearch.cli.ExitCodes;
2526
import org.elasticsearch.cli.Terminal;
@@ -34,6 +35,7 @@ public abstract class BaseKeyStoreCommand extends EnvironmentAwareCommand {
3435
private KeyStoreWrapper keyStore;
3536
private SecureString keyStorePassword;
3637
private final boolean keyStoreMustExist;
38+
OptionSpec<Void> forceOption;
3739

3840
public BaseKeyStoreCommand(String description, boolean keyStoreMustExist) {
3941
super(description);
@@ -49,7 +51,7 @@ protected final void execute(Terminal terminal, OptionSet options, Environment e
4951
if (keyStoreMustExist) {
5052
throw new UserException(ExitCodes.DATA_ERROR, "Elasticsearch keystore not found at [" +
5153
KeyStoreWrapper.keystorePath(env.configFile()) + "]. Use 'create' command to create one.");
52-
} else {
54+
} else if (options.has(forceOption) == false) {
5355
if (terminal.promptYesNo("The elasticsearch keystore does not exist. Do you want to create it?", false) == false) {
5456
terminal.println("Exiting without creating keystore.");
5557
return;
@@ -101,8 +103,7 @@ static SecureString readPassword(Terminal terminal, boolean withVerification) th
101103
} else {
102104
passwordArray = terminal.readSecret("Enter password for the elasticsearch keystore : ");
103105
}
104-
final SecureString password = new SecureString(passwordArray);
105-
return password;
106+
return new SecureString(passwordArray);
106107
}
107108

108109
/**

distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddFileKeyStoreCommandTests.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,21 @@ private void addFile(KeyStoreWrapper keystore, String setting, Path file, String
5858
keystore.save(env.configFile(), password.toCharArray());
5959
}
6060

61-
public void testMissingCreateWithEmptyPassword() throws Exception {
61+
public void testMissingCreateWithEmptyPasswordWhenPrompted() throws Exception {
6262
String password = "";
6363
Path file1 = createRandomFile();
6464
terminal.addTextInput("y");
6565
execute("foo", file1.toString());
6666
assertSecureFile("foo", file1, password);
6767
}
6868

69+
public void testMissingCreateWithEmptyPasswordWithoutPromptIfForced() throws Exception {
70+
String password = "";
71+
Path file1 = createRandomFile();
72+
execute("-f", "foo", file1.toString());
73+
assertSecureFile("foo", file1, password);
74+
}
75+
6976
public void testMissingNoCreate() throws Exception {
7077
terminal.addSecretInput(randomFrom("", "keystorepassword"));
7178
terminal.addTextInput("n"); // explicit no

distribution/tools/keystore-cli/src/test/java/org/elasticsearch/common/settings/AddStringKeyStoreCommandTests.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,13 +59,19 @@ public void testInvalidPassphrease() throws Exception {
5959

6060
}
6161

62-
public void testMissingPromptCreateWithoutPassword() throws Exception {
62+
public void testMissingPromptCreateWithoutPasswordWhenPrompted() throws Exception {
6363
terminal.addTextInput("y");
6464
terminal.addSecretInput("bar");
6565
execute("foo");
6666
assertSecureString("foo", "bar", "");
6767
}
6868

69+
public void testMissingPromptCreateWithoutPasswordWithoutPromptIfForced() throws Exception {
70+
terminal.addSecretInput("bar");
71+
execute("-f", "foo");
72+
assertSecureString("foo", "bar", "");
73+
}
74+
6975
public void testMissingNoCreate() throws Exception {
7076
terminal.addTextInput("n"); // explicit no
7177
execute("foo");

0 commit comments

Comments
 (0)