Separate verification logic and add QueryVerifier

app/routes/api/auth.js

@@ -12,7 +12,7 @@ const passport = require('passport');
 const canonicalizeRequest = require('../../util/canonicalize').canonicalizeRequest;
 const requireAuth = require('../../util/auth').requireAuth;
 const wrap = require('../../util/wrap.js');
-const bodyVerifier = require('../../util/verifyBody').bodyVerifier;
+const verifyBody = require('../../util/verifyBody');
 const rateLimit = require('express-rate-limit');
 
 // Wraps passport.authenticate to return a promise
@@ -81,7 +81,7 @@ const registerProps = [
     {name: 'invite', type: 'string'}];
 router.post('/register',
     registerLimiter,
-    bodyVerifier(registerProps), canonicalizeRequest,
+    verifyBody(registerProps), canonicalizeRequest,
     validateInvite, validateUsername,
     wrap(async (req, res, next) => {
     // Update the database
@@ -111,7 +111,7 @@ const loginProps = [
     {name: 'password', type: 'string'}];
 router.post('/login',
     loginLimiter,
-    bodyVerifier(loginProps),
+    verifyBody(loginProps),
     canonicalizeRequest,
     wrap(async (req, res, next) => {
     // Authenticate

app/routes/api/invites.js

@@ -9,10 +9,10 @@ const User = require(ModelPath + 'User.js');
 const wrap = require('../../util/wrap.js');
 const requireAuth = require('../../util/auth').requireAuth;
 const verifyScope = require('../../util/verifyScope');
-const bodyVerifier = require('../../util/verifyBody').bodyVerifier;
+const verifyBody = require('../../util/verifyBody');
 
 const createParams = [{name: 'scope', instance: Array}];
-router.post('/create', requireAuth('invite.create'), bodyVerifier(createParams), wrap(async (req, res, next) => {
+router.post('/create', requireAuth('invite.create'), verifyBody(createParams), wrap(async (req, res, next) => {
     const scope = req.body.scope;
     if (!scope.every(scope => verifyScope(req.scope, scope)))
         return res.status(403).json({message: 'Requested scope exceeds own scope.'});
@@ -37,7 +37,7 @@ router.post('/create', requireAuth('invite.create'), bodyVerifier(createParams),
 }));
 
 const deleteParams = [{name: 'code', type: 'string'}];
-router.post('/delete', requireAuth('invite.delete'), bodyVerifier(deleteParams), wrap(async (req, res, next) => {
+router.post('/delete', requireAuth('invite.delete'), verifyBody(deleteParams), wrap(async (req, res, next) => {
     let query = {code: req.body.code};
 
     // Users need a permission to delete invites other than their own
@@ -58,7 +58,7 @@ router.post('/delete', requireAuth('invite.delete'), bodyVerifier(deleteParams),
 }));
 
 const getParams = [{name: 'code', type: 'string', optional: true}, {name: 'issuer', type: 'string', optional: true}];
-router.get('/get', requireAuth('invite.get'), bodyVerifier(getParams), wrap(async (req, res, next) => {
+router.get('/get', requireAuth('invite.get'), verifyBody(getParams), wrap(async (req, res, next) => {
     let query = {};
 
     // Users need a permission to list invites other than their own

app/routes/api/keys.js

@@ -7,14 +7,14 @@ const ModelPath = '../../models/';
 const Key = require(ModelPath + 'Key.js');
 
 const wrap = require('../../util/wrap');
-const bodyVerifier = require('../../util/verifyBody').bodyVerifier;
+const verifyBody = require('../../util/verifyBody');
 const verifyScope = require('../../util/verifyScope');
 const requireAuth = require('../../util/auth').requireAuth;
 
 const createParams = [
     {name: 'identifier', type: 'string', sanitize: true},
     {name: 'scope', instance: Array}];
-router.post('/create', requireAuth('key.create'), bodyVerifier(createParams), wrap(async (req, res) => {
+router.post('/create', requireAuth('key.create'), verifyBody(createParams), wrap(async (req, res) => {
     const keyCount = await Key.countDocuments({issuer: req.username});
     if (keyCount >= config.get('Key.limit'))
         return res.status(403).json({message: 'Key limit reached.'});
@@ -42,7 +42,7 @@ router.post('/create', requireAuth('key.create'), bodyVerifier(createParams), wr
 const getProps = [
     {name: 'identifier', type: 'string', optional: true},
     {name: 'issuer', type: 'string', optional: true}];
-router.get('/get', requireAuth('key.get'), bodyVerifier(getProps), wrap(async (req, res) => {
+router.get('/get', requireAuth('key.get'), verifyBody(getProps), wrap(async (req, res) => {
     let query = {};
 
     if (req.body.identifier)
@@ -61,7 +61,7 @@ router.get('/get', requireAuth('key.get'), bodyVerifier(getProps), wrap(async (r
 const deleteProps = [
     {name: 'key', type: 'string'},
     {name: 'issuer', type: 'string', optional: true}];
-router.post('/delete', requireAuth('key.delete'), bodyVerifier(deleteProps), wrap(async (req, res) => {
+router.post('/delete', requireAuth('key.delete'), verifyBody(deleteProps), wrap(async (req, res) => {
     let query = {key : req.body.key};
 
     if (!verifyScope(req.scope, 'key.delete.others'))

app/routes/api/stats.js

@@ -6,7 +6,7 @@ const Upload = require(ModelPath + 'Upload.js');
 const View = require(ModelPath + 'View.js');
 
 const wrap = require('../../util/wrap');
-const bodyVerifier = require('../../util/verifyBody').bodyVerifier;
+const verifyBody = require('../../util/verifyBody');
 const requireAuth = require('../../util/auth').requireAuth;
 
 const uploadProps = [
@@ -15,7 +15,7 @@ const uploadProps = [
     {name: 'limit', type: 'number', min: 1, max: 10000, optional: true}
 ];
 
-router.get('/uploads', requireAuth('stats.get'), bodyVerifier(uploadProps), wrap(async (req, res) => {
+router.get('/uploads', requireAuth('stats.get'), verifyBody(uploadProps), wrap(async (req, res) => {
     let constraints = {uploader: req.username};
 
     // Set date constraints if specified
@@ -56,7 +56,7 @@ const viewProps = [
     {name: 'limit', type: 'number', min: 1, max: 10000, optional: true}
 ];
 
-router.get('/views', requireAuth('stats.get'), bodyVerifier(viewProps), wrap(async (req, res) => {
+router.get('/views', requireAuth('stats.get'), verifyBody(viewProps), wrap(async (req, res) => {
     let constraints = {uploader: req.username};
 
     // Set date constraints if specified

app/routes/api/users.js

@@ -5,13 +5,13 @@ const ModelPath = '../../models/';
 const User = require(ModelPath + 'User.js');
 
 const wrap = require('../../util/wrap');
-const bodyVerifier = require('../../util/verifyBody').bodyVerifier;
+const verifyBody = require('../../util/verifyBody');
 const requireAuth = require('../../util/auth').requireAuth;
 
 const getParams = [
     {name: 'username', type: 'string', optional: true},
     {name: 'displayname', type: 'string', optional: true}];
-router.get('/get', requireAuth('user.get'), bodyVerifier(getParams), wrap(async (req, res) => {
+router.get('/get', requireAuth('user.get'), verifyBody(getParams), wrap(async (req, res) => {
     let query = {};
 
     if (req.body.username)
@@ -27,7 +27,7 @@ router.get('/get', requireAuth('user.get'), bodyVerifier(getParams), wrap(async
 }));
 
 const banParams = [{name: 'username', type: 'string'}];
-router.post('/ban', requireAuth('user.ban'), bodyVerifier(banParams), wrap(async (req, res) => {
+router.post('/ban', requireAuth('user.ban'), verifyBody(banParams), wrap(async (req, res) => {
     const user = await User.findOne({username: req.body.username});
     if (!user)
         return res.status(422).json({message: 'User not found.'});
@@ -42,7 +42,7 @@ router.post('/ban', requireAuth('user.ban'), bodyVerifier(banParams), wrap(async
 }));
 
 const unbanParams = [{name: 'username', type: 'string'}];
-router.post('/unban', requireAuth('user.unban'), bodyVerifier(unbanParams), wrap(async (req, res) => {
+router.post('/unban', requireAuth('user.unban'), verifyBody(unbanParams), wrap(async (req, res) => {
     const user = await User.findOne({username: req.body.username});
     if (!user)
         return res.status(422).json({message: 'User not found.'});

app/util/verify.js

@@ -0,0 +1,46 @@
+const sanitizer = require('sanitizer');
+
+// Verifies a single property is well formed.
+// expected follows the format:
+// ex. {name: 'myList', type: 'array', maxLength: 10}
+// ex. {name: 'myVar', type: 'string', optional: true}
+const verify = async (prop, expected) => {
+    if (!expected.optional && !prop)
+        throw {code: 400, message: expected.name + ' not specified.'};
+    else if (!prop)
+        return;
+
+    if (expected.type) {
+        if (expected.type === 'date') {
+            if (isNaN(new Date(prop)))
+                throw {code: 400, message: `${expected.name} malformed.`};
+        } else if (expected.type === 'array') {
+            if (!(prop instanceof Array))
+                throw {code: 400, message: `${expected.name} malformed.`};
+        } else if (expected.type === 'number') {
+            if (isNaN(parseInt(prop)))
+                throw {code: 400, message: `${expected.name} malformed.`};
+        } else {
+            if (typeof prop !== expected.type)
+                throw {code: 400, message: `${expected.name} malformed.`};
+        }
+    }
+
+
+    if (expected.min && parseInt(prop) < expected.min)
+        throw {code: 400, message: `${expected.name} too small.`};
+
+    if (expected.max && parseInt(prop) > expected.max)
+        throw {code: 400, message: `${expected.name} too large.`};
+
+    if (expected.maxLength && prop.length > expected.maxLength)
+        throw {code: 400, message: `${expected.name} too long.`};
+
+    if (expected.sanitize && sanitizer.sanitize(prop) !== prop)
+        throw {code: 400, message: `${expected.name} contains invalid characters.`};
+
+    if (expected.restrict && prop.replace(expected.restrict, '') !== prop)
+        throw {code: 400, message: `${expected.name} contains invalid characters.`};
+};
+
+module.exports = verify;

app/util/verifyBody.js

@@ -1,57 +1,14 @@
-const sanitizer = require('sanitizer');
-
-// Verifies a single property is well formed
-const verifyProp = async (prop, expected) => {
-    if (!expected.optional && !prop)
-        throw {code: 400, message: expected.name + ' not specified.'};
-    else if (!prop)
-        return;
-
-    if (expected.type) {
-        if (expected.type === 'date') {
-            if (isNaN(new Date(prop)))
-                throw {code: 400, message: `${expected.name} malformed.`};
-        } else if (expected.type === 'array') {
-            if (!(prop instanceof Array))
-                throw {code: 400, message: `${expected.name} malformed.`};
-        } else if (expected.type === 'number') {
-            if (isNaN(parseInt(prop)))
-                throw {code: 400, message: `${expected.name} malformed.`};
-        } else {
-            if (typeof prop !== expected.type)
-                throw {code: 400, message: `${expected.name} malformed.`};
-        }
-    }
-
-
-    if (expected.min && parseInt(prop) < expected.min)
-        throw {code: 400, message: `${expected.name} too small.`};
-
-    if (expected.max && parseInt(prop) > expected.max)
-        throw {code: 400, message: `${expected.name} too large.`};
-
-    if (expected.maxLength && prop.length > expected.maxLength)
-        throw {code: 400, message: `${expected.name} too long.`};
-
-    if (expected.sanitize && sanitizer.sanitize(prop) !== prop)
-        throw {code: 400, message: `${expected.name} contains invalid characters.`};
-
-    if (expected.restrict && prop.replace(expected.restrict, '') !== prop)
-        throw {code: 400, message: `${expected.name} contains invalid characters.`};
-};
+const verify = require('./verify.js');
+const wrap = require('./wrap.js');
 
 // Verifies the entire request body is well formed
-// expectedProps follows the format:
-// [{name: 'myList', instance: 'Array'}, {name: 'myVar', type: 'string', optional: true}, etc.]
-const verifyBody = (body, expectedProps) =>
-    Promise.all(expectedProps.map(expected => verifyProp(body[expected.name], expected)));
-
-const bodyVerifier = expectedProps =>
-    (req, res, next) => {
-        verifyBody(req.body, expectedProps)
-            .then(() => next())
-            .catch(err => res.status(err.code).json({message: err.message}));
-    };
+const verifyBody = expected => wrap(async (req, res, next) => {
+    try {
+        await Promise.all(expected.map(e => verify(req.body[e.name], e)));
+        next();
+    } catch(err) {
+        res.status(err.code).json({message: err.message});
+    }
+});
 
-exports.verifyBody = verifyBody;
-exports.bodyVerifier = bodyVerifier;
+module.exports = verifyBody;

app/util/verifyQuery.js

@@ -0,0 +1,14 @@
+const verify = require('./verify.js');
+const wrap = require('./wrap.js');
+
+// Verifies the entire request query is well formed
+const verifyQuery = expected => wrap(async (req, res, next) => {
+    try {
+        await Promise.all(expected.map(e => verify(req.query[e.name], e)));
+        next();
+    } catch(err) {
+        res.status(err.code).json({message: err.message});
+    }
+});
+
+module.exports = verifyQuery;

test/middleware.js

@@ -3,88 +3,66 @@ chai.use(require('chai-http'));
 const should = chai.should();
 const describe = require('mocha').describe;
 
-const verifyBody = require('../app/util/verifyBody').verifyBody;
+const verify = require('../app/util/verify.js');
 
 describe('Body Verification', () => {
-    const testVerifyBody = async (body, expected, code, message) => {
+    const testVerify = async (prop, expected, code, message) => {
         try {
-            await verifyBody(body, expected);
+            await verify(prop, expected);
         } catch (err) {
-            if (code)
-                err.code.should.equal(code);
-            if (message)
-                err.message.should.equal(message);
+            err.code.should.equal(code);
+            err.message.should.equal(message);
         }
     };
 
     it('must continue properly with valid prop', () => {
         const tests = [{
-            expected: [{name: 'test'}],
-            body: {test: 'test'}
+            expected: {name: 'test'},
+            prop: 'test'
         }, {
-            expected: [{name: 'test', type: 'array'}],
-            body: {test: [1, 2, 3]}
+            expected: {name: 'test', type: 'array'},
+            prop: ['1', '2', '3']
         }, {
-            expected: [{name: 'test', type: 'date'}],
-            body: {test: '11/12/2018'}
+            expected: {name: 'test', type: 'date'},
+            prop: '11/12/2018'
         }, {
-            expected: [{name: 'test', type: 'number'}],
-            body: {test: '1546368715'}
+            expected: {name: 'test', type: 'number'},
+            prop: '1546368715'
         }, {
-            expected: [{name: 'test', type: 'number', min: 12, max: 16}],
-            body: {test: '16'}
+            expected: {name: 'test', type: 'number', min: 12, max: 16},
+            prop: '16'
         }];
 
-        return Promise.all(tests.map(test => testVerifyBody(test.body, test.expected)));
+        return Promise.all(tests.map(test => testVerify(test.prop, test.expected)));
     });
 
-    it('must continue with a missing but optional prop', () => {
-        const expected = [{name: 'test', optional: true}];
-        return testVerifyBody({}, expected);
-    });
+    it('must continue with a missing but optional prop', () =>
+        testVerify(undefined, {name: 'test', optional: true}));
 
-    it('must error with a missing prop', () => {
-        const expected = [{name: 'test'}];
-        return testVerifyBody({}, expected, 400, 'test not specified.');
-    });
+    it('must error with a missing prop', () =>
+        testVerify(undefined, {name: 'test'}, 400, 'test not specified.'));
 
-    it('must error with an invalid primitive type', () => {
-        const expected = [{name: 'test', type: 'string'}];
-        return testVerifyBody({test: [1, 2, 3]}, expected, 400, 'test malformed.');
-    });
+    it('must error with an invalid primitive type', () =>
+        testVerify(['1', '2', '3'], {name: 'test', type: 'string'}, 400, 'test malformed.'));
 
-    it('must error with an invalid date type', () => {
-        const expected = [{name: 'test', type: 'date'}];
-        return testVerifyBody({test: '123abc'}, expected, 400, 'test malformed.');
-    });
+    it('must error with an invalid date type', () =>
+        testVerify('123abc', {name: 'test', type: 'date'}, 400, 'test malformed.'));
 
-    it('must error with an invalid array type', () => {
-        const expected = [{name: 'test', type: 'array'}];
-        return testVerifyBody({test: 'test'}, expected, 400, 'test malformed.');
-    });
+    it('must error with an invalid array type', () =>
+        testVerify('test', {name: 'test', type: 'array'}, 400, 'test malformed.'));
 
-    it('must error when smaller than the minimum', () => {
-        const expected = [{name: 'test', type: 'number', min: 10}];
-        return testVerifyBody({test: 3}, expected, 400, 'test too small.');
-    });
+    it('must error when smaller than the minimum', () =>
+        testVerify('3', {name: 'test', type: 'number', min: 10}, 400, 'test too small.'));
 
-    it('must error when larger than the maximum', () => {
-        const expected = [{name: 'test', type: 'number', max: 10}];
-        return testVerifyBody({test: 15}, expected, 400, 'test too large.');
-    });
+    it('must error when larger than the maximum', () =>
+        testVerify('15', {name: 'test', type: 'number', max: 10}, 400, 'test too large.'));
 
-    it('must error with a length higher than the max', () => {
-        const expected = [{name: 'test', maxLength: 5}];
-        return testVerifyBody({test: '123456'}, expected, 400, 'test too long.');
-    });
+    it('must error with a length higher than the max', () =>
+        testVerify('123456', {name: 'test', maxLength: 5}, 400, 'test too long.'));
 
-    it('must error with a dirty prop that gets sanitized', () => {
-        const expected = [{name: 'test', sanitize: true}];
-        return testVerifyBody({test: 'test<svg/onload=alert("XSS")>'}, expected, 400, 'test contains invalid characters.');
-    });
+    it('must error with a dirty prop that gets sanitized', () =>
+        testVerify('test<svg/onload=alert("XSS")>', {name: 'test', sanitize: true}, 400, 'test contains invalid characters.'));
 
-    it('must error with a restricted character', () => {
-        const expected = [{name: 'test', restrict: new RegExp("\\s")}];
-        return testVerifyBody({test: 'test test'}, expected, 400, 'test contains invalid characters.');
-    })
+    it('must error with a restricted character', () =>
+        testVerify('test test', {name: 'test', restrict: new RegExp("\\s")}, 400, 'test contains invalid characters.'));
 });