From cbdb4e98e5bf2608e55e9dcb786f9fb751244154 Mon Sep 17 00:00:00 2001 From: Sahil Batra Date: Thu, 28 Jul 2022 20:33:42 +0530 Subject: [PATCH] message_edit: Topic editing permission should not depend on message sender. This commit changes the topic edit permssions to not depend whether the user editing the message had sent the message or it was sent by someone else. We only do backend changes in this commit and frontend changes will be done in further commits. Previously, we always allowed topic edits when the user themseleves had sent the message not considering the edit_topic_policy and the 3-day time limit. But now we consider all messages as same and editing is allowed only according to edit_topic_policy setting and the time limit of 3 days in addition for users who are not admins or moderators. --- templates/zerver/api/changelog.md | 2 ++ version.py | 2 +- zerver/actions/message_edit.py | 13 +++---------- zerver/openapi/zulip.yaml | 2 ++ zerver/tests/test_message_edit.py | 21 ++++++++++++++++++++- 5 files changed, 28 insertions(+), 12 deletions(-) diff --git a/templates/zerver/api/changelog.md b/templates/zerver/api/changelog.md index 30086bb534..fca6945518 100644 --- a/templates/zerver/api/changelog.md +++ b/templates/zerver/api/changelog.md @@ -30,6 +30,8 @@ format used by the Zulip server that they are interacting with. `move_messages_between_streams_policy`. * [`PATCH /messages/{message_id}`](/api/update-message): Permission to edit stream and topic of messages do not depend on `allow_message_editing` setting now. +* [`PATCH /messages/{message_id}`](/api/update-message): Message senders are not + allowed to edit topics indefinitely now. Feature levels 157-158 are reserved for future use in 6.x maintenance releases. diff --git a/version.py b/version.py index 3f6ce4a977..802dab6e98 100644 --- a/version.py +++ b/version.py @@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.4.3" # Changes should be accompanied by documentation explaining what the # new level means in templates/zerver/api/changelog.md, as well as # "**Changes**" entries in the endpoint's documentation in `zulip.yaml`. -API_FEATURE_LEVEL = 156 +API_FEATURE_LEVEL = 159 # Bump the minor PROVISION_VERSION to indicate that folks should provision # only when going from an old version of the code to a newer version. Bump diff --git a/zerver/actions/message_edit.py b/zerver/actions/message_edit.py index e8851a9e16..29c31640e2 100644 --- a/zerver/actions/message_edit.py +++ b/zerver/actions/message_edit.py @@ -105,14 +105,9 @@ def validate_message_edit_payload( def can_edit_topic( - message: Message, user_profile: UserProfile, is_no_topic_msg: bool, ) -> bool: - # You have permission to edit the message topic if you sent it. - if message.sender_id == user_profile.id: - return True - # We allow anyone to edit (no topic) messages to help tend them. if is_no_topic_msg: return True @@ -939,7 +934,7 @@ def check_update_message( is_no_topic_msg = message.topic_name() == "(no topic)" - if topic_name is not None and not can_edit_topic(message, user_profile, is_no_topic_msg): + if topic_name is not None and not can_edit_topic(user_profile, is_no_topic_msg): raise JsonableError(_("You don't have permission to edit this message")) # If there is a change to the content, check that it hasn't been too long @@ -954,12 +949,10 @@ def check_update_message( raise JsonableError(_("The time limit for editing this message has passed")) # If there is a change to the topic, check that the user is allowed to - # edit it and that it has not been too long. If this is not the user who - # sent the message, they are not the admin, and the time limit for editing - # topics is passed, raise an error. + # edit it and that it has not been too long. If user is not admin or moderator, + # and the time limit for editing topics is passed, raise an error. if ( topic_name is not None - and message.sender != user_profile and not user_profile.is_realm_admin and not user_profile.is_moderator and not is_no_topic_msg diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index aa65839a16..bee7c00e15 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -6464,6 +6464,8 @@ paths: streams and topics of messages was forbidden if `allow_message_editing` was `false`, regardless of the `edit_topic_policy` or `move_messages_between_streams_policy`. + Before Zulip 7.0 (feature level 159), message senders were allowed + to edit the topic indefinitely. [config-message-editing]: /help/configure-message-editing-and-deletion x-curl-examples-parameters: diff --git a/zerver/tests/test_message_edit.py b/zerver/tests/test_message_edit.py index b57ae24307..fbe6d42532 100644 --- a/zerver/tests/test_message_edit.py +++ b/zerver/tests/test_message_edit.py @@ -1126,23 +1126,38 @@ class EditMessageTest(EditMessageTestCase): set_message_editing_params(True, "unlimited", Realm.POLICY_FULL_MEMBERS_ONLY) cordelia = self.example_user("cordelia") + hamlet = self.example_user("hamlet") do_set_realm_property(cordelia.realm, "waiting_period_threshold", 10, acting_user=None) cordelia.date_joined = timezone_now() - datetime.timedelta(days=9) cordelia.save() + hamlet.date_joined = timezone_now() - datetime.timedelta(days=9) + hamlet.save() do_edit_message_assert_error( id_, "C", "You don't have permission to edit this message", "cordelia" ) + # User who sent the message but is not a full member cannot edit + # the topic + do_edit_message_assert_error( + id_, "C", "You don't have permission to edit this message", "hamlet" + ) cordelia.date_joined = timezone_now() - datetime.timedelta(days=11) cordelia.save() + hamlet.date_joined = timezone_now() - datetime.timedelta(days=11) + hamlet.save() do_edit_message_assert_success(id_, "C", "cordelia") + do_edit_message_assert_success(id_, "CD", "hamlet") # only moderators can edit topic of a message set_message_editing_params(True, "unlimited", Realm.POLICY_MODERATORS_ONLY) do_edit_message_assert_error( id_, "D", "You don't have permission to edit this message", "cordelia" ) + # even user who sent the message but is not a moderator cannot edit the topic. + do_edit_message_assert_error( + id_, "D", "You don't have permission to edit this message", "hamlet" + ) do_edit_message_assert_success(id_, "D", "shiva") # only admins can edit the topics of messages @@ -1165,7 +1180,8 @@ class EditMessageTest(EditMessageTestCase): set_message_editing_params(False, "unlimited", Realm.POLICY_EVERYONE) do_edit_message_assert_success(id_, "D", "cordelia") - # non-admin users cannot edit topics sent > 72 hrs ago + # non-admin users cannot edit topics sent > 72 hrs ago including + # sender of the message. message.date_sent = message.date_sent - datetime.timedelta(seconds=290000) message.save() set_message_editing_params(True, "unlimited", Realm.POLICY_EVERYONE) @@ -1174,6 +1190,9 @@ class EditMessageTest(EditMessageTestCase): do_edit_message_assert_error( id_, "G", "The time limit for editing this message's topic has passed", "cordelia" ) + do_edit_message_assert_error( + id_, "G", "The time limit for editing this message's topic has passed", "hamlet" + ) # anyone should be able to edit "no topic" indefinitely message.set_topic_name("(no topic)")