From c3759814be23aed06230ad482a605d2c1d17c220 Mon Sep 17 00:00:00 2001 From: Sahil Batra Date: Fri, 16 Sep 2022 17:57:38 +0530 Subject: [PATCH] streams: Allow changing can_remove_subscribers_group through API. This commit adds API support to change can_remove_subscribers_group setting for a stream. --- api_docs/changelog.md | 6 ++++ zerver/lib/user_groups.py | 27 ++++++++++++++ zerver/openapi/zulip.yaml | 40 +++++++++++++++++---- zerver/tests/test_subs.py | 74 +++++++++++++++++++++++++++++++++++++++ zerver/views/streams.py | 17 +++++++++ 5 files changed, 157 insertions(+), 7 deletions(-) diff --git a/api_docs/changelog.md b/api_docs/changelog.md index 4bcf7e91c2..f7ee3acf37 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -20,6 +20,12 @@ format used by the Zulip server that they are interacting with. ## Changes in Zulip 7.0 +**Feature level 161** + +* [`PATCH /streams/{stream_id}`](/api/update-stream): Added + `can_remove_subscribers_group_id` parameter to support + changing `can_remove_subscribers_group` setting. + **Feature level 160** * [`POST /api/v1/jwt/fetch_api_key`]: New API endpoint to fetch API diff --git a/zerver/lib/user_groups.py b/zerver/lib/user_groups.py index fe1697e1e9..1d2810a361 100644 --- a/zerver/lib/user_groups.py +++ b/zerver/lib/user_groups.py @@ -58,6 +58,33 @@ def access_user_groups_as_potential_subgroups( return list(user_groups) +def access_user_group_for_setting( + user_group_id: int, + user_profile: UserProfile, + *, + setting_name: str, + require_system_group: bool = False, + allow_internet_group: bool = False, + allow_owners_group: bool = False, +) -> UserGroup: + user_group = access_user_group_by_id(user_group_id, user_profile, for_read=True) + + if require_system_group and not user_group.is_system_group: + raise JsonableError(_("'{}' must be a system user group.").format(setting_name)) + + if not allow_internet_group and user_group.name == UserGroup.EVERYONE_ON_INTERNET_GROUP_NAME: + raise JsonableError( + _("'{}' setting cannot be set to '@role:internet' group.").format(setting_name) + ) + + if not allow_owners_group and user_group.name == UserGroup.OWNERS_GROUP_NAME: + raise JsonableError( + _("'{}' setting cannot be set to '@role:owners' group.").format(setting_name) + ) + + return user_group + + def user_groups_in_realm_serialized(realm: Realm) -> List[UserGroupDict]: """This function is used in do_events_register code path so this code should be performant. We need to do 2 database queries because diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 79bb76f561..1d21a211e9 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -8027,14 +8027,23 @@ paths: Unsubscribe yourself or other users from one or more streams. In addition to managing the current user's subscriptions, this - endpoint can be used by organization administrators to remove - other users from streams, or to remove a bot that the current - user is the `bot_owner` for from any stream that the current - user can access. + endpoint can be used to remove other users from streams. This + is possible in 3 situations: - **Changes**: Before Zulip 6.0 (feature level 145), only - organization administrators could remove other users from - streams. + - Organization administrators can remove any user from any + stream. + - Users can remove a bot that they own from any stream that + the user can access. + - Users who can access a stream and are in the group with ID + `can_remove_subscribers_group_id` for that stream can + unsubscribe any user from that stream. + + **Changes**: Before Zulip 7.0 (feature level 161), + `can_remove_subscribers_group_id` was always the system group + for organization administrators. + + Before Zulip 6.0 (feature level 145), users had no special + privileges for managing bots that they own. x-curl-examples-parameters: oneOf: - type: include @@ -14536,6 +14545,7 @@ paths: required: false - $ref: "#/components/parameters/StreamPostPolicy" - $ref: "#/components/parameters/MessageRetentionDays" + - $ref: "#/components/parameters/CanRemoveSubscribersGroupId" responses: "200": $ref: "#/components/responses/SimpleSuccess" @@ -17562,6 +17572,22 @@ components: - type: integer example: "20" required: false + CanRemoveSubscribersGroupId: + name: can_remove_subscribers_group_id + in: query + description: | + ID of the user group whose members are allowed to unsubscribe others + from the stream, if they have access to this stream, even if + they are not an organization administrator. + + This setting can currently only be set to system user groups + except `@role:internet` and `@role:owners` group. + + **Changes**: New in Zulip 7.0 (feature level 161). + schema: + type: integer + example: 20 + required: false LinkifierPattern: name: pattern in: query diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index 82ffbe5111..5171543e33 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -1977,6 +1977,80 @@ class StreamAdminTest(ZulipTestCase): stream = get_stream("stream_name1", realm) self.assertEqual(stream.message_retention_days, 2) + def test_change_stream_can_remove_subscribers_group(self) -> None: + user_profile = self.example_user("iago") + realm = user_profile.realm + stream = self.subscribe(user_profile, "stream_name1") + + moderators_system_group = UserGroup.objects.get( + name="@role:moderators", realm=realm, is_system_group=True + ) + self.login("shiva") + result = self.client_patch( + f"/json/streams/{stream.id}", + {"can_remove_subscribers_group_id": orjson.dumps(moderators_system_group.id).decode()}, + ) + self.assert_json_error(result, "Must be an organization administrator") + + self.login("iago") + result = self.client_patch( + f"/json/streams/{stream.id}", + {"can_remove_subscribers_group_id": orjson.dumps(moderators_system_group.id).decode()}, + ) + self.assert_json_success(result) + stream = get_stream("stream_name1", realm) + self.assertEqual(stream.can_remove_subscribers_group.id, moderators_system_group.id) + + # This setting can only be set to system groups. + hamletcharacters_group = UserGroup.objects.get(name="hamletcharacters", realm=realm) + result = self.client_patch( + f"/json/streams/{stream.id}", + {"can_remove_subscribers_group_id": orjson.dumps(hamletcharacters_group.id).decode()}, + ) + self.assert_json_error( + result, "'can_remove_subscribers_group' must be a system user group." + ) + + internet_group = UserGroup.objects.get( + name="@role:internet", is_system_group=True, realm=realm + ) + result = self.client_patch( + f"/json/streams/{stream.id}", + {"can_remove_subscribers_group_id": orjson.dumps(internet_group.id).decode()}, + ) + self.assert_json_error( + result, + "'can_remove_subscribers_group' setting cannot be set to '@role:internet' group.", + ) + + owners_group = UserGroup.objects.get(name="@role:owners", is_system_group=True, realm=realm) + result = self.client_patch( + f"/json/streams/{stream.id}", + {"can_remove_subscribers_group_id": orjson.dumps(owners_group.id).decode()}, + ) + self.assert_json_error( + result, + "'can_remove_subscribers_group' setting cannot be set to '@role:owners' group.", + ) + + # For private streams, even admins must be subscribed to the stream to change + # can_remove_subscribers_group setting. + stream = self.make_stream("stream_name2", invite_only=True) + result = self.client_patch( + f"/json/streams/{stream.id}", + {"can_remove_subscribers_group_id": orjson.dumps(moderators_system_group.id).decode()}, + ) + self.assert_json_error(result, "Invalid stream ID") + + self.subscribe(user_profile, "stream_name2") + result = self.client_patch( + f"/json/streams/{stream.id}", + {"can_remove_subscribers_group_id": orjson.dumps(moderators_system_group.id).decode()}, + ) + self.assert_json_success(result) + stream = get_stream("stream_name2", realm) + self.assertEqual(stream.can_remove_subscribers_group.id, moderators_system_group.id) + def test_stream_message_retention_days_on_stream_creation(self) -> None: """ Only admins can create streams with message_retention_days diff --git a/zerver/views/streams.py b/zerver/views/streams.py index 89f7ad7cce..f8164154c9 100644 --- a/zerver/views/streams.py +++ b/zerver/views/streams.py @@ -30,6 +30,7 @@ from zerver.actions.message_send import ( from zerver.actions.streams import ( bulk_add_subscriptions, bulk_remove_subscriptions, + do_change_can_remove_subscribers_group, do_change_stream_description, do_change_stream_message_retention_days, do_change_stream_permission, @@ -79,6 +80,7 @@ from zerver.lib.topic import ( messages_for_topic, ) from zerver.lib.types import Validator +from zerver.lib.user_groups import access_user_group_for_setting from zerver.lib.utils import assert_is_not_none from zerver.lib.validator import ( check_bool, @@ -266,6 +268,7 @@ def update_stream_backend( message_retention_days: Optional[Union[int, str]] = REQ( json_validator=check_string_or_int, default=None ), + can_remove_subscribers_group_id: Optional[int] = REQ(json_validator=check_int, default=None), ) -> HttpResponse: # We allow realm administrators to to update the stream name and # description even for private streams. @@ -380,6 +383,20 @@ def update_stream_backend( if stream_post_policy is not None: do_change_stream_post_policy(stream, stream_post_policy, acting_user=user_profile) + if can_remove_subscribers_group_id is not None: + if sub is None and stream.invite_only: + # Admins cannot change this setting for unsubscribed private streams. + raise JsonableError(_("Invalid stream ID")) + + user_group = access_user_group_for_setting( + can_remove_subscribers_group_id, + user_profile, + setting_name="can_remove_subscribers_group", + require_system_group=True, + ) + + do_change_can_remove_subscribers_group(stream, user_group, acting_user=user_profile) + return json_success(request)