streams: Allow admins to unsubscribe others irrespective of setting.

This commit is contained in:
Sahil Batra
2024-11-21 14:54:24 +05:30
committed by Tim Abbott
parent 02049fbbf3
commit d1e76a9281
5 changed files with 51 additions and 14 deletions

View File

@@ -565,6 +565,10 @@ export function can_unsubscribe_others(sub: StreamSubscription): boolean {
return false;
}
if (current_user.is_admin) {
return true;
}
return user_groups.is_user_in_setting_group(
sub.can_remove_subscribers_group,
people.my_current_user_id(),

View File

@@ -1154,8 +1154,15 @@ test("can_unsubscribe_others", ({override}) => {
is_system_group: true,
direct_subgroup_ids: new Set([]),
};
const students = {
name: "Students",
id: 5,
members: new Set([member_user_id]),
is_system_group: false,
direct_subgroup_ids: new Set([]),
};
user_groups.initialize({realm_user_groups: [admins, moderators, all, nobody]});
user_groups.initialize({realm_user_groups: [admins, moderators, all, nobody, students]});
const sub = {
name: "Denmark",
@@ -1187,10 +1194,17 @@ test("can_unsubscribe_others", ({override}) => {
people.initialize_current_user(member_user_id);
assert.equal(stream_data.can_unsubscribe_others(sub), true);
// With the nobody system group, admins cannot unsubscribe others.
sub.can_remove_subscribers_group = nobody.id;
// With the setting set to user defined group not including admin,
// admin can still unsubscribe others.
sub.can_remove_subscribers_group = students.id;
override(current_user, "is_admin", true);
people.initialize_current_user(admin_user_id);
assert.equal(stream_data.can_unsubscribe_others(sub), true);
override(current_user, "is_admin", false);
people.initialize_current_user(moderator_user_id);
assert.equal(stream_data.can_unsubscribe_others(sub), false);
people.initialize_current_user(member_user_id);
assert.equal(stream_data.can_unsubscribe_others(sub), true);
// This isn't a real state, but we want coverage on !can_view_subscribers.
sub.can_remove_subscribers_group = all.id;

View File

@@ -675,6 +675,9 @@ def can_remove_subscribers_from_stream(
if not check_basic_stream_access(user_profile, stream, sub, allow_realm_admin=True):
return False
if user_profile.is_realm_admin:
return True
group_allowed_to_remove_subscribers = stream.can_remove_subscribers_group
assert group_allowed_to_remove_subscribers is not None
return user_has_permission_for_group_setting(

View File

@@ -19992,6 +19992,8 @@ paths:
The set of users who have permission to unsubscribe others from this
channel expressed as an [update to a group-setting value][update-group-setting].
Administrators can always unsubscribe others from a channel.
Note that a user who is a member of the specified user group must
also [have access](/help/channel-permissions) to the channel in
order to unsubscribe others.
@@ -24570,6 +24572,8 @@ components:
A [group-setting value][setting-values] defining the set of users
who have permission to remove subscribers from this channel.
Administrators can always unsubscribe others from a channel.
Note that a user who is a member of the specified user group must
also [have access](/help/channel-permissions) to the channel in
order to unsubscribe others.

View File

@@ -2714,7 +2714,7 @@ class StreamAdminTest(ZulipTestCase):
those you aren't on.
"""
result = self.attempt_unsubscribe_of_principal(
query_count=17,
query_count=15,
target_users=[self.example_user("cordelia")],
is_realm_admin=True,
is_subbed=True,
@@ -2741,7 +2741,7 @@ class StreamAdminTest(ZulipTestCase):
for name in ["cordelia", "prospero", "iago", "hamlet", "outgoing_webhook_bot"]
]
result = self.attempt_unsubscribe_of_principal(
query_count=24,
query_count=22,
cache_count=8,
target_users=target_users,
is_realm_admin=True,
@@ -2759,7 +2759,7 @@ class StreamAdminTest(ZulipTestCase):
are on.
"""
result = self.attempt_unsubscribe_of_principal(
query_count=18,
query_count=17,
target_users=[self.example_user("cordelia")],
is_realm_admin=True,
is_subbed=True,
@@ -2776,7 +2776,7 @@ class StreamAdminTest(ZulipTestCase):
streams you aren't on.
"""
result = self.attempt_unsubscribe_of_principal(
query_count=18,
query_count=17,
target_users=[self.example_user("cordelia")],
is_realm_admin=True,
is_subbed=False,
@@ -2802,7 +2802,7 @@ class StreamAdminTest(ZulipTestCase):
def test_admin_remove_others_from_stream_legacy_emails(self) -> None:
result = self.attempt_unsubscribe_of_principal(
query_count=17,
query_count=15,
target_users=[self.example_user("cordelia")],
is_realm_admin=True,
is_subbed=True,
@@ -2816,7 +2816,7 @@ class StreamAdminTest(ZulipTestCase):
def test_admin_remove_multiple_users_from_stream_legacy_emails(self) -> None:
result = self.attempt_unsubscribe_of_principal(
query_count=19,
query_count=17,
target_users=[self.example_user("cordelia"), self.example_user("prospero")],
is_realm_admin=True,
is_subbed=True,
@@ -2830,7 +2830,7 @@ class StreamAdminTest(ZulipTestCase):
def test_remove_unsubbed_user_along_with_subbed(self) -> None:
result = self.attempt_unsubscribe_of_principal(
query_count=16,
query_count=14,
target_users=[self.example_user("cordelia"), self.example_user("iago")],
is_realm_admin=True,
is_subbed=True,
@@ -2847,7 +2847,7 @@ class StreamAdminTest(ZulipTestCase):
fails gracefully.
"""
result = self.attempt_unsubscribe_of_principal(
query_count=9,
query_count=7,
target_users=[self.example_user("cordelia")],
is_realm_admin=True,
is_subbed=False,
@@ -2929,8 +2929,10 @@ class StreamAdminTest(ZulipTestCase):
self.assert_length(json["not_removed"], 0)
check_unsubscribing_user(self.example_user("hamlet"), leadership_group, expect_fail=True)
check_unsubscribing_user(self.example_user("desdemona"), leadership_group, expect_fail=True)
check_unsubscribing_user(self.example_user("iago"), leadership_group)
# Owners can always unsubscribe others even when they are not a member
# allowed group.
check_unsubscribing_user(self.example_user("desdemona"), leadership_group)
check_unsubscribing_user(self.example_user("othello"), managers_group, expect_fail=True)
check_unsubscribing_user(self.example_user("shiva"), managers_group)
@@ -2942,7 +2944,9 @@ class StreamAdminTest(ZulipTestCase):
# are not subscribed to even if they are member of the allowed group.
check_unsubscribing_user(self.example_user("shiva"), leadership_group, expect_fail=True)
check_unsubscribing_user(self.example_user("iago"), leadership_group)
# Owners can always unsubscribe others even when they are not a member
# allowed group.
check_unsubscribing_user(self.example_user("desdemona"), leadership_group)
self.subscribe(self.example_user("shiva"), stream.name)
check_unsubscribing_user(self.example_user("shiva"), leadership_group)
@@ -2952,11 +2956,19 @@ class StreamAdminTest(ZulipTestCase):
[leadership_group],
)
check_unsubscribing_user(self.example_user("othello"), setting_group, expect_fail=True)
check_unsubscribing_user(self.example_user("desdemona"), setting_group, expect_fail=True)
check_unsubscribing_user(self.example_user("hamlet"), setting_group)
check_unsubscribing_user(self.example_user("iago"), setting_group)
check_unsubscribing_user(self.example_user("shiva"), setting_group)
# Admins can unsubscribe others even when they are not a member of the
# allowed group.
setting_group = self.create_or_update_anonymous_group_for_setting(
[hamlet],
[],
)
check_unsubscribing_user(self.example_user("desdemona"), setting_group)
check_unsubscribing_user(self.example_user("iago"), setting_group)
def test_remove_invalid_user(self) -> None:
"""
Trying to unsubscribe an invalid user from a stream fails gracefully.