Allow protectedFields for Authenticated users and Public. Fix userField with keys/excludedKeys (#6415)

* fix error message and test it

* protected fields fixes

* clean

* remove duplicate test, add some comments

* no need for 'requiresAuthentication'

spec/ParseGraphQLServer.spec.js

@@ -5,6 +5,8 @@ const fetch = require('node-fetch');
 const FormData = require('form-data');
 const ws = require('ws');
 require('./helper');
+const { updateCLP } = require('./dev');
+
 const pluralize = require('pluralize');
 const { getMainDefinition } = require('apollo-utilities');
 const { ApolloLink, split } = require('apollo-link');
@@ -4632,6 +4634,84 @@ describe('ParseGraphQLServer', () => {
             ).toBeDefined();
           });
 
+          it('should respect protectedFields', async done => {
+            await prepareData();
+            await parseGraphQLServer.parseGraphQLSchema.databaseController.schemaCache.clear();
+
+            const className = 'GraphQLClass';
+
+            await updateCLP(
+              {
+                get: { '*': true },
+                find: { '*': true },
+
+                protectedFields: {
+                  '*': ['someField', 'someOtherField'],
+                  authenticated: ['someField'],
+                  'userField:pointerToUser': [],
+                  [user2.id]: [],
+                },
+              },
+              className
+            );
+
+            const getObject = async (className, id, user) => {
+              const headers = user
+                ? { ['X-Parse-Session-Token']: user.getSessionToken() }
+                : undefined;
+
+              const specificQueryResult = await apolloClient.query({
+                query: gql`
+                  query GetSomeObject($id: ID!) {
+                    get: graphQLClass(id: $id) {
+                      pointerToUser {
+                        username
+                        id
+                      }
+                      someField
+                      someOtherField
+                    }
+                  }
+                `,
+                variables: {
+                  id: id,
+                },
+                context: {
+                  headers: headers,
+                },
+              });
+
+              return specificQueryResult.data.get;
+            };
+
+            const id = object3.id;
+
+            /* not authenticated */
+            const objectPublic = await getObject(className, id, undefined);
+
+            expect(objectPublic.someField).toBeNull();
+            expect(objectPublic.someOtherField).toBeNull();
+
+            /* authenticated */
+            const objectAuth = await getObject(className, id, user1);
+
+            expect(objectAuth.someField).toBeNull();
+            expect(objectAuth.someOtherField).toBe('B');
+
+            /* pointer field */
+            const objectPointed = await getObject(className, id, user5);
+
+            expect(objectPointed.someField).toBe('someValue3');
+            expect(objectPointed.someOtherField).toBe('B');
+
+            /* for user id */
+            const objectForUser = await getObject(className, id, user2);
+
+            expect(objectForUser.someField).toBe('someValue3');
+            expect(objectForUser.someOtherField).toBe('B');
+
+            done();
+          });
           describe_only_db('mongo')('read preferences', () => {
             it('should read from primary by default', async () => {
               try {

spec/ParseQuery.spec.js

@@ -4868,4 +4868,36 @@ describe('Parse.Query testing', () => {
     const results = await query.find();
     equal(results[0].get('array').length, 105);
   });
+
+  it('exclude keys (sdk query)', async done => {
+    const obj = new TestObject({ foo: 'baz', hello: 'world' });
+    await obj.save();
+
+    const query = new Parse.Query('TestObject');
+    query.exclude('foo');
+
+    const object = await query.get(obj.id);
+    expect(object.get('foo')).toBeUndefined();
+    expect(object.get('hello')).toBe('world');
+    done();
+  });
+
+  xit('todo: exclude keys with select key (sdk query get)', async done => {
+    // there is some problem with js sdk caching
+
+    const obj = new TestObject({ foo: 'baz', hello: 'world' });
+    await obj.save();
+
+    const query = new Parse.Query('TestObject');
+
+    query.withJSON({
+      keys: 'hello',
+      excludeKeys: 'hello',
+    });
+
+    const object = await query.get(obj.id);
+    expect(object.get('foo')).toBeUndefined();
+    expect(object.get('hello')).toBeUndefined();
+    done();
+  });
 });

spec/PointerPermissions.spec.js

@@ -2047,7 +2047,7 @@ describe('Pointer Permissions', () => {
     }
 
     async function logIn(userObject) {
-      await Parse.User.logIn(userObject.getUsername(), 'password');
+      return await Parse.User.logIn(userObject.getUsername(), 'password');
     }
 
     async function updateCLP(clp) {
@@ -3098,5 +3098,55 @@ describe('Pointer Permissions', () => {
         done();
       });
     });
+
+    describe('using pointer-fields and queries with keys projection', () => {
+      let user1;
+      /**
+       * owner: user1
+       *
+       * testers: [user1]
+       */
+      let obj;
+
+      /**
+       * Clear cache, create user and object, login user
+       */
+      async function initialize() {
+        await Config.get(Parse.applicationId).database.schemaCache.clear();
+
+        user1 = await createUser('user1');
+        user1 = await logIn(user1);
+
+        obj = new Parse.Object(className);
+
+        obj.set('owner', user1);
+        obj.set('field', 'field');
+        obj.set('test', 'test');
+
+        await Parse.Object.saveAll([obj], { useMasterKey: true });
+
+        await obj.fetch();
+      }
+
+      beforeEach(async () => {
+        await initialize();
+      });
+
+      it('should be enforced regardless of pointer-field being included in keys (select)', async done => {
+        await updateCLP({
+          get: { '*': true },
+          find: { pointerFields: ['owner'] },
+          update: { pointerFields: ['owner'] },
+        });
+
+        const query = new Parse.Query('AnObject');
+        query.select('field', 'test');
+
+        const [object] = await query.find({ objectId: obj.id });
+        expect(object.get('field')).toBe('field');
+        expect(object.get('test')).toBe('test');
+        done();
+      });
+    });
   });
 });

spec/dev.js

@@ -0,0 +1,98 @@
+const Config = require('../lib/Config');
+const Parse = require('parse/node');
+
+const className = 'AnObject';
+const defaultRoleName = 'tester';
+
+let schemaCache;
+
+module.exports = {
+  /* AnObject */
+  className,
+  schemaCache,
+
+  /**
+   * Creates and returns new user.
+   *
+   * This method helps to avoid 'User already exists' when re-running/debugging a single test.
+   * @param {string} username - username base, will be postfixed with current time in millis;
+   * @param {string} [password='password'] - optional, defaults to "password" if not set;
+   */
+  createUser: async (username, password = 'password') => {
+    const user = new Parse.User({
+      username: username + Date.now(),
+      password,
+    });
+    await user.save();
+    return user;
+  },
+
+  /**
+   * Logs the user in.
+   *
+   * If password not provided, default 'password' is used.
+   * @param {string} username - username base, will be postfixed with current time in millis;
+   * @param {string} [password='password'] - optional, defaults to "password" if not set;
+   */
+  logIn: async (userObject, password) => {
+    return await Parse.User.logIn(
+      userObject.getUsername(),
+      password || 'password'
+    );
+  },
+
+  /**
+   * Sets up Class-Level Permissions for 'AnObject' class.
+   * @param clp {ClassLevelPermissions}
+   */
+  updateCLP: async (clp, targetClass = className) => {
+    const config = Config.get(Parse.applicationId);
+    const schemaController = await config.database.loadSchema();
+
+    await schemaController.updateClass(targetClass, {}, clp);
+  },
+
+  /**
+   * Creates and returns role. Adds user(s) if provided.
+   *
+   * This method helps to avoid errors when re-running/debugging a single test.
+   *
+   * @param {Parse.User|Parse.User[]} [users] - user or array of users to be related with this role;
+   * @param {string?} [roleName] - uses this name for role if provided. Generates from datetime if not set;
+   * @param {string?} [exactName] - sets exact name (no generated part added);
+   * @param {Parse.Role[]} [roles] - uses this name for role if provided. Generates from datetime if not set;
+   * @param {boolean} [read] - value for role's acl public read. Defaults to true;
+   * @param {boolean} [write] - value for role's acl public write. Defaults to true;
+   */
+  createRole: async ({
+    users = null,
+    exactName = defaultRoleName + Date.now(),
+    roleName = null,
+    roles = null,
+    read = true,
+    write = true,
+  }) => {
+    const acl = new Parse.ACL();
+    acl.setPublicReadAccess(read);
+    acl.setPublicWriteAccess(write);
+
+    const role = new Parse.Object('_Role');
+    role.setACL(acl);
+
+    // generate name based on roleName or use exactName (if botth not provided name is generated)
+    const name = roleName ? roleName + Date.now() : exactName;
+    role.set('name', name);
+
+    if (roles) {
+      role.relation('roles').add(roles);
+    }
+
+    if (users) {
+      role.relation('users').add(users);
+    }
+
+    await role.save({ useMasterKey: true });
+
+    return role;
+  },
+};

