FIX: ensures bot can always chat

plugins/chat/app/policies/chat/channel/message_creation_policy.rb

@@ -4,7 +4,8 @@ class Chat::Channel::MessageCreationPolicy < PolicyBase
   class DirectMessageStrategy
     class << self
       def call(guardian, channel)
-        guardian.can_create_channel_message?(channel) || guardian.can_create_direct_message?
+        guardian.can_create_channel_message?(channel) && guardian.can_create_direct_message? &&
+          guardian.can_chat?
       end
 
       def reason(*)
@@ -16,7 +17,7 @@ def reason(*)
   class CategoryStrategy
     class << self
       def call(guardian, channel)
-        guardian.can_create_channel_message?(channel)
+        guardian.can_create_channel_message?(channel) && guardian.can_chat?
       end
 
       def reason(_, channel)

plugins/chat/app/services/chat/update_message.rb

@@ -82,7 +82,9 @@ def fetch_uploads(contract:, guardian:)
     end
 
     def can_modify_channel_message(guardian:, message:)
-      guardian.can_modify_channel_message?(message.chat_channel)
+      result = guardian.can_modify_channel_message?(message.chat_channel) && guardian.can_chat?
+      result &&= guardian.can_direct_message? if message.chat_channel.direct_message_channel?
+      result
     end
 
     def can_modify_message(guardian:, message:)

plugins/chat/lib/chat/guardian_extensions.rb

@@ -17,15 +17,16 @@ def can_chat?
     end
 
     def can_direct_message?
-      @user.in_any_groups?(SiteSetting.direct_message_enabled_groups_map)
+      return false if anonymous?
+      is_staff? || @user.bot? || @user.in_any_groups?(SiteSetting.direct_message_enabled_groups_map)
     end
 
     def can_create_chat_message?
       !SpamRule::AutoSilence.prevent_posting?(@user)
     end
 
     def can_create_direct_message?
-      is_staff? || can_direct_message?
+      can_direct_message?
     end
 
     def hidden_tag_names

plugins/chat/spec/lib/chat/guardian_extensions_spec.rb

@@ -2,15 +2,67 @@
 
 RSpec.describe Chat::GuardianExtensions do
   fab!(:chatters) { Fabricate(:group) }
-  fab!(:user) { Fabricate(:user, group_ids: [chatters.id], refresh_auto_groups: true) }
+  fab!(:user) { Fabricate(:user, refresh_auto_groups: true) }
   fab!(:staff) { Fabricate(:user, admin: true) }
-  fab!(:chat_group) { Fabricate(:group) }
   fab!(:channel) { Fabricate(:category_channel) }
   fab!(:dm_channel) { Fabricate(:direct_message_channel) }
+
   let(:guardian) { Guardian.new(user) }
   let(:staff_guardian) { Guardian.new(staff) }
+  let(:bot_guardian) { Discourse.system_user.guardian }
+  let(:anonymous_guardian) { Guardian.new }
+
+  describe "#can_direct_message?" do
+    context "when the user is not in allowed to chat" do
+      let(:guardian) { Guardian.new(Fabricate(:user)) }
+
+      before { SiteSetting.direct_message_enabled_groups = Group::AUTO_GROUPS[:staff] }
+
+      it "cannot direct message" do
+        expect(guardian.can_direct_message?).to eq(false)
+      end
+
+      context "when the user is a bot" do
+        let(:guardian) { bot_guardian }
+
+        it "can direct message" do
+          expect(guardian.can_direct_message?).to eq(true)
+        end
+      end
 
-  before { SiteSetting.chat_allowed_groups = [chatters] }
+      context "when user is staff" do
+        let(:guardian) { staff_guardian }
+
+        it "can direct message" do
+          expect(guardian.can_direct_message?).to eq(true)
+        end
+      end
+    end
+
+    context "when user is anonymous" do
+      let(:guardian) { anonymous_guardian }
+
+      it "cannot chat" do
+        expect(guardian.can_direct_message?).to eq(false)
+      end
+    end
+
+    it "allows TL1 to chat by default and by extension higher trust levels" do
+      expect(guardian.can_direct_message?).to eq(true)
+      user.change_trust_level!(TrustLevel[3])
+      expect(guardian.can_direct_message?).to eq(true)
+    end
+
+    context "when user is in allowed group" do
+      before { SiteSetting.direct_message_enabled_groups = chatters.id }
+
+      it "can chat" do
+        expect { chatters.add(user) }.to change { user.reload.guardian.can_direct_message? }.from(
+          false,
+        ).to(true)
+      end
+    end
+  end
 
   describe "#can_chat?" do
     context "when the user is not in allowed to chat" do
@@ -21,7 +73,7 @@
       end
 
       context "when the user is a bot" do
-        let(:guardian) { Discourse.system_user.guardian }
+        let(:guardian) { bot_guardian }
 
         it "can chat" do
           expect(guardian.can_chat?).to eq(true)
@@ -38,7 +90,7 @@
     end
 
     context "when user is anonymous" do
-      let(:guardian) { Guardian.new }
+      let(:guardian) { anonymous_guardian }
 
       it "cannot chat" do
         expect(guardian.can_chat?).to eq(false)
@@ -51,12 +103,14 @@
       expect(guardian.can_chat?).to eq(true)
     end
 
-    it "allows user in specific group to chat" do
-      SiteSetting.chat_allowed_groups = chat_group.id
-      expect(guardian.can_chat?).to eq(false)
-      chat_group.add(user)
-      user.reload
-      expect(guardian.can_chat?).to eq(true)
+    context "when user is in allowed group" do
+      before { SiteSetting.chat_allowed_groups = chatters.id }
+
+      it "can chat" do
+        expect { chatters.add(user) }.to change { user.reload.guardian.can_chat? }.from(false).to(
+          true,
+        )
+      end
     end
   end
 

