paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-11-03 13:33:24 +00:00
Code Issues Packages Projects Releases Wiki Activity

message: Take into account usergroups for has_message_access.

Browse Source
This commit is contained in:
Shubham Padia
2025-02-16 21:23:07 +00:00
committed by Tim Abbott
parent 9d08063208
commit a260ae8e57
7 changed files with 278 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
api_docs/changelog.md
View File

@@ -20,6 +20,32 @@ format used by the Zulip server that they are interacting with.
## Changes in Zulip 10.0
**Feature level 354**
* [`GET /messages`](/api/get-messages), [`GET
/messages/{message_id}`](/api/get-message), [`POST
/messages/flags/narrow`]: Users can access messages in unsubscribed
private channels that are accessible only via groups that grant
content access.
* [`GET /messages/{message_id}/read_receipts`](/api/get-read-receipts):
Users can access read receipts in unsubscribed private channels that are
accessible only via groups that grant content access.
* [`POST /messages/{message_id}/reactions`](/api/add-reaction),
[`DELETE /messages/{message_id}/reactions`](/api/remove-reaction):
Users can react to messages in unsubscribed private channels that are
accessible only via groups that grant content access.
* `POST /submessage`: Users can interact with polls and similar
widgets in messages in unsubscribed private channels that are
accessible only via groups that grant content access.
* [`PATCH /messages/{message_id}`](/api/update-message): Users can
edit messages they have posted in unsubscribed private channels that
are accessible only via groups that grant content access.
* [`POST
/message_edit_typing`](/api/set-typing-status-for-message-edit):
Users can generate typing notifications when editing messages in
unsubscribed private channels that are accessible only via groups
that grant content access.
**Feature level 353**
* [`POST /register`](/api/register-queue), [`GET /events`](/api/get-events),

2
version.py
View File

@@ -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 = 353 # Last bumped for Zoom server to server video chat option.
API_FEATURE_LEVEL = 354 # Last bumped for accounting groups for has_message_access.
# 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

100
zerver/lib/message.py
View File

