changeset 1704:512f81ecb539

Read users/roles from USER_THERMOSTAT_HOME as fallback PR 2463 Reviewed-by: jerboaa Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2015-May/013878.html Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2015-June/013906.html
author Omair Majid <omajid@redhat.com>
date Wed, 03 Jun 2015 10:03:08 -0400
parents b0e56da7d27f
children 4cf9650b8bb1
files web/server/src/main/java/com/redhat/thermostat/web/server/ConfigurationFinder.java web/server/src/main/java/com/redhat/thermostat/web/server/WebStorageEndPoint.java web/server/src/main/java/com/redhat/thermostat/web/server/auth/spi/PropertiesUserValidator.java web/server/src/main/java/com/redhat/thermostat/web/server/auth/spi/RolesAmender.java web/server/src/test/java/com/redhat/thermostat/web/server/ConfigurationFinderTest.java web/server/src/test/java/com/redhat/thermostat/web/server/WebStorageEndPointUnitTest.java
diffstat 6 files changed, 263 insertions(+), 172 deletions(-) [+]
line wrap: on
line diff
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/web/server/src/main/java/com/redhat/thermostat/web/server/ConfigurationFinder.java	Wed Jun 03 10:03:08 2015 -0400
@@ -0,0 +1,72 @@
+/*
+ * Copyright 2012-2015 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.web.server;
+
+import java.io.File;
+
+import com.redhat.thermostat.shared.config.CommonPaths;
+
+public class ConfigurationFinder {
+
+    private final CommonPaths paths;
+
+    public ConfigurationFinder(CommonPaths paths) {
+        this.paths = paths;
+    }
+
+    /** Finds the best configuration file, accessible to the process */
+    public File getConfiguration(String name) {
+        File systemFile = getConfigurationFile(paths.getSystemConfigurationDirectory(), name);
+        if (isUsable(systemFile)) {
+            return systemFile;
+        }
+        File userFile = getConfigurationFile(paths.getUserConfigurationDirectory(), name);
+        if (isUsable(userFile)) {
+            return userFile;
+        }
+        return null;
+    }
+
+    /** package-private for testing only */
+    File getConfigurationFile(File directory, String name) {
+        return new File(directory, name);
+    }
+
+    private boolean isUsable(File file) {
+        return file.isFile() && file.canRead();
+    }
+}
--- a/web/server/src/main/java/com/redhat/thermostat/web/server/WebStorageEndPoint.java	Thu Jun 18 18:13:42 2015 +0200
+++ b/web/server/src/main/java/com/redhat/thermostat/web/server/WebStorageEndPoint.java	Wed Jun 03 10:03:08 2015 -0400
@@ -148,6 +148,7 @@
     private Storage storage;
     private Gson gson;
     private CommonPaths paths;
+    private ConfigurationFinder finder;
 
     public static final String STORAGE_ENDPOINT = "storage.endpoint";
     public static final String STORAGE_CLASS = "storage.class";
@@ -167,8 +168,10 @@
     }
     
     // Package private for testing
