Skip to content

Commit 6be356d

Browse files
authored
Fix intense CPU usage when sessionToken is invalid in liveQuery (parse-community#5126)
* Ensure we bail out early when auth or userId are not provided (sessionToken fetch is invalid) * Adds changelog * better handling of session token errors and client tokens
1 parent 0707580 commit 6be356d

File tree

3 files changed

+74
-37
lines changed

3 files changed

+74
-37
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
* Expire password reset tokens on email change. See #5104
1010
#### Bug fixes:
1111
* Fixes issue with vkontatke authentication
12+
* Improves performance for roles and ACL's in live query server
1213

1314

1415
### 3.0.0

spec/ParseLiveQueryServer.spec.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,14 @@ describe('ParseLiveQueryServer', function() {
9898
if (sessionToken === 'pleaseThrow') {
9999
return Promise.reject();
100100
}
101+
if (sessionToken === 'invalid') {
102+
return Promise.reject(
103+
new Parse.Error(
104+
Parse.Error.INVALID_SESSION_TOKEN,
105+
'invalid session token'
106+
)
107+
);
108+
}
101109
return Promise.resolve(
102110
new auth.Auth({ cacheController, user: { id: testUserId } })
103111
);
@@ -1629,6 +1637,17 @@ describe('ParseLiveQueryServer', function() {
16291637
expect(parseLiveQueryServer.authCache.get('pleaseThrow')).toBe(undefined);
16301638
});
16311639

1640+
it('should keep a cache of invalid sessions', async () => {
1641+
const parseLiveQueryServer = new ParseLiveQueryServer({});
1642+
const promise = parseLiveQueryServer.getAuthForSessionToken('invalid');
1643+
expect(parseLiveQueryServer.authCache.get('invalid')).toBe(promise);
1644+
// after the promise finishes, it should have removed it from the cache
1645+
await promise;
1646+
const finalResult = await parseLiveQueryServer.authCache.get('invalid');
1647+
expect(finalResult.error).not.toBeUndefined();
1648+
expect(parseLiveQueryServer.authCache.get('invalid')).not.toBe(undefined);
1649+
});
1650+
16321651
afterEach(function() {
16331652
jasmine.restoreLibrary(
16341653
'../lib/LiveQuery/ParseWebSocketServer',

src/LiveQuery/ParseLiveQueryServer.js

Lines changed: 54 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -420,11 +420,21 @@ class ParseLiveQueryServer {
420420
.then(auth => {
421421
return { auth, userId: auth && auth.user && auth.user.id };
422422
})
423-
.catch(() => {
424-
// If you can't continue, let's just wrap it up and delete it.
425-
// Next time, one will try again
426-
this.authCache.del(sessionToken);
427-
return {};
423+
.catch(error => {
424+
// There was an error with the session token
425+
const result = {};
426+
if (error && error.code === Parse.Error.INVALID_SESSION_TOKEN) {
427+
// Store a resolved promise with the error for 10 minutes
428+
result.error = error;
429+
this.authCache.set(
430+
sessionToken,
431+
Promise.resolve(result),
432+
60 * 10 * 1000
433+
);
434+
} else {
435+
this.authCache.del(sessionToken);
436+
}
437+
return result;
428438
});
429439
this.authCache.set(sessionToken, authPromise);
430440
return authPromise;
@@ -482,25 +492,19 @@ class ParseLiveQueryServer {
482492
: 'find';
483493
}
484494

485-
async _matchesACL(
486-
acl: any,
487-
client: any,
488-
requestId: number
489-
): Promise<boolean> {
490-
// Return true directly if ACL isn't present, ACL is public read, or client has master key
491-
if (!acl || acl.getPublicReadAccess() || client.hasMasterKey) {
492-
return true;
493-
}
494-
// Check subscription sessionToken matches ACL first
495-
const subscriptionInfo = client.getSubscriptionInfo(requestId);
496-
if (typeof subscriptionInfo === 'undefined') {
495+
async _verifyACL(acl: any, token: string) {
496+
if (!token) {
497497
return false;
498498
}
499499

500-
// TODO: get auth there and de-duplicate code below to work with the same Auth obj.
501-
const { auth, userId } = await this.getAuthForSessionToken(
502-
subscriptionInfo.sessionToken
503-
);
500+
const { auth, userId } = await this.getAuthForSessionToken(token);
501+
502+
// Getting the session token failed
503+
// This means that no additional auth is available
504+
// At this point, just bail out as no additional visibility can be inferred.
505+
if (!auth || !userId) {
506+
return false;
507+
}
504508
const isSubscriptionSessionTokenMatched = acl.getReadAccess(userId);
505509
if (isSubscriptionSessionTokenMatched) {
506510
return true;
@@ -527,27 +531,40 @@ class ParseLiveQueryServer {
527531
}
528532
return false;
529533
})
530-
.then(async isRoleMatched => {
531-
if (isRoleMatched) {
532-
return Promise.resolve(true);
533-
}
534-
535-
// Check client sessionToken matches ACL
536-
const clientSessionToken = client.sessionToken;
537-
if (clientSessionToken) {
538-
const { userId } = await this.getAuthForSessionToken(
539-
clientSessionToken
540-
);
541-
return acl.getReadAccess(userId);
542-
} else {
543-
return isRoleMatched;
544-
}
545-
})
546534
.catch(() => {
547535
return false;
548536
});
549537
}
550538

539+
async _matchesACL(
540+
acl: any,
541+
client: any,
542+
requestId: number
543+
): Promise<boolean> {
544+
// Return true directly if ACL isn't present, ACL is public read, or client has master key
545+
if (!acl || acl.getPublicReadAccess() || client.hasMasterKey) {
546+
return true;
547+
}
548+
// Check subscription sessionToken matches ACL first
549+
const subscriptionInfo = client.getSubscriptionInfo(requestId);
550+
if (typeof subscriptionInfo === 'undefined') {
551+
return false;
552+
}
553+
554+
const subscriptionToken = subscriptionInfo.sessionToken;
555+
const clientSessionToken = client.sessionToken;
556+
557+
if (await this._verifyACL(acl, subscriptionToken)) {
558+
return true;
559+
}
560+
561+
if (await this._verifyACL(acl, clientSessionToken)) {
562+
return true;
563+
}
564+
565+
return false;
566+
}
567+
551568
_handleConnect(parseWebsocket: any, request: any): any {
552569
if (!this._validateKeys(request, this.keyPairs)) {
553570
Client.pushError(parseWebsocket, 4, 'Key in request is not valid');

0 commit comments

Comments
 (0)