@@ -27,7 +27,12 @@ from zerver.lib.stream_subscription import (
get_subscribed_stream_recipient_ids_for_user,
num_subscribers_for_stream_id,
)
from zerver.lib.streams import can_access_stream_history, get_web_public_streams_queryset
from zerver.lib.streams import (
UserGroupMembershipDetails,
can_access_stream_history,
get_web_public_streams_queryset,
is_user_in_groups_granting_content_access,
)
from zerver.lib.topic import (
MESSAGE__TOPIC,
TOPIC_NAME,
@@ -35,7 +40,10 @@ from zerver.lib.topic import (
messages_for_topic,
)
from zerver.lib.types import UserDisplayRecipient
from zerver.lib.user_groups import user_has_permission_for_group_setting
from zerver.lib.user_groups import (
get_recursive_membership_groups,
user_has_permission_for_group_setting,
)
from zerver.lib.user_topics import build_get_topic_visibility_policy, get_topic_visibility_policy
from zerver.lib.users import get_inaccessible_user_ids
from zerver.models import (
@@ -304,7 +312,13 @@ def access_message(
user_profile=user_profile, message_id=message_id
).exists()
if has_message_access(user_profile, message, has_user_message=has_user_message):
user_group_membership_details = UserGroupMembershipDetails(user_recursive_group_ids=None)
if has_message_access(
user_profile,
message,
has_user_message=has_user_message,
user_group_membership_details=user_group_membership_details,
):
return message
raise JsonableError(_("Invalid message(s)"))
@@ -328,7 +342,13 @@ def access_message_and_usermessage(
user_message = get_usermessage_by_message_id(user_profile, message_id)
has_user_message = lambda: user_message is not None
if has_message_access(user_profile, message, has_user_message=has_user_message):
user_group_membership_details = UserGroupMembershipDetails(user_recursive_group_ids=None)
if has_message_access(
user_profile,
message,
has_user_message=has_user_message,
user_group_membership_details=user_group_membership_details,
):
return (message, user_message)
raise JsonableError(_("Invalid message(s)"))
@@ -375,6 +395,50 @@ def access_web_public_message(
return message
def has_channel_content_access_helper(
stream: Stream,
user_profile: UserProfile,
user_group_membership_details: UserGroupMembershipDetails,
*,
is_subscribed: bool | None,
) -> bool:
"""
Checks whether a user has content access to a channel specifically
via being subscribed or group membership.
Does not consider the implicit permissions associated with web-public
or public channels; callers are responsible for that.
"""
if is_subscribed is None:
assert stream.recipient_id is not None
is_user_subscribed = Subscription.objects.filter(
user_profile=user_profile, active=True, recipient_id=stream.recipient_id
).exists()
else:
is_user_subscribed = is_subscribed
if is_user_subscribed:
return True
if user_profile.is_guest:
# All existing groups granting content access have allow_everyone_group=False.
#
# TODO: is_user_in_groups_granting_content_access needs to
# accept at least `is_guest`, and maybe just the user, when we
# have groups granting content access with
# allow_everyone_group=True.
return False
if user_group_membership_details.user_recursive_group_ids is None:
user_group_membership_details.user_recursive_group_ids = set(
get_recursive_membership_groups(user_profile).values_list("id", flat=True)
)
return is_user_in_groups_granting_content_access(
stream, user_group_membership_details.user_recursive_group_ids
)
def has_message_access(
user_profile: UserProfile,
message: Message,
@@ -382,6 +446,7 @@ def has_message_access(
has_user_message: Callable[[], bool],
stream: Stream | None = None,
is_subscribed: bool | None = None,
user_group_membership_details: UserGroupMembershipDetails,
) -> bool:
"""
Returns whether a user has access to a given message.
@@ -408,14 +473,6 @@ def has_message_access(
# You can't access messages in deactivated streams
return False
def is_subscribed_helper() -> bool:
if is_subscribed is not None:
return is_subscribed
return Subscription.objects.filter(
user_profile=user_profile, active=True, recipient=message.recipient
).exists()
if stream.is_public() and user_profile.can_access_public_streams():
return True
@@ -424,10 +481,14 @@ def has_message_access(
# (1) Have directly received the message.
# AND
# (2) Be subscribed to the stream.
return has_user_message() and is_subscribed_helper()
return has_user_message() and has_channel_content_access_helper(
stream, user_profile, user_group_membership_details, is_subscribed=is_subscribed
)
# is_history_public_to_subscribers, so check if you're subscribed
return is_subscribed_helper()
return has_channel_content_access_helper(
stream, user_profile, user_group_membership_details, is_subscribed=is_subscribed
)
def event_recipient_ids_for_action_on_messages(
@@ -546,6 +607,7 @@ def bulk_access_messages(
subscribed_recipient_ids = set(get_subscribed_stream_recipient_ids_for_user(user_profile))
user_group_membership_details = UserGroupMembershipDetails(user_recursive_group_ids=None)
for message in messages:
is_subscribed = message.recipient_id in subscribed_recipient_ids
if has_message_access(
@@ -554,6 +616,7 @@ def bulk_access_messages(
has_user_message=partial(lambda m: m.id in user_message_set, message),
stream=streams.get(message.recipient_id) if stream is None else stream,
is_subscribed=is_subscribed,
user_group_membership_details=user_group_membership_details,
):
filtered_messages.append(message)
return filtered_messages
@@ -577,9 +640,12 @@ def bulk_access_stream_messages_query(
if stream.is_public() and user_profile.can_access_public_streams():
return messages
if not Subscription.objects.filter(
user_profile=user_profile, active=True, recipient=stream.recipient
).exists():
user_group_membership_details = UserGroupMembershipDetails(user_recursive_group_ids=None)
has_content_access = has_channel_content_access_helper(
stream, user_profile, user_group_membership_details, is_subscribed=None
)
if not has_content_access:
return Message.objects.none()
if not stream.is_history_public_to_subscribers():
messages = messages.alias(

28
zerver/tests/test_message_edit.py
View File

@@ -11,7 +11,7 @@ from zerver.actions.realm_settings import (
do_change_realm_plan_type,
do_set_realm_property,
)
from zerver.actions.streams import do_deactivate_stream
from zerver.actions.streams import do_change_stream_group_based_setting, do_deactivate_stream
from zerver.actions.user_groups import add_subgroups_to_user_group, check_add_user_group
from zerver.actions.user_topics import do_set_user_topic_visibility_policy
from zerver.lib.message import messages_for_ids
@@ -428,7 +428,9 @@ class EditMessageTest(ZulipTestCase):
hamlet = self.example_user("hamlet")
self.login("hamlet")
self.make_stream("privatestream", invite_only=True, history_public_to_subscribers=False)
stream = self.make_stream(
"privatestream", invite_only=True, history_public_to_subscribers=False
)
self.subscribe(hamlet, "privatestream")
msg_id = self.send_stream_message(
hamlet, "privatestream", topic_name="editing", content="before edit"
@@ -456,6 +458,28 @@ class EditMessageTest(ZulipTestCase):
content = Message.objects.get(id=msg_id).content
self.assertEqual(content, "test can edit before unsubscribing")
hamlet_group = check_add_user_group(
hamlet.realm,
"prospero_group",
[hamlet],
acting_user=hamlet,
)
do_change_stream_group_based_setting(
stream,
"can_add_subscribers_group",
hamlet_group,
acting_user=None,
)
result = self.client_patch(
f"/json/messages/{msg_id}",
{
"content": "having content access after unsubscribing",
},
)
self.assert_json_success(result)
content = Message.objects.get(id=msg_id).content
self.assertEqual(content, "having content access after unsubscribing")
def test_edit_message_guest_in_unsubscribed_public_stream(self) -> None:
guest_user = self.example_user("polonius")
self.login("polonius")

45
zerver/tests/test_message_flags.py
View File

@@ -6,7 +6,8 @@ from django.db import connection
from typing_extensions import override
from zerver.actions.message_flags import do_update_message_flags
from zerver.actions.streams import do_change_stream_permission
from zerver.actions.streams import do_change_stream_group_based_setting, do_change_stream_permission
from zerver.actions.user_groups import check_add_user_group
from zerver.actions.user_topics import do_set_user_topic_visibility_policy
from zerver.lib.fix_unreads import fix, fix_unsubscribed
from zerver.lib.message import (
@@ -35,6 +36,7 @@ from zerver.models import (
UserProfile,
UserTopic,
)
from zerver.models.groups import NamedUserGroup
from zerver.models.realms import get_realm
from zerver.models.streams import get_stream
@@ -1807,14 +1809,55 @@ class MessageAccessTests(ZulipTestCase):
# Testing messages accessibility for an unsubscribed user
unsubscribed_user = self.example_user("ZOE")
unsubscribed_guest = self.example_user("polonius")
filtered_messages = self.assert_bulk_access(
unsubscribed_user,
message_ids,
stream,
bulk_access_messages_query_count=5,
bulk_access_stream_messages_query_count=2,
)
self.assert_length(filtered_messages, 0)
# Testing messages accessibility for an unsubscribed user
# present in `can_add_subscribers_group`
unsubscribed_user_group = check_add_user_group(
unsubscribed_user.realm,
"unsubscribed_user_group",
[unsubscribed_user, unsubscribed_guest],
acting_user=unsubscribed_user,
)
do_change_stream_group_based_setting(
stream,
"can_add_subscribers_group",
unsubscribed_user_group,
acting_user=None,
)
filtered_messages = self.assert_bulk_access(
unsubscribed_user,
message_ids,
stream,
bulk_access_messages_query_count=5,
bulk_access_stream_messages_query_count=3,
)
self.assert_length(filtered_messages, 2)
filtered_messages = self.assert_bulk_access(
unsubscribed_guest,
message_ids,
stream,
bulk_access_messages_query_count=4,
bulk_access_stream_messages_query_count=1,
)
self.assert_length(filtered_messages, 0)
nobody_group = NamedUserGroup.objects.get(
name="role:nobody", is_system_group=True, realm=unsubscribed_user.realm
)
do_change_stream_group_based_setting(
stream,
"can_add_subscribers_group",
nobody_group,
acting_user=None,
)
# Adding more message ids to the list increases the query size
# for bulk_access_messages but not

37
zerver/tests/test_message_move_stream.py
View File

@@ -13,6 +13,7 @@ from zerver.actions.streams import do_change_stream_group_based_setting, do_chan
from zerver.actions.user_groups import check_add_user_group
from zerver.lib.message import has_message_access
from zerver.lib.streams import (
UserGroupMembershipDetails,
can_access_stream_metadata_user_ids,
update_stream_active_status_for_realm,
)
@@ -1314,6 +1315,9 @@ class MessageMoveStreamTest(ZulipTestCase):
has_user_message=lambda: has_user_message,
stream=stream,
is_subscribed=is_subscribed,
user_group_membership_details=UserGroupMembershipDetails(
user_recursive_group_ids=None
),
),
has_access,
)
@@ -1710,6 +1714,9 @@ class MessageMoveStreamTest(ZulipTestCase):
has_user_message=lambda: True,
stream=old_stream,
is_subscribed=True,
user_group_membership_details=UserGroupMembershipDetails(
user_recursive_group_ids=None
),
),
True,
)
@@ -1726,6 +1733,9 @@ class MessageMoveStreamTest(ZulipTestCase):
has_user_message=lambda: True,
stream=old_stream,
is_subscribed=False,
user_group_membership_details=UserGroupMembershipDetails(
user_recursive_group_ids=None
),
),
False,
)
@@ -1750,6 +1760,9 @@ class MessageMoveStreamTest(ZulipTestCase):
has_user_message=lambda: True,
stream=new_stream,
is_subscribed=False,
user_group_membership_details=UserGroupMembershipDetails(
user_recursive_group_ids=None
),
),
False,
)
@@ -1784,6 +1797,9 @@ class MessageMoveStreamTest(ZulipTestCase):
has_user_message=lambda: True,
stream=old_stream,
is_subscribed=True,
user_group_membership_details=UserGroupMembershipDetails(
user_recursive_group_ids=None
),
),
True,
)
@@ -1800,6 +1816,9 @@ class MessageMoveStreamTest(ZulipTestCase):
has_user_message=lambda: True,
stream=old_stream,
is_subscribed=False,
user_group_membership_details=UserGroupMembershipDetails(
user_recursive_group_ids=None
),
),
False,
)
@@ -1817,6 +1836,9 @@ class MessageMoveStreamTest(ZulipTestCase):
has_user_message=lambda: False,
stream=old_stream,
is_subscribed=False,
user_group_membership_details=UserGroupMembershipDetails(
user_recursive_group_ids=None
),
),
False,
)
@@ -1843,6 +1865,9 @@ class MessageMoveStreamTest(ZulipTestCase):
has_user_message=lambda: True,
stream=new_stream,
is_subscribed=True,
user_group_membership_details=UserGroupMembershipDetails(
user_recursive_group_ids=None
),
),
True,
)
@@ -1853,6 +1878,9 @@ class MessageMoveStreamTest(ZulipTestCase):
has_user_message=lambda: True,
stream=new_stream,
is_subscribed=True,
user_group_membership_details=UserGroupMembershipDetails(
user_recursive_group_ids=None
),
),
True,
)
@@ -1884,6 +1912,9 @@ class MessageMoveStreamTest(ZulipTestCase):
has_user_message=lambda: False,
stream=old_stream,
is_subscribed=False,
user_group_membership_details=UserGroupMembershipDetails(
user_recursive_group_ids=None
),
),
False,
)
@@ -1901,6 +1932,9 @@ class MessageMoveStreamTest(ZulipTestCase):
has_user_message=lambda: False,
stream=old_stream,
is_subscribed=False,
user_group_membership_details=UserGroupMembershipDetails(
user_recursive_group_ids=None
),
),
False,
)
@@ -1926,6 +1960,9 @@ class MessageMoveStreamTest(ZulipTestCase):
has_user_message=lambda: True,
stream=new_stream,
is_subscribed=True,
user_group_membership_details=UserGroupMembershipDetails(
user_recursive_group_ids=None
),
),
True,
)

63
zerver/tests/test_reactions.py
View File

@@ -5,7 +5,8 @@ import orjson
from typing_extensions import override
from zerver.actions.reactions import notify_reaction_update
from zerver.actions.streams import do_change_stream_permission
from zerver.actions.streams import do_change_stream_group_based_setting, do_change_stream_permission
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
from zerver.lib.exceptions import JsonableError
@@ -14,6 +15,7 @@ from zerver.lib.test_classes import ZulipTestCase
from zerver.lib.test_helpers import zulip_reaction_info
from zerver.models import Message, Reaction, RealmEmoji, UserMessage
from zerver.models.realms import get_realm
from zerver.models.streams import Subscription
if TYPE_CHECKING:
from django.test.client import _MonkeyPatchedWSGIResponse as TestHttpResponse
@@ -864,10 +866,15 @@ class DefaultEmojiReactionTests(EmojiReactionBase):
Reacting with valid emoji on a historical message succeeds.
"""
stream_name = "Saxony"
self.subscribe(self.example_user("cordelia"), stream_name)
stream = self.subscribe(self.example_user("cordelia"), stream_name)
message_id = self.send_stream_message(self.example_user("cordelia"), stream_name)
user_profile = self.example_user("hamlet")
is_user_profile_a_subscriber = Subscription.objects.filter(
user_profile=user_profile,
recipient__type_id=stream.id,
).exists()
self.assertEqual(is_user_profile_a_subscriber, False)
# Verify that hamlet did not receive the message.
self.assertFalse(
@@ -892,6 +899,58 @@ class DefaultEmojiReactionTests(EmojiReactionBase):
self.assertTrue(user_message.flags.read)
self.assertFalse(user_message.flags.starred)
def test_react_unsubscribed_private_stream(self) -> None:
"""
Test reacting with valid emoji on a private stream.
"""
stream_name = "new_private_stream"
user_profile = self.example_user("hamlet")
stream = self.make_stream(stream_name, user_profile.realm, invite_only=True)
self.subscribe(user_profile, stream_name)
message_id = self.send_stream_message(user_profile, stream_name)
# Have hamlet react to the message
reaction_info = {
"reaction_type": "unicode_emoji",
"emoji_name": "hamburger",
"emoji_code": "1f354",
}
result = self.api_post(
user_profile, f"/api/v1/messages/{message_id}/reactions", reaction_info
)
self.assert_json_success(result)
# Unsubscribed user without content access should not be able
# to react
reaction_info = {
"reaction_type": "unicode_emoji",
"emoji_name": "smile",
}
self.unsubscribe(user_profile, stream_name)
result = self.api_post(
user_profile, f"/api/v1/messages/{message_id}/reactions", reaction_info
)
self.assert_json_error(result, "Invalid message(s)")
# Unsubscribed user with content access should be able to react
user_profile_group = check_add_user_group(
user_profile.realm,
"prospero_group",
[user_profile],
acting_user=user_profile,
)
do_change_stream_group_based_setting(
stream,
"can_add_subscribers_group",
user_profile_group,
acting_user=None,
)
result = self.api_post(
user_profile, f"/api/v1/messages/{message_id}/reactions", reaction_info
)
self.assert_json_success(result)
class ZulipExtraEmojiReactionTest(EmojiReactionBase):
def test_add_zulip_emoji_reaction(self) -> None: