# HG changeset patch # User Jon VanAlten # Date 1363983486 14400 # Node ID 12894dbd3517892cb253fefcef1e2549f4b82284 # Parent c551a80f08b24da76b89a3aae812a0c3787e32e3 Avoid authentication parameters as command line arguments Reviewed-by: jerboaa Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2013-April/006376.html diff -r c551a80f08b2 -r 12894dbd3517 agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/AgentApplication.java --- a/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/AgentApplication.java Tue Apr 23 14:02:46 2013 +0200 +++ b/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/AgentApplication.java Fri Mar 22 16:18:06 2013 -0400 @@ -141,8 +141,9 @@ case FAILED_TO_CONNECT: logger.warning("Could not connect to storage."); shutdown(); + break; default: - logger.warning("Unfamiliar ConnectionStatus value"); + logger.warning("Unfamiliar ConnectionStatus value: " + newStatus.toString()); } } }; diff -r c551a80f08b2 -r 12894dbd3517 agent/core/src/main/java/com/redhat/thermostat/agent/config/AgentConfigsUtils.java --- a/agent/core/src/main/java/com/redhat/thermostat/agent/config/AgentConfigsUtils.java Tue Apr 23 14:02:46 2013 +0200 +++ b/agent/core/src/main/java/com/redhat/thermostat/agent/config/AgentConfigsUtils.java Fri Mar 22 16:18:06 2013 -0400 @@ -38,7 +38,9 @@ import java.io.File; import java.io.FileInputStream; +import java.io.FileReader; import java.io.IOException; +import java.util.Arrays; import java.util.Properties; import com.redhat.thermostat.common.config.Configuration; @@ -46,13 +48,20 @@ public class AgentConfigsUtils { + private static char[] pw = {'p', 'a', 's', 's', 'w', 'o', 'r', 'd'}; + private static char[] user = {'u', 's', 'e', 'r', 'n', 'a', 'm', 'e'}; + private static String newLine = System.lineSeparator(); + private static char comment = '#'; + public static AgentStartupConfiguration createAgentConfigs() throws InvalidConfigurationException { AgentStartupConfiguration config = new AgentStartupConfiguration(); - - File propertyFile = new Configuration().getAgentConfigurationFile(); + + Configuration mainConfig = new Configuration(); + File propertyFile = mainConfig.getAgentConfigurationFile(); readAndSetProperties(propertyFile, config); - + File agentAuthFile = mainConfig.getAgentAuthConfigFile(); + setAuthConfigFromFile(agentAuthFile, config); return config; } @@ -86,6 +95,111 @@ configuration.setConfigListenAddress(address); } } - + + static void setAuthConfigFromFile(File authFile, + AgentStartupConfiguration config) { + // Default values will be enough if storage configured with not auth necessary. + config.setUsername(""); + config.setPassword(""); + if (authFile.canRead() && authFile.isFile()) { + long length = authFile.length(); + if (length > Integer.MAX_VALUE || length < 0L) { + throw new InvalidConfigurationException("agent.auth file not valid"); + } + int len = (int) length; // A reasonable file will be within int range. + // file size in bytes >= # of chars so this size should be sufficient. + char[] authData = new char[len]; + // This is probably the most sensitive time for password-in-heap exposure. + // The reader here may contain buffers containing the password. It will, + // of course, be garbage collected in due time. + try (FileReader reader = new FileReader(authFile)) { + int chars = reader.read(authData, 0, len); + parseAuthConfigFromData(authData, chars, config); + } catch (IOException e) { + throw new InvalidConfigurationException(e); + } finally { + Arrays.fill(authData, '\0'); + } + } + } + + static void parseAuthConfigFromData(char[] data, int dataLen, AgentStartupConfiguration config) { + int position = 0; + while (position < dataLen) { + if ((position + 1 == dataLen) || data[position + 1] == newLine.charAt(0)) { + // Empty line + position = nextLine(data, position); + continue; + } + if (data[position] == comment) { + // Comment + position = nextLine(data, position); + continue; + } + char[] value = getPassword(data, position); + if (value != null) { + // Password + config.setPassword(new String(value)); + position = nextLine(data, position); + Arrays.fill(value, '\0'); + value = null; + continue; + } + value = getUserName(data, position); + if (value != null) { + // Username + config.setUsername(new String(value)); + position = nextLine(data, position); + value = null; + continue; + } + throw new InvalidConfigurationException("Unrecognized content in agent auth file."); + } + } + + private static int nextLine(char[] data, int current) { + int next = current + 1; + while (next < data.length) { + if (data[next] == newLine.charAt(0)) { + break; + } + next += newLine.length(); + } + next++; + return next; + } + + private static char[] getPassword(char[] data, int start) { + return getValue(data, start, pw); + } + + private static char[] getUserName(char[] data, int start) { + return getValue(data, start, user); + } + + private static char[] getValue(char[] data, int start, char[] key) { + if (data[start + key.length] != '=') { + return null; + } + for (int i = 0; i < key.length; i++) { + if (key[i] != data[start + i]) { + return null; + } + } + int end = positionOf(newLine.charAt(0), data, start, data.length); + char[] value = Arrays.copyOfRange(data, start + key.length + 1, end); + return value; + } + + private static int positionOf(char character, char[] data, int start, int end) { + int position = -1; + for (int possible = start; possible < data.length && possible <= end; possible++) { + if (data[possible] == character) { + position = possible; + break; + } + } + return position; + } } diff -r c551a80f08b2 -r 12894dbd3517 agent/core/src/main/java/com/redhat/thermostat/agent/config/AgentOptionParser.java --- a/agent/core/src/main/java/com/redhat/thermostat/agent/config/AgentOptionParser.java Tue Apr 23 14:02:46 2013 +0200 +++ b/agent/core/src/main/java/com/redhat/thermostat/agent/config/AgentOptionParser.java Fri Mar 22 16:18:06 2013 -0400 @@ -71,8 +71,6 @@ isHelp = true; } } - configuration.setUsername(args.getArgument("username")); - configuration.setPassword(args.getArgument("password")); } public boolean isHelp() { diff -r c551a80f08b2 -r 12894dbd3517 agent/core/src/test/java/com/redhat/thermostat/agent/config/AgentConfigsUtilsTest.java --- a/agent/core/src/test/java/com/redhat/thermostat/agent/config/AgentConfigsUtilsTest.java Tue Apr 23 14:02:46 2013 +0200 +++ b/agent/core/src/test/java/com/redhat/thermostat/agent/config/AgentConfigsUtilsTest.java Fri Mar 22 16:18:06 2013 -0400 @@ -36,28 +36,103 @@ package com.redhat.thermostat.agent.config; +import java.io.File; +import java.io.FileWriter; import java.io.IOException; +import java.util.Random; import org.junit.Assert; -import org.junit.Before; +import org.junit.BeforeClass; import org.junit.Test; import com.redhat.thermostat.common.config.InvalidConfigurationException; import com.redhat.thermostat.testutils.TestUtils; public class AgentConfigsUtilsTest { - - @Before - public void setUp() throws IOException, InvalidConfigurationException { - TestUtils.setupAgentConfigs(); + + private static Random random; + + @BeforeClass + public static void setUpRandom() { + random = new Random(); } @Test - public void test() throws InvalidConfigurationException { + public void testCreateAgentConfigs() throws InvalidConfigurationException, IOException { + TestUtils.setupAgentConfigs(); AgentStartupConfiguration config = AgentConfigsUtils.createAgentConfigs(); Assert.assertFalse(config.purge()); Assert.assertEquals("42.42.42.42:42", config.getConfigListenAddress()); + Assert.assertEquals("user", config.getUsername()); + Assert.assertEquals("pass", config.getPassword()); + } + + @Test + public void testAuthConfigFromFile() throws IOException { + File tmpAuth = createTempAuthFile("username=user\npassword=pass\n"); + AgentStartupConfiguration config = new AgentStartupConfiguration(); + AgentConfigsUtils.setAuthConfigFromFile(tmpAuth, config); + Assert.assertEquals("user", config.getUsername()); + Assert.assertEquals("pass", config.getPassword()); + } + + @Test + public void testAuthConfigFromEmptyFile() throws IOException { + File tmpAuth = createTempAuthFile(""); + AgentStartupConfiguration config = new AgentStartupConfiguration(); + AgentConfigsUtils.setAuthConfigFromFile(tmpAuth, config); + Assert.assertEquals("", config.getUsername()); + Assert.assertEquals("", config.getPassword()); + } + + @Test + public void testAuthConfigWithConfigCommentedOut() throws IOException { + File tmpAuth = createTempAuthFile("#username=user\n#password=pass\n"); + AgentStartupConfiguration config = new AgentStartupConfiguration(); + AgentConfigsUtils.setAuthConfigFromFile(tmpAuth, config); + Assert.assertEquals("", config.getUsername()); + Assert.assertEquals("", config.getPassword()); + } + + private File createTempAuthFile(String contents) throws IOException { + String tmpAuthLoc = System.getProperty("java.io.tmpdir") + File.separatorChar + + Math.abs(random.nextInt()); + File tmpAuth = new File(tmpAuthLoc); + tmpAuth.deleteOnExit(); + tmpAuth.createNewFile(); + FileWriter authWriter = new FileWriter(tmpAuth); + authWriter.append(contents); + authWriter.flush(); + authWriter.close(); + return tmpAuth; + } + + @Test + public void testParseAuthConfigData() { + char[] authData = "username=user\npassword=pass\n".toCharArray(); + AgentStartupConfiguration config = new AgentStartupConfiguration(); + AgentConfigsUtils.parseAuthConfigFromData(authData, authData.length, config); + Assert.assertEquals("user", config.getUsername()); + Assert.assertEquals("pass", config.getPassword()); + } + + @Test + public void testParseAuthDataIgnoresComments() { + char[] authData = "#username=user\n#password=pass\n".toCharArray(); + AgentStartupConfiguration config = new AgentStartupConfiguration(); + AgentConfigsUtils.parseAuthConfigFromData(authData, authData.length, config); + Assert.assertNull(config.getUsername()); + Assert.assertNull(config.getPassword()); + } + + @Test + public void testParseAuthDataIgnoresDataAfterLength() { + char[] authData = "#username=user\n#password=pass\nusername=user\npassword=pass\n".toCharArray(); + AgentStartupConfiguration config = new AgentStartupConfiguration(); + AgentConfigsUtils.parseAuthConfigFromData(authData, 30, config); + Assert.assertNull(config.getUsername()); + Assert.assertNull(config.getPassword()); } } diff -r c551a80f08b2 -r 12894dbd3517 client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/ConnectCommand.java --- a/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/ConnectCommand.java Tue Apr 23 14:02:46 2013 +0200 +++ b/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/ConnectCommand.java Fri Mar 22 16:18:06 2013 -0400 @@ -36,6 +36,9 @@ package com.redhat.thermostat.client.cli.internal; +import java.io.IOException; +import java.util.Objects; + import org.osgi.framework.BundleContext; import org.osgi.framework.FrameworkUtil; import org.osgi.framework.ServiceReference; @@ -45,6 +48,7 @@ import com.redhat.thermostat.common.cli.CommandException; import com.redhat.thermostat.common.config.ClientPreferences; import com.redhat.thermostat.common.locale.Translate; +import com.redhat.thermostat.common.tools.StorageAuthInfoGetter; import com.redhat.thermostat.storage.core.ConnectionException; import com.redhat.thermostat.storage.core.DbService; import com.redhat.thermostat.storage.core.DbServiceFactory; @@ -62,8 +66,6 @@ public class ConnectCommand extends AbstractCommand { private static final String DB_URL_ARG = "dbUrl"; - private static final String USERNAME_ARG = "username"; - private static final String PASSWORD_ARG = "password"; private static final Translate translator = LocaleResources.createLocalizer(); @@ -100,11 +102,25 @@ context.ungetService(keyringRef); } String dbUrl = ctx.getArguments().getArgument(DB_URL_ARG); - if (dbUrl == null) { - dbUrl = prefs.getConnectionUrl(); + // This argument is considered "required" so option parsing should mean this is impossible. + Objects.requireNonNull(dbUrl); + String username = null; + String password = null; + if (prefs.getConnectionUrl().equals(dbUrl)) { + // Have we previously saved connection parameters for this Url? + username = prefs.getUserName(); + password = prefs.getPassword(); } - String username = ctx.getArguments().getArgument(USERNAME_ARG); - String password = ctx.getArguments().getArgument(PASSWORD_ARG); + if (username == null || password == null) { + com.redhat.thermostat.common.cli.Console console = ctx.getConsole(); + try { + StorageAuthInfoGetter getUserPass = new StorageAuthInfoGetter(console); + username = getUserPass.getUserName(dbUrl); + password = new String(getUserPass.getPassword(dbUrl)); + } catch (IOException e) { + throw new CommandException("Could not get username or password from user.", e); + } + } try { // may throw StorageException if storage url is not supported DbService service = dbServiceFactory.createDbService(username, password, dbUrl); diff -r c551a80f08b2 -r 12894dbd3517 client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/ConnectCommandTest.java --- a/client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/ConnectCommandTest.java Tue Apr 23 14:02:46 2013 +0200 +++ b/client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/ConnectCommandTest.java Fri Mar 22 16:18:06 2013 -0400 @@ -125,9 +125,8 @@ when(dbServiceFactory.createDbService(eq(username), eq(password), eq(dbUrl))).thenReturn(dbService); SimpleArguments args = new SimpleArguments(); args.addArgument("dbUrl", dbUrl); - args.addArgument("username", username); - args.addArgument("password", password); CommandContext ctx = cmdCtxFactory.createContext(args); + cmdCtxFactory.setInput(username + '\r' + password + '\r'); cmd.run(ctx); verify(dbService).connect(); } @@ -136,13 +135,9 @@ public void verifyNoKeyring() throws CommandException { DbService dbService = mock(DbService.class); - String username = "testuser"; - String password = "testpassword"; String dbUrl = "mongodb://10.23.122.1:12578"; SimpleArguments args = new SimpleArguments(); args.addArgument("dbUrl", dbUrl); - args.addArgument("username", username); - args.addArgument("password", password); CommandContext ctx = cmdCtxFactory.createContext(args); try { diff -r c551a80f08b2 -r 12894dbd3517 client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/Main.java --- a/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/Main.java Tue Apr 23 14:02:46 2013 +0200 +++ b/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/Main.java Fri Mar 22 16:18:06 2013 -0400 @@ -251,7 +251,7 @@ } catch (Throwable t) { // Note: DbService fires a ConnectionListener event when it // fails to connect. No need to notify our handler manually. - logger.log(Level.WARNING, "connection attempt failed: ", t); + logger.log(Level.FINE, "connection attempt failed: ", t); } } }); diff -r c551a80f08b2 -r 12894dbd3517 common/core/pom.xml --- a/common/core/pom.xml Tue Apr 23 14:02:46 2013 +0200 +++ b/common/core/pom.xml Fri Mar 22 16:18:06 2013 -0400 @@ -135,6 +135,11 @@ + jline + jline + + + commons-cli commons-cli diff -r c551a80f08b2 -r 12894dbd3517 common/core/src/main/java/com/redhat/thermostat/common/config/ClientPreferences.java --- a/common/core/src/main/java/com/redhat/thermostat/common/config/ClientPreferences.java Tue Apr 23 14:02:46 2013 +0200 +++ b/common/core/src/main/java/com/redhat/thermostat/common/config/ClientPreferences.java Fri Mar 22 16:18:06 2013 -0400 @@ -60,6 +60,7 @@ this(Preferences.userRoot().node("thermostat"), keyring); } + // Testing hook with injectable j.u.p.Preferences ClientPreferences(Preferences prefs, Keyring keyring) { this.prefs = prefs; this.keyring = keyring; diff -r c551a80f08b2 -r 12894dbd3517 common/core/src/main/java/com/redhat/thermostat/common/config/Configuration.java --- a/common/core/src/main/java/com/redhat/thermostat/common/config/Configuration.java Tue Apr 23 14:02:46 2013 +0200 +++ b/common/core/src/main/java/com/redhat/thermostat/common/config/Configuration.java Fri Mar 22 16:18:06 2013 -0400 @@ -115,11 +115,15 @@ } public File getAgentConfigurationFile() throws InvalidConfigurationException { - File agent = new File(getThermostatHome(), "agent"); return new File(agent, "agent.properties"); } + public File getAgentAuthConfigFile() throws InvalidConfigurationException { + File agent = new File(getThermostatHome(), "agent"); + return new File(agent, "agent.auth"); + } + public File getClientConfigurationDirectory() throws InvalidConfigurationException { File client = new File(getThermostatHome(), "client"); return client; diff -r c551a80f08b2 -r 12894dbd3517 common/core/src/main/java/com/redhat/thermostat/common/locale/LocaleResources.java --- a/common/core/src/main/java/com/redhat/thermostat/common/locale/LocaleResources.java Tue Apr 23 14:02:46 2013 +0200 +++ b/common/core/src/main/java/com/redhat/thermostat/common/locale/LocaleResources.java Fri Mar 22 16:18:06 2013 -0400 @@ -45,6 +45,9 @@ APPLICATION_INFO_LICENSE, APPLICATION_INFO_DESCRIPTION, APPLICATION_VERSION_INFO, + + USERNAME_PROMPT, + PASSWORD_PROMPT, ; public static final String RESOURCE_BUNDLE = diff -r c551a80f08b2 -r 12894dbd3517 common/core/src/main/java/com/redhat/thermostat/common/tools/StorageAuthInfoGetter.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/common/core/src/main/java/com/redhat/thermostat/common/tools/StorageAuthInfoGetter.java Fri Mar 22 16:18:06 2013 -0400 @@ -0,0 +1,116 @@ +/* + * Copyright 2012, 2013 Red Hat, Inc. + * + * This file is part of Thermostat. + * + * Thermostat is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation; either version 2, or (at your + * option) any later version. + * + * Thermostat is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Thermostat; see the file COPYING. If not see + * . + * + * Linking this code with other modules is making a combined work + * based on this code. Thus, the terms and conditions of the GNU + * General Public License cover the whole combination. + * + * As a special exception, the copyright holders of this code give + * you permission to link this code with independent modules to + * produce an executable, regardless of the license terms of these + * independent modules, and to copy and distribute the resulting + * executable under terms of your choice, provided that you also + * meet, for each linked independent module, the terms and conditions + * of the license of that module. An independent module is a module + * which is not derived from or based on this code. If you modify + * this code, you may extend this exception to your version of the + * library, but you are not obligated to do so. If you do not wish + * to do so, delete this exception statement from your version. + */ + +package com.redhat.thermostat.common.tools; + +import java.io.IOException; +import java.util.Arrays; + +import jline.console.ConsoleReader; + +import com.redhat.thermostat.common.cli.Console; +import com.redhat.thermostat.common.locale.LocaleResources; +import com.redhat.thermostat.common.locale.Translate; + +/** + * Utility class to isolate the user-facing strings necessary when prompting user + * for authentication parameters for storage connection. Provides simple convenience + * wrappers for jline.console.ConsoleReader methods, and localization of prompt. + */ +public class StorageAuthInfoGetter { + + private static final int PW_SIZE_INCREMENT = 16; + + private ConsoleReader reader; + private Translate t; + + public StorageAuthInfoGetter(Console console) throws IOException { + reader = new ConsoleReader(console.getInput(), console.getOutput()); + t = LocaleResources.createLocalizer(); + } + + /** + * Prompt the user for username necessary for connecting to a given url. + * @param url + * @return The username entered by the user. This could be the empty string. + * @throws IOException + */ + public String getUserName(String url) throws IOException { + String prompt = t.localize(LocaleResources.USERNAME_PROMPT, url); + String name = reader.readLine(prompt); + return name; + } + + /** + * Prompt the user for password necessary for connecting to a given url. + * The caller is responsible for clearing the char[] to minimize the time during + * which the password is available in cleartext in the heap. + * @param url + * @return The password entered by the user. This could be the empty string. + * @throws IOException + */ + public char[] getPassword(String url) throws IOException { + char[] password = new char[PW_SIZE_INCREMENT]; + reader.setHistoryEnabled(false); + reader.print(t.localize(LocaleResources.PASSWORD_PROMPT, url)); + reader.flush(); + Character oldEcho = reader.getEchoCharacter(); + reader.setEchoCharacter('\0'); + int pwChar = reader.readCharacter(); + int length = 0; + while ((char) pwChar != '\n' && (char) pwChar != '\r') { + password[length] = (char) pwChar; + length++; + if (length >= password.length) { + password = secureCopyCharArray(password, length + PW_SIZE_INCREMENT); + } + pwChar = reader.readCharacter(); + } + reader.setEchoCharacter(oldEcho); + reader.setHistoryEnabled(true); + reader.println(); + reader.flush(); + password = secureCopyCharArray(password, length); + return password; + } + + // returns new array, fills original with '\0' + private char[] secureCopyCharArray(char[] original, int newLength) { + char[] newArray = Arrays.copyOf(original, newLength); + Arrays.fill(original, '\0'); + return newArray; + } +} diff -r c551a80f08b2 -r 12894dbd3517 common/core/src/main/java/com/redhat/thermostat/test/TestCommandContextFactory.java --- a/common/core/src/main/java/com/redhat/thermostat/test/TestCommandContextFactory.java Tue Apr 23 14:02:46 2013 +0200 +++ b/common/core/src/main/java/com/redhat/thermostat/test/TestCommandContextFactory.java Fri Mar 22 16:18:06 2013 -0400 @@ -118,6 +118,11 @@ return new String(out.toByteArray()); } + /** + * For simulating user input in tests that require user interaction. + * + * @param input the user's input, including any CR characters + */ public void setInput(String input) { try { inOut.write(input.getBytes()); diff -r c551a80f08b2 -r 12894dbd3517 common/core/src/main/resources/com/redhat/thermostat/common/locale/strings.properties --- a/common/core/src/main/resources/com/redhat/thermostat/common/locale/strings.properties Tue Apr 23 14:02:46 2013 +0200 +++ b/common/core/src/main/resources/com/redhat/thermostat/common/locale/strings.properties Fri Mar 22 16:18:06 2013 -0400 @@ -5,4 +5,8 @@ APPLICATION_INFO_DESCRIPTION = A monitoring and serviceability tool for OpenJDK APPLICATION_INFO_LICENSE = Licensed under GPLv2+ with Classpath exception # First parameter is the application name -APPLICATION_VERSION_INFO = {0} version \ No newline at end of file +APPLICATION_VERSION_INFO = {0} version + +# Parameters for prompts should be storage URL. +USERNAME_PROMPT = Please enter username for storage at {0}: +PASSWORD_PROMPT = Please enter password for storage at {0}: \ No newline at end of file diff -r c551a80f08b2 -r 12894dbd3517 common/core/src/test/java/com/redhat/thermostat/common/config/ConfigurationTest.java --- a/common/core/src/test/java/com/redhat/thermostat/common/config/ConfigurationTest.java Tue Apr 23 14:02:46 2013 +0200 +++ b/common/core/src/test/java/com/redhat/thermostat/common/config/ConfigurationTest.java Fri Mar 22 16:18:06 2013 -0400 @@ -75,6 +75,8 @@ Assert.assertEquals(thermostatHome + "agent" + s + "agent.properties", config.getAgentConfigurationFile().getCanonicalPath()); + Assert.assertEquals(thermostatHome + "agent" + s + "agent.auth", + config.getAgentAuthConfigFile().getCanonicalPath()); Assert.assertEquals(thermostatHome + "storage", config.getStorageBaseDirectory().getCanonicalPath()); Assert.assertEquals(thermostatHome + "storage" + s + "db.properties", config.getStorageConfigurationFile().getCanonicalPath()); diff -r c551a80f08b2 -r 12894dbd3517 common/core/src/test/java/com/redhat/thermostat/common/tools/StorageAuthInfoGetterTest.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/common/core/src/test/java/com/redhat/thermostat/common/tools/StorageAuthInfoGetterTest.java Fri Mar 22 16:18:06 2013 -0400 @@ -0,0 +1,106 @@ +package com.redhat.thermostat.common.tools; + +import java.io.IOException; +import java.io.InputStream; +/* + * Copyright 2012, 2013 Red Hat, Inc. + * + * This file is part of Thermostat. + * + * Thermostat is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation; either version 2, or (at your + * option) any later version. + * + * Thermostat is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Thermostat; see the file COPYING. If not see + * . + * + * Linking this code with other modules is making a combined work + * based on this code. Thus, the terms and conditions of the GNU + * General Public License cover the whole combination. + * + * As a special exception, the copyright holders of this code give + * you permission to link this code with independent modules to + * produce an executable, regardless of the license terms of these + * independent modules, and to copy and distribute the resulting + * executable under terms of your choice, provided that you also + * meet, for each linked independent module, the terms and conditions + * of the license of that module. An independent module is a module + * which is not derived from or based on this code. If you modify + * this code, you may extend this exception to your version of the + * library, but you are not obligated to do so. If you do not wish + * to do so, delete this exception statement from your version. + */ + +import java.io.PrintStream; + +import org.junit.Before; +import org.junit.Test; + +import com.redhat.thermostat.common.cli.Console; + +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class StorageAuthInfoGetterTest { + + private StorageAuthInfoGetter getter; + private InputStream in; + private PrintStream out; + + @Before + public void setUp() throws IOException { + in = mock(InputStream.class); + out = mock(PrintStream.class); + Console console = mock(Console.class); + when(console.getInput()).thenReturn(in); + when(console.getOutput()).thenReturn(out); + getter = new StorageAuthInfoGetter(console); + } + + @Test + public void testGetUserNameCarriageReturn() throws IOException { + testGetUsername((int) '\r'); + } + + @Test + public void testGetUserNameNewLine() throws IOException { + testGetUsername((int) '\n'); + } + + private void testGetUsername(int newLineChar) throws IOException { + when(in.read()).thenReturn((int) 'u') + .thenReturn((int) 's') + .thenReturn((int) 'e') + .thenReturn((int) 'r') + .thenReturn(newLineChar); + assertEquals("user", getter.getUserName("url_doesn't_matter")); + } + + @Test + public void testGetPasswordCarriageReturn() throws IOException { + testGetPassword((int) '\r'); + } + + @Test + public void testGetPasswordNewLine() throws IOException { + testGetPassword((int) '\n'); + } + + private void testGetPassword(int newLineChar) throws IOException { + char[] pass = new char[] {'p', 'a', 's', 's'}; + when(in.read()).thenReturn((int) 'p') + .thenReturn((int) 'a') + .thenReturn((int) 's') + .thenReturn((int) 's') + .thenReturn((int) '\r'); + assertEquals(new String(pass), new String(getter.getPassword("url_doesn't_matter"))); + } +} diff -r c551a80f08b2 -r 12894dbd3517 common/test/src/main/java/com/redhat/thermostat/testutils/TestUtils.java --- a/common/test/src/main/java/com/redhat/thermostat/testutils/TestUtils.java Tue Apr 23 14:02:46 2013 +0200 +++ b/common/test/src/main/java/com/redhat/thermostat/testutils/TestUtils.java Fri Mar 22 16:18:06 2013 -0400 @@ -38,6 +38,7 @@ import java.io.File; import java.io.FileOutputStream; +import java.io.FileWriter; import java.io.IOException; import java.io.OutputStream; import java.lang.management.ManagementFactory; @@ -99,6 +100,11 @@ props.store(propsOutputStream, "thermostat agent test properties"); } + File tmpAuth = new File(agent, "agent.auth"); + FileWriter authWriter = new FileWriter(tmpAuth); + authWriter.append("username=user\npassword=pass\n"); + authWriter.flush(); + authWriter.close(); return tmpDir; } } diff -r c551a80f08b2 -r 12894dbd3517 distribution/config/agent.auth --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/distribution/config/agent.auth Fri Mar 22 16:18:06 2013 -0400 @@ -0,0 +1,9 @@ +# This file is intended to be read by a hand-rolled reader/parser, to avoid +# passwords needing to be represented as String objects at runtime. It must +# be saved with Unix line end characters, and encoded as ascii. +# Uncomment the following lines and replace with your storage authentication +# parameters as needed. +# +#username=thermostat +#password=thermostat + diff -r c551a80f08b2 -r 12894dbd3517 distribution/config/commands/agent.properties --- a/distribution/config/commands/agent.properties Tue Apr 23 14:02:46 2013 +0200 +++ b/distribution/config/commands/agent.properties Fri Mar 22 16:18:06 2013 -0400 @@ -21,7 +21,7 @@ description = starts and stops the thermostat agent -usage = agent [-d ] [-u -p ] [-s] [-l ] +usage = agent [-d ] [-s] [-l ] options = AUTO_LOG_OPTION, AUTO_DB_OPTIONS, saveOnExit diff -r c551a80f08b2 -r 12894dbd3517 distribution/config/commands/connect.properties --- a/distribution/config/commands/connect.properties Tue Apr 23 14:02:46 2013 +0200 +++ b/distribution/config/commands/connect.properties Fri Mar 22 16:18:06 2013 -0400 @@ -14,7 +14,7 @@ description = persistently connect to storage -usage = connect -d [-u ] [-p ] [-l ] +usage = connect -d [-l ] options = AUTO_LOG_OPTION, AUTO_DB_OPTIONS, dbUrl diff -r c551a80f08b2 -r 12894dbd3517 distribution/config/commands/list-vms.properties --- a/distribution/config/commands/list-vms.properties Tue Apr 23 14:02:46 2013 +0200 +++ b/distribution/config/commands/list-vms.properties Fri Mar 22 16:18:06 2013 -0400 @@ -13,7 +13,7 @@ description = lists all currently monitored VMs -usage = list-vms [-d [-u -p ]] [-l ] +usage = list-vms [-d ] [-l ] # This command does not have any options options = AUTO_DB_OPTIONS, AUTO_LOG_OPTION diff -r c551a80f08b2 -r 12894dbd3517 distribution/config/commands/ping.properties --- a/distribution/config/commands/ping.properties Tue Apr 23 14:02:46 2013 +0200 +++ b/distribution/config/commands/ping.properties Fri Mar 22 16:18:06 2013 -0400 @@ -15,6 +15,6 @@ description = using the Command Channel, send a ping to a running agent -usage = ping [-d [-u -p ]] [-l ] +usage = ping [-d ] [-l ] options = AUTO_DB_OPTIONS, AUTO_LOG_OPTION diff -r c551a80f08b2 -r 12894dbd3517 distribution/config/commands/vm-info.properties --- a/distribution/config/commands/vm-info.properties Tue Apr 23 14:02:46 2013 +0200 +++ b/distribution/config/commands/vm-info.properties Fri Mar 22 16:18:06 2013 -0400 @@ -13,7 +13,7 @@ description = shows basic information about a VM -usage = vm-info [--vmId ] [--hostId ] [-d [-u -p ]] [-l ] +usage = vm-info [--vmId ] [--hostId ] [-d ] [-l ] options = hostId, vmId, AUTO_DB_OPTIONS, AUTO_LOG_OPTION diff -r c551a80f08b2 -r 12894dbd3517 distribution/config/commands/vm-stat.properties --- a/distribution/config/commands/vm-stat.properties Tue Apr 23 14:02:46 2013 +0200 +++ b/distribution/config/commands/vm-stat.properties Fri Mar 22 16:18:06 2013 -0400 @@ -13,7 +13,7 @@ description = show various statistics about a VM -usage = vm-stat --hostId --vmId [-d [-u -p ]] [-l ] +usage = vm-stat --hostId --vmId [-d ] [-l ] options = hostId, vmId, continuous, AUTO_DB_OPTIONS, AUTO_LOG_OPTION diff -r c551a80f08b2 -r 12894dbd3517 distribution/config/commands/webservice.properties --- a/distribution/config/commands/webservice.properties Tue Apr 23 14:02:46 2013 +0200 +++ b/distribution/config/commands/webservice.properties Fri Mar 22 16:18:06 2013 -0400 @@ -26,7 +26,7 @@ description = starts and stops the thermostat web service -usage = webservice -d -b [-u -p ] [-l ] +usage = webservice -d -b [-l ] options = AUTO_DB_OPTIONS, dbUrl, bindAddrs, AUTO_LOG_OPTION diff -r c551a80f08b2 -r 12894dbd3517 distribution/pom.xml --- a/distribution/pom.xml Tue Apr 23 14:02:46 2013 +0200 +++ b/distribution/pom.xml Fri Mar 22 16:18:06 2013 -0400 @@ -148,6 +148,7 @@ true agent.properties + agent.auth diff -r c551a80f08b2 -r 12894dbd3517 eclipse/com.redhat.thermostat.client.feature/feature.xml --- a/eclipse/com.redhat.thermostat.client.feature/feature.xml Tue Apr 23 14:02:46 2013 +0200 +++ b/eclipse/com.redhat.thermostat.client.feature/feature.xml Fri Mar 22 16:18:06 2013 -0400 @@ -134,6 +134,13 @@ unpack="false"/> + + args) throws IOException { @@ -146,8 +157,9 @@ public static void deleteFilesUnder(String path) throws IOException { String[] filesToDelete = new File(path).list(); for (String toDelete : filesToDelete) { - if (!new File(path, toDelete).delete()) { - throw new IOException("cant delete: '" + new File(path, toDelete).toString() + "'."); + File theFile = new File(path, toDelete); + if (!theFile.delete()) { + throw new IOException("cant delete: '" + theFile.toString() + "'."); } } } @@ -162,5 +174,36 @@ assertFalse(stdErrContents.contains("Exception")); } + public static void assertOutputEndsWith(String stdOutContents, String expectedOutput) { + String endOfOut = stdOutContents.substring(stdOutContents.length() - expectedOutput.length()); + assertEquals(expectedOutput, endOfOut); + } + + public static void handleAuthPrompt(Spawn spawn, String url, String user, String password) throws IOException, TimeoutException { + spawn.expect("Please enter username for storage at " + url + ":"); + spawn.send(user + "\r"); + spawn.expect("Please enter password for storage at " + url + ":"); + spawn.send(password + "\r"); + } + + private static class LocaleExecutor implements Executor { + + public static final String[] LANG_C = { "LANG=C " }; + private String process; + + public LocaleExecutor(String process) { + this.process = process; + } + + @Override + public Process execute() throws IOException { + return Runtime.getRuntime().exec(process, LANG_C); + } + + @Override + public String toString() { + return process; + } + } } diff -r c551a80f08b2 -r 12894dbd3517 integration-tests/src/test/java/com/redhat/thermostat/itest/StorageConnectionTest.java --- a/integration-tests/src/test/java/com/redhat/thermostat/itest/StorageConnectionTest.java Tue Apr 23 14:02:46 2013 +0200 +++ b/integration-tests/src/test/java/com/redhat/thermostat/itest/StorageConnectionTest.java Fri Mar 22 16:18:06 2013 -0400 @@ -40,7 +40,6 @@ import org.junit.AfterClass; import org.junit.BeforeClass; -import org.junit.Ignore; import org.junit.Test; import expectj.ExpectJException; @@ -69,10 +68,11 @@ @Test public void testConnect() throws ExpectJException, TimeoutException, IOException { - Spawn shell = spawnThermostat("shell"); + Spawn shell = spawnThermostat(true, "shell"); shell.expect(SHELL_PROMPT); shell.send("connect -d mongodb://127.0.0.1:27518\n"); + handleAuthPrompt(shell, "mongodb://127.0.0.1:27518", "", ""); shell.expect(SHELL_PROMPT); shell.send("exit\n"); shell.expectClose(); @@ -97,10 +97,11 @@ @Test public void testConnectAndDisconnectInShell() throws IOException, TimeoutException, ExpectJException { - Spawn shell = spawnThermostat("shell"); + Spawn shell = spawnThermostat(true, "shell"); shell.expect(SHELL_PROMPT); shell.send("connect -d mongodb://127.0.0.1:27518\n"); + handleAuthPrompt(shell, "mongodb://127.0.0.1:27518", "", ""); shell.expect(SHELL_PROMPT); shell.send("disconnect\n"); shell.send("exit\n"); diff -r c551a80f08b2 -r 12894dbd3517 integration-tests/src/test/java/com/redhat/thermostat/itest/VmCommandsTest.java --- a/integration-tests/src/test/java/com/redhat/thermostat/itest/VmCommandsTest.java Tue Apr 23 14:02:46 2013 +0200 +++ b/integration-tests/src/test/java/com/redhat/thermostat/itest/VmCommandsTest.java Fri Mar 22 16:18:06 2013 -0400 @@ -72,14 +72,16 @@ @Test public void testListVms() throws Exception { Spawn vmList = commandAgainstMongo("list-vms"); + handleAuthPrompt(vmList, "mongodb://127.0.0.1:27518", "", ""); vmList.expectClose(); - - assertEquals("HOST_ID HOST VM_ID STATUS VM_NAME\n", vmList.getCurrentStandardOutContents()); + assertOutputEndsWith(vmList.getCurrentStandardOutContents(), "HOST_ID HOST VM_ID STATUS VM_NAME\n\n"); } @Test public void testVmStat() throws Exception { Spawn vmStat = commandAgainstMongo("vm-stat"); + // TODO include required options to test meaningfully + //handleAuthPrompt(vmStat, "mongodb://127.0.0.1:27518", "", ""); vmStat.expectClose(); System.out.println(vmStat.getCurrentStandardOutContents()); @@ -90,6 +92,8 @@ @Test public void testVmInfo() throws Exception { Spawn vmInfo = commandAgainstMongo("vm-info"); + // TODO include required options to test meaningfully + // handleAuthPrompt(vmInfo, "mongodb://127.0.0.1:27518", "", ""); vmInfo.expectClose(); assertNoExceptions(vmInfo.getCurrentStandardOutContents(), vmInfo.getCurrentStandardErrContents()); @@ -109,6 +113,11 @@ for (String command : commands) { Spawn heapCommand = commandAgainstMongo(command); + // TODO include required options to test each command meaningfully + if (command.equals("list-heap-dumps")) { + // No missing options, times out waiting for user/pass input without the following: + handleAuthPrompt(heapCommand, "mongodb://127.0.0.1:27518", "", ""); + } heapCommand.expectClose(); assertCommandIsFound( @@ -128,7 +137,7 @@ completeArgs.addAll(Arrays.asList(args)); completeArgs.add("-d"); completeArgs.add("mongodb://127.0.0.1:27518"); - return spawnThermostat(completeArgs.toArray(new String[0])); + return spawnThermostat(true, completeArgs.toArray(new String[0])); } } diff -r c551a80f08b2 -r 12894dbd3517 launcher/src/main/java/com/redhat/thermostat/launcher/internal/CommonOptions.java --- a/launcher/src/main/java/com/redhat/thermostat/launcher/internal/CommonOptions.java Tue Apr 23 14:02:46 2013 +0200 +++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/CommonOptions.java Fri Mar 22 16:18:06 2013 -0400 @@ -60,15 +60,11 @@ static final String LOG_LEVEL_ARG = "logLevel"; static final String DB_URL_ARG = "dbUrl"; - static final String USERNAME_ARG = "username"; - static final String PASSWORD_ARG = "password"; static final Set ALL_COMMON_OPTIONS = new HashSet<>(4); static { ALL_COMMON_OPTIONS.add(LOG_LEVEL_ARG); ALL_COMMON_OPTIONS.add(DB_URL_ARG); - ALL_COMMON_OPTIONS.add(USERNAME_ARG); - ALL_COMMON_OPTIONS.add(PASSWORD_ARG); } static final Translate t = LocaleResources.createLocalizer(); @@ -78,18 +74,8 @@ Option dbUrlOption = new Option("d", DB_URL_ARG, true, dbUrlDesc); dbUrlOption.setRequired(false); dbUrlOption.setArgName(DB_URL_ARG); - String usernameDesc = t.localize(LocaleResources.OPTION_USERNAME_DESC); - Option usernameOption = new Option("u", USERNAME_ARG, true, usernameDesc); - usernameOption.setRequired(false); - usernameOption.setArgName(USERNAME_ARG); - String passwordDesc = t.localize(LocaleResources.OPTION_PASSWORD_DESC); - Option passwordOption = new Option("p", PASSWORD_ARG, true, passwordDesc); - passwordOption.setRequired(false); - passwordOption.setArgName(PASSWORD_ARG); - List - - @@ -107,7 +101,7 @@ - - @@ -154,21 +142,21 @@ - - @@ -215,14 +197,14 @@ - - @@ -269,14 +245,14 @@ - - @@ -323,14 +293,14 @@ - - @@ -377,7 +341,7 @@ - -