From 173af009630c606524ca50e603b638acdece9caf Mon Sep 17 00:00:00 2001 From: Drew Gross Date: Thu, 19 May 2016 13:21:03 -0700 Subject: [PATCH 1/5] Cache users by objectID, and clear cache when updated via master key --- spec/CloudCode.spec.js | 23 +++++++++++++++++++++++ src/Auth.js | 2 +- src/RestWrite.js | 5 ++--- 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index 2218a25706..05b5b80abd 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -467,4 +467,27 @@ describe('Cloud Code', () => { done(); }); }); + + it('doesnt receive stale user in cloud code functions after user has been updated with master key (regression test for #1836)', done => { + Parse.Cloud.define('testQuery', function(request, response) { + response.success(request.user.get('data')); + }); + + Parse.User.signUp('user', 'pass') + .then(user => { + user.set('data', 'AAA'); + return user.save(); + }) + .then(() => Parse.Cloud.run('testQuery')) + .then(result => { + expect(result).toEqual('AAA'); + Parse.User.current().set('data', 'BBB'); + return Parse.User.current().save(null, {useMasterKey: true}); + }) + .then(() => Parse.Cloud.run('testQuery')) + .then(result => { + expect(result).toEqual('BBB'); + done(); + }); + }); }); diff --git a/src/Auth.js b/src/Auth.js index 8f21567903..4c94b0c081 100644 --- a/src/Auth.js +++ b/src/Auth.js @@ -71,7 +71,7 @@ var getAuthForSessionToken = function({ config, sessionToken, installationId } = delete obj.password; obj['className'] = '_User'; obj['sessionToken'] = sessionToken; - config.cacheController.user.put(sessionToken, obj); + config.cacheController.user.put(obj.objectId, obj); let userObject = Parse.Object.fromJSON(obj); return new Auth({config, isMaster: false, installationId, user: userObject}); diff --git a/src/RestWrite.js b/src/RestWrite.js index f6e758f6bf..b599794a42 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -319,9 +319,8 @@ RestWrite.prototype.transformUser = function() { var promise = Promise.resolve(); // If we're updating a _User object, clear the user cache for the session - if (this.query && this.auth.user && this.auth.user.getSessionToken()) { - let cacheAdapter = this.config.cacheController; - cacheAdapter.user.del(this.auth.user.getSessionToken()); + if (this.query) { + this.config.cacheController.user.del(this.data.objectId); } return promise.then(() => { From 430549b0437b62ad463f6519ee8e8a2c08b8fdad Mon Sep 17 00:00:00 2001 From: Drew Gross Date: Thu, 19 May 2016 17:43:45 -0700 Subject: [PATCH 2/5] Go back to caching by session token. Clear out cache by querying _Session when user is modified with Master Key (ew, hopefully that can be improved later) --- src/Auth.js | 2 +- src/RestWrite.js | 17 +++++++++++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/Auth.js b/src/Auth.js index 4c94b0c081..8f21567903 100644 --- a/src/Auth.js +++ b/src/Auth.js @@ -71,7 +71,7 @@ var getAuthForSessionToken = function({ config, sessionToken, installationId } = delete obj.password; obj['className'] = '_User'; obj['sessionToken'] = sessionToken; - config.cacheController.user.put(obj.objectId, obj); + config.cacheController.user.put(sessionToken, obj); let userObject = Parse.Object.fromJSON(obj); return new Auth({config, isMaster: false, installationId, user: userObject}); diff --git a/src/RestWrite.js b/src/RestWrite.js index b599794a42..27ebda5b34 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -318,9 +318,22 @@ RestWrite.prototype.transformUser = function() { var promise = Promise.resolve(); - // If we're updating a _User object, clear the user cache for the session if (this.query) { - this.config.cacheController.user.del(this.data.objectId); + // If we're updating a _User object, clear the user cache for the session + if (this.auth.user && this.auth.user.getSessionToken()) { + this.config.cacheController.user.del(this.auth.user.getSessionToken()); + } else { + // Update was made with master key, so we don't have the session. Query to find it. Hopefully we can make this faster later, somehow. + let userObject = Parse.Object.createWithoutData('_User', this.data.objectId); + let sessionsQuery = new Parse.Query('_Session'); + //sessionsQuery.equalTo('user', userObject); + promise = sessionsQuery.find({ useMasterKey: true }) + .then(sessions => { + sessions.forEach(session => { + this.config.cacheController.user.del(session.get('sessionToken')); + }); + }) + } } return promise.then(() => { From f649e07a83d34ec70c2d2e023cb0723bf0a0896b Mon Sep 17 00:00:00 2001 From: Drew Gross Date: Fri, 20 May 2016 22:56:16 -0700 Subject: [PATCH 3/5] Fix issue with user updates from different sessions causing stale reads --- spec/CloudCode.spec.js | 67 +++++++++++++++++++++++++++++ src/Adapters/Cache/InMemoryCache.js | 1 - src/Auth.js | 1 - src/RestWrite.js | 32 ++++++-------- src/Routers/UsersRouter.js | 1 + src/middlewares.js | 20 ++++----- 6 files changed, 91 insertions(+), 31 deletions(-) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index 05b5b80abd..86985dc5a3 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -1,5 +1,7 @@ "use strict" const Parse = require("parse/node"); +const request = require('request'); +const InMemoryCacheAdapter = require('../src/Adapters/Cache/InMemoryCacheAdapter').InMemoryCacheAdapter; describe('Cloud Code', () => { it('can load absolute cloud code file', done => { @@ -490,4 +492,69 @@ describe('Cloud Code', () => { done(); }); }); + + it('clears out the user cache for all sessions when the user is changed', done => { + const cacheAdapter = new InMemoryCacheAdapter({ ttl: 100000000 }); + setServerConfiguration({ ...defaultConfiguration, cacheAdapter }); + Parse.Cloud.define('checkStaleUser', (request, response) => { + response.success(request.user.get('data')); + }); + + let user = new Parse.User(); + user.set('username', 'test'); + user.set('password', 'moon-y'); + user.set('data', 'first data'); + user.signUp() + .then(user => { + let session1 = user.getSessionToken(); + request.get({ + url: 'http://localhost:8378/1/login?username=test&password=moon-y', + json: true, + headers: { + 'X-Parse-Application-Id': 'test', + 'X-Parse-REST-API-Key': 'rest', + }, + }, (error, response, body) => { + let session2 = body.sessionToken; + + //Ensure both session tokens are in the cache + Parse.Cloud.run('checkStaleUser') + .then(() => { + request.post({ + url: 'http://localhost:8378/1/functions/checkStaleUser', + json: true, + headers: { + 'X-Parse-Application-Id': 'test', + 'X-Parse-REST-API-Key': 'rest', + 'X-Parse-Session-Token': session2, + } + }, (error, response, body) => { + Parse.Promise.all([cacheAdapter.get('test:user:' + session1), cacheAdapter.get('test:user:' + session2)]) + .then(([cacheVal1, cacheVal2]) => { + expect(cacheVal1.objectId).toEqual(user.id); + expect(cacheVal2.objectId).toEqual(user.id); + + //Change with session 1 and then read with session 2. + user.set('data', 'second data'); + user.save() + .then(() => { + request.post({ + url: 'http://localhost:8378/1/functions/checkStaleUser', + json: true, + headers: { + 'X-Parse-Application-Id': 'test', + 'X-Parse-REST-API-Key': 'rest', + 'X-Parse-Session-Token': session2, + } + }, (error, response, body) => { + expect(body.result).toEqual('second data'); + done(); + }) + }); + }); + }); + }); + }); + }); + }); }); diff --git a/src/Adapters/Cache/InMemoryCache.js b/src/Adapters/Cache/InMemoryCache.js index 37eeb43b02..2d44292a0a 100644 --- a/src/Adapters/Cache/InMemoryCache.js +++ b/src/Adapters/Cache/InMemoryCache.js @@ -53,7 +53,6 @@ export class InMemoryCache { if (record.timeout) { clearTimeout(record.timeout); } - delete this.cache[key]; } diff --git a/src/Auth.js b/src/Auth.js index 8f21567903..634a839b9a 100644 --- a/src/Auth.js +++ b/src/Auth.js @@ -72,7 +72,6 @@ var getAuthForSessionToken = function({ config, sessionToken, installationId } = obj['className'] = '_User'; obj['sessionToken'] = sessionToken; config.cacheController.user.put(sessionToken, obj); - let userObject = Parse.Object.fromJSON(obj); return new Auth({config, isMaster: false, installationId, user: userObject}); }); diff --git a/src/RestWrite.js b/src/RestWrite.js index 27ebda5b34..be460d4c48 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -11,6 +11,7 @@ var cryptoUtils = require('./cryptoUtils'); var passwordCrypto = require('./password'); var Parse = require('parse/node'); var triggers = require('./triggers'); +import RestQuery from './RestQuery'; // query and data are both provided in REST API format. So data // types are encoded by plain old objects. @@ -319,21 +320,16 @@ RestWrite.prototype.transformUser = function() { var promise = Promise.resolve(); if (this.query) { - // If we're updating a _User object, clear the user cache for the session - if (this.auth.user && this.auth.user.getSessionToken()) { - this.config.cacheController.user.del(this.auth.user.getSessionToken()); - } else { - // Update was made with master key, so we don't have the session. Query to find it. Hopefully we can make this faster later, somehow. - let userObject = Parse.Object.createWithoutData('_User', this.data.objectId); - let sessionsQuery = new Parse.Query('_Session'); - //sessionsQuery.equalTo('user', userObject); - promise = sessionsQuery.find({ useMasterKey: true }) - .then(sessions => { - sessions.forEach(session => { - this.config.cacheController.user.del(session.get('sessionToken')); - }); - }) - } + // If we're updating a _User object, we need to clear out the cache for that user. Find all their + // session tokens, and remove them from the cache. + promise = new RestQuery(this.config, Auth.master(this.config), '_Session', { user: { + __type: "Pointer", + className: "_User", + objectId: this.objectId(), + }}).execute() + .then(results => { + results.results.forEach(session => this.config.cacheController.user.del(session.sessionToken)); + }); } return promise.then(() => { @@ -426,8 +422,7 @@ RestWrite.prototype.createSessionTokenIfNeeded = function() { if (this.response && this.response.response) { this.response.response.sessionToken = token; } - var create = new RestWrite(this.config, Auth.master(this.config), - '_Session', null, sessionData); + var create = new RestWrite(this.config, Auth.master(this.config), '_Session', null, sessionData); return create.execute(); } @@ -494,8 +489,7 @@ RestWrite.prototype.handleSession = function() { } sessionData[key] = this.data[key]; } - var create = new RestWrite(this.config, Auth.master(this.config), - '_Session', null, sessionData); + var create = new RestWrite(this.config, Auth.master(this.config), '_Session', null, sessionData); return create.execute().then((results) => { if (!results.response) { throw new Parse.Error(Parse.Error.INTERNAL_SERVER_ERROR, diff --git a/src/Routers/UsersRouter.js b/src/Routers/UsersRouter.js index adba752f83..a5e6299c58 100644 --- a/src/Routers/UsersRouter.js +++ b/src/Routers/UsersRouter.js @@ -85,6 +85,7 @@ export class UsersRouter extends ClassesRouter { user = results[0]; return passwordCrypto.compare(req.body.password, user.password); }).then((correct) => { + if (!correct) { throw new Parse.Error(Parse.Error.OBJECT_NOT_FOUND, 'Invalid username/password.'); } diff --git a/src/middlewares.js b/src/middlewares.js index d534215453..e46eb62557 100644 --- a/src/middlewares.js +++ b/src/middlewares.js @@ -27,9 +27,9 @@ function handleParseHeaders(req, res, next) { dotNetKey: req.get('X-Parse-Windows-Key'), restAPIKey: req.get('X-Parse-REST-API-Key') }; - + var basicAuth = httpAuth(req); - + if (basicAuth) { info.appId = basicAuth.appId info.masterKey = basicAuth.masterKey || info.masterKey; @@ -156,24 +156,24 @@ function httpAuth(req) { if (!(req.req || req).headers.authorization) return ; - var header = (req.req || req).headers.authorization; - var appId, masterKey, javascriptKey; + var header = (req.req || req).headers.authorization; + var appId, masterKey, javascriptKey; // parse header var authPrefix = 'basic '; - + var match = header.toLowerCase().indexOf(authPrefix); - + if (match == 0) { var encodedAuth = header.substring(authPrefix.length, header.length); var credentials = decodeBase64(encodedAuth).split(':'); - + if (credentials.length == 2) { appId = credentials[0]; var key = credentials[1]; - + var jsKeyPrefix = 'javascript-key='; - + var matchKey = key.indexOf(jsKeyPrefix) if (matchKey == 0) { javascriptKey = key.substring(jsKeyPrefix.length, key.length); @@ -183,7 +183,7 @@ function httpAuth(req) { } } } - + return {appId: appId, masterKey: masterKey, javascriptKey: javascriptKey}; } From 9f60ff36c7abbbe9f8cc21d339790e63e380e4e2 Mon Sep 17 00:00:00 2001 From: Drew Gross Date: Fri, 20 May 2016 23:03:54 -0700 Subject: [PATCH 4/5] Tests aren't transpiled... --- spec/CloudCode.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index 86985dc5a3..7dd29b49c9 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -495,7 +495,7 @@ describe('Cloud Code', () => { it('clears out the user cache for all sessions when the user is changed', done => { const cacheAdapter = new InMemoryCacheAdapter({ ttl: 100000000 }); - setServerConfiguration({ ...defaultConfiguration, cacheAdapter }); + setServerConfiguration(Object.assign({}, defaultConfiguration, { cacheAdapter: cacheAdapter })); Parse.Cloud.define('checkStaleUser', (request, response) => { response.success(request.user.get('data')); }); From 1f8ced205ea06d93bcc7d9701258ee3466a676ef Mon Sep 17 00:00:00 2001 From: Drew Gross Date: Fri, 20 May 2016 23:12:07 -0700 Subject: [PATCH 5/5] Still not transpiled --- spec/CloudCode.spec.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/CloudCode.spec.js b/spec/CloudCode.spec.js index 7dd29b49c9..8c2802d3e5 100644 --- a/spec/CloudCode.spec.js +++ b/spec/CloudCode.spec.js @@ -530,9 +530,9 @@ describe('Cloud Code', () => { } }, (error, response, body) => { Parse.Promise.all([cacheAdapter.get('test:user:' + session1), cacheAdapter.get('test:user:' + session2)]) - .then(([cacheVal1, cacheVal2]) => { - expect(cacheVal1.objectId).toEqual(user.id); - expect(cacheVal2.objectId).toEqual(user.id); + .then(cachedVals => { + expect(cachedVals[0].objectId).toEqual(user.id); + expect(cachedVals[1].objectId).toEqual(user.id); //Change with session 1 and then read with session 2. user.set('data', 'second data');