diff --git a/src/controllers/authentication.js b/src/controllers/authentication.js index b3949905d4..bbffc9070a 100644 --- a/src/controllers/authentication.js +++ b/src/controllers/authentication.js @@ -380,7 +380,7 @@ authenticationController.onSuccessfulLogin = async function (req, uid) { new Promise((resolve) => { req.session.save(resolve); }), - user.auth.addSession(uid, req.sessionID, uuid), + user.auth.addSession(uid, req.sessionID), user.updateLastOnlineTime(uid), user.onUserOnline(uid, Date.now()), analytics.increment('logins'), diff --git a/src/upgrades/3.8.3/remove-session-uuid.js b/src/upgrades/3.8.3/remove-session-uuid.js new file mode 100644 index 0000000000..59a975fce2 --- /dev/null +++ b/src/upgrades/3.8.3/remove-session-uuid.js @@ -0,0 +1,21 @@ +'use strict'; + + +const db = require('../../database'); +const batch = require('../../batch'); + +module.exports = { + name: 'Remove uid::sessionUUID:sessionId object', + timestamp: Date.UTC(2024, 5, 26), + method: async function () { + const { progress } = this; + + await batch.processSortedSet('users:joindate', async (uids) => { + progress.incr(uids.length); + await db.deleteAll(uids.map(uid => `uid:${uid}:sessionUUID:sessionId`)); + }, { + batch: 500, + progress: progress, + }); + }, +}; diff --git a/src/user/auth.js b/src/user/auth.js index b35b7d6323..0adf589967 100644 --- a/src/user/auth.js +++ b/src/user/auth.js @@ -2,8 +2,6 @@ const validator = require('validator'); const _ = require('lodash'); -const winston = require('winston'); -const cronJob = require('cron').CronJob; const db = require('../database'); const meta = require('../meta'); const events = require('../events'); @@ -13,21 +11,6 @@ const utils = require('../utils'); module.exports = function (User) { User.auth = {}; - const uidsToClean = Object.create(null); - - new cronJob('*/30 * * * * *', (async () => { - const uids = Object.keys(uidsToClean); - try { - await Promise.all(uids.map(async (uid) => { - delete uidsToClean[uid]; - await cleanExpiredSessions(uid); - await revokeSessionsAboveThreshold(uid); - })); - } catch (err) { - winston.error(err.stack); - } - }), null, true); - User.auth.logAttempt = async function (uid, ip) { if (!(parseInt(uid, 10) > 0)) { return; @@ -93,58 +76,53 @@ module.exports = function (User) { }; async function cleanExpiredSessions(uid) { - const uuidMapping = await db.getObject(`uid:${uid}:sessionUUID:sessionId`); - if (!uuidMapping) { - return; + const sids = await db.getSortedSetRange(`uid:${uid}:sessions`, 0, -1); + if (!sids.length) { + return []; } - const expiredUUIDs = []; + const expiredSids = []; - await Promise.all(Object.keys(uuidMapping).map(async (uuid) => { - const sid = uuidMapping[uuid]; + const activeSids = []; + await Promise.all(sids.map(async (sid) => { const sessionObj = await db.sessionStoreGet(sid); const expired = !sessionObj || !sessionObj.hasOwnProperty('passport') || !sessionObj.passport.hasOwnProperty('user') || parseInt(sessionObj.passport.user, 10) !== parseInt(uid, 10); if (expired) { - expiredUUIDs.push(uuid); expiredSids.push(sid); + } else { + activeSids.push(sid); } })); - await db.deleteObjectFields(`uid:${uid}:sessionUUID:sessionId`, expiredUUIDs); + await db.sortedSetRemove(`uid:${uid}:sessions`, expiredSids); + return activeSids; } - User.auth.addSession = async function (uid, sessionId, uuid) { + User.auth.addSession = async function (uid, sessionId) { if (!(parseInt(uid, 10) > 0)) { return; } - const now = Date.now(); - await Promise.all([ - db.sortedSetAdd(`uid:${uid}:sessions`, now, sessionId), - db.setObjectField(`uid:${uid}:sessionUUID:sessionId`, uuid, sessionId), - ]); - uidsToClean[uid] = now; + + const activeSids = await cleanExpiredSessions(uid); + await db.sortedSetAdd(`uid:${uid}:sessions`, Date.now(), sessionId); + await revokeSessionsAboveThreshold(activeSids.push(sessionId), uid); }; - async function revokeSessionsAboveThreshold(uid) { - const activeSessions = await db.getSortedSetRange(`uid:${uid}:sessions`, 0, -1); - if (activeSessions.length > meta.config.maxUserSessions) { - const sessionsToRevoke = activeSessions.slice(0, activeSessions.length - meta.config.maxUserSessions); + async function revokeSessionsAboveThreshold(activeSids, uid) { + if (meta.config.maxUserSessions > 0 && activeSids.length > meta.config.maxUserSessions) { + const sessionsToRevoke = activeSids.slice(0, activeSids.length - meta.config.maxUserSessions); await User.auth.revokeSession(sessionsToRevoke, uid); } } User.auth.revokeSession = async function (sessionIds, uid) { sessionIds = Array.isArray(sessionIds) ? sessionIds : [sessionIds]; - const sessionObjs = await Promise.all(sessionIds.map(db.sessionStoreGet)); - const sidsToDestroy = sessionObjs.filter(Boolean).map((s, i) => sessionIds[i]); - const uuidsToDelete = sessionObjs.filter(s => s && s.meta && s.meta.uuid).map(s => s.meta.uuid); const destroySids = sids => Promise.all(sids.map(db.sessionStoreDestroy)); await Promise.all([ - db.deleteObjectFields(`uid:${uid}:sessionUUID:sessionId`, uuidsToDelete), db.sortedSetRemove(`uid:${uid}:sessions`, sessionIds), - destroySids(sidsToDestroy), + destroySids(sessionIds), ]); }; @@ -164,11 +142,10 @@ module.exports = function (User) { User.auth.deleteAllSessions = async function () { await batch.processSortedSet('users:joindate', async (uids) => { const sessionKeys = uids.map(uid => `uid:${uid}:sessions`); - const sessionUUIDKeys = uids.map(uid => `uid:${uid}:sessionUUID:sessionId`); const sids = _.flatten(await db.getSortedSetRange(sessionKeys, 0, -1)); await Promise.all([ - db.deleteAll(sessionKeys.concat(sessionUUIDKeys)), + db.deleteAll(sessionKeys), ...sids.map(sid => db.sessionStoreDestroy(sid)), ]); }, { batch: 1000 }); diff --git a/src/user/delete.js b/src/user/delete.js index b84b4ef1d8..8f99117c59 100644 --- a/src/user/delete.js +++ b/src/user/delete.js @@ -119,7 +119,7 @@ module.exports = function (User) { `uid:${uid}:chat:rooms:read`, `uid:${uid}:upvote`, `uid:${uid}:downvote`, `uid:${uid}:flag:pids`, - `uid:${uid}:sessions`, `uid:${uid}:sessionUUID:sessionId`, + `uid:${uid}:sessions`, `invitation:uid:${uid}`, ]; diff --git a/test/api.js b/test/api.js index 47961742ff..0ea9918953 100644 --- a/test/api.js +++ b/test/api.js @@ -562,8 +562,10 @@ describe('API', async () => { const reloginPaths = ['GET /api/user/{userslug}/edit/email', 'PUT /users/{uid}/password', 'DELETE /users/{uid}/sessions/{uuid}']; if (reloginPaths.includes(`${method.toUpperCase()} ${path}`)) { ({ jar } = await helpers.loginUser('admin', '123456')); - const sessionUUIDs = await db.getObject('uid:1:sessionUUID:sessionId'); - mocks.delete['/users/{uid}/sessions/{uuid}'][1].example = Object.keys(sessionUUIDs).pop(); + const sessionIds = await db.getSortedSetRange('uid:1:sessions', 0, -1); + const sessObj = await db.sessionStoreGet(sessionIds[0]); + const { uuid } = sessObj.meta; + mocks.delete['/users/{uid}/sessions/{uuid}'][1].example = uuid; // Retrieve CSRF token using cookie, to test Write API csrfToken = await helpers.getCsrfToken(jar); diff --git a/test/authentication.js b/test/authentication.js index 1dcbe176a8..193d617435 100644 --- a/test/authentication.js +++ b/test/authentication.js @@ -195,7 +195,7 @@ describe('authentication', () => { }); assert(body); assert.equal(body.username, username); - const sessions = await db.getObject(`uid:${uid}:sessionUUID:sessionId`); + const sessions = await db.getSortedSetRange(`uid:${uid}:sessions`, 0, -1); assert(sessions); assert(Object.keys(sessions).length > 0); });