Mercurial > hg > release > thermostat-1.0
changeset 1372:2caadc2d158b
Inject StorageCredentials to delay password loading in memory
reviewed-by: jerboaa
review-thread: http://icedtea.classpath.org/pipermail/thermostat/2013-November/008920.html
line wrap: on
line diff
--- a/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/AgentApplication.java Mon Nov 18 11:48:35 2013 -0700 +++ b/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/AgentApplication.java Thu Nov 21 13:00:46 2013 -0700 @@ -118,7 +118,6 @@ final DbService dbService = dbServiceFactory.createDbService( - configuration.getUsername(), configuration.getPassword(), configuration.getDBConnectionString()); shutdownLatch = new CountDownLatch(1);
--- a/agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/AgentApplicationTest.java Mon Nov 18 11:48:35 2013 -0700 +++ b/agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/AgentApplicationTest.java Thu Nov 21 13:00:46 2013 -0700 @@ -40,7 +40,6 @@ import static org.junit.Assert.fail; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyString; -import static org.mockito.Matchers.isA; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doThrow; @@ -125,7 +124,7 @@ dbServiceFactory = mock(DbServiceFactory.class); dbService = mock(DbService.class); writerId = mock(WriterID.class); - when(dbServiceFactory.createDbService(anyString(), isA(char[].class), anyString())).thenReturn(dbService); + when(dbServiceFactory.createDbService(anyString())).thenReturn(dbService); exitStatus = mock(ExitStatus.class); }
--- a/agent/core/src/main/java/com/redhat/thermostat/agent/config/AgentConfigsUtils.java Mon Nov 18 11:48:35 2013 -0700 +++ b/agent/core/src/main/java/com/redhat/thermostat/agent/config/AgentConfigsUtils.java Thu Nov 21 13:00:46 2013 -0700 @@ -39,37 +39,23 @@ import java.io.File; import java.io.FileInputStream; import java.io.IOException; -import java.io.InputStreamReader; -import java.io.Reader; -import java.nio.charset.StandardCharsets; -import java.util.Arrays; import java.util.Properties; -import com.redhat.thermostat.agent.locale.LocaleResources; import com.redhat.thermostat.shared.config.InvalidConfigurationException; -import com.redhat.thermostat.shared.locale.Translate; public class AgentConfigsUtils { - private static final Translate<LocaleResources> t = LocaleResources.createLocalizer(); - private static File systemConfiguration, userConfiguration, agentAuthFile; + private static File systemConfiguration, userConfiguration; - 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 void setConfigFiles(File systemConfigFile, File userConfigFile, File agentAuthFile) { + public static void setConfigFiles(File systemConfigFile, File userConfigFile) { AgentConfigsUtils.systemConfiguration = systemConfigFile; AgentConfigsUtils.userConfiguration = userConfigFile; - AgentConfigsUtils.agentAuthFile = agentAuthFile; } public static AgentStartupConfiguration createAgentConfigs() throws InvalidConfigurationException { AgentStartupConfiguration config = new AgentStartupConfiguration(); readAndSetProperties(systemConfiguration, userConfiguration, config); - setAuthConfigFromFile(agentAuthFile, config); return config; } @@ -113,113 +99,5 @@ 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(new char[]{}); - if (authFile != null && authFile.canRead() && authFile.isFile()) { - long length = authFile.length(); - char[] authData = null; - try (Reader reader = new InputStreamReader(new FileInputStream(authFile), StandardCharsets.US_ASCII)) { - if (length > Integer.MAX_VALUE || length < 0L) { - throw new InvalidConfigurationException(t.localize(LocaleResources.FILE_NOT_VALID, authFile.getCanonicalPath())); - } - int len = (int) length; // A reasonable file will be within int range. - // file size in bytes >= # of chars so this size should be sufficient. - 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. - int chars = reader.read(authData, 0, len); - parseAuthConfigFromData(authData, chars, config); - } catch (IOException e) { - throw new InvalidConfigurationException(e); - } finally { - if (authData != null) { - 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(value); - position = nextLine(data, position); - 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(t.localize(LocaleResources.BAD_AGENT_AUTH_CONTENTS)); - } - } - - 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; - } }
--- a/agent/core/src/main/java/com/redhat/thermostat/agent/config/AgentStartupConfiguration.java Mon Nov 18 11:48:35 2013 -0700 +++ b/agent/core/src/main/java/com/redhat/thermostat/agent/config/AgentStartupConfiguration.java Thu Nov 21 13:00:46 2013 -0700 @@ -36,16 +36,13 @@ package com.redhat.thermostat.agent.config; -import com.redhat.thermostat.storage.config.AuthenticationConfiguration; import com.redhat.thermostat.storage.config.StartupConfiguration; -public class AgentStartupConfiguration implements StartupConfiguration, AuthenticationConfiguration { +public class AgentStartupConfiguration implements StartupConfiguration { private boolean purge; private String url; - private String username; - private char[] password; private long startTime; private String address; @@ -79,24 +76,6 @@ return purge; } - public void setUsername(String username) { - this.username = username; - } - - @Override - public String getUsername() { - return username; - } - - public void setPassword(char[] password) { - this.password = password; - } - - @Override - public char[] getPassword() { - return password; - } - public void setConfigListenAddress(String address) { this.address = address; }
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/agent/core/src/main/java/com/redhat/thermostat/agent/config/AgentStorageCredentials.java Thu Nov 21 13:00:46 2013 -0700 @@ -0,0 +1,246 @@ +/* + * Copyright 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 + * <http://www.gnu.org/licenses/>. + * + * 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.agent.config; + +import java.io.File; +import java.io.FileInputStream; +import java.io.InputStreamReader; +import java.io.IOException; +import java.io.Reader; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; + +import com.redhat.thermostat.agent.locale.LocaleResources; +import com.redhat.thermostat.shared.config.InvalidConfigurationException; +import com.redhat.thermostat.shared.locale.Translate; +import com.redhat.thermostat.storage.core.StorageCredentials; + +public class AgentStorageCredentials implements StorageCredentials { + + private static final Translate<LocaleResources> t = LocaleResources.createLocalizer(); + + 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 = '#'; + + final File authFile; + private String username = null; // default value + + public AgentStorageCredentials(File agentAuthFile) { + this.authFile = agentAuthFile; + long length = authFile.length(); + if (length > Integer.MAX_VALUE || length < 0L) { + // Unlikely issue with authFile, try to get path to share with user via exception message + String authPath = ""; + try { + authPath = authFile.getCanonicalPath(); + } catch (IOException e) { + authPath = "ERROR_GETTING_CANONICAL_PATH"; + } + throw new InvalidConfigurationException(t.localize(LocaleResources.FILE_NOT_VALID, authPath)); + } + // Cache username but not password, instead read that on demand to prevent heap dump + // password leak attack. + initUsernameFromFile(); + } + + @Override + public String getUsername() { + return username; + } + + @Override + public char[] getPassword() { + return readPasswordFromFile(); + } + + private void initUsernameFromFile() { + char[] authData = getAuthFileData(); + if (authData == null) { + return; + } + try { + setUsernameFromData(authData, (int) authFile.length()); + } finally { + clearCharArray(authData); + } + } + + private char[] readPasswordFromFile() { + char[] authData = getAuthFileData(); + if (authData == null) { + return null; + } + char[] password = null; + try { + password = getValueFromData(authData, (int) authFile.length(), pw); + } finally { + clearCharArray(authData); + } + return password; + } + + private char[] getAuthFileData() { + char[] authData = null; + if (authFile != null && authFile.canRead() && authFile.isFile()) { + int length = (int) authFile.length(); // Verified during constructor that this cast is safe + try (Reader reader = new InputStreamReader(new FileInputStream(authFile), StandardCharsets.US_ASCII)) { + // file size in bytes >= # of chars so this size should be sufficient. + authData = new char[length]; + // 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. + int chars = reader.read(authData, 0, length); + if (chars != length) { + throw new InvalidConfigurationException("End of auth file stream reached unexpectedly."); + } + } catch (IOException e) { + throw new InvalidConfigurationException(e); + } + } + return authData; + } + + private void setUsernameFromData(char[] data, int dataLen) { + try { + char[] userChars = getValueFromData(data, dataLen, user); + if (userChars != null) { + username = new String(userChars); + } + } finally { + + } + } + + private char[] getValueFromData(char[] data, int dataLen, char[] target) { + 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 + if (pw.equals(target)) { + return value; + } else { + clearCharArray(value); + } + position = nextLine(data, position); + value = null; + continue; + } + value = getUserName(data, position); + if (value != null) { + // Username + if (user.equals(target)) { + return value; + } else { + clearCharArray(value); + } + position = nextLine(data, position); + value = null; + continue; + } + // Unrecognized content in file + throw new InvalidConfigurationException(t.localize(LocaleResources.BAD_AGENT_AUTH_CONTENTS)); + } + return null; + } + + private 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 char[] getPassword(char[] data, int start) { + return getValue(data, start, pw); + } + + private char[] getUserName(char[] data, int start) { + return getValue(data, start, user); + } + + private 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 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; + } + + public void clearCharArray(char[] chars) { + if (chars != null) { + Arrays.fill(chars, '\0'); + } + } + + public String getStorageUrl() { + return null; + } +}
--- a/agent/core/src/main/java/com/redhat/thermostat/agent/internal/Activator.java Mon Nov 18 11:48:35 2013 -0700 +++ b/agent/core/src/main/java/com/redhat/thermostat/agent/internal/Activator.java Thu Nov 21 13:00:46 2013 -0700 @@ -43,9 +43,11 @@ import com.redhat.thermostat.agent.RMIRegistry; import com.redhat.thermostat.agent.VmBlacklist; import com.redhat.thermostat.agent.config.AgentConfigsUtils; +import com.redhat.thermostat.agent.config.AgentStorageCredentials; import com.redhat.thermostat.agent.utils.management.MXBeanConnectionPool; import com.redhat.thermostat.agent.utils.username.UserNameUtil; import com.redhat.thermostat.shared.config.CommonPaths; +import com.redhat.thermostat.storage.core.StorageCredentials; import com.redhat.thermostat.utils.management.internal.AgentProxyFilter; import com.redhat.thermostat.utils.management.internal.MXBeanConnectionPoolImpl; import com.redhat.thermostat.utils.username.internal.UserNameUtilImpl; @@ -62,8 +64,9 @@ CommonPaths paths = context.getService(pathsRef); pool = new MXBeanConnectionPoolImpl(registry, paths.getSystemBinRoot()); context.registerService(MXBeanConnectionPool.class, pool, null); - AgentConfigsUtils.setConfigFiles(paths.getSystemAgentConfigurationFile(), paths.getUserAgentConfigurationFile(), - paths.getUserAgentAuthConfigFile()); + StorageCredentials creds = new AgentStorageCredentials(paths.getUserAgentAuthConfigFile()); + context.registerService(StorageCredentials.class, creds, null); + AgentConfigsUtils.setConfigFiles(paths.getSystemAgentConfigurationFile(), paths.getUserAgentConfigurationFile()); paths = null; context.ungetService(pathsRef); context.registerService(UserNameUtil.class, new UserNameUtilImpl(), null);
--- a/agent/core/src/test/java/com/redhat/thermostat/agent/config/AgentConfigsUtilsTest.java Mon Nov 18 11:48:35 2013 -0700 +++ b/agent/core/src/test/java/com/redhat/thermostat/agent/config/AgentConfigsUtilsTest.java Thu Nov 21 13:00:46 2013 -0700 @@ -37,10 +37,8 @@ package com.redhat.thermostat.agent.config; import java.io.File; -import java.io.FileWriter; import java.io.IOException; import java.util.Properties; -import java.util.Random; import org.junit.Assert; import org.junit.BeforeClass; @@ -51,11 +49,8 @@ public class AgentConfigsUtilsTest { - private static Random random; - @BeforeClass public static void setUpOnce() { - random = new Random(); Properties agentProperties = new Properties(); agentProperties.setProperty("SAVE_ON_EXIT", "true"); @@ -64,9 +59,8 @@ try { TestUtils.setupAgentConfigs(agentProperties); File agentConf = TestUtils.getAgentConfFile(); - File agentAuth = TestUtils.getAgentAuthFile(); // By default system config == user config - AgentConfigsUtils.setConfigFiles(agentConf, agentConf, agentAuth); + AgentConfigsUtils.setConfigFiles(agentConf, agentConf); } catch (IOException e) { throw new AssertionError("Unable to create agent configuration", e); } @@ -78,77 +72,6 @@ Assert.assertFalse(config.purge()); Assert.assertEquals("42.42.42.42:42", config.getConfigListenAddress()); - Assert.assertEquals("user", config.getUsername()); - Assert.assertEquals("pass", new String(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", new String(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("", new String(config.getPassword())); - } - - // TODO add test to ensure user agent config overrides system agent config. - - @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("", new String(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", new String(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()); } }
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/agent/core/src/test/java/com/redhat/thermostat/agent/config/AgentStorageCredentialsTest.java Thu Nov 21 13:00:46 2013 -0700 @@ -0,0 +1,92 @@ +/* + * Copyright 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 + * <http://www.gnu.org/licenses/>. + * + * 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.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.BeforeClass; +import org.junit.Test; + +public class AgentStorageCredentialsTest { + + private static Random random; + + @BeforeClass + public static void setUpOnce() { + random = new Random(); + } + + @Test + public void testAuthConfigFromFile() throws IOException { + File tmpAuth = createTempAuthFile("username=user\npassword=pass\n"); + AgentStorageCredentials creds = new AgentStorageCredentials(tmpAuth); + Assert.assertEquals("user", creds.getUsername()); + Assert.assertEquals("pass", new String(creds.getPassword())); + } + + @Test + public void testAuthConfigFromEmptyFile() throws IOException { + File tmpAuth = createTempAuthFile(""); + AgentStorageCredentials creds = new AgentStorageCredentials(tmpAuth); + Assert.assertNull(creds.getUsername()); + Assert.assertNull(creds.getPassword()); + } + + @Test + public void testAuthConfigWithConfigCommentedOut() throws IOException { + File tmpAuth = createTempAuthFile("#username=user\n#password=pass\n"); + AgentStorageCredentials creds = new AgentStorageCredentials(tmpAuth); + Assert.assertNull(creds.getUsername()); + Assert.assertNull(creds.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; + } +}
--- a/agent/core/src/test/java/com/redhat/thermostat/agent/internal/ActivatorTest.java Mon Nov 18 11:48:35 2013 -0700 +++ b/agent/core/src/test/java/com/redhat/thermostat/agent/internal/ActivatorTest.java Thu Nov 21 13:00:46 2013 -0700 @@ -61,6 +61,7 @@ CommonPaths paths = mock(CommonPaths.class); when(paths.getSystemNativeLibsRoot()).thenReturn(new File("target")); + when(paths.getUserAgentAuthConfigFile()).thenReturn(new File("not.exist.does.not.matter")); NativeLibraryResolver.setCommonPaths(paths); StubBundleContext context = new StubBundleContext();
--- a/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/Activator.java Mon Nov 18 11:48:35 2013 -0700 +++ b/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/Activator.java Thu Nov 21 13:00:46 2013 -0700 @@ -74,8 +74,8 @@ public void dependenciesAvailable(Map<String, Object> services) { Keyring keyring = (Keyring) services.get(Keyring.class.getName()); CommonPaths paths = (CommonPaths) services.get(CommonPaths.class.getName()); - ClientPreferences prefs = new ClientPreferences(keyring, paths); - reg.registerCommand("connect", new ConnectCommand(prefs)); + ClientPreferences prefs = new ClientPreferences(paths); + reg.registerCommand("connect", new ConnectCommand(prefs, keyring)); reg.registerCommand("shell", new ShellCommand(context, paths)); }
--- a/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/ConnectCommand.java Mon Nov 18 11:48:35 2013 -0700 +++ b/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/ConnectCommand.java Thu Nov 21 13:00:46 2013 -0700 @@ -36,24 +36,23 @@ package com.redhat.thermostat.client.cli.internal; -import java.io.IOException; -import java.util.Arrays; import java.util.Objects; import org.osgi.framework.BundleContext; import org.osgi.framework.FrameworkUtil; import org.osgi.framework.ServiceReference; +import org.osgi.framework.ServiceRegistration; import com.redhat.thermostat.common.cli.AbstractCommand; import com.redhat.thermostat.common.cli.CommandContext; import com.redhat.thermostat.common.cli.CommandException; import com.redhat.thermostat.common.config.ClientPreferences; -import com.redhat.thermostat.common.tools.StorageAuthInfoGetter; -import com.redhat.thermostat.shared.config.CommonPaths; +import com.redhat.thermostat.launcher.InteractiveStorageCredentials; import com.redhat.thermostat.shared.locale.Translate; import com.redhat.thermostat.storage.core.ConnectionException; import com.redhat.thermostat.storage.core.DbService; import com.redhat.thermostat.storage.core.DbServiceFactory; +import com.redhat.thermostat.storage.core.StorageCredentials; import com.redhat.thermostat.storage.core.StorageException; import com.redhat.thermostat.utils.keyring.Keyring; @@ -72,17 +71,19 @@ private static final Translate<LocaleResources> translator = LocaleResources.createLocalizer(); private ClientPreferences prefs; + private Keyring keyring; private BundleContext context; private DbServiceFactory dbServiceFactory; - public ConnectCommand(ClientPreferences prefs) { - this(FrameworkUtil.getBundle(ConnectCommand.class).getBundleContext(), new DbServiceFactory(), prefs); + public ConnectCommand(ClientPreferences prefs, Keyring keyring) { + this(FrameworkUtil.getBundle(ConnectCommand.class).getBundleContext(), new DbServiceFactory(), prefs, keyring); } - ConnectCommand(BundleContext context, DbServiceFactory dbServiceFactory, ClientPreferences prefs) { + ConnectCommand(BundleContext context, DbServiceFactory dbServiceFactory, ClientPreferences prefs, Keyring keyring) { this.context = context; this.dbServiceFactory = dbServiceFactory; this.prefs = prefs; + this.keyring = keyring; } @Override @@ -98,26 +99,12 @@ String dbUrl = ctx.getArguments().getArgument(DB_URL_ARG); // This argument is considered "required" so option parsing should mean this is impossible. Objects.requireNonNull(dbUrl); - String username = null; - char[] password = null; - if (prefs.getConnectionUrl().equals(dbUrl)) { - // Have we previously saved connection parameters for this Url? - username = prefs.getUserName(); - password = prefs.getPassword(); - } - 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 = getUserPass.getPassword(dbUrl); - } catch (IOException e) { - throw new CommandException(translator.localize(LocaleResources.COMMAND_CONNECT_USER_PROMPT_ERROR), e); - } - } + StorageCredentials creds = new InteractiveStorageCredentials(prefs, keyring, dbUrl, ctx.getConsole()); + ServiceRegistration credsReg = context.registerService(StorageCredentials.class, creds, null); + try { // may throw StorageException if storage url is not supported - DbService service = dbServiceFactory.createDbService(username, password, dbUrl); + DbService service = dbServiceFactory.createDbService(dbUrl); service.connect(); } catch (StorageException ex) { throw new CommandException(translator.localize(LocaleResources.COMMAND_CONNECT_INVALID_STORAGE, dbUrl)); @@ -126,9 +113,7 @@ String message = ( error == null ? "" : " " + translator.localize(LocaleResources.COMMAND_CONNECT_ERROR, error).getContents() ); throw new CommandException(translator.localize(LocaleResources.COMMAND_CONNECT_FAILED_TO_CONNECT, dbUrl + message), ex); } finally { - if (password != null) { - Arrays.fill(password, '\0'); - } + credsReg.unregister(); } }
--- a/client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/ConnectCommandTest.java Mon Nov 18 11:48:35 2013 -0700 +++ b/client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/ConnectCommandTest.java Thu Nov 21 13:00:46 2013 -0700 @@ -58,6 +58,7 @@ import com.redhat.thermostat.storage.core.DbServiceFactory; import com.redhat.thermostat.test.TestCommandContextFactory; import com.redhat.thermostat.testutils.StubBundleContext; +import com.redhat.thermostat.utils.keyring.Keyring; public class ConnectCommandTest { @@ -76,8 +77,9 @@ context = new StubBundleContext(); dbServiceFactory = mock(DbServiceFactory.class); ClientPreferences prefs = mock(ClientPreferences.class); + Keyring keyring = mock(Keyring.class); when(prefs.getConnectionUrl()).thenReturn("http://localhost"); - cmd = new ConnectCommand(context, dbServiceFactory, prefs); + cmd = new ConnectCommand(context, dbServiceFactory, prefs, keyring); } private void setupCommandContextFactory() { @@ -116,7 +118,7 @@ String username = "testuser"; String password = "testpassword"; String dbUrl = "mongodb://10.23.122.1:12578"; - when(dbServiceFactory.createDbService(eq(username), eq(password.toCharArray()), eq(dbUrl))).thenReturn(dbService); + when(dbServiceFactory.createDbService(eq(dbUrl))).thenReturn(dbService); SimpleArguments args = new SimpleArguments(); args.addArgument("dbUrl", dbUrl); CommandContext ctx = cmdCtxFactory.createContext(args);
--- a/client/core/src/main/java/com/redhat/thermostat/client/core/views/ClientConfigurationView.java Mon Nov 18 11:48:35 2013 -0700 +++ b/client/core/src/main/java/com/redhat/thermostat/client/core/views/ClientConfigurationView.java Thu Nov 21 13:00:46 2013 -0700 @@ -55,7 +55,7 @@ void setConnectionUrl(String url); void setPassword(char[] password); void setUserName(String username); - void setSaveEntitlemens(boolean save); + void setSaveEntitlements(boolean save); boolean getSaveEntitlements(); String getUserName();
--- a/client/core/src/main/java/com/redhat/thermostat/client/ui/ClientConfigReconnector.java Mon Nov 18 11:48:35 2013 -0700 +++ b/client/core/src/main/java/com/redhat/thermostat/client/ui/ClientConfigReconnector.java Thu Nov 21 13:00:46 2013 -0700 @@ -38,10 +38,11 @@ package com.redhat.thermostat.client.ui; import com.redhat.thermostat.common.config.ClientPreferences; +import com.redhat.thermostat.storage.core.StorageCredentials; public interface ClientConfigReconnector { - void reconnect(ClientPreferences prefs); + void reconnect(ClientPreferences prefs, StorageCredentials creds); void abort(); }
--- a/client/core/src/main/java/com/redhat/thermostat/client/ui/ClientConfigurationController.java Mon Nov 18 11:48:35 2013 -0700 +++ b/client/core/src/main/java/com/redhat/thermostat/client/ui/ClientConfigurationController.java Thu Nov 21 13:00:46 2013 -0700 @@ -44,22 +44,22 @@ import com.redhat.thermostat.client.core.views.ClientConfigurationView.Action; import com.redhat.thermostat.common.ActionEvent; import com.redhat.thermostat.common.ActionListener; -import com.redhat.thermostat.common.config.ClientPreferences; import com.redhat.thermostat.common.utils.LoggingUtils; +import com.redhat.thermostat.storage.core.StorageCredentials; public class ClientConfigurationController implements ActionListener<Action> { private static final Logger logger = LoggingUtils.getLogger(ClientConfigurationController.class); private final ClientConfigurationView view; - private final ClientPreferences model; + private final ClientPreferencesModel model; private final ClientConfigReconnector reconnector; - public ClientConfigurationController(ClientPreferences model, ClientConfigurationView view) { + public ClientConfigurationController(ClientPreferencesModel model, ClientConfigurationView view) { this(model, view, null); } - public ClientConfigurationController(ClientPreferences model, ClientConfigurationView view, ClientConfigReconnector reconnector) { + public ClientConfigurationController(ClientPreferencesModel model, ClientConfigurationView view, ClientConfigReconnector reconnector) { this.model = model; this.view = view; this.reconnector = reconnector; @@ -72,7 +72,7 @@ } private void updateViewFromModel() { - view.setSaveEntitlemens(model.getSaveEntitlements()); + view.setSaveEntitlements(model.getSaveEntitlements()); view.setConnectionUrl(model.getConnectionUrl()); view.setPassword(model.getPassword()); @@ -84,7 +84,6 @@ model.setConnectionUrl(view.getConnectionUrl()); model.setCredentials(view.getUserName(), view.getPassword()); - try { model.flush(); } catch (IOException e) { @@ -103,7 +102,7 @@ updateModelFromView(); view.hideDialog(); if (reconnector != null) { - reconnector.reconnect(model); + reconnector.reconnect(model.getPreferences(), new PreferencesModelStorageCredentials(model)); } break; case CLOSE_CANCEL: @@ -115,5 +114,24 @@ } } + + class PreferencesModelStorageCredentials implements StorageCredentials { + + private ClientPreferencesModel model; + + PreferencesModelStorageCredentials(ClientPreferencesModel model) { + this.model = model; + } + + public String getUsername() { + return model.getUserName(); + } + + @Override + public char[] getPassword() { + return model.getPassword(); + } + + } }
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/client/core/src/main/java/com/redhat/thermostat/client/ui/ClientPreferencesModel.java Thu Nov 21 13:00:46 2013 -0700 @@ -0,0 +1,129 @@ +/* + * Copyright 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 + * <http://www.gnu.org/licenses/>. + * + * 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.client.ui; + +import java.io.IOException; + +import com.redhat.thermostat.common.config.ClientPreferences; +import com.redhat.thermostat.utils.keyring.Keyring; + +public class ClientPreferencesModel { + private ClientPreferences prefs; + private Keyring keyring; + + private String initialUrl; + private String initialUsername; + + private char[] password; + private String url; + private String username; + + public ClientPreferencesModel(Keyring keyring, ClientPreferences prefs) { + initialUrl = prefs.getConnectionUrl(); + initialUsername = prefs.getUserName(); + + this.prefs = prefs; + this.keyring = keyring; + } + + public boolean getSaveEntitlements() { + return prefs.getSaveEntitlements(); + } + + public void setSaveEntitlements(boolean save) { + prefs.setSaveEntitlements(save); + } + + public String getConnectionUrl() { + if (getSaveEntitlements()) { + return prefs.getConnectionUrl(); + } + return url; + } + + public void setConnectionUrl(String url) { + this.url = url; + if (getSaveEntitlements()) { + prefs.setConnectionUrl(url); + } + } + + public char[] getPassword() { + if (getSaveEntitlements()) { + return keyring.getPassword(getConnectionUrl(), getUserName()); + } + return password; + } + + public String getUserName() { + if (getSaveEntitlements()) { + return prefs.getUserName(); + } + return username; + } + + public void setCredentials(String userName, char[] password) { + this.username = userName; + this.password = password; + + if (getSaveEntitlements()) { + prefs.setUserName(userName); + keyring.savePassword(getConnectionUrl(), userName, password); + } + } + + public void flush() throws IOException { + if (getSaveEntitlements()) { + keyring.savePassword(getConnectionUrl(), username, password); + prefs.flush(); + } else { + // getSaveEntitlements() should still be written to file, it may have changed. + String currentUrl = getConnectionUrl(); + String currentUser = getUserName(); + prefs.setConnectionUrl(initialUrl); + prefs.setUserName(initialUsername); + prefs.flush(); + prefs.setConnectionUrl(currentUrl); + prefs.setUserName(currentUser); + + } + keyring.savePassword(getConnectionUrl(), username, password); + } + + public ClientPreferences getPreferences() { + return prefs; + } +}
--- a/client/core/src/test/java/com/redhat/thermostat/client/ui/ClientConfigurationControllerTest.java Mon Nov 18 11:48:35 2013 -0700 +++ b/client/core/src/test/java/com/redhat/thermostat/client/ui/ClientConfigurationControllerTest.java Thu Nov 21 13:00:46 2013 -0700 @@ -49,19 +49,19 @@ import com.redhat.thermostat.client.core.views.ClientConfigurationView; import com.redhat.thermostat.common.ActionEvent; -import com.redhat.thermostat.common.config.ClientPreferences; +import com.redhat.thermostat.storage.core.StorageCredentials; public class ClientConfigurationControllerTest { - private ClientPreferences model; + private ClientPreferencesModel model; private ClientConfigurationView view; @Before public void setUp() { - model = mock(ClientPreferences.class); + model = mock(ClientPreferencesModel.class); when(model.getConnectionUrl()).thenReturn("mock-connection-url"); + when(model.getUserName()).thenReturn("mock-username"); when(model.getPassword()).thenReturn("mock-password".toCharArray()); - when(model.getUserName()).thenReturn("mock-username"); when(model.getSaveEntitlements()).thenReturn(false); view = mock(ClientConfigurationView.class); @@ -87,7 +87,7 @@ verify(view).setConnectionUrl(eq("mock-connection-url")); verify(view).setPassword(eq("mock-password".toCharArray())); verify(view).setUserName(eq("mock-username")); - verify(view).setSaveEntitlemens(eq(false)); + verify(view).setSaveEntitlements(eq(false)); verify(view).showDialog(); } @@ -139,12 +139,12 @@ controller.actionPerformed(new ActionEvent<>(view, ClientConfigurationView.Action.CLOSE_ACCEPT)); verifyCloseAcceptCommon(); - verify(reconnector).reconnect(model); + verify(reconnector).reconnect(eq(model.getPreferences()), any(StorageCredentials.class)); } private void verifyCloseAcceptCommon() { verify(model).setConnectionUrl(eq("mock-connection-url")); - verify(model).setCredentials(eq("mock-username"), eq("mock-password".toCharArray())); + verify(model).setCredentials(any(String.class), isA(char[].class)); verify(model).setSaveEntitlements(eq(true)); verify(view).getConnectionUrl();
--- a/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/Main.java Mon Nov 18 11:48:35 2013 -0700 +++ b/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/Main.java Thu Nov 21 13:00:46 2013 -0700 @@ -38,7 +38,6 @@ import java.awt.EventQueue; import java.lang.reflect.InvocationTargetException; -import java.util.Arrays; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; import java.util.logging.Level; @@ -48,6 +47,7 @@ import javax.swing.SwingUtilities; import org.osgi.framework.BundleContext; +import org.osgi.framework.ServiceRegistration; import com.redhat.thermostat.client.core.views.ClientConfigurationView; import com.redhat.thermostat.client.locale.LocaleResources; @@ -56,6 +56,7 @@ import com.redhat.thermostat.client.swing.internal.vmlist.UIDefaultsImpl; import com.redhat.thermostat.client.ui.ClientConfigReconnector; import com.redhat.thermostat.client.ui.ClientConfigurationController; +import com.redhat.thermostat.client.ui.ClientPreferencesModel; import com.redhat.thermostat.common.ApplicationService; import com.redhat.thermostat.common.config.ClientPreferences; import com.redhat.thermostat.common.utils.LoggingUtils; @@ -66,6 +67,7 @@ import com.redhat.thermostat.storage.core.Connection.ConnectionStatus; import com.redhat.thermostat.storage.core.DbService; import com.redhat.thermostat.storage.core.DbServiceFactory; +import com.redhat.thermostat.storage.core.StorageCredentials; import com.redhat.thermostat.storage.core.StorageException; import com.redhat.thermostat.utils.keyring.Keyring; @@ -113,7 +115,7 @@ this.paths = paths; this.shutdown = shutdown; this.mainWindowRunnable = mainWindowRunnable; - this.prefs = new ClientPreferences(keyring, paths); + this.prefs = new ClientPreferences(paths); } public void run() { @@ -157,8 +159,9 @@ private void tryConnecting() { final ExecutorService service = appSvc.getApplicationExecutor(); - ClientPreferences prefs = new ClientPreferences(keyring, paths); - connect(prefs, service); + ClientPreferences prefs = new ClientPreferences(paths); + StorageCredentials creds = new KeyringStorageCredentials(keyring, prefs); + connect(prefs, creds, service); } private class ConnectionAttempt implements Runnable { @@ -197,13 +200,16 @@ } } } + - private void connect(ClientPreferences prefs, ExecutorService service) { + + private void connect(ClientPreferences prefs, StorageCredentials creds, ExecutorService service) { + @SuppressWarnings("rawtypes") + final ServiceRegistration credsReg = context.registerService(StorageCredentials.class, creds, null); final ConnectionHandler reconnectionHandler = new ConnectionHandler(service); try { // create DbService with potentially modified parameters - final char[] password = prefs.getPassword(); - final DbService dbService = dbServiceFactory.createDbService(prefs.getUserName(), password, prefs.getConnectionUrl()); + final DbService dbService = dbServiceFactory.createDbService(prefs.getConnectionUrl()); dbService.addConnectionListener(reconnectionHandler); service.execute(new Runnable() { @Override @@ -215,9 +221,7 @@ // fails to connect. No need to notify our handler manually. logger.log(Level.FINE, "connection attempt failed: ", t); } finally { - if (password != null) { - Arrays.fill(password, '\0'); - } + credsReg.unregister(); } } }); @@ -237,8 +241,8 @@ } @Override - public void reconnect(ClientPreferences prefs) { - connect(prefs, service); + public void reconnect(ClientPreferences prefs, StorageCredentials creds) { + connect(prefs, creds, service); } @Override @@ -250,7 +254,7 @@ private void createPreferencesDialog(final ExecutorService service) { ClientConfigurationView configDialog = new ClientConfigurationSwing(); ClientConfigurationController controller = - new ClientConfigurationController(prefs, configDialog, new MainClientConfigReconnector(service)); + new ClientConfigurationController(new ClientPreferencesModel(keyring, prefs), configDialog, new MainClientConfigReconnector(service)); controller.showDialog(); } @@ -302,5 +306,27 @@ mainController.showMainMainWindow(); } } + + class KeyringStorageCredentials implements StorageCredentials { + + private Keyring keyring; + private ClientPreferences prefs; + + KeyringStorageCredentials(Keyring keyring, ClientPreferences prefs) { + this.keyring = keyring; + this.prefs = prefs; + } + + @Override + public String getUsername() { + return prefs.getUserName(); + } + + @Override + public char[] getPassword() { + return keyring.getPassword(prefs.getConnectionUrl(), prefs.getUserName()); + } + + } }
--- a/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/MainWindowControllerImpl.java Mon Nov 18 11:48:35 2013 -0700 +++ b/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/MainWindowControllerImpl.java Thu Nov 21 13:00:46 2013 -0700 @@ -68,6 +68,7 @@ import com.redhat.thermostat.client.ui.AgentInformationDisplayController; import com.redhat.thermostat.client.ui.AgentInformationDisplayModel; import com.redhat.thermostat.client.ui.ClientConfigurationController; +import com.redhat.thermostat.client.ui.ClientPreferencesModel; import com.redhat.thermostat.client.ui.HostInformationController; import com.redhat.thermostat.client.ui.MainWindowController; import com.redhat.thermostat.client.ui.MenuAction; @@ -446,9 +447,10 @@ } private void showConfigureClientPreferences() { - ClientPreferences prefs = new ClientPreferences(keyring, paths); + ClientPreferences prefs = new ClientPreferences(paths); + ClientPreferencesModel model = new ClientPreferencesModel(keyring, prefs); ClientConfigurationView view = clientConfigViewProvider.createView(); - ClientConfigurationController controller = new ClientConfigurationController(prefs, view); + ClientConfigurationController controller = new ClientConfigurationController(model, view); controller.showDialog(); }
--- a/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/views/ClientConfigurationSwing.java Mon Nov 18 11:48:35 2013 -0700 +++ b/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/views/ClientConfigurationSwing.java Thu Nov 21 13:00:46 2013 -0700 @@ -294,7 +294,7 @@ } @Override - public void setSaveEntitlemens(final boolean save) { + public void setSaveEntitlements(final boolean save) { SwingUtilities.invokeLater(new Runnable() { @Override public void run() {
--- a/client/swing/src/test/java/com/redhat/thermostat/client/swing/internal/MainTest.java Mon Nov 18 11:48:35 2013 -0700 +++ b/client/swing/src/test/java/com/redhat/thermostat/client/swing/internal/MainTest.java Thu Nov 21 13:00:46 2013 -0700 @@ -36,7 +36,6 @@ package com.redhat.thermostat.client.swing.internal; -import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.isA; import static org.mockito.Mockito.doAnswer; @@ -111,7 +110,7 @@ dbServiceFactory = mock(DbServiceFactory.class); dbService = mock(DbService.class); - when(dbServiceFactory.createDbService(anyString(), (char[]) any(), anyString())).thenReturn(dbService); + when(dbServiceFactory.createDbService(anyString())).thenReturn(dbService); connectionListenerCaptor = ArgumentCaptor.forClass(ConnectionListener.class); doNothing().when(dbService).addConnectionListener(connectionListenerCaptor.capture());
--- a/common/core/src/main/java/com/redhat/thermostat/common/config/ClientPreferences.java Mon Nov 18 11:48:35 2013 -0700 +++ b/common/core/src/main/java/com/redhat/thermostat/common/config/ClientPreferences.java Thu Nov 21 13:00:46 2013 -0700 @@ -48,7 +48,6 @@ import com.redhat.thermostat.common.utils.LoggingUtils; import com.redhat.thermostat.shared.config.CommonPaths; import com.redhat.thermostat.shared.config.InvalidConfigurationException; -import com.redhat.thermostat.utils.keyring.Keyring; public class ClientPreferences { @@ -60,18 +59,16 @@ private static final Logger logger = LoggingUtils.getLogger(ClientPreferences.class); private Properties prefs; - private Keyring keyring; private File userConfig; private String url; private String username; - private char[] password; - public ClientPreferences(Keyring keyring, CommonPaths files) { + public ClientPreferences(CommonPaths files) { userConfig = files.getUserClientConfigurationFile(); Properties props = new Properties(); loadPrefs(props); - init(props, keyring); + init(props); } private void loadPrefs(Properties props) { @@ -91,16 +88,14 @@ } // Testing hook with injectable j.u.Properties - ClientPreferences(Properties properties, Keyring keyring) { - init(properties, keyring); + ClientPreferences(Properties properties) { + init(properties); } - private void init(Properties properties, Keyring keyring) { + private void init(Properties properties) { this.prefs = properties; - this.keyring = keyring; this.url = DEFAULT_CONNECTION_URL; this.username = ""; - this.password = new char[]{}; } public boolean getSaveEntitlements() { @@ -112,14 +107,17 @@ } public String getConnectionUrl() { - return prefs.getProperty(CONNECTION_URL, DEFAULT_CONNECTION_URL); + if (getSaveEntitlements()) { + return prefs.getProperty(CONNECTION_URL, DEFAULT_CONNECTION_URL); + } + return url; } - public char[] getPassword() { + public void setConnectionUrl(String url) { + this.url = url; if (getSaveEntitlements()) { - return keyring.getPassword(getConnectionUrl(), getUserName()); + prefs.put(CONNECTION_URL, url); } - return password; } public String getUserName() { @@ -129,28 +127,16 @@ return username; } - public void setConnectionUrl(String url) { - this.url = url; - if (getSaveEntitlements()) { - prefs.put(CONNECTION_URL, url); - } - } - - public void setCredentials(String userName, char[] password) { + public void setUserName(String userName) { this.username = userName; - this.password = password; if (getSaveEntitlements()) { prefs.put(USERNAME, userName); - keyring.savePassword(getConnectionUrl(), userName, password); } } public void flush() throws IOException { - prefs.store(new FileWriter(userConfig), ""); - if (getSaveEntitlements()) { - keyring.savePassword(getConnectionUrl(), username, password); - } + prefs.store(new FileWriter(userConfig), null); } }
--- a/common/core/src/test/java/com/redhat/thermostat/common/config/ClientPreferencesTest.java Mon Nov 18 11:48:35 2013 -0700 +++ b/common/core/src/test/java/com/redhat/thermostat/common/config/ClientPreferencesTest.java Thu Nov 21 13:00:46 2013 -0700 @@ -39,10 +39,8 @@ import static org.junit.Assert.assertEquals; import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; -import static org.mockito.Matchers.isA; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -50,18 +48,15 @@ import org.junit.Test; -import com.redhat.thermostat.utils.keyring.Keyring; - public class ClientPreferencesTest { @Test public void testGetConnectionUrl() { - - Keyring keyring = mock(Keyring.class); Properties prefs = mock(Properties.class); when(prefs.getProperty(eq(ClientPreferences.CONNECTION_URL), any(String.class))).thenReturn("mock-value"); + when(prefs.getProperty(eq(ClientPreferences.SAVE_ENTITLEMENTS), any(String.class))).thenReturn("true"); - ClientPreferences clientPrefs = new ClientPreferences(prefs, keyring); + ClientPreferences clientPrefs = new ClientPreferences(prefs); String value = clientPrefs.getConnectionUrl(); assertEquals("mock-value", value); @@ -70,40 +65,33 @@ @Test public void testSetConnectionUrl() { - - Keyring keyring = mock(Keyring.class); Properties prefs = mock(Properties.class); when(prefs.getProperty(eq(ClientPreferences.SAVE_ENTITLEMENTS), eq("false"))).thenReturn("true"); - ClientPreferences clientPrefs = new ClientPreferences(prefs, keyring); + ClientPreferences clientPrefs = new ClientPreferences(prefs); clientPrefs.setConnectionUrl("test"); verify(prefs).put(eq(ClientPreferences.CONNECTION_URL), eq("test")); } @Test - public void testSetUsernamePassowrd() { - - Keyring keyring = mock(Keyring.class); + public void testSetUsername() { Properties prefs = mock(Properties.class); when(prefs.getProperty(eq(ClientPreferences.SAVE_ENTITLEMENTS), any(String.class))).thenReturn("true"); - ClientPreferences clientPrefs = new ClientPreferences(prefs, keyring); - clientPrefs.setCredentials("fluff", "fluffPassword".toCharArray()); + ClientPreferences clientPrefs = new ClientPreferences(prefs); + clientPrefs.setUserName("fluff"); verify(prefs).put(eq(ClientPreferences.USERNAME), eq("fluff")); - verify(keyring).savePassword( (String) any(), eq("fluff"), isA(char[].class)); } @Test public void testGetUsername() { - - Keyring keyring = mock(Keyring.class); Properties prefs = mock(Properties.class); when(prefs.getProperty(eq(ClientPreferences.CONNECTION_URL), any(String.class))).thenReturn("mock-url"); when(prefs.getProperty(eq(ClientPreferences.USERNAME), any(String.class))).thenReturn("mock-value"); - ClientPreferences clientPrefs = new ClientPreferences(prefs, keyring); + ClientPreferences clientPrefs = new ClientPreferences(prefs); String username = clientPrefs.getUserName(); assertEquals("", username); @@ -115,33 +103,11 @@ } @Test - public void testGetPassword() { - - Keyring keyring = mock(Keyring.class); - - Properties prefs = mock(Properties.class); - when(prefs.getProperty(eq(ClientPreferences.USERNAME), any(String.class))).thenReturn("mock-value"); - when(prefs.getProperty(eq(ClientPreferences.SAVE_ENTITLEMENTS), any(String.class))). - thenReturn("false").thenReturn("true"); - - ClientPreferences clientPrefs = new ClientPreferences(prefs, keyring); - char[] password = clientPrefs.getPassword(); - verify(keyring, times(0)).getPassword(any(String.class), any(String.class)); - - assertEquals(new String(password), ""); - - clientPrefs.getPassword(); - verify(keyring, atLeastOnce()).getPassword(any(String.class), any(String.class)); - } - - @Test public void testGetSaveEntitlements() { - - Keyring keyring = mock(Keyring.class); Properties prefs = mock(Properties.class); when(prefs.getProperty(eq(ClientPreferences.SAVE_ENTITLEMENTS), any(String.class))).thenReturn("true"); - ClientPreferences clientPrefs = new ClientPreferences(prefs, keyring); + ClientPreferences clientPrefs = new ClientPreferences(prefs); boolean saveEntitlements = clientPrefs.getSaveEntitlements(); assertEquals(true, saveEntitlements); @@ -150,11 +116,9 @@ @Test public void testSetSaveEntitlements() { - - Keyring keyring = mock(Keyring.class); Properties prefs = mock(Properties.class); - ClientPreferences clientPrefs = new ClientPreferences(prefs, keyring); + ClientPreferences clientPrefs = new ClientPreferences(prefs); clientPrefs.setSaveEntitlements(false); verify(prefs).setProperty(eq(ClientPreferences.SAVE_ENTITLEMENTS), eq("false")); @@ -167,10 +131,9 @@ public void testDefaultPreferencesIsMongodb() { // Default preferences for GUI is mongodb:// since this is accomodates // more use cases - Keyring keyring = mock(Keyring.class); Properties prefs = mock(Properties.class); when(prefs.getProperty(eq("connection-url"), any(String.class))).thenReturn(ClientPreferences.DEFAULT_CONNECTION_URL); - ClientPreferences clientPrefs = new ClientPreferences(prefs, keyring); + ClientPreferences clientPrefs = new ClientPreferences(prefs); assertEquals("mongodb://127.0.0.1:27518", clientPrefs.getConnectionUrl()); } }
--- a/integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/IntegrationTest.java Mon Nov 18 11:48:35 2013 -0700 +++ b/integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/IntegrationTest.java Thu Nov 21 13:00:46 2013 -0700 @@ -52,7 +52,6 @@ import expectj.Executor; import expectj.ExpectJ; import expectj.Spawn; -import expectj.TimeoutException; /** * Helper methods to support writing an integration test. @@ -334,11 +333,10 @@ 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 + ":"); + public static void handleAuthPrompt(Spawn spawn, String url, String user, String password) throws IOException { spawn.send(user + "\r"); - spawn.expect("Please enter password for storage at " + url + ":"); spawn.send(password + "\r"); + } private static class LocaleExecutor extends EnvironmentExecutor {
--- a/integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/MongoQueriesTest.java Mon Nov 18 11:48:35 2013 -0700 +++ b/integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/MongoQueriesTest.java Thu Nov 21 13:00:46 2013 -0700 @@ -60,7 +60,6 @@ import com.redhat.thermostat.shared.config.InvalidConfigurationException; import com.redhat.thermostat.shared.config.SSLConfiguration; import com.redhat.thermostat.shared.config.internal.SSLConfigurationImpl; -import com.redhat.thermostat.storage.config.StartupConfiguration; import com.redhat.thermostat.storage.core.Add; import com.redhat.thermostat.storage.core.BackingStorage; import com.redhat.thermostat.storage.core.Connection.ConnectionListener; @@ -68,6 +67,7 @@ import com.redhat.thermostat.storage.core.Cursor; import com.redhat.thermostat.storage.core.Key; import com.redhat.thermostat.storage.core.Query; +import com.redhat.thermostat.storage.core.StorageCredentials; import com.redhat.thermostat.storage.core.Query.SortDirection; import com.redhat.thermostat.storage.core.Storage; import com.redhat.thermostat.storage.mongodb.internal.MongoStorage; @@ -139,11 +139,16 @@ */ private static BackingStorage getAndConnectStorage(ConnectionListener listener) { final String url = "mongodb://127.0.0.1:27518"; - StartupConfiguration config = new StartupConfiguration() { + StorageCredentials creds = new StorageCredentials() { @Override - public String getDBConnectionString() { - return url; + public String getUsername() { + return null; + } + + @Override + public char[] getPassword() { + return null; } }; @@ -270,7 +275,7 @@ } }); - BackingStorage storage = new MongoStorage(config, sslConf); + BackingStorage storage = new MongoStorage(url, creds, sslConf); if (listener != null) { storage.getConnection().addListener(listener); }
--- a/integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/VmCommandsTest.java Mon Nov 18 11:48:35 2013 -0700 +++ b/integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/VmCommandsTest.java Thu Nov 21 13:00:46 2013 -0700 @@ -67,7 +67,6 @@ @Test public void testListVms() throws Exception { Spawn vmList = commandAgainstMongo("list-vms"); - handleAuthPrompt(vmList, "mongodb://127.0.0.1:27518", "", ""); vmList.expectClose(); assertOutputEndsWith(vmList.getCurrentStandardOutContents(), "HOST_ID HOST VM_ID STATUS VM_NAME\n\n"); } @@ -76,7 +75,6 @@ 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()); @@ -127,10 +125,12 @@ @Test public void testNormalCommandAndPluginInShell() throws Exception { + String storageURL = "mongodb://127.0.0.1:27518"; Spawn shell = spawnThermostat("shell"); shell.expect(SHELL_PROMPT); - shell.send("list-vms\n"); + shell.send("list-vms -d " + storageURL + "\n"); + handleAuthPrompt(shell, storageURL, "", ""); shell.expect(SHELL_PROMPT);
--- a/integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/WebAppTest.java Mon Nov 18 11:48:35 2013 -0700 +++ b/integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/WebAppTest.java Thu Nov 21 13:00:46 2013 -0700 @@ -78,8 +78,6 @@ import com.redhat.thermostat.shared.config.SSLConfiguration; import com.redhat.thermostat.shared.config.internal.CommonPathsImpl; import com.redhat.thermostat.shared.config.internal.SSLConfigurationImpl; -import com.redhat.thermostat.storage.config.ConnectionConfiguration; -import com.redhat.thermostat.storage.config.StartupConfiguration; import com.redhat.thermostat.storage.core.Add; import com.redhat.thermostat.storage.core.BackingStorage; import com.redhat.thermostat.storage.core.Category; @@ -95,6 +93,7 @@ import com.redhat.thermostat.storage.core.StatementDescriptor; import com.redhat.thermostat.storage.core.StatementExecutionException; import com.redhat.thermostat.storage.core.Storage; +import com.redhat.thermostat.storage.core.StorageCredentials; import com.redhat.thermostat.storage.core.auth.DescriptorMetadata; import com.redhat.thermostat.storage.dao.HostInfoDAO; import com.redhat.thermostat.storage.model.AggregateCount; @@ -301,8 +300,20 @@ * use mongo-storage directly (which is a BackingStorage). */ private static BackingStorage getAndConnectBackingStorage() { - String url = "mongodb://127.0.0.1:27518"; - StartupConfiguration config = new ConnectionConfiguration(url, "", new char[]{}); + final String url = "mongodb://127.0.0.1:27518"; + StorageCredentials creds = new StorageCredentials() { + + @Override + public String getUsername() { + return null; + } + + @Override + public char[] getPassword() { + return null; + } + + }; SSLConfiguration sslConfig = new SSLConfiguration() { @Override @@ -331,7 +342,7 @@ return false; } }; - BackingStorage storage = new MongoStorage(config, sslConfig); + BackingStorage storage = new MongoStorage(url, creds, sslConfig); storage.getConnection().connect(); return storage; } @@ -352,14 +363,26 @@ * Storage object). Before initiating the connection, add the ConnectionListener * to Storage. */ - private static Storage getAndConnectStorage(String username, String password, + private static Storage getAndConnectStorage(final String username, final String password, String[] roleNames, ConnectionListener listener) throws IOException { setupJAASForUser(roleNames, username, password); - String url = "http://localhost:" + port + "/thermostat/storage"; - StartupConfiguration config = new ConnectionConfiguration(url, username, password.toCharArray()); + final String url = "http://localhost:" + port + "/thermostat/storage"; + StorageCredentials creds = new StorageCredentials() { + + @Override + public String getUsername() { + return username; + } + + @Override + public char[] getPassword() { + return password.toCharArray(); + } + + }; SSLConfiguration sslConf = new SSLConfigurationImpl(new CommonPathsImpl()); - Storage storage = new WebStorage(config, sslConf); + Storage storage = new WebStorage(url, creds, sslConf); if (listener != null) { storage.getConnection().addListener(listener); }
--- a/keyring/src/main/java/com/redhat/thermostat/utils/keyring/impl/KeyringImpl.java Mon Nov 18 11:48:35 2013 -0700 +++ b/keyring/src/main/java/com/redhat/thermostat/utils/keyring/impl/KeyringImpl.java Thu Nov 21 13:00:46 2013 -0700 @@ -59,7 +59,11 @@ boolean success = false; try { String desc = DESC_PREFIX + username + "@" + url; - pwBytes = Charset.defaultCharset().encode(CharBuffer.wrap(password)).array(); + if (password == null) { + pwBytes = new byte[]{}; + } else { + pwBytes = Charset.defaultCharset().encode(CharBuffer.wrap(password)).array(); + } success = gnomeKeyringWrapperSavePasswordNative(url, username, pwBytes, desc); } finally { if (pwBytes != null) {
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/launcher/src/main/java/com/redhat/thermostat/launcher/InteractiveStorageCredentials.java Thu Nov 21 13:00:46 2013 -0700 @@ -0,0 +1,113 @@ +/* + * Copyright 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 + * <http://www.gnu.org/licenses/>. + * + * 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.launcher; + +import java.io.IOException; +import java.util.logging.Logger; + +import com.redhat.thermostat.common.cli.Console; +import com.redhat.thermostat.common.config.ClientPreferences; +import com.redhat.thermostat.common.tools.StorageAuthInfoGetter; +import com.redhat.thermostat.common.utils.LoggingUtils; +import com.redhat.thermostat.launcher.internal.LocaleResources; +import com.redhat.thermostat.shared.locale.Translate; +import com.redhat.thermostat.storage.core.StorageCredentials; +import com.redhat.thermostat.utils.keyring.Keyring; + +public class InteractiveStorageCredentials implements StorageCredentials { + + private static final Translate<LocaleResources> t = LocaleResources.createLocalizer(); + private static final Logger logger = LoggingUtils.getLogger(InteractiveStorageCredentials.class); + + private ClientPreferences prefs; + private Keyring keyring; + private String url; + private StorageAuthInfoGetter getter; + + public InteractiveStorageCredentials(ClientPreferences prefs, Keyring keyring, String url, Console console) { + this.prefs = prefs; + this.keyring = keyring; + this.url = url; + try { + this.getter = new StorageAuthInfoGetter(console); + } catch (IOException e) { + String message = "IOException while creating interactive authentication credential getter."; + logger.severe(message); + throw new InteractiveException(message, e); + } + } + + @Override + public String getUsername() { + String username = null; + if (url.equals(prefs.getConnectionUrl())) { + prefs.getUserName(); + } + if (username == null) { + try { + username = getter.getUserName(url); + } catch (IOException e) { + throw new InteractiveException(t.localize(LocaleResources.LAUNCHER_USER_AUTH_PROMPT_ERROR).getContents(), e); + } + } + return username.length() == 0 ? null : username; + } + + @Override + public char[] getPassword() { + char[] password = null; + if (url.equals(prefs.getConnectionUrl())) { + keyring.getPassword(url, prefs.getUserName()); + } + if (password == null) { + try { + password = getter.getPassword(url); + } catch (IOException e) { + throw new InteractiveException(t.localize(LocaleResources.LAUNCHER_USER_AUTH_PROMPT_ERROR).getContents(), e); + } + } + return password.length == 0 ? null : password; + } + + class InteractiveException extends RuntimeException { + + private static final long serialVersionUID = -7921653973987512258L; + + InteractiveException(String message, Exception cause) { + super(message, cause); + } + } +}
--- a/launcher/src/main/java/com/redhat/thermostat/launcher/internal/Activator.java Mon Nov 18 11:48:35 2013 -0700 +++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/Activator.java Thu Nov 21 13:00:46 2013 -0700 @@ -80,7 +80,7 @@ public void dependenciesAvailable(Map<String, Object> services) { CommonPaths paths = (CommonPaths) services.get(CommonPaths.class.getName()); Keyring keyring = (Keyring) services.get(Keyring.class.getName()); - ClientPreferences prefs = new ClientPreferences(keyring, paths); + ClientPreferences prefs = new ClientPreferences(paths); String commandsDir = new File(paths.getSystemConfigurationDirectory(), "commands").toString(); CommandInfoSource builtInCommandSource = @@ -101,7 +101,7 @@ // Register Launcher service since FrameworkProvider is waiting for it blockingly. LauncherImpl launcher = new LauncherImpl(context, new CommandContextFactory(context), - bundleService, commands, env, prefs, paths); + bundleService, commands, env, prefs, keyring, paths); launcherReg = context.registerService(Launcher.class.getName(), launcher, null); bundleManReg = context.registerService(BundleManager.class, bundleService, null); ExitStatus exitStatus = new ExitStatusImpl(ExitStatus.EXIT_SUCCESS);
--- a/launcher/src/main/java/com/redhat/thermostat/launcher/internal/LauncherImpl.java Mon Nov 18 11:48:35 2013 -0700 +++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/LauncherImpl.java Thu Nov 21 13:00:46 2013 -0700 @@ -48,6 +48,7 @@ import org.osgi.framework.BundleContext; import org.osgi.framework.BundleException; import org.osgi.framework.ServiceReference; +import org.osgi.framework.ServiceRegistration; import com.redhat.thermostat.common.ActionListener; import com.redhat.thermostat.common.ActionNotifier; @@ -61,12 +62,11 @@ import com.redhat.thermostat.common.cli.CommandContextFactory; import com.redhat.thermostat.common.cli.CommandException; import com.redhat.thermostat.common.cli.CommandLineArgumentParseException; -import com.redhat.thermostat.common.cli.Console; import com.redhat.thermostat.common.config.ClientPreferences; import com.redhat.thermostat.common.tools.ApplicationState; -import com.redhat.thermostat.common.tools.StorageAuthInfoGetter; import com.redhat.thermostat.common.utils.LoggingUtils; import com.redhat.thermostat.launcher.BundleManager; +import com.redhat.thermostat.launcher.InteractiveStorageCredentials; import com.redhat.thermostat.launcher.Launcher; import com.redhat.thermostat.shared.config.CommonPaths; import com.redhat.thermostat.shared.config.InvalidConfigurationException; @@ -76,7 +76,9 @@ import com.redhat.thermostat.storage.core.DbService; import com.redhat.thermostat.storage.core.DbServiceFactory; import com.redhat.thermostat.storage.core.Storage; +import com.redhat.thermostat.storage.core.StorageCredentials; import com.redhat.thermostat.storage.core.StorageException; +import com.redhat.thermostat.utils.keyring.Keyring; /** * This class is thread-safe. @@ -99,19 +101,18 @@ private final CurrentEnvironment currentEnvironment; private final ClientPreferences prefs; - - + private final Keyring keyring; public LauncherImpl(BundleContext context, CommandContextFactory cmdCtxFactory, BundleManager registry, - CommandInfoSource infoSource, CurrentEnvironment env, ClientPreferences prefs, CommonPaths paths) { + CommandInfoSource infoSource, CurrentEnvironment env, ClientPreferences prefs, Keyring keyring, CommonPaths paths) { this(context, cmdCtxFactory, registry, infoSource, new CommandSource(context), env, - new DbServiceFactory(), new Version(), prefs, paths, new LoggingInitializer(paths)); + new DbServiceFactory(), new Version(), prefs, keyring, paths, new LoggingInitializer(paths)); } LauncherImpl(BundleContext context, CommandContextFactory cmdCtxFactory, BundleManager registry, CommandInfoSource commandInfoSource, CommandSource commandSource, CurrentEnvironment currentEnvironment, DbServiceFactory dbServiceFactory, Version version, - ClientPreferences prefs, CommonPaths paths, LoggingInitializer loggingInitializer) { + ClientPreferences prefs, Keyring keyring, CommonPaths paths, LoggingInitializer loggingInitializer) { this.context = context; this.cmdCtxFactory = cmdCtxFactory; this.registry = registry; @@ -121,6 +122,7 @@ this.commandInfoSource = commandInfoSource; this.currentEnvironment = currentEnvironment; this.prefs = prefs; + this.keyring = keyring; loggingInitializer.initialize(); logger = LoggingUtils.getLogger(LauncherImpl.class); @@ -338,21 +340,11 @@ if (dbUrl == null) { dbUrl = prefs.getConnectionUrl(); } - String username = prefs.getUserName(); - char[] password = prefs.getPassword(); - if (username == null || password == null) { - Console console = ctx.getConsole(); - try { - StorageAuthInfoGetter getUserPass = new StorageAuthInfoGetter(console); - username = getUserPass.getUserName(dbUrl); - password = getUserPass.getPassword(dbUrl); - } catch (IOException ex) { - throw new CommandException(t.localize(LocaleResources.LAUNCHER_USER_AUTH_PROMPT_ERROR), ex); - } - } + StorageCredentials creds = new InteractiveStorageCredentials(prefs, keyring, dbUrl, ctx.getConsole()); + ServiceRegistration credsReg = context.registerService(StorageCredentials.class, creds, null); try { // this may throw storage exception - DbService service = dbServiceFactory.createDbService(username, password, dbUrl); + DbService service = dbServiceFactory.createDbService(dbUrl); // This registers the DbService if all goes well service.connect(); } catch (StorageException ex) { @@ -363,9 +355,7 @@ logger.log(Level.SEVERE, "Could not connect to: " + dbUrl + message, ex); throw new CommandException(t.localize(LocaleResources.LAUNCHER_CONNECTION_ERROR, dbUrl), ex); } finally { - if (password != null) { - Arrays.fill(password, '\0'); - } + credsReg.unregister(); } } }
--- a/launcher/src/test/java/com/redhat/thermostat/launcher/internal/LauncherImplTest.java Mon Nov 18 11:48:35 2013 -0700 +++ b/launcher/src/test/java/com/redhat/thermostat/launcher/internal/LauncherImplTest.java Thu Nov 21 13:00:46 2013 -0700 @@ -40,8 +40,6 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.Matchers.anyString; -import static org.mockito.Matchers.isA; -import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -269,10 +267,10 @@ File userConfigFile = mock(File.class); when(userConfigFile.isFile()).thenReturn(false); when(paths.getUserClientConfigurationFile()).thenReturn(userConfigFile); - ClientPreferences prefs = new ClientPreferences(keyring, paths); + ClientPreferences prefs = new ClientPreferences(paths); launcher = new LauncherImpl(bundleContext, ctxFactory, registry, infos, new CommandSource(bundleContext), - environment, dbServiceFactory, version, prefs, paths, loggingInitializer); + environment, dbServiceFactory, version, prefs, keyring, paths, loggingInitializer); } private void setupCommandContextFactory() { @@ -473,17 +471,17 @@ @Test public void verifyPrefsAreUsed() { ClientPreferences prefs = mock(ClientPreferences.class); + Keyring keyring = mock(Keyring.class); String dbUrl = "mongo://fluff:12345"; when(prefs.getConnectionUrl()).thenReturn(dbUrl); when(prefs.getUserName()).thenReturn("user"); - when(prefs.getPassword()).thenReturn("password".toCharArray()); LauncherImpl launcher = new LauncherImpl(bundleContext, ctxFactory, registry, infos, new CommandSource(bundleContext), - environment, dbServiceFactory, version, prefs, paths, loggingInitializer); + environment, dbServiceFactory, version, prefs, keyring, paths, loggingInitializer); DbService dbService = mock(DbService.class); ArgumentCaptor<String> dbUrlCaptor = ArgumentCaptor.forClass(String.class); - when(dbServiceFactory.createDbService(eq("user"), eq("password".toCharArray()), dbUrlCaptor.capture())).thenReturn(dbService); + when(dbServiceFactory.createDbService(dbUrlCaptor.capture())).thenReturn(dbService); wrappedRun(launcher, new String[] { "test3" }, false); verify(dbService).connect(); verify(prefs).getConnectionUrl(); @@ -493,14 +491,15 @@ @Test public void verifyUserInputUsedIfNoSavedAuthInfo() { ClientPreferences prefs = mock(ClientPreferences.class); + Keyring keyring = mock(Keyring.class); String dbUrl = "mongo://fluff:12345"; when(prefs.getConnectionUrl()).thenReturn(dbUrl); LauncherImpl launcher = new LauncherImpl(bundleContext, ctxFactory, registry, infos, new CommandSource(bundleContext), - environment, dbServiceFactory, version, prefs, paths, loggingInitializer); + environment, dbServiceFactory, version, prefs, keyring, paths, loggingInitializer); DbService dbService = mock(DbService.class); ArgumentCaptor<String> dbUrlCaptor = ArgumentCaptor.forClass(String.class); - when(dbServiceFactory.createDbService(eq("user"), eq("pass".toCharArray()), dbUrlCaptor.capture())).thenReturn(dbService); + when(dbServiceFactory.createDbService(dbUrlCaptor.capture())).thenReturn(dbService); ctxFactory.setInput("user\rpass\r"); wrappedRun(launcher, new String[] { "test3" }, false); verify(dbService).connect(); @@ -516,9 +515,10 @@ char[] password = new char[] {'1', '2', '3', '4', '5'}; when(prefs.getConnectionUrl()).thenReturn(dbUrl); when(prefs.getUserName()).thenReturn(user); - when(prefs.getPassword()).thenReturn(password); + Keyring keyring = mock(Keyring.class); + when(keyring.getPassword(dbUrl, user)).thenReturn(password); LauncherImpl launcher = new LauncherImpl(bundleContext, ctxFactory, registry, infos, new CommandSource(bundleContext), - environment, dbServiceFactory, version, prefs, paths, loggingInitializer); + environment, dbServiceFactory, version, prefs, keyring, paths, loggingInitializer); Command mockCmd = mock(Command.class); when(mockCmd.isStorageRequired()).thenReturn(true); @@ -532,7 +532,7 @@ when(infos.getCommandInfo("dummy")).thenReturn(cmdInfo); DbService dbService = mock(DbService.class); - when(dbServiceFactory.createDbService(anyString(), isA(char[].class), anyString())).thenReturn(dbService); + when(dbServiceFactory.createDbService(anyString())).thenReturn(dbService); wrappedRun(launcher, new String[] { "dummy" }, false); verify(dbService).connect();
--- a/storage/core/pom.xml Mon Nov 18 11:48:35 2013 -0700 +++ b/storage/core/pom.xml Thu Nov 21 13:00:46 2013 -0700 @@ -63,6 +63,7 @@ <Bundle-Vendor>Red Hat, Inc.</Bundle-Vendor> <Bundle-Activator>com.redhat.thermostat.storage.internal.Activator</Bundle-Activator> <Export-Package> + com.redhat.thermostat.storage.connect, com.redhat.thermostat.storage.core, com.redhat.thermostat.storage.core.auth, com.redhat.thermostat.storage.config, @@ -81,6 +82,7 @@ same way. --> <DynamicImport-Package>com.redhat.thermostat.*</DynamicImport-Package> <Private-Package> + com.redhat.thermostat.storage.connect.internal, com.redhat.thermostat.storage.internal.dao, com.redhat.thermostat.storage.internal, com.redhat.thermostat.storage.internal.test,
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/config/ConnectionConfiguration.java Mon Nov 18 11:48:35 2013 -0700 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,66 +0,0 @@ -/* - * 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 - * <http://www.gnu.org/licenses/>. - * - * 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.storage.config; - -public class ConnectionConfiguration implements StartupConfiguration, AuthenticationConfiguration { - - private String dbUrl; - private String username; - private char[] password; - - public ConnectionConfiguration(String dbUrl, String username, char[] password) { - this.dbUrl = dbUrl; - this.username = username; - this.password = password; - } - - @Override - public String getDBConnectionString() { - return dbUrl; - } - - @Override - public String getUsername() { - return username; - } - - @Override - public char[] getPassword() { - return password; - } -} -
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/core/DbServiceFactory.java Mon Nov 18 11:48:35 2013 -0700 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/core/DbServiceFactory.java Thu Nov 21 13:00:46 2013 -0700 @@ -61,10 +61,6 @@ * a service, users are encouraged to use that instance for disconnecting * from storage. * - * @param username - * The username to use for the connection to storage. - * @param password - * The password to use for the connection to storage. * @param dbUrl * The URL to the storage endpoint. For example * {@code mongodb://127.0.0.1:27518} or @@ -75,9 +71,8 @@ * If no matching {@link StorageProvider} could be found for the * given dbUrl. */ - public DbService createDbService(String username, char[] password, - String dbUrl) throws StorageException { - return DbServiceImpl.create(username, password, dbUrl); + public DbService createDbService(String dbUrl) throws StorageException { + return DbServiceImpl.create(dbUrl); } }
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/core/StorageCredentials.java Thu Nov 21 13:00:46 2013 -0700 @@ -0,0 +1,44 @@ +/* + * 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 + * <http://www.gnu.org/licenses/>. + * + * 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.storage.core; + +public interface StorageCredentials { + + public String getUsername(); + + public char[] getPassword(); + +}
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/core/StorageProvider.java Mon Nov 18 11:48:35 2013 -0700 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/core/StorageProvider.java Thu Nov 21 13:00:46 2013 -0700 @@ -59,7 +59,7 @@ * @param config * @param sslConf */ - void setConfig(StartupConfiguration config, SSLConfiguration sslConf); + void setConfig(String url, StorageCredentials creds, SSLConfiguration sslConf); /** * Method which determines if this StorageProvider can handle the given
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/internal/DbServiceImpl.java Mon Nov 18 11:48:35 2013 -0700 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/internal/DbServiceImpl.java Thu Nov 21 13:00:46 2013 -0700 @@ -48,13 +48,12 @@ import com.redhat.thermostat.common.utils.LoggingUtils; import com.redhat.thermostat.shared.config.SSLConfiguration; -import com.redhat.thermostat.storage.config.ConnectionConfiguration; -import com.redhat.thermostat.storage.config.StartupConfiguration; import com.redhat.thermostat.storage.core.Connection.ConnectionListener; import com.redhat.thermostat.storage.core.Connection.ConnectionStatus; import com.redhat.thermostat.storage.core.ConnectionException; import com.redhat.thermostat.storage.core.DbService; import com.redhat.thermostat.storage.core.Storage; +import com.redhat.thermostat.storage.core.StorageCredentials; import com.redhat.thermostat.storage.core.StorageException; import com.redhat.thermostat.storage.core.StorageProvider; @@ -69,16 +68,17 @@ private Storage storage; private BundleContext context; private String dbUrl; - private static ServiceReference sslConfRef; + @SuppressWarnings("rawtypes") + private static ServiceReference sslConfRef, storageCredsRef; - DbServiceImpl(String username, char[] password, String dbUrl) throws StorageException { + DbServiceImpl(String dbUrl) throws StorageException { BundleContext context = FrameworkUtil.getBundle(DbService.class).getBundleContext(); - init(context, username, password, dbUrl); + init(context, dbUrl); } // for testing - DbServiceImpl(BundleContext context, String username, char[] password, String dbUrl) { - init(context, username, password, dbUrl); + DbServiceImpl(BundleContext context, String dbUrl) { + init(context, dbUrl); } // For testing. Injects custom storage. @@ -87,8 +87,8 @@ this.storage = storage; } - private void init(BundleContext context, String username, char[] password, String dbUrl) { - Storage storage = createStorage(context, username, password, dbUrl); + private void init(BundleContext context, String dbUrl) { + Storage storage = createStorage(context, dbUrl); this.storage = storage; this.context = context; @@ -105,17 +105,19 @@ // during connection handling. doSynchronousConnect(); } catch (Exception cause) { + throw new ConnectionException(cause); + } finally { if (sslConfRef != null) { context.ungetService(sslConfRef); + sslConfRef = null; } - throw new ConnectionException(cause); + if (storageCredsRef != null) { + context.ungetService(storageCredsRef); + storageCredsRef = null; + } } // Connection didn't throw an exception. Now it is safe to register - // services for general consumption and unregister services used - // only while creating and connecting Storage. - if (sslConfRef != null) { - context.ungetService(sslConfRef); - } + // services for general consumption. dbServiceReg = context.registerService(DbService.class, this, null); storageReg = context.registerService(Storage.class.getName(), this.storage, null); } @@ -186,8 +188,8 @@ * @return a DbService instance * @throws StorageException if no storage provider exists for the given {@code dbUrl}. */ - public static DbService create(String username, char[] password, String dbUrl) throws StorageException { - return new DbServiceImpl(username, password, dbUrl); + public static DbService create(String dbUrl) throws StorageException { + return new DbServiceImpl(dbUrl); } @SuppressWarnings("rawtypes") @@ -216,9 +218,8 @@ } } - private static Storage createStorage(BundleContext context, String username, char[] password, String dbUrl) throws StorageException { - StartupConfiguration config = new ConnectionConfiguration(dbUrl, username, password); - StorageProvider prov = getStorageProvider(context, config); + private static Storage createStorage(BundleContext context, String dbUrl) throws StorageException { + StorageProvider prov = getStorageProvider(context, dbUrl); if (prov == null) { // no suitable provider found throw new StorageException("No storage found for URL " + dbUrl); @@ -227,12 +228,17 @@ } @SuppressWarnings({ "rawtypes", "unchecked" }) - private static StorageProvider getStorageProvider(BundleContext context, StartupConfiguration config) { + private static StorageProvider getStorageProvider(BundleContext context, String url) { try { ServiceReference[] refs = context.getServiceReferences(StorageProvider.class.getName(), null); if (refs == null) { throw new StorageException("No storage provider available"); } + storageCredsRef = context.getServiceReference(StorageCredentials.class.getName()); + if (storageCredsRef == null) { + throw new StorageException("No StorageCredentials available"); + } + StorageCredentials creds = (StorageCredentials) context.getService(storageCredsRef); sslConfRef = context.getServiceReference(SSLConfiguration.class.getName()); if (sslConfRef == null) { throw new StorageException("No SSL configuration available"); @@ -240,7 +246,7 @@ SSLConfiguration sslConf = (SSLConfiguration) context.getService(sslConfRef); for (int i = 0; i < refs.length; i++) { StorageProvider prov = (StorageProvider) context.getService(refs[i]); - prov.setConfig(config, sslConf); + prov.setConfig(url, creds, sslConf); if (prov.canHandleProtocol()) { return prov; }
--- a/storage/core/src/test/java/com/redhat/thermostat/storage/internal/DbServiceImplTest.java Mon Nov 18 11:48:35 2013 -0700 +++ b/storage/core/src/test/java/com/redhat/thermostat/storage/internal/DbServiceImplTest.java Thu Nov 21 13:00:46 2013 -0700 @@ -58,6 +58,7 @@ import com.redhat.thermostat.storage.core.ConnectionException; import com.redhat.thermostat.storage.core.DbService; import com.redhat.thermostat.storage.core.Storage; +import com.redhat.thermostat.storage.core.StorageCredentials; import com.redhat.thermostat.storage.core.StorageException; import com.redhat.thermostat.storage.core.StorageProvider; import com.redhat.thermostat.testutils.StubBundleContext; @@ -84,6 +85,9 @@ when(storageProvider.canHandleProtocol()).thenReturn(true); when(storageProvider.createStorage()).thenReturn(storage); context.registerService(StorageProvider.class, storageProvider, null); + + StorageCredentials creds = mock(StorageCredentials.class); + context.registerService(StorageCredentials.class, creds, null); } @Test @@ -91,7 +95,7 @@ context = new StubBundleContext(); try { - new DbServiceImpl(context, "ignore", "ignore".toCharArray(), "http://ignored.example.com"); + new DbServiceImpl(context, "http://ignored.example.com"); fail("exception expected"); } catch (StorageException se) { assertEquals("No storage provider available", se.getMessage()); @@ -103,7 +107,7 @@ when(storageProvider.canHandleProtocol()).thenReturn(false); try { - new DbServiceImpl(context, "ignore", "ignore".toCharArray(), "http://ignored.example.com"); + new DbServiceImpl(context, "http://ignored.example.com"); fail("exception expected"); } catch (StorageException se) { assertEquals("No storage found for URL http://ignored.example.com", se.getMessage()); @@ -181,7 +185,7 @@ @Test public void testConnectRegistersDbService() { - DbService dbService = new DbServiceImpl(context, "ignore", "ignore".toCharArray(), "http://ignored.example.com"); + DbService dbService = new DbServiceImpl(context, "http://ignored.example.com"); try { dbService.connect(); @@ -200,7 +204,7 @@ @Test public void testConnectRegistersStorage() { - DbService dbService = new DbServiceImpl(context, "ignore", "ignore".toCharArray(), "http://ignored.example.com"); + DbService dbService = new DbServiceImpl(context, "http://ignored.example.com"); try { dbService.connect(); @@ -220,7 +224,7 @@ @SuppressWarnings("rawtypes") @Test public void testConnectEnforcesPreCond() { - DbService dbService = new DbServiceImpl(context, "ignore", "ignore".toCharArray(), "http://ignored.example.com"); + DbService dbService = new DbServiceImpl(context, "http://ignored.example.com"); ServiceRegistration reg = context.registerService(DbService.class, dbService, null); try { @@ -243,7 +247,7 @@ @SuppressWarnings("rawtypes") @Test public void testDisConnectEnforcesPreCond() { - DbService dbService = new DbServiceImpl(context, "ignore", "ignore".toCharArray(), "http://ignored.example.com"); + DbService dbService = new DbServiceImpl(context, "http://ignored.example.com"); ServiceRegistration reg = context.registerService(DbService.class, dbService, null); try { @@ -267,7 +271,7 @@ @Test public void testDisconnect() { - DbService dbService = new DbServiceImpl(context, "ignore", "ignore".toCharArray(), "http://ignored.example.com"); + DbService dbService = new DbServiceImpl(context, "http://ignored.example.com"); try { dbService.connect(); @@ -285,7 +289,7 @@ @Test public void testDisconnectUnregistersDbService() { - DbService dbService = new DbServiceImpl(context, "ignore", "ignore".toCharArray(), "http://ignored.example.com"); + DbService dbService = new DbServiceImpl(context, "http://ignored.example.com"); try { dbService.connect(); @@ -305,7 +309,7 @@ @Test public void testDisconnectUnregistersStorage() { - DbService dbService = new DbServiceImpl(context, "ignore", "ignore".toCharArray(), "http://ignored.example.com"); + DbService dbService = new DbServiceImpl(context, "http://ignored.example.com"); try { dbService.connect(); @@ -328,7 +332,7 @@ public void canGetStorageUrl() { String connectionURL = "http://test.example.com:8082"; - DbService dbService = new DbServiceImpl(context, "ignore", "ignore".toCharArray(), connectionURL); + DbService dbService = new DbServiceImpl(context, connectionURL); assertEquals(connectionURL, dbService.getConnectionUrl()); } @@ -337,7 +341,7 @@ ConnectionListener listener = mock(ConnectionListener.class); Connection connection = mock(Connection.class); when(storage.getConnection()).thenReturn(connection); - DbService dbService = new DbServiceImpl(context, "ignore", "ignore".toCharArray(), "http://ignored.example.com"); + DbService dbService = new DbServiceImpl(context, "http://ignored.example.com"); dbService.addConnectionListener(listener); verify(connection).addListener(listener); @@ -346,7 +350,7 @@ @Test public void testListenerGetsEvent() { ConnectingConnectionListener listener = new ConnectingConnectionListener(); - DbService dbService = new DbServiceImpl(context, "ignore", "ignore".toCharArray(), "http://ignored.example.com"); + DbService dbService = new DbServiceImpl(context, "http://ignored.example.com"); ConnectingConnection connection = new ConnectingConnection(); when(storage.getConnection()).thenReturn(connection); @@ -376,7 +380,7 @@ ConnectionListener listener = mock(ConnectionListener.class); Connection connection = mock(Connection.class); when(storage.getConnection()).thenReturn(connection); - DbService dbService = new DbServiceImpl(context, "ignore", "ignore".toCharArray(), "http://ignored.example.com"); + DbService dbService = new DbServiceImpl(context, "http://ignored.example.com"); dbService.removeConnectionListener(listener); verify(connection).removeListener(listener); }
--- a/storage/mongo/src/main/java/com/redhat/thermostat/storage/mongodb/MongoStorageProvider.java Mon Nov 18 11:48:35 2013 -0700 +++ b/storage/mongo/src/main/java/com/redhat/thermostat/storage/mongodb/MongoStorageProvider.java Thu Nov 21 13:00:46 2013 -0700 @@ -37,31 +37,33 @@ package com.redhat.thermostat.storage.mongodb; import com.redhat.thermostat.shared.config.SSLConfiguration; -import com.redhat.thermostat.storage.config.StartupConfiguration; import com.redhat.thermostat.storage.core.QueuedStorage; import com.redhat.thermostat.storage.core.Storage; +import com.redhat.thermostat.storage.core.StorageCredentials; import com.redhat.thermostat.storage.core.StorageProvider; import com.redhat.thermostat.storage.mongodb.internal.MongoStorage; public class MongoStorageProvider implements StorageProvider { - private StartupConfiguration configuration; + private String url; + private StorageCredentials creds; private SSLConfiguration sslConf; - public void setConfig(StartupConfiguration configuration, SSLConfiguration sslConf) { - this.configuration = configuration; + public void setConfig(String url, StorageCredentials creds, SSLConfiguration sslConf) { + this.url = url; + this.creds = creds; this.sslConf = sslConf; } @Override public Storage createStorage() { - MongoStorage storage = new MongoStorage(configuration, sslConf); + MongoStorage storage = new MongoStorage(url, creds, sslConf); return new QueuedStorage(storage); } @Override public boolean canHandleProtocol() { - return configuration.getDBConnectionString().startsWith("mongodb://"); + return url.startsWith("mongodb://"); } }
--- a/storage/mongo/src/main/java/com/redhat/thermostat/storage/mongodb/internal/MongoConnection.java Mon Nov 18 11:48:35 2013 -0700 +++ b/storage/mongo/src/main/java/com/redhat/thermostat/storage/mongodb/internal/MongoConnection.java Thu Nov 21 13:00:46 2013 -0700 @@ -38,6 +38,7 @@ import java.io.IOException; import java.net.UnknownHostException; +import java.util.Arrays; import java.util.logging.Level; import java.util.logging.Logger; @@ -57,10 +58,9 @@ import com.redhat.thermostat.common.utils.LoggingUtils; import com.redhat.thermostat.shared.config.InvalidConfigurationException; import com.redhat.thermostat.shared.config.SSLConfiguration; -import com.redhat.thermostat.storage.config.AuthenticationConfiguration; -import com.redhat.thermostat.storage.config.StartupConfiguration; import com.redhat.thermostat.storage.core.Connection; import com.redhat.thermostat.storage.core.ConnectionException; +import com.redhat.thermostat.storage.core.StorageCredentials; class MongoConnection extends Connection { @@ -69,11 +69,13 @@ private Mongo m = null; private DB db = null; - private StartupConfiguration conf; + private String url; + StorageCredentials creds; SSLConfiguration sslConf; - MongoConnection(StartupConfiguration conf, SSLConfiguration sslConf) { - this.conf = conf; + MongoConnection(String url, StorageCredentials creds, SSLConfiguration sslConf) { + this.url = url; + this.creds = creds; this.sslConf = sslConf; } @@ -95,11 +97,15 @@ } private void authenticateIfNecessary() { - if (conf instanceof AuthenticationConfiguration) { - AuthenticationConfiguration authConf = (AuthenticationConfiguration) conf; - String username = authConf.getUsername(); - if (username != null && ! username.equals("")) { - authenticate(username, authConf.getPassword()); + String username = creds.getUsername(); + char[] password = creds.getPassword(); + try { + if (username != null && password != null) { + authenticate(username, password); + } + } finally { + if (password != null) { + Arrays.fill(password, '\0'); } } } @@ -158,7 +164,6 @@ } ServerAddress getServerAddress() throws InvalidConfigurationException, UnknownHostException { - String url = conf.getDBConnectionString(); // Strip mongodb prefix: "mongodb://".length() == 10 String hostPort = url.substring(10); HostPortsParser parser = new HostPortsParser(hostPort);
--- a/storage/mongo/src/main/java/com/redhat/thermostat/storage/mongodb/internal/MongoStorage.java Mon Nov 18 11:48:35 2013 -0700 +++ b/storage/mongo/src/main/java/com/redhat/thermostat/storage/mongodb/internal/MongoStorage.java Thu Nov 21 13:00:46 2013 -0700 @@ -55,7 +55,6 @@ import com.mongodb.gridfs.GridFSDBFile; import com.mongodb.gridfs.GridFSInputFile; import com.redhat.thermostat.shared.config.SSLConfiguration; -import com.redhat.thermostat.storage.config.StartupConfiguration; import com.redhat.thermostat.storage.core.AbstractQuery.Sort; import com.redhat.thermostat.storage.core.Add; import com.redhat.thermostat.storage.core.AggregateQuery; @@ -75,6 +74,7 @@ import com.redhat.thermostat.storage.core.Replace; import com.redhat.thermostat.storage.core.Statement; import com.redhat.thermostat.storage.core.StatementDescriptor; +import com.redhat.thermostat.storage.core.StorageCredentials; import com.redhat.thermostat.storage.core.StorageException; import com.redhat.thermostat.storage.core.Update; import com.redhat.thermostat.storage.model.AggregateCount; @@ -288,8 +288,8 @@ this.conn = null; } - public MongoStorage(StartupConfiguration conf, SSLConfiguration sslConf) { - conn = new MongoConnection(conf, sslConf); + public MongoStorage(String url, StorageCredentials creds, SSLConfiguration sslConf) { + conn = new MongoConnection(url, creds, sslConf); connectedLatch = new CountDownLatch(1); conn.addListener(new ConnectionListener() { @Override
--- a/storage/mongo/src/test/java/com/redhat/thermostat/storage/mongodb/internal/MongoConnectionTest.java Mon Nov 18 11:48:35 2013 -0700 +++ b/storage/mongo/src/test/java/com/redhat/thermostat/storage/mongodb/internal/MongoConnectionTest.java Thu Nov 21 13:00:46 2013 -0700 @@ -80,10 +80,10 @@ import com.redhat.thermostat.common.utils.HostPortsParser; import com.redhat.thermostat.shared.config.InvalidConfigurationException; import com.redhat.thermostat.shared.config.SSLConfiguration; -import com.redhat.thermostat.storage.config.StartupConfiguration; import com.redhat.thermostat.storage.core.Connection.ConnectionListener; import com.redhat.thermostat.storage.core.Connection.ConnectionStatus; import com.redhat.thermostat.storage.core.ConnectionException; +import com.redhat.thermostat.storage.core.StorageCredentials; @RunWith(PowerMockRunner.class) // There is a bug (resolved as wontfix) in powermock which results in @@ -100,12 +100,11 @@ @Before public void setUp() { - StartupConfiguration conf = mock(StartupConfiguration.class); mockSSLConf = mock(SSLConfiguration.class); when(mockSSLConf.enableForBackingStorage()).thenReturn(false); - when(conf.getDBConnectionString()).thenReturn("mongodb://127.0.0.1:27518"); + StorageCredentials creds = mock(StorageCredentials.class); - conn = new MongoConnection(conf, mockSSLConf); + conn = new MongoConnection("mongodb://127.0.0.1:27518", creds, mockSSLConf); listener = mock(ConnectionListener.class); conn.addListener(listener); } @@ -246,21 +245,16 @@ doCallRealMethod().when(connection).connect(); doCallRealMethod().when(connection).createConnection(); connection.sslConf = mock(SSLConfiguration.class); + connection.creds = mock(StorageCredentials.class); connection.connect(); verify(connection, Mockito.times(0)).getSSLMongo(); } @Test public void canGetServerAddress() { - StartupConfiguration config = new StartupConfiguration() { - - @Override - public String getDBConnectionString() { - return "mongodb://127.0.1.1:23452"; - } - }; SSLConfiguration sslConf = mock(SSLConfiguration.class); - MongoConnection connection = new MongoConnection(config, sslConf); + StorageCredentials creds = mock(StorageCredentials.class); + MongoConnection connection = new MongoConnection("mongodb://127.0.1.1:23452", creds, sslConf); ServerAddress addr = null; try { addr = connection.getServerAddress(); @@ -269,15 +263,8 @@ } assertEquals(23452, addr.getPort()); assertEquals("127.0.1.1", addr.getHost()); - - config = new StartupConfiguration() { - - @Override - public String getDBConnectionString() { - return "fluff://willnotwork.com:23452"; - } - }; - connection = new MongoConnection(config, sslConf); + + connection = new MongoConnection("fluff://willnotwork.com:23452", creds, sslConf); try { connection.getServerAddress(); fail("should not have been able to parse address");
--- a/storage/mongo/src/test/java/com/redhat/thermostat/storage/mongodb/internal/MongoStorageProviderTest.java Mon Nov 18 11:48:35 2013 -0700 +++ b/storage/mongo/src/test/java/com/redhat/thermostat/storage/mongodb/internal/MongoStorageProviderTest.java Thu Nov 21 13:00:46 2013 -0700 @@ -38,12 +38,9 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; import org.junit.Test; -import com.redhat.thermostat.storage.config.StartupConfiguration; import com.redhat.thermostat.storage.core.QueuedStorage; import com.redhat.thermostat.storage.core.SecureStorage; import com.redhat.thermostat.storage.core.Storage; @@ -53,12 +50,24 @@ @Test public void createStorageReturnsQueuedStorage() { - StartupConfiguration config = mock(StartupConfiguration.class); - when(config.getDBConnectionString()).thenReturn("mongodb://something.com"); MongoStorageProvider provider = new MongoStorageProvider(); - provider.setConfig(config, null); + provider.setConfig("mongodb://something.com", null, null); Storage result = provider.createStorage(); assertTrue(result instanceof QueuedStorage); assertFalse(result instanceof SecureStorage); } + + @Test + public void canHandleMongoProtocol() { + MongoStorageProvider provider = new MongoStorageProvider(); + provider.setConfig("mongodb://something.com", null, null); + assertTrue(provider.canHandleProtocol()); + } + + @Test + public void cannotHandleNotMongoProtocol() { + MongoStorageProvider provider = new MongoStorageProvider(); + provider.setConfig("http://something.com", null, null); + assertFalse(provider.canHandleProtocol()); + } }
--- a/storage/mongo/src/test/java/com/redhat/thermostat/storage/mongodb/internal/MongoStorageTest.java Mon Nov 18 11:48:35 2013 -0700 +++ b/storage/mongo/src/test/java/com/redhat/thermostat/storage/mongodb/internal/MongoStorageTest.java Thu Nov 21 13:00:46 2013 -0700 @@ -83,7 +83,6 @@ import com.mongodb.gridfs.GridFSDBFile; import com.mongodb.gridfs.GridFSInputFile; import com.redhat.thermostat.shared.config.SSLConfiguration; -import com.redhat.thermostat.storage.config.StartupConfiguration; import com.redhat.thermostat.storage.core.Add; import com.redhat.thermostat.storage.core.BackingStorage; import com.redhat.thermostat.storage.core.Category; @@ -94,6 +93,7 @@ import com.redhat.thermostat.storage.core.Persist; import com.redhat.thermostat.storage.core.Query; import com.redhat.thermostat.storage.core.Replace; +import com.redhat.thermostat.storage.core.StorageCredentials; import com.redhat.thermostat.storage.core.Update; import com.redhat.thermostat.storage.dao.HostInfoDAO; import com.redhat.thermostat.storage.model.AggregateCount; @@ -173,7 +173,8 @@ private static final Category<TestClass> testCategory = new Category<>("MongoStorageTest", TestClass.class, key1, key2, key3, key4, key5); private static final Category<TestClass> emptyTestCategory = new Category<>("MongoEmptyCategory", TestClass.class); - private StartupConfiguration conf; + private String url; + private StorageCredentials creds; private SSLConfiguration sslConf; private Mongo m; private DB db; @@ -182,7 +183,7 @@ private ExpressionFactory factory; private MongoStorage makeStorage() { - MongoStorage storage = new MongoStorage(conf, sslConf); + MongoStorage storage = new MongoStorage(url, creds, sslConf); storage.mapCategoryToDBCollection(testCategory, testCollection); storage.mapCategoryToDBCollection(emptyTestCategory, emptyTestCollection); return storage; @@ -190,8 +191,8 @@ @Before public void setUp() throws Exception { - conf = mock(StartupConfiguration.class); - when(conf.getDBConnectionString()).thenReturn("mongodb://127.0.0.1:27518"); + url = "mongodb://127.0.0.1:27518"; + creds = mock(StorageCredentials.class); sslConf = mock(SSLConfiguration.class); db = PowerMockito.mock(DB.class); m = PowerMockito.mock(Mongo.class); @@ -246,7 +247,7 @@ @After public void tearDown() { - conf = null; + url = null; m = null; db = null; testCollection = null; @@ -256,7 +257,7 @@ @Test public void isBackingStorage() { - MongoStorage storage = new MongoStorage(conf, sslConf); + MongoStorage storage = new MongoStorage(url, creds, sslConf); assertTrue(storage instanceof BackingStorage); } @@ -593,7 +594,7 @@ public void verifyMongoCloseOnShutdown() throws Exception { Mongo mockMongo = mock(Mongo.class); when(db.getMongo()).thenReturn(mockMongo); - MongoStorage storage = new MongoStorage(conf, sslConf); + MongoStorage storage = new MongoStorage(url, creds, sslConf); setDbFieldInStorage(storage); storage.shutdown(); verify(mockMongo).close();
--- a/web/client/src/main/java/com/redhat/thermostat/web/client/internal/WebStorage.java Mon Nov 18 11:48:35 2013 -0700 +++ b/web/client/src/main/java/com/redhat/thermostat/web/client/internal/WebStorage.java Thu Nov 21 13:00:46 2013 -0700 @@ -84,8 +84,6 @@ import com.redhat.thermostat.common.ssl.SslInitException; import com.redhat.thermostat.common.utils.LoggingUtils; import com.redhat.thermostat.shared.config.SSLConfiguration; -import com.redhat.thermostat.storage.config.AuthenticationConfiguration; -import com.redhat.thermostat.storage.config.StartupConfiguration; import com.redhat.thermostat.storage.core.AuthToken; import com.redhat.thermostat.storage.core.Category; import com.redhat.thermostat.storage.core.Connection; @@ -99,6 +97,7 @@ import com.redhat.thermostat.storage.core.StatementDescriptor; import com.redhat.thermostat.storage.core.StatementExecutionException; import com.redhat.thermostat.storage.core.Storage; +import com.redhat.thermostat.storage.core.StorageCredentials; import com.redhat.thermostat.storage.core.StorageException; import com.redhat.thermostat.storage.model.Pojo; import com.redhat.thermostat.web.common.PreparedParameterSerializer; @@ -314,24 +313,23 @@ private Gson gson; // package private for testing DefaultHttpClient httpClient; - private String username; - private char[] password; + private StorageCredentials creds; private SecureRandom random; private WebConnection conn; // for testing - WebStorage(StartupConfiguration config, DefaultHttpClient client, + WebStorage(String url, StorageCredentials creds, DefaultHttpClient client, ClientConnectionManager connManager, SSLConfiguration sslConf) { - init(config, client, connManager, sslConf); + init(url, creds, client, connManager, sslConf); } - public WebStorage(StartupConfiguration config, SSLConfiguration sslConf) throws StorageException { + public WebStorage(String url, StorageCredentials creds, SSLConfiguration sslConf) throws StorageException { ClientConnectionManager connManager = new ThreadSafeClientConnManager(); DefaultHttpClient client = new DefaultHttpClient(connManager); - init(config, client, connManager, sslConf); + init(url, creds, client, connManager, sslConf); } - private void init(StartupConfiguration config, DefaultHttpClient client, + private void init(String url, StorageCredentials creds, DefaultHttpClient client, ClientConnectionManager connManager, SSLConfiguration sslConf) { categoryIds = new HashMap<>(); gson = new GsonBuilder().registerTypeHierarchyAdapter(Pojo.class, @@ -344,13 +342,10 @@ random = new SecureRandom(); conn = new WebConnection(); - setEndpoint(config.getDBConnectionString()); - if (config instanceof AuthenticationConfiguration) { - AuthenticationConfiguration authConfig = (AuthenticationConfiguration) config; - setAuthConfig(authConfig.getUsername(), authConfig.getPassword()); - } + this.endpoint = url; + this.creds = creds; // setup SSL if necessary - if (config.getDBConnectionString().startsWith(HTTPS_PREFIX)) { + if (endpoint.startsWith(HTTPS_PREFIX)) { registerSSLScheme(connManager, sslConf); } } @@ -369,14 +364,17 @@ private void initAuthentication(DefaultHttpClient client) throws MalformedURLException { + String username = creds.getUsername(); + char[] password = creds.getPassword(); if (username != null && password != null) { URL endpointURL = new URL(endpoint); // TODO: Maybe also limit to realm like 'Thermostat Realm' or such? AuthScope scope = new AuthScope(endpointURL.getHost(), endpointURL.getPort()); - // FIXME Password as string? bad. + // FIXME Password as string? BAD. Limited by apache API here however. Credentials creds = new UsernamePasswordCredentials(username, new String(password)); + Arrays.fill(password, '\0'); client.getCredentialsProvider().setCredentials(scope, creds); } } @@ -586,15 +584,6 @@ post(endpoint + "/purge", agentIdParams).close(); } - public void setEndpoint(String endpoint) { - this.endpoint = endpoint; - } - - public void setAuthConfig(String username, char[] password) { - this.username = username; - this.password = password; - } - @Override public AuthToken generateToken(String actionName) throws StorageException { byte[] clientToken = new byte[256];
--- a/web/client/src/main/java/com/redhat/thermostat/web/client/internal/WebStorageProvider.java Mon Nov 18 11:48:35 2013 -0700 +++ b/web/client/src/main/java/com/redhat/thermostat/web/client/internal/WebStorageProvider.java Thu Nov 21 13:00:46 2013 -0700 @@ -37,38 +37,34 @@ package com.redhat.thermostat.web.client.internal; import com.redhat.thermostat.shared.config.SSLConfiguration; -import com.redhat.thermostat.storage.config.AuthenticationConfiguration; -import com.redhat.thermostat.storage.config.StartupConfiguration; import com.redhat.thermostat.storage.core.SecureQueuedStorage; import com.redhat.thermostat.storage.core.Storage; +import com.redhat.thermostat.storage.core.StorageCredentials; import com.redhat.thermostat.storage.core.StorageProvider; public class WebStorageProvider implements StorageProvider { - private StartupConfiguration config; + private String url; + private StorageCredentials creds; private SSLConfiguration sslConf; @Override public Storage createStorage() { - WebStorage storage = new WebStorage(config, sslConf); - storage.setEndpoint(config.getDBConnectionString()); - if (config instanceof AuthenticationConfiguration) { - AuthenticationConfiguration authConf = (AuthenticationConfiguration) config; - storage.setAuthConfig(authConf.getUsername(), authConf.getPassword()); - } + WebStorage storage = new WebStorage(url, creds, sslConf); return new SecureQueuedStorage(storage); } @Override - public void setConfig(StartupConfiguration config, SSLConfiguration sslConf) { - this.config = config; + public void setConfig(String connectionURL, StorageCredentials creds, SSLConfiguration sslConf) { + this.url = connectionURL; + this.creds = creds; this.sslConf = sslConf; } @Override public boolean canHandleProtocol() { // use http since this might be https at some point - return config.getDBConnectionString().startsWith("http"); + return url.startsWith("http"); } }
--- a/web/client/src/test/java/com/redhat/thermostat/web/client/internal/WebStorageProviderTest.java Mon Nov 18 11:48:35 2013 -0700 +++ b/web/client/src/test/java/com/redhat/thermostat/web/client/internal/WebStorageProviderTest.java Thu Nov 21 13:00:46 2013 -0700 @@ -39,39 +39,48 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; import org.junit.Test; -import org.mockito.Mockito; import com.redhat.thermostat.shared.config.SSLConfiguration; -import com.redhat.thermostat.storage.config.AuthenticationConfiguration; -import com.redhat.thermostat.storage.config.StartupConfiguration; import com.redhat.thermostat.storage.core.BackingStorage; import com.redhat.thermostat.storage.core.QueuedStorage; import com.redhat.thermostat.storage.core.SecureStorage; import com.redhat.thermostat.storage.core.Storage; +import com.redhat.thermostat.storage.core.StorageCredentials; public class WebStorageProviderTest { @Test public void createStorageCreatesSecureStorage() { WebStorageProvider provider = new WebStorageProvider(); - MockConfiguration config = mock(MockConfiguration.class); - when(config.getDBConnectionString()).thenReturn("http://something"); SSLConfiguration sslConf = mock(SSLConfiguration.class); - provider.setConfig(config, sslConf); + StorageCredentials creds = mock(StorageCredentials.class); + provider.setConfig("http://something", creds, sslConf); Storage storage = provider.createStorage(); assertTrue(storage instanceof SecureStorage); assertTrue(storage instanceof QueuedStorage); assertFalse(storage instanceof BackingStorage); - verify(config, Mockito.atLeastOnce()).getUsername(); - verify(config, Mockito.atLeastOnce()).getPassword(); + } + + @Test + public void canHandleHttpProtocol() { + WebStorageProvider provider = new WebStorageProvider(); + provider.setConfig("http://something.com", null, null); + assertTrue(provider.canHandleProtocol()); } - - private abstract static class MockConfiguration implements - AuthenticationConfiguration, StartupConfiguration { - // no-op + + @Test + public void canHandleHttpsProtocol() { + WebStorageProvider provider = new WebStorageProvider(); + provider.setConfig("https://something.com", null, null); + assertTrue(provider.canHandleProtocol()); + } + + @Test + public void cannotHandleNotWebProtocol() { + WebStorageProvider provider = new WebStorageProvider(); + provider.setConfig("ftp://something.com", null, null); + assertFalse(provider.canHandleProtocol()); } }
--- a/web/client/src/test/java/com/redhat/thermostat/web/client/internal/WebStorageTest.java Mon Nov 18 11:48:35 2013 -0700 +++ b/web/client/src/test/java/com/redhat/thermostat/web/client/internal/WebStorageTest.java Thu Nov 21 13:00:46 2013 -0700 @@ -101,6 +101,7 @@ import com.redhat.thermostat.storage.core.PreparedStatement; import com.redhat.thermostat.storage.core.StatementDescriptor; import com.redhat.thermostat.storage.core.StatementExecutionException; +import com.redhat.thermostat.storage.core.StorageCredentials; import com.redhat.thermostat.storage.model.Pojo; import com.redhat.thermostat.test.FreePortFinder; import com.redhat.thermostat.test.FreePortFinder.TryPort; @@ -159,16 +160,9 @@ } }); - StartupConfiguration config = new StartupConfiguration() { - - @Override - public String getDBConnectionString() { - return "http://fluff.example.org"; - } - }; SSLConfiguration sslConf = mock(SSLConfiguration.class); - storage = new WebStorage(config, sslConf); - storage.setEndpoint("http://localhost:" + port + "/"); + storage = new WebStorage("http://localhost:" + port + "/", + new TrivialStorageCredentials(null, null), sslConf); headers = new HashMap<>(); registerCategory(); } @@ -537,7 +531,8 @@ } }; SSLConfiguration sslConf = mock(SSLConfiguration.class); - WebStorage storage = new WebStorage(config, sslConf); + WebStorage storage = new WebStorage("https://onlyHttpsPrefixUsed.example.com", + new TrivialStorageCredentials(null, null), sslConf); HttpClient client = storage.httpClient; SchemeRegistry schemeReg = client.getConnectionManager().getSchemeRegistry(); Scheme scheme = schemeReg.getScheme("https"); @@ -599,16 +594,9 @@ ClientConnectionManager connManager = mock(ClientConnectionManager.class); // this should make connect fail Mockito.doThrow(RuntimeException.class).when(client).getCredentialsProvider(); - StartupConfiguration config = new StartupConfiguration() { - - @Override - public String getDBConnectionString() { - return "http://fluff.example.org"; - } - }; SSLConfiguration sslConf = mock(SSLConfiguration.class); - storage = new WebStorage(config, client, connManager, sslConf); - storage.setEndpoint("http://localhost:" + port + "/"); + storage = new WebStorage("http://localhost:" + port + "/", new TrivialStorageCredentials(null, null), + client, connManager, sslConf); CountDownLatch latch = new CountDownLatch(1); MyListener listener = new MyListener(latch); @@ -683,5 +671,22 @@ } } } + + private class TrivialStorageCredentials implements StorageCredentials { + private String user; + private char[] pw; + private TrivialStorageCredentials(String user, char[] password) { + this.user = user; + this.pw = password; + } + @Override + public String getUsername() { + return user; + } + @Override + public char[] getPassword() { + return pw; + } + } }
--- a/web/server/src/main/java/com/redhat/thermostat/web/server/StorageFactory.java Mon Nov 18 11:48:35 2013 -0700 +++ b/web/server/src/main/java/com/redhat/thermostat/web/server/StorageFactory.java Thu Nov 21 13:00:46 2013 -0700 @@ -40,9 +40,8 @@ import com.redhat.thermostat.shared.config.CommonPaths; import com.redhat.thermostat.shared.config.SSLConfiguration; import com.redhat.thermostat.shared.config.internal.SSLConfigurationImpl; -import com.redhat.thermostat.storage.config.ConnectionConfiguration; -import com.redhat.thermostat.storage.config.StartupConfiguration; import com.redhat.thermostat.storage.core.Storage; +import com.redhat.thermostat.storage.core.StorageCredentials; import com.redhat.thermostat.storage.core.StorageProvider; import com.redhat.thermostat.storage.mongodb.MongoStorageProvider; @@ -51,16 +50,15 @@ private static Storage storage; // Web server is not OSGi, this factory method is workaround. - static Storage getStorage(String storageClass, final String storageEndpoint, final String username, - final char[] password, final CommonPaths paths) { + static Storage getStorage(String storageClass, final String storageEndpoint, final CommonPaths paths, + final StorageCredentials creds) { if (storage != null) { return storage; } - StartupConfiguration conf = new ConnectionConfiguration(storageEndpoint, username, password); SSLConfiguration sslConf = new SSLConfigurationImpl(paths); try { StorageProvider provider = (StorageProvider) Class.forName(storageClass).newInstance(); - provider.setConfig(conf, sslConf); + provider.setConfig(storageEndpoint, creds, sslConf); storage = provider.createStorage(); storage.getConnection().connect(); return storage; @@ -71,7 +69,7 @@ System.err.println("could not instantiate provider: " + storageClass + ", falling back to MongoStorage"); e.printStackTrace(); StorageProvider provider = new MongoStorageProvider(); - provider.setConfig(conf, sslConf); + provider.setConfig(storageEndpoint, creds, sslConf); storage = provider.createStorage(); return storage; }
--- a/web/server/src/main/java/com/redhat/thermostat/web/server/WebStorageEndPoint.java Mon Nov 18 11:48:35 2013 -0700 +++ b/web/server/src/main/java/com/redhat/thermostat/web/server/WebStorageEndPoint.java Thu Nov 21 13:00:46 2013 -0700 @@ -91,6 +91,7 @@ import com.redhat.thermostat.storage.core.Statement; import com.redhat.thermostat.storage.core.StatementDescriptor; import com.redhat.thermostat.storage.core.Storage; +import com.redhat.thermostat.storage.core.StorageCredentials; import com.redhat.thermostat.storage.core.auth.DescriptorMetadata; import com.redhat.thermostat.storage.core.auth.StatementDescriptorMetadataFactory; import com.redhat.thermostat.storage.model.AggregateResult; @@ -215,10 +216,23 @@ if (storage == null) { String storageClass = getServletConfig().getInitParameter(STORAGE_CLASS); String storageEndpoint = getServletConfig().getInitParameter(STORAGE_ENDPOINT); - String username = getServletConfig().getInitParameter(STORAGE_USERNAME); + final String username = getServletConfig().getInitParameter(STORAGE_USERNAME); // FIXME Password as string? bad. - String password = getServletConfig().getInitParameter(STORAGE_PASSWORD); - storage = StorageFactory.getStorage(storageClass, storageEndpoint, username, password == null ? null : password.toCharArray(), paths); + final String password = getServletConfig().getInitParameter(STORAGE_PASSWORD); + StorageCredentials creds = new StorageCredentials() { + + @Override + public String getUsername() { + return username; + } + + @Override + public char[] getPassword() { + return password == null ? null : password.toCharArray(); + } + + }; + storage = StorageFactory.getStorage(storageClass, storageEndpoint, paths, creds); } String uri = req.getRequestURI(); int lastPartIdx = uri.lastIndexOf("/");