From d559cb2382db88cea31b85e076783378443d1510 Mon Sep 17 00:00:00 2001 From: Drew Date: Sun, 12 Jun 2016 13:39:41 -0700 Subject: [PATCH] Move transform acl (#2021) * Move ACL transforming into Parse Server For the database adapters, it will be more performant and easier to work with _rperm and _wperm than with the ACL object. This way we can type it as an array and so on, and once we have stronger validations in Parse Server, we can type it as an array containing strings of length < x, which will be much much better in sql databases. * Use destructuring --- spec/MongoStorageAdapter.spec.js | 2 +- spec/MongoTransform.spec.js | 35 ++---- src/Adapters/Storage/Mongo/MongoCollection.js | 12 -- .../Storage/Mongo/MongoStorageAdapter.js | 6 +- src/Adapters/Storage/Mongo/MongoTransform.js | 108 +++++++----------- src/Controllers/DatabaseController.js | 52 ++++++++- 6 files changed, 104 insertions(+), 111 deletions(-) diff --git a/spec/MongoStorageAdapter.spec.js b/spec/MongoStorageAdapter.spec.js index fad763ca..069e0166 100644 --- a/spec/MongoStorageAdapter.spec.js +++ b/spec/MongoStorageAdapter.spec.js @@ -54,7 +54,7 @@ describe('MongoStorageAdapter', () => { .then(results => { expect(results.length).toEqual(1); var obj = results[0]; - expect(typeof obj._id).toEqual('string'); + expect(obj._id).toEqual('abcde'); expect(obj.objectId).toBeUndefined(); done(); }); diff --git a/spec/MongoTransform.spec.js b/spec/MongoTransform.spec.js index 56b377f7..44ab1cf5 100644 --- a/spec/MongoTransform.spec.js +++ b/spec/MongoTransform.spec.js @@ -58,10 +58,9 @@ describe('parseObjectToMongoObjectForCreate', () => { done(); }); - it('basic ACL', (done) => { + it('Doesnt allow ACL, as Parse Server should tranform ACL to _wperm + _rperm', done => { var input = {ACL: {'0123': {'read': true, 'write': true}}}; - var output = transform.parseObjectToMongoObjectForCreate(null, input, { fields: {} }); - // This just checks that it doesn't crash, but it should check format. + expect(() => transform.parseObjectToMongoObjectForCreate(null, input, { fields: {} })).toThrow(); done(); }); @@ -220,28 +219,10 @@ describe('transform schema key changes', () => { done(); }); - it('changes ACL storage to _rperm and _wperm', (done) => { - var input = { - ACL: { - "*": { "read": true }, - "Kevin": { "write": true } - } - }; - var output = transform.parseObjectToMongoObjectForCreate(null, input, { fields: {} }); - expect(typeof output._rperm).toEqual('object'); - expect(typeof output._wperm).toEqual('object'); - expect(output.ACL).toBeUndefined(); - expect(output._rperm[0]).toEqual('*'); - expect(output._wperm[0]).toEqual('Kevin'); - done(); - }); - it('writes the old ACL format in addition to rperm and wperm', (done) => { var input = { - ACL: { - "*": { "read": true }, - "Kevin": { "write": true } - } + _rperm: ['*'], + _wperm: ['Kevin'], }; var output = transform.parseObjectToMongoObjectForCreate(null, input, { fields: {} }); @@ -257,11 +238,9 @@ describe('transform schema key changes', () => { _wperm: ["Kevin"] }; var output = transform.mongoObjectToParseObject(null, input, { fields: {} }); - expect(typeof output.ACL).toEqual('object'); - expect(output._rperm).toBeUndefined(); - expect(output._wperm).toBeUndefined(); - expect(output.ACL['*']['read']).toEqual(true); - expect(output.ACL['Kevin']['write']).toEqual(true); + expect(output._rperm).toEqual(['*']); + expect(output._wperm).toEqual(['Kevin']); + expect(output.ACL).toBeUndefined() done(); }); diff --git a/src/Adapters/Storage/Mongo/MongoCollection.js b/src/Adapters/Storage/Mongo/MongoCollection.js index c494869b..889e6406 100644 --- a/src/Adapters/Storage/Mongo/MongoCollection.js +++ b/src/Adapters/Storage/Mongo/MongoCollection.js @@ -44,18 +44,6 @@ export default class MongoCollection { return this._mongoCollection.count(query, { skip, limit, sort }); } - // Atomically finds and updates an object based on query. - // The result is the promise with an object that was in the database !AFTER! changes. - // Postgres Note: Translates directly to `UPDATE * SET * ... RETURNING *`, which will return data after the change is done. - findOneAndUpdate(query, update) { - // arguments: query, sort, update, options(optional) - // Setting `new` option to true makes it return the after document, not the before one. - return this._mongoCollection.findAndModify(query, [], update, { new: true }).then(document => { - // Value is the object where mongo returns multiple fields. - return document.value; - }); - } - insertOne(object) { return this._mongoCollection.insertOne(object); } diff --git a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js index c3bb30d7..622baf65 100644 --- a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js +++ b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js @@ -212,12 +212,14 @@ export class MongoStorageAdapter { .then(collection => collection.updateMany(mongoWhere, mongoUpdate)); } - // Hopefully we can get rid of this in favor of updateObjectsByQuery. + // Atomically finds and updates an object based on query. + // Resolve with the updated object. findOneAndUpdate(className, query, schema, update) { const mongoUpdate = transformUpdate(className, update, schema); const mongoWhere = transformWhere(className, query, schema); return this.adaptiveCollection(className) - .then(collection => collection.findOneAndUpdate(mongoWhere, mongoUpdate)); + .then(collection => collection._mongoCollection.findAndModify(mongoWhere, [], mongoUpdate, { new: true })) + .then(result => result.value); } // Hopefully we can get rid of this. It's only used for config and hooks. diff --git a/src/Adapters/Storage/Mongo/MongoTransform.js b/src/Adapters/Storage/Mongo/MongoTransform.js index b1d5170b..4858a436 100644 --- a/src/Adapters/Storage/Mongo/MongoTransform.js +++ b/src/Adapters/Storage/Mongo/MongoTransform.js @@ -273,11 +273,12 @@ const parseObjectKeyValueToMongoObjectKeyValue = (className, restKey, restValue, // Main exposed method to create new objects. // restCreate is the "create" clause in REST API form. -function parseObjectToMongoObjectForCreate(className, restCreate, schema) { +const parseObjectToMongoObjectForCreate = (className, restCreate, schema) => { if (className == '_User') { restCreate = transformAuthData(restCreate); } - var mongoCreate = transformACL(restCreate); + restCreate = addLegacyACL(restCreate); + let mongoCreate = {} for (let restKey in restCreate) { let { key, value } = parseObjectKeyValueToMongoObjectKeyValue( className, @@ -298,18 +299,18 @@ const transformUpdate = (className, restUpdate, parseFormatSchema) => { restUpdate = transformAuthData(restUpdate); } - var mongoUpdate = {}; - var acl = transformACL(restUpdate); - if (acl._rperm || acl._wperm || acl._acl) { - mongoUpdate['$set'] = {}; + let mongoUpdate = {}; + let acl = addLegacyACL(restUpdate)._acl; + if (acl) { + mongoUpdate.$set = {}; if (acl._rperm) { - mongoUpdate['$set']['_rperm'] = acl._rperm; + mongoUpdate.$set._rperm = acl._rperm; } if (acl._wperm) { - mongoUpdate['$set']['_wperm'] = acl._wperm; + mongoUpdate.$set._wperm = acl._wperm; } if (acl._acl) { - mongoUpdate['$set']['_acl'] = acl._acl; + mongoUpdate.$set._acl = acl._acl; } } for (var restKey in restUpdate) { @@ -347,66 +348,34 @@ function transformAuthData(restObject) { return restObject; } -// Transforms a REST API formatted ACL object to our two-field mongo format. -// This mutates the restObject passed in to remove the ACL key. -function transformACL(restObject) { - var output = {}; - if (!restObject['ACL']) { - return output; - } - var acl = restObject['ACL']; - var rperm = []; - var wperm = []; - var _acl = {}; // old format +// Add the legacy _acl format. +const addLegacyACL = restObject => { + let restObjectCopy = {...restObject}; + let _acl = {}; - for (var entry in acl) { - if (acl[entry].read) { - rperm.push(entry); - _acl[entry] = _acl[entry] || {}; - _acl[entry]['r'] = true; - } - if (acl[entry].write) { - wperm.push(entry); - _acl[entry] = _acl[entry] || {}; - _acl[entry]['w'] = true; - } + if (restObject._wperm) { + restObject._wperm.forEach(entry => { + _acl[entry] = { w: true }; + }); } - output._rperm = rperm; - output._wperm = wperm; - output._acl = _acl; - delete restObject.ACL; - return output; + + if (restObject._rperm) { + restObject._rperm.forEach(entry => { + if (!(entry in _acl)) { + _acl[entry] = { r: true }; + } else { + _acl[entry].r = true; + } + }); + } + + if (Object.keys(_acl).length > 0) { + restObjectCopy._acl = _acl; + } + + return restObjectCopy; } -// Transforms a mongo format ACL to a REST API format ACL key -// This mutates the mongoObject passed in to remove the _rperm/_wperm keys -function untransformACL(mongoObject) { - var output = {}; - if (!mongoObject['_rperm'] && !mongoObject['_wperm']) { - return output; - } - var acl = {}; - var rperm = mongoObject['_rperm'] || []; - var wperm = mongoObject['_wperm'] || []; - rperm.map((entry) => { - if (!acl[entry]) { - acl[entry] = {read: true}; - } else { - acl[entry]['read'] = true; - } - }); - wperm.map((entry) => { - if (!acl[entry]) { - acl[entry] = {write: true}; - } else { - acl[entry]['write'] = true; - } - }); - output['ACL'] = acl; - delete mongoObject._rperm; - delete mongoObject._wperm; - return output; -} // A sentinel value that helper transformations return when they // cannot perform a transformation @@ -752,7 +721,14 @@ const mongoObjectToParseObject = (className, mongoObject, schema) => { return BytesCoder.databaseToJSON(mongoObject); } - var restObject = untransformACL(mongoObject); + let restObject = {}; + if (mongoObject._rperm || mongoObject._wperm) { + restObject._rperm = mongoObject._rperm || []; + restObject._wperm = mongoObject._wperm || []; + delete mongoObject._rperm; + delete mongoObject._wperm; + } + for (var key in mongoObject) { switch(key) { case '_id': diff --git a/src/Controllers/DatabaseController.js b/src/Controllers/DatabaseController.js index 0da314f5..8801d12c 100644 --- a/src/Controllers/DatabaseController.js +++ b/src/Controllers/DatabaseController.js @@ -24,6 +24,26 @@ function addReadACL(query, acl) { return newQuery; } +// Transforms a REST API formatted ACL object to our two-field mongo format. +const transformObjectACL = ({ ACL, ...result }) => { + if (!ACL) { + return result; + } + + result._wperm = []; + result._rperm = []; + + for (let entry in ACL) { + if (ACL[entry].read) { + result._rperm.push(entry); + } + if (ACL[entry].write) { + result._wperm.push(entry); + } + } + return result; +} + const specialQuerykeys = ['$and', '$or', '_rperm', '_wperm', '_perishable_token', '_email_verify_token']; const validateQuery = query => { if (query.ACL) { @@ -210,6 +230,7 @@ DatabaseController.prototype.update = function(className, query, update, { throw new Parse.Error(Parse.Error.INVALID_NESTED_KEY, "Nested keys should not contain the '$' or '.' characters"); } } + update = transformObjectACL(update); if (many) { return this.adapter.updateObjectsByQuery(className, query, schema, update); } else if (upsert) { @@ -376,7 +397,7 @@ DatabaseController.prototype.destroy = function(className, query, { acl } = {}) DatabaseController.prototype.create = function(className, object, { acl } = {}) { // Make a copy of the object, so we don't mutate the incoming data. let originalObject = object; - object = deepcopy(object); + object = transformObjectACL(object); var isMaster = acl === undefined; var aclGroup = acl || []; @@ -671,13 +692,40 @@ DatabaseController.prototype.find = function(className, query, { return this.adapter.count(className, query, schema); } else { return this.adapter.find(className, query, schema, { skip, limit, sort }) - .then(objects => objects.map(object => filterSensitiveData(isMaster, aclGroup, className, object))); + .then(objects => objects.map(object => { + object = untransformObjectACL(object); + return filterSensitiveData(isMaster, aclGroup, className, object) + })); } }); }); }); }; +// Transforms a Database format ACL to a REST API format ACL +const untransformObjectACL = ({_rperm, _wperm, ...output}) => { + if (_rperm || _wperm) { + output.ACL = {}; + + (_rperm || []).forEach(entry => { + if (!output.ACL[entry]) { + output.ACL[entry] = { read: true }; + } else { + output.ACL[entry]['read'] = true; + } + }); + + (_wperm || []).forEach(entry => { + if (!output.ACL[entry]) { + output.ACL[entry] = { write: true }; + } else { + output.ACL[entry]['write'] = true; + } + }); + } + return output; +} + DatabaseController.prototype.deleteSchema = function(className) { return this.collectionExists(className) .then(exist => {