spec/schemas.spec.js

@@ -2861,6 +2861,35 @@ describe('schemas', () => {
     done();
   });
 
+  it('should be rejected if CLP pointerFields is not an array', async done => {
+    const config = Config.get(Parse.applicationId);
+    const schemaController = await config.database.loadSchema();
+
+    const operationKey = 'get';
+    const entity = 'pointerFields';
+    const value = {};
+
+    const schemaSetup = async () =>
+      await schemaController.addClassIfNotExists(
+        'AnObject',
+        {},
+        {
+          [operationKey]: {
+            [entity]: value,
+          },
+        }
+      );
+
+    await expectAsync(schemaSetup()).toBeRejectedWith(
+      new Parse.Error(
+        Parse.Error.INVALID_JSON,
+        `'${value}' is not a valid value for ${operationKey}[${entity}] - expected an array.`
+      )
+    );
+
+    done();
+  });
+
   describe('index management', () => {
     beforeEach(() => require('../lib/TestUtils').destroyAllDataPermanently());
     it('cannot create index if field does not exist', done => {

src/Controllers/DatabaseController.js

@@ -217,7 +217,7 @@ const filterSensitiveData = (
           return { key: key.substring(10), value: perms.protectedFields[key] };
         });
 
-      const newProtectedFields: Array<string> = [];
+      const newProtectedFields: Array<string>[] = [];
       let overrideProtectedFields = false;
 
       // check if the object grants the current user access based on the extracted fields
@@ -238,12 +238,28 @@ const filterSensitiveData = (
 
         if (pointerPermIncludesUser) {
           overrideProtectedFields = true;
-          newProtectedFields.push(...pointerPerm.value);
+          newProtectedFields.push(pointerPerm.value);
         }
       });
 
-      // if atleast one pointer-permission affected the current user override the protectedFields
-      if (overrideProtectedFields) protectedFields = newProtectedFields;
+      // if at least one pointer-permission affected the current user
+      // intersect vs protectedFields from previous stage (@see addProtectedFields)
+      // Sets theory (intersections): A x (B x C) == (A x B) x C
+      if (overrideProtectedFields && protectedFields) {
+        newProtectedFields.push(protectedFields);
+      }
+      // intersect all sets of protectedFields
+      newProtectedFields.forEach(fields => {
+        if (fields) {
+          // if there're no protctedFields by other criteria ( id / role / auth)
+          // then we must intersect each set (per userField)
+          if (!protectedFields) {
+            protectedFields = fields;
+          } else {
+            protectedFields = protectedFields.filter(v => fields.includes(v));
+          }
+        }
+      });
     }
   }
 
