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