# HG changeset patch # User Severin Gehwolf # Date 1397550963 -7200 # Node ID 2f3b449bab76f144a99bf9ce8e85734a36b27236 # Parent e6649b85a05598cd3866514311a73967934acd17 Read ssl.properties from THERMOSTAT_HOME *and* USER_THERMOSTAT_HOME. Reviewed-by: vanaltj Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2014-April/009583.html PR1744 diff -r e6649b85a055 -r 2f3b449bab76 config/pom.xml --- a/config/pom.xml Thu Apr 17 15:45:46 2014 -0400 +++ b/config/pom.xml Tue Apr 15 10:36:03 2014 +0200 @@ -46,6 +46,12 @@ + org.mockito + mockito-core + test + + + org.osgi org.osgi.core provided diff -r e6649b85a055 -r 2f3b449bab76 config/src/main/java/com/redhat/thermostat/shared/config/internal/SSLConfigurationImpl.java --- a/config/src/main/java/com/redhat/thermostat/shared/config/internal/SSLConfigurationImpl.java Thu Apr 17 15:45:46 2014 -0400 +++ b/config/src/main/java/com/redhat/thermostat/shared/config/internal/SSLConfigurationImpl.java Tue Apr 15 10:36:03 2014 +0200 @@ -50,7 +50,8 @@ public class SSLConfigurationImpl implements SSLConfiguration { private CommonPaths paths; - private Properties clientProps = null; + private Properties configProps = null; + private static final String SSL_PROPS_FILENAME = "ssl.properties"; private static final String KEYSTORE_FILE_KEY = "KEYSTORE_FILE"; private static final String KEYSTORE_FILE_PWD_KEY = "KEYSTORE_PASSWORD"; private static final String CMD_CHANNEL_SSL_KEY = "COMMAND_CHANNEL_USE_SSL"; @@ -65,13 +66,13 @@ @Override public File getKeystoreFile() { try { - loadClientProperties(); + loadProperties(); } catch (InvalidConfigurationException e) { // Thermostat home not set? Should have failed earlier. Do something // reasonable. return null; } - String path = clientProps.getProperty(KEYSTORE_FILE_KEY); + String path = configProps.getProperty(KEYSTORE_FILE_KEY); if (path != null) { File file = new File(path); return file; @@ -82,12 +83,12 @@ @Override public String getKeyStorePassword() { try { - loadClientProperties(); + loadProperties(); } catch (InvalidConfigurationException e) { // Thermostat home not set? Do something reasonable return null; } - String pwd = clientProps.getProperty(KEYSTORE_FILE_PWD_KEY); + String pwd = configProps.getProperty(KEYSTORE_FILE_PWD_KEY); return pwd; } @@ -107,10 +108,10 @@ } // testing hook - void initClientProperties(File clientPropertiesFile) { - clientProps = new Properties(); + void initProperties(File clientPropertiesFile) { + configProps = new Properties(); try { - clientProps.load(new FileInputStream(clientPropertiesFile)); + configProps.load(new FileInputStream(clientPropertiesFile)); } catch (IOException | IllegalArgumentException e) { // Could not load ssl properties file. This is fine as it's // an optional config. @@ -120,26 +121,36 @@ private boolean readBooleanProperty(final String property) { boolean result = false; try { - loadClientProperties(); + loadProperties(); } catch (InvalidConfigurationException e) { logger.log(Level.WARNING, "THERMOSTAT_HOME not set and config file attempted to be " + "read from there! Returning false."); return result; } - String token = clientProps.getProperty(property); + String token = configProps.getProperty(property); if (token != null) { result = Boolean.parseBoolean(token); } return result; } - private void loadClientProperties() + // package-private for testing. + void loadProperties() throws InvalidConfigurationException { - if (clientProps == null) { - File clientPropertiesFile = new File(paths.getUserConfigurationDirectory(), - "ssl.properties"); - initClientProperties(clientPropertiesFile); + if (configProps == null) { + File userPropertiesFile = new File(paths.getUserConfigurationDirectory(), + SSL_PROPS_FILENAME); + File systemPropertiesFile = new File(paths.getSystemConfigurationDirectory(), + SSL_PROPS_FILENAME); + if (userPropertiesFile.exists()) { + // user props overrides system file + initProperties(userPropertiesFile); + } else { + // user props does not exist, use system properties file + // (if any) + initProperties(systemPropertiesFile); + } } } } diff -r e6649b85a055 -r 2f3b449bab76 config/src/test/java/com/redhat/thermostat/shared/config/internal/SSLConfigurationImplTest.java --- a/config/src/test/java/com/redhat/thermostat/shared/config/internal/SSLConfigurationImplTest.java Thu Apr 17 15:45:46 2014 -0400 +++ b/config/src/test/java/com/redhat/thermostat/shared/config/internal/SSLConfigurationImplTest.java Tue Apr 15 10:36:03 2014 +0200 @@ -38,13 +38,18 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.mockito.Mockito.verify; import java.io.File; import org.junit.Before; import org.junit.Test; +import com.redhat.thermostat.shared.config.CommonPaths; import com.redhat.thermostat.shared.config.internal.SSLConfigurationImpl; public class SSLConfigurationImplTest { @@ -55,7 +60,7 @@ public void setUp() { sslConf = new SSLConfigurationImpl(null); File clientProps = new File(this.getClass().getResource("/client.properties").getFile()); - sslConf.initClientProperties(clientProps); + sslConf.initProperties(clientProps); } @Test @@ -70,7 +75,7 @@ public void notExistingPropertiesFileReturnsNull() throws Exception { SSLConfigurationImpl badSSLConf = new SSLConfigurationImpl(null); File clientProps = new File("i/am/not/there/file.txt"); - badSSLConf.initClientProperties(clientProps); + badSSLConf.initProperties(clientProps); assertTrue(badSSLConf.getKeystoreFile() == null); assertEquals(null, badSSLConf.getKeyStorePassword()); } @@ -82,10 +87,149 @@ assertTrue(sslConf.disableHostnameVerification()); File disabledSSLProps = new File(this.getClass().getResource("/ssl.properties").getFile()); SSLConfigurationImpl disabledSSLConf = new SSLConfigurationImpl(null); - disabledSSLConf.initClientProperties(disabledSSLProps); + disabledSSLConf.initProperties(disabledSSLProps); assertFalse(disabledSSLConf.enableForCmdChannel()); assertFalse(disabledSSLConf.enableForBackingStorage()); assertFalse(disabledSSLConf.disableHostnameVerification()); } + + /* + * $THERMOSTAT_HOME/etc/ssl.properties is specified, + * $USER_THERMOSTAT_HOME/etc/ssl.properties not specified. + * + * Thus, system ssl.properties should get used. + */ + @Test + public void canInitFromSystemHomeConfig() { + File systemEtc = new File(this.getClass().getResource("/system_th_home").getFile()); + CommonPaths paths = mock(CommonPaths.class); + when(paths.getSystemConfigurationDirectory()).thenReturn(systemEtc); + File userEtc = new File("/thermostat/not-existing-foo"); + + // assert preconditions + assertTrue("ssl.properties in system home expected to exist", + new File(systemEtc, "ssl.properties").exists()); + assertFalse("ssl.properties in user home must not exist", + new File(userEtc, "ssl.properties").exists()); + + SSLConfigurationImpl config = new SSLConfigurationImpl(paths); + // This should use system ssl.properties and use values defined there + config.loadProperties(); + + // Both config location should have been checked. + verify(paths).getSystemConfigurationDirectory(); + verify(paths).getUserConfigurationDirectory(); + + // use this assertion in order to avoid false positives if loading of + // ssl.properties did not work, but boolean matches default values. + assertEquals("system thermostat home", config.getKeyStorePassword()); + assertTrue(config.enableForBackingStorage()); + assertTrue(config.disableHostnameVerification()); + } + + /* + * $THERMOSTAT_HOME/etc/ssl.properties is specified, + * $USER_THERMOSTAT_HOME/etc/ssl.properties also specified. + * + * Thus, user ssl.properties should get used. + */ + @Test + public void userHomeConfigOverridesSystem() { + File systemEtc = new File(this.getClass().getResource("/system_th_home").getFile()); + CommonPaths paths = mock(CommonPaths.class); + when(paths.getSystemConfigurationDirectory()).thenReturn(systemEtc); + File userEtc = new File(this.getClass().getResource("/user_th_home").getFile()); + when(paths.getUserConfigurationDirectory()).thenReturn(userEtc); + + // assert preconditions + assertTrue("ssl.properties in system home expected to exist", + new File(systemEtc, "ssl.properties").exists()); + assertTrue("ssl.properties in user home expected to exist", + new File(userEtc, "ssl.properties").exists()); + + SSLConfigurationImpl config = new SSLConfigurationImpl(paths); + // This should use system ssl.properties and use values defined there + config.loadProperties(); + + // Both config location should have been checked. + verify(paths).getSystemConfigurationDirectory(); + verify(paths).getUserConfigurationDirectory(); + + // use this assertion in order to avoid false positives if loading of + // ssl.properties did not work, but boolean matches default values. + assertEquals("user thermostat home", config.getKeyStorePassword()); + assertFalse(config.enableForBackingStorage()); + assertFalse(config.disableHostnameVerification()); + } + + /* + * $THERMOSTAT_HOME/etc/ssl.properties is missing, + * $USER_THERMOSTAT_HOME/etc/ssl.properties is specified. + * + * Thus, user ssl.properties should get used and it should not fail in some + * weird way since system ssl.properties is not existing. + */ + @Test + public void userHomeConfigCanBeUsedWithSystemMissing() { + File systemEtc = new File("/thermostat/not-existing-foo"); + CommonPaths paths = mock(CommonPaths.class); + when(paths.getSystemConfigurationDirectory()).thenReturn(systemEtc); + File userEtc = new File(this.getClass().getResource("/user_th_home").getFile()); + when(paths.getUserConfigurationDirectory()).thenReturn(userEtc); + + // assert preconditions + assertFalse("system ssl.properties does not exist", + new File(systemEtc, "ssl.properties").exists()); + assertTrue("ssl.properties in user home exists", + new File(userEtc, "ssl.properties").exists()); + + SSLConfigurationImpl config = new SSLConfigurationImpl(paths); + // This should use system ssl.properties and use values defined there + config.loadProperties(); + + // Both config location should have been checked. + verify(paths).getSystemConfigurationDirectory(); + verify(paths).getUserConfigurationDirectory(); + + // use this assertion in order to avoid false positives if loading of + // ssl.properties did not work, but boolean matches default values. + assertEquals("user thermostat home", config.getKeyStorePassword()); + assertFalse(config.enableForBackingStorage()); + assertFalse(config.disableHostnameVerification()); + } + + /* + * Neither ssl.properties file exist. It is expected to initialize and + * provide reasonable default values. + */ + @Test + public void noSSLConfigProvidesReasonableDefaults() { + File systemEtc = new File("/thermostat-home-system/not-existing-foo"); + CommonPaths paths = mock(CommonPaths.class); + when(paths.getSystemConfigurationDirectory()).thenReturn(systemEtc); + File userEtc = new File("/thermostat-home-user/not-existing-bar"); + when(paths.getUserConfigurationDirectory()).thenReturn(userEtc); + + // assert preconditions + assertFalse("system ssl.properties does not exist", + new File(systemEtc, "ssl.properties").exists()); + assertFalse("user ssl.properties does not exist", + new File(userEtc, "ssl.properties").exists()); + + SSLConfigurationImpl config = new SSLConfigurationImpl(paths); + // This should use system ssl.properties and use values defined there + config.loadProperties(); + + // Both config location should have been checked. + verify(paths).getSystemConfigurationDirectory(); + verify(paths).getUserConfigurationDirectory(); + + // assert default values + assertNull(config.getKeyStorePassword()); + assertNull(config.getKeystoreFile()); + assertFalse(config.enableForBackingStorage()); + assertFalse(config.enableForCmdChannel()); + assertFalse(config.disableHostnameVerification()); + } } diff -r e6649b85a055 -r 2f3b449bab76 config/src/test/resources/system_th_home/ssl.properties --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/config/src/test/resources/system_th_home/ssl.properties Tue Apr 15 10:36:03 2014 +0200 @@ -0,0 +1,6 @@ +# used for system vs. user thermostat home config of ssl.properties testing +KEYSTORE_FILE=/path/to/system_thermostat.keystore +KEYSTORE_PASSWORD=system thermostat home +COMMAND_CHANNEL_USE_SSL=true +BACKING_STORAGE_CONNECTION_USE_SSL=true +DISABLE_HOSTNAME_VERIFICATION=true \ No newline at end of file diff -r e6649b85a055 -r 2f3b449bab76 config/src/test/resources/user_th_home/ssl.properties --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/config/src/test/resources/user_th_home/ssl.properties Tue Apr 15 10:36:03 2014 +0200 @@ -0,0 +1,6 @@ +# used for system vs. user thermostat home config of ssl.properties testing +KEYSTORE_FILE=/path/to/user_thermostat.keystore +KEYSTORE_PASSWORD=user thermostat home +COMMAND_CHANNEL_USE_SSL=false +BACKING_STORAGE_CONNECTION_USE_SSL=false +DISABLE_HOSTNAME_VERIFICATION=false \ No newline at end of file