Skip to content

SessionToken cache for invalid sessions tokens #4926

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 8 commits into from
Closed

SessionToken cache for invalid sessions tokens #4926

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
01b772a
invalid session tokens blacklisted
flavionegrao Aug 3, 2018
d7511a8
Invalid sessions tokens tests
flavionegrao Aug 3, 2018
8062f49
unit test adjusts
flavionegrao Aug 3, 2018
4cd9511
better comments
flavionegrao Aug 3, 2018
3e9a0f9
Cache for Parse.Roles added
flavionegrao Aug 7, 2018
92f846a
defaut timeout adjusted
flavionegrao Aug 7, 2018
82ea7c6
bug fix
flavionegrao Aug 7, 2018
378442a
bugfix
flavionegrao Aug 7, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 57 additions & 31 deletions spec/SessionTokenCache.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,49 +2,75 @@ const SessionTokenCache = require('../lib/LiveQuery/SessionTokenCache').SessionT

describe('SessionTokenCache', function() {

beforeEach(function(done) {
const Parse = require('parse/node');
describe('Valid session', function() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned before, can we please not change all tests this way? But add new tests as we go?


spyOn(Parse, "Query").and.returnValue({
first: jasmine.createSpy("first").and.returnValue(Parse.Promise.as(new Parse.Object("_Session", {
user: new Parse.User({id:"userId"})
}))),
equalTo: function(){}
})
beforeEach(function(done) {
const Parse = require('parse/node');

done();
});

it('can get undefined userId', function(done) {
const sessionTokenCache = new SessionTokenCache();
spyOn(Parse, "Query").and.returnValue({
first: jasmine.createSpy("first").and.returnValue(Parse.Promise.as(new Parse.Object("_Session", {
user: new Parse.User({
id: "userId"
})
}))),
equalTo: function() {}
})

sessionTokenCache.getUserId(undefined).then(() => {
}, (error) => {
expect(error).not.toBeNull();
done();
});
});

it('can get existing userId', function(done) {
const sessionTokenCache = new SessionTokenCache();
const sessionToken = 'sessionToken';
const userId = 'userId'
sessionTokenCache.cache.set(sessionToken, userId);
it('can get undefined userId', function(done) {
const sessionTokenCache = new SessionTokenCache();

sessionTokenCache.getUserId(sessionToken).then((userIdFromCache) => {
expect(userIdFromCache).toBe(userId);
done();
sessionTokenCache.getUserId(undefined).then(() => {}, (error) => {
expect(error).not.toBeNull();
done();
});
});

it('can get existing userId', function(done) {
const sessionTokenCache = new SessionTokenCache();
const sessionToken = 'sessionToken';
const userId = 'userId'
sessionTokenCache.cache.set(sessionToken, userId);

sessionTokenCache.getUserId(sessionToken).then((userIdFromCache) => {
expect(userIdFromCache).toBe(userId);
done();
});
});

it('can get new userId', function(done) {
const sessionTokenCache = new SessionTokenCache();

sessionTokenCache.getUserId('sessionToken').then((userIdFromCache) => {
expect(userIdFromCache).toBe('userId');
expect(sessionTokenCache.cache.length).toBe(1);
done();
});
});
});

it('can get new userId', function(done) {
const sessionTokenCache = new SessionTokenCache();
describe('Invalid session', function() {
beforeEach(function(done) {
const Parse = require('parse/node');

spyOn(Parse, "Query").and.returnValue({
first: jasmine.createSpy("first").and.returnValue(Parse.Promise.as(undefined)),
equalTo: function() {}
})

sessionTokenCache.getUserId('sessionToken').then((userIdFromCache) => {
expect(userIdFromCache).toBe('userId');
expect(sessionTokenCache.cache.length).toBe(1);
done();
});
});
fit('can get inexisting userId', function(done) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, this blocks the tests from executing

const sessionTokenCache = new SessionTokenCache();
const sessionToken = 'sessionToken';
sessionTokenCache.cache.set(sessionToken, undefined);

sessionTokenCache.getUserId(sessionToken).then(() => {}, (error) => {
expect(error).not.toBeNull();
done();
});
});
})
});
34 changes: 22 additions & 12 deletions src/LiveQuery/SessionTokenCache.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,16 @@ import Parse from 'parse/node';
import LRU from 'lru-cache';
import logger from '../logger';

function userForSessionToken(sessionToken){
function userIdForSessionToken(sessionToken) {
var q = new Parse.Query("_Session");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need to change this method?

q.equalTo("sessionToken", sessionToken);
return q.first({useMasterKey:true}).then(function(session){
if(!session){
return Parse.Promise.error("No session found for session token");
return q.first({useMasterKey: true}).then(session => {
if (!session) {
logger.verbose("No session found for session token");
return;
}
return session.get("user");
const user = session.get("user");
return user.id;
});
}

Expand All @@ -27,14 +29,22 @@ class SessionTokenCache {
if (!sessionToken) {
return Parse.Promise.error('Empty sessionToken');
}
const userId = this.cache.get(sessionToken);
if (userId) {
logger.verbose('Fetch userId %s of sessionToken %s from Cache', userId, sessionToken);
return Parse.Promise.as(userId);

if (this.cache.has(sessionToken)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic may be problematic, if we need to invalidate a session we should mark it INVALID_SESSION_TOKEN.

Also I’m not sure that the solution is the right one as every time a session will be missing, we’ll interpret as invalid. I’d rather add a Boolean onlyFromCache = false of we don’t want to poke the server.

const userId = this.cache.get(sessionToken);
if (userId) {
logger.verbose('Fetch userId %s of sessionToken %s from Cache', userId, sessionToken);
return Parse.Promise.as(userId);
} else {
// invalid session tokens are set as undefined in the LRU
// it will avoid quering the parse servers for users too often
// with inexistent sessionsToken
return Parse.Promise.error('Invalid sessionToken');
}
}
return userForSessionToken(sessionToken).then((user) => {
logger.verbose('Fetch userId %s of sessionToken %s from Parse', user.id, sessionToken);
const userId = user.id;

return userIdForSessionToken(sessionToken).then((userId) => {
logger.verbose('Fetch userId %s of sessionToken %s from Parse', userId, sessionToken);
this.cache.set(sessionToken, userId);
return Parse.Promise.as(userId);
}, (error) => {
Expand Down