feedback
This commit is contained in:
parent
d4288cce79
commit
4e4a40df7a
|
@ -27,6 +27,7 @@ import java.io.IOException;
|
|||
import java.io.InputStreamReader;
|
||||
import java.io.PrintWriter;
|
||||
import java.nio.charset.Charset;
|
||||
import java.util.Locale;
|
||||
|
||||
/**
|
||||
* A Terminal wraps access to reading input and writing output for a cli.
|
||||
|
@ -92,6 +93,26 @@ public abstract class Terminal {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Prompt for a yes or no answer from the user. This method will loop until 'y' or 'n'
|
||||
* (or the default empty value) is entered.
|
||||
*/
|
||||
public final boolean promptYesNo(String prompt, boolean defaultYes) {
|
||||
String answerPrompt = defaultYes ? " [Y/n]" : " [y/N]";
|
||||
while (true) {
|
||||
String answer = readText(prompt + answerPrompt).toLowerCase(Locale.ROOT);
|
||||
if (answer.isEmpty()) {
|
||||
return defaultYes;
|
||||
}
|
||||
boolean answerYes = answer.equals("y");
|
||||
if (answerYes == false && answer.equals("n") == false) {
|
||||
println("Did not understand answer '" + answer + "'");
|
||||
continue;
|
||||
}
|
||||
return answerYes;
|
||||
}
|
||||
}
|
||||
|
||||
private static class ConsoleTerminal extends Terminal {
|
||||
|
||||
private static final Console CONSOLE = System.console();
|
||||
|
|
|
@ -24,6 +24,7 @@ import java.io.InputStream;
|
|||
import java.io.InputStreamReader;
|
||||
import java.nio.charset.StandardCharsets;
|
||||
import java.util.Arrays;
|
||||
import java.util.Locale;
|
||||
|
||||
import joptsimple.OptionSet;
|
||||
import joptsimple.OptionSpec;
|
||||
|
@ -65,8 +66,7 @@ class AddStringKeyStoreCommand extends EnvironmentAwareCommand {
|
|||
|
||||
String setting = arguments.value(options);
|
||||
if (keystore.getSettings().contains(setting) && options.has(forceOption) == false) {
|
||||
String answer = terminal.readText("Setting " + setting + " already exists. Overwrite? [y/N]");
|
||||
if (answer.equals("y") == false) {
|
||||
if (terminal.promptYesNo("Setting " + setting + " already exists. Overwrite?", false) == false) {
|
||||
terminal.println("Exiting without modifying keystore.");
|
||||
return;
|
||||
}
|
||||
|
|
|
@ -40,8 +40,7 @@ class CreateKeyStoreCommand extends EnvironmentAwareCommand {
|
|||
protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception {
|
||||
Path keystoreFile = KeyStoreWrapper.keystorePath(env.configFile());
|
||||
if (Files.exists(keystoreFile)) {
|
||||
String answer = terminal.readText("An elasticsearch keystore already exists. Overwrite? [y/N] ");
|
||||
if (answer.equals("y") == false) {
|
||||
if (terminal.promptYesNo("An elasticsearch keystore already exists. Overwrite?", false) == false) {
|
||||
terminal.println("Exiting without creating keystore.");
|
||||
return;
|
||||
}
|
||||
|
|
|
@ -37,7 +37,6 @@ import java.security.KeyStore;
|
|||
import java.security.KeyStoreException;
|
||||
import java.security.NoSuchAlgorithmException;
|
||||
import java.util.Arrays;
|
||||
import java.util.Collections;
|
||||
import java.util.Enumeration;
|
||||
import java.util.HashSet;
|
||||
import java.util.Set;
|
||||
|
@ -59,15 +58,8 @@ public class KeyStoreWrapper implements Closeable {
|
|||
/** The keystore type for a newly created keystore. */
|
||||
private static final String NEW_KEYSTORE_TYPE = "PKCS12";
|
||||
|
||||
/** A factory necessary for constructing instances of secrets in a {@link KeyStore}. */
|
||||
private static final SecretKeyFactory passwordFactory;
|
||||
static {
|
||||
try {
|
||||
passwordFactory = SecretKeyFactory.getInstance("PBE");
|
||||
} catch (NoSuchAlgorithmException e) {
|
||||
throw new RuntimeException(e);
|
||||
}
|
||||
}
|
||||
/** The algorithm used to store password for a newly created keystore. */
|
||||
private static final String NEW_KEYSTORE_SECRET_KEY_ALGO = "PBEWithHmacSHA256AndAES_128";
|
||||
|
||||
/** True iff the keystore has a password needed to read. */
|
||||
private final boolean hasPassword;
|
||||
|
@ -75,6 +67,9 @@ public class KeyStoreWrapper implements Closeable {
|
|||
/** The type of the keystore, as passed to {@link java.security.KeyStore#getInstance(String)} */
|
||||
private final String type;
|
||||
|
||||
/** A factory necessary for constructing instances of secrets in a {@link KeyStore}. */
|
||||
private final SecretKeyFactory secretFactory;
|
||||
|
||||
/** A stream of the actual keystore data. */
|
||||
private final InputStream input;
|
||||
|
||||
|
@ -87,9 +82,14 @@ public class KeyStoreWrapper implements Closeable {
|
|||
/** The setting names contained in the loaded keystore. */
|
||||
private final Set<String> settingNames = new HashSet<>();
|
||||
|
||||
private KeyStoreWrapper(boolean hasPassword, String type, InputStream input) {
|
||||
private KeyStoreWrapper(boolean hasPassword, String type, String secretKeyAlgo, InputStream input) {
|
||||
this.hasPassword = hasPassword;
|
||||
this.type = type;
|
||||
try {
|
||||
secretFactory = SecretKeyFactory.getInstance(secretKeyAlgo);
|
||||
} catch (NoSuchAlgorithmException e) {
|
||||
throw new RuntimeException(e);
|
||||
}
|
||||
this.input = input;
|
||||
}
|
||||
|
||||
|
@ -100,7 +100,7 @@ public class KeyStoreWrapper implements Closeable {
|
|||
|
||||
/** Constructs a new keystore with the given password. */
|
||||
static KeyStoreWrapper create(char[] password) throws Exception {
|
||||
KeyStoreWrapper wrapper = new KeyStoreWrapper(password.length != 0, NEW_KEYSTORE_TYPE, null);
|
||||
KeyStoreWrapper wrapper = new KeyStoreWrapper(password.length != 0, NEW_KEYSTORE_TYPE, NEW_KEYSTORE_SECRET_KEY_ALGO, null);
|
||||
KeyStore keyStore = KeyStore.getInstance(NEW_KEYSTORE_TYPE);
|
||||
keyStore.load(null, null);
|
||||
wrapper.keystore.set(keyStore);
|
||||
|
@ -126,7 +126,8 @@ public class KeyStoreWrapper implements Closeable {
|
|||
}
|
||||
boolean hasPassword = inputStream.readBoolean();
|
||||
String type = inputStream.readUTF();
|
||||
return new KeyStoreWrapper(hasPassword, type, inputStream);
|
||||
String secretKeyAlgo = inputStream.readUTF();
|
||||
return new KeyStoreWrapper(hasPassword, type, secretKeyAlgo, inputStream);
|
||||
}
|
||||
|
||||
/** Returns true iff {@link #loadKeystore(char[])} has been called. */
|
||||
|
@ -164,6 +165,7 @@ public class KeyStoreWrapper implements Closeable {
|
|||
char[] password = this.keystorePassword.get().getPassword();
|
||||
outputStream.writeBoolean(password.length != 0);
|
||||
outputStream.writeUTF(type);
|
||||
outputStream.writeUTF(secretFactory.getAlgorithm());
|
||||
keystore.get().store(outputStream, password);
|
||||
}
|
||||
PosixFileAttributeView attrs = Files.getFileAttributeView(keystoreFile, PosixFileAttributeView.class);
|
||||
|
@ -186,7 +188,7 @@ public class KeyStoreWrapper implements Closeable {
|
|||
}
|
||||
// TODO: only allow getting a setting once?
|
||||
KeyStore.SecretKeyEntry secretKeyEntry = (KeyStore.SecretKeyEntry)entry;
|
||||
PBEKeySpec keySpec = (PBEKeySpec)passwordFactory.getKeySpec(secretKeyEntry.getSecretKey(), PBEKeySpec.class);
|
||||
PBEKeySpec keySpec = (PBEKeySpec) secretFactory.getKeySpec(secretKeyEntry.getSecretKey(), PBEKeySpec.class);
|
||||
SecureString value = new SecureString(keySpec.getPassword());
|
||||
keySpec.clearPassword();
|
||||
return value;
|
||||
|
@ -194,7 +196,7 @@ public class KeyStoreWrapper implements Closeable {
|
|||
|
||||
/** Set a string setting. */
|
||||
void setStringSetting(String setting, char[] value) throws GeneralSecurityException {
|
||||
SecretKey secretKey = passwordFactory.generateSecret(new PBEKeySpec(value));
|
||||
SecretKey secretKey = secretFactory.generateSecret(new PBEKeySpec(value));
|
||||
keystore.get().setEntry(setting, new KeyStore.SecretKeyEntry(secretKey), keystorePassword.get());
|
||||
settingNames.add(setting);
|
||||
}
|
||||
|
|
|
@ -54,10 +54,5 @@ class ListKeyStoreCommand extends EnvironmentAwareCommand {
|
|||
for (String entry : sortedEntries) {
|
||||
terminal.println(entry);
|
||||
}
|
||||
|
||||
// TODO:
|
||||
// 2. delete command
|
||||
// 3. tests for delete
|
||||
// 4. shell script
|
||||
}
|
||||
}
|
||||
|
|
|
@ -80,7 +80,7 @@ public final class SecureString implements CharSequence, AutoCloseable {
|
|||
}
|
||||
|
||||
/**
|
||||
* Convert to a {@link String}. This only be used with APIs that do not take {@link CharSequence}.
|
||||
* Convert to a {@link String}. This should only be used with APIs that do not take {@link CharSequence}.
|
||||
*/
|
||||
@Override
|
||||
public String toString() {
|
||||
|
|
|
@ -19,6 +19,7 @@
|
|||
|
||||
package org.elasticsearch.common.settings;
|
||||
|
||||
import org.apache.lucene.util.SetOnce;
|
||||
import org.elasticsearch.Version;
|
||||
import org.elasticsearch.common.Booleans;
|
||||
import org.elasticsearch.common.Strings;
|
||||
|
@ -612,7 +613,7 @@ public final class Settings implements ToXContent {
|
|||
// we use a sorted map for consistent serialization when using getAsMap()
|
||||
private final Map<String, String> map = new TreeMap<>();
|
||||
|
||||
private KeyStoreWrapper keystore;
|
||||
private SetOnce<KeyStoreWrapper> keystore = new SetOnce<>();
|
||||
|
||||
private Builder() {
|
||||
|
||||
|
@ -638,9 +639,10 @@ public final class Settings implements ToXContent {
|
|||
|
||||
/** Sets the secret store for these settings. */
|
||||
public void setKeyStore(KeyStoreWrapper keystore) {
|
||||
assert this.keystore == null; // only set once!
|
||||
assert keystore.isLoaded();
|
||||
this.keystore = Objects.requireNonNull(keystore);
|
||||
if (keystore.isLoaded()) {
|
||||
throw new IllegalStateException("The keystore wrapper must already be loaded");
|
||||
}
|
||||
this.keystore.set(Objects.requireNonNull(keystore));
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -1049,7 +1051,7 @@ public final class Settings implements ToXContent {
|
|||
* set on this builder.
|
||||
*/
|
||||
public Settings build() {
|
||||
return new Settings(map, keystore);
|
||||
return new Settings(map, keystore.get());
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -46,6 +46,33 @@ public class TerminalTests extends ESTestCase {
|
|||
assertPrinted(terminal, Terminal.Verbosity.NORMAL, "This message contains percent like %20n");
|
||||
}
|
||||
|
||||
public void testPromptYesNoDefault() throws Exception {
|
||||
MockTerminal terminal = new MockTerminal();
|
||||
terminal.addTextInput("");
|
||||
assertTrue(terminal.promptYesNo("Answer?", true));
|
||||
terminal.addTextInput("");
|
||||
assertFalse(terminal.promptYesNo("Answer?", false));
|
||||
}
|
||||
|
||||
public void testPromptYesNoReprompt() throws Exception {
|
||||
MockTerminal terminal = new MockTerminal();
|
||||
terminal.addTextInput("blah");
|
||||
terminal.addTextInput("y");
|
||||
assertTrue(terminal.promptYesNo("Answer? [Y/n]\nDid not understand answer 'blah'\nAnswer? [Y/n]", true));
|
||||
}
|
||||
|
||||
public void testPromptYesNoCase() throws Exception {
|
||||
MockTerminal terminal = new MockTerminal();
|
||||
terminal.addTextInput("Y");
|
||||
assertTrue(terminal.promptYesNo("Answer?", false));
|
||||
terminal.addTextInput("y");
|
||||
assertTrue(terminal.promptYesNo("Answer?", false));
|
||||
terminal.addTextInput("N");
|
||||
assertFalse(terminal.promptYesNo("Answer?", true));
|
||||
terminal.addTextInput("n");
|
||||
assertFalse(terminal.promptYesNo("Answer?", true));
|
||||
}
|
||||
|
||||
private void assertPrinted(MockTerminal logTerminal, Terminal.Verbosity verbosity, String text) throws Exception {
|
||||
logTerminal.println(verbosity, text);
|
||||
String output = logTerminal.getOutput();
|
||||
|
|
|
@ -19,6 +19,9 @@
|
|||
|
||||
package org.elasticsearch.common.settings;
|
||||
|
||||
import javax.crypto.SecretKeyFactory;
|
||||
import java.security.Provider;
|
||||
import java.security.Security;
|
||||
import java.util.Map;
|
||||
import java.util.Set;
|
||||
|
||||
|
@ -43,7 +46,7 @@ public class RemoveSettingKeyStoreCommandTests extends KeyStoreCommandTestCase {
|
|||
}
|
||||
|
||||
public void testMissing() throws Exception {
|
||||
UserException e = expectThrows(UserException.class, this::execute);
|
||||
UserException e = expectThrows(UserException.class, () -> execute("foo"));
|
||||
assertEquals(ExitCodes.DATA_ERROR, e.exitCode);
|
||||
assertThat(e.getMessage(), containsString("keystore not found"));
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue