From 8888e63005743df0ef7f95302dd8103af3acc516 Mon Sep 17 00:00:00 2001 From: Alys Date: Fri, 3 Aug 2018 20:04:01 +1000 Subject: [PATCH] limit chat message flagging ability for new players - fixes #10069 (#10567) * remove duplicate module.exports statement * remove commented-out footer in Slack slur notification There's no need for anything to replace this footer. * swap order of flag actions to put most critical first This causes moderators to be notified before the flagged message's flagCount is incremented, because if something happens to prevent the flagGroupMessage Promise from resolving, we still want to mods to see the notification. * limit chat message flagging ability for new players Players who created accounts less than three days ago can flag posts but that does not contribute to the posts' flagCount. This prevents a troll from maliciously hiding innocent messages by creating new accounts to flag them. * add tests * fix other tests --- test/api/unit/libs/slack.js | 2 +- .../integration/chat/POST-chat.flag.test.js | 78 +++++++++++++++++-- .../api/v3/integration/chat/POST-chat.test.js | 2 - ...POST-groups_id_chat_id_clear_flags.test.js | 14 +++- .../libs/chatReporting/groupChatReporter.js | 22 ++++-- website/server/libs/slack.js | 14 ++-- 6 files changed, 107 insertions(+), 25 deletions(-) diff --git a/test/api/unit/libs/slack.js b/test/api/unit/libs/slack.js index 4f63a5dcd9..c8d44ff876 100644 --- a/test/api/unit/libs/slack.js +++ b/test/api/unit/libs/slack.js @@ -58,7 +58,7 @@ describe('slack', () => { title: 'Flag in Some group - (private guild)', title_link: undefined, text: 'some text', - footer: sandbox.match(/<.*?groupId=group-id&chatId=chat-id\|Flag this message>/), + footer: sandbox.match(/<.*?groupId=group-id&chatId=chat-id\|Flag this message.>/), mrkdwn_in: [ 'text', ], diff --git a/test/api/v3/integration/chat/POST-chat.flag.test.js b/test/api/v3/integration/chat/POST-chat.flag.test.js index 762899f052..88b41744c8 100644 --- a/test/api/v3/integration/chat/POST-chat.flag.test.js +++ b/test/api/v3/integration/chat/POST-chat.flag.test.js @@ -3,15 +3,23 @@ import { translate as t, } from '../../../../helpers/api-integration/v3'; import { find } from 'lodash'; +import moment from 'moment'; +import nconf from 'nconf'; +import { IncomingWebhook } from '@slack/client'; + +const BASE_URL = nconf.get('BASE_URL'); describe('POST /chat/:chatId/flag', () => { - let user, admin, anotherUser, group; + let user, admin, anotherUser, newUser, group; const TEST_MESSAGE = 'Test Message'; + const USER_AGE_FOR_FLAGGING = 3; beforeEach(async () => { - user = await generateUser({balance: 1}); + user = await generateUser({balance: 1, 'auth.timestamps.created': moment().subtract(USER_AGE_FOR_FLAGGING + 1, 'days').toDate()}); admin = await generateUser({balance: 1, 'contributor.admin': true}); - anotherUser = await generateUser(); + anotherUser = await generateUser({'auth.timestamps.created': moment().subtract(USER_AGE_FOR_FLAGGING + 1, 'days').toDate()}); + newUser = await generateUser({'auth.timestamps.created': moment().subtract(1, 'days').toDate()}); + sandbox.stub(IncomingWebhook.prototype, 'send'); group = await user.post('/groups', { name: 'Test Guild', @@ -20,6 +28,10 @@ describe('POST /chat/:chatId/flag', () => { }); }); + afterEach(() => { + sandbox.restore(); + }); + it('Returns an error when chat message is not found', async () => { await expect(user.post(`/groups/${group._id}/chat/incorrectMessage/flag`)) .to.eventually.be.rejected.and.eql({ @@ -34,7 +46,7 @@ describe('POST /chat/:chatId/flag', () => { await expect(user.post(`/groups/${group._id}/chat/${message.message.id}/flag`)).to.eventually.be.ok; }); - it('Flags a chat', async () => { + it('Flags a chat and sends normal message to moderator Slack when user is not new', async () => { let { message } = await anotherUser.post(`/groups/${group._id}/chat`, {message: TEST_MESSAGE}); let flagResult = await user.post(`/groups/${group._id}/chat/${message.id}/flag`); @@ -45,6 +57,62 @@ describe('POST /chat/:chatId/flag', () => { let messageToCheck = find(groupWithFlags.chat, {id: message.id}); expect(messageToCheck.flags[user._id]).to.equal(true); + + // Slack message to mods + const timestamp = `${moment(message.timestamp).utc().format('YYYY-MM-DD HH:mm')} UTC`; + + /* eslint-disable camelcase */ + expect(IncomingWebhook.prototype.send).to.be.calledWith({ + text: `${user.profile.name} (${user.id}; language: en) flagged a message`, + attachments: [{ + fallback: 'Flag Message', + color: 'danger', + author_name: `${anotherUser.profile.name} - ${anotherUser.auth.local.email} - ${anotherUser._id}\n${timestamp}`, + title: 'Flag in Test Guild', + title_link: `${BASE_URL}/groups/guild/${group._id}`, + text: TEST_MESSAGE, + footer: ``, + mrkdwn_in: [ + 'text', + ], + }], + }); + /* eslint-ensable camelcase */ + }); + + it('Does not increment message flag count and sends different message to moderator Slack when user is new', async () => { + let automatedComment = `The post's flag count has not been increased because the flagger's account is less than ${USER_AGE_FOR_FLAGGING} days old.`; + let { message } = await newUser.post(`/groups/${group._id}/chat`, {message: TEST_MESSAGE}); + + let flagResult = await newUser.post(`/groups/${group._id}/chat/${message.id}/flag`); + expect(flagResult.flags[newUser._id]).to.equal(true); + expect(flagResult.flagCount).to.equal(0); + + let groupWithFlags = await admin.get(`/groups/${group._id}`); + + let messageToCheck = find(groupWithFlags.chat, {id: message.id}); + expect(messageToCheck.flags[newUser._id]).to.equal(true); + + // Slack message to mods + const timestamp = `${moment(message.timestamp).utc().format('YYYY-MM-DD HH:mm')} UTC`; + + /* eslint-disable camelcase */ + expect(IncomingWebhook.prototype.send).to.be.calledWith({ + text: `${newUser.profile.name} (${newUser.id}; language: en) flagged a message`, + attachments: [{ + fallback: 'Flag Message', + color: 'danger', + author_name: `${newUser.profile.name} - ${newUser.auth.local.email} - ${newUser._id}\n${timestamp}`, + title: 'Flag in Test Guild', + title_link: `${BASE_URL}/groups/guild/${group._id}`, + text: TEST_MESSAGE, + footer: ` ${automatedComment}`, + mrkdwn_in: [ + 'text', + ], + }], + }); + /* eslint-ensable camelcase */ }); it('Flags a chat when the author\'s account was deleted', async () => { @@ -117,7 +185,7 @@ describe('POST /chat/:chatId/flag', () => { }); }); - it('Returns an error when user tries to flag a message that is already flagged', async () => { + it('Returns an error when user tries to flag a message that they already flagged', async () => { let { message } = await anotherUser.post(`/groups/${group._id}/chat`, {message: TEST_MESSAGE}); await user.post(`/groups/${group._id}/chat/${message.id}/flag`); diff --git a/test/api/v3/integration/chat/POST-chat.test.js b/test/api/v3/integration/chat/POST-chat.test.js index 3f52127d01..d607c045f1 100644 --- a/test/api/v3/integration/chat/POST-chat.test.js +++ b/test/api/v3/integration/chat/POST-chat.test.js @@ -259,7 +259,6 @@ describe('POST /chat', () => { title: 'Slur in Test Guild', title_link: `${BASE_URL}/groups/guild/${groupWithChat.id}`, text: testSlurMessage, - // footer: sandbox.match(/<.*?groupId=group-id&chatId=chat-id\|Flag this message>/), mrkdwn_in: [ 'text', ], @@ -312,7 +311,6 @@ describe('POST /chat', () => { title: 'Slur in Party - (private party)', title_link: undefined, text: testSlurMessage, - // footer: sandbox.match(/<.*?groupId=group-id&chatId=chat-id\|Flag this message>/), mrkdwn_in: [ 'text', ], diff --git a/test/api/v3/integration/chat/POST-groups_id_chat_id_clear_flags.test.js b/test/api/v3/integration/chat/POST-groups_id_chat_id_clear_flags.test.js index 89e5b133b7..153bc3bd0a 100644 --- a/test/api/v3/integration/chat/POST-groups_id_chat_id_clear_flags.test.js +++ b/test/api/v3/integration/chat/POST-groups_id_chat_id_clear_flags.test.js @@ -4,23 +4,24 @@ import { translate as t, } from '../../../../helpers/api-integration/v3'; import config from '../../../../../config.json'; +import moment from 'moment'; import { v4 as generateUUID } from 'uuid'; describe('POST /groups/:id/chat/:id/clearflags', () => { + const USER_AGE_FOR_FLAGGING = 3; let groupWithChat, message, author, nonAdmin, admin; before(async () => { - let { group, groupLeader, members } = await createAndPopulateGroup({ + let { group, groupLeader } = await createAndPopulateGroup({ groupDetails: { type: 'guild', privacy: 'public', }, - members: 1, }); groupWithChat = group; author = groupLeader; - nonAdmin = members[0]; + nonAdmin = await generateUser({'auth.timestamps.created': moment().subtract(USER_AGE_FOR_FLAGGING + 1, 'days').toDate()}); admin = await generateUser({'contributor.admin': true}); message = await author.post(`/groups/${groupWithChat._id}/chat`, { message: 'Some message' }); @@ -69,9 +70,14 @@ describe('POST /groups/:id/chat/:id/clearflags', () => { privateMessage = privateMessage.message; await admin.post(`/groups/${group._id}/chat/${privateMessage.id}/flag`); + + // first test that the flag was actually successful + let messages = await members[0].get(`/groups/${group._id}/chat`); + expect(messages[0].flagCount).to.eql(5); + await admin.post(`/groups/${group._id}/chat/${privateMessage.id}/clearflags`); - let messages = await members[0].get(`/groups/${group._id}/chat`); + messages = await members[0].get(`/groups/${group._id}/chat`); expect(messages[0].flagCount).to.eql(0); }); diff --git a/website/server/libs/chatReporting/groupChatReporter.js b/website/server/libs/chatReporting/groupChatReporter.js index c344607418..f883b6716d 100644 --- a/website/server/libs/chatReporting/groupChatReporter.js +++ b/website/server/libs/chatReporting/groupChatReporter.js @@ -1,4 +1,5 @@ import nconf from 'nconf'; +import moment from 'moment'; import ChatReporter from './chatReporter'; import { @@ -15,6 +16,7 @@ const COMMUNITY_MANAGER_EMAIL = nconf.get('EMAILS:COMMUNITY_MANAGER_EMAIL'); const FLAG_REPORT_EMAILS = nconf.get('FLAG_REPORT_EMAIL').split(',').map((email) => { return { email, canSend: true }; }); +const USER_AGE_FOR_FLAGGING = 3; // accounts less than this many days old don't increment flagCount export default class GroupChatReporter extends ChatReporter { constructor (req, res) { @@ -47,7 +49,7 @@ export default class GroupChatReporter extends ChatReporter { return {message, group, userComment}; } - async notify (group, message, userComment) { + async notify (group, message, userComment, automatedComment = '') { await super.notify(group, message); const groupUrl = getGroupUrl(group); @@ -65,10 +67,11 @@ export default class GroupChatReporter extends ChatReporter { group, message, userComment, + automatedComment, }); } - async flagGroupMessage (group, message) { + async flagGroupMessage (group, message, increaseFlagCount) { // Log user ids that have flagged the message if (!message.flags) message.flags = {}; // TODO fix error type @@ -81,7 +84,7 @@ export default class GroupChatReporter extends ChatReporter { if (this.user.contributor.admin) { // Arbitrary amount, higher than 2 message.flagCount = 5; - } else { + } else if (increaseFlagCount) { message.flagCount++; } @@ -90,8 +93,17 @@ export default class GroupChatReporter extends ChatReporter { async flag () { let {message, group, userComment} = await this.validate(); - await this.flagGroupMessage(group, message); - await this.notify(group, message, userComment); + + let increaseFlagCount = true; + let automatedComment = ''; + if (moment().diff(this.user.auth.timestamps.created, 'days') < USER_AGE_FOR_FLAGGING) { + increaseFlagCount = false; + automatedComment = `The post's flag count has not been increased because the flagger's account is less than ${USER_AGE_FOR_FLAGGING} days old.`; + // This is to prevent trolls from making new accounts to maliciously flag-and-hide. + } + + await this.notify(group, message, userComment, automatedComment); + await this.flagGroupMessage(group, message, increaseFlagCount); return message; } } diff --git a/website/server/libs/slack.js b/website/server/libs/slack.js index 0ff3eae014..99a601ee67 100644 --- a/website/server/libs/slack.js +++ b/website/server/libs/slack.js @@ -26,6 +26,7 @@ function sendFlagNotification ({ group, message, userComment, + automatedComment, }) { if (!SLACK_FLAGGING_URL) { return; @@ -34,10 +35,14 @@ function sendFlagNotification ({ let authorName; let title = `Flag in ${group.name}`; let text = `${flagger.profile.name} (${flagger.id}; language: ${flagger.preferences.language}) flagged a message`; + let footer = `<${SLACK_FLAGGING_FOOTER_LINK}?groupId=${group.id}&chatId=${message.id}|Flag this message.>`; if (userComment) { text += ` and commented: ${userComment}`; } + if (automatedComment) { + footer += ` ${automatedComment}`; + } if (group.id === TAVERN_ID) { titleLink = `${BASE_URL}/groups/tavern`; @@ -64,7 +69,7 @@ function sendFlagNotification ({ title, title_link: titleLink, text: message.text, - footer: `<${SLACK_FLAGGING_FOOTER_LINK}?groupId=${group.id}&chatId=${message.id}|Flag this message>`, + footer, mrkdwn_in: [ 'text', ], @@ -97,11 +102,6 @@ function sendSubscriptionNotification ({ }); } -module.exports = { - sendFlagNotification, - sendSubscriptionNotification, -}; - function sendSlurNotification ({ authorEmail, author, @@ -135,8 +135,6 @@ function sendSlurNotification ({ title, title_link: titleLink, text: message, - // What to replace the footer with? - // footer: `<${SLACK_FLAGGING_FOOTER_LINK}?groupId=${group.id}&chatId=${message.id}|Flag this message>`, mrkdwn_in: [ 'text', ],