mirror of
https://github.com/zulip/zulip.git
synced 2025-10-23 04:52:12 +00:00
submessages: Add verify_submessage_sender.
Before this change a rogue actor could try to widgetize another person's message. (The rogue actor would already have access to read the message.)
This commit is contained in:
@@ -2121,6 +2121,30 @@ def bulk_insert_ums(ums: List[UserMessageLite]) -> None:
|
||||
execute_values(cursor.cursor, query, vals)
|
||||
|
||||
|
||||
def verify_submessage_sender(
|
||||
*,
|
||||
message_id: int,
|
||||
message_sender_id: int,
|
||||
submessage_sender_id: int,
|
||||
) -> None:
|
||||
"""Even though our submessage architecture is geared toward
|
||||
collaboration among all message readers, we still enforce
|
||||
the the first person to attach a submessage to the message
|
||||
must be the original sender of the message.
|
||||
"""
|
||||
|
||||
if message_sender_id == submessage_sender_id:
|
||||
return
|
||||
|
||||
if SubMessage.objects.filter(
|
||||
message_id=message_id,
|
||||
sender_id=message_sender_id,
|
||||
).exists():
|
||||
return
|
||||
|
||||
raise JsonableError(_("You cannot attach a submessage to this message."))
|
||||
|
||||
|
||||
def do_add_submessage(
|
||||
realm: Realm,
|
||||
sender_id: int,
|
||||
|
@@ -1,6 +1,7 @@
|
||||
from typing import Any, Dict, List
|
||||
from unittest import mock
|
||||
|
||||
from zerver.lib.actions import do_add_submessage
|
||||
from zerver.lib.message import MessageDict
|
||||
from zerver.lib.test_classes import ZulipTestCase
|
||||
from zerver.models import Message, SubMessage
|
||||
@@ -99,6 +100,42 @@ class TestBasics(ZulipTestCase):
|
||||
result = self.client_post("/json/submessage", payload)
|
||||
self.assert_json_error(result, "Invalid message(s)")
|
||||
|
||||
def test_original_sender_enforced(self) -> None:
|
||||
cordelia = self.example_user("cordelia")
|
||||
hamlet = self.example_user("hamlet")
|
||||
stream_name = "Verona"
|
||||
|
||||
message_id = self.send_stream_message(
|
||||
sender=cordelia,
|
||||
stream_name=stream_name,
|
||||
)
|
||||
self.login_user(hamlet)
|
||||
|
||||
payload = dict(
|
||||
message_id=message_id,
|
||||
msg_type="whatever",
|
||||
content="{}",
|
||||
)
|
||||
|
||||
# Hamlet can't just go attaching submessages to Cordelia's
|
||||
# message, even though he does have read access here to the
|
||||
# message itself.
|
||||
result = self.client_post("/json/submessage", payload)
|
||||
self.assert_json_error(result, "You cannot attach a submessage to this message.")
|
||||
|
||||
# Since Hamlet is actually subcribed to the stream, he is welcome
|
||||
# to send submessages to Cordelia once she initiates the "subconversation".
|
||||
do_add_submessage(
|
||||
realm=cordelia.realm,
|
||||
sender_id=cordelia.id,
|
||||
message_id=message_id,
|
||||
msg_type="whatever",
|
||||
content="whatever",
|
||||
)
|
||||
|
||||
result = self.client_post("/json/submessage", payload)
|
||||
self.assert_json_success(result)
|
||||
|
||||
def test_endpoint_success(self) -> None:
|
||||
cordelia = self.example_user("cordelia")
|
||||
hamlet = self.example_user("hamlet")
|
||||
|
@@ -3,7 +3,7 @@ from django.http import HttpRequest, HttpResponse
|
||||
from django.utils.translation import gettext as _
|
||||
|
||||
from zerver.decorator import REQ, has_request_variables
|
||||
from zerver.lib.actions import do_add_submessage
|
||||
from zerver.lib.actions import do_add_submessage, verify_submessage_sender
|
||||
from zerver.lib.message import access_message
|
||||
from zerver.lib.response import json_error, json_success
|
||||
from zerver.lib.validator import check_int
|
||||
@@ -20,6 +20,12 @@ def process_submessage(
|
||||
) -> HttpResponse:
|
||||
message, user_message = access_message(user_profile, message_id)
|
||||
|
||||
verify_submessage_sender(
|
||||
message_id=message.id,
|
||||
message_sender_id=message.sender_id,
|
||||
submessage_sender_id=user_profile.id,
|
||||
)
|
||||
|
||||
try:
|
||||
orjson.loads(content)
|
||||
except orjson.JSONDecodeError:
|
||||
|
Reference in New Issue
Block a user