feat: check flag values on save (assignee and state) (#8122)

* feat: add assignee checking when updating flag

Prior to this, it was possible to update the assignee to any value (or
any user. This commit adds checking to allow only admins, global
moderators, or in the case of flagged posts, moderators.

Also some prep work was added for value checking `state`.

* feat: value checking `state` on flag update

The state should be one of the constants defined earlier in the file.

src/flags.js

@@ -19,6 +19,16 @@ const utils = require('../public/src/utils');
 
 const Flags = module.exports;
 
+Flags._constants = {
+	states: ['open', 'wip', 'resolved', 'rejected'],
+	state_class: {
+		open: 'info',
+		wip: 'warning',
+		resolved: 'success',
+		rejected: 'danger',
+	},
+};
+
 Flags.init = async function () {
 	// Query plugins for custom filter strategies and merge into core filter strategies
 	function prepareSets(sets, orSets, prefix, value) {
@@ -162,13 +172,7 @@ Flags.list = async function (filters, uid) {
 				'icon:text': userObj['icon:text'],
 			},
 		};
-		const stateToLabel = {
+		flagObj.labelClass = Flags._constants.state_class[flagObj.state];
-			open: 'info',
-			wip: 'warning',
-			resolved: 'success',
-			rejected: 'danger',
-		};
-		flagObj.labelClass = stateToLabel[flagObj.state];
 
 		return Object.assign(flagObj, {
 			description: validator.escape(String(flagObj.description)),
@@ -344,6 +348,7 @@ Flags.getTargetCid = async function (type, id) {
 };
 
 Flags.update = async function (flagId, uid, changeset) {
+	const current = await db.getObjectFields('flag:' + flagId, ['state', 'assignee', 'type', 'targetId']);
 	const now = changeset.datetime || Date.now();
 	const notifyAssignee = async function (assigneeId) {
 		if (assigneeId === '' || parseInt(uid, 10) === parseInt(assigneeId, 10)) {
@@ -359,20 +364,40 @@ Flags.update = async function (flagId, uid, changeset) {
 		});
 		await notifications.push(notifObj, [assigneeId]);
 	};
+	const isAssignable = async function (assigneeId) {
+		let allowed = false;
+		allowed = await user.isAdminOrGlobalMod(assigneeId);
 
-	// Retrieve existing flag data to compare for history-saving purposes
+		// Mods are also allowed to be assigned, if flag target is post in uid's moderated cid
-	const current = await db.getObjectFields('flag:' + flagId, ['state', 'assignee']);
+		if (!allowed && current.type === 'post') {
+			const cid = await posts.getCidByPid(current.targetId);
+			allowed = await user.isModerator(assigneeId, cid);
+		}
+
+		return allowed;
+	};
+
+	// Retrieve existing flag data to compare for history-saving/reference purposes
 	const tasks = [];
 	for (var prop in changeset) {
 		if (changeset.hasOwnProperty(prop)) {
 			if (current[prop] === changeset[prop]) {
 				delete changeset[prop];
 			} else if (prop === 'state') {
-				tasks.push(db.sortedSetAdd('flags:byState:' + changeset[prop], now, flagId));
+				if (!Flags._constants.states.includes(changeset[prop])) {
-				tasks.push(db.sortedSetRemove('flags:byState:' + current[prop], flagId));
+					delete changeset[prop];
+				} else {
+					tasks.push(db.sortedSetAdd('flags:byState:' + changeset[prop], now, flagId));
+					tasks.push(db.sortedSetRemove('flags:byState:' + current[prop], flagId));
+				}
 			} else if (prop === 'assignee') {
-				tasks.push(db.sortedSetAdd('flags:byAssignee:' + changeset[prop], now, flagId));
+				/* eslint-disable-next-line */
-				tasks.push(notifyAssignee(changeset[prop]));
+				if (!await isAssignable(parseInt(changeset[prop], 10))) {
+					delete changeset[prop];
+				} else {
+					tasks.push(db.sortedSetAdd('flags:byAssignee:' + changeset[prop], now, flagId));
+					tasks.push(notifyAssignee(changeset[prop]));
+				}
 			}
 		}
 	}

test/flags.js

@@ -13,40 +13,30 @@ var Groups = require('../src/groups');
 var Meta = require('../src/meta');
 
 describe('Flags', function () {
-	before(function (done) {
+	let uid1;
+	let uid2;
+	let uid3;
+	let category;
+	before(async () => {
 		// Create some stuff to flag
-		async.waterfall([
+		uid1 = await User.create({ username: 'testUser', password: 'abcdef', email: 'b@c.com' });
-			async.apply(User.create, { username: 'testUser', password: 'abcdef', email: 'b@c.com' }),
-			function (uid, next) {
-				Categories.create({
-					name: 'test category',
-				}, function (err, category) {
-					if (err) {
-						return done(err);
-					}
 
-					Topics.post({
+		uid2 = await User.create({ username: 'testUser2', password: 'abcdef', email: 'c@d.com' });
-						cid: category.cid,
+		await Groups.join('administrators', uid2);
-						uid: uid,
+
-						title: 'Topic to flag',
+		category = await Categories.create({
-						content: 'This is flaggable content',
+			name: 'test category',
-					}, next);
+		});
-				});
+		await Topics.post({
-			},
+			cid: category.cid,
-			function (topicData, next) {
+			uid: uid1,
-				User.create({
+			title: 'Topic to flag',
-					username: 'testUser2', password: 'abcdef', email: 'c@d.com',
+			content: 'This is flaggable content',
-				}, next);
+		});
-			},
+
-			function (uid, next) {
+		uid3 = await User.create({
-				Groups.join('administrators', uid, next);
+			username: 'unprivileged', password: 'abcdef', email: 'd@e.com',
-			},
+		});
-			function (next) {
-				User.create({
-					username: 'unprivileged', password: 'abcdef', email: 'd@e.com',
-				}, next);
-			},
-		], done);
 	});
 
 	describe('.create()', function () {
@@ -274,9 +264,9 @@ describe('Flags', function () {
 
 	describe('.update()', function () {
 		it('should alter a flag\'s various attributes and persist them to the database', function (done) {
-			Flags.update(1, 1, {
+			Flags.update(1, uid2, {
 				state: 'wip',
-				assignee: 1,
+				assignee: uid2,
 			}, function (err) {
 				assert.ifError(err);
 				db.getObjectFields('flag:1', ['state', 'assignee'], function (err, data) {
@@ -286,7 +276,7 @@ describe('Flags', function () {
 
 					assert.strictEqual('wip', data.state);
 					assert.ok(!isNaN(parseInt(data.assignee, 10)));
-					assert.strictEqual(1, parseInt(data.assignee, 10));
+					assert.strictEqual(uid2, parseInt(data.assignee, 10));
 					done();
 				});
 			});
@@ -313,6 +303,65 @@ describe('Flags', function () {
 				done();
 			});
 		});
+
+		it('should allow assignment if user is an admin and do nothing otherwise', async () => {
+			await Flags.update(1, uid2, {
+				assignee: uid2,
+			});
+			let assignee = await db.getObjectField('flag:1', 'assignee');
+			assert.strictEqual(uid2, parseInt(assignee, 10));
+
+			await Flags.update(1, uid2, {
+				assignee: uid3,
+			});
+			assignee = await db.getObjectField('flag:1', 'assignee');
+			assert.strictEqual(uid2, parseInt(assignee, 10));
+		});
+
+		it('should allow assignment if user is a global mod and do nothing otherwise', async () => {
+			await Groups.join('Global Moderators', uid3);
+
+			await Flags.update(1, uid3, {
+				assignee: uid3,
+			});
+			let assignee = await db.getObjectField('flag:1', 'assignee');
+			assert.strictEqual(uid3, parseInt(assignee, 10));
+
+			await Flags.update(1, uid3, {
+				assignee: uid1,
+			});
+			assignee = await db.getObjectField('flag:1', 'assignee');
+			assert.strictEqual(uid3, parseInt(assignee, 10));
+
+			await Groups.leave('Global Moderators', uid3);
+		});
+
+		it('should allow assignment if user is a mod of the category, do nothing otherwise', async () => {
+			await Groups.join('cid:' + category.cid + ':privileges:moderate', uid3);
+
+			await Flags.update(1, uid3, {
+				assignee: uid3,
+			});
+			let assignee = await db.getObjectField('flag:1', 'assignee');
+			assert.strictEqual(uid3, parseInt(assignee, 10));
+
+			await Flags.update(1, uid3, {
+				assignee: uid1,
+			});
+			assignee = await db.getObjectField('flag:1', 'assignee');
+			assert.strictEqual(uid3, parseInt(assignee, 10));
+
+			await Groups.leave('cid:' + category.cid + ':privileges:moderate', uid3);
+		});
+
+		it('should do nothing when you attempt to set a bogus state', async () => {
+			await Flags.update(1, uid2, {
+				state: 'hocus pocus',
+			});
+
+			const state = await db.getObjectField('flag:1', 'state');
+			assert.strictEqual('wip', state);
+		});
 	});
 
 	describe('.getTarget()', function () {