From b548d4e15bde0b2ecabefba1ce2676028427fc51 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Thu, 31 Mar 2022 08:54:33 +0100 Subject: [PATCH] Merge pull request #7285 from overleaf/jpa-enforce-edit-own-comment [misc] block users from editing other users comments GitOrigin-RevId: 6f2ba38daf8089a478d79ca495b3557a57390b43 --- .../Messages/MessageHttpController.js | 10 ++- .../js/Features/Messages/MessageManager.js | 5 +- .../acceptance/js/EditingAMessageTests.js | 87 +++++++++++++++---- .../test/acceptance/js/helpers/ChatClient.js | 18 ++++ .../app/src/Features/Chat/ChatApiHandler.js | 3 +- 5 files changed, 103 insertions(+), 20 deletions(-) diff --git a/services/chat/app/js/Features/Messages/MessageHttpController.js b/services/chat/app/js/Features/Messages/MessageHttpController.js index f523434c31..c84c30c8ba 100644 --- a/services/chat/app/js/Features/Messages/MessageHttpController.js +++ b/services/chat/app/js/Features/Messages/MessageHttpController.js @@ -54,11 +54,17 @@ async function deleteThread(req, res) { } async function editMessage(req, res) { - const { content } = req.body + const { content, userId } = req.body const { projectId, threadId, messageId } = req.params logger.log({ projectId, threadId, messageId, content }, 'editing message') const room = await ThreadManager.findOrCreateThread(projectId, threadId) - await MessageManager.updateMessage(room._id, messageId, content, Date.now()) + await MessageManager.updateMessage( + room._id, + messageId, + userId, + content, + Date.now() + ) res.sendStatus(204) } diff --git a/services/chat/app/js/Features/Messages/MessageManager.js b/services/chat/app/js/Features/Messages/MessageManager.js index 694a56e410..76eed9a974 100644 --- a/services/chat/app/js/Features/Messages/MessageManager.js +++ b/services/chat/app/js/Features/Messages/MessageManager.js @@ -45,11 +45,14 @@ async function deleteAllMessagesInRooms(roomIds) { }) } -async function updateMessage(roomId, messageId, content, timestamp) { +async function updateMessage(roomId, messageId, userId, content, timestamp) { const query = _ensureIdsAreObjectIds({ _id: messageId, room_id: roomId, }) + if (userId) { + query.user_id = ObjectId(userId) + } await db.messages.updateOne(query, { $set: { content, diff --git a/services/chat/test/acceptance/js/EditingAMessageTests.js b/services/chat/test/acceptance/js/EditingAMessageTests.js index 93e194bab8..668fbe7969 100644 --- a/services/chat/test/acceptance/js/EditingAMessageTests.js +++ b/services/chat/test/acceptance/js/EditingAMessageTests.js @@ -5,9 +5,7 @@ const ChatClient = require('./helpers/ChatClient') const ChatApp = require('./helpers/ChatApp') describe('Editing a message', async function () { - const projectId = ObjectId().toString() - const userId = ObjectId().toString() - const threadId = ObjectId().toString() + let projectId, userId, threadId before(async function () { await ChatApp.ensureRunning() }) @@ -15,7 +13,12 @@ describe('Editing a message', async function () { describe('in a thread', async function () { const content = 'thread message' const newContent = 'updated thread message' - before(async function () { + let messageId + beforeEach(async function () { + projectId = ObjectId().toString() + userId = ObjectId().toString() + threadId = ObjectId().toString() + const { response, body: message } = await ChatClient.sendMessage( projectId, threadId, @@ -25,20 +28,72 @@ describe('Editing a message', async function () { expect(response.statusCode).to.equal(201) expect(message.id).to.exist expect(message.content).to.equal(content) - const { response: response2 } = await ChatClient.editMessage( - projectId, - threadId, - message.id, - newContent - ) - expect(response2.statusCode).to.equal(204) + messageId = message.id }) - it('should then list the updated message in the threads', async function () { - const { response, body: threads } = await ChatClient.getThreads(projectId) - expect(response.statusCode).to.equal(200) - expect(threads[threadId].messages.length).to.equal(1) - expect(threads[threadId].messages[0].content).to.equal(newContent) + describe('without user', function () { + beforeEach(async function () { + const { response } = await ChatClient.editMessage( + projectId, + threadId, + messageId, + newContent + ) + expect(response.statusCode).to.equal(204) + }) + + it('should then list the updated message in the threads', async function () { + const { response, body: threads } = await ChatClient.getThreads( + projectId + ) + expect(response.statusCode).to.equal(200) + expect(threads[threadId].messages.length).to.equal(1) + expect(threads[threadId].messages[0].content).to.equal(newContent) + }) + }) + + describe('with the same user', function () { + beforeEach(async function () { + const { response } = await ChatClient.editMessageWithUser( + projectId, + threadId, + messageId, + userId, + newContent + ) + expect(response.statusCode).to.equal(204) + }) + + it('should then list the updated message in the threads', async function () { + const { response, body: threads } = await ChatClient.getThreads( + projectId + ) + expect(response.statusCode).to.equal(200) + expect(threads[threadId].messages.length).to.equal(1) + expect(threads[threadId].messages[0].content).to.equal(newContent) + }) + }) + + describe('with another user', function () { + beforeEach(async function () { + const { response } = await ChatClient.editMessageWithUser( + projectId, + threadId, + messageId, + ObjectId(), + newContent + ) + expect(response.statusCode).to.equal(204) + }) + + it('should then list the old message in the threads', async function () { + const { response, body: threads } = await ChatClient.getThreads( + projectId + ) + expect(response.statusCode).to.equal(200) + expect(threads[threadId].messages.length).to.equal(1) + expect(threads[threadId].messages[0].content).to.equal(content) + }) }) }) }) diff --git a/services/chat/test/acceptance/js/helpers/ChatClient.js b/services/chat/test/acceptance/js/helpers/ChatClient.js index 73239792dd..e79cef3209 100644 --- a/services/chat/test/acceptance/js/helpers/ChatClient.js +++ b/services/chat/test/acceptance/js/helpers/ChatClient.js @@ -86,6 +86,23 @@ async function editMessage(projectId, threadId, messageId, content) { }) } +async function editMessageWithUser( + projectId, + threadId, + messageId, + userId, + content +) { + return asyncRequest({ + method: 'post', + url: `/project/${projectId}/thread/${threadId}/messages/${messageId}/edit`, + json: { + content, + userId, + }, + }) +} + async function deleteMessage(projectId, threadId, messageId) { return asyncRequest({ method: 'delete', @@ -109,6 +126,7 @@ module.exports = { reopenThread, deleteThread, editMessage, + editMessageWithUser, deleteMessage, destroyProject, } diff --git a/services/web/app/src/Features/Chat/ChatApiHandler.js b/services/web/app/src/Features/Chat/ChatApiHandler.js index 408ba43121..e5cfeb74aa 100644 --- a/services/web/app/src/Features/Chat/ChatApiHandler.js +++ b/services/web/app/src/Features/Chat/ChatApiHandler.js @@ -151,7 +151,7 @@ module.exports = ChatApiHandler = { ) }, - editMessage(project_id, thread_id, message_id, content, callback) { + editMessage(project_id, thread_id, message_id, userId, content, callback) { if (callback == null) { callback = function () {} } @@ -161,6 +161,7 @@ module.exports = ChatApiHandler = { method: 'POST', json: { content, + userId, }, }, callback