Refactoring organisation, bundling and security improvements (nightsc…

…out#6765) * * Simplified bundling to just one bundle * Removed cache invalidation token from bundling * Generate strong persistent random string on deploy to use for JWT signing * WIP: moving api-secret and JWT signing to a separate centralized security component * Moved some server components away from project root * Fix issues reported by linter * Ignore detect-object-injection everywhere but the client * Make admin message button red * Remove alarms for some security alerts on code * api_secret is now fully contained in the enclave
lpsuerj · Apr 17, 2021 · ba7d0cb · ba7d0cb
1 parent 34498db
commit ba7d0cb
Show file tree

Hide file tree

Showing 64 changed files with 362 additions and 249 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -1,26 +1,35 @@
 module.exports = {
-    "plugins": [
-      "security"
+    'plugins': [
+      'security'
     ],
-    "extends": [
-      "eslint:recommended",
-      "plugin:security/recommended"
+    'extends': [
+      'eslint:recommended',
+      'plugin:security/recommended'
     ],
-    "parser": "babel-eslint",
-    "env": {
-      "browser": true,
-      "commonjs": true,
-      "es6": true,
-      "node": true,
-      "mocha": true,
-      "jquery": true
+    'parser': 'babel-eslint',
+    'env': {
+      'browser': true,
+      'commonjs': true,
+      'es6': true,
+      'node': true,
+      'mocha': true,
+      'jquery': true
     },
-    "rules": {
-      "no-unused-vars": [
-        "error",
+    'rules': {
+      'security/detect-object-injection' : 0,
+      'no-unused-vars': [
+        'error',
         {
-          "varsIgnorePattern": "should|expect"
+          'varsIgnorePattern': 'should|expect'
         }
       ]
-    }
+    },
+    'overrides': [
+      {
+        'files': ['lib/client/*.js'],
+        'rules': {
+          'security/detect-object-injection': 1
+        }
+      }
+    ],
   };
diff --git a/.gitignore b/.gitignore
@@ -9,6 +9,7 @@ bundle/bundle.out.js
 *.iml
 my.env
 my.*.env
+*.pem
 
 static/bower_components/
 .*.sw?

diff --git a/bin/generateRandomString.js b/bin/generateRandomString.js
@@ -0,0 +1,5 @@
+
+require('crypto').randomBytes(1024, function(err, buffer) {
+    var token = buffer.toString('hex');
+    console.log(token);
+});
diff --git a/bundle/bundle.source.js b/bundle/bundle.source.js
@@ -28,6 +28,12 @@ window.Nightscout = {
     admin_plugins: require('../lib/admin_plugins/')()
 };
 
+window.Nightscout.report_plugins_preinit = require('../lib/report_plugins/');
+window.Nightscout.predictions = require('../lib/report/predictions');
+window.Nightscout.reportclient = require('../lib/report/reportclient');
+window.Nightscout.profileclient = require('../lib/profile/profileeditor');
+window.Nightscout.foodclient = require('../lib/food/food');
+
 console.info('Nightscout bundle ready');
 
 // Needed for Hot Module Replacement

diff --git a/lib/api/index.js b/lib/api/index.js
@@ -13,13 +13,13 @@ function create (env, ctx) {
   app.set('version', env.version);
 
   app.set('units', env.DISPLAY_UNITS);
-  // Only allow access to the API if API_SECRET is set on the server.
+  // Only allow access to the API if API KEY is set on the server.
   app.disable('api');
-  if (env.api_secret) {
-    console.log('API_SECRET present, enabling API');
+  if (env.enclave.isApiKeySet()) {
+    console.log('API KEY present, enabling API');
     app.enable('api');
   } else {
-    console.log('API_SECRET not found, API disabled');
+    console.log('API KEY has not been set, API disabled');
   }
 
   if (env.settings.enable) {

diff --git a/lib/authorization/index.js b/lib/authorization/index.js
@@ -96,7 +96,7 @@ function init (env, ctx) {
   }
 
   function authorizeAdminSecret (secret) {
-    return (env.api_secret && env.api_secret.length > 12) ? (secret === env.api_secret) : false;
+    return env.enclave.isApiKey(secret);
   }
 
   authorization.seenPermissions = [];
@@ -185,7 +185,7 @@ function init (env, ctx) {
 
     // Tokens have to be well formed JWTs
     try {
-      const verified = await jwt.verify(data.token, env.api_secret);
+      const verified = env.enclave.verifyJWT(data.token);
       token = verified.accessToken;
     } catch (err) {}
 
@@ -237,7 +237,7 @@ function init (env, ctx) {
 
   /**
    * Check if the client has a permission execute an action,
-   * based on an API_SECRET or JWT in the request.
+   * based on an API KEY or JWT in the request.
    * 
    * Used to authorize API calls
    * 
@@ -281,8 +281,9 @@ function init (env, ctx) {
    */
   authorization.authorize = function authorize (accessToken) {
 
-    let userToken = accessToken
-    const decodedToken = jwt.decode(accessToken);
+
+    let userToken = accessToken;
+    const decodedToken = env.enclave.verifyJWT(accessToken);
 
     if (decodedToken && decodedToken.accessToken) {
       userToken = decodedToken.accessToken;
@@ -292,18 +293,14 @@ function init (env, ctx) {
     var authorized = null;
 
     if (subject) {
-      var token = jwt.sign({ accessToken: subject.accessToken }, env.api_secret, { expiresIn: '8h' });
-
-      //decode so we can tell the client the issued and expired times
-      var decoded = jwt.decode(token);
+      const token = env.enclave.signJWT({ accessToken: subject.accessToken });
+      const decoded = env.enclave.verifyJWT(token);
 
       var roles = _.uniq(subject.roles.concat(defaultRoles));
 
       authorized = {
         token
         , sub: subject.name
-          // not sending roles to client to prevent us from treating them as magic
-          // instead group permissions by role so the we can create correct shiros on the client
         , permissionGroups: _.map(roles, storage.roleToPermissions)
         , iat: decoded.iat
         , exp: decoded.exp

diff --git a/lib/authorization/storage.js b/lib/authorization/storage.js
@@ -151,12 +151,9 @@ function init (env, ctx) {
         }
 
         storage.subjects = _.map(results, function eachSubject (subject) {
-          if (env.api_secret) {
-            var shasum = crypto.createHash('sha1');
-            shasum.update(env.api_secret);
-            shasum.update(subject._id.toString());
+          if (env.enclave.isApiKeySet()) {
+            subject.digest = env.enclave.getSubjectHash(subject._id.toString());
             var abbrev = subject.name.toLowerCase().replace(/[\W]/g, '').substring(0, 10);
-            subject.digest = shasum.digest('hex');
             subject.accessToken = abbrev + '-' + subject.digest.substring(0, 16);
             subject.accessTokenDigest = storage.getSHA1(subject.accessToken);
           }

diff --git a/lib/client/boluscalc.js b/lib/client/boluscalc.js
@@ -593,14 +593,16 @@ function init (client, $) {
       if (qp.hideafteruse) {
         qp.hidden = true;
 
-        var apisecrethash = localStorage.getItem('apisecrethash');
-        var dataJson = JSON.stringify(qp, null, ' ');
-
-        var xhr = new XMLHttpRequest();
-        xhr.open('PUT', '/api/v1/food/', true);
-        xhr.setRequestHeader('Content-Type', 'application/json; charset=UTF-8');
-        xhr.setRequestHeader('api-secret', apisecrethash);
-        xhr.send(dataJson);
+        $.ajax({
+          method: 'PUT'
+          , url: '/api/v1/food/'
+          , headers: client.headers()
+          , data: qp
+        }).done(function treatmentSaved (response) {
+          console.info('quick pick saved', response);
+        }).fail(function treatmentSaveFail (response) {
+          console.info('quick pick failed to save', response);
+        });
       }
     }
 

diff --git a/lib/food/food.js b/lib/food/food.js
@@ -146,7 +146,7 @@ client.init(function loaded () {
     $('#fe_filter_subcategory').empty().append(new Option(translate('(none)'),''));
     if (filter.category !== '') {
       for (s in categories[filter.category]) {
-        if (categories[filter.category].hasOwnProperty(s)) {
+        if (Object.prototype.hasOwnProperty.call(categories[filter.category],s)) {
           $('#fe_filter_subcategory').append(new Option(s,s));
         }
       }
@@ -162,7 +162,7 @@ client.init(function loaded () {
     $('#fe_subcategory_list').empty().append(new Option(translate('(none)'),''));
     if (foodrec.category !== '') {
       for (s in categories[foodrec.category]) {
-        if (categories[foodrec.category].hasOwnProperty(s)) {
+        if (Object.prototype.hasOwnProperty.call(categories[foodrec.category],s)) {
           $('#fe_subcategory_list').append(new Option(s,s));
         }
       }
@@ -198,7 +198,7 @@ client.init(function loaded () {
     $('#fe_filter_category').empty().append(new Option(translate('(none)'),''));
     $('#fe_category_list').empty().append(new Option(translate('(none)'),''));
     for (var s in categories) {
-      if (categories.hasOwnProperty(s)) {
+      if (Object.prototype.hasOwnProperty.call(categories,s)) {
         $('#fe_filter_category').append(new Option(s,s));
         $('#fe_category_list').append(new Option(s,s));
       }
@@ -398,7 +398,7 @@ client.init(function loaded () {
   function savePortions(event) {
     var index = $(this).attr('index');
     var findex = $(this).attr('findex');
-    var val = parseFloat($(this).val().replace(/\,/g,'.'));
+    var val = parseFloat($(this).val().replace(/,/g,'.'));
     foodquickpick[index].foods[findex].portions=val;
     calculateCarbs(index);
     drawQuickpick();

diff --git a/lib/language.js b/lib/language.js
@@ -78,28 +78,32 @@ function init (fs) {
       translated = language.translateCS(text);
     }
 
-    let keys = null;
+    var hasCI = false;
+    var hasParams = false;
 
-    if (options && options.params) {
-      keys = options.params;
+    if (options) {
+      hasCI = Object.prototype.hasOwnProperty.call(options,'ci');
+      hasParams = Object.prototype.hasOwnProperty.call(options,'params');
     }
 
-    if (options && !options.hasOwnProperty('ci') && !options.hasOwnProperty('params')) {
+    var keys = hasParams ? options.params : null;
+
+    if (options && !hasCI && !hasParams) {
       keys = [];
       for (var i = 1; i < arguments.length; i++) {
         keys.push(arguments[i]);
       }
     }
 
-    if (options && (options.hasOwnProperty('ci') || options.hasOwnProperty('params')) && arguments.length > 2) {
+    if (options && (hasCI || hasParams) && arguments.length > 2) {
       if (!keys) keys = [];
-      for (var i = 2; i < arguments.length; i++) {
+      for (i = 2; i < arguments.length; i++) {
         keys.push(arguments[i]);
       }
     }
 
     if (keys) {
-      for (var i = 0; i < keys.length; i++) {
+      for (i = 0; i < keys.length; i++) {
         // eslint-disable-next-line no-useless-escape
         var r = new RegExp('\%' + (i + 1), 'g');
         translated = translated.replace(r, keys[i]);

diff --git a/lib/profile/profileeditor.js b/lib/profile/profileeditor.js
@@ -100,12 +100,12 @@ var init = function init () {
           _.each(mongoprofile.store, function eachStoredProfile (p) {
             // allign with default profile
             for (var key in defaultprofile) {
-              if (defaultprofile.hasOwnProperty(key) && !p.hasOwnProperty(key)) {
+              if (Object.prototype.hasOwnProperty.call(defaultprofile,key) && !Object.prototype.hasOwnProperty.call(p,key)) {
                   p[key] = defaultprofile[key];
               }
             }
             for (key in p) {
-              if (p.hasOwnProperty(key) && !defaultprofile.hasOwnProperty(key)) {
+              if (Object.prototype.hasOwnProperty.call(p,key) && !Object.prototype.hasOwnProperty.call(defaultprofile,key)) {
                   delete p[key];
               }
             }
@@ -232,7 +232,7 @@ var init = function init () {
     $('#pe_profiles').empty();
 
     for (var key in record.store) {
-      if (record.store.hasOwnProperty(key)) {
+      if (Object.prototype.hasOwnProperty.call(record.store,key)) {
         $('#pe_profiles').append('<option value="' + key + '">' + key + '</option>');
       }
     }
@@ -656,7 +656,7 @@ var init = function init () {
     var adjustedRecord = _.cloneDeep(record);
 
     for (var key in adjustedRecord.store) {
-      if (adjustedRecord.store.hasOwnProperty(key)) {
+      if (Object.prototype.hasOwnProperty.call(adjustedRecord.store,key)) {
         var profile = adjustedRecord.store[key];
         if (!profile.perGIvalues) {
           delete profile.perGIvalues;
@@ -707,7 +707,7 @@ var init = function init () {
   function getFirstAvailableProfile(record) {
     var availableProfiles = [];
     for (var key in record.store) {
-      if (record.store.hasOwnProperty(key)) {
+      if (Object.prototype.hasOwnProperty.call(record.store,key)) {
         if (key !== currentprofile) {
           availableProfiles.push(key);
         }