refactor: sessionUUID (#12658)

* refactor: sessionUUID

* test: get uuid from sessionsstore

* refactor: dont load all sids again in revoke

* feat: upgrade script, get rid of second sessionStoreGet

* allow disabling maxUserSessions by setting to 0

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'),

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:<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,
+		});
+	},
+};

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 });

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}`,
 		];
 

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);

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);
 		});