From 5b1cf9898dc8caf35215a0438acbe78d2b445d99 Mon Sep 17 00:00:00 2001 From: Aneesh Devasthale Date: Mon, 7 Mar 2016 18:09:59 +0530 Subject: [PATCH 01/22] Added test command for Windows support Use `npm run test:win` to run tests on Windows. Fixes #323. Variables like `$COVERAGE_OPTION` are not supported in Windows. --- package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package.json b/package.json index ee0e0b4e..a645baaf 100644 --- a/package.json +++ b/package.json @@ -59,6 +59,7 @@ "build": "./node_modules/.bin/babel src/ -d lib/", "pretest": "cross-env MONGODB_VERSION=${MONGODB_VERSION:=3.0.8} ./node_modules/.bin/mongodb-runner start", "test": "cross-env NODE_ENV=test TESTING=1 ./node_modules/.bin/babel-node $COVERAGE_OPTION ./node_modules/jasmine/bin/jasmine.js", + "test:win": "npm run pretest && cross-env NODE_ENV=test TESTING=1 ./node_modules/.bin/babel-node ./node_modules/babel-istanbul/lib/cli.js cover -x **/spec/** ./node_modules/jasmine/bin/jasmine.js && npm run posttest", "posttest": "./node_modules/.bin/mongodb-runner stop", "coverage": "cross-env COVERAGE_OPTION='./node_modules/babel-istanbul/lib/cli.js cover -x **/spec/**' npm test", "start": "node ./bin/parse-server", From d4fd73100c9895948f1192dd8cd6f070aa10eeff Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Mon, 7 Mar 2016 14:49:09 -0500 Subject: [PATCH 02/22] Adds CLP API to Schema router --- spec/schemas.spec.js | 66 ++++++++++++++++++++++++++++++++++++ src/Routers/SchemasRouter.js | 20 +++++++++++ src/Schema.js | 9 +++++ 3 files changed, 95 insertions(+) diff --git a/spec/schemas.spec.js b/spec/schemas.spec.js index 9a410eed..312bd070 100644 --- a/spec/schemas.spec.js +++ b/spec/schemas.spec.js @@ -872,4 +872,70 @@ describe('schemas', () => { }); }); }); + + it('should set/get schema permissions', done => { + + let object = new Parse.Object('AClass'); + object.save().then(() => { + request.put({ + url: 'http://localhost:8378/1/schemas/AClass/permissions', + headers: masterKeyHeaders, + json: true, + body: { + find: { + '*': true + }, + create: { + 'role:admin': true + } + } + }, (error, response, body) => { + expect(error).toEqual(null); + request.get({ + url: 'http://localhost:8378/1/schemas/AClass/permissions', + headers: masterKeyHeaders, + json: true, + }, (error, response, body) => { + expect(response.statusCode).toEqual(200); + expect(response.body).toEqual({ + find: { + '*': true + }, + create: { + 'role:admin': true + } + }); + done(); + }); + }); + }); + }); + + it('should fail setting schema permissions with invalid key', done => { + + let object = new Parse.Object('AClass'); + object.save().then(() => { + request.put({ + url: 'http://localhost:8378/1/schemas/AClass/permissions', + headers: masterKeyHeaders, + json: true, + body: { + find: { + '*': true + }, + create: { + 'role:admin': true + }, + dummy: { + 'some': true + } + } + }, (error, response, body) => { + expect(error).toEqual(null); + expect(body.code).toEqual(107); + expect(body.error).toEqual('dummy is not a valid operation for class level permissions'); + done(); + }); + }); + }); }); diff --git a/src/Routers/SchemasRouter.js b/src/Routers/SchemasRouter.js index a0a90ef2..4f8acbe6 100644 --- a/src/Routers/SchemasRouter.js +++ b/src/Routers/SchemasRouter.js @@ -106,6 +106,24 @@ function modifySchema(req) { }); } +function setSchemaPermissions(req) { + var className = req.params.className; + return req.config.database.loadSchema() + .then(schema => { + return schema.setPermissions(className, req.body); + }).then((res) => { + return Promise.resolve({response: {}}); + }); +} + +function getSchemaPermissions(req) { + var className = req.params.className; + return req.config.database.loadSchema() + .then(schema => { + return Promise.resolve({response: schema.perms[className]}); + }); +} + // A helper function that removes all join tables for a schema. Returns a promise. var removeJoinTables = (database, mongoSchema) => { return Promise.all(Object.keys(mongoSchema) @@ -171,6 +189,8 @@ export class SchemasRouter extends PromiseRouter { this.route('POST', '/schemas', middleware.promiseEnforceMasterKeyAccess, createSchema); this.route('POST', '/schemas/:className', middleware.promiseEnforceMasterKeyAccess, createSchema); this.route('PUT', '/schemas/:className', middleware.promiseEnforceMasterKeyAccess, modifySchema); + this.route('GET', '/schemas/:className/permissions', middleware.promiseEnforceMasterKeyAccess, getSchemaPermissions); + this.route('PUT', '/schemas/:className/permissions', middleware.promiseEnforceMasterKeyAccess, setSchemaPermissions); this.route('DELETE', '/schemas/:className', middleware.promiseEnforceMasterKeyAccess, deleteSchema); } } diff --git a/src/Schema.js b/src/Schema.js index 2a048a54..496c0b2e 100644 --- a/src/Schema.js +++ b/src/Schema.js @@ -76,6 +76,14 @@ var requiredColumns = { _Role: ["name", "ACL"] } +let CLPValidKeys = ['find', 'get', 'create', 'update', 'delete']; +function validateCLP(perms) { + Object.keys(perms).forEach((key) => { + if (CLPValidKeys.indexOf(key) == -1) { + throw new Parse.Error(Parse.Error.INVALID_JSON, `${key} is not a valid operation for class level permissions`); + } + }); +} // Valid classes must: // Be one of _User, _Installation, _Role, _Session OR // Be a join table OR @@ -288,6 +296,7 @@ class Schema { // Sets the Class-level permissions for a given className, which must exist. setPermissions(className, perms) { + validateCLP(perms); var update = { _metadata: { class_permissions: perms From 5780c1e4250e34b367b4fe2c8317ed348183b242 Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Mon, 7 Mar 2016 21:38:20 -0500 Subject: [PATCH 03/22] Merges CLP endpoints with POST, PUT and GET --- spec/schemas.spec.js | 66 +++++++++++++++++----------------- src/Routers/SchemasRouter.js | 60 +++---------------------------- src/Schema.js | 69 ++++++++++++++++++++++++++++++++++-- 3 files changed, 104 insertions(+), 91 deletions(-) diff --git a/spec/schemas.spec.js b/spec/schemas.spec.js index 312bd070..bb40dbc2 100644 --- a/spec/schemas.spec.js +++ b/spec/schemas.spec.js @@ -874,14 +874,12 @@ describe('schemas', () => { }); it('should set/get schema permissions', done => { - - let object = new Parse.Object('AClass'); - object.save().then(() => { - request.put({ - url: 'http://localhost:8378/1/schemas/AClass/permissions', - headers: masterKeyHeaders, - json: true, - body: { + request.post({ + url: 'http://localhost:8378/1/schemas/AClass', + headers: masterKeyHeaders, + json: true, + body: { + classLevelPermissions: { find: { '*': true }, @@ -889,24 +887,24 @@ describe('schemas', () => { 'role:admin': true } } + } + }, (error, response, body) => { + expect(error).toEqual(null); + request.get({ + url: 'http://localhost:8378/1/schemas/AClass', + headers: masterKeyHeaders, + json: true, }, (error, response, body) => { - expect(error).toEqual(null); - request.get({ - url: 'http://localhost:8378/1/schemas/AClass/permissions', - headers: masterKeyHeaders, - json: true, - }, (error, response, body) => { - expect(response.statusCode).toEqual(200); - expect(response.body).toEqual({ - find: { - '*': true - }, - create: { - 'role:admin': true - } - }); - done(); + expect(response.statusCode).toEqual(200); + expect(response.body.classLevelPermissions).toEqual({ + find: { + '*': true + }, + create: { + 'role:admin': true + } }); + done(); }); }); }); @@ -916,18 +914,20 @@ describe('schemas', () => { let object = new Parse.Object('AClass'); object.save().then(() => { request.put({ - url: 'http://localhost:8378/1/schemas/AClass/permissions', + url: 'http://localhost:8378/1/schemas/AClass', headers: masterKeyHeaders, json: true, body: { - find: { - '*': true - }, - create: { - 'role:admin': true - }, - dummy: { - 'some': true + classLevelPermissions: { + find: { + '*': true + }, + create: { + 'role:admin': true + }, + dummy: { + 'some': true + } } } }, (error, response, body) => { diff --git a/src/Routers/SchemasRouter.js b/src/Routers/SchemasRouter.js index 4f8acbe6..49e4bbb2 100644 --- a/src/Routers/SchemasRouter.js +++ b/src/Routers/SchemasRouter.js @@ -46,7 +46,7 @@ function createSchema(req) { } return req.config.database.loadSchema() - .then(schema => schema.addClassIfNotExists(className, req.body.fields)) + .then(schema => schema.addClassIfNotExists(className, req.body.fields, req.body.classLevelPermissions)) .then(result => ({ response: Schema.mongoSchemaToSchemaAPIResponse(result) })); } @@ -60,62 +60,12 @@ function modifySchema(req) { return req.config.database.loadSchema() .then(schema => { - if (!schema.data[className]) { - throw new Parse.Error(Parse.Error.INVALID_CLASS_NAME, `Class ${req.params.className} does not exist.`); - } - - let existingFields = Object.assign(schema.data[className], { _id: className }); - Object.keys(submittedFields).forEach(name => { - let field = submittedFields[name]; - if (existingFields[name] && field.__op !== 'Delete') { - throw new Parse.Error(255, `Field ${name} exists, cannot update.`); - } - if (!existingFields[name] && field.__op === 'Delete') { - throw new Parse.Error(255, `Field ${name} does not exist, cannot delete.`); - } - }); - - let newSchema = Schema.buildMergedSchemaObject(existingFields, submittedFields); - let mongoObject = Schema.mongoSchemaFromFieldsAndClassName(newSchema, className); - if (!mongoObject.result) { - throw new Parse.Error(mongoObject.code, mongoObject.error); - } - - // Finally we have checked to make sure the request is valid and we can start deleting fields. - // Do all deletions first, then add fields to avoid duplicate geopoint error. - let deletePromises = []; - let insertedFields = []; - Object.keys(submittedFields).forEach(fieldName => { - if (submittedFields[fieldName].__op === 'Delete') { - const promise = schema.deleteField(fieldName, className, req.config.database); - deletePromises.push(promise); - } else { - insertedFields.push(fieldName); - } - }); - return Promise.all(deletePromises) // Delete Everything - .then(() => schema.reloadData()) // Reload our Schema, so we have all the new values - .then(() => { - let promises = insertedFields.map(fieldName => { - const mongoType = mongoObject.result[fieldName]; - return schema.validateField(className, fieldName, mongoType); - }); - return Promise.all(promises); - }) - .then(() => ({ response: Schema.mongoSchemaToSchemaAPIResponse(mongoObject.result) })); + return schema.updateClass(className, submittedFields, req.body.classLevelPermissions, req.config.database); + }).then((result) => { + return Promise.resolve({response: result}); }); } -function setSchemaPermissions(req) { - var className = req.params.className; - return req.config.database.loadSchema() - .then(schema => { - return schema.setPermissions(className, req.body); - }).then((res) => { - return Promise.resolve({response: {}}); - }); -} - function getSchemaPermissions(req) { var className = req.params.className; return req.config.database.loadSchema() @@ -189,8 +139,6 @@ export class SchemasRouter extends PromiseRouter { this.route('POST', '/schemas', middleware.promiseEnforceMasterKeyAccess, createSchema); this.route('POST', '/schemas/:className', middleware.promiseEnforceMasterKeyAccess, createSchema); this.route('PUT', '/schemas/:className', middleware.promiseEnforceMasterKeyAccess, modifySchema); - this.route('GET', '/schemas/:className/permissions', middleware.promiseEnforceMasterKeyAccess, getSchemaPermissions); - this.route('PUT', '/schemas/:className/permissions', middleware.promiseEnforceMasterKeyAccess, setSchemaPermissions); this.route('DELETE', '/schemas/:className', middleware.promiseEnforceMasterKeyAccess, deleteSchema); } } diff --git a/src/Schema.js b/src/Schema.js index 496c0b2e..f1090c9c 100644 --- a/src/Schema.js +++ b/src/Schema.js @@ -78,6 +78,9 @@ var requiredColumns = { let CLPValidKeys = ['find', 'get', 'create', 'update', 'delete']; function validateCLP(perms) { + if (!perms) { + return; + } Object.keys(perms).forEach((key) => { if (CLPValidKeys.indexOf(key) == -1) { throw new Parse.Error(Parse.Error.INVALID_JSON, `${key} is not a valid operation for class level permissions`); @@ -229,15 +232,23 @@ class Schema { // on success, and rejects with an error on fail. Ensure you // have authorization (master key, or client class creation // enabled) before calling this function. - addClassIfNotExists(className, fields) { + addClassIfNotExists(className, fields, classLevelPermissions) { if (this.data[className]) { throw new Parse.Error(Parse.Error.INVALID_CLASS_NAME, `Class ${className} already exists.`); } + if (classLevelPermissions) { + validateCLP(classLevelPermissions); + } let mongoObject = mongoSchemaFromFieldsAndClassName(fields, className); if (!mongoObject.result) { return Promise.reject(mongoObject); } + + if (classLevelPermissions) { + mongoObject.result._metadata = mongoObject.result._metadata || {}; + mongoObject.result._metadata.class_permissions = classLevelPermissions; + } return this._collection.addSchema(className, mongoObject.result) .then(result => result.ops[0]) @@ -248,6 +259,56 @@ class Schema { return Promise.reject(error); }); } + + updateClass(className, submittedFields, classLevelPermissions, database) { + if (!this.data[className]) { + throw new Parse.Error(Parse.Error.INVALID_CLASS_NAME, `Class ${className} does not exist.`); + } + let existingFields = Object.assign(this.data[className], {_id: className}); + Object.keys(submittedFields).forEach(name => { + let field = submittedFields[name]; + if (existingFields[name] && field.__op !== 'Delete') { + throw new Parse.Error(255, `Field ${name} exists, cannot update.`); + } + if (!existingFields[name] && field.__op === 'Delete') { + throw new Parse.Error(255, `Field ${name} does not exist, cannot delete.`); + } + }); + + validateCLP(classLevelPermissions); + let newSchema = buildMergedSchemaObject(existingFields, submittedFields); + let mongoObject = mongoSchemaFromFieldsAndClassName(newSchema, className); + if (!mongoObject.result) { + throw new Parse.Error(mongoObject.code, mongoObject.error); + } + // set the class permissions + if (classLevelPermissions) { + mongoObject.result._metadata = mongoObject.result._metadata || {}; + mongoObject.result._metadata.class_permissions = classLevelPermissions; + } + // Finally we have checked to make sure the request is valid and we can start deleting fields. + // Do all deletions first, then a single save to _SCHEMA collection to handle all additions. + let deletePromises = []; + let insertedFields = []; + Object.keys(submittedFields).forEach(fieldName => { + if (submittedFields[fieldName].__op === 'Delete') { + const promise = this.deleteField(fieldName, className, database); + deletePromises.push(promise); + } else { + insertedFields.push(fieldName); + } + }); + return Promise.all(deletePromises) // Delete Everything + .then(() => this.reloadData()) // Reload our Schema, so we have all the new values + .then(() => { + let promises = insertedFields.map(fieldName => { + const mongoType = mongoObject.result[fieldName]; + return this.validateField(className, fieldName, mongoType); + }); + return Promise.all(promises); + }) + .then(() => { return mongoSchemaToSchemaAPIResponse(mongoObject.result) }); + } // Returns whether the schema knows the type of all these keys. @@ -785,10 +846,14 @@ function mongoSchemaAPIResponseFields(schema) { } function mongoSchemaToSchemaAPIResponse(schema) { - return { + let result = { className: schema._id, fields: mongoSchemaAPIResponseFields(schema), }; + if (schema._metadata && schema._metadata.class_permissions) { + result.classLevelPermissions = schema._metadata.class_permissions; + } + return result; } module.exports = { From 64f9fad285e52724e8953bbd37ea7e6a6ef44f60 Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Mon, 7 Mar 2016 22:27:11 -0500 Subject: [PATCH 04/22] Adds addField in CLP valid keys --- src/Schema.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Schema.js b/src/Schema.js index f1090c9c..c54b9008 100644 --- a/src/Schema.js +++ b/src/Schema.js @@ -76,7 +76,7 @@ var requiredColumns = { _Role: ["name", "ACL"] } -let CLPValidKeys = ['find', 'get', 'create', 'update', 'delete']; +let CLPValidKeys = ['find', 'get', 'create', 'update', 'delete', 'addField']; function validateCLP(perms) { if (!perms) { return; From e75d233b7eaeebd5ab29a114791ec30389c81747 Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Mon, 7 Mar 2016 23:07:24 -0500 Subject: [PATCH 05/22] Adds validation of addFields --- spec/schemas.spec.js | 58 +++++++++++++++++++++++++++ src/Controllers/DatabaseController.js | 24 ++++++++++- src/RestWrite.js | 2 +- 3 files changed, 81 insertions(+), 3 deletions(-) diff --git a/spec/schemas.spec.js b/spec/schemas.spec.js index bb40dbc2..f37cf1ca 100644 --- a/spec/schemas.spec.js +++ b/spec/schemas.spec.js @@ -938,4 +938,62 @@ describe('schemas', () => { }); }); }); + + it('should not be able to add a field', done => { + request.post({ + url: 'http://localhost:8378/1/schemas/AClass', + headers: masterKeyHeaders, + json: true, + body: { + classLevelPermissions: { + find: { + '*': true + }, + addField: { + 'role:admin': true + } + } + } + }, (error, response, body) => { + expect(error).toEqual(null); + let object = new Parse.Object('AClass'); + object.set('hello', 'world'); + return object.save().then(() => { + fail('should not be able to add a field'); + done(); + }, (err) => { + expect(err.message).toEqual('Permission denied for this action.'); + done(); + }) + }) + }); + + it('should not be able to add a field', done => { + request.post({ + url: 'http://localhost:8378/1/schemas/AClass', + headers: masterKeyHeaders, + json: true, + body: { + classLevelPermissions: { + find: { + '*': true + }, + addField: { + '*': true + } + } + } + }, (error, response, body) => { + expect(error).toEqual(null); + let object = new Parse.Object('AClass'); + object.set('hello', 'world'); + return object.save().then(() => { + done(); + }, (err) => { + fail('should be able to add a field'); + done(); + }) + }) + }); + }); diff --git a/src/Controllers/DatabaseController.js b/src/Controllers/DatabaseController.js index d5752088..3e85eca0 100644 --- a/src/Controllers/DatabaseController.js +++ b/src/Controllers/DatabaseController.js @@ -101,8 +101,12 @@ DatabaseController.prototype.redirectClassNameForKey = function(className, key) // Returns a promise that resolves to the new schema. // This does not update this.schema, because in a situation like a // batch request, that could confuse other users of the schema. -DatabaseController.prototype.validateObject = function(className, object, query) { - return this.loadSchema().then((schema) => { +DatabaseController.prototype.validateObject = function(className, object, query, options) { + let schema; + return this.loadSchema().then(s => { + schema = s; + return this.canAddField(schema, className, object, options.acl || []); + }).then(() => { return schema.validateObject(className, object, query); }); }; @@ -332,6 +336,22 @@ DatabaseController.prototype.create = function(className, object, options) { }); }; +DatabaseController.prototype.canAddField = function(schema, className, object, aclGroup) { + let classSchema = schema.data[className]; + if (!classSchema) { + return Promise.resolve(); + } + let fields = Object.keys(object); + let schemaFields = Object.keys(classSchema); + let newKeys = fields.filter((field) => { + return schemaFields.indexOf(field) < 0; + }) + if (newKeys.length > 0) { + return schema.validatePermission(className, aclGroup, 'addField'); + } + return Promise.resolve(); +} + // Runs a mongo query on the database. // This should only be used for testing - use 'find' for normal code // to avoid Mongo-format dependencies. diff --git a/src/RestWrite.js b/src/RestWrite.js index 9e07c93a..b68074d4 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -128,7 +128,7 @@ RestWrite.prototype.validateClientClassCreation = function() { // Validates this operation against the schema. RestWrite.prototype.validateSchema = function() { - return this.config.database.validateObject(this.className, this.data, this.query); + return this.config.database.validateObject(this.className, this.data, this.query, this.runOptions); }; // Runs any beforeSave triggers against this operation. From 78425c8a31c86c4165512f5ee5fe1351bf310d1c Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Wed, 9 Mar 2016 20:04:42 -0500 Subject: [PATCH 06/22] re-add shebang --- bin/parse-server | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bin/parse-server b/bin/parse-server index 94ffe964..298906d9 100755 --- a/bin/parse-server +++ b/bin/parse-server @@ -1 +1,3 @@ +#!/usr/bin/env node + require("../lib/cli/parse-server"); From ddd1ae3338e1158261e9e4618fce84f7da5d5934 Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Wed, 9 Mar 2016 19:58:50 -0500 Subject: [PATCH 07/22] Validates key, values and operation in CLP --- spec/schemas.spec.js | 126 +++++++++++++++++++++++++++++++++++++++++++ src/Schema.js | 32 +++++++++-- 2 files changed, 155 insertions(+), 3 deletions(-) diff --git a/spec/schemas.spec.js b/spec/schemas.spec.js index f37cf1ca..2778be21 100644 --- a/spec/schemas.spec.js +++ b/spec/schemas.spec.js @@ -996,4 +996,130 @@ describe('schemas', () => { }) }); + it('should throw with invalid userId (>10 chars)', done => { + request.post({ + url: 'http://localhost:8378/1/schemas/AClass', + headers: masterKeyHeaders, + json: true, + body: { + classLevelPermissions: { + find: { + '1234567890A': true + }, + } + } + }, (error, response, body) => { + expect(body.error).toEqual("'1234567890A' is not a valid key for class level permissions"); + done(); + }) + }); + + it('should throw with invalid userId (<10 chars)', done => { + request.post({ + url: 'http://localhost:8378/1/schemas/AClass', + headers: masterKeyHeaders, + json: true, + body: { + classLevelPermissions: { + find: { + 'a12345678': true + }, + } + } + }, (error, response, body) => { + expect(body.error).toEqual("'a12345678' is not a valid key for class level permissions"); + done(); + }) + }); + + it('should throw with invalid userId (invalid char)', done => { + request.post({ + url: 'http://localhost:8378/1/schemas/AClass', + headers: masterKeyHeaders, + json: true, + body: { + classLevelPermissions: { + find: { + '12345_6789': true + }, + } + } + }, (error, response, body) => { + expect(body.error).toEqual("'12345_6789' is not a valid key for class level permissions"); + done(); + }) + }); + + it('should throw with invalid * (spaces)', done => { + request.post({ + url: 'http://localhost:8378/1/schemas/AClass', + headers: masterKeyHeaders, + json: true, + body: { + classLevelPermissions: { + find: { + ' *': true + }, + } + } + }, (error, response, body) => { + expect(body.error).toEqual("' *' is not a valid key for class level permissions"); + done(); + }) + }); + + it('should throw with invalid * (spaces)', done => { + request.post({ + url: 'http://localhost:8378/1/schemas/AClass', + headers: masterKeyHeaders, + json: true, + body: { + classLevelPermissions: { + find: { + '* ': true + }, + } + } + }, (error, response, body) => { + expect(body.error).toEqual("'* ' is not a valid key for class level permissions"); + done(); + }) + }); + + it('should throw with invalid value', done => { + request.post({ + url: 'http://localhost:8378/1/schemas/AClass', + headers: masterKeyHeaders, + json: true, + body: { + classLevelPermissions: { + find: { + '*': 1 + }, + } + } + }, (error, response, body) => { + expect(body.error).toEqual("'1' is not a valid value for class level permissions find:*:1"); + done(); + }) + }); + + it('should throw with invalid value', done => { + request.post({ + url: 'http://localhost:8378/1/schemas/AClass', + headers: masterKeyHeaders, + json: true, + body: { + classLevelPermissions: { + find: { + '*': "" + }, + } + } + }, (error, response, body) => { + expect(body.error).toEqual("'' is not a valid value for class level permissions find:*:"); + done(); + }) + }); + }); diff --git a/src/Schema.js b/src/Schema.js index c54b9008..846a7490 100644 --- a/src/Schema.js +++ b/src/Schema.js @@ -76,15 +76,41 @@ var requiredColumns = { _Role: ["name", "ACL"] } +// 10 alpha numberic chars + uppercase +const userIdRegex = /^[a-zA-Z0-9]{10}$/; +// Anything that start with role +const roleRegex = /^role:.*/; +// * permission +const publicRegex = /^\*$/ + +const permissionKeyRegex = [userIdRegex, roleRegex, publicRegex]; + +function verifyPermissionKey(key) { + let result = permissionKeyRegex.reduce((isGood, regEx) => { + isGood = isGood || key.match(regEx) != null; + return isGood; + }, false); + if (!result) { + throw new Parse.Error(Parse.Error.INVALID_JSON, `'${key}' is not a valid key for class level permissions`); + } +} + let CLPValidKeys = ['find', 'get', 'create', 'update', 'delete', 'addField']; function validateCLP(perms) { if (!perms) { return; } - Object.keys(perms).forEach((key) => { - if (CLPValidKeys.indexOf(key) == -1) { - throw new Parse.Error(Parse.Error.INVALID_JSON, `${key} is not a valid operation for class level permissions`); + Object.keys(perms).forEach((operation) => { + if (CLPValidKeys.indexOf(operation) == -1) { + throw new Parse.Error(Parse.Error.INVALID_JSON, `${operation} is not a valid operation for class level permissions`); } + Object.keys(perms[operation]).forEach((key) => { + verifyPermissionKey(key); + let perm = perms[operation][key]; + if (perm !== true && perm !== false) { + throw new Parse.Error(Parse.Error.INVALID_JSON, `'${perm}' is not a valid value for class level permissions ${operation}:${key}:${perm}`); + } + }); }); } // Valid classes must: From b47c927377df769b70987bd3757abb46e5d3419a Mon Sep 17 00:00:00 2001 From: Aneesh Devasthale Date: Thu, 10 Mar 2016 11:17:52 +0530 Subject: [PATCH 08/22] Changed FileLoggerAdapterSpec to fail gracefully on Windows Running tests on Windows caused this error: ``` B:\Projects\Parse Server\parse-server\spec\FileLoggerAdapter.spec.js:38 expect(results[0].message).toEqual('testing info logs'); ^ TypeError: Cannot read property 'message' of undefined at B:\Projects\Parse Server\parse-server\spec\FileLoggerAdapter.spec.js:38:26 at ParsePromise. (B:\Projects\Parse Server\parse-server\src\Adapters\Logger\FileLoggerAdapter.js:9:17440) at ParsePromise.wrappedResolvedCallback (B:\Projects\Parse Server\parse-server\node_modules\parse\lib\node\ParsePromise.js:139:41) at ParsePromise.resolve (B:\Projects\Parse Server\parse-server\node_modules\parse\lib\node\ParsePromise.js:72:36) at resolveOne (B:\Projects\Parse Server\parse-server\node_modules\parse\lib\node\ParsePromise.js:471:29) at ParsePromise.object.then.errors.(anonymous function) (B:\Projects\Parse Server\parse-server\node_modules\parse\lib\node\ParsePromise.js:480:13) at ParsePromise.wrappedResolvedCallback (B:\Projects\Parse Server\parse-server\node_modules\parse\lib\node\ParsePromise.js:139:41) at ParsePromise.resolve (B:\Projects\Parse Server\parse-server\node_modules\parse\lib\node\ParsePromise.js:72:36) at ReadFileContext.callback (B:\Projects\Parse Server\parse-server\src\Adapters\Logger\FileLoggerAdapter.js:9:16189) at FSReqWrap.readFileAfterOpen [as oncomplete] (fs.js:303:13) ``` Rest of the tests could not be run as the test runner would break here. This change adds a check to fail when the FileLoggerAdapter returns an empty array from here: https://github.com/ParsePlatform/parse-server/blob/master/src/Adapters/Logger/FileLoggerAdapter.js#L191 Regarding the cause of the error itself, it is due to different filename separators in *nix and Windows. The FileLoggerAdapter would not save logs (have not tested this). This is a separate issue and should also be fixed. --- spec/FileLoggerAdapter.spec.js | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/spec/FileLoggerAdapter.spec.js b/spec/FileLoggerAdapter.spec.js index 4466e087..82c98f63 100644 --- a/spec/FileLoggerAdapter.spec.js +++ b/spec/FileLoggerAdapter.spec.js @@ -35,8 +35,13 @@ describe('info logs', () => { size: 1, level: 'info' }, (results) => { - expect(results[0].message).toEqual('testing info logs'); - done(); + if(results.length == 0) { + fail('The adapter should return non-empty results'); + done(); + } else { + expect(results[0].message).toEqual('testing info logs'); + done(); + } }); }); }); @@ -56,8 +61,14 @@ describe('error logs', () => { size: 1, level: 'error' }, (results) => { - expect(results[0].message).toEqual('testing error logs'); - done(); + if(results.length == 0) { + fail('The adapter should return non-empty results'); + done(); + } + else { + expect(results[0].message).toEqual('testing error logs'); + done(); + } }); }); }); From d71a58c217af3902e177cf7044084ba825735506 Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Wed, 9 Mar 2016 21:40:11 -0500 Subject: [PATCH 09/22] Adds tests, improve coverage, adds ability to delete CLP with classLevelPermissions: null --- spec/schemas.spec.js | 278 +++++++++++++++++++++++++++++++++++++++++++ src/Schema.js | 38 +++--- 2 files changed, 298 insertions(+), 18 deletions(-) diff --git a/spec/schemas.spec.js b/spec/schemas.spec.js index 2778be21..b249df8a 100644 --- a/spec/schemas.spec.js +++ b/spec/schemas.spec.js @@ -1122,4 +1122,282 @@ describe('schemas', () => { }) }); + function setPermissionsOnClass(className, permissions, doPut) { + let op = request.post; + if (doPut) + { + op = request.put; + } + return new Promise((resolve, reject) => { + op({ + url: 'http://localhost:8378/1/schemas/'+className, + headers: masterKeyHeaders, + json: true, + body: { + classLevelPermissions: permissions + } + }, (error, response, body) => { + if (error) { + return reject(error); + } + if (body.error) { + return reject(body); + } + return resolve(body); + }) + }); + } + + it('validate CLP 1', done => { + let user = new Parse.User(); + user.setUsername('user'); + user.setPassword('user'); + + let admin = new Parse.User(); + admin.setUsername('admin'); + admin.setPassword('admin'); + + let role = new Parse.Role('admin', new Parse.ACL()); + + setPermissionsOnClass('AClass', { + 'find': { + 'role:admin': true + } + }).then(() => { + return Parse.Object.saveAll([user, admin, role], {useMasterKey: true}); + }).then(()=> { + role.relation('users').add(admin); + return role.save(null, {useMasterKey: true}); + }).then(() => { + return Parse.User.logIn('user', 'user').then(() => { + let obj = new Parse.Object('AClass'); + return obj.save(); + }) + }).then(() => { + let query = new Parse.Query('AClass'); + return query.find().then((err) => { + expect(err.message).toEqual('Permission denied for this action.'); + fail('Use should hot be able to find!') + }, (err) => { + return Promise.resolve(); + }) + }).then(() => { + return Parse.User.logIn('admin', 'admin'); + }).then( () => { + let query = new Parse.Query('AClass'); + return query.find(); + }).then((results) => { + expect(results.length).toBe(1); + done(); + }, () => { + fail("should not fail!"); + done(); + }).catch( (err) => { + console.error(err); + done(); + }) + }); + + it('validate CLP 2', done => { + let user = new Parse.User(); + user.setUsername('user'); + user.setPassword('user'); + + let admin = new Parse.User(); + admin.setUsername('admin'); + admin.setPassword('admin'); + + let role = new Parse.Role('admin', new Parse.ACL()); + + setPermissionsOnClass('AClass', { + 'find': { + 'role:admin': true + } + }).then(() => { + return Parse.Object.saveAll([user, admin, role], {useMasterKey: true}); + }).then(()=> { + role.relation('users').add(admin); + return role.save(null, {useMasterKey: true}); + }).then(() => { + return Parse.User.logIn('user', 'user').then(() => { + let obj = new Parse.Object('AClass'); + return obj.save(); + }) + }).then(() => { + let query = new Parse.Query('AClass'); + return query.find().then((err) => { + expect(err.message).toEqual('Permission denied for this action.'); + fail('User should not be able to find!') + }, (err) => { + return Promise.resolve(); + }) + }).then(() => { + // let everyone see it now + return setPermissionsOnClass('AClass', { + 'find': { + 'role:admin': true, + '*': true + } + }, true); + }).then(() => { + let query = new Parse.Query('AClass'); + return query.find().then((result) => { + expect(result.length).toBe(1); + }, (err) => { + console.error(err); + expect(err.message).toEqual('Permission denied for this action.'); + fail('User should be able to find!') + done(); + }); + }).then(() => { + return Parse.User.logIn('admin', 'admin'); + }).then( () => { + let query = new Parse.Query('AClass'); + return query.find(); + }).then((results) => { + expect(results.length).toBe(1); + done(); + }, (err) => { + console.error(err); + fail("should not fail!"); + done(); + }).catch( (err) => { + console.error(err); + done(); + }) + }); + + it('validate CLP 3', done => { + let user = new Parse.User(); + user.setUsername('user'); + user.setPassword('user'); + + let admin = new Parse.User(); + admin.setUsername('admin'); + admin.setPassword('admin'); + + let role = new Parse.Role('admin', new Parse.ACL()); + + setPermissionsOnClass('AClass', { + 'find': { + 'role:admin': true + } + }).then(() => { + return Parse.Object.saveAll([user, admin, role], {useMasterKey: true}); + }).then(()=> { + role.relation('users').add(admin); + return role.save(null, {useMasterKey: true}); + }).then(() => { + return Parse.User.logIn('user', 'user').then(() => { + let obj = new Parse.Object('AClass'); + return obj.save(); + }) + }).then(() => { + let query = new Parse.Query('AClass'); + return query.find().then((err) => { + expect(err.message).toEqual('Permission denied for this action.'); + fail('User should not be able to find!') + }, (err) => { + return Promise.resolve(); + }) + }).then(() => { + // delete all CLP + return setPermissionsOnClass('AClass', null, true); + }).then(() => { + let query = new Parse.Query('AClass'); + return query.find().then((result) => { + expect(result.length).toBe(1); + }, (err) => { + console.error(err); + fail('User should be able to find!') + done(); + }); + }).then(() => { + return Parse.User.logIn('admin', 'admin'); + }).then( () => { + let query = new Parse.Query('AClass'); + return query.find(); + }).then((results) => { + expect(results.length).toBe(1); + done(); + }, (err) => { + console.error(err); + fail("should not fail!"); + done(); + }).catch( (err) => { + console.error(err); + done(); + }) + }); + + it('validate CLP 4', done => { + let user = new Parse.User(); + user.setUsername('user'); + user.setPassword('user'); + + let admin = new Parse.User(); + admin.setUsername('admin'); + admin.setPassword('admin'); + + let role = new Parse.Role('admin', new Parse.ACL()); + + setPermissionsOnClass('AClass', { + 'find': { + 'role:admin': true + } + }).then(() => { + return Parse.Object.saveAll([user, admin, role], {useMasterKey: true}); + }).then(()=> { + role.relation('users').add(admin); + return role.save(null, {useMasterKey: true}); + }).then(() => { + return Parse.User.logIn('user', 'user').then(() => { + let obj = new Parse.Object('AClass'); + return obj.save(); + }) + }).then(() => { + let query = new Parse.Query('AClass'); + return query.find().then((err) => { + expect(err.message).toEqual('Permission denied for this action.'); + fail('User should not be able to find!') + }, (err) => { + return Promise.resolve(); + }) + }).then(() => { + // borked CLP should not affec security + return setPermissionsOnClass('AClass', { + 'found': { + 'role:admin': true + } + }, true).then(() => { + fail("Should not be able to save a borked CLP"); + }, () => { + return Promise.resolve(); + }) + }).then(() => { + let query = new Parse.Query('AClass'); + return query.find().then((result) => { + fail('User should not be able to find!') + }, (err) => { + expect(err.message).toEqual('Permission denied for this action.'); + return Promise.resolve(); + }); + }).then(() => { + return Parse.User.logIn('admin', 'admin'); + }).then( () => { + let query = new Parse.Query('AClass'); + return query.find(); + }).then((results) => { + expect(results.length).toBe(1); + done(); + }, (err) => { + console.error(err); + fail("should not fail!"); + done(); + }).catch( (err) => { + console.error(err); + done(); + }) + }); + }); diff --git a/src/Schema.js b/src/Schema.js index 846a7490..b48bbfd9 100644 --- a/src/Schema.js +++ b/src/Schema.js @@ -262,19 +262,11 @@ class Schema { if (this.data[className]) { throw new Parse.Error(Parse.Error.INVALID_CLASS_NAME, `Class ${className} already exists.`); } - if (classLevelPermissions) { - validateCLP(classLevelPermissions); - } - let mongoObject = mongoSchemaFromFieldsAndClassName(fields, className); + let mongoObject = mongoSchemaFromFieldsAndClassNameAndCLP(fields, className, classLevelPermissions); if (!mongoObject.result) { return Promise.reject(mongoObject); } - - if (classLevelPermissions) { - mongoObject.result._metadata = mongoObject.result._metadata || {}; - mongoObject.result._metadata.class_permissions = classLevelPermissions; - } return this._collection.addSchema(className, mongoObject.result) .then(result => result.ops[0]) @@ -301,17 +293,12 @@ class Schema { } }); - validateCLP(classLevelPermissions); let newSchema = buildMergedSchemaObject(existingFields, submittedFields); - let mongoObject = mongoSchemaFromFieldsAndClassName(newSchema, className); + let mongoObject = mongoSchemaFromFieldsAndClassNameAndCLP(newSchema, className, classLevelPermissions); if (!mongoObject.result) { throw new Parse.Error(mongoObject.code, mongoObject.error); } - // set the class permissions - if (classLevelPermissions) { - mongoObject.result._metadata = mongoObject.result._metadata || {}; - mongoObject.result._metadata.class_permissions = classLevelPermissions; - } + // Finally we have checked to make sure the request is valid and we can start deleting fields. // Do all deletions first, then a single save to _SCHEMA collection to handle all additions. let deletePromises = []; @@ -333,6 +320,9 @@ class Schema { }); return Promise.all(promises); }) + .then(() => { + return this.setPermissions(className, classLevelPermissions) + }) .then(() => { return mongoSchemaToSchemaAPIResponse(mongoObject.result) }); } @@ -383,6 +373,9 @@ class Schema { // Sets the Class-level permissions for a given className, which must exist. setPermissions(className, perms) { + if (typeof perms === 'undefined') { + return Promise.resolve(); + } validateCLP(perms); var update = { _metadata: { @@ -644,7 +637,7 @@ function load(collection) { // Returns { code, error } if invalid, or { result }, an object // suitable for inserting into _SCHEMA collection, otherwise -function mongoSchemaFromFieldsAndClassName(fields, className) { +function mongoSchemaFromFieldsAndClassNameAndCLP(fields, className, classLevelPermissions) { if (!classNameIsValid(className)) { return { code: Parse.Error.INVALID_CLASS_NAME, @@ -697,6 +690,16 @@ function mongoSchemaFromFieldsAndClassName(fields, className) { error: 'currently, only one GeoPoint field may exist in an object. Adding ' + geoPoints[1] + ' when ' + geoPoints[0] + ' already exists.', }; } + + validateCLP(classLevelPermissions); + if (typeof classLevelPermissions !== 'undefined') { + mongoObject._metadata = mongoObject._metadata || {}; + if (!classLevelPermissions) { + delete mongoObject._metadata.class_permissions; + } else { + mongoObject._metadata.class_permissions = classLevelPermissions; + } + } return { result: mongoObject }; } @@ -886,7 +889,6 @@ module.exports = { load: load, classNameIsValid: classNameIsValid, invalidClassNameMessage: invalidClassNameMessage, - mongoSchemaFromFieldsAndClassName: mongoSchemaFromFieldsAndClassName, schemaAPITypeToMongoFieldType: schemaAPITypeToMongoFieldType, buildMergedSchemaObject: buildMergedSchemaObject, mongoFieldTypeToSchemaAPIType: mongoFieldTypeToSchemaAPIType, From dbf2afc5eac414466ee01755a4c0e318b4bae8b9 Mon Sep 17 00:00:00 2001 From: steven-supersolid Date: Thu, 10 Mar 2016 16:49:45 +0000 Subject: [PATCH 10/22] Add database options to ParseServer constructor and pass to MongoStorageAdapter --- src/Adapters/Storage/Mongo/MongoStorageAdapter.js | 6 ++++-- src/DatabaseAdapter.js | 8 +++++++- src/index.js | 5 +++++ 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js index e3d59493..d3d2bc7e 100644 --- a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js +++ b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js @@ -10,12 +10,14 @@ const MongoSchemaCollectionName = '_SCHEMA'; export class MongoStorageAdapter { // Private _uri: string; + _options: Object; // Public connectionPromise; database; - constructor(uri: string) { + constructor(uri: string, options: Object) { this._uri = uri; + this._options = options; } connect() { @@ -23,7 +25,7 @@ export class MongoStorageAdapter { return this.connectionPromise; } - this.connectionPromise = MongoClient.connect(this._uri).then(database => { + this.connectionPromise = MongoClient.connect(this._uri, this._options).then(database => { this.database = database; }); return this.connectionPromise; diff --git a/src/DatabaseAdapter.js b/src/DatabaseAdapter.js index 6663f36b..dbc7c6ac 100644 --- a/src/DatabaseAdapter.js +++ b/src/DatabaseAdapter.js @@ -24,6 +24,7 @@ let adapter = MongoStorageAdapter; let dbConnections = {}; let databaseURI = DefaultDatabaseURI; let appDatabaseURIs = {}; +let appDatabaseOptions = {}; function setAdapter(databaseAdapter) { adapter = databaseAdapter; @@ -37,6 +38,10 @@ function setAppDatabaseURI(appId, uri) { appDatabaseURIs[appId] = uri; } +function setAppDatabaseOptions(appId: string, options: Object) { + appDatabaseOptions[appId] = options; +} + //Used by tests function clearDatabaseURIs() { appDatabaseURIs = {}; @@ -50,7 +55,7 @@ function getDatabaseConnection(appId: string, collectionPrefix: string) { var dbURI = (appDatabaseURIs[appId] ? appDatabaseURIs[appId] : databaseURI); - let storageAdapter = new adapter(dbURI); + let storageAdapter = new adapter(dbURI, appDatabaseOptions[appId]); dbConnections[appId] = new DatabaseController(storageAdapter, { collectionPrefix: collectionPrefix }); @@ -62,6 +67,7 @@ module.exports = { getDatabaseConnection: getDatabaseConnection, setAdapter: setAdapter, setDatabaseURI: setDatabaseURI, + setAppDatabaseOptions: setAppDatabaseOptions, setAppDatabaseURI: setAppDatabaseURI, clearDatabaseURIs: clearDatabaseURIs, defaultDatabaseURI: databaseURI diff --git a/src/index.js b/src/index.js index 131f1f69..2e4d553f 100644 --- a/src/index.js +++ b/src/index.js @@ -84,6 +84,7 @@ function ParseServer({ push, loggerAdapter, databaseURI = DatabaseAdapter.defaultDatabaseURI, + databaseOptions, cloud, collectionPrefix = '', clientKey, @@ -120,6 +121,10 @@ function ParseServer({ DatabaseAdapter.setAppDatabaseURI(appId, databaseURI); } + if (databaseOptions) { + DatabaseAdapter.setAppDatabaseOptions(appId, databaseOptions); + } + if (cloud) { addParseCloud(); if (typeof cloud === 'function') { From 76e6f8c775caee5f77b2606f9fb493161b6ea206 Mon Sep 17 00:00:00 2001 From: Raul Rodriguez Date: Thu, 10 Mar 2016 20:05:28 +0100 Subject: [PATCH 11/22] Shutdown standalone parse server gracefully --- src/cli/parse-server.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/cli/parse-server.js b/src/cli/parse-server.js index c5945b6c..b4cbbb12 100755 --- a/src/cli/parse-server.js +++ b/src/cli/parse-server.js @@ -65,7 +65,7 @@ const app = express(); const api = new ParseServer(options); app.use(options.mountPath, api); -app.listen(options.port, function() { +var server = app.listen(options.port, function() { for (let key in options) { let value = options[key]; @@ -77,3 +77,12 @@ app.listen(options.port, function() { console.log(''); console.log('parse-server running on '+options.serverURL); }); + +var handleShutdown = function() { + console.log('Termination signal received. Shutting down.'); + server.close(function () { + process.exit(0); + }); +}; +process.on('SIGTERM', handleShutdown); +process.on('SIGINT', handleShutdown); From b1d399bf8013c42dec0043d70963b874fd4ec0be Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Thu, 10 Mar 2016 18:02:29 -0500 Subject: [PATCH 12/22] Adds blacklist permission, more test scenarios --- spec/schemas.spec.js | 161 ++++++++++++++++++++++++++++++++++++++----- src/Schema.js | 25 ++++--- 2 files changed, 159 insertions(+), 27 deletions(-) diff --git a/spec/schemas.spec.js b/spec/schemas.spec.js index b249df8a..1350c74d 100644 --- a/spec/schemas.spec.js +++ b/spec/schemas.spec.js @@ -1176,9 +1176,9 @@ describe('schemas', () => { }).then(() => { let query = new Parse.Query('AClass'); return query.find().then((err) => { - expect(err.message).toEqual('Permission denied for this action.'); fail('Use should hot be able to find!') }, (err) => { + expect(err.message).toEqual('Permission denied for this action.'); return Promise.resolve(); }) }).then(() => { @@ -1193,7 +1193,6 @@ describe('schemas', () => { fail("should not fail!"); done(); }).catch( (err) => { - console.error(err); done(); }) }); @@ -1226,9 +1225,9 @@ describe('schemas', () => { }).then(() => { let query = new Parse.Query('AClass'); return query.find().then((err) => { - expect(err.message).toEqual('Permission denied for this action.'); fail('User should not be able to find!') }, (err) => { + expect(err.message).toEqual('Permission denied for this action.'); return Promise.resolve(); }) }).then(() => { @@ -1244,8 +1243,6 @@ describe('schemas', () => { return query.find().then((result) => { expect(result.length).toBe(1); }, (err) => { - console.error(err); - expect(err.message).toEqual('Permission denied for this action.'); fail('User should be able to find!') done(); }); @@ -1258,11 +1255,9 @@ describe('schemas', () => { expect(results.length).toBe(1); done(); }, (err) => { - console.error(err); fail("should not fail!"); done(); }).catch( (err) => { - console.error(err); done(); }) }); @@ -1295,9 +1290,9 @@ describe('schemas', () => { }).then(() => { let query = new Parse.Query('AClass'); return query.find().then((err) => { - expect(err.message).toEqual('Permission denied for this action.'); fail('User should not be able to find!') }, (err) => { + expect(err.message).toEqual('Permission denied for this action.'); return Promise.resolve(); }) }).then(() => { @@ -1308,7 +1303,6 @@ describe('schemas', () => { return query.find().then((result) => { expect(result.length).toBe(1); }, (err) => { - console.error(err); fail('User should be able to find!') done(); }); @@ -1321,13 +1315,9 @@ describe('schemas', () => { expect(results.length).toBe(1); done(); }, (err) => { - console.error(err); fail("should not fail!"); done(); - }).catch( (err) => { - console.error(err); - done(); - }) + }); }); it('validate CLP 4', done => { @@ -1358,9 +1348,9 @@ describe('schemas', () => { }).then(() => { let query = new Parse.Query('AClass'); return query.find().then((err) => { - expect(err.message).toEqual('Permission denied for this action.'); fail('User should not be able to find!') }, (err) => { + expect(err.message).toEqual('Permission denied for this action.'); return Promise.resolve(); }) }).then(() => { @@ -1391,13 +1381,150 @@ describe('schemas', () => { expect(results.length).toBe(1); done(); }, (err) => { - console.error(err); fail("should not fail!"); done(); }).catch( (err) => { - console.error(err); done(); }) }); + it('validate CLP 5', done => { + let user = new Parse.User(); + user.setUsername('user'); + user.setPassword('user'); + + let user2 = new Parse.User(); + user2.setUsername('user2'); + user2.setPassword('user2'); + let admin = new Parse.User(); + admin.setUsername('admin'); + admin.setPassword('admin'); + + let role = new Parse.Role('admin', new Parse.ACL()); + + Promise.resolve().then(() => { + return Parse.Object.saveAll([user, user2, admin, role], {useMasterKey: true}); + }).then(()=> { + role.relation('users').add(admin); + return role.save(null, {useMasterKey: true}).then(() => { + let perm = { + 'find': { + // Admins can't read + 'role:admin': false + } + }; + // let the user find + perm['find'][user.id] = true; + return setPermissionsOnClass('AClass', perm); + }) + }).then(() => { + return Parse.User.logIn('user', 'user').then(() => { + let obj = new Parse.Object('AClass'); + return obj.save(); + }) + }).then(() => { + let query = new Parse.Query('AClass'); + return query.find().then((res) => { + expect(res.length).toEqual(1); + }, (err) => { + fail('User should be able to find!') + return Promise.resolve(); + }) + }).then(() => { + return Parse.User.logIn('admin', 'admin'); + }).then( () => { + let query = new Parse.Query('AClass'); + return query.find(); + }).then((results) => { + fail("should not be able to read!"); + return Promise.resolve(); + }, (err) => { + expect(err.message).toEqual('Permission denied for this action.'); + return Promise.resolve(); + }).then(() => { + return Parse.User.logIn('user2', 'user2'); + }).then( () => { + let query = new Parse.Query('AClass'); + return query.find(); + }).then((results) => { + fail("should not be able to read!"); + return Promise.resolve(); + }, (err) => { + expect(err.message).toEqual('Permission denied for this action.'); + return Promise.resolve(); + }).then(() => { + done(); + }); + }); + + it('validate CLP 6', done => { + let user = new Parse.User(); + user.setUsername('user'); + user.setPassword('user'); + + let user2 = new Parse.User(); + user2.setUsername('user2'); + user2.setPassword('user2'); + let admin = new Parse.User(); + admin.setUsername('admin'); + admin.setPassword('admin'); + + let role = new Parse.Role('admin', new Parse.ACL()); + + Promise.resolve().then(() => { + return Parse.Object.saveAll([user, user2, admin, role], {useMasterKey: true}); + }).then(()=> { + role.relation('users').add(admin); + return role.save(null, {useMasterKey: true}).then(() => { + let perm = { + 'find': { + // Anyone can find + '*': true + } + }; + // but the user can't + perm['find'][user.id] = false; + return setPermissionsOnClass('AClass', perm); + }) + }).then(() => { + return Parse.User.logIn('user', 'user').then(() => { + let obj = new Parse.Object('AClass'); + return obj.save(); + }) + }).then(() => { + let query = new Parse.Query('AClass'); + return query.find().then((res) => { + fail('User should not be able to find!') + return Promise.resolve(); + }, (err) => { + expect(err.message).toEqual('Permission denied for this action.'); + return Promise.resolve(); + }) + }).then(() => { + return Parse.User.logIn('admin', 'admin'); + }).then( () => { + let query = new Parse.Query('AClass'); + return query.find(); + }).then((results) => { + expect(results.length).toEqual(1); + return Promise.resolve(); + }, (err) => { + fail('Should find the object as admin'); + return Promise.resolve(); + }).then(() => { + return Parse.User.logIn('user2', 'user2'); + }).then( () => { + let query = new Parse.Query('AClass'); + return query.find(); + }).then((results) => { + expect(results.length).toEqual(1); + return Promise.resolve(); + }, (err) => { + fail('Should find the object as user2'); + return Promise.resolve(); + }).then(() => { + done(); + }); + }); + }); diff --git a/src/Schema.js b/src/Schema.js index b48bbfd9..9b18517a 100644 --- a/src/Schema.js +++ b/src/Schema.js @@ -585,17 +585,22 @@ class Schema { return Promise.resolve(); } var perms = this.perms[className][operation]; - // Handle the public scenario quickly - if (perms['*']) { - return Promise.resolve(); - } - // Check permissions against the aclGroup provided (array of userId/roles) - var found = false; - for (var i = 0; i < aclGroup.length && !found; i++) { - if (perms[aclGroup[i]]) { - found = true; + + // Check permissions against the aclGroup provided (array of userId/roles) + // if perms has a public, check the blacklist + let startfound = perms['*'] ? true : undefined; + let found = aclGroup.reduce((memo, acl) => { + let perm = perms[acl]; + // We have a black listed permission + if (perm === false) { + return false; } - } + // the memo is not blacklisted + if (perm === true && memo !== false) { + return true; + } + return memo; + }, startfound); if (!found) { // TODO: Verify correct error code throw new Parse.Error(Parse.Error.OBJECT_NOT_FOUND, From 9aaaf78a3615cc75500bb8b17769724e3ef4af32 Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Wed, 9 Mar 2016 23:17:40 -0500 Subject: [PATCH 13/22] Anonymous is an OAuth --- src/RestWrite.js | 127 ++++++++++++++------------------------------- src/oauth/index.js | 10 +++- 2 files changed, 49 insertions(+), 88 deletions(-) diff --git a/src/RestWrite.js b/src/RestWrite.js index 9e07c93a..f54d6a73 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -211,17 +211,15 @@ RestWrite.prototype.validateAuthData = function() { } var authData = this.data.authData; - var anonData = this.data.authData.anonymous; - - if (this.config.enableAnonymousUsers === true && (anonData === null || - (anonData && anonData.id))) { - return this.handleAnonymousAuthData(); - } - - // Not anon, try other providers var providers = Object.keys(authData); - if (!anonData && providers.length == 1) { + if (providers.length == 1) { + var provider = providers[0]; + if (provider == 'anonymous' && !this.config.enableAnonymousUsers) { + throw new Parse.Error(Parse.Error.UNSUPPORTED_SERVICE, + 'This authentication method is unsupported.'); + } + var providerAuthData = authData[provider]; var hasToken = (providerAuthData && providerAuthData.id); if (providerAuthData === null || hasToken) { @@ -232,55 +230,8 @@ RestWrite.prototype.validateAuthData = function() { 'This authentication method is unsupported.'); }; -RestWrite.prototype.handleAnonymousAuthData = function() { - var anonData = this.data.authData.anonymous; - if (anonData === null && this.query) { - // We are unlinking the user from the anonymous provider - this.data._auth_data_anonymous = null; - return; - } - - // Check if this user already exists - return this.config.database.find( - this.className, - {'authData.anonymous.id': anonData.id}, {}) - .then((results) => { - if (results.length > 0) { - if (!this.query) { - // We're signing up, but this user already exists. Short-circuit - delete results[0].password; - this.response = { - response: results[0], - location: this.location() - }; - return; - } - - // If this is a PUT for the same user, allow the linking - if (results[0].objectId === this.query.objectId) { - // Delete the rest format key before saving - delete this.data.authData; - return; - } - - // We're trying to create a duplicate account. Forbid it - throw new Parse.Error(Parse.Error.ACCOUNT_ALREADY_LINKED, - 'this auth is already used'); - } - - // This anonymous user does not already exist, so transform it - // to a saveable format - this.data._auth_data_anonymous = anonData; - - // Delete the rest format key before saving - delete this.data.authData; - }) - -}; - RestWrite.prototype.handleOAuthAuthData = function(provider) { var authData = this.data.authData[provider]; - if (authData === null && this.query) { // We are unlinking from the provider. this.data["_auth_data_" + provider ] = null; @@ -298,7 +249,6 @@ RestWrite.prototype.handleOAuthAuthData = function(provider) { var validateAuthData; var validateAppId; - if (oauth[provider]) { validateAuthData = oauth[provider].validateAuthData; validateAppId = oauth[provider].validateAppId; @@ -343,37 +293,36 @@ RestWrite.prototype.handleOAuthAuthData = function(provider) { query, {}); }).then((results) => { this.storage['authProvider'] = provider; - if (results.length > 0) { - if (!this.query) { - // We're signing up, but this user already exists. Short-circuit - delete results[0].password; - this.response = { - response: results[0], - location: this.location() - }; - this.data.objectId = results[0].objectId; - return; - } - - // If this is a PUT for the same user, allow the linking - if (results[0].objectId === this.query.objectId) { - // Delete the rest format key before saving - delete this.data.authData; - return; - } - // We're trying to create a duplicate oauth auth. Forbid it - throw new Parse.Error(Parse.Error.ACCOUNT_ALREADY_LINKED, - 'this auth is already used'); - } else { - this.data.username = cryptoUtils.newToken(); - } - - // This FB auth does not already exist, so transform it to a - // saveable format + + // Put the data in the proper format this.data["_auth_data_" + provider ] = authData; - - // Delete the rest format key before saving - delete this.data.authData; + + if (results.length == 0) { + // this a new user + this.data.username = cryptoUtils.newToken(); + } else if (!this.query) { + // Login with auth data + // Short circuit + delete results[0].password; + this.response = { + response: results[0], + location: this.location() + }; + this.data.objectId = results[0].objectId; + } else if (this.query && this.query.objectId) { + // Trying to update auth data but users + // are different + if (results[0].objectId !== this.query.objectId) { + delete this.data["_auth_data_" + provider ]; + console.log("alerady linked!"); + throw new Parse.Error(Parse.Error.ACCOUNT_ALREADY_LINKED, + 'this auth is already used'); + } + } else { + + delete this.data["_auth_data_" + provider ]; + throw new Parse.Error(Parse.Error.INTERNAL_SERVER_ERROR, 'THis should not be reached...'); + } }); } @@ -780,6 +729,10 @@ RestWrite.prototype.runDatabaseOperation = function() { if (this.data.ACL && this.data.ACL['*unresolved']) { throw new Parse.Error(Parse.Error.INVALID_ACL, 'Invalid ACL.'); } + + if (this.className === '_User') { + delete this.data.authData; + } if (this.query) { // Run an update diff --git a/src/oauth/index.js b/src/oauth/index.js index f39aea07..5067b4a1 100644 --- a/src/oauth/index.js +++ b/src/oauth/index.js @@ -13,5 +13,13 @@ module.exports = { instagram: instagram, linkedin: linkedin, meetup: meetup, - twitter: twitter + twitter: twitter, + anonymous: { + validateAuthData: function() { + return Promise.resolve(); + }, + validateAppId: function() { + return Promise.resolve(); + } + } } \ No newline at end of file From 54d154f7aa692da2dc07969774b90d061872a4a8 Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Thu, 10 Mar 2016 09:41:56 -0500 Subject: [PATCH 14/22] Centralizes AuthData validation --- spec/RestCreate.spec.js | 3 +- spec/helper.js | 3 +- src/Config.js | 1 - src/RestWrite.js | 57 +++------------------ src/index.js | 6 +-- src/oauth/index.js | 111 ++++++++++++++++++++++++++++++++-------- 6 files changed, 103 insertions(+), 78 deletions(-) diff --git a/spec/RestCreate.spec.js b/spec/RestCreate.spec.js index f9b94b37..f593e221 100644 --- a/spec/RestCreate.spec.js +++ b/spec/RestCreate.spec.js @@ -148,7 +148,8 @@ describe('rest create', () => { }); it('handles no anonymous users config', (done) => { - var NoAnnonConfig = Object.assign({}, config, {enableAnonymousUsers: false}); + var NoAnnonConfig = Object.assign({}, config); + NoAnnonConfig.oauth.setEnableAnonymousUsers(false); var data1 = { authData: { anonymous: { diff --git a/spec/helper.js b/spec/helper.js index e2daa6ed..8ed9fe26 100644 --- a/spec/helper.js +++ b/spec/helper.js @@ -7,6 +7,7 @@ var DatabaseAdapter = require('../src/DatabaseAdapter'); var express = require('express'); var facebook = require('../src/oauth/facebook'); var ParseServer = require('../src/index').ParseServer; +var path = require('path'); var databaseURI = process.env.DATABASE_URI; var cloudMain = process.env.CLOUD_CODE_MAIN || '../spec/cloud/main.js'; @@ -36,7 +37,7 @@ var defaultConfiguration = { oauth: { // Override the facebook provider facebook: mockFacebook(), myoauth: { - module: "../spec/myoauth" // relative path as it's run from src + module: path.resolve(__dirname, "myoauth") // relative path as it's run from src } } }; diff --git a/src/Config.js b/src/Config.js index 8042d6db..f65b17d7 100644 --- a/src/Config.js +++ b/src/Config.js @@ -20,7 +20,6 @@ export class Config { this.restAPIKey = cacheInfo.restAPIKey; this.fileKey = cacheInfo.fileKey; this.facebookAppIds = cacheInfo.facebookAppIds; - this.enableAnonymousUsers = cacheInfo.enableAnonymousUsers; this.allowClientClassCreation = cacheInfo.allowClientClassCreation; this.database = DatabaseAdapter.getDatabaseConnection(applicationId, cacheInfo.collectionPrefix); diff --git a/src/RestWrite.js b/src/RestWrite.js index f54d6a73..b0719975 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -9,7 +9,6 @@ var Auth = require('./Auth'); var Config = require('./Config'); var cryptoUtils = require('./cryptoUtils'); var passwordCrypto = require('./password'); -var oauth = require("./oauth"); var Parse = require('parse/node'); var triggers = require('./triggers'); @@ -213,13 +212,7 @@ RestWrite.prototype.validateAuthData = function() { var authData = this.data.authData; var providers = Object.keys(authData); if (providers.length == 1) { - - var provider = providers[0]; - if (provider == 'anonymous' && !this.config.enableAnonymousUsers) { - throw new Parse.Error(Parse.Error.UNSUPPORTED_SERVICE, - 'This authentication method is unsupported.'); - } - + var provider = providers[0]; var providerAuthData = authData[provider]; var hasToken = (providerAuthData && providerAuthData.id); if (providerAuthData === null || hasToken) { @@ -238,52 +231,15 @@ RestWrite.prototype.handleOAuthAuthData = function(provider) { return; } - var appIds; - var oauthOptions = this.config.oauth[provider]; - if (oauthOptions) { - appIds = oauthOptions.appIds; - } else if (provider == "facebook") { - appIds = this.config.facebookAppIds; - } + let validateAuthData = this.config.oauth.getValidatorForProvider(provider); - var validateAuthData; - var validateAppId; - - if (oauth[provider]) { - validateAuthData = oauth[provider].validateAuthData; - validateAppId = oauth[provider].validateAppId; - } - - // Try the configuration methods - if (oauthOptions) { - if (oauthOptions.module) { - validateAuthData = require(oauthOptions.module).validateAuthData; - validateAppId = require(oauthOptions.module).validateAppId; - }; - - if (oauthOptions.validateAuthData) { - validateAuthData = oauthOptions.validateAuthData; - } - if (oauthOptions.validateAppId) { - validateAppId = oauthOptions.validateAppId; - } - } - // try the custom provider first, fallback on the oauth implementation - - if (!validateAuthData || !validateAppId) { - return false; + if (!validateAuthData) { + throw new Parse.Error(Parse.Error.UNSUPPORTED_SERVICE, + 'This authentication method is unsupported.'); }; - return validateAuthData(authData, oauthOptions) + return validateAuthData(authData) .then(() => { - if (appIds && typeof validateAppId === "function") { - return validateAppId(appIds, authData, oauthOptions); - } - - // No validation required by the developer - return Promise.resolve(); - - }).then(() => { // Check if this user already exists // TODO: does this handle re-linking correctly? var query = {}; @@ -314,7 +270,6 @@ RestWrite.prototype.handleOAuthAuthData = function(provider) { // are different if (results[0].objectId !== this.query.objectId) { delete this.data["_auth_data_" + provider ]; - console.log("alerady linked!"); throw new Parse.Error(Parse.Error.ACCOUNT_ALREADY_LINKED, 'this auth is already used'); } diff --git a/src/index.js b/src/index.js index 131f1f69..5c53a413 100644 --- a/src/index.js +++ b/src/index.js @@ -8,7 +8,8 @@ var batch = require('./batch'), express = require('express'), middlewares = require('./middlewares'), multer = require('multer'), - Parse = require('parse/node').Parse; + Parse = require('parse/node').Parse, + oauthManager = require('./oauth'); //import passwordReset from './passwordReset'; import cache from './cache'; @@ -163,9 +164,8 @@ function ParseServer({ hooksController: hooksController, userController: userController, verifyUserEmails: verifyUserEmails, - enableAnonymousUsers: enableAnonymousUsers, allowClientClassCreation: allowClientClassCreation, - oauth: oauth, + oauth: oauthManager(oauth, enableAnonymousUsers), appName: appName, publicServerURL: publicServerURL, customPages: customPages, diff --git a/src/oauth/index.js b/src/oauth/index.js index 5067b4a1..04ef8971 100644 --- a/src/oauth/index.js +++ b/src/oauth/index.js @@ -1,25 +1,94 @@ -var facebook = require('./facebook'); -var instagram = require("./instagram"); -var linkedin = require("./linkedin"); -var meetup = require("./meetup"); -var google = require("./google"); -var github = require("./github"); -var twitter = require("./twitter"); +let facebook = require('./facebook'); +let instagram = require("./instagram"); +let linkedin = require("./linkedin"); +let meetup = require("./meetup"); +let google = require("./google"); +let github = require("./github"); +let twitter = require("./twitter"); -module.exports = { - facebook: facebook, - github: github, - google: google, - instagram: instagram, - linkedin: linkedin, - meetup: meetup, - twitter: twitter, - anonymous: { - validateAuthData: function() { - return Promise.resolve(); - }, - validateAppId: function() { - return Promise.resolve(); +let anonymous = { + validateAuthData: () => { + return Promise.resolve(); + }, + validateAppId: () => { + return Promise.resolve(); + } +} + +let providers = { + facebook, + instagram, + linkedin, + meetup, + google, + github, + twitter, + anonymous +} + +module.exports = function(oauthOptions = {}, enableAnonymousUsers = true) { + let _enableAnonymousUsers = enableAnonymousUsers; + let setEnableAnonymousUsers = function(enable) { + _enableAnonymousUsers = enable; + } + // To handle the test cases on configuration + let getValidatorForProvider = function(provider) { + + if (provider === 'anonymous' && !_enableAnonymousUsers) { + return; + } + + let defaultProvider = providers[provider]; + let optionalProvider = oauthOptions[provider]; + + if (!defaultProvider && !optionalProvider) { + return; + } + + let appIds; + if (optionalProvider) { + appIds = optionalProvider.appIds; + } + + var validateAuthData; + var validateAppId; + + if (defaultProvider) { + validateAuthData = defaultProvider.validateAuthData; + validateAppId = defaultProvider.validateAppId; + } + + // Try the configuration methods + if (optionalProvider) { + if (optionalProvider.module) { + validateAuthData = require(optionalProvider.module).validateAuthData; + validateAppId = require(optionalProvider.module).validateAppId; + }; + + if (optionalProvider.validateAuthData) { + validateAuthData = optionalProvider.validateAuthData; + } + if (optionalProvider.validateAppId) { + validateAppId = optionalProvider.validateAppId; + } + } + + if (!validateAuthData || !validateAppId) { + return; + } + + return function(authData) { + return validateAuthData(authData, optionalProvider).then(() => { + if (appIds) { + return validateAppId(appIds, authData, optionalProvider); + } + return Promise.resolve(); + }) } } + + return Object.freeze({ + getValidatorForProvider, + setEnableAnonymousUsers, + }) } \ No newline at end of file From 9c5f14981e4c8625b061d92b8c2d38e09bd7a01e Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Thu, 10 Mar 2016 09:48:02 -0500 Subject: [PATCH 15/22] Renames oauth to authDataManager in src --- spec/OAuth.spec.js | 4 ++-- spec/RestCreate.spec.js | 2 +- spec/helper.js | 2 +- src/Config.js | 2 +- src/RestWrite.js | 2 +- src/{oauth => authDataManager}/OAuth1Client.js | 0 src/{oauth => authDataManager}/facebook.js | 0 src/{oauth => authDataManager}/github.js | 0 src/{oauth => authDataManager}/google.js | 0 src/{oauth => authDataManager}/index.js | 0 src/{oauth => authDataManager}/instagram.js | 0 src/{oauth => authDataManager}/linkedin.js | 0 src/{oauth => authDataManager}/meetup.js | 0 src/{oauth => authDataManager}/twitter.js | 0 src/index.js | 4 ++-- 15 files changed, 8 insertions(+), 8 deletions(-) rename src/{oauth => authDataManager}/OAuth1Client.js (100%) rename src/{oauth => authDataManager}/facebook.js (100%) rename src/{oauth => authDataManager}/github.js (100%) rename src/{oauth => authDataManager}/google.js (100%) rename src/{oauth => authDataManager}/index.js (100%) rename src/{oauth => authDataManager}/instagram.js (100%) rename src/{oauth => authDataManager}/linkedin.js (100%) rename src/{oauth => authDataManager}/meetup.js (100%) rename src/{oauth => authDataManager}/twitter.js (100%) diff --git a/spec/OAuth.spec.js b/spec/OAuth.spec.js index 47e4349d..48f22ace 100644 --- a/spec/OAuth.spec.js +++ b/spec/OAuth.spec.js @@ -1,4 +1,4 @@ -var OAuth = require("../src/oauth/OAuth1Client"); +var OAuth = require("../src/authDataManager/OAuth1Client"); var request = require('request'); describe('OAuth', function() { @@ -138,7 +138,7 @@ describe('OAuth', function() { ["facebook", "github", "instagram", "google", "linkedin", "meetup", "twitter"].map(function(providerName){ it("Should validate structure of "+providerName, (done) => { - var provider = require("../src/oauth/"+providerName); + var provider = require("../src/authDataManager/"+providerName); jequal(typeof provider.validateAuthData, "function"); jequal(typeof provider.validateAppId, "function"); jequal(provider.validateAuthData({}, {}).constructor, Promise.prototype.constructor); diff --git a/spec/RestCreate.spec.js b/spec/RestCreate.spec.js index f593e221..7af4e346 100644 --- a/spec/RestCreate.spec.js +++ b/spec/RestCreate.spec.js @@ -149,7 +149,7 @@ describe('rest create', () => { it('handles no anonymous users config', (done) => { var NoAnnonConfig = Object.assign({}, config); - NoAnnonConfig.oauth.setEnableAnonymousUsers(false); + NoAnnonConfig.authDataManager.setEnableAnonymousUsers(false); var data1 = { authData: { anonymous: { diff --git a/spec/helper.js b/spec/helper.js index 8ed9fe26..6c7c9414 100644 --- a/spec/helper.js +++ b/spec/helper.js @@ -5,7 +5,7 @@ jasmine.DEFAULT_TIMEOUT_INTERVAL = 2000; var cache = require('../src/cache').default; var DatabaseAdapter = require('../src/DatabaseAdapter'); var express = require('express'); -var facebook = require('../src/oauth/facebook'); +var facebook = require('../src/authDataManager/facebook'); var ParseServer = require('../src/index').ParseServer; var path = require('path'); diff --git a/src/Config.js b/src/Config.js index f65b17d7..e80c3b28 100644 --- a/src/Config.js +++ b/src/Config.js @@ -33,7 +33,7 @@ export class Config { this.pushController = cacheInfo.pushController; this.loggerController = cacheInfo.loggerController; this.userController = cacheInfo.userController; - this.oauth = cacheInfo.oauth; + this.authDataManager = cacheInfo.authDataManager; this.customPages = cacheInfo.customPages || {}; this.mount = mount; } diff --git a/src/RestWrite.js b/src/RestWrite.js index b0719975..d51d61ad 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -231,7 +231,7 @@ RestWrite.prototype.handleOAuthAuthData = function(provider) { return; } - let validateAuthData = this.config.oauth.getValidatorForProvider(provider); + let validateAuthData = this.config.authDataManager.getValidatorForProvider(provider); if (!validateAuthData) { throw new Parse.Error(Parse.Error.UNSUPPORTED_SERVICE, diff --git a/src/oauth/OAuth1Client.js b/src/authDataManager/OAuth1Client.js similarity index 100% rename from src/oauth/OAuth1Client.js rename to src/authDataManager/OAuth1Client.js diff --git a/src/oauth/facebook.js b/src/authDataManager/facebook.js similarity index 100% rename from src/oauth/facebook.js rename to src/authDataManager/facebook.js diff --git a/src/oauth/github.js b/src/authDataManager/github.js similarity index 100% rename from src/oauth/github.js rename to src/authDataManager/github.js diff --git a/src/oauth/google.js b/src/authDataManager/google.js similarity index 100% rename from src/oauth/google.js rename to src/authDataManager/google.js diff --git a/src/oauth/index.js b/src/authDataManager/index.js similarity index 100% rename from src/oauth/index.js rename to src/authDataManager/index.js diff --git a/src/oauth/instagram.js b/src/authDataManager/instagram.js similarity index 100% rename from src/oauth/instagram.js rename to src/authDataManager/instagram.js diff --git a/src/oauth/linkedin.js b/src/authDataManager/linkedin.js similarity index 100% rename from src/oauth/linkedin.js rename to src/authDataManager/linkedin.js diff --git a/src/oauth/meetup.js b/src/authDataManager/meetup.js similarity index 100% rename from src/oauth/meetup.js rename to src/authDataManager/meetup.js diff --git a/src/oauth/twitter.js b/src/authDataManager/twitter.js similarity index 100% rename from src/oauth/twitter.js rename to src/authDataManager/twitter.js diff --git a/src/index.js b/src/index.js index 5c53a413..d496d161 100644 --- a/src/index.js +++ b/src/index.js @@ -9,7 +9,7 @@ var batch = require('./batch'), middlewares = require('./middlewares'), multer = require('multer'), Parse = require('parse/node').Parse, - oauthManager = require('./oauth'); + authDataManager = require('./authDataManager'); //import passwordReset from './passwordReset'; import cache from './cache'; @@ -165,7 +165,7 @@ function ParseServer({ userController: userController, verifyUserEmails: verifyUserEmails, allowClientClassCreation: allowClientClassCreation, - oauth: oauthManager(oauth, enableAnonymousUsers), + authDataManager: authDataManager(oauth, enableAnonymousUsers), appName: appName, publicServerURL: publicServerURL, customPages: customPages, From 16e3529c96959e72033f6c1e07d72486988953c8 Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Thu, 10 Mar 2016 19:20:05 -0500 Subject: [PATCH 16/22] Removes blacklisting, *-but test case --- spec/schemas.spec.js | 78 ++------------------------------------------ src/Schema.js | 27 +++++++-------- 2 files changed, 13 insertions(+), 92 deletions(-) diff --git a/spec/schemas.spec.js b/spec/schemas.spec.js index 1350c74d..40e6150b 100644 --- a/spec/schemas.spec.js +++ b/spec/schemas.spec.js @@ -1408,10 +1408,7 @@ describe('schemas', () => { role.relation('users').add(admin); return role.save(null, {useMasterKey: true}).then(() => { let perm = { - 'find': { - // Admins can't read - 'role:admin': false - } + find: {} }; // let the user find perm['find'][user.id] = true; @@ -1455,76 +1452,5 @@ describe('schemas', () => { }).then(() => { done(); }); - }); - - it('validate CLP 6', done => { - let user = new Parse.User(); - user.setUsername('user'); - user.setPassword('user'); - - let user2 = new Parse.User(); - user2.setUsername('user2'); - user2.setPassword('user2'); - let admin = new Parse.User(); - admin.setUsername('admin'); - admin.setPassword('admin'); - - let role = new Parse.Role('admin', new Parse.ACL()); - - Promise.resolve().then(() => { - return Parse.Object.saveAll([user, user2, admin, role], {useMasterKey: true}); - }).then(()=> { - role.relation('users').add(admin); - return role.save(null, {useMasterKey: true}).then(() => { - let perm = { - 'find': { - // Anyone can find - '*': true - } - }; - // but the user can't - perm['find'][user.id] = false; - return setPermissionsOnClass('AClass', perm); - }) - }).then(() => { - return Parse.User.logIn('user', 'user').then(() => { - let obj = new Parse.Object('AClass'); - return obj.save(); - }) - }).then(() => { - let query = new Parse.Query('AClass'); - return query.find().then((res) => { - fail('User should not be able to find!') - return Promise.resolve(); - }, (err) => { - expect(err.message).toEqual('Permission denied for this action.'); - return Promise.resolve(); - }) - }).then(() => { - return Parse.User.logIn('admin', 'admin'); - }).then( () => { - let query = new Parse.Query('AClass'); - return query.find(); - }).then((results) => { - expect(results.length).toEqual(1); - return Promise.resolve(); - }, (err) => { - fail('Should find the object as admin'); - return Promise.resolve(); - }).then(() => { - return Parse.User.logIn('user2', 'user2'); - }).then( () => { - let query = new Parse.Query('AClass'); - return query.find(); - }).then((results) => { - expect(results.length).toEqual(1); - return Promise.resolve(); - }, (err) => { - fail('Should find the object as user2'); - return Promise.resolve(); - }).then(() => { - done(); - }); - }); - + }); }); diff --git a/src/Schema.js b/src/Schema.js index 9b18517a..f4e1b9bf 100644 --- a/src/Schema.js +++ b/src/Schema.js @@ -107,7 +107,7 @@ function validateCLP(perms) { Object.keys(perms[operation]).forEach((key) => { verifyPermissionKey(key); let perm = perms[operation][key]; - if (perm !== true && perm !== false) { + if (perm !== true) { throw new Parse.Error(Parse.Error.INVALID_JSON, `'${perm}' is not a valid value for class level permissions ${operation}:${key}:${perm}`); } }); @@ -585,22 +585,17 @@ class Schema { return Promise.resolve(); } var perms = this.perms[className][operation]; - - // Check permissions against the aclGroup provided (array of userId/roles) - // if perms has a public, check the blacklist - let startfound = perms['*'] ? true : undefined; - let found = aclGroup.reduce((memo, acl) => { - let perm = perms[acl]; - // We have a black listed permission - if (perm === false) { - return false; + // Handle the public scenario quickly + if (perms['*']) { + return Promise.resolve(); + } + // Check permissions against the aclGroup provided (array of userId/roles) + var found = false; + for (var i = 0; i < aclGroup.length && !found; i++) { + if (perms[aclGroup[i]]) { + found = true; } - // the memo is not blacklisted - if (perm === true && memo !== false) { - return true; - } - return memo; - }, startfound); + } if (!found) { // TODO: Verify correct error code throw new Parse.Error(Parse.Error.OBJECT_NOT_FOUND, From c935ed836420f06f02958bdcb11e006a35e06d8a Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Thu, 10 Mar 2016 23:01:45 -0500 Subject: [PATCH 17/22] Always return default public permissions --- spec/schemas.spec.js | 59 ++++++++++++++++++++++++++++++++++++++------ src/Schema.js | 14 +++++++++-- 2 files changed, 63 insertions(+), 10 deletions(-) diff --git a/spec/schemas.spec.js b/spec/schemas.spec.js index 40e6150b..e9195615 100644 --- a/spec/schemas.spec.js +++ b/spec/schemas.spec.js @@ -23,6 +23,27 @@ var hasAllPODobject = () => { return obj; }; +let defaultClassLevelPermissions = { + find: { + '*': true + }, + create: { + '*': true + }, + get: { + '*': true + }, + update: { + '*': true + }, + addField: { + '*': true + }, + delete: { + '*': true + } +} + var plainOldDataSchema = { className: 'HasAllPOD', fields: { @@ -40,7 +61,8 @@ var plainOldDataSchema = { aArray: {type: 'Array'}, aGeoPoint: {type: 'GeoPoint'}, aFile: {type: 'File'} - } + }, + classLevelPermissions: defaultClassLevelPermissions }; var pointersAndRelationsSchema = { @@ -61,6 +83,7 @@ var pointersAndRelationsSchema = { targetClass: 'HasAllPOD', }, }, + classLevelPermissions: defaultClassLevelPermissions } var noAuthHeaders = { @@ -296,7 +319,8 @@ describe('schemas', () => { objectId: {type: 'String'}, foo: {type: 'Number'}, ptr: {type: 'Pointer', targetClass: 'SomeClass'}, - } + }, + classLevelPermissions: defaultClassLevelPermissions }); done(); }); @@ -318,7 +342,8 @@ describe('schemas', () => { createdAt: {type: 'Date'}, updatedAt: {type: 'Date'}, objectId: {type: 'String'}, - } + }, + classLevelPermissions: defaultClassLevelPermissions }); done(); }); @@ -490,7 +515,8 @@ describe('schemas', () => { "objectId": {"type": "String"}, "updatedAt": {"type": "Date"}, "geo2": {"type": "GeoPoint"}, - } + }, + classLevelPermissions: defaultClassLevelPermissions })).toEqual(undefined); done(); }); @@ -539,6 +565,7 @@ describe('schemas', () => { "updatedAt": {"type": "Date"}, "newField": {"type": "String"}, }, + classLevelPermissions: defaultClassLevelPermissions })).toEqual(undefined); request.get({ url: 'http://localhost:8378/1/schemas/NewClass', @@ -553,7 +580,8 @@ describe('schemas', () => { updatedAt: {type: 'Date'}, objectId: {type: 'String'}, newField: {type: 'String'}, - } + }, + classLevelPermissions: defaultClassLevelPermissions }); done(); }); @@ -590,7 +618,8 @@ describe('schemas', () => { emailVerified: {type: 'Boolean'}, newField: {type: 'String'}, ACL: {type: 'ACL'} - } + }, + classLevelPermissions: defaultClassLevelPermissions }); request.get({ url: 'http://localhost:8378/1/schemas/_User', @@ -610,7 +639,8 @@ describe('schemas', () => { emailVerified: {type: 'Boolean'}, newField: {type: 'String'}, ACL: {type: 'ACL'} - } + }, + classLevelPermissions: defaultClassLevelPermissions }); done(); }); @@ -656,7 +686,8 @@ describe('schemas', () => { aNewString: {type: 'String'}, aNewPointer: {type: 'Pointer', targetClass: 'HasAllPOD'}, aNewRelation: {type: 'Relation', targetClass: 'HasAllPOD'}, - } + }, + classLevelPermissions: defaultClassLevelPermissions }); var obj2 = new Parse.Object('HasAllPOD'); obj2.set('aNewPointer', obj1); @@ -902,6 +933,18 @@ describe('schemas', () => { }, create: { 'role:admin': true + }, + get: { + '*': true + }, + update: { + '*': true + }, + addField: { + '*': true + }, + delete: { + '*': true } }); done(); diff --git a/src/Schema.js b/src/Schema.js index f4e1b9bf..ffb7b088 100644 --- a/src/Schema.js +++ b/src/Schema.js @@ -96,6 +96,13 @@ function verifyPermissionKey(key) { } let CLPValidKeys = ['find', 'get', 'create', 'update', 'delete', 'addField']; +let DefaultClassLevelPermissions = CLPValidKeys.reduce((perms, key) => { + perms[key] = { + '*': true + }; + return perms; + }, {}); + function validateCLP(perms) { if (!perms) { return; @@ -879,9 +886,12 @@ function mongoSchemaToSchemaAPIResponse(schema) { className: schema._id, fields: mongoSchemaAPIResponseFields(schema), }; + + let classLevelPermissions = DefaultClassLevelPermissions; if (schema._metadata && schema._metadata.class_permissions) { - result.classLevelPermissions = schema._metadata.class_permissions; - } + classLevelPermissions = Object.assign(classLevelPermissions, schema._metadata.class_permissions); + } + result.classLevelPermissions = classLevelPermissions; return result; } From 8ea2b615a4bc320b71f15ffa00ebb4615b465ee0 Mon Sep 17 00:00:00 2001 From: wangmengyan95 Date: Thu, 10 Mar 2016 11:30:29 -0800 Subject: [PATCH 18/22] Do master query for before/afterSaveHook --- spec/ParseAPI.spec.js | 51 +++++++++++++++++++++++++++++++++++++++++++ src/rest.js | 5 +++-- 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/spec/ParseAPI.spec.js b/spec/ParseAPI.spec.js index bd29bcc7..92e30732 100644 --- a/spec/ParseAPI.spec.js +++ b/spec/ParseAPI.spec.js @@ -643,6 +643,7 @@ describe('miscellaneous', function() { it('test afterSave get original object on update', function(done) { var triggerTime = 0; // Register a mock beforeSave hook + Parse.Cloud.afterSave('GameScore', function(req, res) { var object = req.object; expect(object instanceof Parse.Object).toBeTruthy(); @@ -693,6 +694,56 @@ describe('miscellaneous', function() { }); }); + it('test afterSave get full original object even req auth can not query it', (done) => { + var triggerTime = 0; + // Register a mock beforeSave hook + Parse.Cloud.afterSave('GameScore', function(req, res) { + var object = req.object; + var originalObject = req.original; + if (triggerTime == 0) { + // Create + } else if (triggerTime == 1) { + // Update + expect(object.get('foo')).toEqual('baz'); + // Make sure we get the full originalObject + expect(originalObject instanceof Parse.Object).toBeTruthy(); + expect(originalObject.get('fooAgain')).toEqual('barAgain'); + expect(originalObject.id).not.toBeUndefined(); + expect(originalObject.createdAt).not.toBeUndefined(); + expect(originalObject.updatedAt).not.toBeUndefined(); + expect(originalObject.get('foo')).toEqual('bar'); + } else { + res.error(); + } + triggerTime++; + res.success(); + }); + + var obj = new Parse.Object('GameScore'); + obj.set('foo', 'bar'); + obj.set('fooAgain', 'barAgain'); + var acl = new Parse.ACL(); + // Make sure our update request can not query the object + acl.setPublicReadAccess(false); + acl.setPublicWriteAccess(true); + obj.setACL(acl); + obj.save().then(function() { + // We only update foo + obj.set('foo', 'baz'); + return obj.save(); + }).then(function() { + // Make sure the checking has been triggered + expect(triggerTime).toBe(2); + // Clear mock afterSave + Parse.Cloud._removeHook("Triggers", "afterSave", "GameScore"); + done(); + }, function(error) { + console.error(error); + fail(error); + done(); + }); + }); + it('afterSave flattens custom operations', done => { var triggerTime = 0; // Register a mock beforeSave hook diff --git a/src/rest.js b/src/rest.js index d624f068..96269bc7 100644 --- a/src/rest.js +++ b/src/rest.js @@ -9,6 +9,7 @@ var Parse = require('parse/node').Parse; import cache from './cache'; +import Auth from './Auth'; var RestQuery = require('./RestQuery'); var RestWrite = require('./RestWrite'); @@ -42,7 +43,7 @@ function del(config, auth, className, objectId) { if (triggers.getTrigger(className, triggers.Types.beforeDelete, config.applicationId) || triggers.getTrigger(className, triggers.Types.afterDelete, config.applicationId) || className == '_Session') { - return find(config, auth, className, {objectId: objectId}) + return find(config, Auth.master(config), className, {objectId: objectId}) .then((response) => { if (response && response.results && response.results.length) { response.results[0].className = className; @@ -97,7 +98,7 @@ function update(config, auth, className, objectId, restObject) { return Promise.resolve().then(() => { if (triggers.getTrigger(className, triggers.Types.beforeSave, config.applicationId) || triggers.getTrigger(className, triggers.Types.afterSave, config.applicationId)) { - return find(config, auth, className, {objectId: objectId}); + return find(config, Auth.master(config), className, {objectId: objectId}); } return Promise.resolve({}); }).then((response) => { From bcffcbade267850280707d8c20dd36ad2a39c595 Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Thu, 10 Mar 2016 18:59:19 -0500 Subject: [PATCH 19/22] Fix when multiple authData keys are passed --- spec/ParseUser.spec.js | 152 ++++++++++++++++++++++++++++++++++++++++ spec/RestCreate.spec.js | 3 +- src/RestWrite.js | 136 ++++++++++++++++++++--------------- 3 files changed, 234 insertions(+), 57 deletions(-) diff --git a/spec/ParseUser.spec.js b/spec/ParseUser.spec.js index a74644ae..50f29331 100644 --- a/spec/ParseUser.spec.js +++ b/spec/ParseUser.spec.js @@ -904,6 +904,50 @@ describe('Parse.User testing', () => { } }; }; + + var getMockMyOauthProvider = function() { + return { + authData: { + id: "12345", + access_token: "12345", + expiration_date: new Date().toJSON(), + }, + shouldError: false, + loggedOut: false, + synchronizedUserId: null, + synchronizedAuthToken: null, + synchronizedExpiration: null, + + authenticate: function(options) { + if (this.shouldError) { + options.error(this, "An error occurred"); + } else if (this.shouldCancel) { + options.error(this, null); + } else { + options.success(this, this.authData); + } + }, + restoreAuthentication: function(authData) { + if (!authData) { + this.synchronizedUserId = null; + this.synchronizedAuthToken = null; + this.synchronizedExpiration = null; + return true; + } + this.synchronizedUserId = authData.id; + this.synchronizedAuthToken = authData.access_token; + this.synchronizedExpiration = authData.expiration_date; + return true; + }, + getAuthType: function() { + return "myoauth"; + }, + deauthenticate: function() { + this.loggedOut = true; + this.restoreAuthentication(null); + } + }; + }; var ExtendedUser = Parse.User.extend({ extended: function() { @@ -1284,6 +1328,114 @@ describe('Parse.User testing', () => { } }); }); + + it("link multiple providers", (done) => { + var provider = getMockFacebookProvider(); + var mockProvider = getMockMyOauthProvider(); + Parse.User._registerAuthenticationProvider(provider); + Parse.User._logInWith("facebook", { + success: function(model) { + ok(model instanceof Parse.User, "Model should be a Parse.User"); + strictEqual(Parse.User.current(), model); + ok(model.extended(), "Should have used the subclass."); + strictEqual(provider.authData.id, provider.synchronizedUserId); + strictEqual(provider.authData.access_token, provider.synchronizedAuthToken); + strictEqual(provider.authData.expiration_date, provider.synchronizedExpiration); + ok(model._isLinked("facebook"), "User should be linked to facebook"); + Parse.User._registerAuthenticationProvider(mockProvider); + let objectId = model.id; + model._linkWith("myoauth", { + success: function(model) { + expect(model.id).toEqual(objectId); + ok(model._isLinked("facebook"), "User should be linked to facebook"); + ok(model._isLinked("myoauth"), "User should be linked to myoauth"); + done(); + }, + error: function(error) { + console.error(error); + fail('SHould not fail'); + done(); + } + }) + }, + error: function(model, error) { + ok(false, "linking should have worked"); + done(); + } + }); + }); + + it("link multiple providers and update token", (done) => { + var provider = getMockFacebookProvider(); + var mockProvider = getMockMyOauthProvider(); + Parse.User._registerAuthenticationProvider(provider); + Parse.User._logInWith("facebook", { + success: function(model) { + ok(model instanceof Parse.User, "Model should be a Parse.User"); + strictEqual(Parse.User.current(), model); + ok(model.extended(), "Should have used the subclass."); + strictEqual(provider.authData.id, provider.synchronizedUserId); + strictEqual(provider.authData.access_token, provider.synchronizedAuthToken); + strictEqual(provider.authData.expiration_date, provider.synchronizedExpiration); + ok(model._isLinked("facebook"), "User should be linked to facebook"); + Parse.User._registerAuthenticationProvider(mockProvider); + let objectId = model.id; + model._linkWith("myoauth", { + success: function(model) { + expect(model.id).toEqual(objectId); + ok(model._isLinked("facebook"), "User should be linked to facebook"); + ok(model._isLinked("myoauth"), "User should be linked to myoauth"); + model._linkWith("facebook", { + success: () => { + ok(model._isLinked("facebook"), "User should be linked to facebook"); + ok(model._isLinked("myoauth"), "User should be linked to myoauth"); + done(); + }, + error: () => { + fail('should link again'); + done(); + } + }) + }, + error: function(error) { + console.error(error); + fail('SHould not fail'); + done(); + } + }) + }, + error: function(model, error) { + ok(false, "linking should have worked"); + done(); + } + }); + }); + + it('should fail linking with existing', (done) => { + var provider = getMockFacebookProvider(); + Parse.User._registerAuthenticationProvider(provider); + Parse.User._logInWith("facebook", { + success: function(model) { + Parse.User.logOut().then(() => { + let user = new Parse.User(); + user.setUsername('user'); + user.setPassword('password'); + return user.signUp().then(() => { + // try to link here + user._linkWith('facebook', { + success: () => { + fail('should not succeed'); + done(); + }, + error: (err) => { + done(); + } + }); + }); + }); + } + }); + }); it('set password then change password', (done) => { Parse.User.signUp('bob', 'barker').then((bob) => { diff --git a/spec/RestCreate.spec.js b/spec/RestCreate.spec.js index 7af4e346..553d37ad 100644 --- a/spec/RestCreate.spec.js +++ b/spec/RestCreate.spec.js @@ -163,6 +163,7 @@ describe('rest create', () => { }, (err) => { expect(err.code).toEqual(Parse.Error.UNSUPPORTED_SERVICE); expect(err.message).toEqual('This authentication method is unsupported.'); + NoAnnonConfig.authDataManager.setEnableAnonymousUsers(true); done(); }) }); @@ -199,7 +200,7 @@ describe('rest create', () => { done(); }); }); - + it('stores pointers with a _p_ prefix', (done) => { var obj = { foo: 'bar', diff --git a/src/RestWrite.js b/src/RestWrite.js index d51d61ad..a31c8e5b 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -32,7 +32,7 @@ function RestWrite(config, auth, className, query, data, originalData) { throw new Parse.Error(Parse.Error.INVALID_KEY_NAME, 'objectId ' + 'is an invalid field name.'); } - + // When the operation is complete, this.response may have several // fields. // response: the actual data to be returned @@ -211,74 +211,98 @@ RestWrite.prototype.validateAuthData = function() { var authData = this.data.authData; var providers = Object.keys(authData); - if (providers.length == 1) { - var provider = providers[0]; + if (providers.length > 0) { + var provider = providers[providers.length-1]; var providerAuthData = authData[provider]; var hasToken = (providerAuthData && providerAuthData.id); if (providerAuthData === null || hasToken) { - return this.handleOAuthAuthData(provider); + return this.handleAuthData(authData); } } throw new Parse.Error(Parse.Error.UNSUPPORTED_SERVICE, 'This authentication method is unsupported.'); }; -RestWrite.prototype.handleOAuthAuthData = function(provider) { - var authData = this.data.authData[provider]; - if (authData === null && this.query) { - // We are unlinking from the provider. - this.data["_auth_data_" + provider ] = null; - return; - } +RestWrite.prototype.handleAuthDataValidation = function(authData) { + let validations = Object.keys(authData).map((provider) => { + if (authData[provider] === null) { + return Promise.resolve(); + } + let validateAuthData = this.config.authDataManager.getValidatorForProvider(provider); + if (!validateAuthData) { + throw new Parse.Error(Parse.Error.UNSUPPORTED_SERVICE, + 'This authentication method is unsupported.'); + }; + return validateAuthData(authData[provider]); + }); + return Promise.all(validations); +} - let validateAuthData = this.config.authDataManager.getValidatorForProvider(provider); - - if (!validateAuthData) { - throw new Parse.Error(Parse.Error.UNSUPPORTED_SERVICE, - 'This authentication method is unsupported.'); - }; - - return validateAuthData(authData) - .then(() => { - // Check if this user already exists - // TODO: does this handle re-linking correctly? - var query = {}; - query['authData.' + provider + '.id'] = authData.id; - return this.config.database.find( +RestWrite.prototype.findUsersWithAuthData = function(authData) { + let providers = Object.keys(authData); + let query = providers.reduce((memo, provider) => { + if (!authData[provider]) { + return memo; + } + let queryKey = `authData.${provider}.id`; + let query = {}; + query[queryKey] = authData[provider].id; + memo.push(query); + return memo; + }, []).filter((q) => { + return typeof q !== undefined; + }); + + let findPromise = Promise.resolve([]); + if (query.length > 0) { + findPromise = this.config.database.find( this.className, - query, {}); - }).then((results) => { - this.storage['authProvider'] = provider; - - // Put the data in the proper format - this.data["_auth_data_" + provider ] = authData; - - if (results.length == 0) { - // this a new user - this.data.username = cryptoUtils.newToken(); - } else if (!this.query) { - // Login with auth data - // Short circuit - delete results[0].password; - this.response = { - response: results[0], - location: this.location() - }; - this.data.objectId = results[0].objectId; - } else if (this.query && this.query.objectId) { - // Trying to update auth data but users - // are different - if (results[0].objectId !== this.query.objectId) { - delete this.data["_auth_data_" + provider ]; - throw new Parse.Error(Parse.Error.ACCOUNT_ALREADY_LINKED, + {'$or': query}, {}) + } + + return findPromise; +} + +RestWrite.prototype.handleAuthData = function(authData) { + let results; + return this.handleAuthDataValidation(authData).then(() => { + return this.findUsersWithAuthData(authData); + }).then((r) => { + results = r; + if (results.length > 1) { + // More than 1 user with the passed id's + throw new Parse.Error(Parse.Error.ACCOUNT_ALREADY_LINKED, 'this auth is already used'); - } - } else { - - delete this.data["_auth_data_" + provider ]; - throw new Parse.Error(Parse.Error.INTERNAL_SERVER_ERROR, 'THis should not be reached...'); - } + } + // set the proper keys + Object.keys(authData).forEach((provider) => { + this.data[`_auth_data_${provider}`] = authData[provider]; }); + + if (results.length == 0) { + this.data.username = cryptoUtils.newToken(); + } else if (!this.query) { + // Login with auth data + // Short circuit + delete results[0].password; + this.response = { + response: results[0], + location: this.location() + }; + this.data.objectId = results[0].objectId; + } else if (this.query && this.query.objectId) { + // Trying to update auth data but users + // are different + if (results[0].objectId !== this.query.objectId) { + Object.keys(authData).forEach((provider) => { + delete this.data[`_auth_data_${provider}`]; + }); + throw new Parse.Error(Parse.Error.ACCOUNT_ALREADY_LINKED, + 'this auth is already used'); + } + } + return Promise.resolve(); + }); } // The non-third-party parts of User transformation From 1ed868b99c696e05f73df3555f9cd802f4a7613b Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Fri, 11 Mar 2016 09:33:09 -0500 Subject: [PATCH 20/22] Fixes #935, cleans up authData null keys on login for android crash --- spec/ParseUser.spec.js | 37 +++++++++++++++++++++++++++++++++++++ src/Routers/UsersRouter.js | 15 ++++++++++++++- 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/spec/ParseUser.spec.js b/spec/ParseUser.spec.js index a74644ae..cb8e9434 100644 --- a/spec/ParseUser.spec.js +++ b/spec/ParseUser.spec.js @@ -9,6 +9,7 @@ var request = require('request'); var passwordCrypto = require('../src/password'); +var Config = require('../src/Config'); function verifyACL(user) { const ACL = user.getACL(); @@ -1780,5 +1781,41 @@ describe('Parse.User testing', () => { } }); }); + + // Sometimes the authData still has null on that keys + // https://github.com/ParsePlatform/parse-server/issues/935 + it('should cleanup null authData keys', (done) => { + let database = new Config(Parse.applicationId).database; + database.create('_User', { + username: 'user', + password: '$2a$10$8/wZJyEuiEaobBBqzTG.jeY.XSFJd0rzaN//ososvEI4yLqI.4aie', + _auth_data_facebook: null + }, {}).then(() => { + return new Promise((resolve, reject) => { + request.get({ + url: 'http://localhost:8378/1/login?username=user&password=test', + headers: { + 'X-Parse-Application-Id': 'test', + 'X-Parse-Master-Key': 'test', + }, + json: true + }, (err, res, body) => { + if (err) { + reject(err); + } else { + resolve(body); + } + }) + }) + }).then((user) => { + let authData = user.authData; + expect(user.username).toEqual('user'); + expect(authData).toBeUndefined(); + done(); + }).catch((err) => { + fail('this should not fail'); + done(); + }) + }); }); diff --git a/src/Routers/UsersRouter.js b/src/Routers/UsersRouter.js index 21dc80ba..ac1d1007 100644 --- a/src/Routers/UsersRouter.js +++ b/src/Routers/UsersRouter.js @@ -102,7 +102,20 @@ export class UsersRouter extends ClassesRouter { let token = 'r:' + cryptoUtils.newToken(); user.sessionToken = token; delete user.password; - + + // Sometimes the authData still has null on that keys + // https://github.com/ParsePlatform/parse-server/issues/935 + if (user.authData) { + Object.keys(user.authData).forEach((provider) => { + if (user.authData[provider] === null) { + delete user.authData[provider]; + } + }); + if (Object.keys(user.authData).length == 0) { + delete user.authData; + } + } + req.config.filesController.expandFilesInObject(req.config, user); let expiresAt = new Date(); From 6e65a8fc6f15e1a0301111e398e39d0e3282c210 Mon Sep 17 00:00:00 2001 From: steven-supersolid Date: Fri, 11 Mar 2016 17:08:39 +0000 Subject: [PATCH 21/22] Add test for options being passed to MongoAdapter from DatabaseAdapter --- spec/DatabaseAdapter.spec.js | 23 +++++++++++++++++++++++ spec/helper.js | 4 ++-- src/DatabaseAdapter.js | 5 +++-- 3 files changed, 28 insertions(+), 4 deletions(-) create mode 100644 spec/DatabaseAdapter.spec.js diff --git a/spec/DatabaseAdapter.spec.js b/spec/DatabaseAdapter.spec.js new file mode 100644 index 00000000..0f43a16b --- /dev/null +++ b/spec/DatabaseAdapter.spec.js @@ -0,0 +1,23 @@ +'use strict'; + +let DatabaseAdapter = require('../src/DatabaseAdapter'); + +describe('DatabaseAdapter', () => { + it('options and URI are available to adapter', done => { + DatabaseAdapter.setAppDatabaseURI('optionsTest', 'mongodb://localhost:27017/optionsTest'); + DatabaseAdapter.setAppDatabaseOptions('optionsTest', {foo: "bar"}); + let optionsTestDatabaseConnection = DatabaseAdapter.getDatabaseConnection('optionsTest'); + + expect(optionsTestDatabaseConnection instanceof Object).toBe(true); + expect(optionsTestDatabaseConnection.adapter._options instanceof Object).toBe(true); + expect(optionsTestDatabaseConnection.adapter._options.foo).toBe("bar"); + + DatabaseAdapter.setAppDatabaseURI('noOptionsTest', 'mongodb://localhost:27017/noOptionsTest'); + let noOptionsTestDatabaseConnection = DatabaseAdapter.getDatabaseConnection('noOptionsTest'); + + expect(noOptionsTestDatabaseConnection instanceof Object).toBe(true); + expect(noOptionsTestDatabaseConnection.adapter._options instanceof Object).toBe(false); + + done(); + }); +}); diff --git a/spec/helper.js b/spec/helper.js index e2daa6ed..74f9dbbd 100644 --- a/spec/helper.js +++ b/spec/helper.js @@ -26,7 +26,7 @@ var defaultConfiguration = { collectionPrefix: 'test_', fileKey: 'test', push: { - 'ios': { + 'ios': { cert: 'prodCert.pem', key: 'prodKey.pem', production: true, @@ -81,7 +81,7 @@ afterEach(function(done) { Parse.User.logOut().then(() => { return clearData(); }).then(() => { - DatabaseAdapter.clearDatabaseURIs(); + DatabaseAdapter.clearDatabaseSettings(); done(); }, (error) => { console.log('error in clearData', error); diff --git a/src/DatabaseAdapter.js b/src/DatabaseAdapter.js index dbc7c6ac..51403ba3 100644 --- a/src/DatabaseAdapter.js +++ b/src/DatabaseAdapter.js @@ -43,9 +43,10 @@ function setAppDatabaseOptions(appId: string, options: Object) { } //Used by tests -function clearDatabaseURIs() { +function clearDatabaseSettings() { appDatabaseURIs = {}; dbConnections = {}; + appDatabaseOptions = {}; } function getDatabaseConnection(appId: string, collectionPrefix: string) { @@ -69,6 +70,6 @@ module.exports = { setDatabaseURI: setDatabaseURI, setAppDatabaseOptions: setAppDatabaseOptions, setAppDatabaseURI: setAppDatabaseURI, - clearDatabaseURIs: clearDatabaseURIs, + clearDatabaseSettings: clearDatabaseSettings, defaultDatabaseURI: databaseURI }; From daad05a00f890630203b9e2685e3c3d55b56d38f Mon Sep 17 00:00:00 2001 From: Florent Vilmart Date: Fri, 11 Mar 2016 14:50:33 -0500 Subject: [PATCH 22/22] removes key transformation for authData from restWrite, ensures authData is set in hooks --- spec/ParseUser.spec.js | 37 ++++++++++++++++++++++++++++++++++++ spec/RestCreate.spec.js | 2 +- src/RestWrite.js | 25 +++++++++--------------- src/authDataManager/index.js | 2 +- src/transform.js | 19 +++++++++++++++++- 5 files changed, 66 insertions(+), 19 deletions(-) diff --git a/spec/ParseUser.spec.js b/spec/ParseUser.spec.js index 50f29331..1776ad56 100644 --- a/spec/ParseUser.spec.js +++ b/spec/ParseUser.spec.js @@ -1436,6 +1436,43 @@ describe('Parse.User testing', () => { } }); }); + + it('should have authData in beforeSave and afterSave', (done) => { + + Parse.Cloud.beforeSave('_User', (request, response) => { + let authData = request.object.get('authData'); + expect(authData).not.toBeUndefined(); + if (authData) { + expect(authData.facebook.id).toEqual('8675309'); + expect(authData.facebook.access_token).toEqual('jenny'); + } else { + fail('authData should be set'); + } + response.success(); + }); + + Parse.Cloud.afterSave('_User', (request, response) => { + let authData = request.object.get('authData'); + expect(authData).not.toBeUndefined(); + if (authData) { + expect(authData.facebook.id).toEqual('8675309'); + expect(authData.facebook.access_token).toEqual('jenny'); + } else { + fail('authData should be set'); + } + response.success(); + }); + + var provider = getMockFacebookProvider(); + Parse.User._registerAuthenticationProvider(provider); + Parse.User._logInWith("facebook", { + success: function(model) { + Parse.Cloud._removeHook('Triggers', 'beforeSave', Parse.User.className); + Parse.Cloud._removeHook('Triggers', 'afterSave', Parse.User.className); + done(); + } + }); + }); it('set password then change password', (done) => { Parse.User.signUp('bob', 'barker').then((bob) => { diff --git a/spec/RestCreate.spec.js b/spec/RestCreate.spec.js index 553d37ad..d07e18ed 100644 --- a/spec/RestCreate.spec.js +++ b/spec/RestCreate.spec.js @@ -200,7 +200,7 @@ describe('rest create', () => { done(); }); }); - + it('stores pointers with a _p_ prefix', (done) => { var obj = { foo: 'bar', diff --git a/src/RestWrite.js b/src/RestWrite.js index a31c8e5b..c2be50af 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -212,10 +212,12 @@ RestWrite.prototype.validateAuthData = function() { var authData = this.data.authData; var providers = Object.keys(authData); if (providers.length > 0) { - var provider = providers[providers.length-1]; - var providerAuthData = authData[provider]; - var hasToken = (providerAuthData && providerAuthData.id); - if (providerAuthData === null || hasToken) { + let canHandleAuthData = providers.reduce((canHandle, provider) => { + var providerAuthData = authData[provider]; + var hasToken = (providerAuthData && providerAuthData.id); + return canHandle && (hasToken || providerAuthData == null); + }, true); + if (canHandleAuthData) { return this.handleAuthData(authData); } } @@ -274,11 +276,9 @@ RestWrite.prototype.handleAuthData = function(authData) { throw new Parse.Error(Parse.Error.ACCOUNT_ALREADY_LINKED, 'this auth is already used'); } - // set the proper keys - Object.keys(authData).forEach((provider) => { - this.data[`_auth_data_${provider}`] = authData[provider]; - }); - + + this.storage['authProvider'] = Object.keys(authData).join(','); + if (results.length == 0) { this.data.username = cryptoUtils.newToken(); } else if (!this.query) { @@ -294,9 +294,6 @@ RestWrite.prototype.handleAuthData = function(authData) { // Trying to update auth data but users // are different if (results[0].objectId !== this.query.objectId) { - Object.keys(authData).forEach((provider) => { - delete this.data[`_auth_data_${provider}`]; - }); throw new Parse.Error(Parse.Error.ACCOUNT_ALREADY_LINKED, 'this auth is already used'); } @@ -708,10 +705,6 @@ RestWrite.prototype.runDatabaseOperation = function() { if (this.data.ACL && this.data.ACL['*unresolved']) { throw new Parse.Error(Parse.Error.INVALID_ACL, 'Invalid ACL.'); } - - if (this.className === '_User') { - delete this.data.authData; - } if (this.query) { // Run an update diff --git a/src/authDataManager/index.js b/src/authDataManager/index.js index 04ef8971..77ee7473 100644 --- a/src/authDataManager/index.js +++ b/src/authDataManager/index.js @@ -91,4 +91,4 @@ module.exports = function(oauthOptions = {}, enableAnonymousUsers = true) { getValidatorForProvider, setEnableAnonymousUsers, }) -} \ No newline at end of file +} diff --git a/src/transform.js b/src/transform.js index 738f2453..aae5cc2a 100644 --- a/src/transform.js +++ b/src/transform.js @@ -87,7 +87,7 @@ export function transformKeyValue(schema, className, restKey, restValue, options return transformWhere(schema, className, s); }); return {key: '$and', value: mongoSubqueries}; - default: + default: // Other auth data var authDataMatch = key.match(/^authData\.([a-zA-Z0-9_]+)\.id$/); if (authDataMatch) { @@ -203,6 +203,9 @@ function transformWhere(schema, className, restWhere) { // restCreate is the "create" clause in REST API form. // Returns the mongo form of the object. function transformCreate(schema, className, restCreate) { + if (className == '_User') { + restCreate = transformAuthData(restCreate); + } var mongoCreate = transformACL(restCreate); for (var restKey in restCreate) { var out = transformKeyValue(schema, className, restKey, restCreate[restKey]); @@ -218,6 +221,10 @@ function transformUpdate(schema, className, restUpdate) { if (!restUpdate) { throw 'got empty restUpdate'; } + if (className == '_User') { + restUpdate = transformAuthData(restUpdate); + } + var mongoUpdate = {}; var acl = transformACL(restUpdate); if (acl._rperm || acl._wperm) { @@ -250,6 +257,16 @@ function transformUpdate(schema, className, restUpdate) { return mongoUpdate; } +function transformAuthData(restObject) { + if (restObject.authData) { + Object.keys(restObject.authData).forEach((provider) => { + restObject[`_auth_data_${provider}`] = restObject.authData[provider]; + }); + delete restObject.authData; + } + 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) {