From 303c03061c88e376b57ebeceffc5d7c29bb8bbfd Mon Sep 17 00:00:00 2001 From: Matt Dailey Date: Thu, 6 Jul 2017 16:14:05 -0400 Subject: [PATCH 1/4] Add ConsolePasswordFinder to read from Console * There was no example `PasswordFinder` to prompt a user for their password --- .../password/ConsolePasswordFinder.java | 52 +++++++++++++++++++ .../password/TestConsolePasswordFinder.java | 36 +++++++++++++ 2 files changed, 88 insertions(+) create mode 100644 src/main/java/net/schmizz/sshj/userauth/password/ConsolePasswordFinder.java create mode 100644 src/test/java/net/schmizz/sshj/userauth/password/TestConsolePasswordFinder.java diff --git a/src/main/java/net/schmizz/sshj/userauth/password/ConsolePasswordFinder.java b/src/main/java/net/schmizz/sshj/userauth/password/ConsolePasswordFinder.java new file mode 100644 index 00000000..f7456907 --- /dev/null +++ b/src/main/java/net/schmizz/sshj/userauth/password/ConsolePasswordFinder.java @@ -0,0 +1,52 @@ +/* + * Copyright (C)2009 - SSHJ Contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package net.schmizz.sshj.userauth.password; + +import java.io.Console; + +/** A PasswordFinder that reads a password from a console */ +public class ConsolePasswordFinder implements PasswordFinder { + + private final Console console; + + /** + * Initializes with the System Console, which will be null if not run from an interactive shell. + */ + public ConsolePasswordFinder() { + this(System.console()); + } + + /** + * @param console the console to read the password from. May be null. + */ + public ConsolePasswordFinder(Console console) { + this.console = console; + } + + @Override + public char[] reqPassword(Resource resource) { + if (console == null) { + // the request cannot be serviced + return null; + } + return console.readPassword("Enter passphrase for %s:", resource.toString()); + } + + @Override + public boolean shouldRetry(Resource resource) { + return true; + } +} diff --git a/src/test/java/net/schmizz/sshj/userauth/password/TestConsolePasswordFinder.java b/src/test/java/net/schmizz/sshj/userauth/password/TestConsolePasswordFinder.java new file mode 100644 index 00000000..3af5e204 --- /dev/null +++ b/src/test/java/net/schmizz/sshj/userauth/password/TestConsolePasswordFinder.java @@ -0,0 +1,36 @@ +/* + * Copyright (C)2009 - SSHJ Contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package net.schmizz.sshj.userauth.password; + +import org.junit.Assert; +import org.junit.Test; +import org.mockito.Mockito; + +public class TestConsolePasswordFinder { + + /* + * Note that Mockito 1.9 cannot mock Console because it is a final class, + * so there are no other tests. + */ + + @Test + public void testReqPasswordNullConsole() { + char[] password = new ConsolePasswordFinder(null) + .reqPassword(Mockito.mock(Resource.class)); + Assert.assertNull("Password should be null with null console", password); + } + +} From aed3decf1d3d633e706951c2ffce8525bdf24609 Mon Sep 17 00:00:00 2001 From: Matt Dailey Date: Fri, 7 Jul 2017 10:36:21 -0400 Subject: [PATCH 2/4] Upgraded Mockito, and added message and retries to ConsolePasswordFinder * Upgraded Mockito to 2.8.47 (latest) * Added extension to allow mocking final classes * ConsolePasswordFinder allows custom message and number of retries * Added builder for ConsolePasswordFinder * Added more unit tests --- build.gradle | 2 +- .../password/ConsolePasswordFinder.java | 81 ++++++++++++++++--- .../password/TestConsolePasswordFinder.java | 63 +++++++++++++-- .../org.mockito.plugins.MockMaker | 2 + 4 files changed, 130 insertions(+), 18 deletions(-) create mode 100644 src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker diff --git a/build.gradle b/build.gradle index a54e995a..5a883ebe 100644 --- a/build.gradle +++ b/build.gradle @@ -36,7 +36,7 @@ dependencies { testCompile "junit:junit:4.11" testCompile 'org.spockframework:spock-core:1.0-groovy-2.4' - testCompile "org.mockito:mockito-core:1.9.5" + testCompile "org.mockito:mockito-core:2.8.47" testCompile "org.apache.sshd:sshd-core:1.2.0" testRuntime "ch.qos.logback:logback-classic:1.1.2" testCompile 'org.glassfish.grizzly:grizzly-http-server:2.3.17' diff --git a/src/main/java/net/schmizz/sshj/userauth/password/ConsolePasswordFinder.java b/src/main/java/net/schmizz/sshj/userauth/password/ConsolePasswordFinder.java index f7456907..44b109e9 100644 --- a/src/main/java/net/schmizz/sshj/userauth/password/ConsolePasswordFinder.java +++ b/src/main/java/net/schmizz/sshj/userauth/password/ConsolePasswordFinder.java @@ -16,37 +16,96 @@ package net.schmizz.sshj.userauth.password; import java.io.Console; +import java.util.IllegalFormatException; /** A PasswordFinder that reads a password from a console */ public class ConsolePasswordFinder implements PasswordFinder { private final Console console; + private final String promptFormat; + private final int maxTries; - /** - * Initializes with the System Console, which will be null if not run from an interactive shell. - */ - public ConsolePasswordFinder() { - this(System.console()); + private int numTries; + + public static ConsolePasswordFinderBuilder builder() { + return new ConsolePasswordFinderBuilder(); } - /** - * @param console the console to read the password from. May be null. - */ - public ConsolePasswordFinder(Console console) { + public ConsolePasswordFinder(Console console, String promptFormat, int maxTries) { this.console = console; + this.promptFormat = promptFormat; + this.maxTries = maxTries; + this.numTries = 0; } @Override public char[] reqPassword(Resource resource) { + numTries++; if (console == null) { // the request cannot be serviced return null; } - return console.readPassword("Enter passphrase for %s:", resource.toString()); + return console.readPassword(promptFormat, resource.toString()); } @Override public boolean shouldRetry(Resource resource) { - return true; + return numTries < maxTries; + } + + public static class ConsolePasswordFinderBuilder { + private Console console; + private String promptFormat; + private int maxTries; + + /** Builder constructor should only be called from parent class */ + private ConsolePasswordFinderBuilder() { + console = System.console(); + promptFormat = "Enter passphrase for %s:"; + maxTries = 3; + } + + public ConsolePasswordFinder build() { + return new ConsolePasswordFinder(console, promptFormat, maxTries); + } + + public ConsolePasswordFinderBuilder setConsole(Console console) { + this.console = console; + return this; + } + + public Console getConsole() { + return console; + } + + /** + * @param promptFormat a StringFormatter string that may contain up to one "%s" + */ + public ConsolePasswordFinderBuilder setPromptFormat(String promptFormat) { + checkFormatString(promptFormat); + this.promptFormat = promptFormat; + return this; + } + + public String getPromptFormat() { + return promptFormat; + } + + public ConsolePasswordFinderBuilder setMaxTries(int maxTries) { + this.maxTries = maxTries; + return this; + } + + public int getMaxTries() { + return maxTries; + } + + private static void checkFormatString(String promptFormat) { + try { + String.format(promptFormat, ""); + } catch (IllegalFormatException e) { + throw new IllegalArgumentException("promptFormat must have no more than one %s and no other markers", e); + } + } } } diff --git a/src/test/java/net/schmizz/sshj/userauth/password/TestConsolePasswordFinder.java b/src/test/java/net/schmizz/sshj/userauth/password/TestConsolePasswordFinder.java index 3af5e204..8d27ca20 100644 --- a/src/test/java/net/schmizz/sshj/userauth/password/TestConsolePasswordFinder.java +++ b/src/test/java/net/schmizz/sshj/userauth/password/TestConsolePasswordFinder.java @@ -19,18 +19,69 @@ import org.junit.Assert; import org.junit.Test; import org.mockito.Mockito; +import java.io.Console; + public class TestConsolePasswordFinder { - /* - * Note that Mockito 1.9 cannot mock Console because it is a final class, - * so there are no other tests. - */ + @Test + public void testReqPassword() { + char[] expectedPassword = "password".toCharArray(); + + Console console = Mockito.mock(Console.class); + Mockito.when(console.readPassword(Mockito.anyString(), Mockito.any())) + .thenReturn(expectedPassword); + + Resource resource = Mockito.mock(Resource.class); + char[] password = ConsolePasswordFinder.builder() + .setConsole(console) + .build() + .reqPassword(resource); + + Assert.assertArrayEquals("Password should match mocked return value", + expectedPassword, password); + Mockito.verifyNoMoreInteractions(resource); + } @Test public void testReqPasswordNullConsole() { - char[] password = new ConsolePasswordFinder(null) - .reqPassword(Mockito.mock(Resource.class)); + Resource resource = Mockito.mock(Resource.class); + char[] password = ConsolePasswordFinder.builder() + .setConsole(null) + .build() + .reqPassword(resource); + Assert.assertNull("Password should be null with null console", password); + Mockito.verifyNoMoreInteractions(resource); + } + + @Test + public void testShouldRetry() { + Resource resource = new PrivateKeyStringResource(""); + ConsolePasswordFinder finder = ConsolePasswordFinder.builder() + .setConsole(null) + .setMaxTries(1) + .build(); + Assert.assertTrue("Should allow a retry at first", finder.shouldRetry(resource)); + + finder.reqPassword(resource); + Assert.assertFalse("Should stop allowing retries after one interaction", finder.shouldRetry(resource)); + } + + @Test + public void testPromptFormat() { + // expecting no Exceptions + ConsolePasswordFinder.builder().setPromptFormat(""); + ConsolePasswordFinder.builder().setPromptFormat("%s"); + } + + @Test(expected = IllegalArgumentException.class) + public void testPromptFormatTooManyMarkers() { + ConsolePasswordFinder.builder().setPromptFormat("%s%s"); + } + + @Test(expected = IllegalArgumentException.class) + public void testPromptFormatWrongMarkerType() { + ConsolePasswordFinder.builder().setPromptFormat("%d"); } } diff --git a/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker b/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker new file mode 100644 index 00000000..ef9230d2 --- /dev/null +++ b/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker @@ -0,0 +1,2 @@ +# incubating feature to allow mocking final classes +mock-maker-inline From 3729119e23ee8d6346d083e42d444fd9a7a55975 Mon Sep 17 00:00:00 2001 From: Matt Dailey Date: Fri, 7 Jul 2017 11:44:31 -0400 Subject: [PATCH 3/4] Added assertions to testPromptFormat --- .../userauth/password/TestConsolePasswordFinder.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/test/java/net/schmizz/sshj/userauth/password/TestConsolePasswordFinder.java b/src/test/java/net/schmizz/sshj/userauth/password/TestConsolePasswordFinder.java index 8d27ca20..f95bae38 100644 --- a/src/test/java/net/schmizz/sshj/userauth/password/TestConsolePasswordFinder.java +++ b/src/test/java/net/schmizz/sshj/userauth/password/TestConsolePasswordFinder.java @@ -69,9 +69,12 @@ public class TestConsolePasswordFinder { @Test public void testPromptFormat() { - // expecting no Exceptions - ConsolePasswordFinder.builder().setPromptFormat(""); - ConsolePasswordFinder.builder().setPromptFormat("%s"); + Assert.assertNotNull( + "Empty format should create valid ConsolePasswordFinder", + ConsolePasswordFinder.builder().setPromptFormat("").build()); + Assert.assertNotNull( + "Single-string format should create valid ConsolePasswordFinder", + ConsolePasswordFinder.builder().setPromptFormat("%s").build()); } @Test(expected = IllegalArgumentException.class) From e5084ed8db11801d9c11ea0508f237ed392bec84 Mon Sep 17 00:00:00 2001 From: Matt Dailey Date: Fri, 7 Jul 2017 20:27:30 -0400 Subject: [PATCH 4/4] Removed Builder, and fixed call to checkFormatString --- .../password/ConsolePasswordFinder.java | 70 ++++--------------- .../password/TestConsolePasswordFinder.java | 25 +++---- 2 files changed, 24 insertions(+), 71 deletions(-) diff --git a/src/main/java/net/schmizz/sshj/userauth/password/ConsolePasswordFinder.java b/src/main/java/net/schmizz/sshj/userauth/password/ConsolePasswordFinder.java index 44b109e9..60e3bd85 100644 --- a/src/main/java/net/schmizz/sshj/userauth/password/ConsolePasswordFinder.java +++ b/src/main/java/net/schmizz/sshj/userauth/password/ConsolePasswordFinder.java @@ -21,17 +21,24 @@ import java.util.IllegalFormatException; /** A PasswordFinder that reads a password from a console */ public class ConsolePasswordFinder implements PasswordFinder { + public static final String DEFAULT_FORMAT = "Enter passphrase for %s:"; + private final Console console; private final String promptFormat; private final int maxTries; private int numTries; - public static ConsolePasswordFinderBuilder builder() { - return new ConsolePasswordFinderBuilder(); + public ConsolePasswordFinder() { + this(System.console()); + } + + public ConsolePasswordFinder(Console console) { + this(console, DEFAULT_FORMAT, 3); } public ConsolePasswordFinder(Console console, String promptFormat, int maxTries) { + checkFormatString(promptFormat); this.console = console; this.promptFormat = promptFormat; this.maxTries = maxTries; @@ -53,59 +60,12 @@ public class ConsolePasswordFinder implements PasswordFinder { return numTries < maxTries; } - public static class ConsolePasswordFinderBuilder { - private Console console; - private String promptFormat; - private int maxTries; - - /** Builder constructor should only be called from parent class */ - private ConsolePasswordFinderBuilder() { - console = System.console(); - promptFormat = "Enter passphrase for %s:"; - maxTries = 3; - } - - public ConsolePasswordFinder build() { - return new ConsolePasswordFinder(console, promptFormat, maxTries); - } - - public ConsolePasswordFinderBuilder setConsole(Console console) { - this.console = console; - return this; - } - - public Console getConsole() { - return console; - } - - /** - * @param promptFormat a StringFormatter string that may contain up to one "%s" - */ - public ConsolePasswordFinderBuilder setPromptFormat(String promptFormat) { - checkFormatString(promptFormat); - this.promptFormat = promptFormat; - return this; - } - - public String getPromptFormat() { - return promptFormat; - } - - public ConsolePasswordFinderBuilder setMaxTries(int maxTries) { - this.maxTries = maxTries; - return this; - } - - public int getMaxTries() { - return maxTries; - } - - private static void checkFormatString(String promptFormat) { - try { - String.format(promptFormat, ""); - } catch (IllegalFormatException e) { - throw new IllegalArgumentException("promptFormat must have no more than one %s and no other markers", e); - } + private static void checkFormatString(String promptFormat) { + try { + String.format(promptFormat, ""); + } catch (IllegalFormatException e) { + throw new IllegalArgumentException("promptFormat must have no more than one %s and no other markers", e); } } + } diff --git a/src/test/java/net/schmizz/sshj/userauth/password/TestConsolePasswordFinder.java b/src/test/java/net/schmizz/sshj/userauth/password/TestConsolePasswordFinder.java index f95bae38..f0bc3d17 100644 --- a/src/test/java/net/schmizz/sshj/userauth/password/TestConsolePasswordFinder.java +++ b/src/test/java/net/schmizz/sshj/userauth/password/TestConsolePasswordFinder.java @@ -23,6 +23,8 @@ import java.io.Console; public class TestConsolePasswordFinder { + private static final String FORMAT = "%s"; + @Test public void testReqPassword() { char[] expectedPassword = "password".toCharArray(); @@ -32,10 +34,7 @@ public class TestConsolePasswordFinder { .thenReturn(expectedPassword); Resource resource = Mockito.mock(Resource.class); - char[] password = ConsolePasswordFinder.builder() - .setConsole(console) - .build() - .reqPassword(resource); + char[] password = new ConsolePasswordFinder(console).reqPassword(resource); Assert.assertArrayEquals("Password should match mocked return value", expectedPassword, password); @@ -45,10 +44,7 @@ public class TestConsolePasswordFinder { @Test public void testReqPasswordNullConsole() { Resource resource = Mockito.mock(Resource.class); - char[] password = ConsolePasswordFinder.builder() - .setConsole(null) - .build() - .reqPassword(resource); + char[] password = new ConsolePasswordFinder(null, FORMAT, 1).reqPassword(resource); Assert.assertNull("Password should be null with null console", password); Mockito.verifyNoMoreInteractions(resource); @@ -57,10 +53,7 @@ public class TestConsolePasswordFinder { @Test public void testShouldRetry() { Resource resource = new PrivateKeyStringResource(""); - ConsolePasswordFinder finder = ConsolePasswordFinder.builder() - .setConsole(null) - .setMaxTries(1) - .build(); + ConsolePasswordFinder finder = new ConsolePasswordFinder(null, FORMAT, 1); Assert.assertTrue("Should allow a retry at first", finder.shouldRetry(resource)); finder.reqPassword(resource); @@ -71,20 +64,20 @@ public class TestConsolePasswordFinder { public void testPromptFormat() { Assert.assertNotNull( "Empty format should create valid ConsolePasswordFinder", - ConsolePasswordFinder.builder().setPromptFormat("").build()); + new ConsolePasswordFinder(null, "", 1)); Assert.assertNotNull( "Single-string format should create valid ConsolePasswordFinder", - ConsolePasswordFinder.builder().setPromptFormat("%s").build()); + new ConsolePasswordFinder(null, FORMAT, 1)); } @Test(expected = IllegalArgumentException.class) public void testPromptFormatTooManyMarkers() { - ConsolePasswordFinder.builder().setPromptFormat("%s%s"); + new ConsolePasswordFinder(null, "%s%s", 1); } @Test(expected = IllegalArgumentException.class) public void testPromptFormatWrongMarkerType() { - ConsolePasswordFinder.builder().setPromptFormat("%d"); + new ConsolePasswordFinder(null, "%d", 1); } }