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.
v1.18.x
Julian Lam 5 years ago committed by GitHub
parent ddce77b343
commit 8e5a2276af
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -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 = {
open: 'info',
wip: 'warning',
resolved: 'success',
rejected: 'danger',
};
flagObj.labelClass = stateToLabel[flagObj.state];
flagObj.labelClass = Flags._constants.state_class[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);
// Mods are also allowed to be assigned, if flag target is post in uid's moderated cid
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 purposes
const current = await db.getObjectFields('flag:' + flagId, ['state', 'assignee']);
// 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));
tasks.push(db.sortedSetRemove('flags:byState:' + current[prop], flagId));
if (!Flags._constants.states.includes(changeset[prop])) {
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));
tasks.push(notifyAssignee(changeset[prop]));
/* eslint-disable-next-line */
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]));
}
}
}
}

@ -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([
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);
}
uid1 = await User.create({ username: 'testUser', password: 'abcdef', email: 'b@c.com' });
Topics.post({
cid: category.cid,
uid: uid,
title: 'Topic to flag',
content: 'This is flaggable content',
}, next);
});
},
function (topicData, next) {
User.create({
username: 'testUser2', password: 'abcdef', email: 'c@d.com',
}, next);
},
function (uid, next) {
Groups.join('administrators', uid, next);
},
function (next) {
User.create({
username: 'unprivileged', password: 'abcdef', email: 'd@e.com',
}, next);
},
], done);
uid2 = await User.create({ username: 'testUser2', password: 'abcdef', email: 'c@d.com' });
await Groups.join('administrators', uid2);
category = await Categories.create({
name: 'test category',
});
await Topics.post({
cid: category.cid,
uid: uid1,
title: 'Topic to flag',
content: 'This is flaggable content',
});
uid3 = await User.create({
username: 'unprivileged', password: 'abcdef', email: 'd@e.com',
});
});
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 () {

Loading…
Cancel
Save