stream_bots: Allow bot owners to unsubscribe their bots from streams.

Users who owns bots can unsubscribe their bots from streams.

Fixes part of: #21402
This commit is contained in:
yogesh sirsat 2022-06-28 01:14:12 +05:30 committed by Tim Abbott
parent 45307affc0
commit 180a9cbdcb
5 changed files with 72 additions and 27 deletions

View File

@ -20,6 +20,11 @@ format used by the Zulip server that they are interacting with.
## Changes in Zulip 6.0 ## Changes in Zulip 6.0
**Feature level 145**
* [`DELETE users/me/subscriptions`](/api/unsubscribe): Normal users can
now remove bots that they own from streams.
**Feature level 144** **Feature level 144**
* [`GET /messages/{message_id}/read_receipts`](/api/get-read-receipts): * [`GET /messages/{message_id}/read_receipts`](/api/get-read-receipts):

View File

@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.4.3"
# Changes should be accompanied by documentation explaining what the # Changes should be accompanied by documentation explaining what the
# new level means in templates/zerver/api/changelog.md, as well as # new level means in templates/zerver/api/changelog.md, as well as
# "**Changes**" entries in the endpoint's documentation in `zulip.yaml`. # "**Changes**" entries in the endpoint's documentation in `zulip.yaml`.
API_FEATURE_LEVEL = 144 API_FEATURE_LEVEL = 145
# Bump the minor PROVISION_VERSION to indicate that folks should provision # 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 # only when going from an old version of the code to a newer version. Bump

View File

@ -7736,6 +7736,16 @@ paths:
tags: ["streams"] tags: ["streams"]
description: | description: |
Unsubscribe yourself or other users from one or more streams. 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.
**Changes**: Before Zulip 6.0 (feature level 145), only
organization administrators could remove other users from
streams.
x-curl-examples-parameters: x-curl-examples-parameters:
oneOf: oneOf:
- type: include - type: include

View File

@ -11,6 +11,7 @@ from django.core.exceptions import ValidationError
from django.http import HttpResponse from django.http import HttpResponse
from django.utils.timezone import now as timezone_now from django.utils.timezone import now as timezone_now
from zerver.actions.bots import do_change_bot_owner
from zerver.actions.create_realm import do_create_realm from zerver.actions.create_realm import do_create_realm
from zerver.actions.default_streams import ( from zerver.actions.default_streams import (
do_add_default_stream, do_add_default_stream,
@ -2236,12 +2237,12 @@ class StreamAdminTest(ZulipTestCase):
return result return result
def test_cant_remove_others_from_stream(self) -> None: def test_cant_remove_other_users_from_stream(self) -> None:
""" """
If you're not an admin, you can't remove other people from streams. If you're not an admin, you can't remove other people from streams except your own bots.
""" """
result = self.attempt_unsubscribe_of_principal( result = self.attempt_unsubscribe_of_principal(
query_count=6, query_count=7,
target_users=[self.example_user("cordelia")], target_users=[self.example_user("cordelia")],
is_realm_admin=False, is_realm_admin=False,
is_subbed=True, is_subbed=True,
@ -2279,10 +2280,11 @@ class StreamAdminTest(ZulipTestCase):
using a queue. using a queue.
""" """
target_users = [ target_users = [
self.example_user(name) for name in ["cordelia", "prospero", "iago", "hamlet", "ZOE"] self.example_user(name)
for name in ["cordelia", "prospero", "iago", "hamlet", "outgoing_webhook_bot"]
] ]
result = self.attempt_unsubscribe_of_principal( result = self.attempt_unsubscribe_of_principal(
query_count=26, query_count=27,
cache_count=9, cache_count=9,
target_users=target_users, target_users=target_users,
is_realm_admin=True, is_realm_admin=True,
@ -2331,7 +2333,7 @@ class StreamAdminTest(ZulipTestCase):
def test_cant_remove_others_from_stream_legacy_emails(self) -> None: def test_cant_remove_others_from_stream_legacy_emails(self) -> None:
result = self.attempt_unsubscribe_of_principal( result = self.attempt_unsubscribe_of_principal(
query_count=6, query_count=7,
is_realm_admin=False, is_realm_admin=False,
is_subbed=True, is_subbed=True,
invite_only=False, invite_only=False,
@ -2386,6 +2388,34 @@ class StreamAdminTest(ZulipTestCase):
self.assert_length(json["removed"], 0) self.assert_length(json["removed"], 0)
self.assert_length(json["not_removed"], 1) self.assert_length(json["not_removed"], 1)
def test_bot_owner_can_remove_bot_from_stream(self) -> None:
user_profile = self.example_user("hamlet")
webhook_bot = self.example_user("webhook_bot")
do_change_bot_owner(webhook_bot, bot_owner=user_profile, acting_user=user_profile)
result = self.attempt_unsubscribe_of_principal(
query_count=14,
target_users=[webhook_bot],
is_realm_admin=False,
is_subbed=True,
invite_only=False,
target_users_subbed=True,
)
self.assert_json_success(result)
def test_non_bot_owner_cannot_remove_bot_from_stream(self) -> None:
other_user = self.example_user("cordelia")
webhook_bot = self.example_user("webhook_bot")
do_change_bot_owner(webhook_bot, bot_owner=other_user, acting_user=other_user)
result = self.attempt_unsubscribe_of_principal(
query_count=8,
target_users=[webhook_bot],
is_realm_admin=False,
is_subbed=True,
invite_only=False,
target_users_subbed=True,
)
self.assert_json_error(result, "Insufficient permission")
def test_can_remove_subscribers_group(self) -> None: def test_can_remove_subscribers_group(self) -> None:
realm = get_realm("zulip") realm = get_realm("zulip")
leadership_group = create_user_group( leadership_group = create_user_group(

View File

@ -130,19 +130,14 @@ def principal_to_user_profile(agent: UserProfile, principal: Union[str, int]) ->
raise PrincipalError(principal) raise PrincipalError(principal)
def check_if_removing_someone_else( def user_directly_controls_user(user_profile: UserProfile, target: UserProfile) -> bool:
user_profile: UserProfile, principals: Optional[Union[List[str], List[int]]] """Returns whether the target user is either the current user or a bot
) -> bool: owned by the current user"""
if principals is None or len(principals) == 0: if user_profile == target:
return False
if len(principals) > 1:
return True return True
if target.is_bot and target.bot_owner == user_profile:
if isinstance(principals[0], int): return True
return principals[0] != user_profile.id return False
else:
return principals[0] != user_profile.email
def deactivate_stream_backend( def deactivate_stream_backend(
@ -464,23 +459,28 @@ def remove_subscriptions_backend(
) -> HttpResponse: ) -> HttpResponse:
realm = user_profile.realm realm = user_profile.realm
removing_someone_else = check_if_removing_someone_else(user_profile, principals)
streams_as_dict: List[StreamDict] = [] streams_as_dict: List[StreamDict] = []
for stream_name in streams_raw: for stream_name in streams_raw:
streams_as_dict.append({"name": stream_name.strip()}) streams_as_dict.append({"name": stream_name.strip()})
streams, __ = list_to_streams( unsubscribing_others = False
streams_as_dict, user_profile, unsubscribing_others=removing_someone_else
)
if principals: if principals:
people_to_unsub = { people_to_unsub = set()
principal_to_user_profile(user_profile, principal) for principal in principals for principal in principals:
} target_user = principal_to_user_profile(user_profile, principal)
people_to_unsub.add(target_user)
if not user_directly_controls_user(user_profile, target_user):
unsubscribing_others = True
else: else:
people_to_unsub = {user_profile} people_to_unsub = {user_profile}
streams, __ = list_to_streams(
streams_as_dict,
user_profile,
unsubscribing_others=unsubscribing_others,
)
result: Dict[str, List[str]] = dict(removed=[], not_removed=[]) result: Dict[str, List[str]] = dict(removed=[], not_removed=[])
(removed, not_subscribed) = bulk_remove_subscriptions( (removed, not_subscribed) = bulk_remove_subscriptions(
realm, people_to_unsub, streams, acting_user=user_profile realm, people_to_unsub, streams, acting_user=user_profile