From cf8b691c2fa62a4c4974c85b47583093361a4b26 Mon Sep 17 00:00:00 2001 From: Benjamin Friedman Date: Tue, 14 Nov 2017 23:33:06 -0800 Subject: [PATCH 1/3] Remove hidden properties from aggregrate responses --- spec/ParseQuery.Aggregate.spec.js | 41 +++++++++++++++++++++++++++++++ src/Routers/AggregateRouter.js | 11 +++++++-- 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/spec/ParseQuery.Aggregate.spec.js b/spec/ParseQuery.Aggregate.spec.js index 1523ba8f77..5b7bb97469 100644 --- a/spec/ParseQuery.Aggregate.spec.js +++ b/spec/ParseQuery.Aggregate.spec.js @@ -409,4 +409,45 @@ describe('Parse.Query Aggregate testing', () => { done(); }).catch(done.fail); }); + + it('does not return sensitive hidden properties', (done) => { + const options = Object.assign({}, masterKeyOptions, { + body: { + match: { + score: { + $gt: 5 + } + }, + } + }); + + const username = 'leaky_user'; + const score = 10; + + const user = new Parse.User(); + user.setUsername(username); + user.setPassword('password'); + user.set('score', score); + user.signUp().then(function() { + return rp.get(Parse.serverURL + '/aggregate/_User', options); + }).then(function(resp) { + expect(resp.results.length).toBe(1); + const result = resp.results[0]; + + expect(result._hashed_password).toBe(undefined); + expect(result._wperm).toBe(undefined); + expect(result._rperm).toBe(undefined); + expect(result._acl).toBe(undefined); + expect(result._created_at).toBe(undefined); + expect(result._updated_at).toBe(undefined); + + expect(result.objectId).not.toBe(undefined); + expect(result.username).toBe(username); + expect(result.score).toBe(score); + + done(); + }).catch(function(err) { + fail(err); + }); + }); }); diff --git a/src/Routers/AggregateRouter.js b/src/Routers/AggregateRouter.js index 925710135d..8f4e859b6d 100644 --- a/src/Routers/AggregateRouter.js +++ b/src/Routers/AggregateRouter.js @@ -2,6 +2,7 @@ import ClassesRouter from './ClassesRouter'; import rest from '../rest'; import * as middleware from '../middlewares'; import Parse from 'parse/node'; +import UsersRouter from './UsersRouter'; const ALLOWED_KEYS = [ 'where', @@ -65,8 +66,14 @@ export class AggregateRouter extends ClassesRouter { if (typeof body.where === 'string') { body.where = JSON.parse(body.where); } - return rest.find(req.config, req.auth, this.className(req), body.where, options, req.info.clientSDK) - .then((response) => { return { response }; }); + return rest.find(req.config, req.auth, this.className(req), body.where, options, req.info.clientSDK).then((response) => { + for(const result of response.results) { + if(typeof result === 'object') { + UsersRouter.removeHiddenProperties(result); + } + } + return { response }; + }); } mountRoutes() { From aa1175fc3137f5cac1265af6c2443219be50ce74 Mon Sep 17 00:00:00 2001 From: Benjamin Friedman Date: Fri, 17 Nov 2017 00:14:40 -0800 Subject: [PATCH 2/3] transform results from mongo & postgres --- spec/ParseQuery.Aggregate.spec.js | 4 + .../Storage/Mongo/MongoStorageAdapter.js | 8 +- .../Postgres/PostgresStorageAdapter.js | 144 +++++++++--------- src/Controllers/DatabaseController.js | 2 +- 4 files changed, 84 insertions(+), 74 deletions(-) diff --git a/spec/ParseQuery.Aggregate.spec.js b/spec/ParseQuery.Aggregate.spec.js index 5b7bb97469..6d8a53e5d5 100644 --- a/spec/ParseQuery.Aggregate.spec.js +++ b/spec/ParseQuery.Aggregate.spec.js @@ -434,6 +434,7 @@ describe('Parse.Query Aggregate testing', () => { expect(resp.results.length).toBe(1); const result = resp.results[0]; + // verify server-side keys are not present... expect(result._hashed_password).toBe(undefined); expect(result._wperm).toBe(undefined); expect(result._rperm).toBe(undefined); @@ -441,6 +442,9 @@ describe('Parse.Query Aggregate testing', () => { expect(result._created_at).toBe(undefined); expect(result._updated_at).toBe(undefined); + // verify createdAt, updatedAt and others are present + expect(result.createdAt).not.toBe(undefined); + expect(result.updatedAt).not.toBe(undefined); expect(result.objectId).not.toBe(undefined); expect(result.username).toBe(username); expect(result.score).toBe(score); diff --git a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js index c3efb7e642..c9b42f315b 100644 --- a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js +++ b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js @@ -409,10 +409,11 @@ export class MongoStorageAdapter { distinct(className, schema, query, fieldName) { schema = convertParseSchemaToMongoSchema(schema); return this._adaptiveCollection(className) - .then(collection => collection.distinct(fieldName, transformWhere(className, query, schema))); + .then(collection => collection.distinct(fieldName, transformWhere(className, query, schema))) + .then(objects => objects.map(object => mongoObjectToParseObject(className, object, schema))); } - aggregate(className, pipeline, readPreference) { + aggregate(className, schema, pipeline, readPreference) { readPreference = this._parseReadPreference(readPreference); return this._adaptiveCollection(className) .then(collection => collection.aggregate(pipeline, { readPreference, maxTimeMS: this._maxTimeMS })) @@ -424,7 +425,8 @@ export class MongoStorageAdapter { } }); return results; - }); + }) + .then(objects => objects.map(object => mongoObjectToParseObject(className, object, schema))); } _parseReadPreference(readPreference) { diff --git a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js index 4560ab9b2e..b6bbb3779e 100644 --- a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js +++ b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js @@ -1254,79 +1254,83 @@ export class PostgresStorageAdapter { } return Promise.reject(err); }) - .then(results => results.map(object => { - Object.keys(schema.fields).forEach(fieldName => { - if (schema.fields[fieldName].type === 'Pointer' && object[fieldName]) { - object[fieldName] = { objectId: object[fieldName], __type: 'Pointer', className: schema.fields[fieldName].targetClass }; - } - if (schema.fields[fieldName].type === 'Relation') { - object[fieldName] = { - __type: "Relation", - className: schema.fields[fieldName].targetClass - } - } - if (object[fieldName] && schema.fields[fieldName].type === 'GeoPoint') { - object[fieldName] = { - __type: "GeoPoint", - latitude: object[fieldName].y, - longitude: object[fieldName].x - } - } - if (object[fieldName] && schema.fields[fieldName].type === 'Polygon') { - let coords = object[fieldName]; - coords = coords.substr(2, coords.length - 4).split('),('); - coords = coords.map((point) => { - return [ - parseFloat(point.split(',')[1]), - parseFloat(point.split(',')[0]) - ]; - }); - object[fieldName] = { - __type: "Polygon", - coordinates: coords - } - } - if (object[fieldName] && schema.fields[fieldName].type === 'File') { - object[fieldName] = { - __type: 'File', - name: object[fieldName] - } - } - }); - //TODO: remove this reliance on the mongo format. DB adapter shouldn't know there is a difference between created at and any other date field. - if (object.createdAt) { - object.createdAt = object.createdAt.toISOString(); - } - if (object.updatedAt) { - object.updatedAt = object.updatedAt.toISOString(); - } - if (object.expiresAt) { - object.expiresAt = { __type: 'Date', iso: object.expiresAt.toISOString() }; - } - if (object._email_verify_token_expires_at) { - object._email_verify_token_expires_at = { __type: 'Date', iso: object._email_verify_token_expires_at.toISOString() }; + .then(results => results.map(object => this.postgresObjectToParseObject(className, object, schema))); + } + + // Converts from a postgres-format object to a REST-format object. + // Does not strip out anything based on a lack of authentication. + postgresObjectToParseObject(className, object, schema) { + Object.keys(schema.fields).forEach(fieldName => { + if (schema.fields[fieldName].type === 'Pointer' && object[fieldName]) { + object[fieldName] = { objectId: object[fieldName], __type: 'Pointer', className: schema.fields[fieldName].targetClass }; + } + if (schema.fields[fieldName].type === 'Relation') { + object[fieldName] = { + __type: "Relation", + className: schema.fields[fieldName].targetClass } - if (object._account_lockout_expires_at) { - object._account_lockout_expires_at = { __type: 'Date', iso: object._account_lockout_expires_at.toISOString() }; + } + if (object[fieldName] && schema.fields[fieldName].type === 'GeoPoint') { + object[fieldName] = { + __type: "GeoPoint", + latitude: object[fieldName].y, + longitude: object[fieldName].x } - if (object._perishable_token_expires_at) { - object._perishable_token_expires_at = { __type: 'Date', iso: object._perishable_token_expires_at.toISOString() }; + } + if (object[fieldName] && schema.fields[fieldName].type === 'Polygon') { + let coords = object[fieldName]; + coords = coords.substr(2, coords.length - 4).split('),('); + coords = coords.map((point) => { + return [ + parseFloat(point.split(',')[1]), + parseFloat(point.split(',')[0]) + ]; + }); + object[fieldName] = { + __type: "Polygon", + coordinates: coords } - if (object._password_changed_at) { - object._password_changed_at = { __type: 'Date', iso: object._password_changed_at.toISOString() }; + } + if (object[fieldName] && schema.fields[fieldName].type === 'File') { + object[fieldName] = { + __type: 'File', + name: object[fieldName] } + } + }); + //TODO: remove this reliance on the mongo format. DB adapter shouldn't know there is a difference between created at and any other date field. + if (object.createdAt) { + object.createdAt = object.createdAt.toISOString(); + } + if (object.updatedAt) { + object.updatedAt = object.updatedAt.toISOString(); + } + if (object.expiresAt) { + object.expiresAt = { __type: 'Date', iso: object.expiresAt.toISOString() }; + } + if (object._email_verify_token_expires_at) { + object._email_verify_token_expires_at = { __type: 'Date', iso: object._email_verify_token_expires_at.toISOString() }; + } + if (object._account_lockout_expires_at) { + object._account_lockout_expires_at = { __type: 'Date', iso: object._account_lockout_expires_at.toISOString() }; + } + if (object._perishable_token_expires_at) { + object._perishable_token_expires_at = { __type: 'Date', iso: object._perishable_token_expires_at.toISOString() }; + } + if (object._password_changed_at) { + object._password_changed_at = { __type: 'Date', iso: object._password_changed_at.toISOString() }; + } - for (const fieldName in object) { - if (object[fieldName] === null) { - delete object[fieldName]; - } - if (object[fieldName] instanceof Date) { - object[fieldName] = { __type: 'Date', iso: object[fieldName].toISOString() }; - } - } + for (const fieldName in object) { + if (object[fieldName] === null) { + delete object[fieldName]; + } + if (object[fieldName] instanceof Date) { + object[fieldName] = { __type: 'Date', iso: object[fieldName].toISOString() }; + } + } - return object; - })); + return object; } // Create a unique index. Unique indexes on nullable fields are not allowed. Since we don't @@ -1399,10 +1403,10 @@ export class PostgresStorageAdapter { } const child = fieldName.split('.')[1]; return results.map(object => object[column][child]); - }); + }).then(results => results.map(object => this.postgresObjectToParseObject(className, object, schema))); } - aggregate(className, pipeline) { + aggregate(className, schema, pipeline) { debug('aggregate', className, pipeline); const values = [className]; let columns = []; @@ -1501,7 +1505,7 @@ export class PostgresStorageAdapter { } }); return results; - }); + }).then(results => results.map(object => this.postgresObjectToParseObject(className, object, schema))); } performInitialization({ VolatileClassesSchemas }) { diff --git a/src/Controllers/DatabaseController.js b/src/Controllers/DatabaseController.js index f49cd2c360..a954cfb409 100644 --- a/src/Controllers/DatabaseController.js +++ b/src/Controllers/DatabaseController.js @@ -875,7 +875,7 @@ DatabaseController.prototype.find = function(className, query, { if (!classExists) { return []; } else { - return this.adapter.aggregate(className, pipeline, readPreference); + return this.adapter.aggregate(className, schema, pipeline, readPreference); } } else { if (!classExists) { From c3642b35f8685c26056803bd195913ed268b27f2 Mon Sep 17 00:00:00 2001 From: Benjamin Friedman Date: Sat, 18 Nov 2017 22:29:42 -0800 Subject: [PATCH 3/3] Adjust ordering to comply with tests --- .../Postgres/PostgresStorageAdapter.js | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js index b6bbb3779e..13d8774a30 100644 --- a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js +++ b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js @@ -1495,17 +1495,19 @@ export class PostgresStorageAdapter { const qs = `SELECT ${columns} FROM $1:name ${wherePattern} ${sortPattern} ${limitPattern} ${skipPattern} ${groupPattern}`; debug(qs, values); - return this._client.any(qs, values).then(results => { - if (countField) { - results[0][countField] = parseInt(results[0][countField], 10); - } - results.forEach(result => { - if (!result.hasOwnProperty('objectId')) { - result.objectId = null; + return this._client.any(qs, values) + .then(results => results.map(object => this.postgresObjectToParseObject(className, object, schema))) + .then(results => { + if (countField) { + results[0][countField] = parseInt(results[0][countField], 10); } + results.forEach(result => { + if (!result.hasOwnProperty('objectId')) { + result.objectId = null; + } + }); + return results; }); - return results; - }).then(results => results.map(object => this.postgresObjectToParseObject(className, object, schema))); } performInitialization({ VolatileClassesSchemas }) {