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.
This commit is contained in:
Martin Brennan
2023-03-22 10:19:59 +10:00
committed by GitHub
parent b5c4e1b813
commit 520d4f504b
38 changed files with 2174 additions and 19 deletions

View File

@@ -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

View File

@@ -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) }

View File

@@ -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

View File

@@ -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

View File

@@ -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

View File

@@ -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

View File

@@ -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

View File

@@ -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,
},

View File

@@ -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