From 018976a7231683420acd982bf9705b0046846840 Mon Sep 17 00:00:00 2001 From: Sabe Jones Date: Tue, 6 Jun 2017 18:49:05 -0700 Subject: [PATCH] Disallow interactions by blocked users; new "get objections" Members API route (#8755) * Make flags.chatRevoked prevent sending private messages (issue #7971) * Disallow sending gems when messages aren't allowed. * Created function to check for objections to an interaction to user model and wired it into the API (issue #7971) * Fixes for issues raised by reviewers. * Added allowed values to apidoc for api.getObjectionsToInteraction. * Refactoring of getObjectionsToInteraction and minor API changes. * fix(objections): address PR comments * fix(strings): use US English for base edits * refactor(test): typos and phrasing --- .../api/v3/integration/chat/POST-chat.test.js | 6 +- .../GET-objections_interaction.test.js | 64 +++++++++++++++++++ .../members/POST-send_private_message.test.js | 14 ++++ .../members/POST-transfer_gems.test.js | 64 ++++++++++++++++++- website/common/locales/en/groups.json | 1 + website/server/controllers/api-v3/chat.js | 4 +- website/server/controllers/api-v3/members.js | 52 +++++++++++---- website/server/models/user/methods.js | 52 ++++++++++++++- 8 files changed, 235 insertions(+), 22 deletions(-) create mode 100644 test/api/v3/integration/members/GET-objections_interaction.test.js diff --git a/test/api/v3/integration/chat/POST-chat.test.js b/test/api/v3/integration/chat/POST-chat.test.js index 7770fb0127..d34a69f912 100644 --- a/test/api/v3/integration/chat/POST-chat.test.js +++ b/test/api/v3/integration/chat/POST-chat.test.js @@ -70,9 +70,9 @@ describe('POST /chat', () => { it('returns an error when chat privileges are revoked when sending a message to a public guild', async () => { let userWithChatRevoked = await member.update({'flags.chatRevoked': true}); await expect(userWithChatRevoked.post(`/groups/${groupWithChat._id}/chat`, { message: testMessage})).to.eventually.be.rejected.and.eql({ - code: 404, - error: 'NotFound', - message: 'Your chat privileges have been revoked.', + code: 401, + error: 'NotAuthorized', + message: t('chatPrivilegesRevoked'), }); }); diff --git a/test/api/v3/integration/members/GET-objections_interaction.test.js b/test/api/v3/integration/members/GET-objections_interaction.test.js new file mode 100644 index 0000000000..cab90bdacd --- /dev/null +++ b/test/api/v3/integration/members/GET-objections_interaction.test.js @@ -0,0 +1,64 @@ +import { + generateUser, + translate as t, +} from '../../../../helpers/api-v3-integration.helper'; +import { v4 as generateUUID } from 'uuid'; + +describe('GET /members/:toUserId/objections/:interaction', () => { + let user; + + before(async () => { + user = await generateUser(); + }); + + it('validates req.params.memberId', async () => { + await expect( + user.get('/members/invalidUUID/objections/send-private-message') + ).to.eventually.be.rejected.and.eql({ + code: 400, + error: 'BadRequest', + message: t('invalidReqParams'), + }); + }); + + it('handles non-existing members', async () => { + let dummyId = generateUUID(); + await expect( + user.get(`/members/${dummyId}/objections/send-private-message`) + ).to.eventually.be.rejected.and.eql({ + code: 404, + error: 'NotFound', + message: t('userWithIDNotFound', {userId: dummyId}), + }); + }); + + it('handles non-existing interactions', async () => { + let receiver = await generateUser(); + + await expect( + user.get(`/members/${receiver._id}/objections/hug-a-whole-forest-of-trees`) + ).to.eventually.be.rejected.and.eql({ + code: 400, + error: 'BadRequest', + message: t('invalidReqParams'), + }); + }); + + it('returns an empty array if there are no objections', async () => { + let receiver = await generateUser(); + + await expect( + user.get(`/members/${receiver._id}/objections/send-private-message`) + ).to.eventually.be.fulfilled.and.eql([]); + }); + + it('returns an array of objections if any exist', async () => { + let receiver = await generateUser({'inbox.blocks': [user._id]}); + + await expect( + user.get(`/members/${receiver._id}/objections/send-private-message`) + ).to.eventually.be.fulfilled.and.eql([ + t('notAuthorizedToSendMessageToThisUser'), + ]); + }); +}); diff --git a/test/api/v3/integration/members/POST-send_private_message.test.js b/test/api/v3/integration/members/POST-send_private_message.test.js index 3bd3380437..7498d2e560 100644 --- a/test/api/v3/integration/members/POST-send_private_message.test.js +++ b/test/api/v3/integration/members/POST-send_private_message.test.js @@ -82,6 +82,20 @@ describe('POST /members/send-private-message', () => { }); }); + it('returns an error when chat privileges are revoked', async () => { + let userWithChatRevoked = await generateUser({'flags.chatRevoked': true}); + let receiver = await generateUser(); + + await expect(userWithChatRevoked.post('/members/send-private-message', { + message: messageToSend, + toUserId: receiver._id, + })).to.eventually.be.rejected.and.eql({ + code: 401, + error: 'NotAuthorized', + message: t('chatPrivilegesRevoked'), + }); + }); + it('sends a private message to a user', async () => { let receiver = await generateUser(); diff --git a/test/api/v3/integration/members/POST-transfer_gems.test.js b/test/api/v3/integration/members/POST-transfer_gems.test.js index 6862df5b95..a56540867b 100644 --- a/test/api/v3/integration/members/POST-transfer_gems.test.js +++ b/test/api/v3/integration/members/POST-transfer_gems.test.js @@ -43,7 +43,7 @@ describe('POST /members/transfer-gems', () => { }); }); - it('returns error when to user is not found', async () => { + it('returns error when recipient is not found', async () => { await expect(userToSendMessage.post('/members/transfer-gems', { message, gemAmount, @@ -55,7 +55,7 @@ describe('POST /members/transfer-gems', () => { }); }); - it('returns error when to user attempts to send gems to themselves', async () => { + it('returns error when user attempts to send gems to themselves', async () => { await expect(userToSendMessage.post('/members/transfer-gems', { message, gemAmount, @@ -67,6 +67,64 @@ describe('POST /members/transfer-gems', () => { }); }); + it('returns error when recipient has blocked the sender', async () => { + let receiverWhoBlocksUser = await generateUser({'inbox.blocks': [userToSendMessage._id]}); + + await expect(userToSendMessage.post('/members/transfer-gems', { + message, + gemAmount, + toUserId: receiverWhoBlocksUser._id, + })).to.eventually.be.rejected.and.eql({ + code: 401, + error: 'NotAuthorized', + message: t('notAuthorizedToSendMessageToThisUser'), + }); + }); + + it('returns error when sender has blocked recipient', async () => { + let sender = await generateUser({'inbox.blocks': [receiver._id]}); + + await expect(sender.post('/members/transfer-gems', { + message, + gemAmount, + toUserId: receiver._id, + })).to.eventually.be.rejected.and.eql({ + code: 401, + error: 'NotAuthorized', + message: t('notAuthorizedToSendMessageToThisUser'), + }); + }); + + it('returns an error when chat privileges are revoked', async () => { + let userWithChatRevoked = await generateUser({'flags.chatRevoked': true}); + + await expect(userWithChatRevoked.post('/members/transfer-gems', { + message, + gemAmount, + toUserId: receiver._id, + })).to.eventually.be.rejected.and.eql({ + code: 401, + error: 'NotAuthorized', + message: t('chatPrivilegesRevoked'), + }); + }); + + it('works when only the recipient\'s chat privileges are revoked', async () => { + let receiverWithChatRevoked = await generateUser({'flags.chatRevoked': true}); + + await expect(userToSendMessage.post('/members/transfer-gems', { + message, + gemAmount, + toUserId: receiverWithChatRevoked._id, + })).to.eventually.be.fulfilled; + + let updatedReceiver = await receiverWithChatRevoked.get('/user'); + let updatedSender = await userToSendMessage.get('/user'); + + expect(updatedReceiver.balance).to.equal(gemAmount / 4); + expect(updatedSender.balance).to.equal(0); + }); + it('returns error when there is no gemAmount', async () => { await expect(userToSendMessage.post('/members/transfer-gems', { message, @@ -144,7 +202,7 @@ describe('POST /members/transfer-gems', () => { expect(updatedSender.balance).to.equal(0); }); - it('does not requrie a message', async () => { + it('does not require a message', async () => { await userToSendMessage.post('/members/transfer-gems', { gemAmount, toUserId: receiver._id, diff --git a/website/common/locales/en/groups.json b/website/common/locales/en/groups.json index 20b9ccddb0..b56f9291f7 100644 --- a/website/common/locales/en/groups.json +++ b/website/common/locales/en/groups.json @@ -209,6 +209,7 @@ "onlyCreatorOrAdminCanDeleteChat": "Not authorized to delete this message!", "onlyGroupLeaderCanEditTasks": "Not authorized to manage tasks!", "onlyGroupTasksCanBeAssigned": "Only group tasks can be assigned", + "chatPrivilegesRevoked": "Your chat privileges have been revoked.", "newChatMessagePlainNotification": "New message in <%= groupName %> by <%= authorName %>. Click here to open the chat page!", "newChatMessageTitle": "New message in <%= groupName %>", "exportInbox": "Export Messages", diff --git a/website/server/controllers/api-v3/chat.js b/website/server/controllers/api-v3/chat.js index f68ecde053..58c772da5e 100644 --- a/website/server/controllers/api-v3/chat.js +++ b/website/server/controllers/api-v3/chat.js @@ -117,8 +117,6 @@ function textContainsBannedWords (message) { * @apiParam (Body) {String} message Message The message to post * @apiParam (Query) {UUID} previousMsg The previous chat message's UUID which will force a return of the full group chat * - * @apiSuccess data An array of chat messages if a new message was posted after previousMsg, otherwise the posted message - * * @apiUse GroupNotFound * @apiUse GroupIdRequired * @apiError (400) {NotFound} ChatPriviledgesRevoked Your chat privileges have been revoked @@ -143,7 +141,7 @@ api.postChat = { if (!group) throw new NotFound(res.t('groupNotFound')); if (group.privacy !== 'private' && user.flags.chatRevoked) { - throw new NotFound('Your chat privileges have been revoked.'); + throw new NotAuthorized(res.t('chatPrivilegesRevoked')); } if (group._id === TAVERN_ID && textContainsBannedWords(req.body.message)) { diff --git a/website/server/controllers/api-v3/members.js b/website/server/controllers/api-v3/members.js index df92bf671e..4c523f75d9 100644 --- a/website/server/controllers/api-v3/members.js +++ b/website/server/controllers/api-v3/members.js @@ -4,6 +4,9 @@ import { publicFields as memberFields, nameFields, } from '../../models/user'; +import { + KNOWN_INTERACTIONS, +} from '../../models/user/methods'; import { model as Group } from '../../models/group'; import { model as Challenge } from '../../models/challenge'; import { @@ -385,6 +388,39 @@ api.getChallengeMemberProgress = { }, }; +/** + * @api {get} /api/v3/members/:toUserId/objections/:interaction Get any objections that would occur if the given interaction was attempted - BETA + * @apiVersion 3.0.0 + * @apiName GetObjectionsToInteraction + * @apiGroup Member + * + * @apiParam {UUID} toUserId The user to interact with + * @apiParam {String="send-private-message","transfer-gems"} interaction Name of the interaction to query + * + * @apiSuccess {Array} data Return an array of objections, if the interaction would be blocked; otherwise an empty array + */ +api.getObjectionsToInteraction = { + method: 'GET', + url: '/members/:toUserId/objections/:interaction', + middlewares: [authWithHeaders()], + async handler (req, res) { + req.checkParams('toUserId', res.t('toUserIDRequired')).notEmpty().isUUID(); + req.checkParams('interaction', res.t('interactionRequired')).notEmpty().isIn(KNOWN_INTERACTIONS); + + let validationErrors = req.validationErrors(); + if (validationErrors) throw validationErrors; + + let sender = res.locals.user; + let receiver = await User.findById(req.params.toUserId).exec(); + if (!receiver) throw new NotFound(res.t('userWithIDNotFound', {userId: req.params.toUserId})); + + let interaction = req.params.interaction; + let response = sender.getObjectionsToInteraction(interaction, receiver); + + res.respond(200, response.map(res.t)); + }, +}; + /** * @api {posts} /api/v3/members/send-private-message Send a private message to a member * @apiName SendPrivateMessage @@ -410,17 +446,11 @@ api.sendPrivateMessage = { let sender = res.locals.user; let message = req.body.message; - let receiver = await User.findById(req.body.toUserId).exec(); if (!receiver) throw new NotFound(res.t('userNotFound')); - let userBlockedSender = receiver.inbox.blocks.indexOf(sender._id) !== -1; - let userIsBlockBySender = sender.inbox.blocks.indexOf(receiver._id) !== -1; - let userOptedOutOfMessaging = receiver.inbox.optOut; - - if (userBlockedSender || userIsBlockBySender || userOptedOutOfMessaging) { - throw new NotAuthorized(res.t('notAuthorizedToSendMessageToThisUser')); - } + let objections = sender.getObjectionsToInteraction('send-private-message', receiver); + if (objections.length > 0) throw new NotAuthorized(res.t(objections[0])); await sender.sendMessage(receiver, { receiverMsg: message }); @@ -472,13 +502,11 @@ api.transferGems = { if (validationErrors) throw validationErrors; let sender = res.locals.user; - let receiver = await User.findById(req.body.toUserId).exec(); if (!receiver) throw new NotFound(res.t('userNotFound')); - if (receiver._id === sender._id) { - throw new NotAuthorized(res.t('cannotSendGemsToYourself')); - } + let objections = sender.getObjectionsToInteraction('transfer-gems', receiver); + if (objections.length > 0) throw new NotAuthorized(res.t(objections[0])); let gemAmount = req.body.gemAmount; let amount = gemAmount / 4; diff --git a/website/server/models/user/methods.js b/website/server/models/user/methods.js index 34cd5608f0..8959171d1d 100644 --- a/website/server/models/user/methods.js +++ b/website/server/models/user/methods.js @@ -5,7 +5,7 @@ import { chatDefaults, TAVERN_ID, } from '../group'; -import { defaults } from 'lodash'; +import { defaults, map, flatten, flow, compact, uniq, partialRight } from 'lodash'; import { model as UserNotification } from '../userNotification'; import schema from './schema'; import payments from '../../libs/payments'; @@ -33,6 +33,56 @@ schema.methods.getGroups = function getUserGroups () { return userGroups; }; +/* eslint-disable no-unused-vars */ // The checks below all get access to sndr and rcvr, but not all use both +const INTERACTION_CHECKS = Object.freeze({ + always: [ + // Revoked chat privileges block all interactions to prevent the evading of harassment protections + // See issue #7971 for some discussion + (sndr, rcvr) => sndr.flags.chatRevoked && 'chatPrivilegesRevoked', + + // Direct user blocks prevent all interactions + (sndr, rcvr) => rcvr.inbox.blocks.includes(sndr._id) && 'notAuthorizedToSendMessageToThisUser', + (sndr, rcvr) => sndr.inbox.blocks.includes(rcvr._id) && 'notAuthorizedToSendMessageToThisUser', + ], + + 'send-private-message': [ + // Private messaging has an opt-out, which does not affect other interactions + (sndr, rcvr) => rcvr.inbox.optOut && 'notAuthorizedToSendMessageToThisUser', + + // We allow a player to message themselves so they can test how PMs work or send their own notes to themselves + ], + + 'transfer-gems': [ + // Unlike private messages, gems can't be sent to oneself + (sndr, rcvr) => rcvr._id === sndr._id && 'cannotSendGemsToYourself', + ], +}); +/* eslint-enable no-unused-vars */ + +export const KNOWN_INTERACTIONS = Object.freeze(Object.keys(INTERACTION_CHECKS).filter(key => key !== 'always')); + +// Get an array of error message keys that would be thrown if the given interaction was attempted +schema.methods.getObjectionsToInteraction = function getObjectionsToInteraction (interaction, receiver) { + if (!KNOWN_INTERACTIONS.includes(interaction)) { + throw new Error(`Unknown kind of interaction: "${interaction}", expected one of ${KNOWN_INTERACTIONS.join(', ')}`); + } + + let sender = this; + let checks = [ + INTERACTION_CHECKS.always, + INTERACTION_CHECKS[interaction], + ]; + + let executeChecks = partialRight(map, (check) => check(sender, receiver)); + + return flow( + flatten, + executeChecks, + compact, // Remove passed checks (passed checks return falsy; failed checks return message keys) + uniq + )(checks); +}; + /** * Sends a message to a user. Archives a copy in sender's inbox.