diff --git a/api_docs/changelog.md b/api_docs/changelog.md index bfdf100d09..b2c70f9495 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -20,6 +20,14 @@ format used by the Zulip server that they are interacting with. ## Changes in Zulip 10.0 +**Feature level 360** + +* [`GET /messages/{message_id}`](/api/get-message), [`GET + /messages/{message_id}/read_receipts`](/api/get-read-receipts): + Messages from an archived channels can now be read through these API + endpoints, if the channel's access control permissions permit doing + so. + **Feature level 359** * `PATCH /bots/{bot_user_id}`: Previously, changing the owner of a bot diff --git a/version.py b/version.py index 7ab064cccd..7085f85bfd 100644 --- a/version.py +++ b/version.py @@ -34,7 +34,7 @@ DESKTOP_WARNING_VERSION = "5.9.3" # new level means in api_docs/changelog.md, as well as "**Changes**" # entries in the endpoint's documentation in `zulip.yaml`. -API_FEATURE_LEVEL = 359 # Last bumped for not adjusting bot subscriptions on owner change. +API_FEATURE_LEVEL = 360 # Last bumped for allowing access to archived channel messages on read. # 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 bd54249218..ef0c15dd89 100644 --- a/zerver/actions/message_edit.py +++ b/zerver/actions/message_edit.py @@ -1350,7 +1350,7 @@ def check_update_message( and raises a JsonableError if otherwise. It returns the number changed. """ - message = access_message(user_profile, message_id, lock_message=True) + message = access_message(user_profile, message_id, lock_message=True, is_modifying_message=True) # If there is a change to the content, check that it hasn't been too long # Allow an extra 20 seconds since we potentially allow editing 15 seconds diff --git a/zerver/actions/reactions.py b/zerver/actions/reactions.py index b1f72d2053..fec7f37dd3 100644 --- a/zerver/actions/reactions.py +++ b/zerver/actions/reactions.py @@ -106,7 +106,7 @@ def check_add_reaction( reaction_type: str | None, ) -> None: message, user_message = access_message_and_usermessage( - user_profile, message_id, lock_message=True + user_profile, message_id, lock_message=True, is_modifying_message=True ) if emoji_code is None or reaction_type is None: diff --git a/zerver/lib/message.py b/zerver/lib/message.py index 4f832ef352..bfff58e198 100644 --- a/zerver/lib/message.py +++ b/zerver/lib/message.py @@ -313,6 +313,8 @@ def access_message( user_profile: UserProfile, message_id: int, lock_message: bool = False, + *, + is_modifying_message: bool = False, ) -> Message: """You can access a message by ID in our APIs that either: (1) You received or have previously accessed via starring @@ -349,6 +351,7 @@ def access_message( message, has_user_message=has_user_message, user_group_membership_details=user_group_membership_details, + is_modifying_message=is_modifying_message, ): return message raise JsonableError(_("Invalid message(s)")) @@ -358,6 +361,8 @@ def access_message_and_usermessage( user_profile: UserProfile, message_id: int, lock_message: bool = False, + *, + is_modifying_message: bool = False, ) -> tuple[Message, UserMessage | None]: """As access_message, but also returns the usermessage, if any.""" try: @@ -379,6 +384,7 @@ def access_message_and_usermessage( message, has_user_message=has_user_message, user_group_membership_details=user_group_membership_details, + is_modifying_message=is_modifying_message, ): return (message, user_message) raise JsonableError(_("Invalid message(s)")) @@ -478,6 +484,7 @@ def has_message_access( stream: Stream | None = None, is_subscribed: bool | None = None, user_group_membership_details: UserGroupMembershipDetails, + is_modifying_message: bool = False, ) -> bool: """ Returns whether a user has access to a given message. @@ -500,7 +507,7 @@ def has_message_access( # You can't access public stream messages in other realms return False - if stream.deactivated: + if is_modifying_message and stream.deactivated: # You can't access messages in deactivated streams return False diff --git a/zerver/tests/test_message_delete.py b/zerver/tests/test_message_delete.py index f398db29f7..4c1860a3c0 100644 --- a/zerver/tests/test_message_delete.py +++ b/zerver/tests/test_message_delete.py @@ -8,6 +8,7 @@ from django.utils.timezone import now as timezone_now from zerver.actions.message_delete import do_delete_messages from zerver.actions.realm_settings import do_change_realm_permission_group_setting +from zerver.actions.streams import do_deactivate_stream from zerver.actions.user_groups import check_add_user_group from zerver.lib.test_classes import ZulipTestCase from zerver.models import Message, NamedUserGroup, UserProfile @@ -534,6 +535,20 @@ class DeleteMessageTest(ZulipTestCase): self.assert_json_success(result) self.assertFalse(Message.objects.filter(id=msg_id).exists()) + def test_delete_message_from_archived_channel(self) -> None: + hamlet = self.example_user("hamlet") + self.login("hamlet") + + stream = self.make_stream("stream1") + self.subscribe(hamlet, "stream1") + msg_id = self.send_stream_message( + hamlet, "stream1", topic_name="editing", content="before edit" + ) + do_deactivate_stream(stream, acting_user=hamlet) + + result = self.client_delete(f"/json/messages/{msg_id}") + self.assert_json_error(result, "Invalid message(s)") + def test_update_first_message_id_on_stream_message_deletion(self) -> None: realm = get_realm("zulip") stream_name = "test" diff --git a/zerver/tests/test_reactions.py b/zerver/tests/test_reactions.py index 2f3a5a4eb2..375fcee1d9 100644 --- a/zerver/tests/test_reactions.py +++ b/zerver/tests/test_reactions.py @@ -5,7 +5,11 @@ import orjson from typing_extensions import override from zerver.actions.reactions import notify_reaction_update -from zerver.actions.streams import do_change_stream_group_based_setting, do_change_stream_permission +from zerver.actions.streams import ( + do_change_stream_group_based_setting, + do_change_stream_permission, + do_deactivate_stream, +) from zerver.actions.user_groups import check_add_user_group from zerver.lib.cache import cache_get, to_dict_cache_key_id from zerver.lib.emoji import get_emoji_data @@ -405,6 +409,55 @@ class ReactionTest(ZulipTestCase): result = self.api_delete(sender, "/api/v1/messages/1/reactions", reaction_info) self.assert_json_success(result) + def test_adding_reaction_to_archived_channel(self) -> None: + """ + Should not be able to remove reaction from a message in an + archived channel. + """ + sender = self.example_user("hamlet") + + emoji = RealmEmoji.objects.get(name="green_tick") + + reaction_info = { + "emoji_name": "green_tick", + "emoji_code": str(emoji.id), + "reaction_type": "realm_emoji", + } + + stream_name = "Saxony" + stream = self.subscribe(self.example_user("cordelia"), stream_name) + message_id = self.send_stream_message(self.example_user("cordelia"), stream_name) + do_deactivate_stream(stream, acting_user=sender) + + result = self.api_post(sender, f"/api/v1/messages/{message_id}/reactions", reaction_info) + self.assert_json_error(result, "Invalid message(s)") + + def test_remove_existing_reaction_from_archived_channel(self) -> None: + """ + Should not be able to remove reaction from a message in an + archived channel. + """ + sender = self.example_user("hamlet") + + emoji = RealmEmoji.objects.get(name="green_tick") + + reaction_info = { + "emoji_name": "green_tick", + "emoji_code": str(emoji.id), + "reaction_type": "realm_emoji", + } + + stream_name = "Saxony" + stream = self.subscribe(self.example_user("cordelia"), stream_name) + message_id = self.send_stream_message(self.example_user("cordelia"), stream_name) + + result = self.api_post(sender, f"/api/v1/messages/{message_id}/reactions", reaction_info) + self.assert_json_success(result) + + do_deactivate_stream(stream, acting_user=sender) + result = self.api_delete(sender, f"/api/v1/messages/{message_id}/reactions", reaction_info) + self.assert_json_error(result, "Invalid message(s)") + class ReactionEventTest(ZulipTestCase): def test_add_event(self) -> None: diff --git a/zerver/views/message_edit.py b/zerver/views/message_edit.py index ca8270e49d..946dda3f81 100644 --- a/zerver/views/message_edit.py +++ b/zerver/views/message_edit.py @@ -205,7 +205,7 @@ def delete_message_backend( # concurrently are serialized properly with deleting the message; this prevents a deadlock # that would otherwise happen because of the other transaction holding a lock on the `Message` # row. - message = access_message(user_profile, message_id, lock_message=True) + message = access_message(user_profile, message_id, lock_message=True, is_modifying_message=True) validate_can_delete_message(user_profile, message) try: do_delete_messages(user_profile.realm, [message], acting_user=user_profile) diff --git a/zerver/views/reactions.py b/zerver/views/reactions.py index a9cfb1bee3..8817f17854 100644 --- a/zerver/views/reactions.py +++ b/zerver/views/reactions.py @@ -40,7 +40,7 @@ def remove_reaction( emoji_code: str | None = None, reaction_type: str = "unicode_emoji", ) -> HttpResponse: - message = access_message(user_profile, message_id, lock_message=True) + message = access_message(user_profile, message_id, lock_message=True, is_modifying_message=True) if emoji_code is None: if emoji_name is None: diff --git a/zerver/views/submessage.py b/zerver/views/submessage.py index dde2b33506..aadea2788a 100644 --- a/zerver/views/submessage.py +++ b/zerver/views/submessage.py @@ -26,7 +26,7 @@ def process_submessage( msg_type: str, content: str, ) -> HttpResponse: - message = access_message(user_profile, message_id, lock_message=True) + message = access_message(user_profile, message_id, lock_message=True, is_modifying_message=True) verify_submessage_sender( message_id=message.id, diff --git a/zerver/views/typing.py b/zerver/views/typing.py index f46ebeb556..6f5dadf131 100644 --- a/zerver/views/typing.py +++ b/zerver/views/typing.py @@ -80,7 +80,10 @@ def send_message_edit_notification_backend( operator: Annotated[Literal["start", "stop"], ApiParamConfig("op")], message_id: Json[int], ) -> HttpResponse: - message = access_message(user_profile, message_id) + # Technically, this endpoint doesn't modify the message, but we're + # attempting to send a typing notification that we're editing the + # message. + message = access_message(user_profile, message_id, is_modifying_message=True) validate_user_can_edit_message(user_profile, message, edit_limit_buffer=0) recipient = message.recipient