diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ee55a1936..24206b79ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ### master [Full Changelog](https://github.com/parse-community/parse-server/compare/4.2.0...master) +- FIX: Optimize query decoration based on pointer CLPs by looking at the class schema to determine the field's type. - NEW (EXPERIMENTAL): Idempotency enforcement for client requests. This deduplicates requests where the client intends to send one request to Parse Server but due to network issues the server receives the request multiple times. **Caution, this is an experimental feature that may not be appropriate for production.** [#6744](https://github.com/parse-community/parse-server/issues/6744). Thanks to [Manuel Trezza](https://github.com/mtrezza). ### 4.2.0 diff --git a/spec/DatabaseController.spec.js b/spec/DatabaseController.spec.js index af373e64f2..e2df0f1e95 100644 --- a/spec/DatabaseController.spec.js +++ b/spec/DatabaseController.spec.js @@ -1,9 +1,9 @@ const DatabaseController = require('../lib/Controllers/DatabaseController.js'); const validateQuery = DatabaseController._validateQuery; -describe('DatabaseController', function() { - describe('validateQuery', function() { - it('should not restructure simple cases of SERVER-13732', done => { +describe('DatabaseController', function () { + describe('validateQuery', function () { + it('should not restructure simple cases of SERVER-13732', (done) => { const query = { $or: [{ a: 1 }, { a: 2 }], _rperm: { $in: ['a', 'b'] }, @@ -18,7 +18,7 @@ describe('DatabaseController', function() { done(); }); - it('should not restructure SERVER-13732 queries with $nears', done => { + it('should not restructure SERVER-13732 queries with $nears', (done) => { let query = { $or: [{ a: 1 }, { b: 1 }], c: { $nearSphere: {} } }; validateQuery(query); expect(query).toEqual({ @@ -31,7 +31,7 @@ describe('DatabaseController', function() { done(); }); - it('should not push refactored keys down a tree for SERVER-13732', done => { + it('should not push refactored keys down a tree for SERVER-13732', (done) => { const query = { a: 1, $or: [{ $or: [{ b: 1 }, { b: 2 }] }, { $or: [{ c: 1 }, { c: 2 }] }], @@ -45,14 +45,274 @@ describe('DatabaseController', function() { done(); }); - it('should reject invalid queries', done => { + it('should reject invalid queries', (done) => { expect(() => validateQuery({ $or: { a: 1 } })).toThrow(); done(); }); - it('should accept valid queries', done => { + it('should accept valid queries', (done) => { expect(() => validateQuery({ $or: [{ a: 1 }, { b: 2 }] })).not.toThrow(); done(); }); }); + + describe('addPointerPermissions', function () { + const CLASS_NAME = 'Foo'; + const USER_ID = 'userId'; + const ACL_GROUP = [USER_ID]; + const OPERATION = 'find'; + + const databaseController = new DatabaseController(); + const schemaController = jasmine.createSpyObj('SchemaController', [ + 'testPermissionsForClassName', + 'getClassLevelPermissions', + 'getExpectedType', + ]); + + it('should not decorate query if no pointer CLPs are present', (done) => { + const clp = buildCLP(); + const query = { a: 'b' }; + + schemaController.testPermissionsForClassName + .withArgs(CLASS_NAME, ACL_GROUP, OPERATION) + .and.returnValue(true); + schemaController.getClassLevelPermissions + .withArgs(CLASS_NAME) + .and.returnValue(clp); + + const output = databaseController.addPointerPermissions( + schemaController, + CLASS_NAME, + OPERATION, + query, + ACL_GROUP + ); + + expect(output).toEqual({ ...query }); + + done(); + }); + + it('should decorate query if a pointer CLP entry is present', (done) => { + const clp = buildCLP(['user']); + const query = { a: 'b' }; + + schemaController.testPermissionsForClassName + .withArgs(CLASS_NAME, ACL_GROUP, OPERATION) + .and.returnValue(false); + schemaController.getClassLevelPermissions + .withArgs(CLASS_NAME) + .and.returnValue(clp); + schemaController.getExpectedType + .withArgs(CLASS_NAME, 'user') + .and.returnValue({ type: 'Pointer' }); + + const output = databaseController.addPointerPermissions( + schemaController, + CLASS_NAME, + OPERATION, + query, + ACL_GROUP + ); + + expect(output).toEqual({ ...query, user: createUserPointer(USER_ID) }); + + done(); + }); + + it('should decorate query if an array CLP entry is present', (done) => { + const clp = buildCLP(['users']); + const query = { a: 'b' }; + + schemaController.testPermissionsForClassName + .withArgs(CLASS_NAME, ACL_GROUP, OPERATION) + .and.returnValue(false); + schemaController.getClassLevelPermissions + .withArgs(CLASS_NAME) + .and.returnValue(clp); + schemaController.getExpectedType + .withArgs(CLASS_NAME, 'users') + .and.returnValue({ type: 'Array' }); + + const output = databaseController.addPointerPermissions( + schemaController, + CLASS_NAME, + OPERATION, + query, + ACL_GROUP + ); + + expect(output).toEqual({ + ...query, + users: { $all: [createUserPointer(USER_ID)] }, + }); + + done(); + }); + + it('should decorate query if an object CLP entry is present', (done) => { + const clp = buildCLP(['user']); + const query = { a: 'b' }; + + schemaController.testPermissionsForClassName + .withArgs(CLASS_NAME, ACL_GROUP, OPERATION) + .and.returnValue(false); + schemaController.getClassLevelPermissions + .withArgs(CLASS_NAME) + .and.returnValue(clp); + schemaController.getExpectedType + .withArgs(CLASS_NAME, 'user') + .and.returnValue({ type: 'Object' }); + + const output = databaseController.addPointerPermissions( + schemaController, + CLASS_NAME, + OPERATION, + query, + ACL_GROUP + ); + + expect(output).toEqual({ + ...query, + user: createUserPointer(USER_ID), + }); + + done(); + }); + + it('should decorate query if a pointer CLP is present and the same field is part of the query', (done) => { + const clp = buildCLP(['user']); + const query = { a: 'b', user: 'a' }; + + schemaController.testPermissionsForClassName + .withArgs(CLASS_NAME, ACL_GROUP, OPERATION) + .and.returnValue(false); + schemaController.getClassLevelPermissions + .withArgs(CLASS_NAME) + .and.returnValue(clp); + schemaController.getExpectedType + .withArgs(CLASS_NAME, 'user') + .and.returnValue({ type: 'Pointer' }); + + const output = databaseController.addPointerPermissions( + schemaController, + CLASS_NAME, + OPERATION, + query, + ACL_GROUP + ); + + expect(output).toEqual({ + $and: [{ user: createUserPointer(USER_ID) }, { ...query }], + }); + + done(); + }); + + it('should transform the query to an $or query if multiple array/pointer CLPs are present', (done) => { + const clp = buildCLP(['user', 'users', 'userObject']); + const query = { a: 'b' }; + + schemaController.testPermissionsForClassName + .withArgs(CLASS_NAME, ACL_GROUP, OPERATION) + .and.returnValue(false); + schemaController.getClassLevelPermissions + .withArgs(CLASS_NAME) + .and.returnValue(clp); + schemaController.getExpectedType + .withArgs(CLASS_NAME, 'user') + .and.returnValue({ type: 'Pointer' }); + schemaController.getExpectedType + .withArgs(CLASS_NAME, 'users') + .and.returnValue({ type: 'Array' }); + schemaController.getExpectedType + .withArgs(CLASS_NAME, 'userObject') + .and.returnValue({ type: 'Object' }); + + const output = databaseController.addPointerPermissions( + schemaController, + CLASS_NAME, + OPERATION, + query, + ACL_GROUP + ); + + expect(output).toEqual({ + $or: [ + { ...query, user: createUserPointer(USER_ID) }, + { ...query, users: { $all: [createUserPointer(USER_ID)] } }, + { ...query, userObject: createUserPointer(USER_ID) }, + ], + }); + + done(); + }); + + it('should throw an error if for some unexpected reason the property specified in the CLP is neither a pointer nor an array', (done) => { + const clp = buildCLP(['user']); + const query = { a: 'b' }; + + schemaController.testPermissionsForClassName + .withArgs(CLASS_NAME, ACL_GROUP, OPERATION) + .and.returnValue(false); + schemaController.getClassLevelPermissions + .withArgs(CLASS_NAME) + .and.returnValue(clp); + schemaController.getExpectedType + .withArgs(CLASS_NAME, 'user') + .and.returnValue({ type: 'Number' }); + + expect(() => { + databaseController.addPointerPermissions( + schemaController, + CLASS_NAME, + OPERATION, + query, + ACL_GROUP + ); + }).toThrow( + Error( + `An unexpected condition occurred when resolving pointer permissions: ${CLASS_NAME} user` + ) + ); + + done(); + }); + }); }); + +function buildCLP(pointerNames) { + const OPERATIONS = [ + 'count', + 'find', + 'get', + 'create', + 'update', + 'delete', + 'addField', + ]; + + const clp = OPERATIONS.reduce((acc, op) => { + acc[op] = {}; + + if (pointerNames && pointerNames.length) { + acc[op].pointerFields = pointerNames; + } + + return acc; + }, {}); + + clp.protectedFields = {}; + clp.writeUserFields = []; + clp.readUserFields = []; + + return clp; +} + +function createUserPointer(userId) { + return { + __type: 'Pointer', + className: '_User', + objectId: userId, + }; +} diff --git a/src/Controllers/DatabaseController.js b/src/Controllers/DatabaseController.js index 657b40db8a..7db5b52cb3 100644 --- a/src/Controllers/DatabaseController.js +++ b/src/Controllers/DatabaseController.js @@ -1567,23 +1567,42 @@ class DatabaseController { objectId: userId, }; - const ors = permFields.flatMap(key => { - // constraint for single pointer setup - const q = { - [key]: userPointer, - }; - // constraint for users-array setup - const qa = { - [key]: { $all: [userPointer] }, - }; + const queries = permFields.map((key) => { + const fieldDescriptor = schema.getExpectedType(className, key); + const fieldType = + fieldDescriptor && + typeof fieldDescriptor === 'object' && + Object.prototype.hasOwnProperty.call(fieldDescriptor, 'type') + ? fieldDescriptor.type + : null; + + let queryClause; + + if (fieldType === 'Pointer') { + // constraint for single pointer setup + queryClause = { [key]: userPointer }; + } else if (fieldType === 'Array') { + // constraint for users-array setup + queryClause = { [key]: { $all: [userPointer] } }; + } else if (fieldType === 'Object') { + // constraint for object setup + queryClause = { [key]: userPointer }; + } else { + // This means that there is a CLP field of an unexpected type. This condition should not happen, which is + // why is being treated as an error. + throw Error( + `An unexpected condition occurred when resolving pointer permissions: ${className} ${key}` + ); + } // if we already have a constraint on the key, use the $and if (Object.prototype.hasOwnProperty.call(query, key)) { - return [{ $and: [q, query] }, { $and: [qa, query] }]; + return { $and: [queryClause, query] }; } // otherwise just add the constaint - return [Object.assign({}, query, q), Object.assign({}, query, qa)]; + return Object.assign({}, query, queryClause); }); - return { $or: ors }; + + return queries.length === 1 ? queries[0] : { $or: queries }; } else { return query; }