From 78c1500bb98918572f06839980b4f88f21a9e9c7 Mon Sep 17 00:00:00 2001 From: Satia Herfert Date: Wed, 28 Aug 2019 09:17:04 +0200 Subject: [PATCH] Prompt silently if the output is redirected. This fixes #159. * Problem 1 is not solved: There still be no visible prompt if the output is redirected and that may be misinterpreted as a hanging process * Problem 2 is solved: Neither the prompt not username and password appear in the output if it is redirected. * Problem 3 is solved: TTY Echo is turned off earlier, so that one can start typing the password without having to fear it appearing in cleartext on the screen. Anything typed before we actually prompt is buffered and processed then, so it is totally fine now to type the password and enter as soon as the process is started. --- .../org/neo4j/shell/MainIntegrationTest.java | 50 +++++-- .../src/main/java/org/neo4j/shell/Main.java | 62 ++++++--- .../test/java/org/neo4j/shell/MainTest.java | 126 +++++++++++++++--- 3 files changed, 184 insertions(+), 54 deletions(-) diff --git a/cypher-shell/src/integration-test/java/org/neo4j/shell/MainIntegrationTest.java b/cypher-shell/src/integration-test/java/org/neo4j/shell/MainIntegrationTest.java index 7b7fb6be..963a0f99 100644 --- a/cypher-shell/src/integration-test/java/org/neo4j/shell/MainIntegrationTest.java +++ b/cypher-shell/src/integration-test/java/org/neo4j/shell/MainIntegrationTest.java @@ -1,5 +1,6 @@ package org.neo4j.shell; +import org.junit.Before; import org.junit.Test; import org.neo4j.shell.cli.CliArgs; import org.neo4j.shell.log.AnsiLogger; @@ -16,17 +17,21 @@ public class MainIntegrationTest { - @Test - public void connectInteractivelyPromptsOnWrongAuthentication() throws Exception { + private String inputString = String.format( "neo4j%nneo%n" ); + private ByteArrayOutputStream baos; + private ConnectionConfig connectionConfig; + private CypherShell shell; + private Main main; + + @Before + public void setup() { // given - // what the user inputs when prompted - String inputString = String.format( "neo4j%nneo%n" ); - InputStream inputStream = new ByteArrayInputStream(inputString.getBytes()); + InputStream inputStream = new ByteArrayInputStream( inputString.getBytes() ); - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - PrintStream ps = new PrintStream(baos); + baos = new ByteArrayOutputStream(); + PrintStream ps = new PrintStream( baos ); - Main main = new Main(inputStream, ps); + main = new Main( inputStream, ps ); CliArgs cliArgs = new CliArgs(); cliArgs.setUsername("", ""); @@ -34,7 +39,7 @@ public void connectInteractivelyPromptsOnWrongAuthentication() throws Exception Logger logger = new AnsiLogger(cliArgs.getDebugMode()); PrettyConfig prettyConfig = new PrettyConfig(cliArgs); - ConnectionConfig connectionConfig = new ConnectionConfig( + connectionConfig = new ConnectionConfig( cliArgs.getScheme(), cliArgs.getHost(), cliArgs.getPort(), @@ -42,13 +47,17 @@ public void connectInteractivelyPromptsOnWrongAuthentication() throws Exception cliArgs.getPassword(), cliArgs.getEncryption()); - CypherShell shell = new CypherShell(logger, prettyConfig); + shell = new CypherShell(logger, prettyConfig); + } + + @Test + public void promptsOnWrongAuthenticationIfInteractive() throws Exception { // when assertEquals("", connectionConfig.username()); assertEquals("", connectionConfig.password()); - main.connectMaybeInteractively(shell, connectionConfig, true); + main.connectMaybeInteractively(shell, connectionConfig, true, true); // then // should be connected @@ -60,4 +69,23 @@ public void connectInteractivelyPromptsOnWrongAuthentication() throws Exception String out = baos.toString(); assertEquals( String.format( "username: neo4j%npassword: ***%n" ), out ); } + + @Test + public void promptsSilentlyOnWrongAuthenticationIfOutputRedirected() throws Exception { + // when + assertEquals("", connectionConfig.username()); + assertEquals("", connectionConfig.password()); + + main.connectMaybeInteractively(shell, connectionConfig, true, false); + + // then + // should be connected + assertTrue(shell.isConnected()); + // should have prompted silently and set the username and password + assertEquals("neo4j", connectionConfig.username()); + assertEquals("neo", connectionConfig.password()); + + String out = baos.toString(); + assertEquals( "", out ); + } } diff --git a/cypher-shell/src/main/java/org/neo4j/shell/Main.java b/cypher-shell/src/main/java/org/neo4j/shell/Main.java index 6d097075..9e391f5e 100644 --- a/cypher-shell/src/main/java/org/neo4j/shell/Main.java +++ b/cypher-shell/src/main/java/org/neo4j/shell/Main.java @@ -5,7 +5,6 @@ import org.neo4j.shell.build.Build; import org.neo4j.shell.cli.CliArgHelper; import org.neo4j.shell.cli.CliArgs; -import org.neo4j.shell.cli.Format; import org.neo4j.shell.commands.CommandHelper; import org.neo4j.shell.exception.CommandException; import org.neo4j.shell.log.AnsiLogger; @@ -15,9 +14,11 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; import java.io.InputStream; +import java.io.OutputStream; import java.io.PrintStream; import static org.neo4j.shell.ShellRunner.isInputInteractive; +import static org.neo4j.shell.ShellRunner.isOutputInteractive; public class Main { static final String NEO_CLIENT_ERROR_SECURITY_UNAUTHORIZED = "Neo.ClientError.Security.Unauthorized"; @@ -72,7 +73,7 @@ void startShell(@Nonnull CliArgs cliArgs) { try { CypherShell shell = new CypherShell(logger, prettyConfig); // Can only prompt for password if input has not been redirected - connectMaybeInteractively(shell, connectionConfig, isInputInteractive()); + connectMaybeInteractively(shell, connectionConfig, isInputInteractive(), isOutputInteractive()); // Construct shellrunner after connecting, due to interrupt handling ShellRunner shellRunner = ShellRunner.getShellRunner(cliArgs, shell, logger, connectionConfig); @@ -92,9 +93,20 @@ void startShell(@Nonnull CliArgs cliArgs) { /** * Connect the shell to the server, and try to handle missing passwords and such */ - void connectMaybeInteractively(@Nonnull CypherShell shell, @Nonnull ConnectionConfig connectionConfig, - boolean interactively) + void connectMaybeInteractively(@Nonnull CypherShell shell, + @Nonnull ConnectionConfig connectionConfig, + boolean inputInteractive, + boolean outputInteractive) throws Exception { + + OutputStream outputStream = outputInteractive ? out : new ThrowawayOutputStream(); + + ConsoleReader consoleReader = new ConsoleReader(in, outputStream); + // Disable expansion of bangs: ! + consoleReader.setExpandEvents(false); + // Ensure Reader does not handle user input for ctrl+C behaviour + consoleReader.setHandleUserInterrupt(false); + try { shell.connect(connectionConfig); } catch (AuthenticationException e) { @@ -103,12 +115,15 @@ void connectMaybeInteractively(@Nonnull CypherShell shell, @Nonnull ConnectionCo throw e; } // else need to prompt for username and password - if (interactively) { + if (inputInteractive) { if (connectionConfig.username().isEmpty()) { - connectionConfig.setUsername(promptForNonEmptyText("username", null)); + String username = outputInteractive ? + promptForNonEmptyText("username", consoleReader, null) : + promptForText("username", consoleReader, null); + connectionConfig.setUsername(username); } if (connectionConfig.password().isEmpty()) { - connectionConfig.setPassword(promptForText("password", '*')); + connectionConfig.setPassword(promptForText("password", consoleReader, '*')); } // try again shell.connect(connectionConfig); @@ -116,6 +131,8 @@ void connectMaybeInteractively(@Nonnull CypherShell shell, @Nonnull ConnectionCo // Can't prompt because input has been redirected throw e; } + } finally { + consoleReader.close(); } } @@ -129,14 +146,14 @@ void connectMaybeInteractively(@Nonnull CypherShell shell, @Nonnull ConnectionCo * in case of errors */ @Nonnull - private String promptForNonEmptyText(@Nonnull String prompt, @Nullable Character mask) throws Exception { - String text = promptForText(prompt, mask); + private String promptForNonEmptyText(@Nonnull String prompt, @Nonnull ConsoleReader consoleReader, @Nullable Character mask) throws Exception { + String text = promptForText(prompt, consoleReader, mask); if (!text.isEmpty()) { return text; } - out.println(prompt + " cannot be empty"); - out.println(); - return promptForNonEmptyText(prompt, mask); + consoleReader.println( prompt + " cannot be empty" ); + consoleReader.println(); + return promptForNonEmptyText(prompt, consoleReader, mask); } /** @@ -144,25 +161,26 @@ private String promptForNonEmptyText(@Nonnull String prompt, @Nullable Character * to display to the user * @param mask * single character to display instead of what the user is typing, use null if text is not secret + * @param consoleReader + * the reader * @return the text which was entered * @throws Exception * in case of errors */ @Nonnull - private String promptForText(@Nonnull String prompt, @Nullable Character mask) throws Exception { - String line; - ConsoleReader consoleReader = new ConsoleReader(in, out); - // Disable expansion of bangs: ! - consoleReader.setExpandEvents(false); - // Ensure Reader does not handle user input for ctrl+C behaviour - consoleReader.setHandleUserInterrupt(false); - line = consoleReader.readLine(prompt + ": ", mask); - consoleReader.close(); - + private String promptForText(@Nonnull String prompt, @Nonnull ConsoleReader consoleReader, @Nullable Character mask) throws Exception { + String line = consoleReader.readLine(prompt + ": ", mask); if (line == null) { throw new CommandException("No text could be read, exiting..."); } return line; } + + private static class ThrowawayOutputStream extends OutputStream { + @Override + public void write( int b ) + { + } + } } diff --git a/cypher-shell/src/test/java/org/neo4j/shell/MainTest.java b/cypher-shell/src/test/java/org/neo4j/shell/MainTest.java index 0cfbcfaf..1010e7d1 100644 --- a/cypher-shell/src/test/java/org/neo4j/shell/MainTest.java +++ b/cypher-shell/src/test/java/org/neo4j/shell/MainTest.java @@ -48,7 +48,7 @@ public void setup() { } @Test - public void connectMaybeInteractivelyNonEndedStringFails() throws Exception { + public void nonEndedStringFails() throws Exception { String inputString = "no newline"; InputStream inputStream = new ByteArrayInputStream(inputString.getBytes()); @@ -57,24 +57,24 @@ public void connectMaybeInteractivelyNonEndedStringFails() throws Exception { thrown.expectMessage("No text could be read, exiting"); Main main = new Main(inputStream, out); - main.connectMaybeInteractively(shell, connectionConfig, true); + main.connectMaybeInteractively(shell, connectionConfig, true, true); verify(shell, times(1)).connect(connectionConfig); } @Test - public void connectMaybeInteractivelyUnrelatedErrorDoesNotPrompt() throws Exception { + public void unrelatedErrorDoesNotPrompt() throws Exception { doThrow(new RuntimeException("bla")).when(shell).connect(connectionConfig); thrown.expect(RuntimeException.class); thrown.expectMessage("bla"); Main main = new Main(mock(InputStream.class), out); - main.connectMaybeInteractively(shell, connectionConfig, true); + main.connectMaybeInteractively(shell, connectionConfig, true, true); verify(shell, times(1)).connect(connectionConfig); } @Test - public void connectMaybeInteractivelyPromptsForBothIfNone() throws Exception { + public void promptsForUsernameAndPasswordIfNoneGivenIfInteractive() throws Exception { doThrow(authException).doNothing().when(shell).connect(connectionConfig); String inputString = "bob\nsecret\n"; @@ -84,18 +84,39 @@ public void connectMaybeInteractivelyPromptsForBothIfNone() throws Exception { PrintStream ps = new PrintStream(baos); Main main = new Main(inputStream, ps); - main.connectMaybeInteractively(shell, connectionConfig, true); + main.connectMaybeInteractively(shell, connectionConfig, true, true); String out = new String(baos.toByteArray(), StandardCharsets.UTF_8); - assertEquals( out, String.format( "username: bob%npassword: ******%n" )); + assertEquals(String.format( "username: bob%npassword: ******%n" ), out); verify(connectionConfig).setUsername("bob"); verify(connectionConfig).setPassword("secret"); verify(shell, times(2)).connect(connectionConfig); } @Test - public void connectMaybeInteractivelyDoesNotPromptIfNotInteractive() throws Exception { + public void promptsSilentlyForUsernameAndPasswordIfNoneGivenIfOutputRedirected() throws Exception { + doThrow(authException).doNothing().when(shell).connect(connectionConfig); + + String inputString = "bob\nsecret\n"; + InputStream inputStream = new ByteArrayInputStream(inputString.getBytes()); + + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + PrintStream ps = new PrintStream(baos); + + Main main = new Main(inputStream, ps); + main.connectMaybeInteractively(shell, connectionConfig, true, false); + + String out = new String(baos.toByteArray(), StandardCharsets.UTF_8); + + assertEquals("", out); + verify(connectionConfig).setUsername("bob"); + verify(connectionConfig).setPassword("secret"); + verify(shell, times(2)).connect(connectionConfig); + } + + @Test + public void doesNotPromptIfInputRedirected() throws Exception { doThrow(authException).doNothing().when(shell).connect(connectionConfig); String inputString = "bob\nsecret\n"; @@ -107,7 +128,7 @@ public void connectMaybeInteractivelyDoesNotPromptIfNotInteractive() throws Exce Main main = new Main(inputStream, ps); try { - main.connectMaybeInteractively(shell, connectionConfig, false); + main.connectMaybeInteractively(shell, connectionConfig, false, true); fail("Expected auth exception"); } catch (AuthenticationException e) { verify(shell, times(1)).connect(connectionConfig); @@ -115,7 +136,7 @@ public void connectMaybeInteractivelyDoesNotPromptIfNotInteractive() throws Exce } @Test - public void connectMaybeInteractivelyPromptsForUserIfPassExists() throws Exception { + public void promptsForUserIfPassExistsIfInteractive() throws Exception { doThrow(authException).doNothing().when(shell).connect(connectionConfig); doReturn("secret").when(connectionConfig).password(); @@ -126,7 +147,7 @@ public void connectMaybeInteractivelyPromptsForUserIfPassExists() throws Excepti PrintStream ps = new PrintStream(baos); Main main = new Main(inputStream, ps); - main.connectMaybeInteractively(shell, connectionConfig, true); + main.connectMaybeInteractively(shell, connectionConfig, true, true); String out = new String(baos.toByteArray(), StandardCharsets.UTF_8); @@ -136,7 +157,28 @@ public void connectMaybeInteractivelyPromptsForUserIfPassExists() throws Excepti } @Test - public void connectMaybeInteractivelyPromptsForPassIfUserExists() throws Exception { + public void promptsSilentlyForUserIfPassExistsIfOutputRedirected() throws Exception { + doThrow(authException).doNothing().when(shell).connect(connectionConfig); + doReturn("secret").when(connectionConfig).password(); + + String inputString = "bob\n"; + InputStream inputStream = new ByteArrayInputStream(inputString.getBytes()); + + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + PrintStream ps = new PrintStream(baos); + + Main main = new Main(inputStream, ps); + main.connectMaybeInteractively(shell, connectionConfig, true, false); + + String out = new String(baos.toByteArray(), StandardCharsets.UTF_8); + + assertEquals(out, ""); + verify(connectionConfig).setUsername("bob"); + verify(shell, times(2)).connect(connectionConfig); + } + + @Test + public void promptsForPassIfUserExistsIfInteractive() throws Exception { doThrow(authException).doNothing().when(shell).connect(connectionConfig); doReturn("bob").when(connectionConfig).username(); @@ -147,7 +189,7 @@ public void connectMaybeInteractivelyPromptsForPassIfUserExists() throws Excepti PrintStream ps = new PrintStream(baos); Main main = new Main(inputStream, ps); - main.connectMaybeInteractively(shell, connectionConfig, true); + main.connectMaybeInteractively(shell, connectionConfig, true, true); String out = new String(baos.toByteArray(), StandardCharsets.UTF_8); @@ -157,7 +199,28 @@ public void connectMaybeInteractivelyPromptsForPassIfUserExists() throws Excepti } @Test - public void connectMaybeInteractivelyPromptsHandlesBang() throws Exception { + public void promptsSielntlyForPassIfUserExistsIfOutputRedirected() throws Exception { + doThrow(authException).doNothing().when(shell).connect(connectionConfig); + doReturn("bob").when(connectionConfig).username(); + + String inputString = "secret\n"; + InputStream inputStream = new ByteArrayInputStream(inputString.getBytes()); + + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + PrintStream ps = new PrintStream(baos); + + Main main = new Main(inputStream, ps); + main.connectMaybeInteractively(shell, connectionConfig, true, false); + + String out = new String(baos.toByteArray(), StandardCharsets.UTF_8); + + assertEquals(out, ""); + verify(connectionConfig).setPassword("secret"); + verify(shell, times(2)).connect(connectionConfig); + } + + @Test + public void promptsHandlesBang() throws Exception { doThrow(authException).doNothing().when(shell).connect(connectionConfig); String inputString = "bo!b\nsec!ret\n"; @@ -167,18 +230,18 @@ public void connectMaybeInteractivelyPromptsHandlesBang() throws Exception { PrintStream ps = new PrintStream(baos); Main main = new Main(inputStream, ps); - main.connectMaybeInteractively(shell, connectionConfig, true); + main.connectMaybeInteractively(shell, connectionConfig, true, true); String out = new String(baos.toByteArray(), StandardCharsets.UTF_8); - assertEquals(out, String.format("username: bo!b%npassword: *******%n")); + assertEquals(String.format("username: bo!b%npassword: *******%n"), out); verify(connectionConfig).setUsername("bo!b"); verify(connectionConfig).setPassword("sec!ret"); verify(shell, times(2)).connect(connectionConfig); } @Test - public void connectMaybeInteractivelyTriesOnlyOnceIfUserPassExists() throws Exception { + public void triesOnlyOnceIfUserPassExists() throws Exception { doThrow(authException).doThrow(new RuntimeException("second try")).when(shell).connect(connectionConfig); doReturn("bob").when(connectionConfig).username(); doReturn("secret").when(connectionConfig).password(); @@ -191,7 +254,7 @@ public void connectMaybeInteractivelyTriesOnlyOnceIfUserPassExists() throws Exce Main main = new Main(inputStream, ps); try { - main.connectMaybeInteractively(shell, connectionConfig, true); + main.connectMaybeInteractively(shell, connectionConfig, true, true); fail("Expected an exception"); } catch (Neo4jException e) { assertEquals(authException.code(), e.code()); @@ -200,7 +263,7 @@ public void connectMaybeInteractivelyTriesOnlyOnceIfUserPassExists() throws Exce } @Test - public void connectMaybeInteractivelyRepromptsIfUserIsNotProvided() throws Exception { + public void repromptsIfUserIsNotProvidedIfInteractive() throws Exception { doThrow(authException).doNothing().when(shell).connect(connectionConfig); String inputString = "\nbob\nsecret\n"; @@ -210,16 +273,37 @@ public void connectMaybeInteractivelyRepromptsIfUserIsNotProvided() throws Excep PrintStream ps = new PrintStream(baos); Main main = new Main(inputStream, ps); - main.connectMaybeInteractively(shell, connectionConfig, true); + main.connectMaybeInteractively(shell, connectionConfig, true, true); String out = new String(baos.toByteArray(), StandardCharsets.UTF_8); - assertEquals(out, String.format( "username: %nusername cannot be empty%n%nusername: bob%npassword: ******%n") ); + assertEquals(String.format( "username: %nusername cannot be empty%n%nusername: bob%npassword: ******%n"), out ); verify(connectionConfig).setUsername("bob"); verify(connectionConfig).setPassword("secret"); verify(shell, times(2)).connect(connectionConfig); } + @Test + public void doesNotRepromptsIfUserIsNotProvidedIfOutputRedirected() throws Exception { + doThrow(authException).doNothing().when(shell).connect(connectionConfig); + + String inputString = "\nsecret\n"; + InputStream inputStream = new ByteArrayInputStream(inputString.getBytes()); + + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + PrintStream ps = new PrintStream(baos); + + Main main = new Main(inputStream, ps); + main.connectMaybeInteractively(shell, connectionConfig, true, false); + + String out = new String(baos.toByteArray(), StandardCharsets.UTF_8); + + assertEquals("", out ); + verify(connectionConfig).setUsername(""); + verify(connectionConfig).setPassword("secret"); + verify(shell, times(2)).connect(connectionConfig); + } + @Test public void printsVersionAndExits() { CliArgs args = new CliArgs();