plugins/chat/spec/services/chat/create_message_spec.rb

@@ -47,7 +47,29 @@
     end
     let(:message) { result[:message_instance].reload }
 
-    before { channel.add(guardian.user) }
+    before do
+      SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:everyone]
+      SiteSetting.direct_message_enabled_groups = Group::AUTO_GROUPS[:everyone]
+      channel.add(guardian.user)
+    end
+
+    context "when posting in a direct message channel" do
+      fab!(:channel) { Fabricate(:direct_message_channel, users: [user, other_user]) }
+
+      it { is_expected.to be_a_success }
+
+      context "when user is not allowed to chat" do
+        before { SiteSetting.chat_allowed_groups = "" }
+
+        it { is_expected.to fail_a_policy(:allowed_to_create_message_in_channel) }
+      end
+
+      context "when user is not allowed to create direct messages" do
+        before { SiteSetting.direct_message_enabled_groups = Group::AUTO_GROUPS[:staff] }
+
+        it { is_expected.to fail_a_policy(:allowed_to_create_message_in_channel) }
+      end
+    end
 
     shared_examples "creating a new message" do
       it "saves the message" do
@@ -260,6 +282,12 @@
               it { is_expected.to fail_a_policy(:allowed_to_create_message_in_channel) }
             end
 
+            context "when user is not allowed to chat" do
+              before { SiteSetting.chat_allowed_groups = "" }
+
+              it { is_expected.to fail_a_policy(:allowed_to_create_message_in_channel) }
+            end
+
             context "when user can create a message in the channel" do
               context "when user is a member of the channel" do
                 fab!(:existing_message) { Fabricate(:chat_message, chat_channel: channel) }

plugins/chat/spec/services/chat/update_message_spec.rb

@@ -40,6 +40,7 @@
     before do
       SiteSetting.chat_enabled = true
       SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:everyone]
+      SiteSetting.direct_message_enabled_groups = Group::AUTO_GROUPS[:everyone]
       SiteSetting.chat_duplicate_message_sensitivity = 0
       Jobs.run_immediately!
 
@@ -850,12 +851,12 @@ def update_message(user)
     subject(:result) { described_class.call(params) }
 
     fab!(:current_user) { Fabricate(:user) }
-    fab!(:channel_1) { Fabricate(:chat_channel) }
     fab!(:upload_1) { Fabricate(:upload, user: current_user) }
+    fab!(:channel_1) { Fabricate(:chat_channel) }
     fab!(:message_1) do
       Fabricate(
         :chat_message,
-        chat_channel_id: channel_1.id,
+        chat_channel: channel_1,
         message: "old",
         upload_ids: [upload_1.id],
         user: current_user,
@@ -874,6 +875,8 @@ def update_message(user)
       SiteSetting.chat_editing_grace_period = 10
       SiteSetting.chat_editing_grace_period_max_diff_low_trust = 10
       SiteSetting.chat_editing_grace_period_max_diff_high_trust = 40
+      SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:everyone]
+      SiteSetting.direct_message_enabled_groups = Group::AUTO_GROUPS[:everyone]
 
       channel_1.add(current_user)
     end
@@ -939,6 +942,29 @@ def update_message(user)
       it { is_expected.to fail_a_policy(:can_modify_channel_message) }
     end
 
+    context "when user is not allowed to chat" do
+      before { SiteSetting.chat_allowed_groups = "" }
+
+      it { is_expected.to fail_a_policy(:can_modify_channel_message) }
+    end
+
+    context "when user is not allowed to chat in direct message channels" do
+      fab!(:channel_1) { Fabricate(:direct_message_channel, users: [current_user]) }
+      fab!(:message_1) do
+        Fabricate(
+          :chat_message,
+          chat_channel: channel_1,
+          message: "old",
+          upload_ids: [upload_1.id],
+          user: current_user,
+        )
+      end
+
+      before { SiteSetting.direct_message_enabled_groups = Group::AUTO_GROUPS[:staff] }
+
+      it { is_expected.to fail_a_policy(:can_modify_channel_message) }
+    end
+
     context "when user is not member of the channel" do
       let(:message_id) { Fabricate(:chat_message).id }
 

plugins/chat/spec/system/bot_spec.rb

@@ -0,0 +1,38 @@
+# frozen_string_literal: true
+
+RSpec.describe "Bot", type: :system do
+  let(:chat_page) { PageObjects::Pages::Chat.new }
+  let(:channel_page) { PageObjects::Pages::ChatChannel.new }
+  let(:current_user) { Discourse.system_user }
+
+  before do
+    chat_system_bootstrap
+    sign_in(current_user)
+  end
+
+  context "when not allowed to chat" do
+    before { SiteSetting.chat_allowed_groups = "" }
+
+    it "can send a message in a public channel" do
+      channel_1 = Fabricate(:category_channel)
+      message_1 =
+        Fabricate(:chat_message, chat_channel: channel_1, user: current_user, use_service: true)
+      chat_page.visit_channel(channel_1)
+
+      expect(channel_page.messages).to have_message(id: message_1.id)
+    end
+  end
+
+  context "when not allowed to use direct message" do
+    before { SiteSetting.direct_message_enabled_groups = Group::AUTO_GROUPS[:staff] }
+
+    it "can send a message in a direct message channel" do
+      channel_1 = Fabricate(:direct_message_channel)
+      message_1 =
+        Fabricate(:chat_message, chat_channel: channel_1, user: current_user, use_service: true)
+      chat_page.visit_channel(channel_1)
+
+      expect(channel_page.messages).to have_message(id: message_1.id)
+    end
+  end
+end