From 3104083670d2bd2c32600f6a10c1dc55395870cf Mon Sep 17 00:00:00 2001
From: PieterGit <pieterg@gmail.com>
Date: Sat, 2 Feb 2019 22:29:02 +0100
Subject: [PATCH] fix readENVTruthy and make INSECURE_USE_HTTP,
 SECURE_HSTS_HEADER, SECURE_HSTS_HEADER_* and SECURE_CSP work as expected.

readENVTruthy never returned defaultValue. if not set to on|off|true|false the default value is returned
---
 app.js            | 13 ++++++-------
 env.js            | 12 ++++++++++--
 lib/settings.js   |  2 ++
 tests/env.test.js | 15 +++++++++++++++
 4 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/app.js b/app.js
index 2928f35629b..bece71ac530 100644
--- a/app.js
+++ b/app.js
@@ -14,18 +14,17 @@ function create(env, ctx) {
     var appInfo = env.name + ' ' + env.version;
     app.set('title', appInfo);
     app.enable('trust proxy'); // Allows req.secure test on heroku https connections.
-    if (!process.env.INSECURE_USE_HTTP=='true') {
+    if (!process.env.insecureUseHttp) {
         app.use((req, res, next) => {
         if (req.header('x-forwarded-proto') !== 'https')
             res.redirect(`https://${req.header('host')}${req.url}`);
         else
             next()
         })
-        //if (env.settings.isEnabled('secureHstsHeader')) { // by TODO: find out why env.settings.isEnabled doest not work
-        if (process.env.SECURE_HSTS_HEADER == 'true') { // Add HSTS (HTTP Strict Transport Security) header
+        if (process.secureHstsHeader) { // Add HSTS (HTTP Strict Transport Security) header
           const helmet = require('helmet');
-          var includeSubDomainsValue = process.env.SECURE_HSTS_HEADER_INCLUDESUBDOMAINS || false ; // _get(env, 'extendedSettings.secureHstsHeader.includesubdomains')
-            var preloadValue = process.env.SECURE_HSTS_HEADER_PRELOAD || false; // _get(env, 'extendedSettings.secureHstsHeader.preload') || false ; // default
+          var includeSubDomainsValue = process.env.secureHstsHeaderIncludeSubdomains;
+            var preloadValue = process.env.secureHstsHeaderPreload;
             app.use(helmet({
                 hsts: {
                     maxAge: 31536000,
@@ -34,8 +33,8 @@ function create(env, ctx) {
                 }
             }))
             //if (env.settings.isEnabled('secureCsp')) { // Add Content-Security-Policy directive by default
-            if (process.env.SECURE_CSP == 'true') {
-              app.use(helmet.contentSecurityPolicy({ // TODO make NS work without 'unsafe-inline'
+            if (process.env.secureCsp) {
+              app.use(helmet.contentSecurityPolicy({ //TODO make NS work without 'unsafe-inline'
                 directives: {
                   defaultSrc: ["'self'"],
                   styleSrc: ["'self'", 'https://fonts.googleapis.com/',"'unsafe-inline'"],
diff --git a/env.js b/env.js
index 4ac1655570f..808b18fd9ff 100644
--- a/env.js
+++ b/env.js
@@ -55,6 +55,13 @@ function setSSL() {
       env.ca = fs.readFileSync(env.SSL_CA);
     }
   }
+
+  env.insecureUseHttp = readENVTruthy("INSECURE_USE_HTTP", false);
+  env.secureHstsHeader = readENVTruthy("SECURE_HSTS_HEADER", true);
+  env.secureHstsHeaderIncludeSubdomains = readENVTruthy("SECURE_HSTS_HEADER_INCLUDESUBDOMAINS", false);
+  env.secureHstsHeaderPreload= readENVTruthy("SECURE_HSTS_HEADER_PRELOAD", false);
+  env.secureCsp = readENVTruthy("SECURE_CSP", false);
+
 }
 
 // A little ugly, but we don't want to read the secret into a var
@@ -144,7 +151,8 @@ function readENV(varName, defaultValue) {
 function readENVTruthy(varName, defaultValue) {
   var value = readENV(varName, defaultValue);
   if (typeof value === 'string' && (value.toLowerCase() === 'on' || value.toLowerCase() === 'true')) { value = true; }
-  if (typeof value === 'string' && (value.toLowerCase() === 'off' || value.toLowerCase() === 'false')) { value = false; }
+  else if (typeof value === 'string' && (value.toLowerCase() === 'off' || value.toLowerCase() === 'false')) { value = false; }
+  else { value=defaultValue }
   return value;
 }
 
@@ -178,6 +186,6 @@ function findExtendedSettings (envs) {
     }
   });
   return extended;
-}
+  }
 
 module.exports = config;
diff --git a/lib/settings.js b/lib/settings.js
index 276632432e5..e4d15f99f63 100644
--- a/lib/settings.js
+++ b/lib/settings.js
@@ -44,6 +44,8 @@ function init ( ) {
     },
     insecureUseHttp: false,
     secureHstsHeader: true,
+    secureHstsHeaderIncludeSubdomains: false,
+    secureHstsHeaderPreload: false,
     secureCsp: false
   };
 
diff --git a/tests/env.test.js b/tests/env.test.js
index 12d9f5793bd..e8b63eae1d0 100644
--- a/tests/env.test.js
+++ b/tests/env.test.js
@@ -52,4 +52,19 @@ describe('env', function ( ) {
     delete process.env.PUSHOVER_API_TOKEN;
   });
 
+  it('readENVTruthy ', function () {
+    process.env.INSECURE_USE_HTTP = 'true';
+    var env = require('../env')();
+    env.insecureUseHttp.should.be.true();
+    process.env.INSECURE_USE_HTTP = 'false';
+    env = require('../env')();
+    env.insecureUseHttp.should.be.false();
+    process.env.INSECURE_USE_HTTP = 'not set ok, so use default value false';
+    env = require('../env')();
+    env.insecureUseHttp.should.be.false();
+    delete process.env.INSECURE_USE_HTTP; // unset INSECURE_USE_HTTP
+    env = require('../env')();
+    env.insecureUseHttp.should.be.false(); // not defined should be false
+  });
+
 });