message: Allow accessing archived channel when not modifying message.

Fixes #33567.

We have used the flag `is_modifying_message` since it's more generic
than an archived channel specific flag and helps us understand better
what is the condition where we do not want to allow archived channels.
We have not added tests for message edit since it  has an existing test
for this.

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

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

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

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:

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
 

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"

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:

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)

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:

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,

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