diff --git a/shell/src/main/java/org/apache/accumulo/shell/Shell.java b/shell/src/main/java/org/apache/accumulo/shell/Shell.java index e40338e79c9..d7a821d593c 100644 --- a/shell/src/main/java/org/apache/accumulo/shell/Shell.java +++ b/shell/src/main/java/org/apache/accumulo/shell/Shell.java @@ -190,15 +190,6 @@ @AutoService(KeywordExecutable.class) public class Shell extends ShellOptions implements KeywordExecutable { - public static interface Authenticator { - default boolean authenticateUser(AccumuloClient client, String principal, - AuthenticationToken token) throws AccumuloException, AccumuloSecurityException { - return client.securityOperations().authenticateUser(principal, token); - } - } - - public static Authenticator AUTHENTICATOR = new Authenticator() {}; - public static final Logger log = LoggerFactory.getLogger(Shell.class); private static final Logger audit = LoggerFactory.getLogger(Shell.class.getName() + ".audit"); @@ -267,29 +258,29 @@ public Shell(LineReader reader) { this.writer = terminal.writer(); } - private boolean authenticateUser(AccumuloClient client, AuthenticationToken token) + // this is visible only for FateCommandTest, otherwise, should be private or inline + protected boolean authenticateUser(AccumuloClient client, AuthenticationToken token) throws AccumuloException, AccumuloSecurityException { - return AUTHENTICATOR.authenticateUser(client, client.whoami(), token); + return client.securityOperations().authenticateUser(client.whoami(), token); } - private AuthenticationToken getAuthenticationToken(String principal, - String authenticationString) { + private AuthenticationToken getAuthenticationToken(String principal, String password, + String passwordPrompt) { AuthenticationToken token = null; - if (authenticationString == null - && clientProperties.containsKey(ClientProperty.AUTH_TOKEN.getKey()) + if (password == null && clientProperties.containsKey(ClientProperty.AUTH_TOKEN.getKey()) && principal.equals(ClientProperty.AUTH_PRINCIPAL.getValue(clientProperties))) { token = ClientProperty.getAuthenticationToken(clientProperties); } if (token == null) { // Read password if the user explicitly asked for it, or didn't specify anything at all - if (PasswordConverter.STDIN.equals(authenticationString) || authenticationString == null) { - authenticationString = reader.readLine("Password: ", '*'); + if (PasswordConverter.STDIN.equals(password) || password == null) { + password = reader.readLine(passwordPrompt, '*'); } - if (authenticationString == null) { + if (password == null) { // User cancel, e.g. Ctrl-D pressed throw new ParameterException("No password or token option supplied"); } else { - token = new PasswordToken(authenticationString); + token = new PasswordToken(password); } } return token; @@ -369,13 +360,12 @@ public boolean config(String... args) throws IOException { exitCode = 1; return false; } - String authenticationString = options.getPassword(); - final AuthenticationToken authToken = getAuthenticationToken(principal, authenticationString); + String password = options.getPassword(); + final AuthenticationToken token = getAuthenticationToken(principal, password, "Password: "); try { this.setTableName(""); - accumuloClient = - Accumulo.newClient().from(clientProperties).as(principal, authToken).build(); - authenticateUser(accumuloClient, authToken); + accumuloClient = Accumulo.newClient().from(clientProperties).as(principal, token).build(); + authenticateUser(accumuloClient, token); context = (ClientContext) accumuloClient; } catch (Exception e) { printException(e); @@ -756,8 +746,8 @@ public void execCommand(String input, boolean ignoreAuthTimeout, boolean echoPro writer.println("Shell has been idle for too long. Please re-authenticate."); boolean authFailed = true; do { - final AuthenticationToken authToken = - getAuthenticationToken(accumuloClient.whoami(), null); + final AuthenticationToken authToken = getAuthenticationToken(accumuloClient.whoami(), + null, "Enter current password for '" + accumuloClient.whoami() + "': "); try { authFailed = !authenticateUser(accumuloClient, authToken); } catch (Exception e) { diff --git a/shell/src/test/java/org/apache/accumulo/shell/commands/FateCommandTest.java b/shell/src/test/java/org/apache/accumulo/shell/commands/FateCommandTest.java index aefbdcd94d2..0deacfb47a8 100644 --- a/shell/src/test/java/org/apache/accumulo/shell/commands/FateCommandTest.java +++ b/shell/src/test/java/org/apache/accumulo/shell/commands/FateCommandTest.java @@ -37,8 +37,6 @@ import java.util.List; import org.apache.accumulo.core.client.AccumuloClient; -import org.apache.accumulo.core.client.AccumuloException; -import org.apache.accumulo.core.client.AccumuloSecurityException; import org.apache.accumulo.core.client.security.tokens.AuthenticationToken; import org.apache.accumulo.core.clientImpl.ClientContext; import org.apache.accumulo.core.fate.AdminUtil; @@ -134,13 +132,6 @@ public void reset() { public static void setup() { zk = createMock(ZooReaderWriter.class); managerLockPath = createMock(ServiceLockPath.class); - Shell.AUTHENTICATOR = new Shell.Authenticator() { - @Override - public boolean authenticateUser(AccumuloClient client, String principal, - AuthenticationToken token) throws AccumuloException, AccumuloSecurityException { - return true; - } - }; } @Test @@ -331,7 +322,12 @@ private Shell createShell(TestOutputStream output) throws IOException { Terminal terminal = new DumbTerminal(new FileInputStream(FileDescriptor.in), output); terminal.setSize(new Size(80, 24)); LineReader reader = LineReaderBuilder.builder().terminal(terminal).build(); - Shell shell = new Shell(reader); + Shell shell = new Shell(reader) { + @Override + protected boolean authenticateUser(AccumuloClient client, AuthenticationToken token) { + return true; + } + }; shell.setLogErrorsToConsole(); return shell; } diff --git a/test/src/main/java/org/apache/accumulo/test/shell/ShellAuthenticatorIT.java b/test/src/main/java/org/apache/accumulo/test/shell/ShellAuthenticatorIT.java index d0ecbca3676..8dd631adea0 100644 --- a/test/src/main/java/org/apache/accumulo/test/shell/ShellAuthenticatorIT.java +++ b/test/src/main/java/org/apache/accumulo/test/shell/ShellAuthenticatorIT.java @@ -18,15 +18,10 @@ */ package org.apache.accumulo.test.shell; -import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; -import java.io.File; -import java.io.FileWriter; import java.io.IOException; -import java.nio.file.Files; -import java.util.Properties; import java.util.TimeZone; import org.apache.accumulo.harness.SharedMiniClusterBase; @@ -59,7 +54,6 @@ public static void teardown() { private StringInputStream input; private TestOutputStream output; private Shell shell; - private File config; public LineReader reader; public Terminal terminal; @@ -71,17 +65,6 @@ public void setupShell() throws IOException { terminal = new DumbTerminal(input, output); terminal.setSize(new Size(80, 24)); reader = LineReaderBuilder.builder().terminal(terminal).build(); - if (config != null) { - if (!config.delete()) { - throw new IllegalStateException("failed to delete: " + config.getAbsolutePath()); - } - } - config = Files.createTempFile(null, null).toFile(); - try (FileWriter writer = new FileWriter(config, UTF_8)) { - Properties p = SharedMiniClusterBase.getClientProps(); - p.store(writer, null); - } - config.deleteOnExit(); } @AfterEach @@ -95,7 +78,7 @@ public void tearDownShell() { public void testClientPropertiesFile() throws IOException { shell = new Shell(reader); shell.setLogErrorsToConsole(); - assertTrue(shell.config("--config-file", config.toString())); + assertTrue(shell.config("--config-file", getCluster().getClientPropsPath())); } @Test @@ -122,20 +105,10 @@ public void testAuthTimeoutPropertiesFile() throws IOException, InterruptedExcep terminal = new DumbTerminal(input, output); terminal.setSize(new Size(80, 24)); reader = LineReaderBuilder.builder().terminal(terminal).build(); - if (config != null) { - if (!config.delete()) { - throw new IllegalStateException("failed to delete: " + config.getAbsolutePath()); - } - } - config = Files.createTempFile(null, null).toFile(); - try (FileWriter writer = new FileWriter(config, UTF_8)) { - Properties p = SharedMiniClusterBase.getClientProps(); - p.store(writer, null); - } - config.deleteOnExit(); shell = new Shell(reader); shell.setLogErrorsToConsole(); - assertTrue(shell.config("--auth-timeout", "1", "--config-file", config.toString())); + assertTrue( + shell.config("--auth-timeout", "1", "--config-file", getCluster().getClientPropsPath())); Thread.sleep(90000); shell.execCommand("whoami", false, false); assertTrue(output.get().contains("root"));