From 520d4f504b5d0e8b76343b179f72f8a137423d52 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Wed, 22 Mar 2023 10:19:59 +1000 Subject: [PATCH] FEATURE: Auto-remove users without permission from channel (#20344) There are many situations that may cause users to lose permission to send messages in a chat channel. Until now we have relied on security checks in `Chat::ChatChannelFetcher` to remove channels which the user may have a `UserChatChannelMembership` record for but which they do not have access to. This commit takes a more proactive approach. Now any of these following `DiscourseEvent` triggers may cause `UserChatChannelMembership` records to be deleted: * `category_updated` - Permissions of the category changed (i.e. CategoryGroup records changed) * `user_removed_from_group` - Means the user may not be able to access the channel based on `GroupUser` or also `chat_allowed_groups` * `site_setting_changed` - The `chat_allowed_groups` was updated, some users may no longer be in groups that can access chat. * `group_destroyed` - Means the user may not be able to access the channel based on `GroupUser` or also `chat_allowed_groups` All of these are handled in a distinct service run in a background job. Users removed are logged via `StaffActionLog` and then we publish messages on a per-channel basis to users who had their memberships deleted. When the user has a channel they are kicked from open, we show a dialog saying "You no longer have access to this channel". When they click OK we redirect them either: * To their first other public channel, if they have any followed * The chat browse page if they don't This is to save on tons of requests from kicked out users getting messages from other channels. When the user does not have the kicked channel open, we can just silently yoink it out of their sidebar and turn off subscriptions. --- app/models/group.rb | 12 +- lib/job_time_spacer.rb | 25 ++ .../jobs/regular/auto_join_channel_batch.rb | 81 +++++ .../regular/chat/auto_join_channel_batch.rb | 2 +- ...ps.rb => auto_join_channel_memberships.rb} | 2 +- ...move_membership_handle_category_updated.rb | 11 + ...rship_handle_chat_allowed_groups_change.rb | 11 + ...emove_membership_handle_destroyed_group.rb | 11 + ...mbership_handle_user_removed_from_group.rb | 11 + .../regular/chat/kick_users_from_channel.rb | 13 + .../chat/user_chat_channel_membership.rb | 2 +- .../serializers/chat/channel_serializer.rb | 26 +- .../chat/structured_channel_serializer.rb | 3 + .../serializers/chat_channel_serializer.rb | 143 +++++++++ .../calculate_memberships_for_removal.rb | 104 +++++++ .../chat/action/publish_auto_removed_user.rb | 37 +++ .../chat/action/remove_memberships.rb | 15 + .../auto_remove/handle_category_updated.rb | 83 +++++ .../handle_chat_allowed_groups_change.rb | 85 ++++++ .../auto_remove/handle_destroyed_group.rb | 121 ++++++++ .../handle_user_removed_from_group.rb | 100 ++++++ plugins/chat/app/services/chat/publisher.rb | 12 + .../services/chat-channels-manager.js | 5 + .../services/chat-subscriptions-manager.js | 45 +++ plugins/chat/config/locales/client.en.yml | 2 + .../lib/chat/channel_membership_manager.rb | 2 +- plugins/chat/lib/chat/notifier.rb | 2 +- plugins/chat/lib/chat/steps_inspector.rb | 2 +- plugins/chat/plugin.rb | 27 +- .../auto_channel_user_removal_spec.rb | 285 ++++++++++++++++++ ... => auto_join_channel_memberships_spec.rb} | 2 +- .../chat/kick_users_from_channel_spec.rb | 38 +++ .../handle_category_updated_spec.rb | 154 ++++++++++ .../handle_chat_allowed_groups_change_spec.rb | 160 ++++++++++ .../handle_destroyed_group_spec.rb | 251 +++++++++++++++ .../handle_user_removed_from_group_spec.rb | 241 +++++++++++++++ .../spec/services/chat/update_channel_spec.rb | 2 +- .../system/kick_user_from_channel_spec.rb | 65 ++++ 38 files changed, 2174 insertions(+), 19 deletions(-) create mode 100644 lib/job_time_spacer.rb create mode 100644 plugins/chat/app/jobs/regular/auto_join_channel_batch.rb rename plugins/chat/app/jobs/regular/chat/{auto_manage_channel_memberships.rb => auto_join_channel_memberships.rb} (97%) create mode 100644 plugins/chat/app/jobs/regular/chat/auto_remove_membership_handle_category_updated.rb create mode 100644 plugins/chat/app/jobs/regular/chat/auto_remove_membership_handle_chat_allowed_groups_change.rb create mode 100644 plugins/chat/app/jobs/regular/chat/auto_remove_membership_handle_destroyed_group.rb create mode 100644 plugins/chat/app/jobs/regular/chat/auto_remove_membership_handle_user_removed_from_group.rb create mode 100644 plugins/chat/app/jobs/regular/chat/kick_users_from_channel.rb create mode 100644 plugins/chat/app/serializers/chat_channel_serializer.rb create mode 100644 plugins/chat/app/services/chat/action/calculate_memberships_for_removal.rb create mode 100644 plugins/chat/app/services/chat/action/publish_auto_removed_user.rb create mode 100644 plugins/chat/app/services/chat/action/remove_memberships.rb create mode 100644 plugins/chat/app/services/chat/auto_remove/handle_category_updated.rb create mode 100644 plugins/chat/app/services/chat/auto_remove/handle_chat_allowed_groups_change.rb create mode 100644 plugins/chat/app/services/chat/auto_remove/handle_destroyed_group.rb create mode 100644 plugins/chat/app/services/chat/auto_remove/handle_user_removed_from_group.rb create mode 100644 plugins/chat/spec/integration/auto_channel_user_removal_spec.rb rename plugins/chat/spec/jobs/regular/chat/{auto_manage_channel_memberships_spec.rb => auto_join_channel_memberships_spec.rb} (98%) create mode 100644 plugins/chat/spec/jobs/regular/chat/kick_users_from_channel_spec.rb create mode 100644 plugins/chat/spec/services/auto_remove/handle_category_updated_spec.rb create mode 100644 plugins/chat/spec/services/auto_remove/handle_chat_allowed_groups_change_spec.rb create mode 100644 plugins/chat/spec/services/auto_remove/handle_destroyed_group_spec.rb create mode 100644 plugins/chat/spec/services/auto_remove/handle_user_removed_from_group_spec.rb create mode 100644 plugins/chat/spec/system/kick_user_from_channel_spec.rb diff --git a/app/models/group.rb b/app/models/group.rb index f6ca9fc0bcb..d24d05f696b 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -65,6 +65,7 @@ class Group < ActiveRecord::Base after_commit :automatic_group_membership, on: %i[create update] after_commit :trigger_group_created_event, on: :create after_commit :trigger_group_updated_event, on: :update + before_destroy :cache_group_users_for_destroyed_event, prepend: true after_commit :trigger_group_destroyed_event, on: :destroy after_commit :set_default_notifications, on: %i[create update] @@ -911,13 +912,22 @@ class Group < ActiveRecord::Base self.member_of(groups, user).where("gu.owner") end - %i[group_created group_updated group_destroyed].each do |event| + def cache_group_users_for_destroyed_event + @cached_group_user_ids = group_users.pluck(:user_id) + end + + %i[group_created group_updated].each do |event| define_method("trigger_#{event}_event") do DiscourseEvent.trigger(event, self) true end end + def trigger_group_destroyed_event + DiscourseEvent.trigger(:group_destroyed, self, @cached_group_user_ids) + true + end + def flair_type return :icon if flair_icon.present? return :image if flair_upload.present? diff --git a/lib/job_time_spacer.rb b/lib/job_time_spacer.rb new file mode 100644 index 00000000000..18a7913ddd4 --- /dev/null +++ b/lib/job_time_spacer.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +## +# In some cases we may want to enqueue_at several of the same job with +# batches, spacing them out or incrementing by some amount of seconds, +# in case the jobs do heavy work or send many MessageBus messages and the like. +# This class handles figuring out the seconds increments. +# +# @example +# spacer = JobTimeSpacer.new +# user_ids.in_groups_of(200, false) do |user_id_batch| +# spacer.enqueue(:kick_users_from_topic, { topic_id: topic_id, user_ids: user_id_batch }) +# end +class JobTimeSpacer + def initialize(seconds_space_increment: 1, seconds_delay: 5) + @seconds_space_increment = seconds_space_increment + @seconds_space_modifier = seconds_space_increment + @seconds_step = seconds_delay + end + + def enqueue(job_name, job_args = {}) + Jobs.enqueue_at((@seconds_step * @seconds_space_modifier).seconds.from_now, job_name, job_args) + @seconds_space_modifier += @seconds_space_increment + end +end diff --git a/plugins/chat/app/jobs/regular/auto_join_channel_batch.rb b/plugins/chat/app/jobs/regular/auto_join_channel_batch.rb new file mode 100644 index 00000000000..71ad21734ef --- /dev/null +++ b/plugins/chat/app/jobs/regular/auto_join_channel_batch.rb @@ -0,0 +1,81 @@ +# NOTE: When changing auto-join logic, make sure to update the `settings.auto_join_users_info` translation as well. +# frozen_string_literal: true + +module Jobs + class AutoJoinChannelBatch < ::Jobs::Base + def execute(args) + return "starts_at or ends_at missing" if args[:starts_at].blank? || args[:ends_at].blank? + start_user_id = args[:starts_at].to_i + end_user_id = args[:ends_at].to_i + + return "End is higher than start" if end_user_id < start_user_id + + channel = + ChatChannel.find_by( + id: args[:chat_channel_id], + auto_join_users: true, + chatable_type: "Category", + ) + + return if !channel + + category = channel.chatable + return if !category + + query_args = { + chat_channel_id: channel.id, + start: start_user_id, + end: end_user_id, + suspended_until: Time.zone.now, + last_seen_at: 3.months.ago, + channel_category: channel.chatable_id, + mode: Chat::UserChatChannelMembership.join_modes[:automatic], + } + + new_member_ids = DB.query_single(create_memberships_query(category), query_args) + + # Only do this if we are running auto-join for a single user, if we + # are doing it for many then we should do it after all batches are + # complete for the channel in Jobs::AutoJoinChannelMemberships + if start_user_id == end_user_id + Chat::ChatChannelMembershipManager.new(channel).recalculate_user_count + end + + Chat::Publisher.publish_new_channel(channel.reload, User.where(id: new_member_ids)) + end + + private + + def create_memberships_query(category) + query = <<~SQL + INSERT INTO user_chat_channel_memberships (user_id, chat_channel_id, following, created_at, updated_at, join_mode) + SELECT DISTINCT(users.id), :chat_channel_id, TRUE, NOW(), NOW(), :mode + FROM users + INNER JOIN user_options uo ON uo.user_id = users.id + LEFT OUTER JOIN user_chat_channel_memberships uccm ON + uccm.chat_channel_id = :chat_channel_id AND uccm.user_id = users.id + SQL + + query += <<~SQL if category.read_restricted? + INNER JOIN group_users gu ON gu.user_id = users.id + LEFT OUTER JOIN category_groups cg ON cg.group_id = gu.group_id + SQL + + query += <<~SQL + WHERE (users.id >= :start AND users.id <= :end) AND + users.staged IS FALSE AND users.active AND + NOT EXISTS(SELECT 1 FROM anonymous_users a WHERE a.user_id = users.id) AND + (suspended_till IS NULL OR suspended_till <= :suspended_until) AND + (last_seen_at > :last_seen_at) AND + uo.chat_enabled AND + uccm.id IS NULL + SQL + + query += <<~SQL if category.read_restricted? + AND cg.category_id = :channel_category + SQL + + query += "RETURNING user_chat_channel_memberships.user_id" + end + end +end diff --git a/plugins/chat/app/jobs/regular/chat/auto_join_channel_batch.rb b/plugins/chat/app/jobs/regular/chat/auto_join_channel_batch.rb index 19ca7e5b03a..a525d35c8b7 100644 --- a/plugins/chat/app/jobs/regular/chat/auto_join_channel_batch.rb +++ b/plugins/chat/app/jobs/regular/chat/auto_join_channel_batch.rb @@ -37,7 +37,7 @@ module Jobs # Only do this if we are running auto-join for a single user, if we # are doing it for many then we should do it after all batches are - # complete for the channel in Jobs::Chat::AutoManageChannelMemberships + # complete for the channel in Jobs::Chat::AutoJoinChannelMemberships if start_user_id == end_user_id ::Chat::ChannelMembershipManager.new(channel).recalculate_user_count end diff --git a/plugins/chat/app/jobs/regular/chat/auto_manage_channel_memberships.rb b/plugins/chat/app/jobs/regular/chat/auto_join_channel_memberships.rb similarity index 97% rename from plugins/chat/app/jobs/regular/chat/auto_manage_channel_memberships.rb rename to plugins/chat/app/jobs/regular/chat/auto_join_channel_memberships.rb index 8fe8fbe4826..fbb57ded563 100644 --- a/plugins/chat/app/jobs/regular/chat/auto_manage_channel_memberships.rb +++ b/plugins/chat/app/jobs/regular/chat/auto_join_channel_memberships.rb @@ -3,7 +3,7 @@ module Jobs module Chat - class AutoManageChannelMemberships < ::Jobs::Base + class AutoJoinChannelMemberships < ::Jobs::Base def execute(args) channel = ::Chat::Channel.includes(:chatable).find_by( diff --git a/plugins/chat/app/jobs/regular/chat/auto_remove_membership_handle_category_updated.rb b/plugins/chat/app/jobs/regular/chat/auto_remove_membership_handle_category_updated.rb new file mode 100644 index 00000000000..fa1e6504fbf --- /dev/null +++ b/plugins/chat/app/jobs/regular/chat/auto_remove_membership_handle_category_updated.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module Jobs + module Chat + class AutoRemoveMembershipHandleCategoryUpdated < ::Jobs::Base + def execute(args) + ::Chat::AutoRemove::HandleCategoryUpdated.call(**args) + end + end + end +end diff --git a/plugins/chat/app/jobs/regular/chat/auto_remove_membership_handle_chat_allowed_groups_change.rb b/plugins/chat/app/jobs/regular/chat/auto_remove_membership_handle_chat_allowed_groups_change.rb new file mode 100644 index 00000000000..94ac518b35d --- /dev/null +++ b/plugins/chat/app/jobs/regular/chat/auto_remove_membership_handle_chat_allowed_groups_change.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module Jobs + module Chat + class AutoRemoveMembershipHandleChatAllowedGroupsChange < ::Jobs::Base + def execute(args) + ::Chat::AutoRemove::HandleChatAllowedGroupsChange.call(**args) + end + end + end +end diff --git a/plugins/chat/app/jobs/regular/chat/auto_remove_membership_handle_destroyed_group.rb b/plugins/chat/app/jobs/regular/chat/auto_remove_membership_handle_destroyed_group.rb new file mode 100644 index 00000000000..e24f87247ee --- /dev/null +++ b/plugins/chat/app/jobs/regular/chat/auto_remove_membership_handle_destroyed_group.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module Jobs + module Chat + class AutoRemoveMembershipHandleDestroyedGroup < ::Jobs::Base + def execute(args) + ::Chat::AutoRemove::HandleDestroyedGroup.call(**args) + end + end + end +end diff --git a/plugins/chat/app/jobs/regular/chat/auto_remove_membership_handle_user_removed_from_group.rb b/plugins/chat/app/jobs/regular/chat/auto_remove_membership_handle_user_removed_from_group.rb new file mode 100644 index 00000000000..bd599a840d2 --- /dev/null +++ b/plugins/chat/app/jobs/regular/chat/auto_remove_membership_handle_user_removed_from_group.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module Jobs + module Chat + class AutoRemoveMembershipHandleUserRemovedFromGroup < ::Jobs::Base + def execute(args) + ::Chat::AutoRemove::HandleUserRemovedFromGroup.call(**args) + end + end + end +end diff --git a/plugins/chat/app/jobs/regular/chat/kick_users_from_channel.rb b/plugins/chat/app/jobs/regular/chat/kick_users_from_channel.rb new file mode 100644 index 00000000000..aa67049c4c3 --- /dev/null +++ b/plugins/chat/app/jobs/regular/chat/kick_users_from_channel.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module Jobs + module Chat + class KickUsersFromChannel < Jobs::Base + def execute(args) + return if !::Chat::Channel.exists?(id: args[:channel_id]) + return if args[:user_ids].blank? + ::Chat::Publisher.publish_kick_users(args[:channel_id], args[:user_ids]) + end + end + end +end diff --git a/plugins/chat/app/models/chat/user_chat_channel_membership.rb b/plugins/chat/app/models/chat/user_chat_channel_membership.rb index c28b2524871..f3c5e2d8b9d 100644 --- a/plugins/chat/app/models/chat/user_chat_channel_membership.rb +++ b/plugins/chat/app/models/chat/user_chat_channel_membership.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Chat - class UserChatChannelMembership < ActiveRecord::Base + class Chat::UserChatChannelMembership < ActiveRecord::Base self.table_name = "user_chat_channel_memberships" NOTIFICATION_LEVELS = { never: 0, mention: 1, always: 2 } diff --git a/plugins/chat/app/serializers/chat/channel_serializer.rb b/plugins/chat/app/serializers/chat/channel_serializer.rb index 7f267a9d60b..01702b8fb48 100644 --- a/plugins/chat/app/serializers/chat/channel_serializer.rb +++ b/plugins/chat/app/serializers/chat/channel_serializer.rb @@ -112,12 +112,9 @@ module Chat { message_bus_last_ids: { channel_message_bus_last_id: MessageBus.last_id("/chat/#{object.id}"), - new_messages: - @opts[:new_messages_message_bus_last_id] || - MessageBus.last_id(Chat::Publisher.new_messages_message_bus_channel(object.id)), - new_mentions: - @opts[:new_mentions_message_bus_last_id] || - MessageBus.last_id(Chat::Publisher.new_mentions_message_bus_channel(object.id)), + new_messages: new_messages_message_bus_id, + new_mentions: new_mentions_message_bus_id, + kick: kick_message_bus_id, }, } end @@ -127,5 +124,22 @@ module Chat alias_method :include_archived_messages?, :include_archive_status? alias_method :include_archive_failed?, :include_archive_status? alias_method :include_archive_completed?, :include_archive_status? + + private + + def new_messages_message_bus_id + @opts[:new_messages_message_bus_last_id] || + MessageBus.last_id(Chat::Publisher.new_messages_message_bus_channel(object.id)) + end + + def new_mentions_message_bus_id + @opts[:new_mentions_message_bus_last_id] || + MessageBus.last_id(Chat::Publisher.new_mentions_message_bus_channel(object.id)) + end + + def kick_message_bus_id + @opts[:kick_message_bus_last_id] || + MessageBus.last_id(Chat::Publisher.kick_users_message_bus_channel(object.id)) + end end end diff --git a/plugins/chat/app/serializers/chat/structured_channel_serializer.rb b/plugins/chat/app/serializers/chat/structured_channel_serializer.rb index aca74386956..553f85795d8 100644 --- a/plugins/chat/app/serializers/chat/structured_channel_serializer.rb +++ b/plugins/chat/app/serializers/chat/structured_channel_serializer.rb @@ -30,6 +30,8 @@ module Chat chat_message_bus_last_ids[Chat::Publisher.new_messages_message_bus_channel(channel.id)], new_mentions_message_bus_last_id: chat_message_bus_last_ids[Chat::Publisher.new_mentions_message_bus_channel(channel.id)], + kick_message_bus_last_id: + chat_message_bus_last_ids[Chat::Publisher.kick_users_message_bus_channel(channel.id)], ) end end @@ -84,6 +86,7 @@ module Chat object[:public_channels].each do |channel| message_bus_channels.push(Chat::Publisher.new_messages_message_bus_channel(channel.id)) message_bus_channels.push(Chat::Publisher.new_mentions_message_bus_channel(channel.id)) + message_bus_channels.push(Chat::Publisher.kick_users_message_bus_channel(channel.id)) end object[:direct_message_channels].each do |channel| diff --git a/plugins/chat/app/serializers/chat_channel_serializer.rb b/plugins/chat/app/serializers/chat_channel_serializer.rb new file mode 100644 index 00000000000..58a5939100d --- /dev/null +++ b/plugins/chat/app/serializers/chat_channel_serializer.rb @@ -0,0 +1,143 @@ +# frozen_string_literal: true + +class ChatChannelSerializer < ApplicationSerializer + attributes :id, + :auto_join_users, + :allow_channel_wide_mentions, + :chatable, + :chatable_id, + :chatable_type, + :chatable_url, + :description, + :title, + :slug, + :last_message_sent_at, + :status, + :archive_failed, + :archive_completed, + :archived_messages, + :total_messages, + :archive_topic_id, + :memberships_count, + :current_user_membership, + :meta, + :threading_enabled + + def threading_enabled + SiteSetting.enable_experimental_chat_threaded_discussions && object.threading_enabled + end + + def initialize(object, opts) + super(object, opts) + + @opts = opts + @current_user_membership = opts[:membership] + end + + def include_description? + object.description.present? + end + + def memberships_count + object.user_count + end + + def chatable_url + object.chatable_url + end + + def title + object.name || object.title(scope.user) + end + + def chatable + case object.chatable_type + when "Category" + BasicCategorySerializer.new(object.chatable, root: false).as_json + when "DirectMessage" + DirectMessageSerializer.new(object.chatable, scope: scope, root: false).as_json + when "Site" + nil + end + end + + def archive + object.chat_channel_archive + end + + def include_archive_status? + !object.direct_message_channel? && scope.is_staff? && archive.present? + end + + def archive_completed + archive.complete? + end + + def archive_failed + archive.failed? + end + + def archived_messages + archive.archived_messages + end + + def total_messages + archive.total_messages + end + + def archive_topic_id + archive.destination_topic_id + end + + def include_auto_join_users? + scope.can_edit_chat_channel? + end + + def include_current_user_membership? + @current_user_membership.present? + end + + def current_user_membership + @current_user_membership.chat_channel = object + + BaseChatChannelMembershipSerializer.new( + @current_user_membership, + scope: scope, + root: false, + ).as_json + end + + def meta + { + message_bus_last_ids: { + new_messages: new_messages_message_bus_id, + new_mentions: new_mentions_message_bus_id, + kick: kick_message_bus_id, + channel_message_bus_last_id: MessageBus.last_id("/chat/#{object.id}"), + }, + } + end + + alias_method :include_archive_topic_id?, :include_archive_status? + alias_method :include_total_messages?, :include_archive_status? + alias_method :include_archived_messages?, :include_archive_status? + alias_method :include_archive_failed?, :include_archive_status? + alias_method :include_archive_completed?, :include_archive_status? + + private + + def new_messages_message_bus_id + @opts[:new_messages_message_bus_last_id] || + MessageBus.last_id(Chat::Publisher.new_messages_message_bus_channel(object.id)) + end + + def new_mentions_message_bus_id + @opts[:new_mentions_message_bus_last_id] || + MessageBus.last_id(Chat::Publisher.new_mentions_message_bus_channel(object.id)) + end + + def kick_message_bus_id + @opts[:kick_message_bus_last_id] || + MessageBus.last_id(Chat::Publisher.kick_users_message_bus_channel(object.id)) + end +end diff --git a/plugins/chat/app/services/chat/action/calculate_memberships_for_removal.rb b/plugins/chat/app/services/chat/action/calculate_memberships_for_removal.rb new file mode 100644 index 00000000000..fa4f74615e5 --- /dev/null +++ b/plugins/chat/app/services/chat/action/calculate_memberships_for_removal.rb @@ -0,0 +1,104 @@ +# frozen_string_literal: true + +module Chat + module Action + # There is significant complexity around category channel permissions, + # since they are inferred from [CategoryGroup] records and their corresponding + # permission types. + # + # To be able to join and chat in a channel, a user must either be staff, + # or be in a group that has either `full` or `create_post` permissions + # via [CategoryGroup]. + # + # However, there is an edge case. If there are no [CategoryGroup] records + # for a given category, this means that the [Group::AUTO_GROUPS[:everyone]] + # group has `full` access to the channel, therefore everyone can post in + # the chat channel (so long as they are in one of the `SiteSetting.chat_allowed_groups`) + # + # Here, we can efficiently query the channel category permissions and figure + # out which of the users provided should have their [Chat::UserChatChannelMembership] + # records removed based on those security cases. + class CalculateMembershipsForRemoval + def self.call(scoped_users:, channel_ids: nil) + channel_permissions_map = + DB.query(<<~SQL, readonly: CategoryGroup.permission_types[:readonly]) + WITH category_group_channel_map AS ( + SELECT category_groups.group_id, + category_groups.permission_type, + chat_channels.id AS channel_id + FROM category_groups + INNER JOIN categories ON categories.id = category_groups.category_id + INNER JOIN chat_channels ON categories.id = chat_channels.chatable_id + AND chat_channels.chatable_type = 'Category' + ) + + SELECT chat_channels.id AS channel_id, + chat_channels.chatable_id AS category_id, + ( + SELECT string_agg(category_group_channel_map.group_id::varchar, ',') + FROM category_group_channel_map + WHERE category_group_channel_map.permission_type < :readonly AND + category_group_channel_map.channel_id = chat_channels.id + ) AS groups_with_write_permissions, + ( + SELECT string_agg(category_group_channel_map.group_id::varchar, ',') + FROM category_group_channel_map + WHERE category_group_channel_map.permission_type = :readonly AND + category_group_channel_map.channel_id = chat_channels.id + ) AS groups_with_readonly_permissions, + categories.read_restricted + FROM category_group_channel_map + INNER JOIN chat_channels ON chat_channels.id = category_group_channel_map.channel_id + INNER JOIN categories ON categories.id = chat_channels.chatable_id + WHERE chat_channels.chatable_type = 'Category' + #{channel_ids.present? ? "AND chat_channels.id IN (#{channel_ids.join(",")})" : ""} + GROUP BY chat_channels.id, chat_channels.chatable_id, categories.read_restricted + ORDER BY channel_id + SQL + + scoped_memberships = + Chat::UserChatChannelMembership + .joins(:chat_channel) + .where(user: scoped_users) + .where(chat_channel_id: channel_permissions_map.map(&:channel_id)) + + memberships_to_remove = [] + scoped_memberships.find_each do |membership| + scoped_user = scoped_users.find { |su| su.id == membership.user_id } + channel_permission = + channel_permissions_map.find { |cpm| cpm.channel_id == membership.chat_channel_id } + + # If there is no channel in the map, this means there are no + # category_groups for the channel. + # + # This in turn means the Everyone group with full permission + # is the only group that can access the channel (no category_group + # record is created in this case), we do not need to remove any users. + next if channel_permission.blank? + + group_ids_with_write_permission = + channel_permission.groups_with_write_permissions.to_s.split(",").map(&:to_i) + group_ids_with_read_permission = + channel_permission.groups_with_readonly_permissions.to_s.split(",").map(&:to_i) + + # None of the groups on the channel have permission to do anything + # more than read only, remove the membership. + if group_ids_with_write_permission.empty? && group_ids_with_read_permission.any? + memberships_to_remove << membership.id + next + end + + # At least one of the groups on the channel can create_post or + # has full permission, remove the membership if the user is in none + # of these groups. + if group_ids_with_write_permission.any? && + !scoped_user.in_any_groups?(group_ids_with_write_permission) + memberships_to_remove << membership.id + end + end + + memberships_to_remove + end + end + end +end diff --git a/plugins/chat/app/services/chat/action/publish_auto_removed_user.rb b/plugins/chat/app/services/chat/action/publish_auto_removed_user.rb new file mode 100644 index 00000000000..8d3912ecba3 --- /dev/null +++ b/plugins/chat/app/services/chat/action/publish_auto_removed_user.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +module Chat + module Action + # All of the handlers that auto-remove users from chat + # (under services/auto_remove) need to publish which users + # were removed and from which channel, as well as logging + # this in staff actions so it's obvious why these users were + # removed. + class PublishAutoRemovedUser + # @param [Symbol] event_type What caused the users to be removed, + # each handler will define this, e.g. category_updated, user_removed_from_group + # @param [Hash] users_removed_map A hash with channel_id as its keys and an + # array of user_ids who were removed from the channel. + def self.call(event_type:, users_removed_map:) + return if users_removed_map.empty? + + users_removed_map.each do |channel_id, user_ids| + job_spacer = JobTimeSpacer.new + user_ids.in_groups_of(1000, false) do |user_id_batch| + job_spacer.enqueue( + Jobs::Chat::KickUsersFromChannel, + { channel_id: channel_id, user_ids: user_id_batch }, + ) + end + + if user_ids.any? + StaffActionLogger.new(Discourse.system_user).log_custom( + "chat_auto_remove_membership", + { users_removed: user_ids.length, channel_id: channel_id, event: event_type }, + ) + end + end + end + end + end +end diff --git a/plugins/chat/app/services/chat/action/remove_memberships.rb b/plugins/chat/app/services/chat/action/remove_memberships.rb new file mode 100644 index 00000000000..6a2330ffe74 --- /dev/null +++ b/plugins/chat/app/services/chat/action/remove_memberships.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module Chat + module Action + class RemoveMemberships + def self.call(memberships:) + memberships + .destroy_all + .each_with_object(Hash.new { |h, k| h[k] = [] }) do |obj, hash| + hash[obj.chat_channel_id] << obj.user_id + end + end + end + end +end diff --git a/plugins/chat/app/services/chat/auto_remove/handle_category_updated.rb b/plugins/chat/app/services/chat/auto_remove/handle_category_updated.rb new file mode 100644 index 00000000000..f09bebb46a1 --- /dev/null +++ b/plugins/chat/app/services/chat/auto_remove/handle_category_updated.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +module Chat + module AutoRemove + # Fired from [Jobs::AutoRemoveMembershipHandleCategoryUpdated], which + # in turn is enqueued whenever the [DiscourseEvent] for :category_updated + # is triggered. Any users who can no longer access category-based channels + # based on category_groups and in turn group_users will be removed from + # those chat channels. + # + # If a user is in any groups that have the `full` or `create_post` + # [CategoryGroup#permission_types] or if the category has no groups remaining, + # then the user will remain in the channel. + class HandleCategoryUpdated + include Service::Base + + contract + step :assign_defaults + policy :chat_enabled + model :category + model :category_channel_ids + model :users + step :remove_users_without_channel_permission + step :publish + + class Contract + attribute :category_id, :integer + + validates :category_id, presence: true + end + + private + + def assign_defaults + context[:users_removed_map] = {} + end + + def chat_enabled + SiteSetting.chat_enabled + end + + def fetch_category(contract:, **) + Category.find_by(id: contract.category_id) + end + + def fetch_category_channel_ids(category:, **) + Chat::Channel.where(chatable: category).pluck(:id) + end + + def fetch_users(category_channel_ids:, **) + User + .real + .activated + .not_suspended + .not_staged + .joins(:user_chat_channel_memberships) + .where("user_chat_channel_memberships.chat_channel_id IN (?)", category_channel_ids) + .where("NOT admin AND NOT moderator") + end + + def remove_users_without_channel_permission(users:, category_channel_ids:, **) + memberships_to_remove = + Chat::Action::CalculateMembershipsForRemoval.call( + scoped_users: users, + channel_ids: category_channel_ids, + ) + + return if memberships_to_remove.blank? + + context[:users_removed_map] = Chat::Action::RemoveMemberships.call( + memberships: Chat::UserChatChannelMembership.where(id: memberships_to_remove), + ) + end + + def publish(users_removed_map:, **) + Chat::Action::PublishAutoRemovedUser.call( + event_type: :category_updated, + users_removed_map: users_removed_map, + ) + end + end + end +end diff --git a/plugins/chat/app/services/chat/auto_remove/handle_chat_allowed_groups_change.rb b/plugins/chat/app/services/chat/auto_remove/handle_chat_allowed_groups_change.rb new file mode 100644 index 00000000000..3da46ab23ed --- /dev/null +++ b/plugins/chat/app/services/chat/auto_remove/handle_chat_allowed_groups_change.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +module Chat + module AutoRemove + # Fired from [Jobs::AutoRemoveMembershipHandleChatAllowedGroupsChange], which + # in turn is enqueued whenever the [DiscourseEvent] for :site_setting_changed + # is triggered for the chat_allowed_groups setting. + # + # If any of the chat_allowed_groups is the everyone auto group then nothing + # needs to be done. + # + # Otherwise, if there are no longer any chat_allowed_groups, we have to + # remove all non-admin users from category channels. Otherwise we just + # remove the ones who are not in any of the chat_allowed_groups. + # + # Direct message channel memberships are intentionally left alone, + # these are private communications between two people. + class HandleChatAllowedGroupsChange + include Service::Base + + policy :chat_enabled + step :cast_new_allowed_groups_to_array + policy :not_everyone_allowed + model :users + model :memberships_to_remove + step :remove_users_outside_allowed_groups + step :publish + + private + + def chat_enabled + SiteSetting.chat_enabled + end + + def cast_new_allowed_groups_to_array(new_allowed_groups:, **) + context[:new_allowed_groups] = new_allowed_groups.to_s.split("|").map(&:to_i) + end + + def not_everyone_allowed(new_allowed_groups:, **) + !new_allowed_groups.include?(Group::AUTO_GROUPS[:everyone]) + end + + def fetch_users(new_allowed_groups:, **) + User + .real + .activated + .not_suspended + .not_staged + .where("NOT admin AND NOT moderator") + .joins(:user_chat_channel_memberships) + .distinct + .then do |users| + break users if new_allowed_groups.blank? + users.where(<<~SQL, new_allowed_groups) + users.id NOT IN ( + SELECT DISTINCT group_users.user_id + FROM group_users + WHERE group_users.group_id IN (?) + ) + SQL + end + end + + def fetch_memberships_to_remove(users:, **) + Chat::UserChatChannelMembership + .joins(:chat_channel) + .where(user_id: users.pluck(:id)) + .where.not(chat_channel: { type: "DirectMessageChannel" }) + end + + def remove_users_outside_allowed_groups(memberships_to_remove:, **) + context[:users_removed_map] = Chat::Action::RemoveMemberships.call( + memberships: memberships_to_remove, + ) + end + + def publish(users_removed_map:, **) + Chat::Action::PublishAutoRemovedUser.call( + event_type: :chat_allowed_groups_changed, + users_removed_map: users_removed_map, + ) + end + end + end +end diff --git a/plugins/chat/app/services/chat/auto_remove/handle_destroyed_group.rb b/plugins/chat/app/services/chat/auto_remove/handle_destroyed_group.rb new file mode 100644 index 00000000000..b8aa9a9e153 --- /dev/null +++ b/plugins/chat/app/services/chat/auto_remove/handle_destroyed_group.rb @@ -0,0 +1,121 @@ +# frozen_string_literal: true + +module Chat + module AutoRemove + # Fired from [Jobs::AutoRemoveMembershipHandleUserRemovedFromGroup], which + # is in turn enqueued whenever the [DiscourseEvent] for :group_destroyed + # is triggered. + # + # The :group_destroyed event provides us with the user_ids of the former + # GroupUser records so we can scope this better. + # + # Since this could have potential wide-ranging impact, we have to check: + # * The chat_allowed_groups [SiteSetting], and if any of the scoped users + # are still allowed to use public chat channels based on this setting. + # * The channel permissions of all the category chat channels the users + # are a part of, based on [CategoryGroup] records + # + # If a user is in a groups that has the `full` or `create_post` + # [CategoryGroup#permission_types] or if the category has no groups remaining, + # then the user will remain in the channel. + class HandleDestroyedGroup + include Service::Base + + contract + step :assign_defaults + policy :chat_enabled + policy :not_everyone_allowed + model :scoped_users + step :remove_users_outside_allowed_groups + step :remove_users_without_channel_permission + step :publish + + class Contract + attribute :destroyed_group_user_ids + + validates :destroyed_group_user_ids, presence: true + end + + private + + def assign_defaults + context[:users_removed_map] = {} + end + + def chat_enabled + SiteSetting.chat_enabled + end + + def not_everyone_allowed + !SiteSetting.chat_allowed_groups_map.include?(Group::AUTO_GROUPS[:everyone]) + end + + def fetch_scoped_users(destroyed_group_user_ids:, **) + User + .real + .activated + .not_suspended + .not_staged + .includes(:group_users) + .where("NOT admin AND NOT moderator") + .where(id: destroyed_group_user_ids) + .joins(:user_chat_channel_memberships) + .distinct + end + + def remove_users_outside_allowed_groups(scoped_users:, **) + users = scoped_users + + # Remove any of these users from all category channels if they + # are not in any of the chat_allowed_groups or if there are no + # chat allowed groups. + if SiteSetting.chat_allowed_groups_map.any? + group_user_sql = <<~SQL + users.id NOT IN ( + SELECT DISTINCT group_users.user_id + FROM group_users + WHERE group_users.group_id IN (#{SiteSetting.chat_allowed_groups_map.join(",")}) + ) + SQL + users = users.where(group_user_sql) + end + + user_ids_to_remove = users.pluck(:id) + return if user_ids_to_remove.empty? + + memberships_to_remove = + Chat::UserChatChannelMembership + .joins(:chat_channel) + .where(user_id: user_ids_to_remove) + .where.not(chat_channel: { type: "DirectMessageChannel" }) + + return if memberships_to_remove.empty? + + context[:users_removed_map] = Chat::Action::RemoveMemberships.call( + memberships: memberships_to_remove, + ) + end + + def remove_users_without_channel_permission(scoped_users:, **) + memberships_to_remove = + Chat::Action::CalculateMembershipsForRemoval.call(scoped_users: scoped_users) + + return if memberships_to_remove.empty? + + context.merge( + users_removed_map: + Chat::Action::RemoveMemberships.call( + memberships: Chat::UserChatChannelMembership.where(id: memberships_to_remove), + ), + ) + end + + def publish(users_removed_map:, **) + Chat::Action::PublishAutoRemovedUser.call( + event_type: :destroyed_group, + users_removed_map: users_removed_map, + ) + end + end + end +end diff --git a/plugins/chat/app/services/chat/auto_remove/handle_user_removed_from_group.rb b/plugins/chat/app/services/chat/auto_remove/handle_user_removed_from_group.rb new file mode 100644 index 00000000000..dea3af483d5 --- /dev/null +++ b/plugins/chat/app/services/chat/auto_remove/handle_user_removed_from_group.rb @@ -0,0 +1,100 @@ +# frozen_string_literal: true + +module Chat + module AutoRemove + # Fired from [Jobs::AutoRemoveMembershipHandleUserRemovedFromGroup], which + # in turn is enqueued whenever the [DiscourseEvent] for :user_removed_from_group + # is triggered. + # + # Staff users will never be affected by this, they can always chat regardless + # of group permissions. + # + # Since this could have potential wide-ranging impact, we have to check: + # * The chat_allowed_groups [SiteSetting], and if the scoped user + # is still allowed to use public chat channels based on this setting. + # * The channel permissions of all the category chat channels the user + # is a part of, based on [CategoryGroup] records + # + # Direct message channel memberships are intentionally left alone, + # these are private communications between two people. + class HandleUserRemovedFromGroup + include Service::Base + + contract + step :assign_defaults + policy :chat_enabled + policy :not_everyone_allowed + model :user + policy :user_not_staff + step :remove_if_outside_chat_allowed_groups + step :remove_from_private_channels + step :publish + + class Contract + attribute :user_id, :integer + + validates :user_id, presence: true + end + + private + + def assign_defaults + context[:users_removed_map] = {} + end + + def chat_enabled + SiteSetting.chat_enabled + end + + def not_everyone_allowed + !SiteSetting.chat_allowed_groups_map.include?(Group::AUTO_GROUPS[:everyone]) + end + + def fetch_user(contract:, **) + User.find_by(id: contract.user_id) + end + + def user_not_staff(user:, **) + !user.staff? + end + + def remove_if_outside_chat_allowed_groups(user:, **) + if SiteSetting.chat_allowed_groups_map.empty? || + !GroupUser.exists?(group_id: SiteSetting.chat_allowed_groups_map, user: user) + memberships_to_remove = + Chat::UserChatChannelMembership + .joins(:chat_channel) + .where(user_id: user.id) + .where.not(chat_channel: { type: "DirectMessageChannel" }) + + return if memberships_to_remove.empty? + + context[:users_removed_map] = Chat::Action::RemoveMemberships.call( + memberships: memberships_to_remove, + ) + end + end + + def remove_from_private_channels(user:, **) + memberships_to_remove = + Chat::Action::CalculateMembershipsForRemoval.call(scoped_users: [user]) + + return if memberships_to_remove.empty? + + context.merge( + users_removed_map: + Chat::Action::RemoveMemberships.call( + memberships: Chat::UserChatChannelMembership.where(id: memberships_to_remove), + ), + ) + end + + def publish(users_removed_map:, **) + Chat::Action::PublishAutoRemovedUser.call( + event_type: :user_removed_from_group, + users_removed_map: users_removed_map, + ) + end + end + end +end diff --git a/plugins/chat/app/services/chat/publisher.rb b/plugins/chat/app/services/chat/publisher.rb index b04cfa069c9..d8605e36aa1 100644 --- a/plugins/chat/app/services/chat/publisher.rb +++ b/plugins/chat/app/services/chat/publisher.rb @@ -144,6 +144,10 @@ module Chat "/chat/#{chat_channel_id}/new-mentions" end + def self.kick_users_message_bus_channel(chat_channel_id) + "/chat/#{chat_channel_id}/kick" + end + def self.publish_new_mention(user_id, chat_channel_id, chat_message_id) MessageBus.publish( self.new_mentions_message_bus_channel(chat_channel_id), @@ -197,6 +201,14 @@ module Chat ) end + def self.publish_kick_users(channel_id, user_ids) + MessageBus.publish( + kick_users_message_bus_channel(channel_id), + { channel_id: channel_id }, + user_ids: user_ids, + ) + end + CHANNEL_EDITS_MESSAGE_BUS_CHANNEL = "/chat/channel-edits" def self.publish_chat_channel_edit(chat_channel, acting_user) diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js b/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js index 0905f46802c..4a9e26dd698 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js @@ -82,6 +82,11 @@ export default class ChatChannelsManager extends Service { }); } + remove(model) { + this.chatSubscriptionsManager.stopChannelSubscription(model); + delete this._cached[model.id]; + } + get unreadCount() { let count = 0; this.publicMessageChannels.forEach((channel) => { diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-subscriptions-manager.js b/plugins/chat/assets/javascripts/discourse/services/chat-subscriptions-manager.js index c8979c903e4..ee38eaf62ec 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-subscriptions-manager.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-subscriptions-manager.js @@ -1,4 +1,5 @@ import Service, { inject as service } from "@ember/service"; +import I18n from "I18n"; import { bind } from "discourse-common/utils/decorators"; import { CHANNEL_STATUSES } from "discourse/plugins/chat/discourse/models/chat-channel"; @@ -7,6 +8,9 @@ export default class ChatSubscriptionsManager extends Service { @service chatChannelsManager; @service currentUser; @service appEvents; + @service chat; + @service dialog; + @service router; _channelSubscriptions = new Set(); @@ -22,6 +26,7 @@ export default class ChatSubscriptionsManager extends Service { if (!channel.isDirectMessageChannel) { this._startChannelMentionsSubscription(channel); + this._startKickFromChannelSubscription(channel); } this._startChannelNewMessagesSubscription(channel); @@ -37,6 +42,10 @@ export default class ChatSubscriptionsManager extends Service { `/chat/${channel.id}/new-mentions`, this._onNewMentions ); + this.messageBus.unsubscribe( + `/chat/${channel.id}/kick`, + this._onKickFromChannel + ); } this._channelSubscriptions.delete(channel.id); @@ -93,6 +102,14 @@ export default class ChatSubscriptionsManager extends Service { ); } + _startKickFromChannelSubscription(channel) { + this.messageBus.subscribe( + `/chat/${channel.id}/kick`, + this._onKickFromChannel, + channel.meta.message_bus_last_ids.kick + ); + } + @bind _onChannelArchiveStatusUpdate(busData) { // we don't want to fetch a channel we don't have locally because archive status changed @@ -123,6 +140,34 @@ export default class ChatSubscriptionsManager extends Service { }); } + @bind + _onKickFromChannel(busData) { + this.chatChannelsManager.find(busData.channel_id).then((channel) => { + if (this.chat.activeChannel.id === channel.id) { + this.dialog.alert({ + message: I18n.t("chat.kicked_from_channel"), + didConfirm: () => { + this.chatChannelsManager.remove(channel); + + const firstChannel = + this.chatChannelsManager.publicMessageChannels[0]; + + if (firstChannel) { + this.router.transitionTo( + "chat.channel", + ...firstChannel.routeModels + ); + } else { + this.router.transitionTo("chat.browse"); + } + }, + }); + } else { + this.chatChannelsManager.remove(channel); + } + }); + } + _startChannelNewMessagesSubscription(channel) { this.messageBus.subscribe( `/chat/${channel.id}/new-messages`, diff --git a/plugins/chat/config/locales/client.en.yml b/plugins/chat/config/locales/client.en.yml index 5925fe1e6f7..3e62bd650c5 100644 --- a/plugins/chat/config/locales/client.en.yml +++ b/plugins/chat/config/locales/client.en.yml @@ -9,6 +9,7 @@ en: actions: chat_channel_status_change: "Chat channel status changed" chat_channel_delete: "Chat channel deleted" + chat_auto_remove_membership: "Memberships automatically removed from channels" api: scopes: descriptions: @@ -207,6 +208,7 @@ en: channels_list: "Chat channels list" no_public_channels: "You have not joined any channels." + kicked_from_channel: "You can no longer access this channel." only_chat_push_notifications: title: "Only send chat push notifications" description: "Block all non-chat push notifications from being sent" diff --git a/plugins/chat/lib/chat/channel_membership_manager.rb b/plugins/chat/lib/chat/channel_membership_manager.rb index d55a11546e6..838456c1018 100644 --- a/plugins/chat/lib/chat/channel_membership_manager.rb +++ b/plugins/chat/lib/chat/channel_membership_manager.rb @@ -66,7 +66,7 @@ module Chat end def enforce_automatic_channel_memberships - Jobs.enqueue(Jobs::Chat::AutoManageChannelMemberships, chat_channel_id: channel.id) + Jobs.enqueue(Jobs::Chat::AutoJoinChannelMemberships, chat_channel_id: channel.id) end def enforce_automatic_user_membership(user) diff --git a/plugins/chat/lib/chat/notifier.rb b/plugins/chat/lib/chat/notifier.rb index 8527e4ae205..0c9d16b37c8 100644 --- a/plugins/chat/lib/chat/notifier.rb +++ b/plugins/chat/lib/chat/notifier.rb @@ -7,7 +7,7 @@ # * Individual user mentions like @alfred # * Group mentions that include N users such as @support # * Global @here and @all mentions -# * Users watching the channel via UserChatChannelMembership +# * Users watching the channel via Chat::UserChatChannelMembership # # For various reasons a mention may not notify a user: # diff --git a/plugins/chat/lib/chat/steps_inspector.rb b/plugins/chat/lib/chat/steps_inspector.rb index 965a0f7612c..0f9b56263e7 100644 --- a/plugins/chat/lib/chat/steps_inspector.rb +++ b/plugins/chat/lib/chat/steps_inspector.rb @@ -3,7 +3,7 @@ module Chat # = Chat::StepsInspector # - # This class takes a {Chat::Service::Base::Context} object and inspects it. + # This class takes a {Service::Base::Context} object and inspects it. # It will output a list of steps and what is their known state. class StepsInspector # @!visibility private diff --git a/plugins/chat/plugin.rb b/plugins/chat/plugin.rb index 628b05aff98..83433ccdf60 100644 --- a/plugins/chat/plugin.rb +++ b/plugins/chat/plugin.rb @@ -286,6 +286,13 @@ after_initialize do if name == :secure_uploads && old_value == false && new_value == true Chat::SecureUploadsCompatibility.update_settings end + + if name == :chat_allowed_groups + Jobs.enqueue( + Jobs::Chat::AutoRemoveMembershipHandleChatAllowedGroupsChange, + new_allowed_groups: new_value, + ) + end end on(:post_alerter_after_save_post) do |post, new_record, notified| @@ -293,6 +300,13 @@ after_initialize do Chat::PostNotificationHandler.new(post, notified).handle end + on(:group_destroyed) do |group, user_ids| + Jobs.enqueue( + Jobs::Chat::AutoRemoveMembershipHandleDestroyedGroup, + destroyed_group_user_ids: user_ids, + ) + end + register_presence_channel_prefix("chat") do |channel_name| next nil unless channel_name == "/chat/online" config = PresenceChannel::Config.new @@ -369,14 +383,19 @@ after_initialize do end end + on(:user_removed_from_group) do |user, group| + Jobs.enqueue(Jobs::Chat::AutoRemoveMembershipHandleUserRemovedFromGroup, user_id: user.id) + end + on(:category_updated) do |category| # TODO(roman): remove early return after 2.9 release. # There's a bug on core where this event is triggered with an `#update` result (true/false) - return if !category.is_a?(Category) - category_channel = Chat::Channel.find_by(auto_join_users: true, chatable: category) + if category.is_a?(Category) && category_channel = Chat::Channel.find_by(chatable: category) + if category_channel.auto_join_users + Chat::ChannelMembershipManager.new(category_channel).enforce_automatic_channel_memberships + end - if category_channel - Chat::ChannelMembershipManager.new(category_channel).enforce_automatic_channel_memberships + Jobs.enqueue(Jobs::Chat::AutoRemoveMembershipHandleCategoryUpdated, category_id: category.id) end end diff --git a/plugins/chat/spec/integration/auto_channel_user_removal_spec.rb b/plugins/chat/spec/integration/auto_channel_user_removal_spec.rb new file mode 100644 index 00000000000..3265bacd172 --- /dev/null +++ b/plugins/chat/spec/integration/auto_channel_user_removal_spec.rb @@ -0,0 +1,285 @@ +# frozen_string_literal: true + +describe "Automatic user removal from channels" do + fab!(:user_1) { Fabricate(:user, trust_level: TrustLevel[1]) } + let(:user_1_guardian) { Guardian.new(user_1) } + fab!(:user_2) { Fabricate(:user, trust_level: TrustLevel[1]) } + + fab!(:secret_group) { Fabricate(:group) } + fab!(:private_category) { Fabricate(:private_category, group: secret_group) } + + fab!(:public_channel) { Fabricate(:chat_channel) } + fab!(:private_channel) { Fabricate(:chat_channel, chatable: private_category) } + fab!(:dm_channel) { Fabricate(:direct_message_channel, users: [user_1, user_2]) } + + before do + SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:trust_level_1] + SiteSetting.chat_enabled = true + Group.refresh_automatic_groups! + Jobs.run_immediately! + + secret_group.add(user_1) + public_channel.add(user_1) + private_channel.add(user_1) + public_channel.add(user_2) + + CategoryGroup.create(category: public_channel.chatable, group_id: Group::AUTO_GROUPS[:everyone]) + end + + context "when the chat_allowed_groups site setting changes" do + it "removes the user who is no longer in chat_allowed_groups" do + expect { SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:trust_level_3] }.to change { + Chat::UserChatChannelMembership.count + }.by(-3) + + expect( + Chat::UserChatChannelMembership.exists?(user: user_1, chat_channel: public_channel), + ).to eq(false) + expect(Chat::ChannelFetcher.all_secured_channel_ids(user_1_guardian)).not_to include( + public_channel.id, + ) + + expect( + Chat::UserChatChannelMembership.exists?(user: user_1, chat_channel: private_channel), + ).to eq(false) + expect(Chat::ChannelFetcher.all_secured_channel_ids(user_1_guardian)).not_to include( + private_channel.id, + ) + end + + it "does not remove the user who is in one of the chat_allowed_groups" do + user_2.change_trust_level!(TrustLevel[4]) + expect { SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:trust_level_3] }.to change { + Chat::UserChatChannelMembership.count + }.by(-2) + expect( + Chat::UserChatChannelMembership.exists?(user: user_2, chat_channel: public_channel), + ).to eq(true) + end + + it "does not remove users from their DM channels" do + expect { SiteSetting.chat_allowed_groups = "" }.to change { + Chat::UserChatChannelMembership.count + }.by(-3) + + expect(Chat::UserChatChannelMembership.exists?(user: user_1, chat_channel: dm_channel)).to eq( + true, + ) + expect(Chat::UserChatChannelMembership.exists?(user: user_2, chat_channel: dm_channel)).to eq( + true, + ) + end + + context "for staff users" do + fab!(:staff_user) { Fabricate(:admin) } + + it "does not remove them from public channels" do + public_channel.add(staff_user) + private_channel.add(staff_user) + SiteSetting.chat_allowed_groups = "" + expect( + Chat::UserChatChannelMembership.where( + user: staff_user, + chat_channel: [public_channel, private_channel], + ).count, + ).to eq(2) + end + + it "does not remove them from DM channels" do + staff_dm_channel = Fabricate(:direct_message_channel, users: [user_1, staff_user]) + expect( + Chat::UserChatChannelMembership.where( + user: staff_user, + chat_channel: [staff_dm_channel], + ).count, + ).to eq(1) + end + end + end + + context "when a user is removed from a group" do + context "when the user is no longer in any chat_allowed_groups" do + fab!(:group) { Fabricate(:group) } + + before do + group.add(user_1) + SiteSetting.chat_allowed_groups = group.id + end + + it "removes the user from the category channels" do + group.remove(user_1) + expect( + Chat::UserChatChannelMembership.where( + user: user_1, + chat_channel: [public_channel, private_channel], + ).count, + ).to eq(0) + end + + it "does not remove the user from DM channels" do + group.remove(user_1) + expect( + Chat::UserChatChannelMembership.where(user: user_1, chat_channel: dm_channel).count, + ).to eq(1) + end + + context "for staff users" do + fab!(:staff_user) { Fabricate(:admin) } + + it "does not remove them from public channels" do + public_channel.add(staff_user) + private_channel.add(staff_user) + group.add(staff_user) + group.remove(staff_user) + + expect( + Chat::UserChatChannelMembership.where( + user: staff_user, + chat_channel: [public_channel, private_channel], + ).count, + ).to eq(2) + end + end + end + + context "when a user is removed from a private category group" do + context "when the user is in another group that can interact with the channel" do + fab!(:stealth_group) { Fabricate(:group) } + before do + CategoryGroup.create!( + category: private_category, + group: stealth_group, + permission_type: CategoryGroup.permission_types[:full], + ) + stealth_group.add(user_1) + end + + it "does not remove them from the corresponding channel" do + secret_group.remove(user_1) + expect( + Chat::UserChatChannelMembership.exists?(user: user_1, chat_channel: private_channel), + ).to eq(true) + expect(Chat::ChannelFetcher.all_secured_channel_ids(user_1_guardian)).to include( + private_channel.id, + ) + end + end + + context "when the user is in no other groups that can interact with the channel" do + it "removes them from the corresponding channel" do + secret_group.remove(user_1) + expect( + Chat::UserChatChannelMembership.exists?(user: user_1, chat_channel: private_channel), + ).to eq(false) + expect(Chat::ChannelFetcher.all_secured_channel_ids(user_1_guardian)).not_to include( + private_channel.id, + ) + end + end + end + end + + context "when a category is updated" do + context "when the group's permission changes from reply+see to just see for the category" do + it "removes the user from the corresponding category channel" do + private_category.update!(permissions: { secret_group.id => :readonly }) + expect( + Chat::UserChatChannelMembership.exists?(user: user_1, chat_channel: private_channel), + ).to eq(false) + expect(Chat::ChannelFetcher.all_secured_channel_ids(user_1_guardian)).not_to include( + private_channel.id, + ) + end + + context "for staff users" do + fab!(:staff_user) { Fabricate(:admin) } + + it "does not remove them from the channel" do + secret_group.add(staff_user) + private_channel.add(staff_user) + private_category.update!(permissions: { secret_group.id => :readonly }) + expect( + Chat::UserChatChannelMembership.exists?( + user: staff_user, + chat_channel: private_channel, + ), + ).to eq(true) + end + end + end + + context "when the secret_group is no longer allowed to access the private category" do + it "removes the user from the corresponding category channel" do + private_category.update!(permissions: { Group::AUTO_GROUPS[:staff] => :full }) + expect( + Chat::UserChatChannelMembership.exists?(user: user_1, chat_channel: private_channel), + ).to eq(false) + expect(Chat::ChannelFetcher.all_secured_channel_ids(user_1_guardian)).not_to include( + private_channel.id, + ) + end + + context "for staff users" do + fab!(:staff_user) { Fabricate(:admin) } + + it "does not remove them from the channel" do + secret_group.add(staff_user) + private_channel.add(staff_user) + private_category.update!(permissions: {}) + expect( + Chat::UserChatChannelMembership.exists?( + user: staff_user, + chat_channel: private_channel, + ), + ).to eq(true) + end + end + end + end + + context "when a group is destroyed" do + context "when it was the last group on the private category" do + it "no users are removed because the category defaults to Everyone having full access" do + secret_group.destroy! + + expect( + Chat::UserChatChannelMembership.exists?(user: user_1, chat_channel: private_channel), + ).to eq(true) + expect(Chat::ChannelFetcher.all_secured_channel_ids(user_1_guardian)).to include( + private_channel.id, + ) + + expect( + Chat::UserChatChannelMembership.exists?(user: user_1, chat_channel: public_channel), + ).to eq(true) + expect(Chat::ChannelFetcher.all_secured_channel_ids(user_1_guardian)).to include( + public_channel.id, + ) + end + end + + context "when there is another group on the private category" do + before do + CategoryGroup.create(group_id: Group::AUTO_GROUPS[:staff], category: private_category) + end + + it "only removes users who are not in that group" do + secret_group.destroy! + + expect( + Chat::UserChatChannelMembership.exists?(user: user_1, chat_channel: private_channel), + ).to eq(false) + expect(Chat::ChannelFetcher.all_secured_channel_ids(user_1_guardian)).not_to include( + private_channel.id, + ) + + expect( + Chat::UserChatChannelMembership.exists?(user: user_1, chat_channel: public_channel), + ).to eq(true) + expect(Chat::ChannelFetcher.all_secured_channel_ids(user_1_guardian)).to include( + public_channel.id, + ) + end + end + end +end diff --git a/plugins/chat/spec/jobs/regular/chat/auto_manage_channel_memberships_spec.rb b/plugins/chat/spec/jobs/regular/chat/auto_join_channel_memberships_spec.rb similarity index 98% rename from plugins/chat/spec/jobs/regular/chat/auto_manage_channel_memberships_spec.rb rename to plugins/chat/spec/jobs/regular/chat/auto_join_channel_memberships_spec.rb index cedec7d7e80..bfb6cacfd02 100644 --- a/plugins/chat/spec/jobs/regular/chat/auto_manage_channel_memberships_spec.rb +++ b/plugins/chat/spec/jobs/regular/chat/auto_join_channel_memberships_spec.rb @@ -2,7 +2,7 @@ require "rails_helper" -describe Jobs::Chat::AutoManageChannelMemberships do +describe Jobs::Chat::AutoJoinChannelMemberships do let(:user) { Fabricate(:user, last_seen_at: 15.minutes.ago) } let(:category) { Fabricate(:category, user: user) } let(:channel) { Fabricate(:category_channel, auto_join_users: true, chatable: category) } diff --git a/plugins/chat/spec/jobs/regular/chat/kick_users_from_channel_spec.rb b/plugins/chat/spec/jobs/regular/chat/kick_users_from_channel_spec.rb new file mode 100644 index 00000000000..caecf6ef3e7 --- /dev/null +++ b/plugins/chat/spec/jobs/regular/chat/kick_users_from_channel_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +RSpec.describe Jobs::Chat::KickUsersFromChannel do + fab!(:channel) { Fabricate(:chat_channel) } + + it "publishes the correct MessageBus message" do + message = + MessageBus + .track_publish(Chat::Publisher.kick_users_message_bus_channel(channel.id)) do + described_class.new.execute(channel_id: channel.id, user_ids: [1, 2, 3]) + end + .first + + expect(message.user_ids).to eq([1, 2, 3]) + end + + it "does nothing if the channel is deleted" do + channel_id = channel.id + channel.trash! + message = + MessageBus + .track_publish(Chat::Publisher.kick_users_message_bus_channel(channel.id)) do + described_class.new.execute(channel_id: channel_id, user_ids: [1, 2, 3]) + end + .first + expect(message).to be_nil + end + + it "does nothing if no user_ids are provided" do + message = + MessageBus + .track_publish(Chat::Publisher.kick_users_message_bus_channel(channel.id)) do + described_class.new.execute(channel_id: channel.id) + end + .first + expect(message).to be_nil + end +end diff --git a/plugins/chat/spec/services/auto_remove/handle_category_updated_spec.rb b/plugins/chat/spec/services/auto_remove/handle_category_updated_spec.rb new file mode 100644 index 00000000000..95a1cfdc52b --- /dev/null +++ b/plugins/chat/spec/services/auto_remove/handle_category_updated_spec.rb @@ -0,0 +1,154 @@ +# frozen_string_literal: true + +RSpec.describe Chat::AutoRemove::HandleCategoryUpdated do + describe ".call" do + subject(:result) { described_class.call(params) } + + let(:params) { { category_id: updated_category.id } } + + fab!(:updated_category) { Fabricate(:category) } + fab!(:user_1) { Fabricate(:user) } + fab!(:user_2) { Fabricate(:user) } + fab!(:admin_1) { Fabricate(:admin) } + fab!(:admin_2) { Fabricate(:admin) } + fab!(:channel_1) { Fabricate(:chat_channel, chatable: updated_category) } + fab!(:channel_2) { Fabricate(:chat_channel, chatable: updated_category) } + + context "when chat is not enabled" do + before { SiteSetting.chat_enabled = false } + + it { is_expected.to fail_a_policy(:chat_enabled) } + end + + context "when chat is enabled" do + before { SiteSetting.chat_enabled = true } + + context "if the category is deleted" do + before { updated_category.destroy! } + + it "fails to find category model" do + expect(result).to fail_to_find_a_model(:category) + end + end + + context "when there are no channels associated with the category" do + before do + channel_1.destroy! + channel_2.destroy! + end + + it "fails to find category_channel_ids model" do + expect(result).to fail_to_find_a_model(:category_channel_ids) + end + end + + context "when the category has no more category_group records" do + before do + [user_1, user_2, admin_1, admin_2].each do |user| + channel_1.add(user) + channel_2.add(user) + end + updated_category.category_groups.delete_all + end + + it "sets the service result as successful" do + expect(result).to be_a_success + end + + it "does not kick any users since the default permission is Everyone (full)" do + expect { result }.not_to change { + Chat::UserChatChannelMembership.where( + user: [user_1, user_2, admin_1, admin_2], + chat_channel: [channel_1, channel_2], + ).count + } + end + end + + context "when the category still has category_group records" do + let(:action) { UserHistory.where(custom_type: "chat_auto_remove_membership").last } + + before do + [user_1, user_2, admin_1, admin_2].each do |user| + channel_1.add(user) + channel_2.add(user) + end + + group_1 = Fabricate(:group) + CategoryGroup.create( + group: group_1, + category: updated_category, + permission_type: CategoryGroup.permission_types[:full], + ) + + group_2 = Fabricate(:group) + CategoryGroup.create( + group: group_2, + category: updated_category, + permission_type: CategoryGroup.permission_types[:readonly], + ) + + group_1.add(user_1) + group_2.add(user_1) + end + + it "sets the service result as successful" do + expect(result).to be_a_success + end + + it "kicks all regular users who are not in any groups with reply + see permissions" do + expect { result }.to change { + Chat::UserChatChannelMembership.where( + user: [user_1, user_2], + chat_channel: [channel_1, channel_2], + ).count + }.to 2 + end + + it "does not kick admin users who are not in any groups with reply + see permissions" do + expect { result }.not_to change { + Chat::UserChatChannelMembership.where( + user: [admin_1, admin_2], + chat_channel: [channel_1, channel_2], + ).count + } + end + + it "enqueues a job to kick each batch of users from the channel" do + freeze_time + result + expect( + job_enqueued?( + job: Jobs::Chat::KickUsersFromChannel, + at: 5.seconds.from_now, + args: { + user_ids: [user_2.id], + channel_id: channel_1.id, + }, + ), + ).to eq(true) + + expect( + job_enqueued?( + job: Jobs::Chat::KickUsersFromChannel, + at: 5.seconds.from_now, + args: { + user_ids: [user_2.id], + channel_id: channel_2.id, + }, + ), + ).to eq(true) + end + + it "logs a staff action" do + result + expect(action).to have_attributes( + details: "users_removed: 1\nchannel_id: #{channel_2.id}\nevent: category_updated", + acting_user_id: Discourse.system_user.id, + custom_type: "chat_auto_remove_membership", + ) + end + end + end + end +end diff --git a/plugins/chat/spec/services/auto_remove/handle_chat_allowed_groups_change_spec.rb b/plugins/chat/spec/services/auto_remove/handle_chat_allowed_groups_change_spec.rb new file mode 100644 index 00000000000..741312730a1 --- /dev/null +++ b/plugins/chat/spec/services/auto_remove/handle_chat_allowed_groups_change_spec.rb @@ -0,0 +1,160 @@ +# frozen_string_literal: true + +RSpec.describe Chat::AutoRemove::HandleChatAllowedGroupsChange do + describe ".call" do + subject(:result) { described_class.call(params) } + + let(:params) { { new_allowed_groups: new_allowed_groups } } + fab!(:user_1) { Fabricate(:user) } + fab!(:user_2) { Fabricate(:user) } + fab!(:admin_1) { Fabricate(:admin) } + fab!(:admin_2) { Fabricate(:admin) } + + fab!(:dm_channel_1) { Fabricate(:direct_message_channel, users: [admin_1, user_1]) } + fab!(:dm_channel_2) { Fabricate(:direct_message_channel, users: [user_1, user_2]) } + + fab!(:public_channel_1) { Fabricate(:chat_channel) } + fab!(:public_channel_2) { Fabricate(:chat_channel) } + + context "when chat is not enabled" do + let(:new_allowed_groups) { "1|2" } + + before { SiteSetting.chat_enabled = false } + + it { is_expected.to fail_a_policy(:chat_enabled) } + end + + context "when chat is enabled" do + before { SiteSetting.chat_enabled = true } + + context "when new_allowed_groups is empty" do + let(:new_allowed_groups) { "" } + let(:action) { UserHistory.where(custom_type: "chat_auto_remove_membership").last } + + before do + public_channel_1.add(user_1) + public_channel_1.add(user_2) + public_channel_2.add(user_1) + public_channel_2.add(user_2) + public_channel_1.add(admin_1) + public_channel_1.add(admin_2) + freeze_time + end + + it "sets the service result as successful" do + expect(result).to be_a_success + end + + it "removes users from all public channels" do + expect { result }.to change { + Chat::UserChatChannelMembership.where( + user: [user_1, user_2], + chat_channel: [public_channel_1, public_channel_2], + ).count + }.to 0 + end + + it "does not remove admin users from public channels" do + expect { result }.not_to change { + Chat::UserChatChannelMembership.where( + user: [admin_1, admin_2], + chat_channel: [public_channel_1], + ).count + } + end + + it "does not remove users from direct message channels" do + expect { result }.not_to change { + Chat::UserChatChannelMembership.where(chat_channel: [dm_channel_1, dm_channel_2]).count + } + end + + it "enqueues a job to kick each batch of users from the channel" do + result + expect( + job_enqueued?( + job: Jobs::Chat::KickUsersFromChannel, + at: 5.seconds.from_now, + args: { + user_ids: [user_1.id, user_2.id], + channel_id: public_channel_1.id, + }, + ), + ).to eq(true) + expect( + job_enqueued?( + job: Jobs::Chat::KickUsersFromChannel, + at: 5.seconds.from_now, + args: { + user_ids: [user_1.id, user_2.id], + channel_id: public_channel_2.id, + }, + ), + ).to eq(true) + end + + it "logs a staff action" do + result + expect(action).to have_attributes( + details: + "users_removed: 2\nchannel_id: #{public_channel_2.id}\nevent: chat_allowed_groups_changed", + acting_user_id: Discourse.system_user.id, + custom_type: "chat_auto_remove_membership", + ) + end + end + + context "when new_allowed_groups includes all the users in public channels" do + let(:new_allowed_groups) { Group::AUTO_GROUPS[:trust_level_1] } + + before do + public_channel_1.add(user_1) + public_channel_2.add(user_1) + Group.refresh_automatic_groups! + end + + it "does nothing" do + expect { result }.not_to change { Chat::UserChatChannelMembership.count } + expect(result).to fail_to_find_a_model(:users) + end + end + + context "when new_allowed_groups includes everyone" do + let(:new_allowed_groups) { Group::AUTO_GROUPS[:everyone] } + + it { is_expected.to fail_a_policy(:not_everyone_allowed) } + + it "does nothing" do + expect { result }.not_to change { Chat::UserChatChannelMembership.count } + end + end + + context "when some users are not in any of the new allowed groups" do + let(:new_allowed_groups) { Group::AUTO_GROUPS[:trust_level_4] } + + before do + public_channel_1.add(user_1) + public_channel_1.add(user_2) + public_channel_2.add(user_1) + public_channel_2.add(user_2) + user_1.change_trust_level!(TrustLevel[2]) + user_2.change_trust_level!(TrustLevel[4]) + end + + it "removes them from public channels" do + expect { result }.to change { + Chat::UserChatChannelMembership.where( + chat_channel: [public_channel_1, public_channel_2], + ).count + }.by(-2) + end + + it "does not remove them from direct message channels" do + expect { result }.not_to change { + Chat::UserChatChannelMembership.where(chat_channel: [dm_channel_2]).count + } + end + end + end + end +end diff --git a/plugins/chat/spec/services/auto_remove/handle_destroyed_group_spec.rb b/plugins/chat/spec/services/auto_remove/handle_destroyed_group_spec.rb new file mode 100644 index 00000000000..1999747a867 --- /dev/null +++ b/plugins/chat/spec/services/auto_remove/handle_destroyed_group_spec.rb @@ -0,0 +1,251 @@ +# frozen_string_literal: true + +RSpec.describe Chat::AutoRemove::HandleDestroyedGroup do + describe ".call" do + subject(:result) { described_class.call(params) } + + let(:params) { { destroyed_group_user_ids: [admin_1.id, admin_2.id, user_1.id, user_2.id] } } + fab!(:user_1) { Fabricate(:user) } + fab!(:user_2) { Fabricate(:user) } + fab!(:admin_1) { Fabricate(:admin) } + fab!(:admin_2) { Fabricate(:admin) } + + fab!(:dm_channel_1) { Fabricate(:direct_message_channel, users: [admin_1, user_1]) } + fab!(:dm_channel_2) { Fabricate(:direct_message_channel, users: [user_1, user_2]) } + + fab!(:channel_1) { Fabricate(:chat_channel) } + fab!(:channel_2) { Fabricate(:chat_channel) } + + context "when chat is not enabled" do + before { SiteSetting.chat_enabled = false } + + it { is_expected.to fail_a_policy(:chat_enabled) } + end + + context "when chat is enabled" do + before { SiteSetting.chat_enabled = true } + + context "if none of the group_user_ids users exist" do + before { User.where(id: params[:destroyed_group_user_ids]).destroy_all } + + it "fails to find scoped_users model" do + expect(result).to fail_to_find_a_model(:scoped_users) + end + end + + describe "step remove_users_outside_allowed_groups" do + context "when chat_allowed_groups is empty" do + let(:action) { UserHistory.where(custom_type: "chat_auto_remove_membership").last } + + before do + SiteSetting.chat_allowed_groups = "" + channel_1.add(user_1) + channel_1.add(user_2) + channel_2.add(user_1) + channel_2.add(user_2) + channel_1.add(admin_1) + channel_1.add(admin_2) + end + + it "sets the service result as successful" do + expect(result).to be_a_success + end + + it "removes the destroyed_group_user_ids from all public channels" do + expect { result }.to change { + Chat::UserChatChannelMembership.where( + user: [user_1, user_2], + chat_channel: [channel_1, channel_2], + ).count + }.to 0 + end + + it "does not remove admin users from public channels" do + expect { result }.not_to change { + Chat::UserChatChannelMembership.where( + user: [admin_1, admin_2], + chat_channel: [channel_1], + ).count + } + end + + it "does not remove regular or admin users from direct message channels" do + expect { result }.not_to change { + Chat::UserChatChannelMembership.where( + chat_channel: [dm_channel_1, dm_channel_2], + ).count + } + end + + it "enqueues a job to kick each batch of users from the channel" do + freeze_time + result + expect( + job_enqueued?( + job: Jobs::Chat::KickUsersFromChannel, + at: 5.seconds.from_now, + args: { + user_ids: [user_1.id, user_2.id], + channel_id: channel_1.id, + }, + ), + ).to eq(true) + expect( + job_enqueued?( + job: Jobs::Chat::KickUsersFromChannel, + at: 5.seconds.from_now, + args: { + user_ids: [user_1.id, user_2.id], + channel_id: channel_2.id, + }, + ), + ).to eq(true) + end + + it "logs a staff action" do + result + expect(action).to have_attributes( + details: "users_removed: 2\nchannel_id: #{channel_2.id}\nevent: destroyed_group", + acting_user_id: Discourse.system_user.id, + custom_type: "chat_auto_remove_membership", + ) + end + end + + context "when chat_allowed_groups includes all the users in public channels" do + before do + SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:trust_level_1] + channel_1.add(user_1) + channel_1.add(user_2) + channel_2.add(user_1) + channel_2.add(user_2) + channel_1.add(admin_1) + channel_1.add(admin_2) + Group.refresh_automatic_groups! + end + + it "sets the service result as successful" do + expect(result).to be_a_success + end + + it "does not remove any memberships" do + expect { result }.not_to change { Chat::UserChatChannelMembership.count } + end + end + + context "when chat_allowed_groups includes everyone" do + before do + SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:everyone] + channel_1.add(user_1) + channel_1.add(user_2) + channel_2.add(user_1) + channel_2.add(user_2) + channel_1.add(admin_1) + channel_1.add(admin_2) + Group.refresh_automatic_groups! + end + + it { is_expected.to fail_a_policy(:not_everyone_allowed) } + + it "does not remove any memberships" do + expect { result }.not_to change { Chat::UserChatChannelMembership.count } + end + end + end + + describe "step remove_users_without_channel_permission" do + before do + channel_1.add(user_1) + channel_1.add(user_2) + channel_2.add(user_1) + channel_2.add(user_2) + channel_1.add(admin_1) + channel_1.add(admin_2) + Group.refresh_automatic_groups! + end + + context "when channel category not read_restricted with no category_groups" do + before do + channel_1.chatable.update!(read_restricted: false) + channel_1.chatable.category_groups.destroy_all + end + + it "does not remove any memberships" do + expect { result }.not_to change { Chat::UserChatChannelMembership.count } + end + end + + context "when category channel not read_restricted with no full/create_post permission groups" do + before do + channel_1.chatable.update!(read_restricted: false) + CategoryGroup.create!( + category: channel_1.chatable, + group_id: Group::AUTO_GROUPS[:everyone], + permission_type: CategoryGroup.permission_types[:readonly], + ) + CategoryGroup.create!( + category: channel_1.chatable, + group_id: Group::AUTO_GROUPS[:trust_level_1], + permission_type: CategoryGroup.permission_types[:readonly], + ) + end + + it "sets the service result as successful" do + expect(result).to be_a_success + end + + it "removes the destroyed_group_user_ids from the channel" do + expect { result }.to change { + Chat::UserChatChannelMembership.where( + user: [user_1, user_2], + chat_channel: [channel_1], + ).count + }.to 0 + end + + it "does not remove any admin destroyed_group_user_ids from the channel" do + expect { result }.not_to change { + Chat::UserChatChannelMembership.where( + user: [admin_1, admin_2], + chat_channel: [channel_1], + ).count + } + end + end + + context "when category channel not read_restricted with at least one full/create_post permission group" do + before do + channel_1.chatable.update!(read_restricted: false) + CategoryGroup.create!( + category: channel_1.chatable, + group_id: Group::AUTO_GROUPS[:everyone], + permission_type: CategoryGroup.permission_types[:readonly], + ) + CategoryGroup.create!( + category: channel_1.chatable, + group_id: Group::AUTO_GROUPS[:trust_level_2], + permission_type: CategoryGroup.permission_types[:create_post], + ) + end + + context "when one of the users is not in any of the groups" do + before { user_2.change_trust_level!(TrustLevel[3]) } + + it "sets the service result as successful" do + expect(result).to be_a_success + end + + it "removes the destroyed_group_user_ids from the channel" do + expect { result }.to change { + Chat::UserChatChannelMembership.where( + user: [user_1, user_2], + chat_channel: [channel_1], + ).count + }.to 1 + end + end + end + end + end + end +end diff --git a/plugins/chat/spec/services/auto_remove/handle_user_removed_from_group_spec.rb b/plugins/chat/spec/services/auto_remove/handle_user_removed_from_group_spec.rb new file mode 100644 index 00000000000..54d01b51b67 --- /dev/null +++ b/plugins/chat/spec/services/auto_remove/handle_user_removed_from_group_spec.rb @@ -0,0 +1,241 @@ +# frozen_string_literal: true + +RSpec.describe Chat::AutoRemove::HandleUserRemovedFromGroup do + describe ".call" do + subject(:result) { described_class.call(params) } + + let(:params) { { user_id: removed_user.id } } + fab!(:removed_user) { Fabricate(:user) } + fab!(:user_1) { Fabricate(:user) } + fab!(:user_2) { Fabricate(:user) } + + fab!(:dm_channel_1) { Fabricate(:direct_message_channel, users: [removed_user, user_1]) } + fab!(:dm_channel_2) { Fabricate(:direct_message_channel, users: [removed_user, user_2]) } + + fab!(:public_channel_1) { Fabricate(:chat_channel) } + fab!(:public_channel_2) { Fabricate(:chat_channel) } + + context "when chat is not enabled" do + before { SiteSetting.chat_enabled = false } + + it { is_expected.to fail_a_policy(:chat_enabled) } + end + + context "when chat is enabled" do + before { SiteSetting.chat_enabled = true } + + context "if user is deleted" do + before { removed_user.destroy! } + + it "fails to find the user model" do + expect(result).to fail_to_find_a_model(:user) + end + end + + context "when the user is no longer in any of the chat_allowed_groups" do + let(:action) { UserHistory.where(custom_type: "chat_auto_remove_membership").last } + + before do + SiteSetting.chat_allowed_groups = Fabricate(:group).id + public_channel_1.add(removed_user) + public_channel_2.add(removed_user) + end + + it "sets the service result as successful" do + expect(result).to be_a_success + end + + it "removes them from public channels" do + expect { result }.to change { + Chat::UserChatChannelMembership.where( + user: [removed_user], + chat_channel: [public_channel_1, public_channel_2], + ).count + }.to 0 + end + + it "does not remove them from direct message channels" do + expect { result }.not_to change { + Chat::UserChatChannelMembership.where( + user: [removed_user], + chat_channel: [dm_channel_1, dm_channel_2], + ).count + } + end + + it "enqueues a job to kick each batch of users from the channel" do + freeze_time + result + expect( + job_enqueued?( + job: Jobs::Chat::KickUsersFromChannel, + at: 5.seconds.from_now, + args: { + user_ids: [removed_user.id], + channel_id: public_channel_1.id, + }, + ), + ).to eq(true) + expect( + job_enqueued?( + job: Jobs::Chat::KickUsersFromChannel, + at: 5.seconds.from_now, + args: { + user_ids: [removed_user.id], + channel_id: public_channel_2.id, + }, + ), + ).to eq(true) + end + + it "logs a staff action" do + result + expect(action).to have_attributes( + details: + "users_removed: 1\nchannel_id: #{public_channel_2.id}\nevent: user_removed_from_group", + acting_user_id: Discourse.system_user.id, + custom_type: "chat_auto_remove_membership", + ) + end + + context "when the user is staff" do + fab!(:removed_user) { Fabricate(:admin) } + + it { is_expected.to fail_a_policy(:user_not_staff) } + + it "does not remove them from public channels" do + expect { result }.not_to change { + Chat::UserChatChannelMembership.where( + user: [removed_user], + chat_channel: [public_channel_1, public_channel_2], + ).count + } + end + end + + context "when the only chat_allowed_group is everyone" do + before { SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:everyone] } + + it { is_expected.to fail_a_policy(:not_everyone_allowed) } + + it "does not remove them from public channels" do + expect { result }.not_to change { + Chat::UserChatChannelMembership.where( + user: [removed_user], + chat_channel: [public_channel_1, public_channel_2], + ).count + } + end + end + end + + context "for private channels" do + fab!(:group_1) { Fabricate(:group) } + fab!(:group_2) { Fabricate(:group) } + fab!(:private_category) { Fabricate(:private_category, group: group_1) } + fab!(:private_channel_1) { Fabricate(:chat_channel, chatable: private_category) } + + before do + group_1.add(removed_user) + group_2.add(removed_user) + SiteSetting.chat_allowed_groups = group_1.id.to_s + "|" + group_2.id.to_s + CategoryGroup.create( + category: private_category, + group: group_2, + permission_type: CategoryGroup.permission_types[:full], + ) + private_channel_1.add(removed_user) + end + + context "when the user remains in one of the groups that can access a private channel" do + before { group_1.remove(removed_user) } + + it "sets the service result as successful" do + expect(result).to be_a_success + end + + it "does not remove them from that channel" do + expect { result }.not_to change { + Chat::UserChatChannelMembership.where( + user: [removed_user], + chat_channel: [private_channel_1], + ).count + } + end + end + + context "when the user in remains in one of the groups but that group only has readonly access to the channel" do + before do + CategoryGroup.find_by(group: group_2, category: private_category).update!( + permission_type: CategoryGroup.permission_types[:readonly], + ) + group_1.remove(removed_user) + end + + it "sets the service result as successful" do + expect(result).to be_a_success + end + + it "removes them from that channel" do + expect { result }.to change { + Chat::UserChatChannelMembership.where( + user: [removed_user], + chat_channel: [private_channel_1], + ).count + }.to 0 + end + + context "when the user is staff" do + fab!(:removed_user) { Fabricate(:admin) } + + it { is_expected.to fail_a_policy(:user_not_staff) } + + it "does not remove them from that channel" do + expect { result }.not_to change { + Chat::UserChatChannelMembership.where( + user: [removed_user], + chat_channel: [private_channel_1], + ).count + } + end + end + end + + context "when the user is no longer in any group that can access a private channel" do + before do + group_1.remove(removed_user) + group_2.remove(removed_user) + end + + it "sets the service result as successful" do + expect(result).to be_a_success + end + + it "removes them from that channel" do + expect { result }.to change { + Chat::UserChatChannelMembership.where( + user: [removed_user], + chat_channel: [private_channel_1], + ).count + }.to 0 + end + + context "when the user is staff" do + fab!(:removed_user) { Fabricate(:admin) } + + it { is_expected.to fail_a_policy(:user_not_staff) } + + it "does not remove them from that channel" do + expect { result }.not_to change { + Chat::UserChatChannelMembership.where( + user: [removed_user], + chat_channel: [private_channel_1], + ).count + } + end + end + end + end + end + end +end diff --git a/plugins/chat/spec/services/chat/update_channel_spec.rb b/plugins/chat/spec/services/chat/update_channel_spec.rb index 5b8d26b2a87..cc718d12eb2 100644 --- a/plugins/chat/spec/services/chat/update_channel_spec.rb +++ b/plugins/chat/spec/services/chat/update_channel_spec.rb @@ -97,7 +97,7 @@ RSpec.describe Chat::UpdateChannel do it "auto joins users" do expect_enqueued_with( - job: Jobs::Chat::AutoManageChannelMemberships, + job: Jobs::Chat::AutoJoinChannelMemberships, args: { chat_channel_id: channel.id, }, diff --git a/plugins/chat/spec/system/kick_user_from_channel_spec.rb b/plugins/chat/spec/system/kick_user_from_channel_spec.rb new file mode 100644 index 00000000000..82517026f7f --- /dev/null +++ b/plugins/chat/spec/system/kick_user_from_channel_spec.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +describe "Kick user from chat channel", type: :system, js: true do + fab!(:current_user) { Fabricate(:user) } + fab!(:channel_1) { Fabricate(:chat_channel) } + fab!(:channel_2) { Fabricate(:chat_channel) } + + let(:chat) { PageObjects::Pages::Chat.new } + let(:channel) { PageObjects::Pages::ChatChannel.new } + let(:dialog) { PageObjects::Components::Dialog.new } + let(:sidebar_page) { PageObjects::Pages::Sidebar.new } + + before do + SiteSetting.navigation_menu = "sidebar" + chat_system_bootstrap + sign_in(current_user) + channel_1.add(current_user) + channel_2.add(current_user) + end + + def publish_kick + Chat::Publisher.publish_kick_users(channel_1.id, [current_user.id]) + end + + context "when the user is looking at the channel they are kicked from" do + before { chat.visit_channel(channel_1) } + + it "shows an alert" do + publish_kick + expect(dialog).to have_content(I18n.t("js.chat.kicked_from_channel")) + end + + context "when the user presses ok" do + it "redirects them to the first other public channel they have" do + publish_kick + dialog.click_yes + expect(page).to have_current_path(channel_2.url) + end + + context "when the user has no other public channels" do + before do + channel_2.remove(current_user) + chat.visit_channel(channel_1) + end + + it "redirects them to the chat browse page" do + publish_kick + dialog.click_yes + expect(page).to have_current_path("/chat/browse/open") + end + end + end + end + + context "when the user is not looking at the channel they are kicked from" do + before { chat.visit_channel(channel_2) } + + it "removes it from their sidebar and does not redirect" do + publish_kick + expect(sidebar_page.channels_section).not_to have_css( + ".sidebar-section-link.channel-#{channel_1.id}", + ) + end + end +end