From 5a0553e18bcdda1b274f29a3003e2c1bcf808878 Mon Sep 17 00:00:00 2001
From: Mateusz Mandera <mateusz.mandera@zulip.com>
Date: Sat, 10 Apr 2021 17:47:11 +0200
Subject: [PATCH] message_edit: Verify the message is in a stream in move
 message API.

This wasn't being validated before. There wasn't any possibility to
actually succeed in moving a private message, because the codepath would
fail at assert message.is_stream_message() in do_update_message - but we
should have proper error handling for that case instead of internal
server errors.
---
 zerver/tests/test_message_edit.py | 25 ++++++++++++++++++++++++-
 zerver/views/message_edit.py      |  2 ++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/zerver/tests/test_message_edit.py b/zerver/tests/test_message_edit.py
index d266af93f2..35f641212c 100644
--- a/zerver/tests/test_message_edit.py
+++ b/zerver/tests/test_message_edit.py
@@ -17,7 +17,7 @@ from zerver.lib.message import MessageDict, has_message_access, messages_for_ids
 from zerver.lib.test_classes import ZulipTestCase
 from zerver.lib.test_helpers import queries_captured
 from zerver.lib.topic import LEGACY_PREV_TOPIC, TOPIC_NAME
-from zerver.models import Message, Stream, UserMessage, UserProfile, get_realm
+from zerver.models import Message, Stream, UserMessage, UserProfile, get_realm, get_stream
 
 
 class EditMessageTest(ZulipTestCase):
@@ -917,6 +917,29 @@ class EditMessageTest(ZulipTestCase):
 
         self.assert_json_error(result, "Invalid stream id")
 
+    def test_move_message_cant_move_private_message(
+        self,
+    ) -> None:
+        user_profile = self.example_user("iago")
+        self.assertEqual(user_profile.role, UserProfile.ROLE_REALM_ADMINISTRATOR)
+        self.login("iago")
+
+        hamlet = self.example_user("hamlet")
+        msg_id = self.send_personal_message(user_profile, hamlet)
+
+        verona = get_stream("Verona", user_profile.realm)
+
+        result = self.client_patch(
+            "/json/messages/" + str(msg_id),
+            {
+                "message_id": msg_id,
+                "stream_id": verona.id,
+                "propagate_mode": "change_all",
+            },
+        )
+
+        self.assert_json_error(result, "Message must be a stream message")
+
     def test_move_message_to_stream_change_later(self) -> None:
         (user_profile, old_stream, new_stream, msg_id, msg_id_later) = self.prepare_move_topics(
             "iago", "test move stream", "new stream", "test")
diff --git a/zerver/views/message_edit.py b/zerver/views/message_edit.py
index 1f9287bbdb..3f8cf0abcc 100644
--- a/zerver/views/message_edit.py
+++ b/zerver/views/message_edit.py
@@ -191,6 +191,8 @@ def update_message_backend(request: HttpRequest, user_profile: UserMessage,
     number_changed = 0
 
     if stream_id is not None:
+        if not message.is_stream_message():
+            raise JsonableError(_("Message must be a stream message"))
         if not user_profile.is_realm_admin:
             raise JsonableError(_("You don't have permission to move this message"))
         if content is not None: