Merge pull request #27 from davidffrench/RHMAP-18301

Rhmap 18301
feedhenry · Nov 3, 2017 · 9ffc04c · 9ffc04c
2 parents 7547140 + 7fa1713
commit 9ffc04c
Show file tree

Hide file tree

Showing 6 changed files with 190 additions and 53 deletions.
diff --git a/lib/common/authenticate.js b/lib/common/authenticate.js
@@ -11,6 +11,7 @@ var nodejsUrl = require('url');
 
 var authorizationCache = {};
 var authenticateAppApiKeyCache = {};
+var authenticateAppApiKeyAuthProjCache = {};
 
 module.exports = function(req, res, params) {
   var apiKey = process.env.FH_APP_API_KEY;
@@ -114,6 +115,88 @@ module.exports = function(req, res, params) {
     });
   }
 
+   /**
+   * private
+   */
+  function authenticateAppApiKeyAgainstAuthorisedProjects(cb) {
+    var sentApiKey = getAppApiKey();
+
+    // Fail fast if no sent api key
+    if (!sentApiKey) {
+      return cb({
+        code: UNAUTHORISED_HTTP_CODE,
+        message: "Missing App API key(s)"
+      });
+    }
+
+    var millicore = process.env.FH_MILLICORE;
+    var millicoreProt = process.env.FH_MILLICORE_PROTOCOL || "https";
+    var env = process.env.FH_ENV;
+    var project_id = process.env.FH_WIDGET;
+    var app_id = process.env.FH_INSTANCE;
+
+    var now = new Date().getTime();
+
+    //check cache
+    if (authenticateAppApiKeyAuthProjCache.hasOwnProperty(sentApiKey)) {
+      var cache = authenticateAppApiKeyAuthProjCache[sentApiKey];
+      //cache not timed out return cb else auth again
+      if (cache.timeout > now) {
+        // Cache hit
+        return cb();
+      } else {
+        // Cache expired - remove and continue
+        delete authenticateAppApiKeyAuthProjCache[sentApiKey];
+      }
+    }
+
+    var data = {
+      environment: env,
+      clientApiKey: sentApiKey
+    };
+
+    // Call Core and validate passed key
+    var url = millicoreProt + "://" + millicore + "/box/api/projects/" + project_id + "/apps/" + app_id + "/validate_key_against_authorised_projects";
+    request.post({
+      url: url,
+      json: true,
+      body: data
+    }, function(err, res) {
+      // Can set API_KEY_VALIDATION_TIMEOUT to expire more frequently than a day
+      var expiration = parseInt(process.env.API_KEY_AUTH_PROJ_VALIDATION_TIMEOUT, 10) || 300000; // A day
+      var cacheLength = now + expiration; // A day
+
+      // Explicitly handle 404s from Millicore here - the validate_key_against_authoried_projects endpoint may not exist in this cluster
+      // If we get a 404, continue
+      if (res.statusCode === 404) {
+        authenticateAppApiKeyAuthProjCache[sentApiKey] = {
+          "timeout": cacheLength
+        };
+        return cb();
+      }
+
+      if (err) {
+        return cb({
+          code: 500,
+          message: "Error talking to Core to validate API Key - please try again",
+          details: err.message
+        });
+      }
+
+      if (res.statusCode === 200) {
+        authenticateAppApiKeyAuthProjCache[sentApiKey] = {
+          "timeout": cacheLength
+        };
+        return cb();
+      } else {
+        return cb({
+          code: UNAUTHORISED_HTTP_CODE,
+          message: "Invalid API Key"
+        });
+      }
+    });
+  }
+
   function isServiceApp() {
     return process.env.FH_SERVICE_APP === 'true';
   }
@@ -165,31 +248,34 @@ module.exports = function(req, res, params) {
     return vars;
   }
 
-  function verifyServiceAuthorisation() {
+  function verifyServiceAuthorisation(cb) {
     // Not a service
-    if (!isServiceApp()) return true;
+    if (!isServiceApp()) return cb();
 
     var envs = resolveServiceVars();
 
     // Global/public services do not require auth
-    if (envs.service_app_public) return true;
+    if (envs.service_app_public) return cb();
 
     //Preferably, the service access key headers will be used to
     //validate a service request
     if(envs.request_access_key && envs.service_access_key){
-      return envs.request_access_key === envs.service_access_key;
+      if(envs.request_access_key === envs.service_access_key) {
+        return cb()
+      } else {
+        return cb(new Error())
+      }
     }
 
     //If no service access key, validate that accessor project can access the service
     if (typeof envs.service_app !== 'undefined' && typeof envs.service_app_public !== 'undefined' && typeof envs.service_authorised_projects !== 'undefined' && typeof envs.accessor !== 'undefined') {
-      // Perms check
+      // If accessor project can access, also check the sent app api key is valid against the authorised projects.
       if (envs.service_authorised_projects.indexOf(envs.accessor) !== -1) {
-        return true;
+        return authenticateAppApiKeyAgainstAuthorisedProjects(cb);
       }
     }
-
     // No access
-    return false;
+    return cb(new Error());
   }
 
   function getAppApiKey() {
@@ -283,47 +369,50 @@ module.exports = function(req, res, params) {
       //extract only the pathname
       endpoint = nodejsUrl.parse(endpoint).pathname;
 
-      if (!verifyServiceAuthorisation()) {
-        return cb({
-          code: 401,
-          message: "You do not have permission to access this service."
-        });
-      }
-
-      //if there is no auth config then assume nothing has been setup for this app yet and continue as normal.
-      if (!authConfig) {
-        return cb();
-      }
-      if ('string' === typeof authConfig) {
-        try {
-          authConfig = JSON.parse(authConfig);
-        } catch (e) {
+      verifyServiceAuthorisation(function(err, ok){
+        if(err) {
           return cb({
-            code: 503,
-            message: "failed to parse auth config " + e.message
+            code: err.code || 401,
+            message: err.message || "You do not have permission to access this service.",
+            details: err.details
           });
         }
-      }
-      var overrides = authConfig[OVERRIDES_KEY];
-      var defaultOpt = authConfig[DEFAULT_KEY];
-      //if there is a config set for this option process it.
-      if (typeof overrides === 'object' && (overrides.hasOwnProperty(endpoint) || overrides.hasOwnProperty('*'))) {
-        var enpointConfig = overrides[endpoint] || overrides['*'];
-        //there is a config for this endpoint it must have a security property otherwise we cannot decide how to proceed.
-
-        if ('object' === typeof enpointConfig && enpointConfig.hasOwnProperty("security")) {
-          var authType = enpointConfig.security.trim();
-          processAuth(authType, cb);
+
+        //if there is no auth config then assume nothing has been setup for this app yet and continue as normal.
+        if (!authConfig) {
+          return cb();
+        }
+        if ('string' === typeof authConfig) {
+          try {
+            authConfig = JSON.parse(authConfig);
+          } catch (e) {
+            return cb({
+              code: 503,
+              message: "failed to parse auth config " + e.message
+            });
+          }
+        }
+        var overrides = authConfig[OVERRIDES_KEY];
+        var defaultOpt = authConfig[DEFAULT_KEY];
+        //if there is a config set for this option process it.
+        if (typeof overrides === 'object' && (overrides.hasOwnProperty(endpoint) || overrides.hasOwnProperty('*'))) {
+          var enpointConfig = overrides[endpoint] || overrides['*'];
+          //there is a config for this endpoint it must have a security property otherwise we cannot decide how to proceed.
+
+          if ('object' === typeof enpointConfig && enpointConfig.hasOwnProperty("security")) {
+            var authType = enpointConfig.security.trim();
+            processAuth(authType, cb);
+          } else {
+            return cb({
+              code: 503,
+              message: " internal error"
+            });
+          }
         } else {
-          return cb({
-            code: 503,
-            message: " internal error"
-          });
+          //fall back to config default
+          processAuth(defaultOpt, cb);
         }
-      } else {
-        //fall back to config default
-        processAuth(defaultOpt, cb);
-      }
+      });
     },
     /**
      *

diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "fh-mbaas-express",
-  "version": "5.8.0",
+  "version": "5.9.0",
   "description": "FeedHenry MBAAS Express",
   "main": "lib/webapp.js",
   "dependencies": {

diff --git a/sonar-project.properties b/sonar-project.properties
@@ -1,6 +1,6 @@
 sonar.projectKey=fh-mbaas-express
 sonar.projectName=fh-mbaas-express-nightly-master
-sonar.projectVersion=5.8.0
+sonar.projectVersion=5.9.0
 
 sonar.sources=./lib
 sonar.tests=./test

diff --git a/test/fixtures/validate_key_call.js b/test/fixtures/validate_key_call.js
@@ -18,4 +18,23 @@ module.exports.good_key = nock('https://testing.feedhenry.me')
   })
   .post('/box/api/projects/undefined/apps/undefined/validate_key', '*')
   .times(2)
-  .reply(200, {});
+  .reply(200, {});
+
+  module.exports.good_key_auth_proj = nock('https://testing.feedhenry.me')
+  .persist()
+  .post('/box/api/projects/undefined/apps/undefined/validate_key_against_authorised_projects', {
+    "environment": "dev",
+    "clientApiKey": "rightkey"
+  })
+  .times(1)
+  .reply(200, {})
+
+  module.exports.wrong_key_auth_proj = nock('https://testing.feedhenry.me')
+  .persist()
+  .post('/box/api/projects/undefined/apps/undefined/validate_key_against_authorised_projects', {
+    "environment": "dev",
+    "clientApiKey": "wrongkey"
+  })
+  .times(1)
+  .reply(400, {})
+
diff --git a/test/unit/test-authenticate.js b/test/unit/test-authenticate.js
@@ -1,14 +1,18 @@
 var authenticate = require('../../lib/common/authenticate');
 var assert = require('assert');
+var nockValidateKeyCall = require("../fixtures/validate_key_call");
 
 module.exports = {
-  "test service access using allowed project id": function(finish){
+  "test service access using allowed project id and correct api key": function(finish){
     process.env.FH_SERVICE_APP = 'true';
     process.env.FH_SERVICE_AUTHORISED_PROJECTS = "projectguid1,projectguid2";
+    process.env.FH_MILLICORE = "testing.feedhenry.me";
+    process.env.FH_ENV = "dev";
 
     var req = {
       headers: {
-        'x-request-with': "projectguid2"
+        'x-request-with': "projectguid2",
+        'x-fh-auth-app': "rightkey"
       }
     };
 
@@ -22,20 +26,45 @@ module.exports = {
     });
 
   },
-  "test service access using disallowed project id": function(finish){
+  "test service access using disallowed project id and correct api key": function(finish){
     process.env.FH_SERVICE_APP = 'true';
     process.env.FH_SERVICE_AUTHORISED_PROJECTS = "projectguid1,projectguid2";
+    process.env.FH_MILLICORE = "testing.feedhenry.me";
 
     var req = {
       headers: {
-        'x-request-with': "wrongprojectguid"
+        'x-request-with': "wrongprojectguid",
+        'x-fh-auth-app': "rightkey"
       }
     };
 
     authenticate(req, {}, {}).authenticate("some/path/to/something", function(err){
       assert.ok(err, "Expected An Error ");
       assert.equal(401, err.code);
-      assert.ok(err.message.indexOf('service') > -1, "Expected A Service Error Message");
+      assert.ok(err.message, "Invalid API Key");
+
+      //Restore Env
+      process.env.FH_SERVICE_APP = '';
+      process.env.FH_SERVICE_AUTHORISED_PROJECTS = "";
+      finish();
+    });
+  },
+  "test service access using allowed project id and incorrect api key": function(finish){
+    process.env.FH_SERVICE_APP = 'true';
+    process.env.FH_SERVICE_AUTHORISED_PROJECTS = "projectguid1,projectguid2";
+    process.env.FH_MILLICORE = "testing.feedhenry.me";
+
+    var req = {
+      headers: {
+        'x-request-with': "projectguid2",
+        'x-fh-auth-app': "wrongkey"
+      }
+    };
+
+    authenticate(req, {}, {}).authenticate("some/path/to/something", function(err){
+      assert.ok(err, "Expected An Error ");
+      assert.equal(401, err.code);
+      assert.ok(err.message, "Invalid API Key");
 
       //Restore Env
       process.env.FH_SERVICE_APP = '';