changeset 1395:2f3b449bab76

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
author Severin Gehwolf <sgehwolf@redhat.com>
date Tue, 15 Apr 2014 10:36:03 +0200
parents e6649b85a055
children 4a8b8abc4498
files config/pom.xml config/src/main/java/com/redhat/thermostat/shared/config/internal/SSLConfigurationImpl.java config/src/test/java/com/redhat/thermostat/shared/config/internal/SSLConfigurationImplTest.java config/src/test/resources/system_th_home/ssl.properties config/src/test/resources/user_th_home/ssl.properties
diffstat 5 files changed, 191 insertions(+), 18 deletions(-) [+]
line wrap: on
line diff
--- 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 @@
     </dependency>
     
     <dependency>
+      <groupId>org.mockito</groupId>
+      <artifactId>mockito-core</artifactId>
+      <scope>test</scope>
+    </dependency>
+    
+    <dependency>
       <groupId>org.osgi</groupId>
       <artifactId>org.osgi.core</artifactId>
       <scope>provided</scope>
--- 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);
+            }
         }
     }
 }
--- 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());
+    }
 }
 
--- /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
--- /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