From b823a6eb0c9fad7468d85d7699440ed1c701af5d Mon Sep 17 00:00:00 2001
From: Jeffrey Pope <jpopeufl@gmail.com>
Date: Mon, 23 Nov 2020 12:58:12 -0500
Subject: [PATCH 1/5] add soft validation for page put

---
 lib/services/pages.js           |  7 ++-
 lib/services/validation.js      | 77 +++++++++++++++++++++++++++++++++
 lib/services/validation.test.js |  0
 3 files changed, 80 insertions(+), 4 deletions(-)
 create mode 100644 lib/services/validation.js
 create mode 100644 lib/services/validation.test.js

diff --git a/lib/services/pages.js b/lib/services/pages.js
index 51e78a08..926276e2 100644
--- a/lib/services/pages.js
+++ b/lib/services/pages.js
@@ -21,6 +21,7 @@ const _ = require('lodash'),
   { getComponentName, replaceVersion, getPrefix, isLayout } = require('clayutils'),
   publishService = require('./publish'),
   bus = require('./bus'),
+  validation = require('./validation'),
   timeoutPublishCoefficient = 5;
 
 /**
@@ -334,10 +335,8 @@ function create(uri, data, locals) {
 function putLatest(uri, data, locals) {
   const user = locals && locals.user;
 
-  // check the page for a proper layout
-  if (!data.layout || !isLayout(data.layout)) {
-    throw Error('Page must contain a `layout` property whose value is a `_layouts` instance');
-  }
+  // should we promisify and add to chain?
+  validation.validatePage(uri, data, locals);
 
   // continue saving the page normally
   return db.getLatestData(uri)
diff --git a/lib/services/validation.js b/lib/services/validation.js
new file mode 100644
index 00000000..af083635
--- /dev/null
+++ b/lib/services/validation.js
@@ -0,0 +1,77 @@
+'use strict';
+
+const sites = require('./sites'),
+  references = require('./references'),
+  { getPrefix, isPage, isComponent, isLayout } = require('clayutils');
+
+/**
+ * 
+ * @param {string} uri
+ * @param {Object} locals
+ * @throws {Error}
+ * @returns {boolean}
+ */
+function validateSite(uri, locals) {
+  const uriSite = sites.getSiteFromPrefix(getPrefix(uri)),
+    site = locals && locals.site;
+
+  if (!uriSite) {
+    throw new Error(`Site for URI not found, ${uri}`);
+  }
+
+  if (!site) {
+    throw new Error('Site not found on locals.');
+  }
+
+  const uriSlug = uriSite.subsiteSlug || uriSite.slug,
+    siteSlug = site.subsiteSlug || site.slug;
+
+  if (uriSlug !== siteSlug) {
+    throw new Error(`URI Site (${uriSlug}) not the same as current site (${siteSlug})`);
+  }
+}
+
+/** 
+ * @param {string} uri
+ * @param {Object} data
+ * @param {Object}
+ */
+function validateComponent(uri, data, locals) {
+  // uri is from site
+  validateSite(uri, locals);
+
+  // all references are from site
+}
+
+function validatePage(uri, data, locals) {
+  if (!uri || !isPage(uri)) {
+    throw new Error(`Client: Page URI invalid, ${uri}`);
+  }
+
+  // page is for this site
+  validateSite(uri, locals);
+
+  // page has a layout
+  const layout = data && data.layout;
+
+  if (!layout || !isLayout(layout)) {
+    throw new Error('Client: Data missing layout reference.');
+  }
+
+  // check to make sure it's from this site
+  validateSite(layout, locals);
+
+  // all component references are valid and are from this site
+  const componentList = references.getPageReferences(data);
+
+  for (let i = 0; i < componentList.length; i++) {
+    if (!isComponent(componentList[i])) {
+      throw new Error(`Page references a non-valid component: ${componentList[i]}`);
+    }
+
+    validateSite(componentList[i], locals);
+  }
+}
+
+module.exports.validateComponent = validateComponent;
+module.exports.validatePage = validatePage;
diff --git a/lib/services/validation.test.js b/lib/services/validation.test.js
new file mode 100644
index 00000000..e69de29b

