Mercurial > hg > release > thermostat-1.6
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"); - } - - } - }