Fixes for Class Level and Pointer Permissions (#1989)

* Fixes for Pointer Permissions

- Fix bug that would leave public CLP when setting a new set of permissions
- Sets empty permissions if missing to match parse.com API
- Updates tests to reflect changes

* Adds regression test for #1991

* Fit -> It

spec/PointerPermissions.spec.js

@@ -282,7 +282,7 @@ describe('Pointer Permissions', () => {
   it('tests CLP / Pointer Perms / ACL write (PP Locked)', (done) => {
     /*
       tests:
-      CLP: update open ({"*": true})
+      CLP: update closed ({})
       PointerPerm: "owner"
       ACL: logged in user has access
 
@@ -300,7 +300,7 @@ describe('Pointer Permissions', () => {
       password: 'password'
     });
     let obj = new Parse.Object('AnObject');
-    Parse.Object.saveAll([user, user2]).then(() => {
+    Parse.Object.saveAll([user, user2]).then(() => {
       let ACL = new Parse.ACL();
       ACL.setReadAccess(user, true);
       ACL.setWriteAccess(user, true);
@@ -310,7 +310,7 @@ describe('Pointer Permissions', () => {
     }).then(() => {
       return config.database.loadSchema().then((schema) => {
         // Lock the update, and let only owner write
-        return schema.updateClass('AnObject', {}, {update: {"*": true}, writeUserFields: ['owner']});
+        return schema.updateClass('AnObject', {}, {update: {}, writeUserFields: ['owner']});
       });
     }).then(() => {
       return Parse.User.logIn('user1', 'password');
@@ -329,7 +329,7 @@ describe('Pointer Permissions', () => {
   it('tests CLP / Pointer Perms / ACL write (ACL Locked)', (done) => {
     /*
       tests:
-      CLP: update open ({"*": true})
+      CLP: update closed ({})
       PointerPerm: "owner"
       ACL: logged in user has access
      */
@@ -355,7 +355,7 @@ describe('Pointer Permissions', () => {
     }).then(() => {
       return config.database.loadSchema().then((schema) => {
         // Lock the update, and let only owner write
-        return schema.updateClass('AnObject', {}, {update: {"*": true}, writeUserFields: ['owner']});
+        return schema.updateClass('AnObject', {}, {update: {}, writeUserFields: ['owner']});
       });
     }).then(() => {
       return Parse.User.logIn('user2', 'password');
@@ -374,7 +374,7 @@ describe('Pointer Permissions', () => {
   it('tests CLP / Pointer Perms / ACL write (ACL/PP OK)', (done) => {
     /*
       tests:
-      CLP: update open ({"*": true})
+      CLP: update closed ({})
       PointerPerm: "owner"
       ACL: logged in user has access
      */
@@ -400,7 +400,7 @@ describe('Pointer Permissions', () => {
     }).then(() => {
       return config.database.loadSchema().then((schema) => {
         // Lock the update, and let only owner write
-        return schema.updateClass('AnObject', {}, {update: {"*": true}, writeUserFields: ['owner']});
+        return schema.updateClass('AnObject', {}, {update: {}, writeUserFields: ['owner']});
       });
     }).then(() => {
       return Parse.User.logIn('user2', 'password');
@@ -419,7 +419,7 @@ describe('Pointer Permissions', () => {
   it('tests CLP / Pointer Perms / ACL read (PP locked)', (done) => {
     /*
       tests:
-      CLP: find/get open ({"*": true})
+      CLP: find/get open ({})
       PointerPerm: "owner" : read
       ACL: logged in user has access
 
@@ -447,7 +447,7 @@ describe('Pointer Permissions', () => {
     }).then(() => {
       return config.database.loadSchema().then((schema) => {
         // Lock the update, and let only owner write
-        return schema.updateClass('AnObject', {}, {find: {"*": true}, get: {"*": true}, readUserFields: ['owner']});
+        return schema.updateClass('AnObject', {}, {find: {}, get: {}, readUserFields: ['owner']});
       });
     }).then(() => {
       return Parse.User.logIn('user1', 'password');

spec/Schema.spec.js

@@ -132,6 +132,7 @@ describe('SchemaController', () => {
         var get = {};
         get[user.id] = true;
         return schema.setPermissions('Stuff', {
+          'create': {'*': true},
           'find': find,
           'get': get
         });
@@ -152,6 +153,7 @@ describe('SchemaController', () => {
           done();
         }, (e) => {
           fail('Class permissions should have allowed this get query');
+          done();
         });
       });
     });

spec/schemas.spec.js

@@ -963,18 +963,10 @@ describe('schemas', () => {
           create: {
             'role:admin': true
           },
-          get: {
+          get: {},
-            '*': true
+          update: {},
-          },
+          delete: {},
-          update: {
+          addField: {}
-            '*': true
-          },
-          addField: {
-            '*': true
-          },
-          delete: {
-            '*': true
-          }
         });
         done();
       });
@@ -1018,6 +1010,9 @@ describe('schemas', () => {
       json: true,
       body: {
         classLevelPermissions: {
+          create: {
+            '*': true
+          },
           find: {
             '*': true
           },
@@ -1040,14 +1035,14 @@ describe('schemas', () => {
     })
   });
 
-  it('should not be able to add a field', done => {
+  it('should be able to add a field', done => {
     request.post({
       url: 'http://localhost:8378/1/schemas/AClass',
       headers: masterKeyHeaders,
       json: true,
       body: {
         classLevelPermissions: {
-          find: {
+          create: {
             '*': true
           },
           addField: {
@@ -1243,7 +1238,7 @@ describe('schemas', () => {
     }).then(() => {
      return Parse.User.logIn('user', 'user').then(() => {
         let obj = new Parse.Object('AClass');
-        return obj.save();
+        return obj.save(null, {useMasterKey: true});
       })
     }).then(() => {
       let query = new Parse.Query('AClass');
@@ -1292,7 +1287,7 @@ describe('schemas', () => {
     }).then(() => {
      return Parse.User.logIn('user', 'user').then(() => {
         let obj = new Parse.Object('AClass');
-        return obj.save();
+        return obj.save(null, {useMasterKey: true});
       })
     }).then(() => {
       let query = new Parse.Query('AClass');
@@ -1357,7 +1352,7 @@ describe('schemas', () => {
     }).then(() => {
      return Parse.User.logIn('user', 'user').then(() => {
         let obj = new Parse.Object('AClass');
-        return obj.save();
+        return obj.save(null, {useMasterKey: true});
       })
     }).then(() => {
       let query = new Parse.Query('AClass');
@@ -1415,7 +1410,7 @@ describe('schemas', () => {
     }).then(() => {
      return Parse.User.logIn('user', 'user').then(() => {
         let obj = new Parse.Object('AClass');
-        return obj.save();
+        return obj.save(null, {useMasterKey: true});
       })
     }).then(() => {
       let query = new Parse.Query('AClass');
@@ -1544,6 +1539,7 @@ describe('schemas', () => {
 
   it('can login when addFields is false (issue #1355)', (done) => {
     setPermissionsOnClass('_User', {
+      'create': {'*': true},
       'addField': {}
     }).then(() => {
       return Parse.User.signUp('foo', 'bar');
@@ -1573,4 +1569,40 @@ describe('schemas', () => {
       });
     });
   });
+
+  it("regression test for #1991", done => {
+    let user = new Parse.User();
+    user.setUsername('user');
+    user.setPassword('user');
+    let role = new Parse.Role('admin', new Parse.ACL());
+    let obj = new Parse.Object('AnObject');
+    Parse.Object.saveAll([user, role]).then(() => {
+      role.relation('users').add(user);
+      return role.save(null, {useMasterKey: true});
+    }).then(() => {
+      return setPermissionsOnClass('AnObject', {
+        'get': {"*": true},
+        'find': {"*": true},
+        'create': {'*': true},
+        'update': {'role:admin': true},
+        'delete': {'role:admin': true}
+      })
+    }).then(() => {
+      return obj.save();
+    }).then(() => {
+      return Parse.User.logIn('user', 'user')
+    }).then(() => {
+      return obj.destroy();
+    }).then((result) => {
+      let query = new Parse.Query('AnObject');
+      return query.find();
+    }).then((results) => {
+      expect(results.length).toBe(0);
+      done();
+    }).catch((err) => {
+      fail('should not fail');
+      console.error(err);
+      done();
+    });
+  });
 });

src/Adapters/Storage/Mongo/MongoSchemaCollection.js

@@ -43,6 +43,15 @@ function mongoSchemaFieldsToParseSchemaFields(schema) {
   return response;
 }
 
+const emptyCLPS = Object.freeze({
+  find: {},
+  get: {},
+  create: {},
+  update: {},
+  delete: {},
+  addField: {},
+});
+
 const defaultCLPS = Object.freeze({
   find: {'*': true},
   get: {'*': true},
@@ -53,14 +62,14 @@ const defaultCLPS = Object.freeze({
 });
 
 function mongoSchemaToParseSchema(mongoSchema) {
-  let clpsFromMongoObject = {};
+  let clps = defaultCLPS;
   if (mongoSchema._metadata && mongoSchema._metadata.class_permissions) {
-    clpsFromMongoObject = mongoSchema._metadata.class_permissions;
+    clps = {...emptyCLPS, ...mongoSchema._metadata.class_permissions};
   }
   return {
     className: mongoSchema._id,
     fields: mongoSchemaFieldsToParseSchemaFields(mongoSchema),
-    classLevelPermissions: {...defaultCLPS, ...clpsFromMongoObject},
+    classLevelPermissions: clps,
   };
 }
 

src/Controllers/DatabaseController.js

@@ -704,6 +704,11 @@ DatabaseController.prototype.deleteSchema = function(className) {
 }
 
 DatabaseController.prototype.addPointerPermissions = function(schema, className, operation, query, aclGroup = []) {
+  // Check if class has public permission for operation
+  // If the BaseCLP pass, let go through
+  if (schema.testBaseCLP(className, aclGroup, operation)) {
+    return query;
+  }
   let perms = schema.perms[className];
   let field = ['get', 'find'].indexOf(operation) > -1 ? 'readUserFields' : 'writeUserFields';
   let userACL = aclGroup.filter((acl) => {

src/Controllers/SchemaController.js

@@ -632,30 +632,36 @@ class SchemaController {
     }
     return Promise.resolve(this);
   }
-
+  
-  // Validates an operation passes class-level-permissions set in the schema
+  // Validates the base CLP for an operation
-  validatePermission(className, aclGroup, operation) {
+  testBaseCLP(className, aclGroup, operation) {
     if (!this.perms[className] || !this.perms[className][operation]) {
-      return Promise.resolve();
+      return true;
     }
     let classPerms = this.perms[className];
     let perms = classPerms[operation];
     // Handle the public scenario quickly
     if (perms['*']) {
-      return Promise.resolve();
+      return true;
     }
     // Check permissions against the aclGroup provided (array of userId/roles)
-    let found = false;
+    if (aclGroup.some(acl => { return perms[acl] === true })) {
-    for (let i = 0; i < aclGroup.length && !found; i++) {
+      return true;
-      if (perms[aclGroup[i]]) {
-        found = true;
-      }
     }
+    return false;
+  }
 
-    if (found) {
+  // Validates an operation passes class-level-permissions set in the schema
+  validatePermission(className, aclGroup, operation) {
+    if (this.testBaseCLP(className, aclGroup, operation)) {
       return Promise.resolve();
     }
 
+    if (!this.perms[className] || !this.perms[className][operation]) {
+      return true;
+    }
+    let classPerms = this.perms[className];
+    let perms = classPerms[operation];
     // No matching CLP, let's check the Pointer permissions
     // And handle those later
     let permissionField = ['get', 'find'].indexOf(operation) > -1 ? 'readUserFields' : 'writeUserFields';
@@ -666,6 +672,7 @@ class SchemaController {
         'Permission denied for this action.');
     }
 
+    // Process the readUserFields later
     if (Array.isArray(classPerms[permissionField]) && classPerms[permissionField].length > 0) {
         return Promise.resolve();
     }