From d0c0837a0b23a7c6aa90c99002e3ad0d15d3d75e Mon Sep 17 00:00:00 2001 From: Bryan Goldstein Date: Sat, 19 May 2018 19:06:54 -0400 Subject: [PATCH 1/4] add promise support * use async await this time --- index.js | 88 ++++++++++++++++++++++++-------------------- lib/helpers.js | 52 +++++++++++++------------- test/helpers.test.js | 72 ++++++++++++++++++------------------ 3 files changed, 111 insertions(+), 101 deletions(-) diff --git a/index.js b/index.js index 8f35b77..c9f012e 100644 --- a/index.js +++ b/index.js @@ -10,69 +10,77 @@ const { const PermissionDeniedError = require('./lib/PermissionDeniedError'); module.exports = (schema) => { - function save(doc, options, next) { - if (doc.isNew && !hasPermission(schema, options, 'create', doc)) { - return next(new PermissionDeniedError('create')); + async function save(doc, options, next) { + if (doc.isNew && !await hasPermission(schema, options, 'create', doc)) { + next(new PermissionDeniedError('create')); + return; } - const authorizedFields = getAuthorizedFields(schema, options, 'write', doc); + const authorizedFields = await getAuthorizedFields(schema, options, 'write', doc); const modifiedPaths = doc.modifiedPaths(); const discrepancies = _.difference(modifiedPaths, authorizedFields); if (discrepancies.length > 0) { - return next(new PermissionDeniedError('write', discrepancies)); + next(new PermissionDeniedError('write', discrepancies)); + return; } - return next(); + next(); } - function removeQuery(query, next) { - if (!hasPermission(schema, query.options, 'remove')) { - return next(new PermissionDeniedError('remove')); + async function removeQuery(query, next) { + if (!await hasPermission(schema, query.options, 'remove')) { + next(new PermissionDeniedError('remove')); + return; } - return next(); + next(); + return; } - function removeDoc(doc, options, next) { - if (!hasPermission(schema, options, 'remove', doc)) { - return next(new PermissionDeniedError('remove')); + async function removeDoc(doc, options, next) { + if (!await hasPermission(schema, options, 'remove', doc)) { + next(new PermissionDeniedError('remove')); + return; } - return next(); + next(); + return; } - function find(query, docs, next) { - const sanitizedResult = sanitizeDocumentList(schema, query.options, docs); + async function find(query, docs, next) { + const sanitizedResult = await sanitizeDocumentList(schema, query.options, docs); - return next(null, sanitizedResult); + next(null, sanitizedResult); } - function update(query, next) { + async function update(query, next) { // If this is an upsert, you'll need the create permission // TODO add some tests for the upset case if ( query.options && query.options.upsert - && !hasPermission(schema, query.options, 'create') + && !await hasPermission(schema, query.options, 'create') ) { - return next(new PermissionDeniedError('create')); + next(new PermissionDeniedError('create')); + return; } - const authorizedFields = getAuthorizedFields(schema, query.options, 'write'); + const authorizedFields = await getAuthorizedFields(schema, query.options, 'write'); // check to see if the group is trying to update a field it does not have permission to const modifiedPaths = getUpdatePaths(query._update); const discrepancies = _.difference(modifiedPaths, authorizedFields); if (discrepancies.length > 0) { - return next(new PermissionDeniedError('write', discrepancies)); + next(new PermissionDeniedError('write', discrepancies)); + return; } // TODO handle the overwrite option // TODO handle Model.updateMany // Detect which fields can be returned if 'new: true' is set - const authorizedReturnFields = getAuthorizedFields(schema, query.options, 'read'); + const authorizedReturnFields = await getAuthorizedFields(schema, query.options, 'read'); // create a sanitizedReturnFields object that will be used to return only the fields that a // group has access to read @@ -84,7 +92,7 @@ module.exports = (schema) => { }); query._fields = sanitizedReturnFields; - return next(); + next(); } // Find paths with permissioned schemas and store those so deep checks can be done @@ -98,34 +106,34 @@ module.exports = (schema) => { }); schema.pre('findOneAndRemove', function preFindOneAndRemove(next) { - if (authIsDisabled(this.options)) { return next(); } - return removeQuery(this, next); + if (authIsDisabled(this.options)) { next(); return; } + removeQuery(this, next); }); // TODO, WTF, how to prevent someone from Model.find().remove().exec(); That doesn't // fire any remove hooks. Does it fire a find hook? schema.pre('remove', function preRemove(next, options) { - if (authIsDisabled(options)) { return next(); } - return removeDoc(this, options, next); + if (authIsDisabled(options)) { next(); return; } + removeDoc(this, options, next); }); schema.pre('save', function preSave(next, options) { - if (authIsDisabled(options)) { return next(); } - return save(this, options, next); + if (authIsDisabled(options)) { next(); return; } + save(this, options, next); }); schema.post('find', function postFind(doc, next) { - if (authIsDisabled(this.options)) { return next(); } - return find(this, doc, next); + if (authIsDisabled(this.options)) { next(); return; } + find(this, doc, next); }); schema.post('findOne', function postFindOne(doc, next) { - if (authIsDisabled(this.options)) { return next(); } - return find(this, doc, next); + if (authIsDisabled(this.options)) { next(); return; } + find(this, doc, next); }); schema.pre('update', function preUpdate(next) { - if (authIsDisabled(this.options)) { return next(); } - return update(this, next); + if (authIsDisabled(this.options)) { next(); return; } + update(this, next); }); schema.pre('findOneAndUpdate', function preFindOneAndUpdate(next) { - if (authIsDisabled(this.options)) { return next(); } - return update(this, next); + if (authIsDisabled(this.options)) { next(); return; } + update(this, next); }); schema.query.setAuthLevel = function setAuthLevel(authLevel) { @@ -133,7 +141,7 @@ module.exports = (schema) => { return this; }; - schema.statics.canCreate = function canCreate(options) { - return hasPermission(this.schema, options, 'create'); + schema.statics.canCreate = async function canCreate(options) { + return await hasPermission(this.schema, options, 'create'); }; }; diff --git a/lib/helpers.js b/lib/helpers.js index 37d613f..80116ee 100644 --- a/lib/helpers.js +++ b/lib/helpers.js @@ -1,17 +1,20 @@ const _ = require('lodash'); -function resolveAuthLevel(schema, options, doc) { +async function resolveAuthLevel(schema, options, doc) { // Look into options the options and try to find authLevels. Always prefer to take // authLevels from the direct authLevel option as opposed to the computed // ones from getAuthLevel in the schema object. - let authLevels = []; + let authLevelsIn = []; if (options) { if (options.authLevel) { - authLevels = _.castArray(options.authLevel); + authLevelsIn = options.authLevel; } else if (typeof schema.getAuthLevel === 'function') { - authLevels = _.castArray(schema.getAuthLevel(options.authPayload, doc)); + authLevelsIn = schema.getAuthLevel(options.authPayload, doc); } } + + const authLevels = _.castArray(await Promise.resolve(authLevelsIn)); + // Add `defaults` to the list of levels since you should always be able to do what's specified // in defaults. authLevels.push('defaults'); @@ -23,8 +26,8 @@ function resolveAuthLevel(schema, options, doc) { .value(); } -function getAuthorizedFields(schema, options, action, doc) { - const authLevels = resolveAuthLevel(schema, options, doc); +async function getAuthorizedFields(schema, options, action, doc) { + const authLevels = await resolveAuthLevel(schema, options, doc); return _.chain(authLevels) .flatMap(level => schema.permissions[level][action]) @@ -33,8 +36,8 @@ function getAuthorizedFields(schema, options, action, doc) { .value(); } -function hasPermission(schema, options, action, doc) { - const authLevels = resolveAuthLevel(schema, options, doc); +async function hasPermission(schema, options, action, doc) { + const authLevels = await resolveAuthLevel(schema, options, doc); const perms = schema.permissions || {}; // look for any permissions setting for this action that is set to true (for these authLevels) @@ -45,21 +48,21 @@ function authIsDisabled(options) { return options && options.authLevel === false; } -function embedPermissions(schema, options, doc) { +async function embedPermissions(schema, options, doc) { if (!options || !options.permissions) { return; } const permsKey = options.permissions === true ? 'permissions' : options.permissions; doc[permsKey] = { - read: getAuthorizedFields(schema, options, 'read', doc), - write: getAuthorizedFields(schema, options, 'write', doc), - remove: hasPermission(schema, options, 'remove', doc), + read: await getAuthorizedFields(schema, options, 'read', doc), + write: await getAuthorizedFields(schema, options, 'write', doc), + remove: await hasPermission(schema, options, 'remove', doc), }; } -function sanitizeDocument(schema, options, doc) { - const authorizedFields = getAuthorizedFields(schema, options, 'read', doc); +async function sanitizeDocument(schema, options, doc) { + const authorizedFields = await getAuthorizedFields(schema, options, 'read', doc); - if (!doc || getAuthorizedFields.length === 0) { return false; } + if (!doc || authorizedFields.length === 0) { return false; } // Check to see if group has the permission to see the fields that came back. // We must edit the document in place to maintain the right reference @@ -97,36 +100,35 @@ function sanitizeDocument(schema, options, doc) { // Check to see if we're going to be inserting the permissions info if (options.permissions) { - embedPermissions(schema, options, doc); + await embedPermissions(schema, options, doc); } // Apply the rules down one level if there are any path specific permissions - _.each(schema.pathsWithPermissionedSchemas, (path, subSchema) => { + await Promise.all(_.map(schema.pathsWithPermissionedSchemas, async (path, subSchema) => { if (innerDoc[path]) { // eslint-disable-next-line no-use-before-define - innerDoc[path] = sanitizeDocumentList(subSchema, options, innerDoc[path]); + innerDoc[path] = await sanitizeDocumentList(subSchema, options, innerDoc[path]); } - }); + })); return doc; } -function sanitizeDocumentList(schema, options, docs) { +async function sanitizeDocumentList(schema, options, docs) { const multi = _.isArrayLike(docs); const docList = _.castArray(docs); - const filteredResult = _.chain(docList) - .map((doc) => { + const filteredResult = _.chain(await Promise.all(_.map(docList, async (doc) => { const upgradedOptions = _.isEmpty(schema.pathsWithPermissionedSchemas) ? options : _.merge({}, options, { authPayload: { originalDoc: doc } }); - return sanitizeDocument(schema, upgradedOptions, doc); - }) + return await sanitizeDocument(schema, upgradedOptions, doc); + }))) .filter(docList) .value(); - return multi ? filteredResult : filteredResult[0]; + return multi ? Promise.all(filteredResult) : filteredResult[0]; } function getUpdatePaths(updates) { diff --git a/test/helpers.test.js b/test/helpers.test.js index 4a66c52..bf3a26c 100644 --- a/test/helpers.test.js +++ b/test/helpers.test.js @@ -67,150 +67,150 @@ const queryOpt = { authLevel: 'admin' }; module.exports = { resolveAuthLevel: { - 'single in query options': (test) => { + 'single in query options': async (test) => { // Single Auth level in query options test.deepEqual( - resolveAuthLevel(goodSchema, queryOpt), + await resolveAuthLevel(goodSchema, queryOpt), ['admin', 'defaults'], ); test.done(); }, - 'unknown in query options': (test) => { + 'unknown in query options': async (test) => { test.deepEqual( - resolveAuthLevel(goodSchema, { authLevel: 'foobar' }), + await resolveAuthLevel(goodSchema, { authLevel: 'foobar' }), ['defaults'], ); test.deepEqual( - resolveAuthLevel(goodSchema, { authLevel: ['foobar', 'self'] }), + await resolveAuthLevel(goodSchema, { authLevel: ['foobar', 'self'] }), ['self', 'defaults'], ); test.deepEqual( - resolveAuthLevel(bareBonesSchema, { authLevel: ['foobar'] }), + await resolveAuthLevel(bareBonesSchema, { authLevel: ['foobar'] }), [], ); test.deepEqual( - resolveAuthLevel(bareBonesSchema, { authLevel: 'default' }), + await resolveAuthLevel(bareBonesSchema, { authLevel: 'default' }), [], ); test.done(); }, - 'bad schema': (test) => { + 'bad schema': async (test) => { test.deepEqual( - resolveAuthLevel(emptySchema, { authLevel: 'admin' }), + await resolveAuthLevel(emptySchema, { authLevel: 'admin' }), [], ); test.done(); }, - 'multiple in query options': (test) => { + 'multiple in query options': async (test) => { test.deepEqual( - resolveAuthLevel(goodSchema, { authLevel: ['self', 'admin'] }), + await resolveAuthLevel(goodSchema, { authLevel: ['self', 'admin'] }), ['self', 'admin', 'defaults'], ); test.deepEqual( - resolveAuthLevel(goodSchema, { authLevel: ['defaults'] }), + await resolveAuthLevel(goodSchema, { authLevel: ['defaults'] }), ['defaults'], ); test.deepEqual( - resolveAuthLevel(goodSchema, { authLevel: ['self', 'admin', 'self', 'default', 'admin'] }), + await resolveAuthLevel(goodSchema, { authLevel: ['self', 'admin', 'self', 'default', 'admin'] }), ['self', 'admin', 'defaults'], ); test.done(); }, - 'from document getAuthLevel': (test) => { + 'from document getAuthLevel': async (test) => { test.deepEqual( - resolveAuthLevel(goodSchema, {}, { foo: 1 }), + await resolveAuthLevel(goodSchema, {}, { foo: 1 }), ['defaults'], ); test.deepEqual( - resolveAuthLevel(goodSchema, { authPayload: { authLevel: 'admin' } }, { foo: 1 }), + await resolveAuthLevel(goodSchema, { authPayload: { authLevel: 'admin' } }, { foo: 1 }), ['admin', 'defaults'], ); test.deepEqual( - resolveAuthLevel(goodSchema, { authPayload: { authLevel: false } }, { foo: 1 }), + await resolveAuthLevel(goodSchema, { authPayload: { authLevel: false } }, { foo: 1 }), ['defaults'], ); test.deepEqual( - resolveAuthLevel(goodSchema, { authPayload: { authLevel: 'self' }, authLevel: 'admin' }, { foo: 1 }), + await resolveAuthLevel(goodSchema, { authPayload: { authLevel: 'self' }, authLevel: 'admin' }, { foo: 1 }), ['admin', 'defaults'], ); test.deepEqual( - resolveAuthLevel(goodSchema, { authPayload: { authLevel: 'self' } }), + await resolveAuthLevel(goodSchema, { authPayload: { authLevel: 'self' } }), ['self', 'defaults'], ); test.done(); }, }, - getAuthorizedFields(test) { + async getAuthorizedFields(test) { test.deepEqual( - getAuthorizedFields(bareBonesSchema, { authLevel: 'foobar' }, 'read'), + await getAuthorizedFields(bareBonesSchema, { authLevel: 'foobar' }, 'read'), [], ); test.deepEqual( - getAuthorizedFields(goodSchema, { authLevel: 'foobar' }, 'read').sort(), + (await getAuthorizedFields(goodSchema, { authLevel: 'foobar' }, 'read')).sort(), ['_id', 'name'].sort(), ); test.deepEqual( - getAuthorizedFields(goodSchema, { authLevel: 'admin' }, 'read').sort(), + (await getAuthorizedFields(goodSchema, { authLevel: 'admin' }, 'read')).sort(), ['_id', 'name', 'address', 'phone', 'birthday'].sort(), ); test.deepEqual( - getAuthorizedFields(goodSchema, { authLevel: 'stranger' }, 'read').sort(), + (await getAuthorizedFields(goodSchema, { authLevel: 'stranger' }, 'read')).sort(), ['_id', 'name'].sort(), ); test.deepEqual( - getAuthorizedFields(goodSchema, { authLevel: ['self', 'admin'] }, 'read').sort(), + (await getAuthorizedFields(goodSchema, { authLevel: ['self', 'admin'] }, 'read')).sort(), ['_id', 'name', 'address', 'phone', 'birthday'].sort(), ); test.deepEqual( - getAuthorizedFields(goodSchema, { authLevel: 'self' }, 'write').sort(), + (await getAuthorizedFields(goodSchema, { authLevel: 'self' }, 'write')).sort(), ['address', 'phone'].sort(), ); test.deepEqual( - getAuthorizedFields(bareBonesSchema, { authLevel: 'admin' }, 'write'), + await getAuthorizedFields(bareBonesSchema, { authLevel: 'admin' }, 'write'), [], ); test.deepEqual( - getAuthorizedFields(goodSchema, { authLevel: 'hasVirtuals' }, 'read').sort(), + (await getAuthorizedFields(goodSchema, { authLevel: 'hasVirtuals' }, 'read')).sort(), ['_id', 'name', 'virtual_name'].sort(), 'virtuals should be included in the list of fields', ); test.deepEqual( - getAuthorizedFields(goodSchema, { authLevel: 'nested_top' }, 'read').sort(), + (await getAuthorizedFields(goodSchema, { authLevel: 'nested_top' }, 'read')).sort(), ['_id', 'name', 'nested'].sort(), 'top level nested field should be ok as authorized field', ); test.deepEqual( - getAuthorizedFields(goodSchema, { authLevel: 'nested_deep' }, 'read').sort(), + (await getAuthorizedFields(goodSchema, { authLevel: 'nested_deep' }, 'read')).sort(), ['_id', 'name', 'nested.thing'].sort(), 'deeply nested field should be ok as authorized field', ); test.done(); }, - hasPermission(test) { + async hasPermission(test) { test.equal( - hasPermission(bareBonesSchema, undefined, 'create'), + await hasPermission(bareBonesSchema, undefined, 'create'), false, 'should return false when no options provided', ); test.equal( - hasPermission(bareBonesSchema, {}, 'create'), + await hasPermission(bareBonesSchema, {}, 'create'), false, 'should return false when no permissions exist', ); test.equal( - hasPermission(goodSchema, {}, 'create'), + await hasPermission(goodSchema, {}, 'create'), false, 'default write permission not respected when no authLevel specified', ); test.equal( - hasPermission(goodSchema, {}, 'create'), + await hasPermission(goodSchema, {}, 'create'), false, 'should return false when no permission has been set for the action', ); test.equal( - hasPermission(goodSchema, { authLevel: 'admin' }, 'create'), + await hasPermission(goodSchema, { authLevel: 'admin' }, 'create'), true, 'should return true when an AuthLevel says so, despite default', ); From 5d0d837fc3802f16fe64d737158381b5c215265e Mon Sep 17 00:00:00 2001 From: Bryan Goldstein Date: Tue, 22 May 2018 22:35:41 -0400 Subject: [PATCH 2/4] some fixes from code review --- lib/helpers.js | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/lib/helpers.js b/lib/helpers.js index 80116ee..a2e3a41 100644 --- a/lib/helpers.js +++ b/lib/helpers.js @@ -4,17 +4,15 @@ async function resolveAuthLevel(schema, options, doc) { // Look into options the options and try to find authLevels. Always prefer to take // authLevels from the direct authLevel option as opposed to the computed // ones from getAuthLevel in the schema object. - let authLevelsIn = []; + let authLevels = []; if (options) { if (options.authLevel) { - authLevelsIn = options.authLevel; + authLevels = _.castArray(options.authLevel); } else if (typeof schema.getAuthLevel === 'function') { - authLevelsIn = schema.getAuthLevel(options.authPayload, doc); + authLevels = _.castArray(await schema.getAuthLevel(options.authPayload, doc)); } } - const authLevels = _.castArray(await Promise.resolve(authLevelsIn)); - // Add `defaults` to the list of levels since you should always be able to do what's specified // in defaults. authLevels.push('defaults'); @@ -117,18 +115,18 @@ async function sanitizeDocument(schema, options, doc) { async function sanitizeDocumentList(schema, options, docs) { const multi = _.isArrayLike(docs); const docList = _.castArray(docs); + const sanitizeAndAddOptions = (doc) => { + const upgradedOptions = _.isEmpty(schema.pathsWithPermissionedSchemas) + ? options + : _.merge({}, options, { authPayload: { originalDoc: doc } }); + return sanitizeDocument(schema, upgradedOptions, doc); + }; - const filteredResult = _.chain(await Promise.all(_.map(docList, async (doc) => { - const upgradedOptions = _.isEmpty(schema.pathsWithPermissionedSchemas) - ? options - : _.merge({}, options, { authPayload: { originalDoc: doc } }); - - return await sanitizeDocument(schema, upgradedOptions, doc); - }))) - .filter(docList) - .value(); + const filteredResult = ( + await Promise.all(docList.map(sanitizeAndAddOptions)) + ).filter(doc => !doc); - return multi ? Promise.all(filteredResult) : filteredResult[0]; + return multi ? (filteredResult) : filteredResult[0]; } function getUpdatePaths(updates) { From 31f27ebe126fa0176698b1a2f1e3a68b7937fb09 Mon Sep 17 00:00:00 2001 From: Bryan Goldstein Date: Tue, 22 May 2018 22:59:59 -0400 Subject: [PATCH 3/4] a dumb implementation of auth level caching --- index.js | 10 ++++++++-- lib/helpers.js | 20 +++++++++++++++++++- test/helpers.test.js | 29 +++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 3 deletions(-) diff --git a/index.js b/index.js index c9f012e..a76af45 100644 --- a/index.js +++ b/index.js @@ -1,5 +1,6 @@ const _ = require('lodash'); const { + resetCache, getAuthorizedFields, hasPermission, authIsDisabled, @@ -11,6 +12,7 @@ const PermissionDeniedError = require('./lib/PermissionDeniedError'); module.exports = (schema) => { async function save(doc, options, next) { + resetCache(); if (doc.isNew && !await hasPermission(schema, options, 'create', doc)) { next(new PermissionDeniedError('create')); return; @@ -29,6 +31,7 @@ module.exports = (schema) => { } async function removeQuery(query, next) { + resetCache(); if (!await hasPermission(schema, query.options, 'remove')) { next(new PermissionDeniedError('remove')); return; @@ -39,6 +42,7 @@ module.exports = (schema) => { } async function removeDoc(doc, options, next) { + resetCache(); if (!await hasPermission(schema, options, 'remove', doc)) { next(new PermissionDeniedError('remove')); return; @@ -49,12 +53,14 @@ module.exports = (schema) => { } async function find(query, docs, next) { + resetCache(); const sanitizedResult = await sanitizeDocumentList(schema, query.options, docs); next(null, sanitizedResult); } async function update(query, next) { + resetCache(); // If this is an upsert, you'll need the create permission // TODO add some tests for the upset case if ( @@ -141,7 +147,7 @@ module.exports = (schema) => { return this; }; - schema.statics.canCreate = async function canCreate(options) { - return await hasPermission(this.schema, options, 'create'); + schema.statics.canCreate = function canCreate(options) { + return hasPermission(this.schema, options, 'create'); }; }; diff --git a/lib/helpers.js b/lib/helpers.js index a2e3a41..6b5f52c 100644 --- a/lib/helpers.js +++ b/lib/helpers.js @@ -1,5 +1,11 @@ const _ = require('lodash'); +let authLevelCache; + +function resetCache() { + authLevelCache = new Map(); +} + async function resolveAuthLevel(schema, options, doc) { // Look into options the options and try to find authLevels. Always prefer to take // authLevels from the direct authLevel option as opposed to the computed @@ -9,7 +15,18 @@ async function resolveAuthLevel(schema, options, doc) { if (options.authLevel) { authLevels = _.castArray(options.authLevel); } else if (typeof schema.getAuthLevel === 'function') { - authLevels = _.castArray(await schema.getAuthLevel(options.authPayload, doc)); + let arg2Map = authLevelCache.get(options.authLevel) + if (!arg2Map) { + arg2Map = new Map(); + authLevelCache.set(options.authLevel, arg2Map); + } + const cachedValue = arg2Map.get(doc); + if (cachedValue) { + authLevels = cachedValue; + } else { + authLevels = _.castArray(await schema.getAuthLevel(options.authPayload, doc)); + arg2Map.set(doc, authLevels); + } } } @@ -143,6 +160,7 @@ function getUpdatePaths(updates) { } module.exports = { + resetCache, resolveAuthLevel, getAuthorizedFields, hasPermission, diff --git a/test/helpers.test.js b/test/helpers.test.js index bf3a26c..17c1cdf 100644 --- a/test/helpers.test.js +++ b/test/helpers.test.js @@ -5,8 +5,11 @@ const { getAuthorizedFields, hasPermission, getUpdatePaths, + resetCache, } = require('../lib/helpers'); +resetCache(); + // Set up a bunch of schemas for testing. We're not going to connect to the database // since these tests only depend on the schema definitions. const goodSchema = new mongoose.Schema({ @@ -46,7 +49,9 @@ goodSchema.permissions = { goodSchema.virtual('virtual_name').get(function getVirtualName() { return `virtual${this.name}`; }); +let authLevelCalls = 0; goodSchema.getAuthLevel = function getAuthLevel(payload) { + authLevelCalls++; return payload && payload.authLevel; }; @@ -140,6 +145,30 @@ module.exports = { ); test.done(); }, + 'getAuthLevel is cached': async (test) => { + const options = { authPayload: { authLevel: 'admin' } }; + const doc = { foo: 1 }; + authLevelCalls = 0; + test.deepEqual( + await resolveAuthLevel(goodSchema, options, doc), + ['admin', 'defaults'], + ); + test.equal(authLevelCalls, 1); + test.deepEqual( + await resolveAuthLevel(goodSchema, options, doc), + ['admin', 'defaults'], + ); + test.deepEqual( + await resolveAuthLevel(goodSchema, options, doc), + ['admin', 'defaults'], + ); + test.deepEqual( + await resolveAuthLevel(goodSchema, options, doc), + ['admin', 'defaults'], + ); + test.equal(authLevelCalls, 1); + test.done(); + }, }, async getAuthorizedFields(test) { test.deepEqual( From 846c7754dac09c6c1ac1bd89c80852c6176983da Mon Sep 17 00:00:00 2001 From: Bryan Goldstein Date: Wed, 23 May 2018 22:35:23 -0400 Subject: [PATCH 4/4] switch to es6 WeakMap --- index.js | 6 - lib/helpers.js | 22 ++-- test/helpers.test.js | 296 +++++++++++++++++++++++++------------------ 3 files changed, 181 insertions(+), 143 deletions(-) diff --git a/index.js b/index.js index a76af45..6f617fc 100644 --- a/index.js +++ b/index.js @@ -1,6 +1,5 @@ const _ = require('lodash'); const { - resetCache, getAuthorizedFields, hasPermission, authIsDisabled, @@ -12,7 +11,6 @@ const PermissionDeniedError = require('./lib/PermissionDeniedError'); module.exports = (schema) => { async function save(doc, options, next) { - resetCache(); if (doc.isNew && !await hasPermission(schema, options, 'create', doc)) { next(new PermissionDeniedError('create')); return; @@ -31,7 +29,6 @@ module.exports = (schema) => { } async function removeQuery(query, next) { - resetCache(); if (!await hasPermission(schema, query.options, 'remove')) { next(new PermissionDeniedError('remove')); return; @@ -42,7 +39,6 @@ module.exports = (schema) => { } async function removeDoc(doc, options, next) { - resetCache(); if (!await hasPermission(schema, options, 'remove', doc)) { next(new PermissionDeniedError('remove')); return; @@ -53,14 +49,12 @@ module.exports = (schema) => { } async function find(query, docs, next) { - resetCache(); const sanitizedResult = await sanitizeDocumentList(schema, query.options, docs); next(null, sanitizedResult); } async function update(query, next) { - resetCache(); // If this is an upsert, you'll need the create permission // TODO add some tests for the upset case if ( diff --git a/lib/helpers.js b/lib/helpers.js index 6b5f52c..143c9d2 100644 --- a/lib/helpers.js +++ b/lib/helpers.js @@ -1,11 +1,6 @@ const _ = require('lodash'); -let authLevelCache; - -function resetCache() { - authLevelCache = new Map(); -} - +const authLevelCache = new WeakMap(); async function resolveAuthLevel(schema, options, doc) { // Look into options the options and try to find authLevels. Always prefer to take // authLevels from the direct authLevel option as opposed to the computed @@ -15,15 +10,16 @@ async function resolveAuthLevel(schema, options, doc) { if (options.authLevel) { authLevels = _.castArray(options.authLevel); } else if (typeof schema.getAuthLevel === 'function') { - let arg2Map = authLevelCache.get(options.authLevel) + let arg2Map = authLevelCache.get(options) if (!arg2Map) { - arg2Map = new Map(); - authLevelCache.set(options.authLevel, arg2Map); + arg2Map = new WeakMap(); + authLevelCache.set(options, arg2Map); } const cachedValue = arg2Map.get(doc); if (cachedValue) { authLevels = cachedValue; } else { + if (!doc) throw new Error("getAuthLevel only supports methods with model data available"); authLevels = _.castArray(await schema.getAuthLevel(options.authPayload, doc)); arg2Map.set(doc, authLevels); } @@ -52,7 +48,12 @@ async function getAuthorizedFields(schema, options, action, doc) { } async function hasPermission(schema, options, action, doc) { - const authLevels = await resolveAuthLevel(schema, options, doc); + let authLevels + try { + authLevels = await resolveAuthLevel(schema, options, doc); + } catch (e) { + authLevels = []; + } const perms = schema.permissions || {}; // look for any permissions setting for this action that is set to true (for these authLevels) @@ -160,7 +161,6 @@ function getUpdatePaths(updates) { } module.exports = { - resetCache, resolveAuthLevel, getAuthorizedFields, hasPermission, diff --git a/test/helpers.test.js b/test/helpers.test.js index 17c1cdf..29e6783 100644 --- a/test/helpers.test.js +++ b/test/helpers.test.js @@ -1,14 +1,11 @@ -const mongoose = require('mongoose'); +const mongoose = require("mongoose"); const { resolveAuthLevel, getAuthorizedFields, hasPermission, - getUpdatePaths, - resetCache, -} = require('../lib/helpers'); - -resetCache(); + getUpdatePaths +} = require("../lib/helpers"); // Set up a bunch of schemas for testing. We're not going to connect to the database // since these tests only depend on the schema definitions. @@ -17,36 +14,36 @@ const goodSchema = new mongoose.Schema({ address: String, phone: String, birthday: String, - nested: { thing: String }, + nested: { thing: String } }); goodSchema.permissions = { defaults: { - read: ['_id', 'name'], + read: ["_id", "name"], write: [], - create: false, + create: false }, admin: { - read: ['address', 'phone', 'birthday'], - write: ['address', 'phone', 'birthday'], + read: ["address", "phone", "birthday"], + write: ["address", "phone", "birthday"], create: true, - remove: true, + remove: true }, self: { - read: ['address', 'phone', 'birthday'], - write: ['address', 'phone'], + read: ["address", "phone", "birthday"], + write: ["address", "phone"] }, stranger: {}, hasVirtuals: { - read: ['virtual_name'], + read: ["virtual_name"] }, nested_top: { - read: ['nested'], + read: ["nested"] }, nested_deep: { - read: ['nested.thing'], - }, + read: ["nested.thing"] + } }; -goodSchema.virtual('virtual_name').get(function getVirtualName() { +goodSchema.virtual("virtual_name").get(function getVirtualName() { return `virtual${this.name}`; }); let authLevelCalls = 0; @@ -58,190 +55,244 @@ goodSchema.getAuthLevel = function getAuthLevel(payload) { const bareBonesSchema = new mongoose.Schema({}); bareBonesSchema.permissions = { admin: { - read: ['address', 'phone', 'birthday', 'does_not_exist'], - write: ['address', 'phone', 'birthday', 'not_here_either'], + read: ["address", "phone", "birthday", "does_not_exist"], + write: ["address", "phone", "birthday", "not_here_either"], create: true, - remove: true, - }, + remove: true + } }; const emptySchema = new mongoose.Schema({}); // Options Configs -const queryOpt = { authLevel: 'admin' }; +const queryOpt = { authLevel: "admin" }; module.exports = { resolveAuthLevel: { - 'single in query options': async (test) => { + "single in query options": async test => { // Single Auth level in query options - test.deepEqual( - await resolveAuthLevel(goodSchema, queryOpt), - ['admin', 'defaults'], - ); + test.deepEqual(await resolveAuthLevel(goodSchema, queryOpt), [ + "admin", + "defaults" + ]); test.done(); }, - 'unknown in query options': async (test) => { + "unknown in query options": async test => { test.deepEqual( - await resolveAuthLevel(goodSchema, { authLevel: 'foobar' }), - ['defaults'], + await resolveAuthLevel(goodSchema, { authLevel: "foobar" }), + ["defaults"] ); test.deepEqual( - await resolveAuthLevel(goodSchema, { authLevel: ['foobar', 'self'] }), - ['self', 'defaults'], + await resolveAuthLevel(goodSchema, { authLevel: ["foobar", "self"] }), + ["self", "defaults"] ); test.deepEqual( - await resolveAuthLevel(bareBonesSchema, { authLevel: ['foobar'] }), - [], + await resolveAuthLevel(bareBonesSchema, { authLevel: ["foobar"] }), + [] ); test.deepEqual( - await resolveAuthLevel(bareBonesSchema, { authLevel: 'default' }), - [], + await resolveAuthLevel(bareBonesSchema, { authLevel: "default" }), + [] ); test.done(); }, - 'bad schema': async (test) => { + "bad schema": async test => { test.deepEqual( - await resolveAuthLevel(emptySchema, { authLevel: 'admin' }), - [], + await resolveAuthLevel(emptySchema, { authLevel: "admin" }), + [] ); test.done(); }, - 'multiple in query options': async (test) => { + "multiple in query options": async test => { test.deepEqual( - await resolveAuthLevel(goodSchema, { authLevel: ['self', 'admin'] }), - ['self', 'admin', 'defaults'], + await resolveAuthLevel(goodSchema, { authLevel: ["self", "admin"] }), + ["self", "admin", "defaults"] ); test.deepEqual( - await resolveAuthLevel(goodSchema, { authLevel: ['defaults'] }), - ['defaults'], + await resolveAuthLevel(goodSchema, { authLevel: ["defaults"] }), + ["defaults"] ); test.deepEqual( - await resolveAuthLevel(goodSchema, { authLevel: ['self', 'admin', 'self', 'default', 'admin'] }), - ['self', 'admin', 'defaults'], + await resolveAuthLevel(goodSchema, { + authLevel: ["self", "admin", "self", "default", "admin"] + }), + ["self", "admin", "defaults"] ); test.done(); }, - 'from document getAuthLevel': async (test) => { - test.deepEqual( - await resolveAuthLevel(goodSchema, {}, { foo: 1 }), - ['defaults'], - ); - test.deepEqual( - await resolveAuthLevel(goodSchema, { authPayload: { authLevel: 'admin' } }, { foo: 1 }), - ['admin', 'defaults'], - ); + "from document getAuthLevel": async test => { + test.deepEqual(await resolveAuthLevel(goodSchema, {}, { foo: 1 }), [ + "defaults" + ]); test.deepEqual( - await resolveAuthLevel(goodSchema, { authPayload: { authLevel: false } }, { foo: 1 }), - ['defaults'], + await resolveAuthLevel( + goodSchema, + { authPayload: { authLevel: "admin" } }, + { foo: 1 } + ), + ["admin", "defaults"] ); test.deepEqual( - await resolveAuthLevel(goodSchema, { authPayload: { authLevel: 'self' }, authLevel: 'admin' }, { foo: 1 }), - ['admin', 'defaults'], + await resolveAuthLevel( + goodSchema, + { authPayload: { authLevel: false } }, + { foo: 1 } + ), + ["defaults"] ); test.deepEqual( - await resolveAuthLevel(goodSchema, { authPayload: { authLevel: 'self' } }), - ['self', 'defaults'], + await resolveAuthLevel( + goodSchema, + { authPayload: { authLevel: "self" }, authLevel: "admin" }, + { foo: 1 } + ), + ["admin", "defaults"] ); + let caught; + await resolveAuthLevel(goodSchema, { + authPayload: { authLevel: "self" } + }).catch(() => { caught = true; }); + test.equal(caught, true); test.done(); }, - 'getAuthLevel is cached': async (test) => { - const options = { authPayload: { authLevel: 'admin' } }; + "getAuthLevel is cached": async test => { + const options = { authPayload: { authLevel: "admin" } }; const doc = { foo: 1 }; authLevelCalls = 0; - test.deepEqual( - await resolveAuthLevel(goodSchema, options, doc), - ['admin', 'defaults'], - ); + test.deepEqual(await resolveAuthLevel(goodSchema, options, doc), [ + "admin", + "defaults" + ]); test.equal(authLevelCalls, 1); - test.deepEqual( - await resolveAuthLevel(goodSchema, options, doc), - ['admin', 'defaults'], - ); - test.deepEqual( - await resolveAuthLevel(goodSchema, options, doc), - ['admin', 'defaults'], - ); - test.deepEqual( - await resolveAuthLevel(goodSchema, options, doc), - ['admin', 'defaults'], - ); + test.deepEqual(await resolveAuthLevel(goodSchema, options, doc), [ + "admin", + "defaults" + ]); + test.deepEqual(await resolveAuthLevel(goodSchema, options, doc), [ + "admin", + "defaults" + ]); + test.deepEqual(await resolveAuthLevel(goodSchema, options, doc), [ + "admin", + "defaults" + ]); test.equal(authLevelCalls, 1); test.done(); - }, + } }, async getAuthorizedFields(test) { test.deepEqual( - await getAuthorizedFields(bareBonesSchema, { authLevel: 'foobar' }, 'read'), - [], + await getAuthorizedFields( + bareBonesSchema, + { authLevel: "foobar" }, + "read" + ), + [] ); test.deepEqual( - (await getAuthorizedFields(goodSchema, { authLevel: 'foobar' }, 'read')).sort(), - ['_id', 'name'].sort(), + (await getAuthorizedFields( + goodSchema, + { authLevel: "foobar" }, + "read" + )).sort(), + ["_id", "name"].sort() ); test.deepEqual( - (await getAuthorizedFields(goodSchema, { authLevel: 'admin' }, 'read')).sort(), - ['_id', 'name', 'address', 'phone', 'birthday'].sort(), + (await getAuthorizedFields( + goodSchema, + { authLevel: "admin" }, + "read" + )).sort(), + ["_id", "name", "address", "phone", "birthday"].sort() ); test.deepEqual( - (await getAuthorizedFields(goodSchema, { authLevel: 'stranger' }, 'read')).sort(), - ['_id', 'name'].sort(), + (await getAuthorizedFields( + goodSchema, + { authLevel: "stranger" }, + "read" + )).sort(), + ["_id", "name"].sort() ); test.deepEqual( - (await getAuthorizedFields(goodSchema, { authLevel: ['self', 'admin'] }, 'read')).sort(), - ['_id', 'name', 'address', 'phone', 'birthday'].sort(), + (await getAuthorizedFields( + goodSchema, + { authLevel: ["self", "admin"] }, + "read" + )).sort(), + ["_id", "name", "address", "phone", "birthday"].sort() ); test.deepEqual( - (await getAuthorizedFields(goodSchema, { authLevel: 'self' }, 'write')).sort(), - ['address', 'phone'].sort(), + (await getAuthorizedFields( + goodSchema, + { authLevel: "self" }, + "write" + )).sort(), + ["address", "phone"].sort() ); test.deepEqual( - await getAuthorizedFields(bareBonesSchema, { authLevel: 'admin' }, 'write'), - [], + await getAuthorizedFields( + bareBonesSchema, + { authLevel: "admin" }, + "write" + ), + [] ); test.deepEqual( - (await getAuthorizedFields(goodSchema, { authLevel: 'hasVirtuals' }, 'read')).sort(), - ['_id', 'name', 'virtual_name'].sort(), - 'virtuals should be included in the list of fields', + (await getAuthorizedFields( + goodSchema, + { authLevel: "hasVirtuals" }, + "read" + )).sort(), + ["_id", "name", "virtual_name"].sort(), + "virtuals should be included in the list of fields" ); test.deepEqual( - (await getAuthorizedFields(goodSchema, { authLevel: 'nested_top' }, 'read')).sort(), - ['_id', 'name', 'nested'].sort(), - 'top level nested field should be ok as authorized field', + (await getAuthorizedFields( + goodSchema, + { authLevel: "nested_top" }, + "read" + )).sort(), + ["_id", "name", "nested"].sort(), + "top level nested field should be ok as authorized field" ); test.deepEqual( - (await getAuthorizedFields(goodSchema, { authLevel: 'nested_deep' }, 'read')).sort(), - ['_id', 'name', 'nested.thing'].sort(), - 'deeply nested field should be ok as authorized field', + (await getAuthorizedFields( + goodSchema, + { authLevel: "nested_deep" }, + "read" + )).sort(), + ["_id", "name", "nested.thing"].sort(), + "deeply nested field should be ok as authorized field" ); test.done(); }, async hasPermission(test) { test.equal( - await hasPermission(bareBonesSchema, undefined, 'create'), + await hasPermission(bareBonesSchema, undefined, "create"), false, - 'should return false when no options provided', + "should return false when no options provided" ); test.equal( - await hasPermission(bareBonesSchema, {}, 'create'), + await hasPermission(bareBonesSchema, {}, "create"), false, - 'should return false when no permissions exist', + "should return false when no permissions exist" ); test.equal( - await hasPermission(goodSchema, {}, 'create'), + await hasPermission(goodSchema, {}, "create"), false, - 'default write permission not respected when no authLevel specified', + "default write permission not respected when no authLevel specified" ); test.equal( - await hasPermission(goodSchema, {}, 'create'), + await hasPermission(goodSchema, {}, "create"), false, - 'should return false when no permission has been set for the action', + "should return false when no permission has been set for the action" ); test.equal( - await hasPermission(goodSchema, { authLevel: 'admin' }, 'create'), + await hasPermission(goodSchema, { authLevel: "admin" }, "create"), true, - 'should return true when an AuthLevel says so, despite default', + "should return true when an AuthLevel says so, despite default" ); test.done(); }, @@ -262,19 +313,12 @@ module.exports = { test.done(); }, getUpdatePaths(test) { - test.deepEqual( - getUpdatePaths({ $set: { foo: 1 } }).sort(), - ['foo'], - ); + test.deepEqual(getUpdatePaths({ $set: { foo: 1 } }).sort(), ["foo"]); test.deepEqual( getUpdatePaths({ $set: { foo: 1 }, $inc: { bar: 2 } }).sort(), - ['bar', 'foo'], - ); - test.deepEqual( - getUpdatePaths({ foo: 1, bar: 2 }).sort(), - ['bar', 'foo'], + ["bar", "foo"] ); + test.deepEqual(getUpdatePaths({ foo: 1, bar: 2 }).sort(), ["bar", "foo"]); test.done(); - }, + } }; -