From de063624c01dbaa407f7b73592c843f48d1f60fc Mon Sep 17 00:00:00 2001
From: Jeffrey Pope <jpopeufl@gmail.com>
Date: Thu, 3 Dec 2020 11:47:08 -0500
Subject: [PATCH 2/5] add jsdocs

---
 lib/services/validation.js | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/lib/services/validation.js b/lib/services/validation.js
index af083635..36a9fe46 100644
--- a/lib/services/validation.js
+++ b/lib/services/validation.js
@@ -31,10 +31,11 @@ function validateSite(uri, locals) {
   }
 }
 
-/** 
+/**
+ * soft validation for a component to make sure all references are for the same, current site.
  * @param {string} uri
  * @param {Object} data
- * @param {Object}
+ * @param {Object} locals
  */
 function validateComponent(uri, data, locals) {
   // uri is from site
@@ -43,6 +44,12 @@ function validateComponent(uri, data, locals) {
   // all references are from site
 }
 
+/**
+ * soft validation for a page to make sure all references are for the same, current site.
+ * @param {string} uri
+ * @param {Object} data
+ * @param {Object} locals
+ */
 function validatePage(uri, data, locals) {
   if (!uri || !isPage(uri)) {
     throw new Error(`Client: Page URI invalid, ${uri}`);

From 324aef8c6920e88d75c0de43e8cbc2172bf5ec4c Mon Sep 17 00:00:00 2001
From: Jeffrey Pope <jpopeufl@gmail.com>
Date: Mon, 7 Dec 2020 11:58:06 -0500
Subject: [PATCH 3/5] fix eslint, add component validation

---
 lib/services/validation.js | 61 ++++++++++++++++++++++++++++++--------
 1 file changed, 48 insertions(+), 13 deletions(-)

diff --git a/lib/services/validation.js b/lib/services/validation.js
index 36a9fe46..c3197330 100644
--- a/lib/services/validation.js
+++ b/lib/services/validation.js
@@ -5,15 +5,25 @@ const sites = require('./sites'),
   { getPrefix, isPage, isComponent, isLayout } = require('clayutils');
 
 /**
- * 
+ * gets the site's slug identifier from a site obj
+ * @param {Object} site
+ * @returns {string}
+ */
+function getSiteSlug(site) {
+  return site && (site.subsiteSlug || site.slug);
+}
+
+/**
+ * soft validation to make sure a uri is for the current site
  * @param {string} uri
  * @param {Object} locals
  * @throws {Error}
- * @returns {boolean}
  */
 function validateSite(uri, locals) {
   const uriSite = sites.getSiteFromPrefix(getPrefix(uri)),
-    site = locals && locals.site;
+    site = locals && locals.site,
+    uriSlug = getSiteSlug(uriSite),
+    siteSlug = getSiteSlug(site);
 
   if (!uriSite) {
     throw new Error(`Site for URI not found, ${uri}`);
@@ -23,14 +33,23 @@ function validateSite(uri, locals) {
     throw new Error('Site not found on locals.');
   }
 
-  const uriSlug = uriSite.subsiteSlug || uriSite.slug,
-    siteSlug = site.subsiteSlug || site.slug;
-
   if (uriSlug !== siteSlug) {
     throw new Error(`URI Site (${uriSlug}) not the same as current site (${siteSlug})`);
   }
 }
 
+/**
+ * checks to see if a component property is a reference to another component
+ * if it is, it validates that it is a valid reference
+ * @param {Object} value
+ * @param {Object} locals
+ */
+function isComponentReference(value, locals) {
+  if (value && value._ref) {
+    validateComponent(value._ref, value, locals);
+  }
+}
+
 /**
  * soft validation for a component to make sure all references are for the same, current site.
  * @param {string} uri
@@ -41,7 +60,26 @@ function validateComponent(uri, data, locals) {
   // uri is from site
   validateSite(uri, locals);
 
-  // all references are from site
+  // check to see if this component references other components. if so, recursively validate that component
+  if (data) {
+    for (const [, value] of Object.entries(data)) {
+      // if this is an object with a _ref property, verify _ref is for this site
+      // ie:
+      // property: { _ref: 'site.com/_components/test/instances/id' }
+      isComponentReference(value, locals);
+
+      // we can also  have arrays of references:
+      // property: [
+      //   { _ref: 'site.com/_components/test/instances/id-1' },
+      //   { _ref: 'site.com/_components/test/instances/id-2' }
+      // ]
+      if (Array.isArray(value)) {
+        for (let i = 0; i < value.length; i++) {
+          isComponentReference(value[i], locals);
+        }
+      }
+    }
+  }
 }
 
 /**
@@ -51,6 +89,9 @@ function validateComponent(uri, data, locals) {
  * @param {Object} locals
  */
 function validatePage(uri, data, locals) {
+  const layout = data && data.layout,
+    componentList = references.getPageReferences(data);
+
   if (!uri || !isPage(uri)) {
     throw new Error(`Client: Page URI invalid, ${uri}`);
   }
@@ -58,9 +99,6 @@ function validatePage(uri, data, locals) {
   // page is for this site
   validateSite(uri, locals);
 
-  // page has a layout
-  const layout = data && data.layout;
-
   if (!layout || !isLayout(layout)) {
     throw new Error('Client: Data missing layout reference.');
   }
@@ -68,9 +106,6 @@ function validatePage(uri, data, locals) {
   // check to make sure it's from this site
   validateSite(layout, locals);
 
-  // all component references are valid and are from this site
-  const componentList = references.getPageReferences(data);
-
   for (let i = 0; i < componentList.length; i++) {
     if (!isComponent(componentList[i])) {
       throw new Error(`Page references a non-valid component: ${componentList[i]}`);

From dd07c86af06d8037a1ce67d7338d73ba608eb134 Mon Sep 17 00:00:00 2001
From: Jeffrey Pope <jpopeufl@gmail.com>
Date: Tue, 8 Dec 2020 12:10:38 -0500
Subject: [PATCH 4/5] fix tests

---
 lib/bootstrap.test.js           |  2 ++
 lib/services/components.js      |  8 +++++++
 lib/services/components.test.js |  2 ++
 lib/services/pages.test.js      |  6 ++++--
 lib/services/validation.js      | 38 ++++++---------------------------
 5 files changed, 23 insertions(+), 33 deletions(-)

diff --git a/lib/bootstrap.test.js b/lib/bootstrap.test.js
index 5544583d..d8c597de 100644
--- a/lib/bootstrap.test.js
+++ b/lib/bootstrap.test.js
@@ -6,6 +6,7 @@ const _ = require('lodash'),
   filename = __filename.split('/').pop().split('.').shift(),
   lib = require('./bootstrap'),
   siteService = require('./services/sites'),
+  validationService = require('./services/validation'),
   expect = require('chai').expect,
   sinon = require('sinon'),
   storage = require('../test/fixtures/mocks/storage');
@@ -52,6 +53,7 @@ describe(_.startCase(filename), function () {
     lib.setLog(fakeLog);
     sandbox.stub(siteService);
     siteService.sites.returns(_.cloneDeep(sitesFake));
+    sandbox.stub(validationService);
     db = storage();
     lib.setDb(db);
     db.batch.callsFake(db.batchToInMem); // we want to make sure to send to the actual in-mem batch
diff --git a/lib/services/components.js b/lib/services/components.js
index 0485986c..074b9a39 100644
--- a/lib/services/components.js
+++ b/lib/services/components.js
@@ -6,6 +6,7 @@ const _ = require('lodash'),
   files = require('../files'),
   models = require('./models'),
   dbOps = require('./db-operations'),
+  validation = require('./validation'),
   { getComponentName, replaceVersion } = require('clayutils');
 
 /**
@@ -42,6 +43,8 @@ function put(uri, data, locals) {
     callHooks = _.get(locals, 'hooks') !== 'false',
     result;
 
+  validation.validateComponent(uri, data, locals);
+
   if (model && _.isFunction(model.save) && callHooks) {
     result = models.put(model, uri, data, locals);
   } else {
@@ -60,6 +63,8 @@ function put(uri, data, locals) {
  */
 function publish(uri, data, locals) {
   if (data && _.size(data) > 0) {
+    validation.validateComponent(uri, data, locals);
+
     return dbOps.cascadingPut(put)(uri, data, locals);
   }
 
@@ -76,6 +81,9 @@ function publish(uri, data, locals) {
  */
 function post(uri, data, locals) {
   uri += '/' + uid.get();
+
+  validation.validateComponent(uri, data, locals);
+
   return dbOps.cascadingPut(put)(uri, data, locals)
     .then(result => {
       result._ref = uri;
diff --git a/lib/services/components.test.js b/lib/services/components.test.js
index 0fe911b5..4b433afa 100644
--- a/lib/services/components.test.js
+++ b/lib/services/components.test.js
@@ -6,6 +6,7 @@ const _ = require('lodash'),
   sinon = require('sinon'),
   files = require('../files'),
   siteService = require('./sites'),
+  validationService = require('./validation'),
   composer = require('./composer'),
   models = require('./models'),
   dbOps = require('./db-operations'),
@@ -19,6 +20,7 @@ describe(_.startCase(filename), function () {
 
     sandbox.stub(composer, 'resolveComponentReferences');
     sandbox.stub(siteService);
+    sandbox.stub(validationService);
     sandbox.stub(files, 'getComponentModule');
     sandbox.stub(models, 'put');
     sandbox.stub(models, 'get');
diff --git a/lib/services/pages.test.js b/lib/services/pages.test.js
index abae556f..a6873421 100644
--- a/lib/services/pages.test.js
+++ b/lib/services/pages.test.js
@@ -10,6 +10,7 @@ const _ = require('lodash'),
   notifications = require('./notifications'),
   sinon = require('sinon'),
   siteService = require('./sites'),
+  validationService = require('./validation'),
   timer = require('../timer'),
   meta = require('./metadata'),
   schema = require('../schema'),
@@ -31,6 +32,7 @@ describe(_.startCase(filename), function () {
     sandbox.stub(components, 'get');
     sandbox.stub(layouts, 'get');
     sandbox.stub(siteService, 'getSiteFromPrefix');
+    sandbox.stub(validationService);
     sandbox.stub(notifications, 'notify');
     sandbox.stub(dbOps);
     sandbox.stub(timer);
@@ -375,7 +377,7 @@ describe(_.startCase(filename), function () {
       db.getLatestData.returns(Promise.resolve({}));
       db.put.returns(Promise.resolve());
 
-      return fn('domain.com/path/pages', {layout: 'domain.com/path/_layouts/thing'})
+      return fn('domain.com/path/_pages', {layout: 'domain.com/path/_layouts/thing'})
         .then(() => {
           sinon.assert.notCalled(bus.publish);
         });
@@ -385,7 +387,7 @@ describe(_.startCase(filename), function () {
       db.getLatestData.returns(Promise.reject());
       db.put.returns(Promise.resolve());
 
-      return fn('domain.com/path/pages', {layout: 'domain.com/path/_layouts/thing'}).then(function () {
+      return fn('domain.com/path/_pages', {layout: 'domain.com/path/_layouts/thing'}).then(function () {
         sinon.assert.calledOnce(bus.publish);
         expect(bus.publish.getCall(0).args[0]).to.equal('createPage');
       });
diff --git a/lib/services/validation.js b/lib/services/validation.js
index c3197330..573d66ac 100644
--- a/lib/services/validation.js
+++ b/lib/services/validation.js
@@ -38,18 +38,6 @@ function validateSite(uri, locals) {
   }
 }
 
-/**
- * checks to see if a component property is a reference to another component
- * if it is, it validates that it is a valid reference
- * @param {Object} value
- * @param {Object} locals
- */
-function isComponentReference(value, locals) {
-  if (value && value._ref) {
-    validateComponent(value._ref, value, locals);
-  }
-}
-
 /**
  * soft validation for a component to make sure all references are for the same, current site.
  * @param {string} uri
@@ -60,24 +48,12 @@ function validateComponent(uri, data, locals) {
   // uri is from site
   validateSite(uri, locals);
 
-  // check to see if this component references other components. if so, recursively validate that component
+  // make sure all _refs are for the same current site
   if (data) {
-    for (const [, value] of Object.entries(data)) {
-      // if this is an object with a _ref property, verify _ref is for this site
-      // ie:
-      // property: { _ref: 'site.com/_components/test/instances/id' }
-      isComponentReference(value, locals);
-
-      // we can also  have arrays of references:
-      // property: [
-      //   { _ref: 'site.com/_components/test/instances/id-1' },
-      //   { _ref: 'site.com/_components/test/instances/id-2' }
-      // ]
-      if (Array.isArray(value)) {
-        for (let i = 0; i < value.length; i++) {
-          isComponentReference(value[i], locals);
-        }
-      }
+    const refs = references.listDeepObjects(data, '_ref');
+
+    for (const ref of refs) {
+      validateComponent(ref._ref, ref, locals);
     }
   }
 }
@@ -93,14 +69,14 @@ function validatePage(uri, data, locals) {
     componentList = references.getPageReferences(data);
 
   if (!uri || !isPage(uri)) {
-    throw new Error(`Client: Page URI invalid, ${uri}`);
+    throw new Error(`Page URI invalid, '${uri}'`);
   }
 
   // page is for this site
   validateSite(uri, locals);
 
   if (!layout || !isLayout(layout)) {
-    throw new Error('Client: Data missing layout reference.');
+    throw new Error('Page must contain a `layout` property whose value is a `_layouts` instance');
   }
 
   // check to make sure it's from this site

From 2e26791e467895599af555dcb0ab1590ff8320d9 Mon Sep 17 00:00:00 2001
From: Jeffrey Pope <jpopeufl@gmail.com>
Date: Tue, 8 Dec 2020 13:20:23 -0500
Subject: [PATCH 5/5] test coverage

---
 lib/services/validation.js      |   3 +-
 lib/services/validation.test.js | 152 ++++++++++++++++++++++++++++++++
 2 files changed, 154 insertions(+), 1 deletion(-)

diff --git a/lib/services/validation.js b/lib/services/validation.js
index 573d66ac..af5f009e 100644
--- a/lib/services/validation.js
+++ b/lib/services/validation.js
@@ -87,9 +87,10 @@ function validatePage(uri, data, locals) {
       throw new Error(`Page references a non-valid component: ${componentList[i]}`);
     }
 
-    validateSite(componentList[i], locals);
+    validateComponent(componentList[i], null, locals);
   }
 }
 
+module.exports.validateSite = validateSite;
 module.exports.validateComponent = validateComponent;
 module.exports.validatePage = validatePage;
diff --git a/lib/services/validation.test.js b/lib/services/validation.test.js
index e69de29b..377e3bea 100644
--- a/lib/services/validation.test.js
+++ b/lib/services/validation.test.js
@@ -0,0 +1,152 @@
+'use strict';
+
+const _ = require('lodash'),
+  filename = __filename.split('/').pop().split('.').shift(),
+  lib = require('./' + filename),
+  sinon = require('sinon'),
+  siteService = require('./sites'),
+  expect = require('chai').expect,
+  locals = {
+    site: {
+      slug: 'domain'
+    }
+  };
+
+describe(_.startCase(filename), function () {
+  let sandbox;
+
+  beforeEach(function () {
+    sandbox = sinon.sandbox.create();
+
+    sandbox.stub(siteService, 'getSiteFromPrefix').callsFake(function fake(prefix) {
+      const sites = {
+        'domain.com': locals.site,
+        'other.com': { slug: 'notit' }
+      };
+
+      return sites[prefix];
+    });
+  });
+
+  afterEach(function () {
+    sandbox.restore();
+  });
+
+  describe('validateSite', function () {
+    const fn = lib.validateSite;
+
+    it('will not throw if no error', function () {
+      const uri = 'domain.com/_pages/foo';
+
+      fn(uri, locals);
+      sinon.assert.calledOnce(siteService.getSiteFromPrefix);
+    });
+
+    it('will throw if site does not exist for uri', function () {
+      const uri = 'idontexist.com/_pages/foo';
+
+      try {
+        fn(uri, locals);
+      } catch (e) {
+        expect(e.message).to.eql(`Site for URI not found, ${uri}`);
+      }
+    });
+
+    it('will throw if locals does not have a site', function () {
+      const uri = 'domain.com/_pages/foo';
+
+      try {
+        fn(uri, {});
+      } catch (e) {
+        expect(e.message).to.eql('Site not found on locals.');
+      }
+    });
+
+    it('will throw if uri different site than locals', function () {
+      const uri = 'other.com/_pages/foo';
+
+      try {
+        fn(uri, locals);
+      } catch (e) {
+        expect(e.message).to.eql('URI Site (notit) not the same as current site (domain)');
+      }
+    });
+  });
+
+  describe('validatePage', function () {
+    const fn = lib.validatePage;
+
+    it('will not throw if no error', function () {
+      const uri = 'domain.com/_pages/foo',
+        data = {
+          head: [
+            'domain.com/_components/header/instances/foo'
+          ],
+          main:[
+            'domain.com/_components/article/instances/foo'
+          ],
+          layout: 'domain.com/_layouts/layout/instances/article'
+        };
+
+      fn(uri, data, locals);
+      sinon.assert.callCount(siteService.getSiteFromPrefix, 4);
+    });
+
+    it('will throw error if page URI invalid', function () {
+      const uri = 'domain.com/im/not/a/page/uri',
+        data = {
+          head: [
+            'domain.com/_components/header/instances/foo'
+          ],
+          main:[
+            'domain.com/_components/article/instances/foo'
+          ],
+          layout: 'domain.com/_layouts/layout/instances/article'
+        };
+
+      try {
+        fn(uri, data, locals);
+      } catch (e) {
+        expect(e.message).to.eql(`Page URI invalid, '${uri}'`);
+      }
+    });
+
+    it('will throw error if layout URI invalid', function () {
+      const uri = 'domain.com/_pages/foo',
+        data = {
+          head: [
+            'domain.com/_components/header/instances/foo'
+          ],
+          main:[
+            'domain.com/_components/article/instances/foo'
+          ],
+          layout: 'domain.com/im/not/a/layout'
+        };
+
+      try {
+        fn(uri, data, locals);
+      } catch (e) {
+        expect(e.message).to.eql('Page must contain a `layout` property whose value is a `_layouts` instance');
+      }
+    });
+
+    it('will not throw if no error', function () {
+      const uri = 'domain.com/_pages/foo',
+        data = {
+          head: [
+            'domain.com/_components/header/instances/foo'
+          ],
+          main:[
+            'domain.com/im/not/a/component'
+          ],
+          layout: 'domain.com/_layouts/layout/instances/article'
+        };
+
+      try {
+        fn(uri, data, locals);
+      } catch (e) {
+        expect(e.message).to.eql('Page references a non-valid component: domain.com/im/not/a/component');
+      }
+    });
+  });
+});