diff --git a/src/node/db/SessionStore.js b/src/node/db/SessionStore.js index ec54d8f8c..195233f28 100644 --- a/src/node/db/SessionStore.js +++ b/src/node/db/SessionStore.js @@ -8,33 +8,89 @@ const util = require('util'); const logger = log4js.getLogger('SessionStore'); class SessionStore extends Store { - async _checkExpiration(sid, sess) { + /** + * @param {?number} [refresh] - How often (in milliseconds) `touch()` will update a session's + * database record with the cookie's latest expiration time. If the difference between the + * value saved in the database and the actual value is greater than this amount, the database + * record will be updated to reflect the actual value. Use this to avoid continual database + * writes caused by express-session's rolling=true feature (see + * https://github.com/expressjs/session#rolling). A good value is high enough to keep query + * rate low but low enough to avoid annoying premature logouts (session invalidation) if + * Etherpad is restarted. Use `null` to prevent `touch()` from ever updating the record. + * Ignored if the cookie does not expire. + */ + constructor(refresh = null) { + super(); + this._refresh = refresh; + // Maps session ID to an object with the following properties: + // - `db`: Session expiration as recorded in the database (ms since epoch, not a Date). + // - `real`: Actual session expiration (ms since epoch, not a Date). Always greater than or + // equal to `db`. + this._expirations = new Map(); + } + + async _updateExpirations(sid, sess, updateDbExp = true) { + const exp = this._expirations.get(sid) || {}; const {cookie: {expires} = {}} = sess || {}; - if (expires && new Date() >= new Date(expires)) return await this._destroy(sid); + if (expires) { + const sessExp = new Date(expires).getTime(); + if (updateDbExp) exp.db = sessExp; + exp.real = Math.max(exp.real || 0, exp.db || 0, sessExp); + const now = Date.now(); + if (exp.real <= now) return await this._destroy(sid); + // If reading from the database, update the expiration with the latest value from touch() so + // that touch() appears to write to the database every time even though it doesn't. + if (typeof expires === 'string') sess.cookie.expires = new Date(exp.real).toJSON(); + this._expirations.set(sid, exp); + } else { + this._expirations.delete(sid); + } return sess; } + async _write(sid, sess) { + await DB.set(`sessionstorage:${sid}`, sess); + } + async _get(sid) { logger.debug(`GET ${sid}`); const s = await DB.get(`sessionstorage:${sid}`); - return await this._checkExpiration(sid, s); + return await this._updateExpirations(sid, s); } async _set(sid, sess) { logger.debug(`SET ${sid}`); - sess = await this._checkExpiration(sid, sess); - if (sess != null) await DB.set(`sessionstorage:${sid}`, sess); + sess = await this._updateExpirations(sid, sess); + if (sess != null) await this._write(sid, sess); } async _destroy(sid) { logger.debug(`DESTROY ${sid}`); + this._expirations.delete(sid); await DB.remove(`sessionstorage:${sid}`); } + + // Note: express-session might call touch() before it calls set() for the first time. Ideally this + // would behave like set() in that case but it's OK if it doesn't -- express-session will call + // set() soon enough. + async _touch(sid, sess) { + logger.debug(`TOUCH ${sid}`); + sess = await this._updateExpirations(sid, sess, false); + if (sess == null) return; // Already expired. + const exp = this._expirations.get(sid); + // If the session doesn't expire, don't do anything. Ideally we would write the session to the + // database if it didn't already exist, but we have no way of knowing that without querying the + // database. The query overhead is not worth it because set() should be called soon anyway. + if (exp == null) return; + if (exp.db != null && (this._refresh == null || exp.real < exp.db + this._refresh)) return; + await this._write(sid, sess); + exp.db = new Date(sess.cookie.expires).getTime(); + } } // express-session doesn't support Promise-based methods. This is where the callbackified versions // used by express-session are defined. -for (const m of ['get', 'set', 'destroy']) { +for (const m of ['get', 'set', 'destroy', 'touch']) { SessionStore.prototype[m] = util.callbackify(SessionStore.prototype[`_${m}`]); } diff --git a/src/tests/backend/specs/SessionStore.js b/src/tests/backend/specs/SessionStore.js index 7849f3529..26460c9d6 100644 --- a/src/tests/backend/specs/SessionStore.js +++ b/src/tests/backend/specs/SessionStore.js @@ -13,6 +13,7 @@ describe(__filename, function () { const set = async (sess) => await util.promisify(ss.set).call(ss, sid, sess); const get = async () => await util.promisify(ss.get).call(ss, sid); const destroy = async () => await util.promisify(ss.destroy).call(ss, sid); + const touch = async (sess) => await util.promisify(ss.touch).call(ss, sid, sess); before(async function () { await common.init(); @@ -119,4 +120,80 @@ describe(__filename, function () { await destroy(); }); }); + + describe('touch without refresh', function () { + it('touch before set is equivalent to set if session expires', async function () { + const sess = {cookie: {expires: new Date(Date.now() + 1000)}}; + await touch(sess); + assert.equal(JSON.stringify(await get()), JSON.stringify(sess)); + }); + + it('touch updates observed expiration but not database', async function () { + const start = Date.now(); + const sess = {cookie: {expires: new Date(start + 200)}}; + await set(sess); + const sess2 = {cookie: {expires: new Date(start + 12000)}}; + await touch(sess2); + assert.equal(JSON.stringify(await db.get(`sessionstorage:${sid}`)), JSON.stringify(sess)); + assert.equal(JSON.stringify(await get()), JSON.stringify(sess2)); + }); + }); + + describe('touch with refresh', function () { + beforeEach(async function () { + ss = new SessionStore(200); + }); + + it('touch before set is equivalent to set if session expires', async function () { + const sess = {cookie: {expires: new Date(Date.now() + 1000)}}; + await touch(sess); + assert.equal(JSON.stringify(await get()), JSON.stringify(sess)); + }); + + it('touch before eligible for refresh updates expiration but not DB', async function () { + const now = Date.now(); + const sess = {foo: 'bar', cookie: {expires: new Date(now + 1000)}}; + await set(sess); + const sess2 = {foo: 'bar', cookie: {expires: new Date(now + 1001)}}; + await touch(sess2); + assert.equal(JSON.stringify(await db.get(`sessionstorage:${sid}`)), JSON.stringify(sess)); + assert.equal(JSON.stringify(await get()), JSON.stringify(sess2)); + }); + + it('touch before eligible for refresh updates timeout', async function () { + const start = Date.now(); + const sess = {foo: 'bar', cookie: {expires: new Date(start + 200)}}; + await set(sess); + await new Promise((resolve) => setTimeout(resolve, 100)); + const sess2 = {foo: 'bar', cookie: {expires: new Date(start + 399)}}; + await touch(sess2); + await new Promise((resolve) => setTimeout(resolve, 110)); + assert.equal(JSON.stringify(await db.get(`sessionstorage:${sid}`)), JSON.stringify(sess)); + assert.equal(JSON.stringify(await get()), JSON.stringify(sess2)); + }); + + it('touch after eligible for refresh updates db', async function () { + const start = Date.now(); + const sess = {foo: 'bar', cookie: {expires: new Date(start + 200)}}; + await set(sess); + await new Promise((resolve) => setTimeout(resolve, 100)); + const sess2 = {foo: 'bar', cookie: {expires: new Date(start + 400)}}; + await touch(sess2); + await new Promise((resolve) => setTimeout(resolve, 110)); + assert.equal(JSON.stringify(await db.get(`sessionstorage:${sid}`)), JSON.stringify(sess2)); + assert.equal(JSON.stringify(await get()), JSON.stringify(sess2)); + }); + + it('refresh=0 updates db every time', async function () { + ss = new SessionStore(0); + const sess = {foo: 'bar', cookie: {expires: new Date(Date.now() + 1000)}}; + await set(sess); + await db.remove(`sessionstorage:${sid}`); + await touch(sess); // No change in expiration time. + assert.equal(JSON.stringify(await db.get(`sessionstorage:${sid}`)), JSON.stringify(sess)); + await db.remove(`sessionstorage:${sid}`); + await touch(sess); // No change in expiration time. + assert.equal(JSON.stringify(await db.get(`sessionstorage:${sid}`)), JSON.stringify(sess)); + }); + }); });