diff --git a/zerver/actions/streams.py b/zerver/actions/streams.py index 715b6bd376..ba19eba8fe 100644 --- a/zerver/actions/streams.py +++ b/zerver/actions/streams.py @@ -1317,33 +1317,39 @@ def do_change_stream_message_retention_days( ) -def do_change_can_remove_subscribers_group( - stream: Stream, user_group: UserGroup, *, acting_user: Optional[UserProfile] = None +def do_change_stream_group_based_setting( + stream: Stream, + setting_name: str, + user_group: UserGroup, + *, + acting_user: Optional[UserProfile] = None, ) -> None: - old_user_group = stream.can_remove_subscribers_group + old_user_group = getattr(stream, setting_name) old_user_group_id = None if old_user_group is not None: old_user_group_id = old_user_group.id - stream.can_remove_subscribers_group = user_group + setattr(stream, setting_name, user_group) stream.save() + RealmAuditLog.objects.create( realm=stream.realm, acting_user=acting_user, modified_stream=stream, - event_type=RealmAuditLog.STREAM_CAN_REMOVE_SUBSCRIBERS_GROUP_CHANGED, + event_type=RealmAuditLog.STREAM_GROUP_BASED_SETTING_CHANGED, event_time=timezone_now(), extra_data=orjson.dumps( { RealmAuditLog.OLD_VALUE: old_user_group_id, RealmAuditLog.NEW_VALUE: user_group.id, + "property": setting_name, } ).decode(), ) event = dict( op="update", type="stream", - property="can_remove_subscribers_group_id", + property=setting_name + "_id", value=user_group.id, stream_id=stream.id, name=stream.name, diff --git a/zerver/lib/types.py b/zerver/lib/types.py index 94e5b23ec9..83b9859a42 100644 --- a/zerver/lib/types.py +++ b/zerver/lib/types.py @@ -272,3 +272,10 @@ class RealmPlaygroundDict(TypedDict): name: str pygments_language: str url_prefix: str + + +@dataclass +class GroupPermissionSetting: + require_system_group: bool + allow_internet_group: bool + allow_owners_group: bool diff --git a/zerver/models.py b/zerver/models.py index a86919c396..3e9a344f3d 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -91,6 +91,7 @@ from zerver.lib.types import ( ExtendedFieldElement, ExtendedValidator, FieldElement, + GroupPermissionSetting, LinkifierDict, ProfileData, ProfileDataElementBase, @@ -2541,6 +2542,14 @@ class Stream(models.Model): # stream based on what messages they have cached. first_message_id = models.IntegerField(null=True, db_index=True) + stream_permission_group_settings = { + "can_remove_subscribers_group": GroupPermissionSetting( + require_system_group=True, + allow_internet_group=False, + allow_owners_group=False, + ), + } + def __str__(self) -> str: return f"" @@ -4382,7 +4391,7 @@ class AbstractRealmAuditLog(models.Model): STREAM_REACTIVATED = 604 STREAM_MESSAGE_RETENTION_DAYS_CHANGED = 605 STREAM_PROPERTY_CHANGED = 607 - STREAM_CAN_REMOVE_SUBSCRIBERS_GROUP_CHANGED = 608 + STREAM_GROUP_BASED_SETTING_CHANGED = 608 # The following values are only for RemoteZulipServerAuditLog # Values should be exactly 10000 greater than the corresponding diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index fa7d3b0902..1fb3666b63 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -78,8 +78,8 @@ from zerver.actions.realm_settings 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_group_based_setting, do_change_stream_message_retention_days, do_change_stream_permission, do_change_stream_post_policy, @@ -3018,8 +3018,11 @@ class SubscribeActionTest(BaseAction): is_system_group=True, realm=self.user_profile.realm, ) - action = lambda: do_change_can_remove_subscribers_group( - stream, moderators_group, acting_user=self.example_user("hamlet") + action = lambda: do_change_stream_group_based_setting( + stream, + "can_remove_subscribers_group", + moderators_group, + acting_user=self.example_user("hamlet"), ) events = self.verify_action(action, include_subscribers=include_subscribers, num_events=1) check_stream_update("events[0]", events[0]) diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index 856d93dff4..9127d8257e 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -29,7 +29,7 @@ from zerver.actions.realm_settings import do_change_realm_plan_type, do_set_real from zerver.actions.streams import ( bulk_add_subscriptions, bulk_remove_subscriptions, - do_change_can_remove_subscribers_group, + do_change_stream_group_based_setting, do_change_stream_post_policy, do_deactivate_stream, ) @@ -2585,8 +2585,11 @@ class StreamAdminTest(ZulipTestCase): ) -> None: self.login_user(user) self.subscribe(cordelia, stream.name) - do_change_can_remove_subscribers_group( - stream, can_remove_subscribers_group, acting_user=None + do_change_stream_group_based_setting( + stream, + "can_remove_subscribers_group", + can_remove_subscribers_group, + acting_user=None, ) result = self.client_delete( "/json/users/me/subscriptions", diff --git a/zerver/views/streams.py b/zerver/views/streams.py index ca1ba58eed..7b98b0ec2c 100644 --- a/zerver/views/streams.py +++ b/zerver/views/streams.py @@ -30,8 +30,8 @@ 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_group_based_setting, do_change_stream_message_retention_days, do_change_stream_permission, do_change_stream_post_policy, @@ -384,19 +384,32 @@ 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")) + for setting_name, permissions_configuration in Stream.stream_permission_group_settings.items(): + request_settings_dict = locals() + setting_group_id_name = setting_name + "_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, - ) + if setting_group_id_name not in request_settings_dict: # nocoverage + continue - do_change_can_remove_subscribers_group(stream, user_group, acting_user=user_profile) + if request_settings_dict[setting_group_id_name] is not None and request_settings_dict[ + setting_group_id_name + ] != getattr(stream, setting_name): + if sub is None and stream.invite_only: + # Admins cannot change this setting for unsubscribed private streams. + raise JsonableError(_("Invalid stream ID")) + + user_group_id = request_settings_dict[setting_group_id_name] + user_group = access_user_group_for_setting( + user_group_id, + user_profile, + setting_name=setting_name, + require_system_group=permissions_configuration.require_system_group, + allow_internet_group=permissions_configuration.allow_internet_group, + allow_owners_group=permissions_configuration.allow_owners_group, + ) + do_change_stream_group_based_setting( + stream, setting_name, user_group, acting_user=user_profile + ) return json_success(request)