closes #6738 closes #6290 (#6785)

* closes #6738 closes #6290

* fix test, delete keys

* delete all bans on user delete

* fix upgrade script to actually read reason

src/socket.io/user/ban.js

@@ -3,6 +3,7 @@
 var async = require('async');
 var winston = require('winston');
 
+var db = require('../../database');
 var user = require('../../user');
 var meta = require('../../meta');
 var websockets = require('../index');
@@ -22,7 +23,7 @@ module.exports = function (SocketUser) {
 		toggleBan(socket.uid, data.uids, function (uid, next) {
 			async.waterfall([
 				function (next) {
-					banUser(uid, data.until || 0, data.reason || '', next);
+					banUser(socket.uid, uid, data.until || 0, data.reason || '', next);
 				},
 				function (next) {
 					events.log({
@@ -94,7 +95,7 @@ module.exports = function (SocketUser) {
 		], callback);
 	}
 
-	function banUser(uid, until, reason, callback) {
+	function banUser(callerUid, uid, until, reason, callback) {
 		async.waterfall([
 			function (next) {
 				user.isAdministrator(uid, next);
@@ -125,6 +126,9 @@ module.exports = function (SocketUser) {
 			function (next) {
 				user.ban(uid, until, reason, next);
 			},
+			function (banData, next) {
+				db.setObjectField('uid:' + uid + ':ban:' + banData.timestamp, 'fromUid', callerUid, next);
+			},
 			function (next) {
 				if (!reason) {
 					return translator.translate('[[user:info.banned-no-reason]]', function (translated) {

src/upgrades/1.10.2/upgrade_bans_to_hashes.js

@@ -0,0 +1,83 @@
+'use strict';
+
+var db = require('../../database');
+
+var async = require('async');
+var batch = require('../../batch');
+// var user = require('../../user');
+
+module.exports = {
+	name: 'Upgrade bans to hashes',
+	timestamp: Date.UTC(2018, 8, 24),
+	method: function (callback) {
+		const progress = this.progress;
+
+		batch.processSortedSet('users:joindate', function (uids, next) {
+			async.eachSeries(uids, function (uid, next) {
+				progress.incr();
+
+				async.parallel({
+					bans: function (next) {
+						db.getSortedSetRevRangeWithScores('uid:' + uid + ':bans', 0, -1, next);
+					},
+					reasons: function (next) {
+						db.getSortedSetRevRangeWithScores('banned:' + uid + ':reasons', 0, -1, next);
+					},
+					userData: function (next) {
+						db.getObjectFields('user:' + uid, ['banned', 'banned:expire', 'joindate', 'lastposttime', 'lastonline'], next);
+					},
+				}, function (err, results) {
+					function addBan(key, data, callback) {
+						async.waterfall([
+							function (next) {
+								db.setObject(key, data, next);
+							},
+							function (next) {
+								db.sortedSetAdd('uid:' + uid + ':bans:timestamp', data.timestamp, key, next);
+							},
+						], callback);
+					}
+					if (err) {
+						return next(err);
+					}
+					// has no ban history and isn't banned, skip
+					if (!results.bans.length && !parseInt(results.userData.banned, 10)) {
+						return next();
+					}
+
+					// has no history, but is banned, create plain object with just uid and timestmap
+					if (!results.bans.length && parseInt(results.userData.banned, 10)) {
+						const banTimestamp = results.userData.lastonline || results.userData.lastposttime || results.userData.joindate || Date.now();
+						const banKey = 'uid:' + uid + ':ban:' + banTimestamp;
+						addBan(banKey, { uid: uid, timestamp: banTimestamp }, next);
+						return;
+					}
+
+					// process ban history
+					async.eachSeries(results.bans, function (ban, next) {
+						function findReason(score) {
+							return results.reasons.find(function (reasonData) {
+								return reasonData.score === score;
+							});
+						}
+						const reasonData = findReason(ban.score);
+						const banKey = 'uid:' + uid + ':ban:' + ban.score;
+						var data = {
+							uid: uid,
+							timestamp: ban.score,
+							expire: parseInt(ban.value, 10),
+						};
+						if (reasonData) {
+							data.reason = reasonData.value;
+						}
+						addBan(banKey, data, next);
+					}, function (err) {
+						next(err);
+					});
+				});
+			}, next);
+		}, {
+			progress: this.progress,
+		}, callback);
+	},
+};

src/user/bans.js

@@ -24,10 +24,20 @@ module.exports = function (User) {
 			return callback(new Error('[[error:ban-expiry-missing]]'));
 		}
 
+		const banKey = 'uid:' + uid + ':ban:' + now;
+		var banData = {
+			uid: uid,
+			timestamp: now,
+			expire: until > now ? until : 0,
+		};
+		if (reason) {
+			banData.reason = reason;
+		}
 		var tasks = [
 			async.apply(User.setUserField, uid, 'banned', 1),
 			async.apply(db.sortedSetAdd, 'users:banned', now, uid),
-			async.apply(db.sortedSetAdd, 'uid:' + uid + ':bans', now, until),
+			async.apply(db.sortedSetAdd, 'uid:' + uid + ':bans:timestamp', now, banKey),
+			async.apply(db.setObject, banKey, banData),
 		];
 
 		if (until > now) {
@@ -37,12 +47,8 @@ module.exports = function (User) {
 			until = 0;
 		}
 
-		if (reason) {
-			tasks.push(async.apply(db.sortedSetAdd, 'banned:' + uid + ':reasons', now, reason));
-		}
-
 		async.series(tasks, function (err) {
-			callback(err);
+			callback(err, banData);
 		});
 	};
 
@@ -86,10 +92,16 @@ module.exports = function (User) {
 	User.getBannedReason = function (uid, callback) {
 		async.waterfall([
 			function (next) {
-				db.getSortedSetRevRange('banned:' + uid + ':reasons', 0, 0, next);
+				db.getSortedSetRevRange('uid:' + uid + ':bans:timestamp', 0, 0, next);
+			},
+			function (keys, next) {
+				if (!keys.length) {
+					return callback(null, '');
+				}
+				db.getObject(keys[0], next);
 			},
-			function (reasons, next) {
+			function (banObj, next) {
-				next(null, reasons.length ? reasons[0] : '');
+				next(null, banObj && banObj.reason ? banObj.reason : '');
 			},
 		], callback);
 	};

src/user/delete.js

@@ -119,6 +119,7 @@ module.exports = function (User) {
 							'users:postcount',
 							'users:reputation',
 							'users:banned',
+							'users:banned:expire',
 							'users:online',
 							'users:notvalidated',
 							'digest:day:uids',
@@ -149,6 +150,9 @@ module.exports = function (User) {
 					function (next) {
 						deleteUserIps(uid, next);
 					},
+					function (next) {
+						deleteBans(uid, next);
+					},
 					function (next) {
 						deleteUserFromFollowers(uid, next);
 					},
@@ -220,6 +224,20 @@ module.exports = function (User) {
 		], callback);
 	}
 
+	function deleteBans(uid, callback) {
+		async.waterfall([
+			function (next) {
+				db.getSortedSetRange('uid:' + uid + ':bans:timestamp', 0, -1, next);
+			},
+			function (bans, next) {
+				db.deleteAll(bans, next);
+			},
+			function (next) {
+				db.delete('uid:' + uid + ':bans:timestamp', next);
+			},
+		], callback);
+	}
+
 	function deleteUserFromFollowers(uid, callback) {
 		async.parallel({
 			followers: async.apply(db.getSortedSetRange, 'followers:' + uid, 0, -1),

src/user/info.js

@@ -5,6 +5,7 @@ var _ = require('lodash');
 var validator = require('validator');
 
 var db = require('../database');
+var user = require('../user');
 var posts = require('../posts');
 var topics = require('../topics');
 var utils = require('../../public/src/utils');
@@ -12,30 +13,25 @@ var utils = require('../../public/src/utils');
 module.exports = function (User) {
 	User.getLatestBanInfo = function (uid, callback) {
 		// Simply retrieves the last record of the user's ban, even if they've been unbanned since then.
-		var timestamp;
-		var expiry;
-		var reason;
-
 		async.waterfall([
-			async.apply(db.getSortedSetRevRangeWithScores, 'uid:' + uid + ':bans', 0, 0),
+			function (next) {
+				db.getSortedSetRevRange('uid:' + uid + ':bans:timestamp', 0, 0, next);
+			},
 			function (record, next) {
 				if (!record.length) {
 					return next(new Error('no-ban-info'));
 				}
-
+				db.getObject(record[0], next);
-				timestamp = record[0].score;
-				expiry = record[0].value;
-
-				db.getSortedSetRangeByScore('banned:' + uid + ':reasons', 0, -1, timestamp, timestamp, next);
 			},
-			function (_reason, next) {
+			function (banInfo, next) {
-				reason = _reason && _reason.length ? _reason[0] : '';
+				var expiry = banInfo.expire;
+
 				next(null, {
 					uid: uid,
-					timestamp: timestamp,
+					timestamp: banInfo.timestamp,
 					expiry: parseInt(expiry, 10),
 					expiry_readable: new Date(parseInt(expiry, 10)).toString(),
-					reason: validator.escape(String(reason)),
+					reason: validator.escape(String(banInfo.reason || '')),
 				});
 			},
 		], callback);
@@ -46,8 +42,7 @@ module.exports = function (User) {
 			function (next) {
 				async.parallel({
 					flags: async.apply(db.getSortedSetRevRangeWithScores, 'flags:byTargetUid:' + uid, 0, 19),
-					bans: async.apply(db.getSortedSetRevRangeWithScores, 'uid:' + uid + ':bans', 0, 19),
+					bans: async.apply(db.getSortedSetRevRange, 'uid:' + uid + ':bans:timestamp', 0, 19),
-					reasons: async.apply(db.getSortedSetRevRangeWithScores, 'banned:' + uid + ':reasons', 0, 19),
 				}, next);
 			},
 			function (data, next) {
@@ -76,8 +71,7 @@ module.exports = function (User) {
 				});
 			},
 			function (data, next) {
-				formatBanData(data);
+				formatBanData(data, next);
-				next(null, data);
 			},
 		], callback);
 	};
@@ -131,26 +125,32 @@ module.exports = function (User) {
 		], callback);
 	}
 
-	function formatBanData(data) {
+	function formatBanData(data, callback) {
-		var reasons = data.reasons.reduce(function (memo, cur) {
+		var banData;
-			memo[cur.score] = cur.value;
+		async.waterfall([
-			return memo;
+			function (next) {
-		}, {});
+				console.log(data);
-
+				db.getObjects(data.bans, next);
-		data.bans = data.bans.map(function (banObj) {
+			},
-			banObj.until = parseInt(banObj.value, 10);
+			function (_banData, next) {
-			banObj.untilReadable = new Date(banObj.until).toString();
+				banData = _banData;
-			banObj.timestamp = parseInt(banObj.score, 10);
+				var uids = banData.map(banData => banData.fromUid);
-			banObj.timestampReadable = new Date(banObj.score).toString();
+
-			banObj.timestampISO = new Date(banObj.score).toISOString();
+				user.getUsersFields(uids, ['uid', 'username', 'userslug', 'picture'], next);
-			banObj.reason = validator.escape(String(reasons[banObj.score] || '')) || '[[user:info.banned-no-reason]]';
+			},
-
+			function (usersData, next) {
-			delete banObj.value;
+				data.bans = banData.map(function (banObj, index) {
-			delete banObj.score;
+					banObj.user = usersData[index];
-			delete data.reasons;
+					banObj.until = parseInt(banObj.expire, 10);
-
+					banObj.untilReadable = new Date(banObj.until).toString();
-			return banObj;
+					banObj.timestampReadable = new Date(banObj.timestamp).toString();
-		});
+					banObj.timestampISO = utils.toISOString(banObj.timestamp);
+					banObj.reason = validator.escape(String(banObj.reason || '')) || '[[user:info.banned-no-reason]]';
+					return banObj;
+				});
+				next(null, data);
+			},
+		], callback);
 	}
 
 	User.getModerationNotes = function (uid, start, stop, callback) {

test/controllers-admin.js

@@ -153,7 +153,6 @@ describe('Admin Controllers', function () {
 			assert(body.history);
 			assert(Array.isArray(body.history.flags));
 			assert(Array.isArray(body.history.bans));
-			assert(Array.isArray(body.history.reasons));
 			assert(Array.isArray(body.sessions));
 			done();
 		});