Browse Source

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

Revert "Show current session on user sessions page"

GitOrigin-RevId: 80e4c667d96b2016066657dc74d9f27d6b52b6f8
Thomas 5 years ago
parent
commit
42fa5e28ed

+ 1 - 6
services/web/app/src/Features/User/UserController.js

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

+ 14 - 11
services/web/app/src/Features/User/UserPagesController.js

@@ -137,19 +137,22 @@ const UserPagesController = {
   sessionsPage(req, res, next) {
     const user = SessionManager.getSessionUser(req.session)
     logger.log({ userId: user._id }, 'loading sessions page')
-    UserSessionsManager.getAllUserSessions(user, (err, sessions) => {
-      if (err != null) {
-        OError.tag(err, 'error getting all user sessions', {
-          userId: user._id,
+    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,
         })
-        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) {

+ 5 - 17
services/web/app/src/Features/User/UserSessionsManager.js

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

+ 0 - 13
services/web/app/views/user/sessions.pug

@@ -9,19 +9,6 @@ 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")}

+ 0 - 2
services/web/locales/en.json

@@ -625,8 +625,6 @@
   "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",

+ 1 - 20
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,25 +621,6 @@ 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 () {

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

+ 2 - 2
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([
-          { id: 'one', ip_address: 'a', session_created: 'b' },
-          { id: 'three', ip_address: 'c', session_created: 'd' },
+          { ip_address: 'a', session_created: 'b' },
+          { ip_address: 'c', session_created: 'd' },
         ])
         return done()
       })