Merge pull request #5484 from overleaf/tm-show-current-session

Show current session on user sessions page

GitOrigin-RevId: 6ae130bfa8c3d82a305fd865e162c19f5c8b208c

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

@@ -22,6 +22,7 @@ const { expressify } = require('../../util/promises')
 const {
   acceptsJson,
 } = require('../../infrastructure/RequestContentTypeDetection')
+const _ = require('lodash')
 
 async function _sendSecurityAlertClearedSessions(user) {
   const emailOptions = {
@@ -132,7 +133,11 @@ async function clearSessions(req, res, next) {
     'clear-sessions',
     user._id,
     req.ip,
-    { sessions }
+    {
+      sessions: sessions.map(
+        session => _.pick(session, ['ip_address', 'session_created']) // omit other session data from log
+      ),
+    }
   )
   await UserSessionsManager.promises.revokeAllUserSessions(user, [
     req.sessionID,

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

@@ -137,22 +137,19 @@ const UserPagesController = {
   sessionsPage(req, res, next) {
     const user = SessionManager.getSessionUser(req.session)
     logger.log({ userId: user._id }, 'loading sessions page')
-    UserSessionsManager.getAllUserSessions(
-      user,
-      [req.sessionID],
-      (err, sessions) => {
-        if (err != null) {
-          OError.tag(err, 'error getting all user sessions', {
-            userId: user._id,
-          })
-          return next(err)
-        }
-        res.render('user/sessions', {
-          title: 'sessions',
-          sessions,
+    UserSessionsManager.getAllUserSessions(user, (err, sessions) => {
+      if (err != null) {
+        OError.tag(err, 'error getting all user sessions', {
+          userId: user._id,
         })
+        return next(err)
       }
-    )
+      res.render('user/sessions', {
+        title: 'sessions',
+        sessions: sessions.filter(session => session.id !== req.sessionID),
+        currentSession: sessions.find(session => session.id === req.sessionID),
+      })
+    })
   },
 
   _restructureThirdPartyIds(user) {

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

@@ -77,6 +77,10 @@ const UserSessionsManager = {
   },
 
   getAllUserSessions(user, exclude, callback) {
+    if (typeof exclude === 'function') {
+      callback = exclude
+      exclude = []
+    }
     exclude = _.map(exclude, UserSessionsManager._sessionKey)
     const sessionSetKey = UserSessionsRedis.sessionSetKey(user)
     rclient.smembers(sessionSetKey, function (err, sessionKeys) {
@@ -94,7 +98,14 @@ const UserSessionsManager = {
 
       Async.mapSeries(
         sessionKeys,
-        (k, cb) => rclient.get(k, cb),
+        (k, cb) => {
+          rclient.get(k, (err, res) => {
+            if (err) {
+              return cb(err)
+            }
+            cb(null, { id: k, data: res })
+          })
+        },
         function (err, sessions) {
           if (err) {
             OError.tag(err, 'error getting all sessions for user from redis', {
@@ -104,17 +115,18 @@ const UserSessionsManager = {
           }
 
           const result = []
-          for (let session of Array.from(sessions)) {
+          for (const session of Array.from(sessions)) {
             if (!session) {
               continue
             }
-            session = JSON.parse(session)
-            let sessionUser = session.passport && session.passport.user
+            const sessionData = JSON.parse(session.data)
+            let sessionUser = sessionData.passport && sessionData.passport.user
             if (!sessionUser) {
-              sessionUser = session.user
+              sessionUser = sessionData.user
             }
 
             result.push({
+              id: session.id.replace('sess:', ''),
               ip_address: sessionUser.ip_address,
               session_created: sessionUser.session_created,
             })

services/web/app/views/user/sessions.pug

@@ -9,6 +9,19 @@ block content
 						.page-header
 							h1  #{translate("your_sessions")}
 
+						if currentSession
+							h3 #{translate("current_session")}
+							div
+								table.table.table-striped
+									thead
+										tr
+											th #{translate("ip_address")}
+											th #{translate("session_created_at")}
+										tr
+											td #{currentSession.ip_address}
+											td #{moment(currentSession.session_created).utc().format('Do MMM YYYY, h:mm a')} UTC
+
+						h3 #{translate("other_sessions")}
 						div
 							p.small
 								| !{translate("clear_sessions_description")}

services/web/locales/en.json

@@ -625,6 +625,8 @@
   "clear_sessions_success": "Sessions Cleared",
   "sessions": "Sessions",
   "manage_sessions": "Manage Your Sessions",
+  "current_session": "Current Session",
+  "other_sessions": "Other Sessions",
   "syntax_validation": "Code check",
   "history": "History",
   "joining": "Joining",

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

@@ -74,7 +74,7 @@ describe('UserController', function () {
       untrackSession: sinon.stub(),
       revokeAllUserSessions: sinon.stub().callsArgWith(2, null),
       promises: {
-        getAllUserSessions: sinon.stub().resolves(),
+        getAllUserSessions: sinon.stub().resolves([]),
         revokeAllUserSessions: sinon.stub().resolves(),
       },
     }
@@ -621,6 +621,25 @@ describe('UserController', function () {
 
         this.UserController.clearSessions(this.req, this.res)
       })
+
+      it('should include only relevant session data in the audit log', function (done) {
+        this.UserSessionsManager.promises.getAllUserSessions.resolves([
+          { id: 'session-id', ip_address: 'ip', session_created: 'created' },
+        ])
+        this.res.sendStatus.callsFake(status => {
+          this.UserAuditLogHandler.promises.addEntry.callCount.should.equal(1)
+          const addEntryCall = this.UserAuditLogHandler.promises.addEntry
+            .lastCall
+          expect(addEntryCall.args[4].sessions).to.be.instanceOf(Array)
+          expect(addEntryCall.args[4].sessions[0]).to.have.keys([
+            'ip_address',
+            'session_created',
+          ])
+          expect(addEntryCall.args[4].sessions[0]).to.not.have.keys(['id'])
+          done()
+        })
+        this.UserController.clearSessions(this.req, this.res)
+      })
     })
 
     describe('errors', function () {

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

@@ -154,7 +154,7 @@ describe('UserPagesController', function () {
   describe('sessionsPage', function () {
     beforeEach(function () {
       return this.UserSessionsManager.getAllUserSessions.callsArgWith(
-        2,
+        1,
         null,
         []
       )
@@ -179,7 +179,7 @@ describe('UserPagesController', function () {
     describe('when getAllUserSessions produces an error', function () {
       beforeEach(function () {
         return this.UserSessionsManager.getAllUserSessions.callsArgWith(
-          2,
+          1,
           new Error('woops')
         )
       })

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

@@ -606,8 +606,8 @@ describe('UserSessionsManager', function () {
     it('should get sessions', function (done) {
       return this.call((err, sessions) => {
         expect(sessions).to.deep.equal([
-          { ip_address: 'a', session_created: 'b' },
-          { ip_address: 'c', session_created: 'd' },
+          { id: 'one', ip_address: 'a', session_created: 'b' },
+          { id: 'three', ip_address: 'c', session_created: 'd' },
         ])
         return done()
       })