diff --git a/services/web/app/src/Features/User/UserController.js b/services/web/app/src/Features/User/UserController.js index a581fd39cf..4e016e499e 100644 --- a/services/web/app/src/Features/User/UserController.js +++ b/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, diff --git a/services/web/app/src/Features/User/UserPagesController.js b/services/web/app/src/Features/User/UserPagesController.js index a653164033..975f13ac1b 100644 --- a/services/web/app/src/Features/User/UserPagesController.js +++ b/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) { diff --git a/services/web/app/src/Features/User/UserSessionsManager.js b/services/web/app/src/Features/User/UserSessionsManager.js index a4f2b73575..ce1abf1f0c 100644 --- a/services/web/app/src/Features/User/UserSessionsManager.js +++ b/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, }) diff --git a/services/web/app/views/user/sessions.pug b/services/web/app/views/user/sessions.pug index 45755fcf2a..708a909849 100644 --- a/services/web/app/views/user/sessions.pug +++ b/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")} diff --git a/services/web/locales/en.json b/services/web/locales/en.json index c93ac876fd..5b3300088a 100644 --- a/services/web/locales/en.json +++ b/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", diff --git a/services/web/test/unit/src/User/UserControllerTests.js b/services/web/test/unit/src/User/UserControllerTests.js index a5a4c46555..77cf549500 100644 --- a/services/web/test/unit/src/User/UserControllerTests.js +++ b/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 () { diff --git a/services/web/test/unit/src/User/UserPagesControllerTests.js b/services/web/test/unit/src/User/UserPagesControllerTests.js index b0dfb56e1c..cc60bb104e 100644 --- a/services/web/test/unit/src/User/UserPagesControllerTests.js +++ b/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') ) }) diff --git a/services/web/test/unit/src/User/UserSessionsManagerTests.js b/services/web/test/unit/src/User/UserSessionsManagerTests.js index e3615b6fa3..ea5e354000 100644 --- a/services/web/test/unit/src/User/UserSessionsManagerTests.js +++ b/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() })