-    WebStorageEndPoint(TimerRegistry timerRegistry) {
+    WebStorageEndPoint(TimerRegistry timerRegistry, CommonPaths paths, ConfigurationFinder finder) {
         this.timerRegistry = timerRegistry;
+        this.paths = paths;
+        this.finder = finder;
     }
 
     @Override
@@ -244,7 +247,7 @@
     @Override
     protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException, ServletException {
         if (storage == null) {
-            StorageCredentials creds = getStorageCredentials(paths);
+            StorageCredentials creds = getStorageCredentials();
             // if creds are null there is no point to continue, fail prominently.
             if (creds == null) {
                 String errorMsg = "No backing storage credentials file (" + CREDENTIALS_FILE + ") available";
@@ -283,44 +286,41 @@
     }
     
     // package private for testing
-    StorageCredentials getStorageCredentials(CommonPaths commonPaths) throws IOException {
-        File systemFile = getStorageCredentialsFile(commonPaths.getSystemConfigurationDirectory());
-        if (systemFile.exists() && systemFile.canRead()) {
-            logger.log(Level.CONFIG, "Loading authentication data from " + systemFile);
-            return createStorageCredentials(systemFile);
+    StorageCredentials getStorageCredentials() throws IOException {
+        File credentialsFile = finder.getConfiguration(CREDENTIALS_FILE);
+        if (credentialsFile != null) {
+            logger.log(Level.CONFIG, "Loading authentication data from " + credentialsFile);
+            return createStorageCredentials(credentialsFile);
         } else {
-            File userCredentials = getStorageCredentialsFile(commonPaths.getUserConfigurationDirectory());
-            logger.log(Level.CONFIG, "Loading authentication data from " + userCredentials);
-            if (userCredentials.isFile() && userCredentials.canRead()) {
-                return createStorageCredentials(userCredentials);
-            } else {
-                logger.warning("Unable to read database credentials from " + userCredentials);
-                return null;
-            }
+            logger.warning("Unable to read database credentials.");
+            return null;
         }
     }
     
     // package private for testing
-    File getStorageCredentialsFile(File parent) {
-        return new File(parent, CREDENTIALS_FILE);
-    }
-    
-    // package private for testing
     StorageCredentials createStorageCredentials(File underlyingFile) {
         return new FileBasedStorageCredentials(underlyingFile);
     }
 
     // Side effect: sets this.paths
     private void sanityCheckNecessaryFiles() {
-        try {
-            // Throws config exception if basic sanity checks for
-            // THERMOSTAT_HOME don't pass.
-            paths = new CommonPathsImpl();
-        } catch (InvalidConfigurationException e) {
-            logger.log(Level.SEVERE, e.getMessage());
-            throw new RuntimeException(e);
+        if (paths == null) { // for unit tests, we inject this instance
+            try {
+                // Throws config exception if basic sanity checks for
+                // THERMOSTAT_HOME don't pass.
+                paths = new CommonPathsImpl();
+            } catch (InvalidConfigurationException e) {
+                logger.log(Level.SEVERE, e.getMessage());
+                throw new RuntimeException(e);
+            }
         }
+
+        if (finder == null) { // for unit tests, we inject this instance
+            finder = new ConfigurationFinder(paths);
+        }
+
         File thermostatHomeFile = getThermostatHome();
+
         String notReadableMsg = " is not readable or does not exist!";
         // we need to be able to read ssl config for backing storage
         // paths got set in isThermostatHomeSet()
@@ -331,7 +331,7 @@
             logger.log(Level.SEVERE, msg);
             throw new RuntimeException(msg);
         }
-        // Thermost home looks OK and seems usable
+        // Thermostat home looks OK and seems usable
         logger.log(Level.FINEST, "THERMOSTAT_HOME == "
                 + thermostatHomeFile.getAbsolutePath());
     }
--- a/web/server/src/main/java/com/redhat/thermostat/web/server/auth/spi/PropertiesUserValidator.java	Thu Jun 18 18:13:42 2015 +0200
+++ b/web/server/src/main/java/com/redhat/thermostat/web/server/auth/spi/PropertiesUserValidator.java	Wed Jun 03 10:03:08 2015 -0400
@@ -48,6 +48,7 @@
 import com.redhat.thermostat.common.utils.LoggingUtils;
 import com.redhat.thermostat.shared.config.InvalidConfigurationException;
 import com.redhat.thermostat.shared.config.internal.CommonPathsImpl;
+import com.redhat.thermostat.web.server.ConfigurationFinder;
 
 /**
  * 
@@ -63,11 +64,15 @@
     PropertiesUserValidator() {
         // this is the default configuration. it should be overriden through different means
         // see javadoc of PropertiesUsernameRolesLoginModule
-        this((new CommonPathsImpl().getSystemConfigurationDirectory() + File.separator + DEFAULT_USERS_FILE));
+        this(new ConfigurationFinder(new CommonPathsImpl()).getConfiguration(DEFAULT_USERS_FILE));
     }
     
     PropertiesUserValidator(String usersFile) {
-        loadUsers(new File(usersFile));
+        this(new File(usersFile));
+    }
+
+    PropertiesUserValidator(File file) {
+        loadUsers(file);
     }
 
     @Override
--- a/web/server/src/main/java/com/redhat/thermostat/web/server/auth/spi/RolesAmender.java	Thu Jun 18 18:13:42 2015 +0200
+++ b/web/server/src/main/java/com/redhat/thermostat/web/server/auth/spi/RolesAmender.java	Wed Jun 03 10:03:08 2015 -0400
@@ -53,6 +53,7 @@
 import com.redhat.thermostat.common.utils.LoggingUtils;
 import com.redhat.thermostat.shared.config.InvalidConfigurationException;
 import com.redhat.thermostat.shared.config.internal.CommonPathsImpl;
+import com.redhat.thermostat.web.server.ConfigurationFinder;
 import com.redhat.thermostat.web.server.auth.BasicRole;
 import com.redhat.thermostat.web.server.auth.RolePrincipal;
 
@@ -75,12 +76,16 @@
         // this is the default configuration supplied with thermostat
         // it should not be overriden by editing this configuraton file
         // see javadocs of PropertiesUsernameRolesLoginModule
-        this((new CommonPathsImpl().getSystemConfigurationDirectory() + File.separator + DEFAULT_ROLES_FILE), users);
+        this(new ConfigurationFinder(new CommonPathsImpl()).getConfiguration(DEFAULT_ROLES_FILE), users);
     }
     
     RolesAmender(String rolesFile, Set<Object> users) {
+        this(new File(rolesFile), users);
+    }
+
+    RolesAmender(File rolesFile, Set<Object> users) {
         this.users = Objects.requireNonNull(users);
-        loadRoles(new File(rolesFile));
+        loadRoles(rolesFile);
     }
     
     /**
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/web/server/src/test/java/com/redhat/thermostat/web/server/ConfigurationFinderTest.java	Wed Jun 03 10:03:08 2015 -0400
@@ -0,0 +1,139 @@
+/*
+ * Copyright 2012-2015 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.web.server;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import com.redhat.thermostat.shared.config.CommonPaths;
+
+public class ConfigurationFinderTest {
+
+    private File systemConfigDir = mock(File.class);
+    private File userConfigDir = mock(File.class);
+
+    private File realFile1;
+    private File realFile2;
+
+    private CommonPaths paths;
+
+    @Before
+    public void setUp() throws IOException {
+        realFile1 = Files.createTempFile("configfinder-unit-test", null).toFile();
+        realFile1.createNewFile();
+
+        realFile2 = Files.createTempFile("configfinder-unit-test", null).toFile();
+        realFile2.createNewFile();
+
+        paths = mock(CommonPaths.class);
+        when(paths.getSystemConfigurationDirectory()).thenReturn(systemConfigDir);
+        when(paths.getUserConfigurationDirectory()).thenReturn(userConfigDir);
+    }
+
+    @After
+    public void tearDown() {
+        realFile1.delete();
+        realFile2.delete();
+    }
+
+    @Test
+    public void verifyFileFromUserHomeIsUsedIfSystemHomeIsNotUsable() throws Exception {
+        ConfigurationFinder finder = new ConfigurationFinder(paths) {
+            @Override
+            File getConfigurationFile(File directory, String name) {
+                if (directory == systemConfigDir) {
+                    return new File("/does-not-exist/really-it-doesn't");
+                } else if (directory == userConfigDir) {
+                    return realFile1;
+                }
+                throw new AssertionError("Unknown test case");
+            };
+        };
+
+        File config = finder.getConfiguration("web.auth");
+        assertEquals(realFile1, config);
+    }
+
+    @Test
+    public void verifyFileFromSystemHomeIsUsedIfBothAreUsable() throws IOException {
+        final File systemFile = realFile1;
+        final File userFile = realFile2;
+
+        ConfigurationFinder finder = new ConfigurationFinder(paths) {
+            @Override
+            File getConfigurationFile(File directory, String name) {
+                if (directory == systemConfigDir) {
+                    return systemFile;
+                } else if (directory == userConfigDir) {
+                    return userFile;
+                }
+                throw new AssertionError("Unknown test case");
+            };
+        };
+
+        File config = finder.getConfiguration("web.auth");
+        assertEquals(systemFile, config);
+    }
+
+    @Test
+    public void verifyFileFromSystemHomeIsUsedIfUserHomeNotUsable() throws IOException {
+        ConfigurationFinder finder = new ConfigurationFinder(paths) {
+            @Override
+            File getConfigurationFile(File directory, String name) {
+                if (directory == systemConfigDir) {
+                    return realFile1;
+                } else if (directory == userConfigDir) {
+                    return new File("/does-not-exist/really-it-doesn't");
+                }
+                throw new AssertionError("Unknown test case");
+            };
+        };
+
+        File config = finder.getConfiguration("web.auth");
+        assertEquals(realFile1, config);
+    }
+
+}
--- a/web/server/src/test/java/com/redhat/thermostat/web/server/WebStorageEndPointUnitTest.java	Thu Jun 18 18:13:42 2015 +0200
+++ b/web/server/src/test/java/com/redhat/thermostat/web/server/WebStorageEndPointUnitTest.java	Wed Jun 03 10:03:08 2015 -0400
@@ -248,116 +248,24 @@
     @Test
     public void testShutDownCancelsTimers() {
         TimerRegistry registry = mock(TimerRegistry.class);
-        WebStorageEndPoint endpoint = new WebStorageEndPoint(registry);
+        WebStorageEndPoint endpoint = new WebStorageEndPoint(registry, null, null);
         endpoint.destroy();
         verify(registry).shutDown();
     }
     
     /**
-     * Storage credentials should be read from USER_THERMOSTAT_HOME if
-     * relevant credentials file is not readable or does not exist in
-     * THERMOSTAT_HOME.
-     * 
-     * Only the user file is readable.
-     * 
-     * @throws IOException 
-     */
-    @Test
-    public void canGetUserStorageCredentials() throws IOException {
-        StorageCredsTestSetupResult setupResult = setupForStorageCredentialsTest(false, true);
-        WebStorageEndPoint endPoint = setupResult.endpoint;
-        CommonPaths paths = setupResult.commonPaths;
-        StorageCredentials creds = endPoint.getStorageCredentials(paths);
-        assertNotNull(creds);
-        assertTrue(creds instanceof TestStorageCredentials);
-        TestStorageCredentials testCreds = (TestStorageCredentials)creds;
-        assertTrue(setupResult.userCredsFile == testCreds.underlyingFile);
-    }
-    
-    /**
-     * If Storage credentials file exists and is readable it should
-     * get used over USER_THERMOSTAT_HOME's version. In this test *both*
-     * files are present and readable.
-     * 
-     * @throws IOException 
-     */
-    @Test
-    public void canGetSystemStorageCredentialsBothReadable() throws IOException {
-        StorageCredsTestSetupResult setupResult = setupForStorageCredentialsTest(true, true);
-        WebStorageEndPoint endPoint = setupResult.endpoint;
-        CommonPaths paths = setupResult.commonPaths;
-        StorageCredentials creds = endPoint.getStorageCredentials(paths);
-        assertNotNull(creds);
-        assertTrue(creds instanceof TestStorageCredentials);
-        TestStorageCredentials testCreds = (TestStorageCredentials)creds;
-        assertTrue(setupResult.systemCredsFile == testCreds.underlyingFile);
-    }
-    
-    /**
-     * If Storage credentials file exists and is readable it should
-     * get used over USER_THERMOSTAT_HOME's version. In this test only the
-     * system version is readable.
-     * 
-     * @throws IOException 
-     */
-    @Test
-    public void canGetSystemStorageCredentialsOnlySystemReadable() throws IOException {
-        StorageCredsTestSetupResult setupResult = setupForStorageCredentialsTest(true, false);
-        WebStorageEndPoint endPoint = setupResult.endpoint;
-        CommonPaths paths = setupResult.commonPaths;
-        StorageCredentials creds = endPoint.getStorageCredentials(paths);
-        assertNotNull(creds);
-        assertTrue(creds instanceof TestStorageCredentials);
-        TestStorageCredentials testCreds = (TestStorageCredentials)creds;
-        assertTrue(setupResult.systemCredsFile == testCreds.underlyingFile);
-    }
-    
-    private StorageCredsTestSetupResult setupForStorageCredentialsTest(boolean systemCanRead, boolean userCanRead) {
-        final File systemCredsFile = mock(File.class);
-        when(systemCredsFile.canRead()).thenReturn(systemCanRead);
-        when(systemCredsFile.exists()).thenReturn(true);
-        when(systemCredsFile.getPath()).thenReturn("system_creds_file");
-        final File systemCredsDirectory = mock(File.class);
-        final File userCredsDirectory = mock(File.class);
-        final File userCredsFile = mock(File.class);
-        when(userCredsFile.canRead()).thenReturn(userCanRead);
-        when(userCredsFile.isFile()).thenReturn(true);
-        when(userCredsFile.getPath()).thenReturn("user_creds_file");
-        CommonPaths paths = mock(CommonPaths.class);
-        when(paths.getSystemConfigurationDirectory()).thenReturn(systemCredsDirectory);
-        when(paths.getUserConfigurationDirectory()).thenReturn(userCredsDirectory);
-        @SuppressWarnings("serial")
-        WebStorageEndPoint endpoint = new WebStorageEndPoint() {
-            @Override
-            File getStorageCredentialsFile(File parent) {
-                // intentionally compare exact instances
-                if (systemCredsDirectory == parent) {
-                    return systemCredsFile;
-                }
-                // intentionally compare exact instances
-                if (userCredsDirectory == parent) {
-                    return userCredsFile;
-                }
-                return null;
-            }
-            
-            @Override
-            StorageCredentials createStorageCredentials(File underlyingFile) {
-                return new TestStorageCredentials(underlyingFile);
-            }
-        };
-        return new StorageCredsTestSetupResult(endpoint, paths, systemCredsFile, userCredsFile);
-    }
-    
-    /**
-     * If neither storage credentials exist (none in USER_THERMOSTAT_HOME *and*
-     * in THERMOSTAT_HOME) then null is expected to get returned.
-     * @throws IOException 
+     * If storage credentials are not found then null is expected to get returned.
      */
     @Test
     public void storageCredentialsNull() throws IOException {
-        StorageCredsTestSetupResult setupResult = setupForStorageCredentialsTest(false, false);
-        StorageCredentials creds = setupResult.endpoint.getStorageCredentials(setupResult.commonPaths);
+        CommonPaths paths = mock(CommonPaths.class);
+        TimerRegistry registry = mock(TimerRegistry.class);
+        ConfigurationFinder finder = mock(ConfigurationFinder.class);
+        when(finder.getConfiguration("web.auth")).thenReturn(null);
+
+        WebStorageEndPoint endpoint = new WebStorageEndPoint(registry, paths, finder);
+        StorageCredentials creds = endpoint.getStorageCredentials();
+
         assertNull(creds);
     }
     
@@ -387,42 +295,4 @@
         }
     }
     
-    private static class StorageCredsTestSetupResult {
-        private WebStorageEndPoint endpoint;
-        private CommonPaths commonPaths;
-        private File systemCredsFile;
-        private File userCredsFile;
-        
-        private StorageCredsTestSetupResult(WebStorageEndPoint endpoint,
-                                            CommonPaths commonPaths,
-                                            File systemCredsFile,
-                                            File userCredsFile) {
-            this.endpoint = endpoint;
-            this.commonPaths = commonPaths;
-            this.systemCredsFile = systemCredsFile;
-            this.userCredsFile = userCredsFile;
-        }
-        
-    }
-    
-    private static class TestStorageCredentials implements StorageCredentials {
-
-        private File underlyingFile;
-        
-        private TestStorageCredentials(File underlyingFile) {
-            this.underlyingFile = underlyingFile;
-        }
-        
-        @Override
-        public String getUsername() {
-            throw new AssertionError("not implemented");
-        }
-
-        @Override
-        public char[] getPassword() {
-            throw new AssertionError("not implemented");
-        }
-        
-    }
-    
 }