From 1f367e666b116f2c7db4afef5292d242bf60c990 Mon Sep 17 00:00:00 2001
From: Wyatt Johnson <wyattjoh@gmail.com>
Date: Thu, 6 Apr 2017 10:39:36 -0600
Subject: [PATCH 1/2] Added joi plugin validation

---
 app.js                   |  3 +++
 graph/context.js         |  3 +++
 graph/hooks.js           |  5 +++++
 graph/loaders/index.js   |  3 +++
 graph/mutators/index.js  |  3 +++
 graph/resolvers/index.js |  3 +++
 graph/typeDefs.js        |  3 +++
 package.json             |  1 +
 routes/index.js          |  3 +++
 yarn.lock                | 27 +++++++++++++++++++++++++++
 10 files changed, 54 insertions(+)

diff --git a/app.js b/app.js
index e1e864b10..3c9142da5 100644
--- a/app.js
+++ b/app.js
@@ -10,6 +10,7 @@ const enabled = require('debug').enabled;
 const RedisStore = require('connect-redis')(session);
 const redis = require('./services/redis');
 const csrf = require('csurf');
+const Joi = require('joi');
 const errors = require('./errors');
 const graph = require('./graph');
 const apollo = require('graphql-server-express');
@@ -80,6 +81,8 @@ const passportDebug = require('debug')('talk:passport');
 
 // Install the passport plugins.
 plugins.get('server', 'passport').forEach((plugin) => {
+  Joi.assert(plugin.passport, Joi.func().arity(1), `Plugin '${plugin.name}' had an error loading the passport hook`);
+
   passportDebug(`added plugin '${plugin.plugin.name}'`);
 
   // Pass the passport.js instance to the plugin to allow it to inject it's
diff --git a/graph/context.js b/graph/context.js
index f3ed42ad8..030647bf7 100644
--- a/graph/context.js
+++ b/graph/context.js
@@ -3,6 +3,7 @@ const mutators = require('./mutators');
 
 const plugins = require('../services/plugins');
 const debug = require('debug')('talk:graph:context');
+const Joi = require('joi');
 
 /**
  * Contains the array of plugins that provide context to the server, these top
@@ -10,6 +11,8 @@ const debug = require('debug')('talk:graph:context');
  * @type {Array}
  */
 const contextPlugins = plugins.get('server', 'context').map(({plugin, context}) => {
+  Joi.assert(context, Joi.object().pattern(/\w/, Joi.func().maxArity(1)), `Plugin '${plugin.name}' had an error loading the context`);
+
   debug(`added plugin '${plugin.name}'`);
   return {context};
 });
diff --git a/graph/hooks.js b/graph/hooks.js
index 3fe1306cf..4977c796e 100644
--- a/graph/hooks.js
+++ b/graph/hooks.js
@@ -1,5 +1,6 @@
 const {forEachField} = require('graphql-tools');
 const debug = require('debug')('talk:graph:schema');
+const Joi = require('joi');
 
 /**
  * XXX taken from graphql-js: src/execution/execute.js, because that function
@@ -46,6 +47,10 @@ const decorateWithHooks = (schema, hooks) => forEachField(schema, (field, typeNa
     // Combine the pre/post hooks from each plugin into an array we can
     // execute.
     .reduce((acc, {plugin, hooks}) => {
+      Joi.assert(hooks, Joi.object({
+        pre: Joi.func(),
+        post: Joi.func()
+      }), `Plugin '${plugin.name}' had an error loading the hooks`);
 
       // Itterate over the hooks on the fields and look at it with a switch
       // block to check for misconfigured plugins.
diff --git a/graph/loaders/index.js b/graph/loaders/index.js
index 26727940f..3c3ccef2b 100644
--- a/graph/loaders/index.js
+++ b/graph/loaders/index.js
@@ -1,4 +1,5 @@
 const _ = require('lodash');
+const Joi = require('joi');
 const debug = require('debug')('talk:graph:loaders');
 
 const Actions = require('./actions');
@@ -23,6 +24,8 @@ let loaders = [
   // Load the plugin loaders from the manager.
   ...plugins
     .get('server', 'loaders').map(({plugin, loaders}) => {
+      Joi.assert(loaders, Joi.object().pattern(/\w/, Joi.object().pattern(/\w/, Joi.func())), `Plugin '${plugin.name}' had an error loading the loaders`);
+
       debug(`added plugin '${plugin.name}'`);
 
       return loaders;
diff --git a/graph/mutators/index.js b/graph/mutators/index.js
index 9975bedde..8fc065514 100644
--- a/graph/mutators/index.js
+++ b/graph/mutators/index.js
@@ -1,4 +1,5 @@
 const _ = require('lodash');
+const Joi = require('joi');
 const debug = require('debug')('talk:graph:mutators');
 
 const Comment = require('./comment');
@@ -17,6 +18,8 @@ let mutators = [
   // Load the plugin mutators from the manager.
   ...plugins
     .get('server', 'mutators').map(({plugin, mutators}) => {
+      Joi.assert(mutators, Joi.object().pattern(/\w/, Joi.object().pattern(/\w/, Joi.func())), `Plugin '${plugin.name}' had an error loading the mutators`);
+
       debug(`added plugin '${plugin.name}'`);
 
       return mutators;
diff --git a/graph/resolvers/index.js b/graph/resolvers/index.js
index 05d633547..743e32062 100644
--- a/graph/resolvers/index.js
+++ b/graph/resolvers/index.js
@@ -1,4 +1,5 @@
 const _ = require('lodash');
+const Joi = require('joi');
 const debug = require('debug')('talk:graph:resolvers');
 
 const ActionSummary = require('./action_summary');
@@ -50,6 +51,8 @@ let resolvers = {
  * as provide new ones.
  */
 resolvers = plugins.get('server', 'resolvers').reduce((acc, {plugin, resolvers}) => {
+  Joi.assert(resolvers, Joi.object().pattern(/\w/, Joi.object().pattern(/\w/, Joi.func())), `Plugin '${plugin.name}' had an error loading the resolvers`);
+
   debug(`added plugin '${plugin.name}'`);
 
   return _.merge(acc, resolvers);
diff --git a/graph/typeDefs.js b/graph/typeDefs.js
index 9b2c8b7aa..64ee07f6d 100644
--- a/graph/typeDefs.js
+++ b/graph/typeDefs.js
@@ -3,6 +3,7 @@
 // this change is done now everything will likely break on the front end.
 
 const fs = require('fs');
+const Joi = require('joi');
 const path = require('path');
 const {mergeStrings} = require('gql-merge');
 const debug = require('debug')('talk:graph:typeDefs');
@@ -20,6 +21,8 @@ const typeDefs = mergeStrings([
 
   // Load the plugin definitions from the manager.
   ...plugins.get('server', 'typeDefs').map(({plugin, typeDefs}) => {
+    Joi.assert(typeDefs, Joi.string(), `Plugin '${plugin.name}' had an error loading the typeDefs`);
+
     debug(`added plugin '${plugin.name}'`);
 
     return typeDefs;
diff --git a/package.json b/package.json
index 3ea18dfcd..adff68924 100644
--- a/package.json
+++ b/package.json
@@ -74,6 +74,7 @@
     "graphql-tools": "^0.9.0",
     "helmet": "^3.5.0",
     "inquirer": "^3.0.6",
+    "joi": "^10.4.1",
     "jsonwebtoken": "^7.3.0",
     "kue": "^0.11.5",
     "linkify-it": "^2.0.3",
diff --git a/routes/index.js b/routes/index.js
index 448bd6964..b7c08694f 100644
--- a/routes/index.js
+++ b/routes/index.js
@@ -1,5 +1,6 @@
 const express = require('express');
 const path = require('path');
+const Joi = require('joi');
 const plugins = require('../services/plugins');
 const debug = require('debug')('talk:routes');
 
@@ -27,6 +28,8 @@ if (process.env.NODE_ENV !== 'production') {
 
 // Inject server route plugins.
 plugins.get('server', 'router').forEach((plugin) => {
+  Joi.assert(plugin.router, Joi.func().arity(1), `Plugin '${plugin.name}' had an error loading the passport router`);
+
   debug(`added plugin '${plugin.plugin.name}'`);
 
   // Pass the root router to the plugin to mount it's routes.
diff --git a/yarn.lock b/yarn.lock
index c9e0c2463..4d1a4ed9c 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -3567,6 +3567,10 @@ hoek@2.x.x:
   version "2.16.3"
   resolved "https://registry.yarnpkg.com/hoek/-/hoek-2.16.3.tgz#20bb7403d3cea398e91dc4710a8ff1b8274a25ed"
 
+hoek@4.x.x:
+  version "4.1.1"
+  resolved "https://registry.yarnpkg.com/hoek/-/hoek-4.1.1.tgz#9cc573ffba2b7b408fb5e9c2a13796be94cddce9"
+
 hoist-non-react-statics@^1.0.3, hoist-non-react-statics@^1.2.0:
   version "1.2.0"
   resolved "https://registry.yarnpkg.com/hoist-non-react-statics/-/hoist-non-react-statics-1.2.0.tgz#aa448cf0986d55cc40773b17174b7dd066cb7cfb"
@@ -4097,6 +4101,10 @@ isemail@1.x.x:
   version "1.2.0"
   resolved "https://registry.yarnpkg.com/isemail/-/isemail-1.2.0.tgz#be03df8cc3e29de4d2c5df6501263f1fa4595e9a"
 
+isemail@2.x.x:
+  version "2.2.1"
+  resolved "https://registry.yarnpkg.com/isemail/-/isemail-2.2.1.tgz#0353d3d9a62951080c262c2aa0a42b8ea8e9e2a6"
+
 isexe@^1.1.1:
   version "1.1.2"
   resolved "https://registry.yarnpkg.com/isexe/-/isexe-1.1.2.tgz#36f3e22e60750920f5e7241a476a8c6a42275ad0"
@@ -4195,6 +4203,10 @@ istanbul@^1.1.0-alpha.1:
     which "^1.1.1"
     wordwrap "^1.0.0"
 
+items@2.x.x:
+  version "2.1.1"
+  resolved "https://registry.yarnpkg.com/items/-/items-2.1.1.tgz#8bd16d9c83b19529de5aea321acaada78364a198"
+
 iterall@1.0.2:
   version "1.0.2"
   resolved "https://registry.yarnpkg.com/iterall/-/iterall-1.0.2.tgz#41a2e96ce9eda5e61c767ee5dc312373bb046e91"
@@ -4212,6 +4224,15 @@ jodid25519@^1.0.0:
   dependencies:
     jsbn "~0.1.0"
 
+joi@^10.4.1:
+  version "10.4.1"
+  resolved "https://registry.yarnpkg.com/joi/-/joi-10.4.1.tgz#a2fca1f0d603d1b843f2c1e086b52461f6be1f36"
+  dependencies:
+    hoek "4.x.x"
+    isemail "2.x.x"
+    items "2.x.x"
+    topo "2.x.x"
+
 joi@^6.10.1:
   version "6.10.1"
   resolved "https://registry.yarnpkg.com/joi/-/joi-6.10.1.tgz#4d50c318079122000fe5f16af1ff8e1917b77e06"
@@ -7658,6 +7679,12 @@ topo@1.x.x:
   dependencies:
     hoek "2.x.x"
 
+topo@2.x.x:
+  version "2.0.2"
+  resolved "https://registry.yarnpkg.com/topo/-/topo-2.0.2.tgz#cd5615752539057c0dc0491a621c3bc6fbe1d182"
+  dependencies:
+    hoek "4.x.x"
+
 touch@1.0.0:
   version "1.0.0"
   resolved "https://registry.yarnpkg.com/touch/-/touch-1.0.0.tgz#449cbe2dbae5a8c8038e30d71fa0ff464947c4de"

From 28ce77a00220036d2c73c30d3c38e21a1ab60151 Mon Sep 17 00:00:00 2001
From: Wyatt Johnson <wyattjoh@gmail.com>
Date: Thu, 6 Apr 2017 10:56:32 -0600
Subject: [PATCH 2/2] Moved validation to one place

---
 app.js                   |  3 ---
 graph/context.js         |  3 ---
 graph/hooks.js           |  5 -----
 graph/loaders/index.js   |  3 ---
 graph/mutators/index.js  |  3 ---
 graph/resolvers/index.js |  3 ---
 graph/typeDefs.js        |  3 ---
 plugins.js               | 24 ++++++++++++++++++++++++
 routes/index.js          |  3 ---
 9 files changed, 24 insertions(+), 26 deletions(-)

diff --git a/app.js b/app.js
index 3c9142da5..e1e864b10 100644
--- a/app.js
+++ b/app.js
@@ -10,7 +10,6 @@ const enabled = require('debug').enabled;
 const RedisStore = require('connect-redis')(session);
 const redis = require('./services/redis');
 const csrf = require('csurf');
-const Joi = require('joi');
 const errors = require('./errors');
 const graph = require('./graph');
 const apollo = require('graphql-server-express');
@@ -81,8 +80,6 @@ const passportDebug = require('debug')('talk:passport');
 
 // Install the passport plugins.
 plugins.get('server', 'passport').forEach((plugin) => {
-  Joi.assert(plugin.passport, Joi.func().arity(1), `Plugin '${plugin.name}' had an error loading the passport hook`);
-
   passportDebug(`added plugin '${plugin.plugin.name}'`);
 
   // Pass the passport.js instance to the plugin to allow it to inject it's
diff --git a/graph/context.js b/graph/context.js
index 030647bf7..f3ed42ad8 100644
--- a/graph/context.js
+++ b/graph/context.js
@@ -3,7 +3,6 @@ const mutators = require('./mutators');
 
 const plugins = require('../services/plugins');
 const debug = require('debug')('talk:graph:context');
-const Joi = require('joi');
 
 /**
  * Contains the array of plugins that provide context to the server, these top
@@ -11,8 +10,6 @@ const Joi = require('joi');
  * @type {Array}
  */
 const contextPlugins = plugins.get('server', 'context').map(({plugin, context}) => {
-  Joi.assert(context, Joi.object().pattern(/\w/, Joi.func().maxArity(1)), `Plugin '${plugin.name}' had an error loading the context`);
-
   debug(`added plugin '${plugin.name}'`);
   return {context};
 });
diff --git a/graph/hooks.js b/graph/hooks.js
index 4977c796e..3fe1306cf 100644
--- a/graph/hooks.js
+++ b/graph/hooks.js
@@ -1,6 +1,5 @@
 const {forEachField} = require('graphql-tools');
 const debug = require('debug')('talk:graph:schema');
-const Joi = require('joi');
 
 /**
  * XXX taken from graphql-js: src/execution/execute.js, because that function
@@ -47,10 +46,6 @@ const decorateWithHooks = (schema, hooks) => forEachField(schema, (field, typeNa
     // Combine the pre/post hooks from each plugin into an array we can
     // execute.
     .reduce((acc, {plugin, hooks}) => {
-      Joi.assert(hooks, Joi.object({
-        pre: Joi.func(),
-        post: Joi.func()
-      }), `Plugin '${plugin.name}' had an error loading the hooks`);
 
       // Itterate over the hooks on the fields and look at it with a switch
       // block to check for misconfigured plugins.
diff --git a/graph/loaders/index.js b/graph/loaders/index.js
index 3c3ccef2b..26727940f 100644
--- a/graph/loaders/index.js
+++ b/graph/loaders/index.js
@@ -1,5 +1,4 @@
 const _ = require('lodash');
-const Joi = require('joi');
 const debug = require('debug')('talk:graph:loaders');
 
 const Actions = require('./actions');
@@ -24,8 +23,6 @@ let loaders = [
   // Load the plugin loaders from the manager.
   ...plugins
     .get('server', 'loaders').map(({plugin, loaders}) => {
-      Joi.assert(loaders, Joi.object().pattern(/\w/, Joi.object().pattern(/\w/, Joi.func())), `Plugin '${plugin.name}' had an error loading the loaders`);
-
       debug(`added plugin '${plugin.name}'`);
 
       return loaders;
diff --git a/graph/mutators/index.js b/graph/mutators/index.js
index 8fc065514..9975bedde 100644
--- a/graph/mutators/index.js
+++ b/graph/mutators/index.js
@@ -1,5 +1,4 @@
 const _ = require('lodash');
-const Joi = require('joi');
 const debug = require('debug')('talk:graph:mutators');
 
 const Comment = require('./comment');
@@ -18,8 +17,6 @@ let mutators = [
   // Load the plugin mutators from the manager.
   ...plugins
     .get('server', 'mutators').map(({plugin, mutators}) => {
-      Joi.assert(mutators, Joi.object().pattern(/\w/, Joi.object().pattern(/\w/, Joi.func())), `Plugin '${plugin.name}' had an error loading the mutators`);
-
       debug(`added plugin '${plugin.name}'`);
 
       return mutators;
diff --git a/graph/resolvers/index.js b/graph/resolvers/index.js
index 743e32062..05d633547 100644
--- a/graph/resolvers/index.js
+++ b/graph/resolvers/index.js
@@ -1,5 +1,4 @@
 const _ = require('lodash');
-const Joi = require('joi');
 const debug = require('debug')('talk:graph:resolvers');
 
 const ActionSummary = require('./action_summary');
@@ -51,8 +50,6 @@ let resolvers = {
  * as provide new ones.
  */
 resolvers = plugins.get('server', 'resolvers').reduce((acc, {plugin, resolvers}) => {
-  Joi.assert(resolvers, Joi.object().pattern(/\w/, Joi.object().pattern(/\w/, Joi.func())), `Plugin '${plugin.name}' had an error loading the resolvers`);
-
   debug(`added plugin '${plugin.name}'`);
 
   return _.merge(acc, resolvers);
diff --git a/graph/typeDefs.js b/graph/typeDefs.js
index 64ee07f6d..9b2c8b7aa 100644
--- a/graph/typeDefs.js
+++ b/graph/typeDefs.js
@@ -3,7 +3,6 @@
 // this change is done now everything will likely break on the front end.
 
 const fs = require('fs');
-const Joi = require('joi');
 const path = require('path');
 const {mergeStrings} = require('gql-merge');
 const debug = require('debug')('talk:graph:typeDefs');
@@ -21,8 +20,6 @@ const typeDefs = mergeStrings([
 
   // Load the plugin definitions from the manager.
   ...plugins.get('server', 'typeDefs').map(({plugin, typeDefs}) => {
-    Joi.assert(typeDefs, Joi.string(), `Plugin '${plugin.name}' had an error loading the typeDefs`);
-
     debug(`added plugin '${plugin.name}'`);
 
     return typeDefs;
diff --git a/plugins.js b/plugins.js
index d7c9f9426..9120c8d71 100644
--- a/plugins.js
+++ b/plugins.js
@@ -2,6 +2,7 @@ const fs = require('fs');
 const path = require('path');
 const resolve = require('resolve');
 const debug = require('debug')('talk:plugins');
+const Joi = require('joi');
 
 // Add support for require rewriting.
 require('app-module-path').addPath(__dirname);
@@ -21,6 +22,20 @@ try {
   }
 }
 
+const hookSchemas = {
+  passport: Joi.func().arity(1),
+  router: Joi.func().arity(1),
+  context: Joi.object().pattern(/\w/, Joi.func().maxArity(1)),
+  hooks: Joi.object({
+    pre: Joi.func(),
+    post: Joi.func()
+  }),
+  loaders: Joi.object().pattern(/\w/, Joi.object().pattern(/\w/, Joi.func())),
+  mutators: Joi.object().pattern(/\w/, Joi.object().pattern(/\w/, Joi.func())),
+  resolvers: Joi.object().pattern(/\w/, Joi.object().pattern(/\w/, Joi.func())),
+  typeDefs: Joi.string()
+};
+
 /**
  * isInternal checks to see if a given plugin is internal, and returns true
  * if it is.
@@ -122,6 +137,15 @@ class PluginSection {
   hook(hook) {
     return this.plugins
       .filter(({module}) => hook in module)
+      .filter((plugin) => {
+
+        // Validate the hook.
+        if (hook in hookSchemas) {
+          Joi.assert(plugin.module[hook], hookSchemas[hook], `Plugin '${plugin.name}' failed validation for the '${hook}' hook`);
+        }
+
+        return true;
+      })
       .map((plugin) => ({
         plugin,
         [hook]: plugin.module[hook]
diff --git a/routes/index.js b/routes/index.js
index b7c08694f..448bd6964 100644
--- a/routes/index.js
+++ b/routes/index.js
@@ -1,6 +1,5 @@
 const express = require('express');
 const path = require('path');
-const Joi = require('joi');
 const plugins = require('../services/plugins');
 const debug = require('debug')('talk:routes');
 
@@ -28,8 +27,6 @@ if (process.env.NODE_ENV !== 'production') {
 
 // Inject server route plugins.
 plugins.get('server', 'router').forEach((plugin) => {
-  Joi.assert(plugin.router, Joi.func().arity(1), `Plugin '${plugin.name}' had an error loading the passport router`);
-
   debug(`added plugin '${plugin.plugin.name}'`);
 
   // Pass the root router to the plugin to mount it's routes.