Granular CLP pointer permissions (#6352)

* set pointer permissions per operatioon; tests

* more tests

* fixes addField permission; tests

spec/ParseObject.spec.js

@@ -313,7 +313,15 @@ describe('Parse.Object testing', () => {
 
   it('invalid __type', function(done) {
     const item = new Parse.Object('Item');
-    const types = ['Pointer', 'File', 'Date', 'GeoPoint', 'Bytes', 'Polygon'];
+    const types = [
+      'Pointer',
+      'File',
+      'Date',
+      'GeoPoint',
+      'Bytes',
+      'Polygon',
+      'Relation',
+    ];
     const tests = types.map(type => {
       const test = new Parse.Object('Item');
       test.set('foo', {

spec/ProtectedFields.spec.js

@@ -758,4 +758,37 @@ describe('ProtectedFields', function() {
       });
     });
   });
+
+  describe('schema setup', () => {
+    const className = 'AObject';
+    async function updateCLP(clp) {
+      const config = Config.get(Parse.applicationId);
+      const schemaController = await config.database.loadSchema();
+
+      await schemaController.updateClass(className, {}, clp);
+    }
+
+    it('should fail setting non-existing protected field', async () => {
+      const object = new Parse.Object(className, {
+        revision: 0,
+      });
+      await object.save();
+
+      const field = 'non-existing';
+      const entity = '*';
+
+      await expectAsync(
+        updateCLP({
+          protectedFields: {
+            [entity]: [field],
+          },
+        })
+      ).toBeRejectedWith(
+        new Parse.Error(
+          Parse.Error.INVALID_JSON,
+          `Field '${field}' in protectedFields:${entity} does not exist`
+        )
+      );
+    });
+  });
 });

spec/Schema.spec.js

@@ -1665,7 +1665,7 @@ describe('Class Level Permissions for requiredAuth', () => {
       );
   });
 
-  it('required auth test get not authenitcated', done => {
+  it('required auth test get not authenticated', done => {
     config.database
       .loadSchema()
       .then(schema => {
@@ -1704,7 +1704,7 @@ describe('Class Level Permissions for requiredAuth', () => {
       );
   });
 
-  it('required auth test find not authenitcated', done => {
+  it('required auth test find not authenticated', done => {
     config.database
       .loadSchema()
       .then(schema => {

spec/schemas.spec.js

@@ -2752,6 +2752,115 @@ describe('schemas', () => {
       );
   });
 
+  it('should reject creating class schema with field with invalid key', async done => {
+    const config = Config.get(Parse.applicationId);
+    const schemaController = await config.database.loadSchema();
+
+    const fieldName = '1invalid';
+
+    const schemaCreation = () =>
+      schemaController.addClassIfNotExists('AnObject', {
+        [fieldName]: { __type: 'String' },
+      });
+
+    await expectAsync(schemaCreation()).toBeRejectedWith(
+      new Parse.Error(
+        Parse.Error.INVALID_KEY_NAME,
+        `invalid field name: ${fieldName}`
+      )
+    );
+    done();
+  });
+
+  it('should reject creating invalid field name', async done => {
+    const object = new Parse.Object('AnObject');
+
+    await expectAsync(
+      object.save({
+        '!12field': 'field',
+      })
+    ).toBeRejectedWith(new Parse.Error(Parse.Error.INVALID_KEY_NAME));
+    done();
+  });
+
+  it('should be rejected if CLP operation is not an object', async done => {
+    const config = Config.get(Parse.applicationId);
+    const schemaController = await config.database.loadSchema();
+
+    const operationKey = 'get';
+    const operation = true;
+
+    const schemaSetup = async () =>
+      await schemaController.addClassIfNotExists(
+        'AnObject',
+        {},
+        {
+          [operationKey]: operation,
+        }
+      );
+
+    await expectAsync(schemaSetup()).toBeRejectedWith(
+      new Parse.Error(
+        Parse.Error.INVALID_JSON,
+        `'${operation}' is not a valid value for class level permissions ${operationKey} - must be an object`
+      )
+    );
+
+    done();
+  });
+
+  it('should be rejected if CLP protectedFields is not an object', async done => {
+    const config = Config.get(Parse.applicationId);
+    const schemaController = await config.database.loadSchema();
+
+    const operationKey = 'get';
+    const operation = 'wrongtype';
+
+    const schemaSetup = async () =>
+      await schemaController.addClassIfNotExists(
+        'AnObject',
+        {},
+        {
+          [operationKey]: operation,
+        }
+      );
+
+    await expectAsync(schemaSetup()).toBeRejectedWith(
+      new Parse.Error(
+        Parse.Error.INVALID_JSON,
+        `'${operation}' is not a valid value for class level permissions ${operationKey} - must be an object`
+      )
+    );
+
+    done();
+  });
+
+  it('should be rejected if CLP read/writeUserFields is not an array', async done => {
+    const config = Config.get(Parse.applicationId);
+    const schemaController = await config.database.loadSchema();
+
+    const operationKey = 'readUserFields';
+    const operation = true;
+
+    const schemaSetup = async () =>
+      await schemaController.addClassIfNotExists(
+        'AnObject',
+        {},
+        {
+          [operationKey]: operation,
+        }
+      );
+
+    await expectAsync(schemaSetup()).toBeRejectedWith(
+      new Parse.Error(
+        Parse.Error.INVALID_JSON,
+        `'${operation}' is not a valid value for class level permissions ${operationKey} - must be an array`
+      )
+    );
+
+    done();
+  });
+
   describe('index management', () => {
     beforeEach(() => require('../lib/TestUtils').destroyAllDataPermanently());
     it('cannot create index if field does not exist', done => {

src/Adapters/Storage/StorageAdapter.js

@@ -16,6 +16,8 @@ export type QueryOptions = {
   readPreference?: ?string,
   hint?: ?mixed,
   explain?: Boolean,
+  action?: string,
+  addsField?: boolean,
 };
 
 export type UpdateQueryOptions = {

src/Controllers/DatabaseController.js

@@ -553,9 +553,10 @@ class DatabaseController {
     className: string,
     object: any,
     query: any,
-    { acl }: QueryOptions
+    runOptions: QueryOptions
   ): Promise<boolean> {
     let schema;
+    const acl = runOptions.acl;
     const isMaster = acl === undefined;
     var aclGroup: string[] = acl || [];
     return this.loadSchema()
@@ -564,7 +565,13 @@ class DatabaseController {
         if (isMaster) {
           return Promise.resolve();
         }
-        return this.canAddField(schema, className, object, aclGroup);
+        return this.canAddField(
+          schema,
+          className,
+          object,
+          aclGroup,
+          runOptions
+        );
       })
       .then(() => {
         return schema.validateObject(className, object, query);
@@ -575,7 +582,7 @@ class DatabaseController {
     className: string,
     query: any,
     update: any,
-    { acl, many, upsert }: FullQueryOptions = {},
+    { acl, many, upsert, addsField }: FullQueryOptions = {},
     skipSanitization: boolean = false,
     validateOnly: boolean = false,
     validSchemaController: SchemaController.SchemaController
@@ -608,6 +615,21 @@ class DatabaseController {
                 query,
                 aclGroup
               );
+
+              if (addsField) {
+                query = {
+                  $and: [
+                    query,
+                    this.addPointerPermissions(
+                      schemaController,
+                      className,
+                      'addField',
+                      query,
+                      aclGroup
+                    ),
+                  ],
+                };
+              }
             }
             if (!query) {
               return Promise.resolve();
@@ -994,7 +1016,8 @@ class DatabaseController {
     schema: SchemaController.SchemaController,
     className: string,
     object: any,
-    aclGroup: string[]
+    aclGroup: string[],
+    runOptions: QueryOptions
   ): Promise<void> {
     const classSchema = schema.schemaData[className];
     if (!classSchema) {
@@ -1014,7 +1037,11 @@ class DatabaseController {
       return schemaFields.indexOf(field) < 0;
     });
     if (newKeys.length > 0) {
-      return schema.validatePermission(className, aclGroup, 'addField');
+      // adds a marker that new field is being adding during update
+      runOptions.addsField = true;
+
+      const action = runOptions.action;
+      return schema.validatePermission(className, aclGroup, 'addField', action);
     }
     return Promise.resolve();
   }
@@ -1525,28 +1552,50 @@ class DatabaseController {
       });
   }
 
+  // Constraints query using CLP's pointer permissions (PP) if any.
+  // 1. Etract the user id from caller's ACLgroup;
+  // 2. Exctract a list of field names that are PP for target collection and operation;
+  // 3. Constraint the original query so that each PP field must
+  // point to caller's id (or contain it in case of PP field being an array)
   addPointerPermissions(
     schema: SchemaController.SchemaController,
     className: string,
     operation: string,
     query: any,
     aclGroup: any[] = []
-  ) {
+  ): any {
     // Check if class has public permission for operation
     // If the BaseCLP pass, let go through
     if (schema.testPermissionsForClassName(className, aclGroup, operation)) {
       return query;
     }
     const perms = schema.getClassLevelPermissions(className);
-    const field =
-      ['get', 'find'].indexOf(operation) > -1
-        ? 'readUserFields'
-        : 'writeUserFields';
+
     const userACL = aclGroup.filter(acl => {
       return acl.indexOf('role:') != 0 && acl != '*';
     });
+
+    const groupKey =
+      ['get', 'find', 'count'].indexOf(operation) > -1
+        ? 'readUserFields'
+        : 'writeUserFields';
+
+    const permFields = [];
+
+    if (perms[operation] && perms[operation].pointerFields) {
+      permFields.push(...perms[operation].pointerFields);
+    }
+
+    if (perms[groupKey]) {
+      for (const field of perms[groupKey]) {
+        if (!permFields.includes(field)) {
+          permFields.push(field);
+        }
+      }
+    }
     // the ACL should have exactly 1 user
-    if (perms && perms[field] && perms[field].length > 0) {
+    if (permFields.length > 0) {
+      // the ACL should have exactly 1 user
       // No user set return undefined
       // If the length is > 1, that means we didn't de-dupe users correctly
       if (userACL.length != 1) {
@@ -1559,7 +1608,6 @@ class DatabaseController {
         objectId: userId,
       };
 
-      const permFields = perms[field];
       const ors = permFields.flatMap(key => {
         // constraint for single pointer setup
         const q = {
@@ -1588,7 +1636,7 @@ class DatabaseController {
     query: any = {},
     aclGroup: any[] = [],
     auth: any = {}
-  ) {
+  ): null | string[] {
     const perms = schema.getClassLevelPermissions(className);
     if (!perms) return null;
 

src/Controllers/SchemaController.js

@@ -182,11 +182,14 @@ const publicRegex = /^\*$/;
 
 const requireAuthenticationRegex = /^requiresAuthentication$/;
 
+const pointerFieldsRegex = /^pointerFields$/;
+
 const permissionKeyRegex = Object.freeze([
   roleRegex,
   pointerPermissionRegex,
   publicRegex,
   requireAuthenticationRegex,
+  pointerFieldsRegex,
 ]);
 
 function validatePermissionKey(key, userIdRegExp) {
@@ -238,26 +241,19 @@ function validateCLP(
     }
 
     const operation = perms[operationKey];
-    if (!operation) {
-      // proceed with next operationKey
-      continue;
-    }
+    // proceed with next operationKey
+
+    // throws when root fields are of wrong type
+    validateCLPjson(operation, operationKey);
 
-    // validate grouped pointer permissions
     if (
       operationKey === 'readUserFields' ||
       operationKey === 'writeUserFields'
     ) {
+      // validate grouped pointer permissions
       // must be an array with field names
-      if (!Array.isArray(operation)) {
-        throw new Parse.Error(
-          Parse.Error.INVALID_JSON,
-          `'${operation}' is not a valid value for class level permissions ${operationKey}`
-        );
-      } else {
-        for (const fieldName of operation) {
-          validatePointerPermission(fieldName, fields, operationKey);
-        }
+      for (const fieldName of operation) {
+        validatePointerPermission(fieldName, fields, operationKey);
       }
       // readUserFields and writerUserFields do not have nesdted fields
       // proceed with next operationKey
@@ -299,11 +295,29 @@ function validateCLP(
     // "*" - Public,
     // "requiresAuthentication" - authenticated users,
     // "objectId" - _User id,
-    // "role:objectId",
+    // "role:rolename",
+    // "pointerFields" - array of field names containing pointers to users
     for (const entity in operation) {
       // throws on unexpected key
       validatePermissionKey(entity, userIdRegExp);
 
+      if (entity === 'pointerFields') {
+        const pointerFields = operation[entity];
+
+        if (Array.isArray(pointerFields)) {
+          for (const pointerField of pointerFields) {
+            validatePointerPermission(pointerField, fields, operation);
+          }
+        } else {
+          throw new Parse.Error(
+            Parse.Error.INVALID_JSON,
+            `'${pointerFields}' is not a valid value for protectedFields[${entity}] - expected an array.`
+          );
+        }
+        // proceed with next entity key
+        continue;
+      }
+
       const permit = operation[entity];
 
       if (permit !== true) {
@@ -316,13 +330,34 @@ function validateCLP(
   }
 }
 
+function validateCLPjson(operation: any, operationKey: string) {
+  if (operationKey === 'readUserFields' || operationKey === 'writeUserFields') {
+    if (!Array.isArray(operation)) {
+      throw new Parse.Error(
+        Parse.Error.INVALID_JSON,
+        `'${operation}' is not a valid value for class level permissions ${operationKey} - must be an array`
+      );
+    }
+  } else {
+    if (typeof operation === 'object' && operation !== null) {
+      // ok to proceed
+      return;
+    } else {
+      throw new Parse.Error(
+        Parse.Error.INVALID_JSON,
+        `'${operation}' is not a valid value for class level permissions ${operationKey} - must be an object`
+      );
+    }
+  }
+}
+
 function validatePointerPermission(
   fieldName: string,
   fields: Object,
   operation: string
 ) {
   // Uses collection schema to ensure the field is of type:
-  // - Pointer<_User> (pointers/relations)
+  // - Pointer<_User> (pointers)
   // - Array
   //
   //    It's not possible to enforce type on Array's items in schema
@@ -1340,7 +1375,8 @@ export default class SchemaController {
     classPermissions: ?any,
     className: string,
     aclGroup: string[],
-    operation: string
+    operation: string,
+    action?: string
   ) {
     if (
       SchemaController.testPermissions(classPermissions, aclGroup, operation)
@@ -1394,6 +1430,16 @@ export default class SchemaController {
     ) {
       return Promise.resolve();
     }
+
+    const pointerFields = classPermissions[operation].pointerFields;
+    if (Array.isArray(pointerFields) && pointerFields.length > 0) {
+      // any op except 'addField as part of create' is ok.
+      if (operation !== 'addField' || action === 'update') {
+        // We can allow adding field on update flow only.
+        return Promise.resolve();
+      }
+    }
+
     throw new Parse.Error(
       Parse.Error.OPERATION_FORBIDDEN,
       `Permission denied for action ${operation} on class ${className}.`
@@ -1401,12 +1447,18 @@ export default class SchemaController {
   }
 
   // Validates an operation passes class-level-permissions set in the schema
-  validatePermission(className: string, aclGroup: string[], operation: string) {
+  validatePermission(
+    className: string,
+    aclGroup: string[],
+    operation: string,
+    action?: string
+  ) {
     return SchemaController.validatePermission(
       this.getClassLevelPermissions(className),
       className,
       aclGroup,
-      operation
+      operation,
+      action
     );
   }
 

src/RestWrite.js

@@ -31,7 +31,8 @@ function RestWrite(
   query,
   data,
   originalData,
-  clientSDK
+  clientSDK,
+  action
 ) {
   if (auth.isReadOnly) {
     throw new Parse.Error(
@@ -47,6 +48,10 @@ function RestWrite(
   this.runOptions = {};
   this.context = {};
 
+  if (action) {
+    this.runOptions.action = action;
+  }
+
   if (!query) {
     if (this.config.allowCustomObjectId) {
       if (

src/rest.js

@@ -251,7 +251,8 @@ function update(config, auth, className, restWhere, restObject, clientSDK) {
         restWhere,
         restObject,
         originalRestObject,
-        clientSDK
+        clientSDK,
+        'update'
       ).execute();
     })
     .catch(error => {