[web] Simplify removeSessionsFromRedis signature (#18440)

* Simplify `removeSessionsFromRedis` signature

* Update usage of `removeSessionsFromRedis`

* Fix tests around `removeSessionsFromRedis`

* Add comments "remove all sessions except the current session"

GitOrigin-RevId: 03bf99c14faf2c8e403bc4bcc16463a70e031284
This commit is contained in:
Antoine Clausse 2024-05-21 12:42:47 +02:00 committed by Copybot
parent 78a0bc2b05
commit 3300811d3a
7 changed files with 27 additions and 33 deletions

View file

@ -130,9 +130,10 @@ async function changePassword(req, res, next) {
// no need to wait, errors are logged and not passed back // no need to wait, errors are logged and not passed back
_sendSecurityAlertPasswordChanged(user) _sendSecurityAlertPasswordChanged(user)
await UserSessionsManager.promises.removeSessionsFromRedis(user, req, { await UserSessionsManager.promises.removeSessionsFromRedis(
stayLoggedIn: true, user,
}) req.sessionID // remove all sessions except the current session
)
await OneTimeTokenHandler.promises.expireAllTokensForUser( await OneTimeTokenHandler.promises.expireAllTokensForUser(
userId.toString(), userId.toString(),
@ -162,9 +163,10 @@ async function clearSessions(req, res, next) {
req.ip, req.ip,
{ sessions } { sessions }
) )
await UserSessionsManager.promises.removeSessionsFromRedis(user, req, { await UserSessionsManager.promises.removeSessionsFromRedis(
stayLoggedIn: true, user,
}) req.sessionID // remove all sessions except the current session
)
await _sendSecurityAlertClearedSessions(user) await _sendSecurityAlertClearedSessions(user)

View file

@ -512,8 +512,7 @@ const UserEmailsController = {
const user = SessionManager.getSessionUser(req.session) const user = SessionManager.getSessionUser(req.session)
UserSessionsManager.removeSessionsFromRedis( UserSessionsManager.removeSessionsFromRedis(
user, user,
req, req.sessionID, // remove all sessions except the current session
{ stayLoggedIn: true },
err => { err => {
if (err) if (err)
logger.warn( logger.warn(

View file

@ -66,8 +66,7 @@ async function settingsPage(req, res) {
// The user has just deleted their account. // The user has just deleted their account.
return UserSessionsManager.removeSessionsFromRedis( return UserSessionsManager.removeSessionsFromRedis(
{ _id: userId }, { _id: userId },
req, null,
{ stayLoggedIn: false },
() => res.redirect('/') () => res.redirect('/')
) )
} }

View file

@ -128,12 +128,10 @@ const UserSessionsManager = {
/** /**
* @param {{_id: string}} user * @param {{_id: string}} user
* @param {(import('express').Request & {sessionID?: string}) | undefined} req - the request object. Can be omitted if stayLoggedIn is false. * @param {string | null | undefined} retainSessionID - the session ID to exclude from deletion
* @param {{stayLoggedIn: boolean}?} options
* @param {(err: Error | null, data?: unknown) => void} callback * @param {(err: Error | null, data?: unknown) => void} callback
*/ */
removeSessionsFromRedis(user, req, options, callback) { removeSessionsFromRedis(user, retainSessionID, callback) {
const stayLoggedIn = options?.stayLoggedIn ?? false
if (!user) { if (!user) {
return callback(null) return callback(null)
} }
@ -146,13 +144,12 @@ const UserSessionsManager = {
}) })
return callback(err) return callback(err)
} }
const keysToDelete = const keysToDelete = retainSessionID
stayLoggedIn && req?.sessionID ? _.without(
? _.without( sessionKeys,
sessionKeys, UserSessionsManager._sessionKey(retainSessionID)
UserSessionsManager._sessionKey(req.sessionID) )
) : sessionKeys
: sessionKeys
if (keysToDelete.length === 0) { if (keysToDelete.length === 0) {
logger.debug( logger.debug(
{ userId: user._id }, { userId: user._id },
@ -248,8 +245,11 @@ const UserSessionsManager = {
UserSessionsManager.promises = { UserSessionsManager.promises = {
getAllUserSessions: promisify(UserSessionsManager.getAllUserSessions), getAllUserSessions: promisify(UserSessionsManager.getAllUserSessions),
removeSessionsFromRedis: (user, req = null, options = null) => removeSessionsFromRedis: (user, retainSessionID = null) =>
promisify(UserSessionsManager.removeSessionsFromRedis)(user, req, options), promisify(UserSessionsManager.removeSessionsFromRedis)(
user,
retainSessionID
),
untrackSession: promisify(UserSessionsManager.untrackSession), untrackSession: promisify(UserSessionsManager.untrackSession),
} }

View file

@ -40,7 +40,6 @@ function _handleUser(userId, callback) {
UserSessionsManager.removeSessionsFromRedis( UserSessionsManager.removeSessionsFromRedis(
{ _id: userId }, { _id: userId },
null, null,
null,
error => { error => {
if (error) { if (error) {
console.log(`Failed to clear sessions for ${userId}`, error) console.log(`Failed to clear sessions for ${userId}`, error)

View file

@ -566,7 +566,7 @@ describe('UserEmailsController', function () {
this.res.callback = () => { this.res.callback = () => {
expect( expect(
this.UserSessionsManager.removeSessionsFromRedis this.UserSessionsManager.removeSessionsFromRedis
).to.have.been.calledWith(this.user, this.req) ).to.have.been.calledWith(this.user, this.req.sessionID)
done() done()
} }

View file

@ -344,15 +344,13 @@ describe('UserSessionsManager', function () {
beforeEach(function () { beforeEach(function () {
this.sessionKeys = ['sess:one', 'sess:two'] this.sessionKeys = ['sess:one', 'sess:two']
this.currentSessionID = undefined this.currentSessionID = undefined
this.stayLoggedIn = false
this.rclient.smembers.callsArgWith(1, null, this.sessionKeys) this.rclient.smembers.callsArgWith(1, null, this.sessionKeys)
this.rclient.del = sinon.stub().callsArgWith(1, null) this.rclient.del = sinon.stub().callsArgWith(1, null)
this.rclient.srem = sinon.stub().callsArgWith(2, null) this.rclient.srem = sinon.stub().callsArgWith(2, null)
return (this.call = callback => { return (this.call = callback => {
return this.UserSessionsManager.removeSessionsFromRedis( return this.UserSessionsManager.removeSessionsFromRedis(
this.user, this.user,
{ sessionID: this.currentSessionID }, this.currentSessionID,
{ stayLoggedIn: this.stayLoggedIn },
callback callback
) )
}) })
@ -390,14 +388,12 @@ describe('UserSessionsManager', function () {
beforeEach(function () { beforeEach(function () {
this.sessionKeys = ['sess:one', 'sess:two', 'sess:three', 'sess:four'] this.sessionKeys = ['sess:one', 'sess:two', 'sess:three', 'sess:four']
this.currentSessionID = 'two' this.currentSessionID = 'two'
this.stayLoggedIn = true
this.rclient.smembers.callsArgWith(1, null, this.sessionKeys) this.rclient.smembers.callsArgWith(1, null, this.sessionKeys)
this.rclient.del = sinon.stub().callsArgWith(1, null) this.rclient.del = sinon.stub().callsArgWith(1, null)
return (this.call = callback => { return (this.call = callback => {
return this.UserSessionsManager.removeSessionsFromRedis( return this.UserSessionsManager.removeSessionsFromRedis(
this.user, this.user,
{ sessionID: this.currentSessionID }, this.currentSessionID,
{ stayLoggedIn: this.stayLoggedIn },
callback callback
) )
}) })
@ -463,8 +459,7 @@ describe('UserSessionsManager', function () {
return (this.call = callback => { return (this.call = callback => {
return this.UserSessionsManager.removeSessionsFromRedis( return this.UserSessionsManager.removeSessionsFromRedis(
null, null,
{ sessionID: this.currentSessionID }, this.currentSessionID,
{ stayLoggedIn: this.stayLoggedIn },
callback callback
) )
}) })