Merge pull request #2146 from overleaf/bg-fix-redis-user-session-expiry

fix expiry of user sessions in redis

GitOrigin-RevId: e53aa93bba99fe0cd632ac8bce36c0228e20d04a

services/web/app/src/Features/User/UserSessionsManager.js

@@ -45,7 +45,7 @@ module.exports = UserSessionsManager = {
     return rclient
       .multi()
       .sadd(sessionSetKey, value)
-      .expire(sessionSetKey, `${Settings.cookieSessionLength}`)
+      .pexpire(sessionSetKey, `${Settings.cookieSessionLength}`) // in milliseconds
       .exec(function(err, response) {
         if (err != null) {
           logger.warn(
@@ -77,7 +77,7 @@ module.exports = UserSessionsManager = {
     return rclient
       .multi()
       .srem(sessionSetKey, value)
-      .expire(sessionSetKey, `${Settings.cookieSessionLength}`)
+      .pexpire(sessionSetKey, `${Settings.cookieSessionLength}`) // in milliseconds
       .exec(function(err, response) {
         if (err != null) {
           logger.warn(
@@ -218,9 +218,9 @@ module.exports = UserSessionsManager = {
       return callback(null)
     }
     const sessionSetKey = UserSessionsRedis.sessionSetKey(user)
-    return rclient.expire(
+    return rclient.pexpire(
       sessionSetKey,
-      `${Settings.cookieSessionLength}`,
+      `${Settings.cookieSessionLength}`, // in milliseconds
       function(err, response) {
         if (err != null) {
           logger.warn(

services/web/app/src/infrastructure/Server.js

@@ -105,7 +105,7 @@ webRouter.use(
     proxy: Settings.behindProxy,
     cookie: {
       domain: Settings.cookieDomain,
-      maxAge: Settings.cookieSessionLength,
+      maxAge: Settings.cookieSessionLength, // in milliseconds, see https://github.com/expressjs/session#cookiemaxage
       secure: Settings.secureCookie
     },
     store: sessionStore,

services/web/test/unit/src/User/UserSessionsManagerTests.js

@@ -36,7 +36,7 @@ describe('UserSessionsManager', function() {
       srem: sinon.stub(),
       smembers: sinon.stub(),
       mget: sinon.stub(),
-      expire: sinon.stub()
+      pexpire: sinon.stub()
     }
     this.rclient.multi.returns(this.rclient)
     this.rclient.get.returns(this.rclient)
@@ -44,7 +44,7 @@ describe('UserSessionsManager', function() {
     this.rclient.sadd.returns(this.rclient)
     this.rclient.srem.returns(this.rclient)
     this.rclient.smembers.returns(this.rclient)
-    this.rclient.expire.returns(this.rclient)
+    this.rclient.pexpire.returns(this.rclient)
     this.rclient.exec.callsArgWith(0, null)
 
     this.UserSessionsRedis = {
@@ -112,7 +112,7 @@ describe('UserSessionsManager', function() {
       return this.call(err => {
         this.rclient.multi.callCount.should.equal(1)
         this.rclient.sadd.callCount.should.equal(1)
-        this.rclient.expire.callCount.should.equal(1)
+        this.rclient.pexpire.callCount.should.equal(1)
         this.rclient.exec.callCount.should.equal(1)
         return done()
       })
@@ -168,7 +168,7 @@ describe('UserSessionsManager', function() {
         return this.call(err => {
           this.rclient.multi.callCount.should.equal(0)
           this.rclient.sadd.callCount.should.equal(0)
-          this.rclient.expire.callCount.should.equal(0)
+          this.rclient.pexpire.callCount.should.equal(0)
           this.rclient.exec.callCount.should.equal(0)
           return done()
         })
@@ -205,7 +205,7 @@ describe('UserSessionsManager', function() {
         return this.call(err => {
           this.rclient.multi.callCount.should.equal(0)
           this.rclient.sadd.callCount.should.equal(0)
-          this.rclient.expire.callCount.should.equal(0)
+          this.rclient.pexpire.callCount.should.equal(0)
           this.rclient.exec.callCount.should.equal(0)
           return done()
         })
@@ -251,7 +251,7 @@ describe('UserSessionsManager', function() {
       return this.call(err => {
         this.rclient.multi.callCount.should.equal(1)
         this.rclient.srem.callCount.should.equal(1)
-        this.rclient.expire.callCount.should.equal(1)
+        this.rclient.pexpire.callCount.should.equal(1)
         this.rclient.exec.callCount.should.equal(1)
         return done()
       })
@@ -307,7 +307,7 @@ describe('UserSessionsManager', function() {
         return this.call(err => {
           this.rclient.multi.callCount.should.equal(0)
           this.rclient.srem.callCount.should.equal(0)
-          this.rclient.expire.callCount.should.equal(0)
+          this.rclient.pexpire.callCount.should.equal(0)
           this.rclient.exec.callCount.should.equal(0)
           return done()
         })
@@ -344,7 +344,7 @@ describe('UserSessionsManager', function() {
         return this.call(err => {
           this.rclient.multi.callCount.should.equal(0)
           this.rclient.srem.callCount.should.equal(0)
-          this.rclient.expire.callCount.should.equal(0)
+          this.rclient.pexpire.callCount.should.equal(0)
           this.rclient.exec.callCount.should.equal(0)
           return done()
         })
@@ -530,7 +530,7 @@ describe('UserSessionsManager', function() {
 
   describe('touch', function() {
     beforeEach(function() {
-      this.rclient.expire.callsArgWith(2, null)
+      this.rclient.pexpire.callsArgWith(2, null)
       return (this.call = callback => {
         return this.UserSessionsManager.touch(this.user, callback)
       })
@@ -544,16 +544,16 @@ describe('UserSessionsManager', function() {
       })
     })
 
-    it('should call rclient.expire', function(done) {
+    it('should call rclient.pexpire', function(done) {
       return this.call(err => {
-        this.rclient.expire.callCount.should.equal(1)
+        this.rclient.pexpire.callCount.should.equal(1)
         return done()
       })
     })
 
     describe('when rclient produces an error', function() {
       beforeEach(function() {
-        return this.rclient.expire.callsArgWith(2, new Error('woops'))
+        return this.rclient.pexpire.callsArgWith(2, new Error('woops'))
       })
 
       it('should produce an error', function(done) {
@@ -579,9 +579,9 @@ describe('UserSessionsManager', function() {
         })
       })
 
-      it('should not call expire', function(done) {
+      it('should not call pexpire', function(done) {
         return this.call(err => {
-          this.rclient.expire.callCount.should.equal(0)
+          this.rclient.pexpire.callCount.should.equal(0)
           return done()
         })
       })