SessionStore: Option to update DB record on touch()

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);
+    sess = await this._updateExpirations(sid, sess);
-    if (sess != null) await DB.set(`sessionstorage:${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}`]);
 }
 

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));
+    });
+  });
 });