@@ -251,9 +267,16 @@ const filterSensitiveData = (
 
   /* special treat for the user class: don't filter protectedFields if currently loggedin user is
   the retrieved user */
-  if (!(isUserClass && userId && object.objectId === userId))
+  if (!(isUserClass && userId && object.objectId === userId)) {
     protectedFields && protectedFields.forEach(k => delete object[k]);
 
+    // fields not requested by client (excluded),
+    //but were needed to apply protecttedFields
+    perms.protectedFields &&
+      perms.protectedFields.temporaryKeys &&
+      perms.protectedFields.temporaryKeys.forEach(k => delete object[k]);
+  }
+
   if (!isUserClass) {
     return object;
   }
@@ -1416,7 +1439,8 @@ class DatabaseController {
                     className,
                     query,
                     aclGroup,
-                    auth
+                    auth,
+                    queryOptions
                   );
                 }
                 if (!query) {
@@ -1638,7 +1662,8 @@ class DatabaseController {
     className: string,
     query: any = {},
     aclGroup: any[] = [],
-    auth: any = {}
+    auth: any = {},
+    queryOptions: FullQueryOptions = {}
   ): null | string[] {
     const perms = schema.getClassLevelPermissions(className);
     if (!perms) return null;
@@ -1648,14 +1673,85 @@ class DatabaseController {
 
     if (aclGroup.indexOf(query.objectId) > -1) return null;
 
-    // remove userField keys since they are filtered after querying
-    let protectedKeys = Object.keys(protectedFields).reduce((acc, val) => {
-      if (val.startsWith('userField:')) return acc;
-      return acc.concat(protectedFields[val]);
+    // for queries where "keys" are set and do not include all 'userField':{field},
+    // we have to transparently include it, and then remove before returning to client
+    // Because if such key not projected the permission won't be enforced properly
+    // PS this is called when 'excludeKeys' already reduced to 'keys'
+    const preserveKeys = queryOptions.keys;
+
+    // these are keys that need to be included only
+    // to be able to apply protectedFields by pointer
+    // and then unset before returning to client (later in  filterSensitiveFields)
+    const serverOnlyKeys = [];
+
+    const authenticated = auth.user;
+
+    // map to allow check without array search
+    const roles = (auth.userRoles || []).reduce((acc, r) => {
+      acc[r] = protectedFields[r];
+      return acc;
+    }, {});
+
+    // array of sets of protected fields. separate item for each applicable criteria
+    const protectedKeysSets = [];
+
+    for (const key in protectedFields) {
+      // skip userFields
+      if (key.startsWith('userField:')) {
+        if (preserveKeys) {
+          const fieldName = key.substring(10);
+          if (!preserveKeys.includes(fieldName)) {
+            // 1. put it there temporarily
+            queryOptions.keys && queryOptions.keys.push(fieldName);
+            // 2. preserve it delete later
+            serverOnlyKeys.push(fieldName);
+          }
+        }
+        continue;
+      }
+
+      // add public tier
+      if (key === '*') {
+        protectedKeysSets.push(protectedFields[key]);
+        continue;
+      }
+
+      if (authenticated) {
+        if (key === 'authenticated') {
+          // for logged in users
+          protectedKeysSets.push(protectedFields[key]);
+          continue;
+        }
+
+        if (roles[key] && key.startsWith('role:')) {
+          // add applicable roles
+          protectedKeysSets.push(roles[key]);
+        }
+      }
+    }
+
+    // check if there's a rule for current user's id
+    if (authenticated) {
+      const userId = auth.user.id;
+      if (perms.protectedFields[userId]) {
+        protectedKeysSets.push(perms.protectedFields[userId]);
+      }
+    }
+
+    // preserve fields to be removed before sending response to client
+    if (serverOnlyKeys.length > 0) {
+      perms.protectedFields.temporaryKeys = serverOnlyKeys;
+    }
+
+    let protectedKeys = protectedKeysSets.reduce((acc, next) => {
+      if (next) {
+        acc.push(...next);
+      }
+      return acc;
     }, []);
 
-    [...(auth.userRoles || [])].forEach(role => {
-      const fields = protectedFields[role];
+    // intersect all sets of protectedFields
+    protectedKeysSets.forEach(fields => {
       if (fields) {
         protectedKeys = protectedKeys.filter(v => fields.includes(v));
       }

src/Controllers/SchemaController.js

@@ -175,32 +175,62 @@ const volatileClasses = Object.freeze([
 
 // Anything that start with role
 const roleRegex = /^role:.*/;
-// Anything that starts with userField
-const pointerPermissionRegex = /^userField:.*/;
+// Anything that starts with userField (allowed for protected fields only)
+const protectedFieldsPointerRegex = /^userField:.*/;
 // * permission
 const publicRegex = /^\*$/;
 
-const requireAuthenticationRegex = /^requiresAuthentication$/;
+const authenticatedRegex = /^authenticated$/;
 
-const pointerFieldsRegex = /^pointerFields$/;
+const requiresAuthenticationRegex = /^requiresAuthentication$/;
 
-const permissionKeyRegex = Object.freeze([
-  roleRegex,
-  pointerPermissionRegex,
+const clpPointerRegex = /^pointerFields$/;
+
+// regex for validating entities in protectedFields object
+const protectedFieldsRegex = Object.freeze([
+  protectedFieldsPointerRegex,
   publicRegex,
-  requireAuthenticationRegex,
-  pointerFieldsRegex,
+  authenticatedRegex,
+  roleRegex,
+]);
+
+// clp regex
+const clpFieldsRegex = Object.freeze([
+  clpPointerRegex,
+  publicRegex,
+  requiresAuthenticationRegex,
+  roleRegex,
 ]);
 
 function validatePermissionKey(key, userIdRegExp) {
   let matchesSome = false;
-  for (const regEx of permissionKeyRegex) {
+  for (const regEx of clpFieldsRegex) {
     if (key.match(regEx) !== null) {
       matchesSome = true;
       break;
     }
   }
 
+  // userId depends on startup options so it's dynamic
+  const valid = matchesSome || key.match(userIdRegExp) !== null;
+  if (!valid) {
+    throw new Parse.Error(
+      Parse.Error.INVALID_JSON,
+      `'${key}' is not a valid key for class level permissions`
+    );
+  }
+}
+
+function validateProtectedFieldsKey(key, userIdRegExp) {
+  let matchesSome = false;
+  for (const regEx of protectedFieldsRegex) {
+    if (key.match(regEx) !== null) {
+      matchesSome = true;
+      break;
+    }
+  }
+
+  // userId regex depends on launch options so it's dynamic
   const valid = matchesSome || key.match(userIdRegExp) !== null;
   if (!valid) {
     throw new Parse.Error(
@@ -264,7 +294,7 @@ function validateCLP(
     if (operationKey === 'protectedFields') {
       for (const entity in operation) {
         // throws on unexpected key
-        validatePermissionKey(entity, userIdRegExp);
+        validateProtectedFieldsKey(entity, userIdRegExp);
 
         const protectedFields = operation[entity];
 
@@ -301,6 +331,8 @@ function validateCLP(
       // throws on unexpected key
       validatePermissionKey(entity, userIdRegExp);
 
+      // entity can be either:
+      // "pointerFields": string[]
       if (entity === 'pointerFields') {
         const pointerFields = operation[entity];
 
@@ -311,13 +343,14 @@ function validateCLP(
         } else {
           throw new Parse.Error(
             Parse.Error.INVALID_JSON,
-            `'${pointerFields}' is not a valid value for protectedFields[${entity}] - expected an array.`
+            `'${pointerFields}' is not a valid value for ${operationKey}[${entity}] - expected an array.`
           );
         }
         // proceed with next entity key
         continue;
       }
 
+      // or [entity]: boolean
       const permit = operation[entity];
 
       if (permit !== true) {

src/Routers/ClassesRouter.js

@@ -6,6 +6,7 @@ import Parse from 'parse/node';
 const ALLOWED_GET_QUERY_KEYS = [
   'keys',
   'include',
+  'excludeKeys',
   'readPreference',
   'includeReadPreference',
   'subqueryReadPreference',
@@ -69,6 +70,9 @@ export class ClassesRouter extends PromiseRouter {
     if (body.include) {
       options.include = String(body.include);
     }
+    if (typeof body.excludeKeys == 'string') {
+      options.excludeKeys = body.excludeKeys;
+    }
     if (typeof body.readPreference === 'string') {
       options.readPreference = body.readPreference;
     }