feat: update unban logic/invocation and refactor User.bans module

* auto unban when User.getUsersFields is called and the user is banned but has expired

* cleanups and removal of expiry_readable

* expiry_readable make an alias for backward compatibility

* User.bans.func vs User.*ban*Func

* console.log cleanups, plus todo message added

* use code util.deprecate

* fix: remove ununsed winston require
v1.18.x
Aziz Khoury 6 years ago committed by Julian Lam
parent ed91d3f2c8
commit 3fbb6faf28

@ -446,7 +446,7 @@ authenticationController.localLogin = function (req, username, password, next) {
user.isAdminOrGlobalMod(uid, next);
},
banned: function (next) {
user.isBanned(uid, next);
user.bans.isBanned(uid, next);
},
hasLoginPrivilege: function (next) {
privileges.global.can('local:login', uid, next);
@ -551,7 +551,7 @@ function getBanInfo(uid, callback) {
});
},
function (next) {
next(new Error(banInfo.expiry ? '[[error:user-banned-reason-until, ' + banInfo.expiry_readable + ', ' + banInfo.reason + ']]' : '[[error:user-banned-reason, ' + banInfo.reason + ']]'));
next(new Error(banInfo.banned_until ? '[[error:user-banned-reason-until, ' + banInfo.banned_until_readable + ', ' + banInfo.reason + ']]' : '[[error:user-banned-reason, ' + banInfo.reason + ']]'));
},
], function (err) {
if (err) {

@ -108,8 +108,8 @@ module.exports = function (middleware) {
},
navigation: async.apply(navigation.get, req.uid),
tags: async.apply(meta.tags.parse, req, data, res.locals.metaTags, res.locals.linkTags),
banned: async.apply(user.isBanned, req.uid),
banReason: async.apply(user.getBannedReason, req.uid),
banned: async.apply(user.bans.isBanned, req.uid),
banReason: async.apply(user.bans.getReason, req.uid),
unreadData: async.apply(topics.getUnreadData, { uid: req.uid }),
unreadChatCount: async.apply(messaging.getUnreadCount, req.uid),

@ -55,7 +55,7 @@ module.exports = function (SocketUser) {
toggleBan(socket.uid, uids, function (uid, next) {
async.waterfall([
function (next) {
user.unban(uid, next);
user.bans.unban(uid, next);
},
function (next) {
events.log({
@ -124,7 +124,7 @@ module.exports = function (SocketUser) {
});
},
function (next) {
user.ban(uid, until, reason, next);
user.bans.ban(uid, until, reason, next);
},
function (banData, next) {
db.setObjectField('uid:' + uid + ':ban:' + banData.timestamp, 'fromUid', callerUid, next);

@ -1,11 +1,13 @@
'use strict';
var async = require('async');
var db = require('../database');
const util = require('util');
const async = require('async');
const db = require('../database');
module.exports = function (User) {
User.ban = function (uid, until, reason, callback) {
User.bans = {};
User.bans.ban = function (uid, until, reason, callback) {
// "until" (optional) is unix timestamp in milliseconds
// "reason" (optional) is a string
if (!callback && typeof until === 'function') {
@ -43,8 +45,6 @@ module.exports = function (User) {
if (until > now) {
tasks.push(async.apply(db.sortedSetAdd, 'users:banned:expire', until, uid));
tasks.push(async.apply(User.setUserField, uid, 'banned:expire', until));
} else {
until = 0;
}
async.series(tasks, function (err) {
@ -52,7 +52,7 @@ module.exports = function (User) {
});
};
User.unban = function (uid, callback) {
User.bans.unban = function (uid, callback) {
async.waterfall([
function (next) {
User.setUserFields(uid, { banned: 0, 'banned:expire': 0 }, next);
@ -63,36 +63,50 @@ module.exports = function (User) {
], callback);
};
User.isBanned = function (uid, callback) {
User.bans.getBannedAndExpired = function (uid, callback) {
if (parseInt(uid, 10) <= 0) {
return setImmediate(callback, null, false);
}
async.waterfall([
async.apply(User.getUserFields, uid, ['banned', 'banned:expire']),
function (userData, next) {
var banned = userData && userData.banned;
if (!banned) {
return next(null, banned);
}
User.getUserFields(uid, ['banned', 'banned:expire'], function (err, userData) {
if (err) {
return callback(err);
}
callback(null, User.bans.calcExpiredFromUserData(userData));
});
};
// If they are banned, see if the ban has expired
var stillBanned = !userData['banned:expire'] || Date.now() < userData['banned:expire'];
User.bans.calcExpiredFromUserData = function (userData) {
return {
banned: !!userData.banned,
'banned:expire': userData['banned:expire'],
banExpired: userData['banned:expire'] <= Date.now() && userData['banned:expire'] !== 0,
};
};
if (stillBanned) {
return next(null, true);
}
async.parallel([
async.apply(db.sortedSetRemove.bind(db), 'users:banned:expire', uid),
async.apply(db.sortedSetRemove.bind(db), 'users:banned', uid),
async.apply(User.setUserFields, uid, { banned: 0, 'banned:expire': 0 }),
], function (err) {
next(err, false);
User.bans.unbanIfExpired = function (uid, callback) {
User.bans.getBannedAndExpired(uid, function (err, result) {
if (err) {
return callback(err);
}
if (result.banned && result.banExpired) {
return User.bans.unban(uid, function (err) {
callback(err, { banned: false, banExpired: true, 'banned:expire': 0 });
});
},
], callback);
}
callback(null, result);
});
};
User.bans.isBanned = function (uid, callback) {
if (parseInt(uid, 10) <= 0) {
return setImmediate(callback, null, false);
}
User.bans.unbanIfExpired(uid, function (err, result) {
callback(err, result.banned);
});
};
User.getBannedReason = function (uid, callback) {
User.bans.getReason = function (uid, callback) {
if (parseInt(uid, 10) <= 0) {
return setImmediate(callback, null, '');
}
@ -111,4 +125,14 @@ module.exports = function (User) {
},
], callback);
};
// TODO Remove in v1.13.0
const deprecatedMessage = (oldPath, newPath) => `function ${oldPath} is deprecated, please use ${newPath} instead`;
User.ban = util.deprecate(User.bans.ban, deprecatedMessage('User.ban', 'User.bans.ban'));
User.unban = util.deprecate(User.bans.unban, deprecatedMessage('User.unban', 'User.bans.unban'));
User.getBannedAndExpired = util.deprecate(User.bans.getBannedAndExpired, deprecatedMessage('User.getBannedAndExpired', 'User.bans.getBannedAndExpired'));
User.calcBanExpiredFromUserData = util.deprecate(User.bans.calcExpiredFromUserData, deprecatedMessage('User.calcBanExpiredFromUserData', 'User.bans.calcExpiredFromUserData'));
User.unbanIfBanExpired = util.deprecate(User.bans.unbanIfExpired, deprecatedMessage('User.unbanIfBanExpired', 'User.bans.unbanIfExpired'));
User.isBanned = util.deprecate(User.bans.isBanned, deprecatedMessage('User.isBanned', 'User.bans.isBanned'));
User.getBannedReason = util.deprecate(User.bans.getReason, deprecatedMessage('User.getBannedReason', 'User.bans.getReason'));
};

@ -74,6 +74,10 @@ module.exports = function (User) {
addField('lastonline');
}
if (fields.includes('banned') && !fields.includes('banned:expire')) {
addField('banned:expire');
}
var uniqueUids = _.uniq(uids).filter(uid => uid > 0);
async.waterfall([
@ -148,9 +152,9 @@ module.exports = function (User) {
}
function modifyUserData(users, requestedFields, fieldsToRemove, callback) {
users.forEach(function (user) {
async.map(users, function (user, next) {
if (!user) {
return;
return next(null, user);
}
db.parseIntFields(user, intFields, requestedFields);
@ -211,13 +215,28 @@ module.exports = function (User) {
user.lastonlineISO = utils.toISOString(user.lastonline) || user.joindateISO;
}
if (user.hasOwnProperty('banned:expire')) {
user.banned_until = user['banned:expire'];
user.banned_until_readable = user.banned_until ? new Date(user.banned_until).toString() : 'Not Banned';
if (user.hasOwnProperty('banned') || user.hasOwnProperty('banned:expire')) {
var result = User.bans.calcExpiredFromUserData(user);
var unban = result.banned && result.banExpired;
user.banned_until = unban ? 0 : user['banned:expire'];
user.banned_until_readable = user.banned_until && !unban ? utils.toISOString(user.banned_until) : 'Not Banned';
if (unban) {
return User.bans.unban(user.uid, function (err) {
if (err) {
return next(err);
}
user.banned = false;
next(null, user);
});
}
}
next(null, user);
}, function (err, users) {
if (err) {
return callback(err);
}
plugins.fireHook('filter:users.get', users, callback);
});
plugins.fireHook('filter:users.get', users, callback);
}
function parseGroupTitle(user) {

@ -79,7 +79,7 @@ Digest.getSubscribers = function (interval, callback) {
},
function (subscribers, next) {
async.filter(subscribers, function (uid, next) {
user.isBanned(uid, function (err, banned) {
user.bans.isBanned(uid, function (err, banned) {
next(err, !banned);
});
}, next);

@ -24,13 +24,16 @@ module.exports = function (User) {
db.getObject(record[0], next);
},
function (banInfo, next) {
var expiry = banInfo.expire;
const expire = parseInt(banInfo.expire, 10);
const expire_readable = utils.toISOString(expire);
next(null, {
uid: uid,
timestamp: banInfo.timestamp,
expiry: parseInt(expiry, 10),
expiry_readable: new Date(parseInt(expiry, 10)).toString(),
banned_until: expire,
expiry: expire, /* backward compatible alias */
banned_until_readable: expire_readable,
expiry_readable: expire_readable, /* backward compatible alias */
reason: validator.escape(String(banInfo.reason || '')),
});
},

@ -8,6 +8,7 @@ var async = require('async');
var db = require('./mocks/databasemock');
var user = require('../src/user');
var utils = require('../src/utils');
var meta = require('../src/meta');
var privileges = require('../src/privileges');
var helpers = require('./helpers');
@ -456,21 +457,21 @@ describe('authentication', function () {
it('should prevent banned user from logging in', function (done) {
user.create({ username: 'banme', password: '123456', email: 'ban@me.com' }, function (err, uid) {
assert.ifError(err);
user.ban(uid, 0, 'spammer', function (err) {
user.bans.ban(uid, 0, 'spammer', function (err) {
assert.ifError(err);
loginUser('banme', '123456', function (err, res, body) {
assert.ifError(err);
assert.equal(res.statusCode, 403);
assert.equal(body, '[[error:user-banned-reason, spammer]]');
user.unban(uid, function (err) {
user.bans.unban(uid, function (err) {
assert.ifError(err);
var expiry = Date.now() + 10000;
user.ban(uid, expiry, '', function (err) {
user.bans.ban(uid, expiry, '', function (err) {
assert.ifError(err);
loginUser('banme', '123456', function (err, res, body) {
assert.ifError(err);
assert.equal(res.statusCode, 403);
assert.equal(body, '[[error:user-banned-reason-until, ' + (new Date(parseInt(expiry, 10)).toString()) + ', No reason given.]]');
assert.equal(body, '[[error:user-banned-reason-until, ' + utils.toISOString(expiry) + ', No reason given.]]');
done();
});
});

@ -132,8 +132,8 @@ describe('socket.io', function () {
assert.ifError(err);
assert(data.uid);
assert(data.timestamp);
assert(data.hasOwnProperty('expiry'));
assert(data.hasOwnProperty('expiry_readable'));
assert(data.hasOwnProperty('banned_until'));
assert(data.hasOwnProperty('banned_until_readable'));
assert.equal(data.reason, 'spammer');
done();
});
@ -141,7 +141,7 @@ describe('socket.io', function () {
});
it('should return ban reason', function (done) {
user.getBannedReason(regularUid, function (err, reason) {
user.bans.getReason(regularUid, function (err, reason) {
assert.ifError(err);
assert.equal(reason, 'spammer');
done();
@ -152,7 +152,7 @@ describe('socket.io', function () {
var socketUser = require('../src/socket.io/user');
socketUser.unbanUsers({ uid: adminUid }, [regularUid], function (err) {
assert.ifError(err);
user.isBanned(regularUid, function (err, isBanned) {
user.bans.isBanned(regularUid, function (err, isBanned) {
assert.ifError(err);
assert(!isBanned);
done();

@ -879,7 +879,7 @@ describe('Topic\'s', function () {
title: 'topic for controller test',
content: 'topic content',
cid: topic.categoryId,
thumb: 'http://i.imgur.com/64iBdBD.jpg',
// thumb: 'http://i.imgur.com/64iBdBD.jpg',
}, function (err, result) {
assert.ifError(err);
assert.ok(result);

@ -1114,7 +1114,7 @@ describe('User', function () {
it('should return the correct ban reason', function (done) {
async.series([
function (next) {
User.ban(testUid, 0, '', function (err) {
User.bans.ban(testUid, 0, '', function (err) {
assert.ifError(err);
next(err);
});
@ -1130,7 +1130,7 @@ describe('User', function () {
},
], function (err) {
assert.ifError(err);
User.unban(testUid, function (err) {
User.bans.unban(testUid, function (err) {
assert.ifError(err);
done();
});
@ -1138,28 +1138,28 @@ describe('User', function () {
});
it('should ban user permanently', function (done) {
User.ban(testUid, function (err) {
User.bans.ban(testUid, function (err) {
assert.ifError(err);
User.isBanned(testUid, function (err, isBanned) {
User.bans.isBanned(testUid, function (err, isBanned) {
assert.ifError(err);
assert.equal(isBanned, true);
User.unban(testUid, done);
User.bans.unban(testUid, done);
});
});
});
it('should ban user temporarily', function (done) {
User.ban(testUid, Date.now() + 2000, function (err) {
User.bans.ban(testUid, Date.now() + 2000, function (err) {
assert.ifError(err);
User.isBanned(testUid, function (err, isBanned) {
User.bans.isBanned(testUid, function (err, isBanned) {
assert.ifError(err);
assert.equal(isBanned, true);
setTimeout(function () {
User.isBanned(testUid, function (err, isBanned) {
User.bans.isBanned(testUid, function (err, isBanned) {
assert.ifError(err);
assert.equal(isBanned, false);
User.unban(testUid, done);
User.bans.unban(testUid, done);
});
}, 3000);
});
@ -1167,7 +1167,7 @@ describe('User', function () {
});
it('should error if until is NaN', function (done) {
User.ban(testUid, 'asd', function (err) {
User.bans.ban(testUid, 'asd', function (err) {
assert.equal(err.message, '[[error:ban-expiry-missing]]');
done();
});

Loading…
Cancel
Save