From fa4c049a1fb80ea36b22f8d0649cd073b838050f Mon Sep 17 00:00:00 2001 From: Florent Vilmart <364568+flovilmart@users.noreply.github.com> Date: Wed, 17 Oct 2018 21:19:27 -0400 Subject: [PATCH 1/3] Ensure we bail out early when auth or userId are not provided (sessionToken fetch is invalid) --- src/LiveQuery/ParseLiveQueryServer.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/LiveQuery/ParseLiveQueryServer.js b/src/LiveQuery/ParseLiveQueryServer.js index 27157de5be..aea85f45fe 100644 --- a/src/LiveQuery/ParseLiveQueryServer.js +++ b/src/LiveQuery/ParseLiveQueryServer.js @@ -497,10 +497,16 @@ class ParseLiveQueryServer { return false; } - // TODO: get auth there and de-duplicate code below to work with the same Auth obj. const { auth, userId } = await this.getAuthForSessionToken( subscriptionInfo.sessionToken ); + + // Getting the session token failed + // This means that no additional auth is available + // At this point, just bail out as no additional visibility can be inferred. + if (!auth || !userId) { + return false; + } const isSubscriptionSessionTokenMatched = acl.getReadAccess(userId); if (isSubscriptionSessionTokenMatched) { return true; From 410a3952d13da9f883024b7dcbb34f4d4bfef6a5 Mon Sep 17 00:00:00 2001 From: Florent Vilmart <364568+flovilmart@users.noreply.github.com> Date: Wed, 17 Oct 2018 21:22:14 -0400 Subject: [PATCH 2/3] Adds changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e754f07cf..fc2dc9ef11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ * Expire password reset tokens on email change. See #5104 #### Bug fixes: * Fixes issue with vkontatke authentication +* Improves performance for roles and ACL's in live query server ### 3.0.0 From dc97391d68e33933aa39539ba1cef9c81d9dd1bd Mon Sep 17 00:00:00 2001 From: Florent Vilmart <364568+flovilmart@users.noreply.github.com> Date: Wed, 17 Oct 2018 21:52:38 -0400 Subject: [PATCH 3/3] better handling of session token errors and client tokens --- spec/ParseLiveQueryServer.spec.js | 19 ++++++ src/LiveQuery/ParseLiveQueryServer.js | 83 +++++++++++++++------------ 2 files changed, 66 insertions(+), 36 deletions(-) diff --git a/spec/ParseLiveQueryServer.spec.js b/spec/ParseLiveQueryServer.spec.js index bb839d6d6d..6def159240 100644 --- a/spec/ParseLiveQueryServer.spec.js +++ b/spec/ParseLiveQueryServer.spec.js @@ -98,6 +98,14 @@ describe('ParseLiveQueryServer', function() { if (sessionToken === 'pleaseThrow') { return Promise.reject(); } + if (sessionToken === 'invalid') { + return Promise.reject( + new Parse.Error( + Parse.Error.INVALID_SESSION_TOKEN, + 'invalid session token' + ) + ); + } return Promise.resolve( new auth.Auth({ cacheController, user: { id: testUserId } }) ); @@ -1629,6 +1637,17 @@ describe('ParseLiveQueryServer', function() { expect(parseLiveQueryServer.authCache.get('pleaseThrow')).toBe(undefined); }); + it('should keep a cache of invalid sessions', async () => { + const parseLiveQueryServer = new ParseLiveQueryServer({}); + const promise = parseLiveQueryServer.getAuthForSessionToken('invalid'); + expect(parseLiveQueryServer.authCache.get('invalid')).toBe(promise); + // after the promise finishes, it should have removed it from the cache + await promise; + const finalResult = await parseLiveQueryServer.authCache.get('invalid'); + expect(finalResult.error).not.toBeUndefined(); + expect(parseLiveQueryServer.authCache.get('invalid')).not.toBe(undefined); + }); + afterEach(function() { jasmine.restoreLibrary( '../lib/LiveQuery/ParseWebSocketServer', diff --git a/src/LiveQuery/ParseLiveQueryServer.js b/src/LiveQuery/ParseLiveQueryServer.js index aea85f45fe..b175ebe36a 100644 --- a/src/LiveQuery/ParseLiveQueryServer.js +++ b/src/LiveQuery/ParseLiveQueryServer.js @@ -420,11 +420,21 @@ class ParseLiveQueryServer { .then(auth => { return { auth, userId: auth && auth.user && auth.user.id }; }) - .catch(() => { - // If you can't continue, let's just wrap it up and delete it. - // Next time, one will try again - this.authCache.del(sessionToken); - return {}; + .catch(error => { + // There was an error with the session token + const result = {}; + if (error && error.code === Parse.Error.INVALID_SESSION_TOKEN) { + // Store a resolved promise with the error for 10 minutes + result.error = error; + this.authCache.set( + sessionToken, + Promise.resolve(result), + 60 * 10 * 1000 + ); + } else { + this.authCache.del(sessionToken); + } + return result; }); this.authCache.set(sessionToken, authPromise); return authPromise; @@ -482,24 +492,12 @@ class ParseLiveQueryServer { : 'find'; } - async _matchesACL( - acl: any, - client: any, - requestId: number - ): Promise { - // Return true directly if ACL isn't present, ACL is public read, or client has master key - if (!acl || acl.getPublicReadAccess() || client.hasMasterKey) { - return true; - } - // Check subscription sessionToken matches ACL first - const subscriptionInfo = client.getSubscriptionInfo(requestId); - if (typeof subscriptionInfo === 'undefined') { + async _verifyACL(acl: any, token: string) { + if (!token) { return false; } - const { auth, userId } = await this.getAuthForSessionToken( - subscriptionInfo.sessionToken - ); + const { auth, userId } = await this.getAuthForSessionToken(token); // Getting the session token failed // This means that no additional auth is available @@ -533,27 +531,40 @@ class ParseLiveQueryServer { } return false; }) - .then(async isRoleMatched => { - if (isRoleMatched) { - return Promise.resolve(true); - } - - // Check client sessionToken matches ACL - const clientSessionToken = client.sessionToken; - if (clientSessionToken) { - const { userId } = await this.getAuthForSessionToken( - clientSessionToken - ); - return acl.getReadAccess(userId); - } else { - return isRoleMatched; - } - }) .catch(() => { return false; }); } + async _matchesACL( + acl: any, + client: any, + requestId: number + ): Promise { + // Return true directly if ACL isn't present, ACL is public read, or client has master key + if (!acl || acl.getPublicReadAccess() || client.hasMasterKey) { + return true; + } + // Check subscription sessionToken matches ACL first + const subscriptionInfo = client.getSubscriptionInfo(requestId); + if (typeof subscriptionInfo === 'undefined') { + return false; + } + + const subscriptionToken = subscriptionInfo.sessionToken; + const clientSessionToken = client.sessionToken; + + if (await this._verifyACL(acl, subscriptionToken)) { + return true; + } + + if (await this._verifyACL(acl, clientSessionToken)) { + return true; + } + + return false; + } + _handleConnect(parseWebsocket: any, request: any): any { if (!this._validateKeys(request, this.keyPairs)) { Client.pushError(parseWebsocket, 4, 'Key in request is not valid');