Mercurial > hg > release > thermostat-1.0
changeset 1371:b0b7a213659b
New Keyring, password as char[]
Reviewed-by: jerboaa
Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2013-November/008814.html
line wrap: on
line diff
--- a/README Mon Nov 25 20:27:27 2013 -0500 +++ b/README Mon Nov 18 11:48:35 2013 -0700 @@ -146,22 +146,9 @@ 3.1.2. GNOME KEYRING AND THERMOSTAT -Thermostat includes experimental support to store user credentials in the -Gnome Keyring daemon, this support is currently limited to passwords only. - -Runtime supports for the Keyring is optional and can be disabled via a system -property: - - * -J-Dcom.redhat.thermostat.utils.keyring.provider=com.redhat.thermostat.utils.keyring.MemoryKeyring - -The memory keyring stores passwords in memory and not on disk. This means -passwords don't persist across Thermostat sessions. A session is terminated by -exiting the application. - -NOTE: thermostat keeps all the passwords in clear text memory during execution, -the keyring is only used to securely store passwords on disk. The passwords are -saved in the user session keychain and available to other applications that -can access the keyring. +Thermostat includes support to store user credentials in the Gnome Keyring +daemon, if the user wishes. If this is not available at runtime, no credentials +will be stored. -------------------------------------------------------------------------------- 3.2. RUNNING THERMOSTAT ECLIPSE
--- a/agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/AgentApplicationTest.java Mon Nov 25 20:27:27 2013 -0500 +++ b/agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/AgentApplicationTest.java Mon Nov 18 11:48:35 2013 -0700 @@ -40,6 +40,7 @@ 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; @@ -124,7 +125,7 @@ dbServiceFactory = mock(DbServiceFactory.class); dbService = mock(DbService.class); writerId = mock(WriterID.class); - when(dbServiceFactory.createDbService(anyString(), anyString(), anyString())).thenReturn(dbService); + when(dbServiceFactory.createDbService(anyString(), isA(char[].class), anyString())).thenReturn(dbService); exitStatus = mock(ExitStatus.class); }
--- a/agent/core/src/main/java/com/redhat/thermostat/agent/config/AgentConfigsUtils.java Mon Nov 25 20:27:27 2013 -0500 +++ b/agent/core/src/main/java/com/redhat/thermostat/agent/config/AgentConfigsUtils.java Mon Nov 18 11:48:35 2013 -0700 @@ -118,7 +118,7 @@ AgentStartupConfiguration config) { // Default values will be enough if storage configured with not auth necessary. config.setUsername(""); - config.setPassword(""); + config.setPassword(new char[]{}); if (authFile != null && authFile.canRead() && authFile.isFile()) { long length = authFile.length(); char[] authData = null; @@ -160,9 +160,8 @@ char[] value = getPassword(data, position); if (value != null) { // Password - config.setPassword(new String(value)); + config.setPassword(value); position = nextLine(data, position); - Arrays.fill(value, '\0'); value = null; continue; }
--- a/agent/core/src/main/java/com/redhat/thermostat/agent/config/AgentStartupConfiguration.java Mon Nov 25 20:27:27 2013 -0500 +++ b/agent/core/src/main/java/com/redhat/thermostat/agent/config/AgentStartupConfiguration.java Mon Nov 18 11:48:35 2013 -0700 @@ -40,13 +40,12 @@ import com.redhat.thermostat.storage.config.StartupConfiguration; public class AgentStartupConfiguration implements StartupConfiguration, AuthenticationConfiguration { - - private boolean debugConsole; + private boolean purge; private String url; private String username; - private String password; + private char[] password; private long startTime; private String address; @@ -89,12 +88,12 @@ return username; } - public void setPassword(String password) { + public void setPassword(char[] password) { this.password = password; } @Override - public String getPassword() { + public char[] getPassword() { return password; }
--- a/agent/core/src/test/java/com/redhat/thermostat/agent/config/AgentConfigsUtilsTest.java Mon Nov 25 20:27:27 2013 -0500 +++ b/agent/core/src/test/java/com/redhat/thermostat/agent/config/AgentConfigsUtilsTest.java Mon Nov 18 11:48:35 2013 -0700 @@ -79,7 +79,7 @@ Assert.assertFalse(config.purge()); Assert.assertEquals("42.42.42.42:42", config.getConfigListenAddress()); Assert.assertEquals("user", config.getUsername()); - Assert.assertEquals("pass", config.getPassword()); + Assert.assertEquals("pass", new String(config.getPassword())); } @Test @@ -88,7 +88,7 @@ AgentStartupConfiguration config = new AgentStartupConfiguration(); AgentConfigsUtils.setAuthConfigFromFile(tmpAuth, config); Assert.assertEquals("user", config.getUsername()); - Assert.assertEquals("pass", config.getPassword()); + Assert.assertEquals("pass", new String(config.getPassword())); } @Test @@ -97,7 +97,7 @@ AgentStartupConfiguration config = new AgentStartupConfiguration(); AgentConfigsUtils.setAuthConfigFromFile(tmpAuth, config); Assert.assertEquals("", config.getUsername()); - Assert.assertEquals("", config.getPassword()); + Assert.assertEquals("", new String(config.getPassword())); } // TODO add test to ensure user agent config overrides system agent config. @@ -108,7 +108,7 @@ AgentStartupConfiguration config = new AgentStartupConfiguration(); AgentConfigsUtils.setAuthConfigFromFile(tmpAuth, config); Assert.assertEquals("", config.getUsername()); - Assert.assertEquals("", config.getPassword()); + Assert.assertEquals("", new String(config.getPassword())); } private File createTempAuthFile(String contents) throws IOException { @@ -130,7 +130,7 @@ AgentStartupConfiguration config = new AgentStartupConfiguration(); AgentConfigsUtils.parseAuthConfigFromData(authData, authData.length, config); Assert.assertEquals("user", config.getUsername()); - Assert.assertEquals("pass", config.getPassword()); + Assert.assertEquals("pass", new String(config.getPassword())); } @Test
--- a/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/ConnectCommand.java Mon Nov 25 20:27:27 2013 -0500 +++ b/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/ConnectCommand.java Mon Nov 18 11:48:35 2013 -0700 @@ -37,6 +37,7 @@ package com.redhat.thermostat.client.cli.internal; import java.io.IOException; +import java.util.Arrays; import java.util.Objects; import org.osgi.framework.BundleContext; @@ -98,7 +99,7 @@ // This argument is considered "required" so option parsing should mean this is impossible. Objects.requireNonNull(dbUrl); String username = null; - String password = null; + char[] password = null; if (prefs.getConnectionUrl().equals(dbUrl)) { // Have we previously saved connection parameters for this Url? username = prefs.getUserName(); @@ -109,7 +110,7 @@ try { StorageAuthInfoGetter getUserPass = new StorageAuthInfoGetter(console); username = getUserPass.getUserName(dbUrl); - password = new String(getUserPass.getPassword(dbUrl)); + password = getUserPass.getPassword(dbUrl); } catch (IOException e) { throw new CommandException(translator.localize(LocaleResources.COMMAND_CONNECT_USER_PROMPT_ERROR), e); } @@ -124,6 +125,10 @@ String error = ex.getMessage(); 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'); + } } }
--- a/client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/ConnectCommandTest.java Mon Nov 25 20:27:27 2013 -0500 +++ b/client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/ConnectCommandTest.java Mon Nov 18 11:48:35 2013 -0700 @@ -116,7 +116,7 @@ String username = "testuser"; String password = "testpassword"; String dbUrl = "mongodb://10.23.122.1:12578"; - when(dbServiceFactory.createDbService(eq(username), eq(password), eq(dbUrl))).thenReturn(dbService); + when(dbServiceFactory.createDbService(eq(username), eq(password.toCharArray()), 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 25 20:27:27 2013 -0500 +++ b/client/core/src/main/java/com/redhat/thermostat/client/core/views/ClientConfigurationView.java Mon Nov 18 11:48:35 2013 -0700 @@ -53,13 +53,13 @@ void removeListener(ActionListener<Action> listener); void setConnectionUrl(String url); - void setPassword(String password); + void setPassword(char[] password); void setUserName(String username); void setSaveEntitlemens(boolean save); boolean getSaveEntitlements(); String getUserName(); - String getPassword(); + char[] getPassword(); String getConnectionUrl(); void showDialog();
--- a/client/core/src/main/java/com/redhat/thermostat/client/ui/ClientConfigurationController.java Mon Nov 25 20:27:27 2013 -0500 +++ b/client/core/src/main/java/com/redhat/thermostat/client/ui/ClientConfigurationController.java Mon Nov 18 11:48:35 2013 -0700 @@ -39,7 +39,6 @@ import java.io.IOException; import java.util.logging.Level; import java.util.logging.Logger; -import java.util.prefs.BackingStoreException; import com.redhat.thermostat.client.core.views.ClientConfigurationView; import com.redhat.thermostat.client.core.views.ClientConfigurationView.Action;
--- a/client/core/src/test/java/com/redhat/thermostat/client/ui/ClientConfigurationControllerTest.java Mon Nov 25 20:27:27 2013 -0500 +++ b/client/core/src/test/java/com/redhat/thermostat/client/ui/ClientConfigurationControllerTest.java Mon Nov 18 11:48:35 2013 -0700 @@ -38,6 +38,7 @@ import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; +import static org.mockito.Matchers.isA; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -59,13 +60,13 @@ public void setUp() { model = mock(ClientPreferences.class); when(model.getConnectionUrl()).thenReturn("mock-connection-url"); - when(model.getPassword()).thenReturn("mock-password"); + when(model.getPassword()).thenReturn("mock-password".toCharArray()); when(model.getUserName()).thenReturn("mock-username"); when(model.getSaveEntitlements()).thenReturn(false); view = mock(ClientConfigurationView.class); when(view.getConnectionUrl()).thenReturn("mock-connection-url"); - when(view.getPassword()).thenReturn("mock-password"); + when(view.getPassword()).thenReturn("mock-password".toCharArray()); when(view.getUserName()).thenReturn("mock-username"); when(view.getSaveEntitlements()).thenReturn(true); } @@ -84,7 +85,7 @@ verify(model).getConnectionUrl(); verify(view).setConnectionUrl(eq("mock-connection-url")); - verify(view).setPassword(eq("mock-password")); + verify(view).setPassword(eq("mock-password".toCharArray())); verify(view).setUserName(eq("mock-username")); verify(view).setSaveEntitlemens(eq(false)); verify(view).showDialog(); @@ -112,7 +113,7 @@ private void verifyCloseCancelCommon() { verify(model, times(0)).setConnectionUrl(any(String.class)); - verify(model, times(0)).setCredentials(any(String.class), any(String.class)); + verify(model, times(0)).setCredentials(any(String.class), isA(char[].class)); verify(view, times(0)).getConnectionUrl(); verify(view, times(0)).showDialog(); @@ -143,7 +144,7 @@ private void verifyCloseAcceptCommon() { verify(model).setConnectionUrl(eq("mock-connection-url")); - verify(model).setCredentials(eq("mock-username"), eq("mock-password")); + verify(model).setCredentials(eq("mock-username"), eq("mock-password".toCharArray())); verify(model).setSaveEntitlements(eq(true)); verify(view).getConnectionUrl();
--- a/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/Main.java Mon Nov 25 20:27:27 2013 -0500 +++ b/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/Main.java Mon Nov 18 11:48:35 2013 -0700 @@ -38,6 +38,7 @@ 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; @@ -79,6 +80,7 @@ private DbServiceFactory dbServiceFactory; private Keyring keyring; private CommonPaths paths; + private ClientPreferences prefs; private CountDownLatch shutdown; private MainWindowControllerImpl mainController; private MainWindowRunnable mainWindowRunnable; @@ -111,6 +113,7 @@ this.paths = paths; this.shutdown = shutdown; this.mainWindowRunnable = mainWindowRunnable; + this.prefs = new ClientPreferences(keyring, paths); } public void run() { @@ -199,7 +202,8 @@ final ConnectionHandler reconnectionHandler = new ConnectionHandler(service); try { // create DbService with potentially modified parameters - final DbService dbService = dbServiceFactory.createDbService(prefs.getUserName(), prefs.getPassword(), prefs.getConnectionUrl()); + final char[] password = prefs.getPassword(); + final DbService dbService = dbServiceFactory.createDbService(prefs.getUserName(), password, prefs.getConnectionUrl()); dbService.addConnectionListener(reconnectionHandler); service.execute(new Runnable() { @Override @@ -210,6 +214,10 @@ // Note: DbService fires a ConnectionListener event when it // 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'); + } } } }); @@ -240,7 +248,6 @@ } private void createPreferencesDialog(final ExecutorService service) { - ClientPreferences prefs = new ClientPreferences(keyring, paths); ClientConfigurationView configDialog = new ClientConfigurationSwing(); ClientConfigurationController controller = new ClientConfigurationController(prefs, configDialog, new MainClientConfigReconnector(service));
--- a/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/views/ClientConfigurationSwing.java Mon Nov 25 20:27:27 2013 -0500 +++ b/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/views/ClientConfigurationSwing.java Mon Nov 18 11:48:35 2013 -0700 @@ -37,11 +37,14 @@ package com.redhat.thermostat.client.swing.internal.views; import java.awt.Frame; +import java.awt.event.KeyEvent; +import java.awt.event.KeyListener; import java.awt.event.WindowAdapter; import java.awt.event.WindowEvent; import java.beans.PropertyChangeEvent; import java.beans.PropertyChangeListener; import java.lang.reflect.InvocationTargetException; +import java.util.Arrays; import java.util.concurrent.Callable; import java.util.concurrent.CopyOnWriteArrayList; @@ -65,6 +68,8 @@ private final WindowClosingListener windowClosingListener; private final ClientConfigurationPanel configurationPanel; + private char[] previousPassword = new char[]{}; + private boolean userOverrodePassword = false; private final CopyOnWriteArrayList<ActionListener<Action>> listeners = new CopyOnWriteArrayList<>(); @@ -75,6 +80,24 @@ windowClosingListener = new WindowClosingListener(); configurationPanel = new ClientConfigurationPanel(); + configurationPanel.password.addKeyListener(new KeyListener() { + + @Override + public void keyPressed(KeyEvent arg0) { + // Ignore + } + + @Override + public void keyReleased(KeyEvent arg0) { + // Ignore + } + + @Override + public void keyTyped(KeyEvent arg0) { + userOverrodePassword = true; + } + + }); java.awt.event.ActionListener acceptOnEnterListener = new java.awt.event.ActionListener() { @Override @@ -197,18 +220,16 @@ class WindowClosingListener extends WindowAdapter { @Override public void windowClosing(WindowEvent e) { + if (previousPassword != null) { + Arrays.fill(previousPassword, '\0'); + } fireAction(new ActionEvent<>(ClientConfigurationSwing.this, Action.CLOSE_CANCEL)); } } @Override - public void setPassword(final String password) { - SwingUtilities.invokeLater(new Runnable() { - @Override - public void run() { - configurationPanel.password.setText(password); - } - }); + public void setPassword(final char[] password) { + this.previousPassword = password; } @Override @@ -222,12 +243,15 @@ }; @Override - public String getPassword() { + public char[] getPassword() { + if (!userOverrodePassword) { + return previousPassword; + } try { - return new EdtHelper().callAndWait(new Callable<String>() { + return new EdtHelper().callAndWait(new Callable<char[]>() { @Override - public String call() throws Exception { - return configurationPanel.password.getText(); + public char[] call() throws Exception { + return configurationPanel.password.getPassword(); } }); } catch (InvocationTargetException | InterruptedException e) {
--- a/client/swing/src/test/java/com/redhat/thermostat/client/swing/internal/MainTest.java Mon Nov 25 20:27:27 2013 -0500 +++ b/client/swing/src/test/java/com/redhat/thermostat/client/swing/internal/MainTest.java Mon Nov 18 11:48:35 2013 -0700 @@ -36,6 +36,7 @@ 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; @@ -110,7 +111,7 @@ dbServiceFactory = mock(DbServiceFactory.class); dbService = mock(DbService.class); - when(dbServiceFactory.createDbService(anyString(), anyString(), anyString())).thenReturn(dbService); + when(dbServiceFactory.createDbService(anyString(), (char[]) any(), anyString())).thenReturn(dbService); connectionListenerCaptor = ArgumentCaptor.forClass(ConnectionListener.class); doNothing().when(dbService).addConnectionListener(connectionListenerCaptor.capture());
--- a/client/swing/src/test/java/com/redhat/thermostat/client/swing/internal/views/ClientConfigurationSwingTest.java Mon Nov 25 20:27:27 2013 -0500 +++ b/client/swing/src/test/java/com/redhat/thermostat/client/swing/internal/views/ClientConfigurationSwingTest.java Mon Nov 18 11:48:35 2013 -0700 @@ -137,7 +137,7 @@ JTextComponentFixture textBox = frameFixture.textBox("password"); textBox.enterText("foobar"); - assertEquals("foobar", frame.getPassword()); + assertEquals("foobar", new String(frame.getPassword())); }
--- a/common/core/src/main/java/com/redhat/thermostat/common/config/ClientPreferences.java Mon Nov 25 20:27:27 2013 -0500 +++ b/common/core/src/main/java/com/redhat/thermostat/common/config/ClientPreferences.java Mon Nov 18 11:48:35 2013 -0700 @@ -48,25 +48,25 @@ 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.Credentials; import com.redhat.thermostat.utils.keyring.Keyring; public class ClientPreferences { static final String USERNAME = "username"; - static final String PASSWORD = "password"; static final String CONNECTION_URL = "connection-url"; static final String SAVE_ENTITLEMENTS = "save-entitlements"; static final String DEFAULT_CONNECTION_URL = "mongodb://127.0.0.1:27518"; private static final Logger logger = LoggingUtils.getLogger(ClientPreferences.class); - + private Properties prefs; private Keyring keyring; private File userConfig; - - private Credentials userCredentials; - + + private String url; + private String username; + private char[] password; + public ClientPreferences(Keyring keyring, CommonPaths files) { userConfig = files.getUserClientConfigurationFile(); Properties props = new Properties(); @@ -98,15 +98,9 @@ private void init(Properties properties, Keyring keyring) { this.prefs = properties; this.keyring = keyring; - this.userCredentials = new Credentials(); - userCredentials.setDescription("thermostat keychain"); - - // load the initial defaults - userCredentials.setUserName(prefs.getProperty(USERNAME, "")); - this.userCredentials.setPassword(""); - if (getSaveEntitlements()) { - keyring.loadPassword(userCredentials); - } + this.url = DEFAULT_CONNECTION_URL; + this.username = ""; + this.password = new char[]{}; } public boolean getSaveEntitlements() { @@ -121,37 +115,41 @@ return prefs.getProperty(CONNECTION_URL, DEFAULT_CONNECTION_URL); } - public String getPassword() { + public char[] getPassword() { if (getSaveEntitlements()) { - keyring.loadPassword(userCredentials); + return keyring.getPassword(getConnectionUrl(), getUserName()); } - return userCredentials.getPassword(); + return password; } - public String getUserName() { - return prefs.getProperty(USERNAME, ""); + public String getUserName() { + if (getSaveEntitlements()) { + return prefs.getProperty(USERNAME, ""); + } + return username; } public void setConnectionUrl(String url) { - prefs.put(CONNECTION_URL, url); + this.url = url; + if (getSaveEntitlements()) { + prefs.put(CONNECTION_URL, url); + } } - public void setCredentials(String userName, String password) { - - userCredentials.setUserName(userName); - userCredentials.setPassword(password); - prefs.put(USERNAME, userName); - + public void setCredentials(String userName, char[] password) { + this.username = userName; + this.password = password; + if (getSaveEntitlements()) { - keyring.savePassword(userCredentials); + prefs.put(USERNAME, userName); + keyring.savePassword(getConnectionUrl(), userName, password); } } public void flush() throws IOException { prefs.store(new FileWriter(userConfig), ""); - userCredentials.setUserName(getUserName()); if (getSaveEntitlements()) { - keyring.loadPassword(userCredentials); + keyring.savePassword(getConnectionUrl(), username, password); } } }
--- a/common/core/src/test/java/com/redhat/thermostat/common/config/ClientPreferencesTest.java Mon Nov 25 20:27:27 2013 -0500 +++ b/common/core/src/test/java/com/redhat/thermostat/common/config/ClientPreferencesTest.java Mon Nov 18 11:48:35 2013 -0700 @@ -39,6 +39,7 @@ 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; @@ -49,7 +50,6 @@ import org.junit.Test; -import com.redhat.thermostat.utils.keyring.Credentials; import com.redhat.thermostat.utils.keyring.Keyring; public class ClientPreferencesTest { @@ -73,6 +73,7 @@ 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); clientPrefs.setConnectionUrl("test"); @@ -88,10 +89,10 @@ when(prefs.getProperty(eq(ClientPreferences.SAVE_ENTITLEMENTS), any(String.class))).thenReturn("true"); ClientPreferences clientPrefs = new ClientPreferences(prefs, keyring); - clientPrefs.setCredentials("fluff", "fluffPassword"); + clientPrefs.setCredentials("fluff", "fluffPassword".toCharArray()); verify(prefs).put(eq(ClientPreferences.USERNAME), eq("fluff")); - verify(prefs, times(0)).put(eq(ClientPreferences.PASSWORD), any(String.class)); + verify(keyring).savePassword( (String) any(), eq("fluff"), isA(char[].class)); } @Test @@ -99,11 +100,16 @@ 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); String username = clientPrefs.getUserName(); + assertEquals("", username); + + when(prefs.getProperty(eq(ClientPreferences.SAVE_ENTITLEMENTS), any(String.class))).thenReturn("true"); + username = clientPrefs.getUserName(); assertEquals("mock-value", username); verify(prefs, atLeastOnce()).getProperty(eq(ClientPreferences.USERNAME), any(String.class)); } @@ -116,17 +122,16 @@ 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("false").thenReturn("true"); + thenReturn("false").thenReturn("true"); ClientPreferences clientPrefs = new ClientPreferences(prefs, keyring); - String password = clientPrefs.getPassword(); - verify(prefs, times(0)).getProperty(eq(ClientPreferences.PASSWORD), any(String.class)); - verify(keyring, times(0)).loadPassword(any(Credentials.class)); + char[] password = clientPrefs.getPassword(); + verify(keyring, times(0)).getPassword(any(String.class), any(String.class)); - assertEquals("", password); + assertEquals(new String(password), ""); clientPrefs.getPassword(); - verify(keyring, atLeastOnce()).loadPassword(any(Credentials.class)); + verify(keyring, atLeastOnce()).getPassword(any(String.class), any(String.class)); } @Test
--- a/eclipse/com.redhat.thermostat.eclipse/src/com/redhat/thermostat/eclipse/internal/ConnectionConfiguration.java Mon Nov 25 20:27:27 2013 -0500 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,67 +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.eclipse.internal; - -import com.redhat.thermostat.storage.config.StartupConfiguration; - -public class ConnectionConfiguration implements StartupConfiguration { - - private String dbUrl; - private String username; - private String password; - - public ConnectionConfiguration(String username, String password, String dbUrl) { - this.dbUrl = dbUrl; - this.username = username; - this.password = password; - } - - public String getUsername() { - return username; - } - - public String getPassword() { - return password; - } - - @Override - public String getDBConnectionString() { - return dbUrl; - } - -} -
--- a/eclipse/com.redhat.thermostat.eclipse/src/com/redhat/thermostat/eclipse/internal/jobs/ConnectDbJob.java Mon Nov 25 20:27:27 2013 -0500 +++ b/eclipse/com.redhat.thermostat.eclipse/src/com/redhat/thermostat/eclipse/internal/jobs/ConnectDbJob.java Mon Nov 18 11:48:35 2013 -0700 @@ -36,23 +36,25 @@ package com.redhat.thermostat.eclipse.internal.jobs; +import java.util.Arrays; + import org.eclipse.core.runtime.IProgressMonitor; import org.eclipse.core.runtime.IStatus; import org.eclipse.core.runtime.Status; import org.eclipse.core.runtime.jobs.Job; +import com.redhat.thermostat.common.config.ClientPreferences; import com.redhat.thermostat.eclipse.LoggerFacility; import com.redhat.thermostat.eclipse.internal.Activator; -import com.redhat.thermostat.eclipse.internal.ConnectionConfiguration; import com.redhat.thermostat.storage.core.ConnectionException; import com.redhat.thermostat.storage.core.DbService; import com.redhat.thermostat.storage.core.DbServiceFactory; public class ConnectDbJob extends Job { - private ConnectionConfiguration configuration; + private ClientPreferences configuration; - public ConnectDbJob(String name, ConnectionConfiguration configuration) { + public ConnectDbJob(String name, ClientPreferences configuration) { super(name); this.configuration = configuration; } @@ -60,7 +62,7 @@ @Override protected IStatus run(IProgressMonitor monitor) { monitor.beginTask( - "Connecting to " + configuration.getDBConnectionString(), + "Connecting to " + configuration.getConnectionUrl(), IProgressMonitor.UNKNOWN); try { connectToBackEnd(); @@ -76,11 +78,18 @@ * Establish a DB connection. */ private void connectToBackEnd() throws ConnectionException { - DbServiceFactory dbServiceFactory = new DbServiceFactory(); - DbService dbService = dbServiceFactory.createDbService(configuration.getUsername(), - configuration.getPassword(), configuration.getDBConnectionString()); - dbService.connect(); + char[] password = null; + try { + DbServiceFactory dbServiceFactory = new DbServiceFactory(); + password = configuration.getPassword(); + DbService dbService = dbServiceFactory.createDbService(configuration.getUserName(), + password, configuration.getConnectionUrl()); + dbService.connect(); + } finally { + if (password != null) { + Arrays.fill(password, '\0'); + } + } } - }
--- a/eclipse/com.redhat.thermostat.eclipse/src/com/redhat/thermostat/eclipse/internal/preferences/MainPreferencePage.java Mon Nov 25 20:27:27 2013 -0500 +++ b/eclipse/com.redhat.thermostat.eclipse/src/com/redhat/thermostat/eclipse/internal/preferences/MainPreferencePage.java Mon Nov 18 11:48:35 2013 -0700 @@ -110,7 +110,9 @@ clientPrefs.setConnectionUrl(connectionUrlEditor.getStringValue()); clientPrefs.setSaveEntitlements(saveEntitlementsEditor.getBooleanValue()); if (saveEntitlementsEditor.getBooleanValue()) { - clientPrefs.setCredentials(usernameEditor.getStringValue(), passwordEditor.getStringValue()); + // FIXME Eclipse "Text" doesn't have a widget to return a char[] password, yet. + // https://bugs.eclipse.org/bugs/show_bug.cgi?id=297412 + clientPrefs.setCredentials(usernameEditor.getStringValue(), passwordEditor.getTextControl(getFieldEditorParent()).getText().toCharArray()); try { clientPrefs.flush(); } catch (IOException e) { @@ -190,8 +192,10 @@ IEclipsePreferences node = DefaultScope.INSTANCE.getNode(Activator.PLUGIN_ID); if (clientPrefs.getSaveEntitlements()) { node.put(ThermostatConstants.USERNAME_PREF_NAME, clientPrefs.getUserName()); - node.put(ThermostatConstants.PASSWORD_PREF_NAME, clientPrefs.getPassword()); - passwordEditor.setStringValue(clientPrefs.getPassword()); + // FIXME Need Eclipse prefs and widgets that supports char[] + String passString = new String(clientPrefs.getPassword()); + node.put(ThermostatConstants.PASSWORD_PREF_NAME, passString); + passwordEditor.setStringValue(passString); usernameEditor.setStringValue(clientPrefs.getUserName()); } else { try {
--- a/eclipse/com.redhat.thermostat.eclipse/src/com/redhat/thermostat/eclipse/internal/views/HostsVmsTreeViewPart.java Mon Nov 25 20:27:27 2013 -0500 +++ b/eclipse/com.redhat.thermostat.eclipse/src/com/redhat/thermostat/eclipse/internal/views/HostsVmsTreeViewPart.java Mon Nov 18 11:48:35 2013 -0700 @@ -63,7 +63,6 @@ import com.redhat.thermostat.common.Timer.SchedulingType; import com.redhat.thermostat.common.config.ClientPreferences; import com.redhat.thermostat.eclipse.internal.Activator; -import com.redhat.thermostat.eclipse.internal.ConnectionConfiguration; import com.redhat.thermostat.eclipse.internal.controllers.ConnectDBAction; import com.redhat.thermostat.eclipse.internal.controllers.ConnectionJobListener; import com.redhat.thermostat.eclipse.internal.jobs.ConnectDbJob; @@ -99,11 +98,8 @@ public HostsVmsTreeViewPart() { ClientPreferences clientPrefs = new ClientPreferences(Activator.getDefault().getKeyring(), Activator.getDefault().getCommonPaths()); - ConnectionConfiguration configuration = new ConnectionConfiguration( - clientPrefs.getUserName(), clientPrefs.getPassword(), - clientPrefs.getConnectionUrl()); Job connectJob = new ConnectDbJob( - "Connecting to Thermostat storage...", configuration); + "Connecting to Thermostat storage...", clientPrefs); connectJob.setSystem(true); connectAction = new ConnectDBAction(connectJob); connectAction.setImageDescriptor(Activator
--- a/integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/WebAppTest.java Mon Nov 25 20:27:27 2013 -0500 +++ b/integration-tests/itest-run/src/test/java/com/redhat/thermostat/itest/WebAppTest.java Mon Nov 18 11:48:35 2013 -0700 @@ -302,7 +302,7 @@ */ private static BackingStorage getAndConnectBackingStorage() { String url = "mongodb://127.0.0.1:27518"; - StartupConfiguration config = new ConnectionConfiguration(url, "", ""); + StartupConfiguration config = new ConnectionConfiguration(url, "", new char[]{}); SSLConfiguration sslConfig = new SSLConfiguration() { @Override @@ -357,7 +357,7 @@ ConnectionListener listener) throws IOException { setupJAASForUser(roleNames, username, password); String url = "http://localhost:" + port + "/thermostat/storage"; - StartupConfiguration config = new ConnectionConfiguration(url, username, password); + StartupConfiguration config = new ConnectionConfiguration(url, username, password.toCharArray()); SSLConfiguration sslConf = new SSLConfigurationImpl(new CommonPathsImpl()); Storage storage = new WebStorage(config, sslConf); if (listener != null) {
--- a/keyring/Makefile Mon Nov 25 20:27:27 2013 -0500 +++ b/keyring/Makefile Mon Nov 18 11:48:35 2013 -0700 @@ -18,7 +18,7 @@ MYLDFLAGS += `pkg-config --libs gnome-keyring-1` .PHONY: -JNI_LIST = com.redhat.thermostat.utils.keyring.GnomeKeyringLibraryNative +JNI_LIST = com.redhat.thermostat.utils.keyring.impl.KeyringImpl $(JNI_LIST): $(JAVAH) -force -classpath $(CLASSPATH) -d $(TARGET_DIR) $(JNI_LIST)
--- a/keyring/pom.xml Mon Nov 25 20:27:27 2013 -0500 +++ b/keyring/pom.xml Mon Nov 18 11:48:35 2013 -0700 @@ -21,7 +21,10 @@ <instructions> <Bundle-Activator>com.redhat.thermostat.utils.keyring.activator.Activator</Bundle-Activator> <Bundle-SymbolicName>com.redhat.thermostat.keyring</Bundle-SymbolicName> - <Private-Package>com.redhat.thermostat.utils.keyring.activator</Private-Package> + <Private-Package> + com.redhat.thermostat.utils.keyring.activator, + com.redhat.thermostat.utils.keyring.impl, + </Private-Package> <Export-Package>com.redhat.thermostat.utils.keyring</Export-Package> <Bundle-Vendor>Red Hat, Inc.</Bundle-Vendor> <!-- Do not autogenerate uses clauses in Manifests -->
--- a/keyring/src/main/java/com/redhat/thermostat/utils/keyring/Credentials.java Mon Nov 25 20:27:27 2013 -0500 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,115 +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.utils.keyring; - -/** - * Data used to identify and authenticate a user. - */ -public class Credentials implements Cloneable { - - private String userName; - private String password; - private String description; - - public String getDescription() { - return description; - } - - public void setDescription(String description) { - this.description = description; - } - - public String getUserName() { - return userName; - } - - public void setUserName(String userName) { - this.userName = userName; - } - - public String getPassword() { - return password; - } - - public void setPassword(String password) { - this.password = password; - } - - @Override - public int hashCode() { - final int prime = 31; - int result = 1; - result = prime * result - + ((description == null) ? 0 : description.hashCode()); - result = prime * result - + ((userName == null) ? 0 : userName.hashCode()); - return result; - } - - @Override - public boolean equals(Object obj) { - if (this == obj) - return true; - if (obj == null) - return false; - if (getClass() != obj.getClass()) - return false; - Credentials other = (Credentials) obj; - if (description == null) { - if (other.description != null) - return false; - } else if (!description.equals(other.description)) - return false; - if (userName == null) { - if (other.userName != null) - return false; - } else if (!userName.equals(other.userName)) - return false; - return true; - } - - @Override - public Credentials clone() { - Credentials cloned = new Credentials(); - cloned.userName = userName; - cloned.password = password; - cloned.description = description; - - return cloned; - } -} -
--- a/keyring/src/main/java/com/redhat/thermostat/utils/keyring/GnomeKeyringLibraryNative.java Mon Nov 25 20:27:27 2013 -0500 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,90 +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.utils.keyring; - -import com.redhat.thermostat.shared.config.NativeLibraryResolver; - -class GnomeKeyringLibraryNative implements Keyring { - - static { - String lib = NativeLibraryResolver.getAbsoluteLibraryPath("GnomeKeyringWrapper"); - System.load(lib); - } - - public GnomeKeyringLibraryNative() { - // nothing - } - - @Override - public synchronized void loadPassword(Credentials credentials) { - if (credentials == null) { - throw new NullPointerException("credentials can't be null"); - } - if (credentials.getUserName() == null) { - throw new NullPointerException("username can't be null"); - } - String password = gnomeKeyringWrapperGetPasswordNative(credentials.getUserName()); - credentials.setPassword(password); - } - - @Override - public synchronized boolean savePassword(Credentials credentials) { - - if (credentials == null) { - throw new NullPointerException("credentials can't be null"); - } - if (credentials.getUserName() == null) { - throw new NullPointerException("username can't be null"); - } - - String password = credentials.getPassword(); - if (password == null) { - password = ""; - } - - String description = credentials.getDescription(); - if (description == null) { - description = ""; - } - - return gnomeKeyringWrapperSetPasswordNative(credentials.getUserName(), password, description); - } - - private static native boolean gnomeKeyringWrapperSetPasswordNative(String userName, String password, String description); - private static native String gnomeKeyringWrapperGetPasswordNative(String userName); -} -
--- a/keyring/src/main/java/com/redhat/thermostat/utils/keyring/Keyring.java Mon Nov 25 20:27:27 2013 -0500 +++ b/keyring/src/main/java/com/redhat/thermostat/utils/keyring/Keyring.java Mon Nov 18 11:48:35 2013 -0700 @@ -39,36 +39,33 @@ import com.redhat.thermostat.annotations.Service; /** - * Manages sensitive data, like {@link Credentials}, securely. - * <p> - * An instance of this class can be obtained from OSGi as a service. + * Maps url/username to passwords, securely. Implementations may support persisting + * this mapping across sessions. */ @Service public interface Keyring { /** - * Loads the password into the given {@link Credentials}. - * - * <br /><br /> - * - * A {@link NullPointerException} is thrown is {@link Credentials} is - * {@code null} or {@link Credentials#getUserName()} is {@code null}. + * Map the given password to the given url and username. + * @param url The url for this saved password + * @param username The username for this saved password + * @param password The password to be saved */ - void loadPassword(Credentials credentials); - + public void savePassword(String url, String username, char[] password); + /** - * Stores the password from the {@link Credentials} into the keyring. - * - * <br /><br /> - * - * A {@link NullPointerException} is thrown is {@link Credentials} is - * {@code null} or {@link Credentials#getUserName()} is {@code null}. - * - * <br /><br /> - * - * If {@link Credentials#getPassword()} is {@code null}, an emtpy String - * password is saved in the keyring. + * Retrieve the password associated with the given url and username. + * @param url The url for the desired password + * @param username The username for the desired password + * @return The password mapped to the given url and username, if any. Null otherwise */ - boolean savePassword(Credentials credentials); + public char[] getPassword(String url, String username); + + /** + * Clear the password associated with the given url and username, if any. + * @param url The url for the password to be cleared + * @param username The username for the password to be cleared + */ + public void clearPassword(String url, String username); + } -
--- a/keyring/src/main/java/com/redhat/thermostat/utils/keyring/KeyringProvider.java Mon Nov 25 20:27:27 2013 -0500 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,113 +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.utils.keyring; - -import java.lang.reflect.Constructor; -import java.lang.reflect.InvocationTargetException; -import java.security.AccessController; -import java.security.PrivilegedAction; -import java.util.logging.Level; -import java.util.logging.Logger; - -public class KeyringProvider { - - // Does not depend on common-core. Use java.util.logging.Logger - private static final Logger logger = Logger.getLogger(KeyringProvider.class.getName()); - - public static final String DEFAULT_KEYRING = "com.redhat.thermostat.utils.keyring.GnomeKeyringLibraryNative"; - public static final String MEMORY_KEYRING = "com.redhat.thermostat.utils.keyring.MemoryKeyring"; - - public static final String KEYRING_FACTORY_PROPERTY = "com.redhat.thermostat.utils.keyring.provider"; - - private KeyringProvider() { /* nothing to do*/ } - - private static final Keyring keyring = getDefaultKeyring(); - - public static Keyring getKeyring() { - return keyring; - } - - static Keyring getDefaultKeyring() { - - Keyring keyring = null; - String keyringName = - AccessController.doPrivileged(new GetUserDefaultKeyringClassName()); - if (keyringName != null) { - // try to load it first - keyring = AccessController.doPrivileged(new CreateKeying(keyringName)); - } - - if (keyring == null) { - throw new InternalError("can't instantiate keyring from class: " + keyringName); - } - - return keyring; - } - - private static class CreateKeying implements PrivilegedAction<Keyring> { - private String className; - CreateKeying(String className) { - this.className = className; - } - - @SuppressWarnings({ "rawtypes", "unchecked" }) - @Override - public Keyring run() { - try { - Class keyringClass = Class.forName(className, false, getClass().getClassLoader()); - Constructor constructor = keyringClass.getDeclaredConstructor(); - constructor.setAccessible(true); - Keyring keyring = (Keyring) constructor.newInstance(); - return keyring; - - } catch (InstantiationException | IllegalAccessException | ClassNotFoundException | - IllegalArgumentException | InvocationTargetException | - NoSuchMethodException | SecurityException e) { - logger.log(Level.SEVERE, "can't create keyring for class: " + className, e); - } - return null; - } - } - - private static class GetUserDefaultKeyringClassName implements PrivilegedAction<String> { - @Override - public String run() { - return System.getProperty(KEYRING_FACTORY_PROPERTY, DEFAULT_KEYRING); - } - } -} -
--- a/keyring/src/main/java/com/redhat/thermostat/utils/keyring/MemoryKeyring.java Mon Nov 25 20:27:27 2013 -0500 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,80 +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.utils.keyring; - -import java.util.HashMap; -import java.util.Map; - -class MemoryKeyring implements Keyring { - - private Map<Credentials, String> keyring = new HashMap<>(); - - public MemoryKeyring() { - - } - - @Override - public synchronized void loadPassword(Credentials credentials) { - if (credentials == null) { - throw new NullPointerException("credentials can't be null"); - } - if (credentials.getUserName() == null) { - throw new NullPointerException("username can't be null"); - } - credentials.setPassword(keyring.get(credentials)); - } - - @Override - public synchronized boolean savePassword(Credentials credentials) { - if (credentials == null) { - throw new NullPointerException("credentials can't be null"); - } - if (credentials.getUserName() == null) { - throw new NullPointerException("username can't be null"); - } - - String password = credentials.getPassword(); - if (password == null) { - password = ""; - } - - Credentials saved = credentials.clone(); - keyring.put(saved, password); - return true; - } -} -
--- a/keyring/src/main/java/com/redhat/thermostat/utils/keyring/activator/Activator.java Mon Nov 25 20:27:27 2013 -0500 +++ b/keyring/src/main/java/com/redhat/thermostat/utils/keyring/activator/Activator.java Mon Nov 18 11:48:35 2013 -0700 @@ -40,17 +40,47 @@ import org.osgi.framework.BundleContext; import com.redhat.thermostat.utils.keyring.Keyring; -import com.redhat.thermostat.utils.keyring.KeyringProvider; +import com.redhat.thermostat.utils.keyring.impl.KeyringImpl; public class Activator implements BundleActivator { @Override public void start(BundleContext context) throws Exception { - context.registerService(Keyring.class.getName(), KeyringProvider.getKeyring(), null); + Keyring theKeyring = null; + try { + theKeyring = new KeyringImpl(); + } catch (UnsatisfiedLinkError e) { + theKeyring = new Keyring() { + /* Trivial implementation just to keep the world from blowing up. + * Everything noop. + */ + + @Override + public void savePassword(String url, String username, + char[] password) { + // NOOP + } + + @Override + public char[] getPassword(String url, String username) { + // NOOP + return new char[]{}; + } + + @Override + public void clearPassword(String url, String username) { + // NOOP + } + + }; + } + context.registerService(Keyring.class.getName(), theKeyring, null); + } @Override public void stop(BundleContext context) throws Exception { + // Nothing to do } }
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/keyring/src/main/java/com/redhat/thermostat/utils/keyring/impl/KeyringImpl.java Mon Nov 18 11:48:35 2013 -0700 @@ -0,0 +1,99 @@ +/* + * 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.utils.keyring.impl; + +import java.nio.ByteBuffer; +import java.nio.CharBuffer; +import java.nio.charset.Charset; +import java.util.Arrays; + +import com.redhat.thermostat.shared.config.NativeLibraryResolver; +import com.redhat.thermostat.utils.keyring.Keyring; + +public class KeyringImpl implements Keyring { + + private static String DESC_PREFIX = "Thermostat auth info for: "; + + static { + String lib = NativeLibraryResolver.getAbsoluteLibraryPath("GnomeKeyringWrapper"); + System.load(lib); + } + + @Override + public synchronized void savePassword(String url, String username, char[] password) { + byte[] pwBytes = null; + boolean success = false; + try { + String desc = DESC_PREFIX + username + "@" + url; + pwBytes = Charset.defaultCharset().encode(CharBuffer.wrap(password)).array(); + success = gnomeKeyringWrapperSavePasswordNative(url, username, pwBytes, desc); + } finally { + if (pwBytes != null) { + Arrays.fill(pwBytes, (byte) 0); + } + } + if (!success) { + throw new RuntimeException("Couldn't save password."); + } + } + + @Override + public synchronized char[] getPassword(String url, String username) { + byte[] pwBytes = gnomeKeyringWrapperGetPasswordNative(url, username); + char[] password = null; + if (pwBytes != null) { + try { + password = Charset.defaultCharset().decode(ByteBuffer.wrap(pwBytes)).array(); + } finally { + Arrays.fill(pwBytes, (byte) 0); + } + } + return password; + } + + @Override + public synchronized void clearPassword(String url, String username) { + if (!gnomeKeyringWrapperClearPasswordNative(url, username)) { + throw new RuntimeException("Couldn't clear password."); + } + + } + + private static native boolean gnomeKeyringWrapperSavePasswordNative(String url, String userName, byte[] password, String description); + private static native byte[] gnomeKeyringWrapperGetPasswordNative(String url, String userName); + private static native boolean gnomeKeyringWrapperClearPasswordNative(String url, String userName); +}
--- a/keyring/src/main/native/GnomeKeyringLibraryNative.c Mon Nov 25 20:27:27 2013 -0500 +++ b/keyring/src/main/native/GnomeKeyringLibraryNative.c Mon Nov 18 11:48:35 2013 -0700 @@ -34,62 +34,117 @@ * to do so, delete this exception statement from your version. */ -#include "com_redhat_thermostat_utils_keyring_GnomeKeyringLibraryNative.h" +#include "com_redhat_thermostat_utils_keyring_impl_KeyringImpl.h" #include <jni.h> #include <glib.h> #include <gnome-keyring.h> +#include <stdlib.h> +#include <string.h> + +GnomeKeyringPasswordSchema thermostat_schema = { + GNOME_KEYRING_ITEM_GENERIC_SECRET, + { + { "username", GNOME_KEYRING_ATTRIBUTE_TYPE_STRING }, + { "url", GNOME_KEYRING_ATTRIBUTE_TYPE_STRING }, + { NULL, 0 } + } +}; static void init(void) { if (g_get_application_name() == NULL) { - g_set_application_name(""); + g_set_application_name("Thermostat"); } } JNIEXPORT jboolean JNICALL -Java_com_redhat_thermostat_utils_keyring_GnomeKeyringLibraryNative_gnomeKeyringWrapperSetPasswordNative - (JNIEnv *env, jclass GnomeKeyringLibraryNativeClass, jstring juserName, jstring jpassword, jstring jdescription) +Java_com_redhat_thermostat_utils_keyring_impl_KeyringImpl_gnomeKeyringWrapperSavePasswordNative + (JNIEnv *env, jclass GnomeKeyringLibraryNativeClass, jstring jurl, jstring juserName, jbyteArray jpassword, jstring jdescription) { + int passIndex; + const char *url = (*env)->GetStringUTFChars(env, jurl, NULL); + if (url == NULL) { + return JNI_FALSE; + } const char *userName = (*env)->GetStringUTFChars(env, juserName, NULL); if (userName == NULL) { + (*env)->ReleaseStringUTFChars(env, jurl, url); return JNI_FALSE; } - const char *password = (*env)->GetStringUTFChars(env, jpassword, NULL); + jsize passwordLength = (*env)->GetArrayLength(env, jpassword); + char *password = (*env)->GetByteArrayElements(env, jpassword, NULL); if (password == NULL) { + (*env)->ReleaseStringUTFChars(env, jurl, url); (*env)->ReleaseStringUTFChars(env, juserName, userName); return JNI_FALSE; } + /* Make into null terminated (g)char * to make gnome api happy */ + gchar *gpassword = malloc(sizeof(gchar) * (passwordLength + 1)); + if (gpassword == NULL) { + (*env)->ReleaseStringUTFChars(env, jurl, url); + (*env)->ReleaseStringUTFChars(env, juserName, userName); + for (passIndex = 0; passIndex < (int) passwordLength; passIndex++) { + password[passIndex] = '\0'; + } + (*env)->ReleaseByteArrayElements(env, jpassword, password, JNI_ABORT); + return JNI_FALSE; + } + for (passIndex = 0; passIndex < passwordLength; passIndex++) { + gpassword[passIndex] = (gchar) (password[passIndex]); + } + gpassword[passwordLength] = (gchar) '\0'; + + /* Overwrite original array, release back to java-land. */ + for (passIndex = 0; passIndex < (int) passwordLength; passIndex++) { + password[passIndex] = '\0'; + } + (*env)->ReleaseByteArrayElements(env, jpassword, password, JNI_ABORT); + const char *description = (*env)->GetStringUTFChars(env, jdescription, NULL); if (description == NULL) { + (*env)->ReleaseStringUTFChars(env, jurl, url); (*env)->ReleaseStringUTFChars(env, juserName, userName); - (*env)->ReleaseStringUTFChars(env, jpassword, password); + for (passIndex = 0; passIndex < (int) passwordLength; passIndex++) { + gpassword[passIndex] = (gchar) '\0'; + } + free(gpassword); return JNI_FALSE; } init(); - GnomeKeyringResult res = gnome_keyring_store_password_sync(GNOME_KEYRING_NETWORK_PASSWORD, + GnomeKeyringResult res = gnome_keyring_store_password_sync(&thermostat_schema, GNOME_KEYRING_DEFAULT, description, - password, - "user", userName, - "server", "gnome.org", + gpassword, + "username", userName, + "url", url, NULL); - + (*env)->ReleaseStringUTFChars(env, jurl, url); (*env)->ReleaseStringUTFChars(env, juserName, userName); - (*env)->ReleaseStringUTFChars(env, jpassword, password); + for (passIndex = 0; passIndex < (int) passwordLength; passIndex++) { + gpassword[passIndex] = '\0'; + } + free(gpassword); (*env)->ReleaseStringUTFChars(env, jdescription, description); return (res == GNOME_KEYRING_RESULT_OK) ? JNI_TRUE : JNI_FALSE; } -JNIEXPORT jstring JNICALL -Java_com_redhat_thermostat_utils_keyring_GnomeKeyringLibraryNative_gnomeKeyringWrapperGetPasswordNative - (JNIEnv *env, jclass GnomeKeyringLibraryNative, jstring juserName) +JNIEXPORT jbyteArray JNICALL +Java_com_redhat_thermostat_utils_keyring_impl_KeyringImpl_gnomeKeyringWrapperGetPasswordNative + (JNIEnv *env, jclass GnomeKeyringLibraryNative, jstring jurl, jstring juserName) { + const char *url = (*env)->GetStringUTFChars(env, jurl, NULL); + if (url == NULL) { + + return NULL; + } + const char *userName = (*env)->GetStringUTFChars(env, juserName, NULL); if (userName == NULL) { + (*env)->ReleaseStringUTFChars(env, jurl, url); return NULL; } @@ -97,21 +152,48 @@ GnomeKeyringResult res; init(); - res = gnome_keyring_find_password_sync(GNOME_KEYRING_NETWORK_PASSWORD, + res = gnome_keyring_find_password_sync(&thermostat_schema, &password, - "user", userName, - "server", "gnome.org", + "username", userName, + "url", url, NULL); + (*env)->ReleaseStringUTFChars(env, jurl, url); (*env)->ReleaseStringUTFChars(env, juserName, userName); if (res == GNOME_KEYRING_RESULT_OK) { - jstring jpassword = (*env)->NewStringUTF(env, password); + jsize passwordLength = strlen(password); + jbyteArray jpassword = (*env)->NewByteArray(env, passwordLength); + (*env)->SetByteArrayRegion(env, jpassword, 0, passwordLength, password); gnome_keyring_free_password(password); return jpassword; - } else { return NULL; } } +JNIEXPORT jboolean JNICALL +Java_com_redhat_thermostat_utils_keyring_impl_KeyringImpl_gnomeKeyringWrapperClearPasswordNative + (JNIEnv *env, jclass GnomeKeyringLibraryNative, jstring jurl, jstring juserName) +{ + const char *url = (*env)->GetStringUTFChars(env, jurl, NULL); + if (url == NULL) { + return JNI_FALSE; + } + const char *userName = (*env)->GetStringUTFChars(env, juserName, NULL); + if (userName == NULL) { + (*env)->ReleaseStringUTFChars(env, jurl, url); + return JNI_FALSE; + } + + init(); + GnomeKeyringResult res = gnome_keyring_delete_password_sync(&thermostat_schema, + "username", userName, + "url", url, + NULL); + + (*env)->ReleaseStringUTFChars(env, jurl, url); + (*env)->ReleaseStringUTFChars(env, juserName, userName); + + return (res == GNOME_KEYRING_RESULT_OK) ? JNI_TRUE : JNI_FALSE; +}
--- a/keyring/src/test/java/com/redhat/thermostat/utils/keyring/KeyringProviderTest.java Mon Nov 25 20:27:27 2013 -0500 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,96 +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.utils.keyring; - -import static org.junit.Assert.*; - -import org.junit.AfterClass; -import org.junit.BeforeClass; -import org.junit.Test; - -import com.redhat.thermostat.utils.keyring.GnomeKeyringLibraryNative; -import com.redhat.thermostat.utils.keyring.Keyring; -import com.redhat.thermostat.utils.keyring.KeyringProvider; -import com.redhat.thermostat.utils.keyring.MemoryKeyring; - -public class KeyringProviderTest { - - private static String defaultKeyringProvider; - - @BeforeClass - public static void setUp() { - defaultKeyringProvider = System.getProperty(KeyringProvider.KEYRING_FACTORY_PROPERTY); - } - - @Test - public void testLoadMemoryProvider() { - System.setProperty(KeyringProvider.KEYRING_FACTORY_PROPERTY, KeyringProvider.MEMORY_KEYRING); - // setting the system property causes the keyring to load always the same class - // depending on the order of the tests, since it is a singleton, so we can't rely - // on that; we test only getDefaultKeyring, which is what initialises the keyring - Keyring keyring = KeyringProvider.getDefaultKeyring(); - assertTrue(keyring instanceof MemoryKeyring); - } - - @Test - public void testLoadDefaultProvider() { - System.setProperty(KeyringProvider.KEYRING_FACTORY_PROPERTY, KeyringProvider.DEFAULT_KEYRING); - try { - Keyring keyring = KeyringProvider.getDefaultKeyring(); - assertTrue(keyring instanceof GnomeKeyringLibraryNative); - } catch (UnsatisfiedLinkError e) { - // this is expected if the build has not been configured correctly - // since it also means that the class has been initialized - e.printStackTrace(); - } - } - - @Test(expected=InternalError.class) - public void testInvalidProvider() { - System.setProperty(KeyringProvider.KEYRING_FACTORY_PROPERTY, "exception expected!"); - Keyring keyring = KeyringProvider.getDefaultKeyring(); - fail(); - } - - @AfterClass - public static void tearDown() { - if (defaultKeyringProvider != null) { - System.setProperty(KeyringProvider.KEYRING_FACTORY_PROPERTY, defaultKeyringProvider); - } - } -} -
--- a/keyring/src/test/java/com/redhat/thermostat/utils/keyring/KeyringTest.java Mon Nov 25 20:27:27 2013 -0500 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,88 +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.utils.keyring; - -import static org.junit.Assert.*; - -import org.junit.Test; - -import com.redhat.thermostat.utils.keyring.Credentials; -import com.redhat.thermostat.utils.keyring.Keyring; -import com.redhat.thermostat.utils.keyring.MemoryKeyring; - -public class KeyringTest { - - @Test - public void testSetGetPassword() { - Credentials credentials = new Credentials(); - - String userName = System.getProperty("user.name"); - - credentials.setUserName(userName); - credentials.setPassword("fluff-password"); - credentials.setDescription("fluff"); - - Keyring keyring = new MemoryKeyring(); - assertTrue(keyring.savePassword(credentials)); - - credentials.setPassword("fluff-wrong"); - assertEquals("fluff-wrong", credentials.getPassword()); - - keyring.loadPassword(credentials); - assertEquals("fluff-password", credentials.getPassword()); - } - - @Test - public void testSetGetPasswordNullValues() { - Credentials credentials = new Credentials(); - - String userName = System.getProperty("user.name"); - - credentials.setUserName(userName); - credentials.setPassword("fluff-password"); - - Keyring keyring = new MemoryKeyring(); - assertTrue(keyring.savePassword(credentials)); - - credentials.setPassword("fluff-wrong"); - assertEquals("fluff-wrong", credentials.getPassword()); - - keyring.loadPassword(credentials); - assertEquals("fluff-password", credentials.getPassword()); - } -} -
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/keyring/src/test/java/com/redhat/thermostat/utils/keyring/impl/KeyringImplTest.java Mon Nov 18 11:48:35 2013 -0700 @@ -0,0 +1,126 @@ +/* + * 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.utils.keyring.impl; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; + +import org.junit.Test; + +public class KeyringImplTest { + + @Test + public void verifySavedPasswordIsRetrieveable() { + String url = "www.example1.com"; + String user = "mike"; + char[] pw = new char[] {'1', '2', '3'}; + KeyringImpl k = new KeyringImpl(); + k.savePassword(url, user, pw); + assertEquals(new String(pw), new String(k.getPassword(url, user))); + + // Cleanup + k.clearPassword(url, user); + } + + @Test + public void verifySavePasswordReplacesExisting() { + String url = "www.example2.com"; + String user = "mike"; + char[] pw1 = new char[] {'1', '2', '3'}; + char[] pw2 = new char[] {'4', '5', '6'}; + KeyringImpl k = new KeyringImpl(); + k.savePassword(url, user, pw1); + k.savePassword(url, user, pw2); + + assertEquals(new String(pw2), new String(k.getPassword(url, user))); + + // Cleanup + k.clearPassword(url, user); + } + + @Test + public void verifyClearedPasswordIsCleared() { + String url = "www.example3.com"; + String user = "mike"; + char[] pw = new char[] {'1', '2', '3'}; + KeyringImpl k = new KeyringImpl(); + k.savePassword(url, user, pw); + assertNotNull(k.getPassword(url, user)); + k.clearPassword(url, user); + assertNull(k.getPassword(url, user)); + } + + @Test + public void multipleServersSameUser() { + String url1 = "www.example4.com"; + String url2 = "www.fake.com"; + String user = "mike"; + char[] pw1 = new char[] {'1', '2', '3'}; + char[] pw2 = new char[] {'4', '5', '6'}; + KeyringImpl k = new KeyringImpl(); + k.savePassword(url1, user, pw1); + k.savePassword(url2, user, pw2); + + assertEquals(new String(pw1), new String(k.getPassword(url1, user))); + assertEquals(new String(pw2), new String(k.getPassword(url2, user))); + + // Cleanup + k.clearPassword(url1, user); + k.clearPassword(url2, user); + } + + @Test + public void multipleUsersSameServer() { + String url = "www.example5.com"; + String user1 = "mike"; + String user2 = "mary"; + char[] pw1 = new char[] {'1', '2', '3'}; + char[] pw2 = new char[] {'4', '5', '6'}; + KeyringImpl k = new KeyringImpl(); + k.savePassword(url, user1, pw1); + k.savePassword(url, user2, pw2); + + assertEquals(new String(pw1), new String(k.getPassword(url, user1))); + assertEquals(new String(pw2), new String(k.getPassword(url, user2))); + + // Cleanup + k.clearPassword(url, user1); + k.clearPassword(url, user2); + } + +}
--- a/launcher/src/main/java/com/redhat/thermostat/launcher/internal/LauncherImpl.java Mon Nov 25 20:27:27 2013 -0500 +++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/LauncherImpl.java Mon Nov 18 11:48:35 2013 -0700 @@ -339,13 +339,13 @@ dbUrl = prefs.getConnectionUrl(); } String username = prefs.getUserName(); - String password = prefs.getPassword(); + char[] password = prefs.getPassword(); if (username == null || password == null) { Console console = ctx.getConsole(); try { StorageAuthInfoGetter getUserPass = new StorageAuthInfoGetter(console); username = getUserPass.getUserName(dbUrl); - password = new String(getUserPass.getPassword(dbUrl)); + password = getUserPass.getPassword(dbUrl); } catch (IOException ex) { throw new CommandException(t.localize(LocaleResources.LAUNCHER_USER_AUTH_PROMPT_ERROR), ex); } @@ -362,6 +362,10 @@ String message = ( error == null ? "" : " Error: " + error ); 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'); + } } } }
--- a/launcher/src/test/java/com/redhat/thermostat/launcher/internal/LauncherImplTest.java Mon Nov 25 20:27:27 2013 -0500 +++ b/launcher/src/test/java/com/redhat/thermostat/launcher/internal/LauncherImplTest.java Mon Nov 18 11:48:35 2013 -0700 @@ -40,6 +40,7 @@ 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; @@ -89,11 +90,9 @@ import com.redhat.thermostat.test.TestTimerFactory; import com.redhat.thermostat.testutils.StubBundleContext; import com.redhat.thermostat.utils.keyring.Keyring; -import com.redhat.thermostat.utils.keyring.KeyringProvider; public class LauncherImplTest { - - private static String defaultKeyringProvider; + private static final String name1 = "test1"; private static final String name2 = "test2"; private static final String name3 = "test3"; @@ -102,7 +101,6 @@ @BeforeClass public static void beforeClassSetUp() { - defaultKeyringProvider = System.getProperty(KeyringProvider.KEYRING_FACTORY_PROPERTY); // Launcher calls System.exit(). This causes issues for unit testing. // We work around this by installing a security manager which disallows // System.exit() and throws an ExitException instead. This exception in @@ -113,9 +111,6 @@ @AfterClass public static void afterClassTearDown() { - if (defaultKeyringProvider != null) { - System.setProperty(KeyringProvider.KEYRING_FACTORY_PROPERTY, defaultKeyringProvider); - } System.setSecurityManager(secMan); } @@ -481,14 +476,14 @@ String dbUrl = "mongo://fluff:12345"; when(prefs.getConnectionUrl()).thenReturn(dbUrl); when(prefs.getUserName()).thenReturn("user"); - when(prefs.getPassword()).thenReturn("password"); + when(prefs.getPassword()).thenReturn("password".toCharArray()); LauncherImpl launcher = new LauncherImpl(bundleContext, ctxFactory, registry, infos, new CommandSource(bundleContext), environment, dbServiceFactory, version, prefs, paths, loggingInitializer); DbService dbService = mock(DbService.class); ArgumentCaptor<String> dbUrlCaptor = ArgumentCaptor.forClass(String.class); - when(dbServiceFactory.createDbService(eq("user"), eq("password"), dbUrlCaptor.capture())).thenReturn(dbService); + when(dbServiceFactory.createDbService(eq("user"), eq("password".toCharArray()), dbUrlCaptor.capture())).thenReturn(dbService); wrappedRun(launcher, new String[] { "test3" }, false); verify(dbService).connect(); verify(prefs).getConnectionUrl(); @@ -505,7 +500,7 @@ DbService dbService = mock(DbService.class); ArgumentCaptor<String> dbUrlCaptor = ArgumentCaptor.forClass(String.class); - when(dbServiceFactory.createDbService(eq("user"), eq("pass"), dbUrlCaptor.capture())).thenReturn(dbService); + when(dbServiceFactory.createDbService(eq("user"), eq("pass".toCharArray()), dbUrlCaptor.capture())).thenReturn(dbService); ctxFactory.setInput("user\rpass\r"); wrappedRun(launcher, new String[] { "test3" }, false); verify(dbService).connect(); @@ -515,6 +510,16 @@ @Test public void verifyDbServiceConnectIsCalledForStorageCommand() throws Exception { + ClientPreferences prefs = mock(ClientPreferences.class); + String dbUrl = "mongo://fluff:12345"; + String user = "user"; + char[] password = new char[] {'1', '2', '3', '4', '5'}; + when(prefs.getConnectionUrl()).thenReturn(dbUrl); + when(prefs.getUserName()).thenReturn(user); + when(prefs.getPassword()).thenReturn(password); + LauncherImpl launcher = new LauncherImpl(bundleContext, ctxFactory, registry, infos, new CommandSource(bundleContext), + environment, dbServiceFactory, version, prefs, paths, loggingInitializer); + Command mockCmd = mock(Command.class); when(mockCmd.isStorageRequired()).thenReturn(true); @@ -527,7 +532,7 @@ when(infos.getCommandInfo("dummy")).thenReturn(cmdInfo); DbService dbService = mock(DbService.class); - when(dbServiceFactory.createDbService(anyString(), anyString(), anyString())).thenReturn(dbService); + when(dbServiceFactory.createDbService(anyString(), isA(char[].class), anyString())).thenReturn(dbService); wrappedRun(launcher, new String[] { "dummy" }, false); verify(dbService).connect();
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/config/AuthenticationConfiguration.java Mon Nov 25 20:27:27 2013 -0500 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/config/AuthenticationConfiguration.java Mon Nov 18 11:48:35 2013 -0700 @@ -39,6 +39,6 @@ public interface AuthenticationConfiguration { String getUsername(); - String getPassword(); + char[] getPassword(); }
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/config/ConnectionConfiguration.java Mon Nov 25 20:27:27 2013 -0500 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/config/ConnectionConfiguration.java Mon Nov 18 11:48:35 2013 -0700 @@ -40,9 +40,9 @@ private String dbUrl; private String username; - private String password; + private char[] password; - public ConnectionConfiguration(String dbUrl, String username, String password) { + public ConnectionConfiguration(String dbUrl, String username, char[] password) { this.dbUrl = dbUrl; this.username = username; this.password = password; @@ -59,7 +59,7 @@ } @Override - public String getPassword() { + public char[] getPassword() { return password; } }
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/core/DbServiceFactory.java Mon Nov 25 20:27:27 2013 -0500 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/core/DbServiceFactory.java Mon Nov 18 11:48:35 2013 -0700 @@ -75,7 +75,7 @@ * If no matching {@link StorageProvider} could be found for the * given dbUrl. */ - public DbService createDbService(String username, String password, + public DbService createDbService(String username, char[] password, String dbUrl) throws StorageException { return DbServiceImpl.create(username, password, dbUrl); }
--- a/storage/core/src/main/java/com/redhat/thermostat/storage/internal/DbServiceImpl.java Mon Nov 25 20:27:27 2013 -0500 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/internal/DbServiceImpl.java Mon Nov 18 11:48:35 2013 -0700 @@ -71,13 +71,13 @@ private String dbUrl; private static ServiceReference sslConfRef; - DbServiceImpl(String username, String password, String dbUrl) throws StorageException { + DbServiceImpl(String username, char[] password, String dbUrl) throws StorageException { BundleContext context = FrameworkUtil.getBundle(DbService.class).getBundleContext(); init(context, username, password, dbUrl); } // for testing - DbServiceImpl(BundleContext context, String username, String password, String dbUrl) { + DbServiceImpl(BundleContext context, String username, char[] password, String dbUrl) { init(context, username, password, dbUrl); } @@ -87,7 +87,7 @@ this.storage = storage; } - private void init(BundleContext context, String username, String password, String dbUrl) { + private void init(BundleContext context, String username, char[] password, String dbUrl) { Storage storage = createStorage(context, username, password, dbUrl); this.storage = storage; @@ -186,7 +186,7 @@ * @return a DbService instance * @throws StorageException if no storage provider exists for the given {@code dbUrl}. */ - public static DbService create(String username, String password, String dbUrl) throws StorageException { + public static DbService create(String username, char[] password, String dbUrl) throws StorageException { return new DbServiceImpl(username, password, dbUrl); } @@ -216,7 +216,7 @@ } } - private static Storage createStorage(BundleContext context, String username, String password, String dbUrl) throws StorageException { + 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); if (prov == null) {
--- a/storage/core/src/test/java/com/redhat/thermostat/storage/internal/DbServiceImplTest.java Mon Nov 25 20:27:27 2013 -0500 +++ b/storage/core/src/test/java/com/redhat/thermostat/storage/internal/DbServiceImplTest.java Mon Nov 18 11:48:35 2013 -0700 @@ -91,7 +91,7 @@ context = new StubBundleContext(); try { - new DbServiceImpl(context, "ignore", "ignore", "http://ignored.example.com"); + new DbServiceImpl(context, "ignore", "ignore".toCharArray(), "http://ignored.example.com"); fail("exception expected"); } catch (StorageException se) { assertEquals("No storage provider available", se.getMessage()); @@ -103,7 +103,7 @@ when(storageProvider.canHandleProtocol()).thenReturn(false); try { - new DbServiceImpl(context, "ignore", "ignore", "http://ignored.example.com"); + new DbServiceImpl(context, "ignore", "ignore".toCharArray(), "http://ignored.example.com"); fail("exception expected"); } catch (StorageException se) { assertEquals("No storage found for URL http://ignored.example.com", se.getMessage()); @@ -181,7 +181,7 @@ @Test public void testConnectRegistersDbService() { - DbService dbService = new DbServiceImpl(context, "ignore", "ignore", "http://ignored.example.com"); + DbService dbService = new DbServiceImpl(context, "ignore", "ignore".toCharArray(), "http://ignored.example.com"); try { dbService.connect(); @@ -200,7 +200,7 @@ @Test public void testConnectRegistersStorage() { - DbService dbService = new DbServiceImpl(context, "ignore", "ignore", "http://ignored.example.com"); + DbService dbService = new DbServiceImpl(context, "ignore", "ignore".toCharArray(), "http://ignored.example.com"); try { dbService.connect(); @@ -220,7 +220,7 @@ @SuppressWarnings("rawtypes") @Test public void testConnectEnforcesPreCond() { - DbService dbService = new DbServiceImpl(context, "ignore", "ignore", "http://ignored.example.com"); + DbService dbService = new DbServiceImpl(context, "ignore", "ignore".toCharArray(), "http://ignored.example.com"); ServiceRegistration reg = context.registerService(DbService.class, dbService, null); try { @@ -243,7 +243,7 @@ @SuppressWarnings("rawtypes") @Test public void testDisConnectEnforcesPreCond() { - DbService dbService = new DbServiceImpl(context, "ignore", "ignore", "http://ignored.example.com"); + DbService dbService = new DbServiceImpl(context, "ignore", "ignore".toCharArray(), "http://ignored.example.com"); ServiceRegistration reg = context.registerService(DbService.class, dbService, null); try { @@ -267,7 +267,7 @@ @Test public void testDisconnect() { - DbService dbService = new DbServiceImpl(context, "ignore", "ignore", "http://ignored.example.com"); + DbService dbService = new DbServiceImpl(context, "ignore", "ignore".toCharArray(), "http://ignored.example.com"); try { dbService.connect(); @@ -285,7 +285,7 @@ @Test public void testDisconnectUnregistersDbService() { - DbService dbService = new DbServiceImpl(context, "ignore", "ignore", "http://ignored.example.com"); + DbService dbService = new DbServiceImpl(context, "ignore", "ignore".toCharArray(), "http://ignored.example.com"); try { dbService.connect(); @@ -305,7 +305,7 @@ @Test public void testDisconnectUnregistersStorage() { - DbService dbService = new DbServiceImpl(context, "ignore", "ignore", "http://ignored.example.com"); + DbService dbService = new DbServiceImpl(context, "ignore", "ignore".toCharArray(), "http://ignored.example.com"); try { dbService.connect(); @@ -328,7 +328,7 @@ public void canGetStorageUrl() { String connectionURL = "http://test.example.com:8082"; - DbService dbService = new DbServiceImpl(context, "ignore", "ignore", connectionURL); + DbService dbService = new DbServiceImpl(context, "ignore", "ignore".toCharArray(), connectionURL); assertEquals(connectionURL, dbService.getConnectionUrl()); } @@ -337,7 +337,7 @@ ConnectionListener listener = mock(ConnectionListener.class); Connection connection = mock(Connection.class); when(storage.getConnection()).thenReturn(connection); - DbService dbService = new DbServiceImpl(context, "ignore", "ignore", "http://ignored.example.com"); + DbService dbService = new DbServiceImpl(context, "ignore", "ignore".toCharArray(), "http://ignored.example.com"); dbService.addConnectionListener(listener); verify(connection).addListener(listener); @@ -346,7 +346,7 @@ @Test public void testListenerGetsEvent() { ConnectingConnectionListener listener = new ConnectingConnectionListener(); - DbService dbService = new DbServiceImpl(context, "ignore", "ignore", "http://ignored.example.com"); + DbService dbService = new DbServiceImpl(context, "ignore", "ignore".toCharArray(), "http://ignored.example.com"); ConnectingConnection connection = new ConnectingConnection(); when(storage.getConnection()).thenReturn(connection); @@ -376,7 +376,7 @@ ConnectionListener listener = mock(ConnectionListener.class); Connection connection = mock(Connection.class); when(storage.getConnection()).thenReturn(connection); - DbService dbService = new DbServiceImpl(context, "ignore", "ignore", "http://ignored.example.com"); + DbService dbService = new DbServiceImpl(context, "ignore", "ignore".toCharArray(), "http://ignored.example.com"); dbService.removeConnectionListener(listener); verify(connection).removeListener(listener); }
--- a/storage/mongo/src/main/java/com/redhat/thermostat/storage/mongodb/internal/MongoConnection.java Mon Nov 25 20:27:27 2013 -0500 +++ b/storage/mongo/src/main/java/com/redhat/thermostat/storage/mongodb/internal/MongoConnection.java Mon Nov 18 11:48:35 2013 -0700 @@ -104,8 +104,8 @@ } } - private void authenticate(String username, String password) { - if (! db.authenticate(username, password.toCharArray())) { + private void authenticate(String username, char[] password) { + if (! db.authenticate(username, password)) { throw new MongoException("Invalid username/password"); } }
--- a/web/client/src/main/java/com/redhat/thermostat/web/client/internal/WebStorage.java Mon Nov 25 20:27:27 2013 -0500 +++ b/web/client/src/main/java/com/redhat/thermostat/web/client/internal/WebStorage.java Mon Nov 18 11:48:35 2013 -0700 @@ -315,7 +315,7 @@ // package private for testing DefaultHttpClient httpClient; private String username; - private String password; + private char[] password; private SecureRandom random; private WebConnection conn; @@ -374,8 +374,9 @@ // TODO: Maybe also limit to realm like 'Thermostat Realm' or such? AuthScope scope = new AuthScope(endpointURL.getHost(), endpointURL.getPort()); + // FIXME Password as string? bad. Credentials creds = new UsernamePasswordCredentials(username, - password); + new String(password)); client.getCredentialsProvider().setCredentials(scope, creds); } } @@ -589,7 +590,7 @@ this.endpoint = endpoint; } - public void setAuthConfig(String username, String password) { + public void setAuthConfig(String username, char[] password) { this.username = username; this.password = password; }
--- a/web/server/src/main/java/com/redhat/thermostat/web/server/StorageFactory.java Mon Nov 25 20:27:27 2013 -0500 +++ b/web/server/src/main/java/com/redhat/thermostat/web/server/StorageFactory.java Mon Nov 18 11:48:35 2013 -0700 @@ -52,7 +52,7 @@ // Web server is not OSGi, this factory method is workaround. static Storage getStorage(String storageClass, final String storageEndpoint, final String username, - final String password, final CommonPaths paths) { + final char[] password, final CommonPaths paths) { if (storage != null) { return storage; }
--- a/web/server/src/main/java/com/redhat/thermostat/web/server/WebStorageEndPoint.java Mon Nov 25 20:27:27 2013 -0500 +++ b/web/server/src/main/java/com/redhat/thermostat/web/server/WebStorageEndPoint.java Mon Nov 18 11:48:35 2013 -0700 @@ -216,8 +216,9 @@ String storageClass = getServletConfig().getInitParameter(STORAGE_CLASS); String storageEndpoint = getServletConfig().getInitParameter(STORAGE_ENDPOINT); String username = getServletConfig().getInitParameter(STORAGE_USERNAME); + // FIXME Password as string? bad. String password = getServletConfig().getInitParameter(STORAGE_PASSWORD); - storage = StorageFactory.getStorage(storageClass, storageEndpoint, username, password, paths); + storage = StorageFactory.getStorage(storageClass, storageEndpoint, username, password == null ? null : password.toCharArray(), paths); } String uri = req.getRequestURI(); int lastPartIdx = uri.lastIndexOf("/");