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

Author: drew-gross

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();
     });

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();
   });
 

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);
   }

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.

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,67 +348,35 @@ 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
-
-  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;
-    }
-  }
-  output._rperm = rperm;
-  output._wperm = wperm;
-  output._acl = _acl;
-  delete restObject.ACL;
-  return output;
-}
+// Add the legacy _acl format.
+const addLegacyACL = restObject => {
+  let restObjectCopy = {...restObject};
+  let _acl = {};
 
-// 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;
+  if (restObject._wperm) {
+    restObject._wperm.forEach(entry => {
+      _acl[entry] = { w: true };
+    });
+  }
+
+  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;
 }
 
+
 // A sentinel value that helper transformations return when they
 // cannot perform a transformation
 function CannotTransform() {}
@@ -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':

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 => {