Merge pull request #7285 from overleaf/jpa-enforce-edit-own-comment

[misc] block users from editing other users comments

GitOrigin-RevId: 6f2ba38daf8089a478d79ca495b3557a57390b43

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)
 }
 

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,

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)
+      })
     })
   })
 })

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,
 }

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