jerboaa pushed to thermostat (f22). "Read mongodb credentials from separate file. (..more)"
notifications at fedoraproject.org
notifications at fedoraproject.org
Fri May 22 15:47:48 UTC 2015
From 9d7bd12f82d5a4b269b31ccca8a82977c9d85127 Mon Sep 17 00:00:00 2001
From: Severin Gehwolf <sgehwolf at redhat.com>
Date: Mon, 18 May 2015 12:28:05 -0400
Subject: Read mongodb credentials from separate file.
Resolves: CVE-2015-3201
diff --git a/CVE-2015-3201-config-file-world-readable.patch b/CVE-2015-3201-config-file-world-readable.patch
new file mode 100644
index 0000000..caf122f
--- /dev/null
+++ b/CVE-2015-3201-config-file-world-readable.patch
@@ -0,0 +1,469 @@
+diff --git a/distribution/scripts/thermostat-setup b/distribution/scripts/thermostat-setup
+--- a/distribution/scripts/thermostat-setup
++++ b/distribution/scripts/thermostat-setup
+@@ -120,7 +120,6 @@ exitFail() {
+ # in this script already, but failed somewhere
+ # down the line.
+ removeTempStampFile
+- cleanupSedFiles
+ echo 'Thermostat setup failed!' 1>&2
+ exit 1
+ }
+@@ -137,18 +136,10 @@
+ fi
+ }
+
+-cleanupSedFiles() {
+- if [ ! -z "$SED_FILES_DIR" ] &&
+- [ -e $SED_FILES_DIR ]; then
+- rm -rf "$SED_FILES_DIR"
+- fi
+-}
+-
+ exitSuccess() {
+ # Remove temporary unlock file and create the actual setup-complete
+ # file.
+ removeTempStampFile
+- cleanupSedFiles
+ echo $SETUP_UNLOCK_CONTENT_REGULAR > "$SETUP_COMPLETE_FILE"
+ echo -e "\nThermostat setup complete!\n"
+ echo -e "Be sure to configure thermostat-users.properties and"
+@@ -206,45 +197,19 @@
+ default_webapp="$THERMOSTAT_HOME/webapp"
+ THERMOSTAT_WEBAPP_LOCATION="$(readlink -f $default_webapp)"
+ fi
+- TH_WEB_INF="$THERMOSTAT_WEBAPP_LOCATION/WEB-INF/web.xml"
++ TH_WEB_INF="$THERMOSTAT_WEBAPP_LOCATION/WEB-INF/web.auth"
+
+ if [ ! -e $TH_WEB_INF ]; then
+ echo "File not found: $TH_WEB_INF" 1>&2
+ exitFail
+ fi
+
+- # We use this var for cleaning up after sed has run and
+- # the script exits.
+- SED_FILES_DIR="$(mktemp -d)"
+- setSedExprs "storage.username" "$USERNAME"
+- writeSedFiles "$SED_FILES_DIR" "storage.username"
+- SED_CMD_USER1="sed -i -f $SED_FILE_UNCOMMENTED $TH_WEB_INF"
+- SED_CMD_USER2="sed -i -f $SED_FILE_COMMENTED $TH_WEB_INF"
+-
+- setSedExprs "storage.password" "$PASSWORD"
+- writeSedFiles "$SED_FILES_DIR" "storage.password"
+- SED_CMD_PWD1="sed -i -f $SED_FILE_UNCOMMENTED $TH_WEB_INF"
+- SED_CMD_PWD2="sed -i -f $SED_FILE_COMMENTED $TH_WEB_INF"
+-
+ if [ ! -w $TH_WEB_INF ]; then
+- local sed_one_line_user="$SED_CMD_USER1 && $SED_CMD_USER2"
+- local sed_one_line_pwd="$SED_CMD_PWD1 && $SED_CMD_PWD2"
+ echo -e "\n\nWARNING: $(readlink -f $TH_WEB_INF) is NOT writable.\n"
+- echo -e "You have the following options:\n"
+- echo -e " 1. Run the following command(s) as root:"
+- echo -e " #> $sed_one_line_user"
+- echo -e " #> $sed_one_line_pwd"
+- echo -e " #> rm -r $SED_FILES_DIR"
+- echo -e " 2. Modify the file manually and add the following"
+- echo -e " credentials config snippet (in 'servlet'):"
+- echo -e " <init-param>"
+- echo -e " <param-name>storage.username</param-name>"
+- echo -e " <param-value>$USERNAME</param-value>"
+- echo -e " </init-param>"
+- echo -e " <init-param>"
+- echo -e " <param-name>storage.password</param-name>"
+- echo -e " <param-value>$PASSWORD</param-value>"
+- echo -e " </init-param>"
++ echo -e " Modify the file manually and add the following"
++ echo -e " credentials config snippet:"
++ echo -e " storage.username = $USERNAME"
++ echo -e " storage.password = $PASSWORD"
+ echo -e " The file in which you need to put this snippet"
+ echo -e " is:"
+ echo -e " $TH_WEB_INF"
+@@ -252,9 +217,9 @@
+ else
+ # Run the sed command(s)
+ local sedSuccess=0
+- $SED_CMD_USER1 && $SED_CMD_USER2
++ echo "storage.username = $USERNAME" > "$TH_WEB_INF"
+ sedSuccess=$(( $sedSuccess + $? ))
+- $SED_CMD_PWD1 && $SED_CMD_PWD2
++ echo "storage.password = $PASSWORD" >> "$TH_WEB_INF"
+ sedSuccess=$(( $sedSuccess + $? ))
+ if [ $sedSuccess -eq 0 ]; then
+ exitSuccess
+@@ -265,109 +230,6 @@
+ fi
+ }
+
+-writeSedFiles() {
+- local tmpDir="$1"
+- local paramName="$2"
+- SED_FILE_UNCOMMENTED="$tmpDir/uncommented-${paramName}.sed"
+- SED_FILE_COMMENTED="$tmpDir/commented-${paramName}.sed"
+- echo "$SED_EXPR_COMMENTED" > $SED_FILE_COMMENTED
+- echo "$SED_EXPR_UNCOMMENTED" > $SED_FILE_UNCOMMENTED
+-}
+-
+-setSedExprs() {
+- local paramName=$1
+- local paramVal=$2
+- setSedExprUnCommented "$paramName" "$paramVal"
+- setSedExprCommented "$paramName" "$paramVal"
+-}
+-
+-setSedExprCommented() {
+- local paramName=$1
+- local paramVal=$2
+- SED_EXPR_COMMENTED="
+- # Finds pattern A and replaces it with B.
+- #
+- # Pattern A is something like the following:
+- # <!--
+- # <init-param>
+- # <param-name>storage.username</param-name>
+- # <param-value>something</param-value>
+- # </init-param>
+- # -->
+- #
+- # Replacement (B) would then be:
+- #
+- # <init-param>
+- # <param-name>storage.username</param-name>
+- # <param-value>foo-bar</param-value>
+- # </init-param>
+- #
+- # In essence it removes the comments and sets
+- # the param value we want.
+- #
+- /<[!]--/ {
+- N
+- /<init-param>/ {
+- N
+- /<param-name>$paramName<[/]param-name>/ {
+- N
+- /<param-value>.*<[/]param-value>/ {
+- N
+- /<[/]init-param>/ {
+- N
+- /-->/ {
+- N
+- # Do the substitution with all matching lines in
+- # current buffer
+- s%.*<[!]--\n.*<init-param>\n.*<param-name>$paramName<[/]param-name>\n.*<param-value>.*<[/]param-value>.*<[/]init-param>.*-->%<init-param>\n<param-name>$paramName</param-name>\n<param-value>$paramVal</param-value>\n</init-param>%
+- }
+- }
+- }
+- }
+- }
+- }"
+-}
+-
+-setSedExprUnCommented() {
+- paramName=$1
+- paramVal=$2
+- SED_EXPR_UNCOMMENTED="
+- # Finds pattern C and replaces it with D.
+- #
+- # Pattern C is something like the following:
+- # <init-param>
+- # <param-name>storage.username</param-name>
+- # <param-value>something</param-value>
+- # </init-param>
+- #
+- # Replacement (D) would then be:
+- #
+- # <init-param>
+- # <param-name>storage.username</param-name>
+- # <param-value>foo-bar</param-value>
+- # </init-param>
+- #
+- # I.e. it changes the username to what we'd
+- # like it to be. If that section is already
+- # in place (not commented out)
+- #
+- /<init-param>/ {
+- N
+- /<param-name>$paramName<[/]param-name>/ {
+- N
+- /<param-value>.*<[/]param-value>/ {
+- N
+- /<[/]init-param>/ {
+- N
+- # Do the substitution with all matching lines in
+- # current buffer
+- s%.*<init-param>\n.*<param-name>$paramName<[/]param-name>\n.*<param-value>.*<[/]param-value>.*<[/]init-param>%<init-param>\n<param-name>$paramName</param-name>\n<param-value>$paramVal</param-value>\n</init-param>%
+- }
+- }
+- }
+- }"
+-}
+-
+ readUsername() {
+ dUsername="$1"
+ prompt="Please enter the desired Mongodb username (press return in order to use '$dUsername'): "
+diff --git a/pom.xml b/pom.xml
+--- a/pom.xml
++++ b/pom.xml
+@@ -98,17 +98,11 @@
+ <mongodb.dev.password> mongodevpassword </mongodb.dev.password>
+ <!-- used in web.xml of the war artifact -->
+ <web.war.backingstorage.username.snippet>
+- <init-param>
+- <param-name>storage.username</param-name>
+- <param-value>${mongodb.dev.username}</param-value>
+- </init-param>
++storage.username=${mongodb.dev.username}
+ </web.war.backingstorage.username.snippet>
+ <!-- used in web.xml of the war artifact -->
+ <web.war.backingstorage.password.snippet>
+- <init-param>
+- <param-name>storage.password</param-name>
+- <param-value>${mongodb.dev.password}</param-value>
+- </init-param>
++storage.password=${mongodb.dev.password}
+ </web.war.backingstorage.password.snippet>
+ <!--
+ Used in thermostat-users.properties. We define two users.
+@@ -273,21 +267,11 @@
+
+ <!-- used in web.xml of the war artifact -->
+ <web.war.backingstorage.username.snippet>
+- <!--
+- <init-param>
+- <param-name>storage.username</param-name>
+- <param-value>thermostat-mongodb-user</param-value>
+- </init-param>
+- -->
++#storage.username=thermostat-mongodb-user
+ </web.war.backingstorage.username.snippet>
+ <!-- used in web.xml of the war artifact -->
+ <web.war.backingstorage.password.snippet>
+- <!--
+- <init-param>
+- <param-name>storage.password</param-name>
+- <param-value>supersecrit</param-value>
+- </init-param>
+- -->
++#storage.password=supersecrit
+ </web.war.backingstorage.password.snippet>
+ <!--
+ Used in thermostat-users.properties and
+diff --git a/web/server/src/main/java/com/redhat/thermostat/web/server/FileBasedStorageCredentials.java b/web/server/src/main/java/com/redhat/thermostat/web/server/FileBasedStorageCredentials.java
+new file mode 100644
+--- /dev/null
++++ b/web/server/src/main/java/com/redhat/thermostat/web/server/FileBasedStorageCredentials.java
+@@ -0,0 +1,89 @@
++/*
++ * 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 java.io.FileInputStream;
++import java.io.IOException;
++import java.io.InputStream;
++import java.util.Properties;
++import java.util.logging.Level;
++import java.util.logging.Logger;
++
++import com.redhat.thermostat.common.utils.LoggingUtils;
++import com.redhat.thermostat.storage.core.StorageCredentials;
++
++public class FileBasedStorageCredentials implements StorageCredentials {
++
++ private static final Logger logger = LoggingUtils.getLogger(FileBasedStorageCredentials.class);
++
++ // authentication tokens
++ public static final String STORAGE_USERNAME = "storage.username";
++ public static final String STORAGE_PASSWORD = "storage.password";
++
++ private Properties props;
++
++ public FileBasedStorageCredentials(File file) {
++ try (InputStream in = new FileInputStream(file)) {
++ initialize(in);
++ } catch (IOException e) {
++ logger.log(Level.WARNING, "Unable to read " + file);
++ }
++
++ }
++
++ public FileBasedStorageCredentials(InputStream in) throws IOException {
++ initialize(in);
++ }
++
++ private void initialize(InputStream in) throws IOException {
++ props = new Properties();
++ props.load(in);
++ }
++
++ @Override
++ public String getUsername() {
++ return props.getProperty(STORAGE_USERNAME, "");
++ }
++
++ @Override
++ public char[] getPassword() {
++ // FIXME Password as string? bad.
++ return props.getProperty(STORAGE_PASSWORD, "").toCharArray();
++ }
++
++}
+diff --git a/web/server/src/main/java/com/redhat/thermostat/web/server/WebStorageEndPoint.java b/web/server/src/main/java/com/redhat/thermostat/web/server/WebStorageEndPoint.java
+--- a/web/server/src/main/java/com/redhat/thermostat/web/server/WebStorageEndPoint.java
++++ b/web/server/src/main/java/com/redhat/thermostat/web/server/WebStorageEndPoint.java
+@@ -37,6 +37,7 @@
+ package com.redhat.thermostat.web.server;
+
+ import java.io.File;
++import java.io.FileInputStream;
+ import java.io.IOException;
+ import java.io.InputStream;
+ import java.io.OutputStream;
+@@ -148,8 +149,6 @@
+ private CommonPaths paths;
+
+ public static final String STORAGE_ENDPOINT = "storage.endpoint";
+- public static final String STORAGE_USERNAME = "storage.username";
+- public static final String STORAGE_PASSWORD = "storage.password";
+ public static final String STORAGE_CLASS = "storage.class";
+
+ // read-only set of all known statement descriptors we trust and allow
+@@ -244,24 +243,26 @@
+ @Override
+ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException, ServletException {
+ if (storage == null) {
++ StorageCredentials creds = null;
++ final String CREDENTIALS_FILE = "web.auth";
++ // this assumes this is always an exploded war
++ File systemFile = new File(getServletContext().getRealPath("/WEB-INF/" + CREDENTIALS_FILE));
++ if (systemFile.exists() && systemFile.canRead()) {
++ logger.log(Level.CONFIG, "Loading authentication data from WEB-INF/" + CREDENTIALS_FILE);
++ try (InputStream in = new FileInputStream(systemFile)) {
++ creds = new FileBasedStorageCredentials(in);
++ }
++ } else {
++ File userCredentials = new File(paths.getUserConfigurationDirectory(), CREDENTIALS_FILE);
++ logger.log(Level.CONFIG, "Loading authentication data from " + userCredentials);
++ if (userCredentials.isFile() && userCredentials.canRead()) {
++ creds = new FileBasedStorageCredentials(userCredentials);
++ } else {
++ logger.warning("Unable to read database credentials from " + userCredentials);
++ }
++ }
+ String storageClass = getServletConfig().getInitParameter(STORAGE_CLASS);
+ String storageEndpoint = getServletConfig().getInitParameter(STORAGE_ENDPOINT);
+- final String username = getServletConfig().getInitParameter(STORAGE_USERNAME);
+- // FIXME Password as string? bad.
+- final String password = getServletConfig().getInitParameter(STORAGE_PASSWORD);
+- StorageCredentials creds = new StorageCredentials() {
+-
+- @Override
+- public String getUsername() {
+- return username;
+- }
+-
+- @Override
+- public char[] getPassword() {
+- return password == null ? null : password.toCharArray();
+- }
+-
+- };
+ storage = StorageFactory.getStorage(storageClass, storageEndpoint, paths, creds);
+ }
+ String uri = req.getRequestURI();
+diff --git a/web/war/pom.xml b/web/war/pom.xml
+--- a/web/war/pom.xml
++++ b/web/war/pom.xml
+@@ -215,13 +215,13 @@
+ <goal>exploded</goal>
+ </goals>
+ <configuration>
+- <!-- web.xml contains properties, which we'd like to have interpolated -->
+ <webResources>
+ <resource>
+ <filtering>true</filtering>
+ <directory>src/main/webapp</directory>
+ <includes>
+ <include>**/web.xml</include>
++ <include>**/web.auth</include>
+ </includes>
+ </resource>
+ </webResources>
+@@ -244,6 +244,7 @@
+ <directory>src/main/webapp</directory>
+ <includes>
+ <include>**/web.xml</include>
++ <include>**/web.auth</include>
+ </includes>
+ </resource>
+ </webResources>
+diff --git a/web/war/src/main/webapp/WEB-INF/web.auth b/web/war/src/main/webapp/WEB-INF/web.auth
+new file mode 100644
+--- /dev/null
++++ b/web/war/src/main/webapp/WEB-INF/web.auth
+@@ -0,0 +1,11 @@
++# Credentials to use for connecting to the backing storage
++# (currently mongodb). Uncomment the following two blocks in
++# order to use this username/password for the connection.
++
++# Username to use for connecting to the backing storage
++# implementation
++${web.war.backingstorage.username.snippet}
++
++# Password to use for connecting to the backing storage
++# implementation
++${web.war.backingstorage.password.snippet}
+diff --git a/web/war/src/main/webapp/WEB-INF/web.xml b/web/war/src/main/webapp/WEB-INF/web.xml
+--- a/web/war/src/main/webapp/WEB-INF/web.xml
++++ b/web/war/src/main/webapp/WEB-INF/web.xml
+@@ -52,16 +52,6 @@
+ <param-name>storage.endpoint</param-name>
+ <param-value>mongodb://127.0.0.1:27518</param-value>
+ </init-param>
+- <!-- Credentials to use for connecting to the backing storage
+- (currently mongodb). Uncomment the following two blocks in
+- order to use this username/password for the connection. -->
+-
+- <!-- Username to use for connecting to the backing storage
+- implementation. -->
+- ${web.war.backingstorage.username.snippet}
+- <!-- Password to use for connecting to the backing storage
+- implementation -->
+- ${web.war.backingstorage.password.snippet}
+ <!-- The timeout of the token manager in ms -->
+ <init-param>
+ <param-name>token-manager-timeout</param-name>
diff --git a/thermostat.spec b/thermostat.spec
index 3d60b30..d0f564a 100644
--- a/thermostat.spec
+++ b/thermostat.spec
@@ -80,7 +80,7 @@ Name: %{?scl_prefix}thermostat
Version: %{major}.%{minor}.%{patchlevel}
# If building from snapshot out of hg, uncomment and adjust below value as appropriate
#Release: 0.1.20131122hg%{hgrev}%{?dist}
-Release: 6%{?dist}
+Release: 7%{?dist}
Summary: A monitoring and serviceability tool for OpenJDK
License: GPLv2+ with exceptions and OFL
URL: http://icedtea.classpath.org/thermostat/
@@ -123,6 +123,11 @@ Patch3: mongodb26_setup_changes.patch
# Allow 'thermostat web-storage-service' to read configs from
# ~/.thermostat, by using a custom jaas config.
Patch4: webstorage_service_custom_jaas.patch
+# FIXME: remove with next upstream release.
+# See: http://icedtea.classpath.org/hg/thermostat/rev/c2f18f81f57a
+# Plus potential 1.4/1.2 backports.
+Patch5: CVE-2015-3201-config-file-world-readable.patch
+Patch6: web_storage_endpoint_fix.patch
# Self-BR in order for xmvn-subst to work for symlinking
# thermostat deps.
@@ -278,6 +283,8 @@ security.
%patch2 -p1
%patch3 -p1
%patch4 -p1
+%patch5 -p1
+%patch6 -p1
# Fix up artifact names which have different name upstream
# lucene
@@ -806,6 +813,7 @@ end
# Those files should be readable by root and tomcat only
%attr(0640,root,tomcat) %config(noreplace) %{_sysconfdir}/%{pkg_name}/%{pkg_name}-users.properties
%attr(0640,root,tomcat) %config(noreplace) %{_sysconfdir}/%{pkg_name}/%{pkg_name}-roles.properties
+%attr(0640,root,tomcat) %config(noreplace) %{_sysconfdir}/%{pkg_name}/web.auth
# We need an extra file in order to make thermostat-webapp work with
# our custom CATALINA_BASE. This sets the JAAS-config option.
%config(noreplace) %{system_confdir}/sysconfig/tomcat@%{pkg_name}
@@ -813,6 +821,10 @@ end
%{_datadir}/%{pkg_name}/plugins/embedded-web-endpoint
%changelog
+* Thu May 21 2015 Severin Gehwolf <sgehwolf at redhat.com> - 1.2.2-7
+- Read mongodb credentials from separate file.
+- Resolves: CVE-2015-3201
+
* Tue Mar 24 2015 Severin Gehwolf <sgehwolf at redhat.com> - 1.2.2-6
- Fix vm-stat bundle wiring issues.
diff --git a/web_storage_endpoint_fix.patch b/web_storage_endpoint_fix.patch
new file mode 100644
index 0000000..81cc3a9
--- /dev/null
+++ b/web_storage_endpoint_fix.patch
@@ -0,0 +1,344 @@
+diff --git a/web/war/src/main/webapp/WEB-INF/web.auth b/distribution/config/web.auth
+rename from web/war/src/main/webapp/WEB-INF/web.auth
+rename to distribution/config/web.auth
+diff --git a/distribution/pom.xml b/distribution/pom.xml
+--- a/distribution/pom.xml
++++ b/distribution/pom.xml
+@@ -157,6 +157,7 @@
+ <include>agent.properties</include>
+ <include>web-storage-service.properties</include>
+ <include>agent.auth</include>
++ <include>web.auth</include>
+ <include>ssl.properties</include>
+ <include>thermostat-users.properties</include>
+ <include>thermostat-roles.properties</include>
+diff --git a/distribution/scripts/thermostat-setup b/distribution/scripts/thermostat-setup
+--- a/distribution/scripts/thermostat-setup
++++ b/distribution/scripts/thermostat-setup
+@@ -181,20 +181,7 @@ unlockThermostat() {
+ runSetup() {
+ unlockThermostat
+ setupMongodbUser
+- # Location of the exploded web archive. We need this
+- # in order to be able to inject mongodb user credentials
+- # in there. Allow THERMOSTAT_WEBAPP_LOCATION to be set
+- # as env variable. This allows for running setup on
+- # packaged thermostat.
+- if [ "x$THERMOSTAT_WEBAPP_LOCATION" = "x" ]; then
+- # Expecting an upstream build where THERMOSTAT_HOME is
+- # distribution/target/image. The web archive gets copied
+- # as an exploded archive to the webapp subdir of the
+- # image.
+- default_webapp="$THERMOSTAT_HOME/webapp"
+- THERMOSTAT_WEBAPP_LOCATION="$(readlink -f $default_webapp)"
+- fi
+- TH_WEB_INF="$THERMOSTAT_WEBAPP_LOCATION/WEB-INF/web.auth"
++ TH_WEB_INF="$THERMOSTAT_HOME/etc/web.auth"
+
+ if [ ! -e $TH_WEB_INF ]; then
+ echo "File not found: $TH_WEB_INF" 1>&2
+diff --git a/web/server/src/main/java/com/redhat/thermostat/web/server/FileBasedStorageCredentials.java b/web/server/src/main/java/com/redhat/thermostat/web/server/FileBasedStorageCredentials.java
+--- a/web/server/src/main/java/com/redhat/thermostat/web/server/FileBasedStorageCredentials.java
++++ b/web/server/src/main/java/com/redhat/thermostat/web/server/FileBasedStorageCredentials.java
+@@ -66,10 +66,6 @@
+
+ }
+
+- public FileBasedStorageCredentials(InputStream in) throws IOException {
+- initialize(in);
+- }
+-
+ private void initialize(InputStream in) throws IOException {
+ props = new Properties();
+ props.load(in);
+diff --git a/web/server/src/main/java/com/redhat/thermostat/web/server/WebStorageEndPoint.java b/web/server/src/main/java/com/redhat/thermostat/web/server/WebStorageEndPoint.java
+--- a/web/server/src/main/java/com/redhat/thermostat/web/server/WebStorageEndPoint.java
++++ b/web/server/src/main/java/com/redhat/thermostat/web/server/WebStorageEndPoint.java
+@@ -141,6 +141,7 @@
+ // our strings can contain non-ASCII characters. Use UTF-8
+ // see also PR 1344
+ private static final String RESPONSE_JSON_CONTENT_TYPE = "application/json; charset=UTF-8";
++ private static final String CREDENTIALS_FILE = "web.auth";
+
+ private static final Logger logger = LoggingUtils.getLogger(WebStorageEndPoint.class);
+
+@@ -243,23 +244,11 @@
+ @Override
+ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException, ServletException {
+ if (storage == null) {
+- StorageCredentials creds = null;
+- final String CREDENTIALS_FILE = "web.auth";
+- // this assumes this is always an exploded war
+- File systemFile = new File(getServletContext().getRealPath("/WEB-INF/" + CREDENTIALS_FILE));
+- if (systemFile.exists() && systemFile.canRead()) {
+- logger.log(Level.CONFIG, "Loading authentication data from WEB-INF/" + CREDENTIALS_FILE);
+- try (InputStream in = new FileInputStream(systemFile)) {
+- creds = new FileBasedStorageCredentials(in);
+- }
+- } else {
+- File userCredentials = new File(paths.getUserConfigurationDirectory(), CREDENTIALS_FILE);
+- logger.log(Level.CONFIG, "Loading authentication data from " + userCredentials);
+- if (userCredentials.isFile() && userCredentials.canRead()) {
+- creds = new FileBasedStorageCredentials(userCredentials);
+- } else {
+- logger.warning("Unable to read database credentials from " + userCredentials);
+- }
++ StorageCredentials creds = getStorageCredentials(paths);
++ // 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";
++ throw new InvalidConfigurationException(errorMsg);
+ }
+ String storageClass = getServletConfig().getInitParameter(STORAGE_CLASS);
+ String storageEndpoint = getServletConfig().getInitParameter(STORAGE_ENDPOINT);
+@@ -292,6 +281,34 @@
+ getMore(req, resp);
+ }
+ }
++
++ // 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);
++ } 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;
++ }
++ }
++ }
++
++ // 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() {
+diff --git a/web/server/src/test/java/com/redhat/thermostat/web/server/WebStorageEndPointUnitTest.java b/web/server/src/test/java/com/redhat/thermostat/web/server/WebStorageEndPointUnitTest.java
+--- a/web/server/src/test/java/com/redhat/thermostat/web/server/WebStorageEndPointUnitTest.java
++++ b/web/server/src/test/java/com/redhat/thermostat/web/server/WebStorageEndPointUnitTest.java
+@@ -39,6 +39,7 @@
+ import static org.junit.Assert.assertEquals;
+ import static org.junit.Assert.assertFalse;
+ import static org.junit.Assert.assertNotNull;
++import static org.junit.Assert.assertNull;
+ import static org.junit.Assert.assertTrue;
+ import static org.junit.Assert.fail;
+ import static org.mockito.Mockito.mock;
+@@ -64,6 +65,8 @@
+ import org.junit.Test;
+ import org.mockito.ArgumentCaptor;
+
++import com.redhat.thermostat.shared.config.CommonPaths;
++import com.redhat.thermostat.storage.core.StorageCredentials;
+ import com.redhat.thermostat.web.server.auth.WebStoragePathHandler;
+
+ /**
+@@ -250,6 +253,114 @@
+ 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
++ */
++ @Test
++ public void storageCredentialsNull() throws IOException {
++ StorageCredsTestSetupResult setupResult = setupForStorageCredentialsTest(false, false);
++ StorageCredentials creds = setupResult.endpoint.getStorageCredentials(setupResult.commonPaths);
++ assertNull(creds);
++ }
++
+ private ThCreatorResult creatWorkingThermostatHome() throws IOException {
+ Path testThermostatHome = Files.createTempDirectory(
+ "foo-thermostat-home-", new FileAttribute[] {});
+@@ -276,4 +387,44 @@
+ }
+ }
+
++ 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() {
++ // not implemented
++ return null;
++ }
++
++ @Override
++ public char[] getPassword() {
++ // not implemented
++ return null;
++ }
++
++ }
++
+ }
+diff --git a/web/server/src/test/java/com/redhat/thermostat/web/server/WebStorageEndpointTest.java b/web/server/src/test/java/com/redhat/thermostat/web/server/WebStorageEndpointTest.java
+--- a/web/server/src/test/java/com/redhat/thermostat/web/server/WebStorageEndpointTest.java
++++ b/web/server/src/test/java/com/redhat/thermostat/web/server/WebStorageEndpointTest.java
+@@ -266,6 +266,13 @@
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
++ File webAuthFile = new File(configDirectory, "web.auth");
++ // only creates file if it doesn't exist yet
++ try {
++ webAuthFile.createNewFile();
++ } catch (IOException e) {
++ throw new RuntimeException(e);
++ }
+ }
+
+ @Override
+diff --git a/web/war/pom.xml b/web/war/pom.xml
+--- a/web/war/pom.xml
++++ b/web/war/pom.xml
+@@ -226,7 +226,6 @@
+ <directory>src/main/webapp</directory>
+ <includes>
+ <include>**/web.xml</include>
+- <include>**/web.auth</include>
+ </includes>
+ </resource>
+ </webResources>
+@@ -249,7 +248,6 @@
+ <directory>src/main/webapp</directory>
+ <includes>
+ <include>**/web.xml</include>
+- <include>**/web.auth</include>
+ </includes>
+ </resource>
+ </webResources>
--
cgit v0.10.2
http://pkgs.fedoraproject.org/cgit/thermostat.git/commit/?h=f22&id=9d7bd12f82d5a4b269b31ccca8a82977c9d85127
More information about the scm-commits
mailing list