SECURITY: Ability to bypass disabling chat of users

lib/guardian.rb

@@ -570,6 +570,22 @@ class Guardian
     @user.staff? || @user.in_any_groups?(SiteSetting.ignore_allowed_groups_map)
   end
 
+  def is_muting_user?(target_user)
+    @user.muted_user_ids.include?(target_user.id)
+  end
+
+  def is_ignoring_user?(target_user)
+    @user.ignored_user_ids.include?(target_user.id)
+  end
+
+  def is_muted_by_user?(target_user)
+    target_user.muted_user_ids.include?(@user.id)
+  end
+
+  def is_ignored_by_user?(target_user)
+    target_user.ignored_user_ids.include?(@user.id)
+  end
+
   def allowed_theme_repo_import?(repo)
     return false if !@user.admin?
 

plugins/chat/app/services/chat/channel/policy/message_creation.rb

@@ -1,27 +1,71 @@
 # frozen_string_literal: true
 
 class Chat::Channel::Policy::MessageCreation < Service::PolicyBase
-  class DirectMessageStrategy
-    class << self
-      def call(guardian, channel)
-        guardian.can_create_channel_message?(channel) || guardian.can_create_direct_message?
+  class Strategy
+    extend Dry::Initializer
+
+    param :guardian
+    param :channel
+  end
+
+  class DirectMessageStrategy < Strategy
+    delegate :username, to: :target, allow_nil: true, private: true
+
+    def call
+      if !guardian.can_create_channel_message?(channel) ||
+           !guardian.can_send_direct_message?(channel) || !guardian.allowing_direct_messages?
+        return false
+      end
+      if solo_chat? || channel.chatable.group? || guardian.user.is_system_user? ||
+           guardian.user.bot?
+        return true
       end
 
-      def reason(*)
+      target.present? && guardian.recipient_not_muted?(target) &&
+        guardian.recipient_not_ignored?(target) && guardian.recipient_can_chat?(target) &&
+        guardian.recipient_allows_direct_messages?(target)
+    end
+
+    def reason
+      if !guardian.can_create_channel_message?(channel)
+        I18n.t("chat.errors.channel_new_message_disallowed.closed")
+      elsif !guardian.can_send_direct_message?(channel)
+        I18n.t("chat.errors.user_cannot_send_direct_messages")
+      elsif !guardian.allowing_direct_messages?
+        I18n.t("chat.errors.actor_disallowed_dms")
+      elsif target.blank?
+        I18n.t("chat.errors.user_cannot_send_direct_messages")
+      elsif !guardian.recipient_not_muted?(target)
+        I18n.t("chat.errors.actor_muting_target_user", username:)
+      elsif !guardian.recipient_not_ignored?(target)
+        I18n.t("chat.errors.actor_ignoring_target_user", username:)
+      elsif !guardian.recipient_can_chat?(target)
+        I18n.t("chat.errors.not_reachable", username:)
+      elsif !guardian.recipient_allows_direct_messages?(target)
+        I18n.t("chat.errors.not_accepting_dms", username:)
+      else
         I18n.t("chat.errors.user_cannot_send_direct_messages")
       end
     end
+
+    private
+
+    def target
+      @target ||= channel.chatable.users.reject { |u| u.id == guardian.user.id }.first
+    end
+
+    def solo_chat?
+      @solo_chat ||= channel.chatable.users.size === 1
+    end
   end
 
-  class CategoryStrategy
-    class << self
-      def call(guardian, channel)
-        guardian.can_create_channel_message?(channel)
-      end
+  class CategoryStrategy < Strategy
+    def call
+      guardian.can_create_channel_message?(channel)
+    end
 
-      def reason(_, channel)
-        I18n.t("chat.errors.channel_new_message_disallowed.#{channel.status}")
-      end
+    def reason
+      I18n.t("chat.errors.channel_new_message_disallowed.#{channel.status}")
     end
   end
 
@@ -33,13 +77,14 @@ class Chat::Channel::Policy::MessageCreation < Service::PolicyBase
     super
     @strategy = CategoryStrategy
     @strategy = DirectMessageStrategy if channel.direct_message_channel?
+    @strategy = @strategy.new(guardian, channel)
   end
 
   def call
-    strategy.call(guardian, channel)
+    strategy.call
   end
 
   def reason
-    strategy.reason(guardian, channel)
+    strategy.reason
   end
 end

plugins/chat/config/locales/server.en.yml

@@ -85,6 +85,7 @@ en:
       actor_muting_target_user: "You are muting %{username}, so you cannot send messages to them."
       actor_disallowed_dms: "You have chosen to prevent users from sending you personal and direct messages, so you cannot create new direct messages."
       actor_preventing_target_user_from_dm: "You have chosen to prevent %{username} from sending you personal and direct messages, so you cannot create new direct messages to them."
+      not_reachable: "Sorry, %{username} is not using chat."
       user_cannot_send_direct_messages: "Sorry, you cannot send direct messages."
       over_chat_max_direct_message_users_allow_self: "You can only create a direct message with yourself."
       over_chat_max_direct_message_users:

plugins/chat/lib/chat/guardian_extensions.rb

@@ -28,6 +28,35 @@ module Chat
       is_staff? || can_direct_message?
     end
 
+    def can_send_direct_message?(channel)
+      return true if is_staff? || @user.bot?
+
+      can_chat? && channel.chatable.user_can_access?(@user) && !@user.suspended?
+    end
+
+    def allowing_direct_messages?
+      @user.user_option.allow_private_messages
+    end
+
+    def recipient_can_chat?(target)
+      target.guardian.can_chat? && target.user_option.chat_enabled
+    end
+
+    def recipient_not_muted?(target)
+      !is_muting_user?(target)
+    end
+
+    def recipient_not_ignored?(target)
+      !is_ignoring_user?(target)
+    end
+
+    def recipient_allows_direct_messages?(target)
+      return true if is_staff?
+      return false if !target.user_option.allow_private_messages
+
+      !is_ignored_by_user?(target) && !is_muted_by_user?(target) && !target.suspended?
+    end
+
     def hidden_tag_names
       @hidden_tag_names ||= DiscourseTagging.hidden_tag_names(self)
     end

plugins/chat/spec/fabricators/chat_fabricator.rb

@@ -96,6 +96,10 @@ Fabricator(:chat_message_with_service, class_name: "Chat::CreateMessage") do
     Group.refresh_automatic_groups!
     channel.add(user)
 
+    if !transients[:user] && channel.direct_message_channel?
+      channel.chatable.direct_message_users.find_or_create_by!(user: user)
+    end
+
     result =
       resolved_class.call(
         params: {

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

@@ -87,6 +87,8 @@ RSpec.describe Chat::GuardianExtensions do
         end
 
         context "when not group" do
+          before { dm_channel.add(user) }
+
           it "allows to edit the channel" do
             Chat::DirectMessageUser.create(user: user, direct_message: dm_channel.chatable)
             expect(user.guardian.can_edit_chat_channel?(dm_channel)).to eq(true)
@@ -631,13 +633,198 @@ RSpec.describe Chat::GuardianExtensions do
         channel.update!(status: :archived)
         expect(guardian.can_create_channel_message?(channel)).to eq(false)
       end
+    end
+  end
 
-      context "for direct message channels" do
-        it "it still allows the user to message even if they are not in direct_message_enabled_groups because they are not creating the channel" do
-          SiteSetting.direct_message_enabled_groups = Group::AUTO_GROUPS[:trust_level_4]
-          dm_channel.update!(status: :open)
-          expect(guardian.can_create_channel_message?(dm_channel)).to eq(true)
-        end
+  describe "#can_send_direct_message?" do
+    fab!(:other_user) { Fabricate(:user) }
+    fab!(:dm_channel) { Fabricate(:direct_message_channel, users: [user, other_user]) }
+    alias_matcher :be_able_to_send_direct_message, :be_can_send_direct_message
+
+    context "when sender is chatting to self" do
+      let(:dm_channel_solo) { Fabricate(:direct_message_channel, users: [user]) }
+
+      it "returns true" do
+        expect(guardian).to be_able_to_send_direct_message(dm_channel_solo)
+      end
+    end
+
+    context "when sender is not in chat enabled groups" do
+      before { SiteSetting.chat_allowed_groups = "" }
+
+      it "returns false" do
+        expect(guardian).not_to be_able_to_send_direct_message(dm_channel)
+      end
+    end
+
+    context "when sender is suspended" do
+      before do
+        UserSuspender.new(
+          user,
+          suspended_till: 1.day.from_now,
+          by_user: Discourse.system_user,
+          reason: "spam",
+        ).suspend
+      end
+
+      it "returns false" do
+        expect(guardian).not_to be_able_to_send_direct_message(dm_channel)
+      end
+    end
+
+    context "when sender is a bot" do
+      before { user.id = -1 }
+
+      it "returns true" do
+        expect(guardian).to be_able_to_send_direct_message(dm_channel)
+      end
+    end
+  end
+
+  describe "#allowing_direct_messages?" do
+    context "when user has disabled private messages" do
+      before { user.user_option.update!(allow_private_messages: false) }
+
+      it "returns false" do
+        expect(guardian.allowing_direct_messages?).to eq(false)
+      end
+    end
+
+    context "when user has enabled private messages" do
+      it "returns true" do
+        expect(guardian.allowing_direct_messages?).to eq(true)
+      end
+    end
+  end
+
+  describe "#recipient_can_chat?" do
+    fab!(:other_user) { Fabricate(:user) }
+    fab!(:dm_channel) { Fabricate(:direct_message_channel, users: [user, other_user]) }
+    alias_matcher :be_able_to_chat, :be_recipient_can_chat
+
+    context "when target user is not in chat enabled groups" do
+      before { SiteSetting.chat_allowed_groups = "" }
+
+      it "returns false" do
+        expect(guardian).not_to be_able_to_chat(other_user)
+      end
+    end
+
+    context "when target user is not in direct message enabled groups" do
+      before { SiteSetting.direct_message_enabled_groups = Group::AUTO_GROUPS[:admins] }
+
+      it "returns true" do
+        expect(guardian).to be_able_to_chat(other_user)
+      end
+    end
+
+    context "when target user has disabled chat" do
+      before { other_user.user_option.update!(chat_enabled: false) }
+
+      it "returns false" do
+        expect(guardian).not_to be_able_to_chat(other_user)
+      end
+    end
+  end
+
+  describe "#recipient_not_muted?" do
+    fab!(:other_user) { Fabricate(:user) }
+
+    context "when target user is not muted" do
+      it "returns true" do
+        expect(guardian.recipient_not_muted?(other_user)).to eq(true)
+      end
+    end
+
+    context "when target user is muted" do
+      before { MutedUser.create!(user: user, muted_user: other_user) }
+
+      it "returns false" do
+        expect(guardian.recipient_not_muted?(other_user)).to eq(false)
+      end
+    end
+  end
+
+  describe "#recipient_not_ignored?" do
+    fab!(:other_user) { Fabricate(:user) }
+
+    context "when target user is not ignored" do
+      it "returns true" do
+        expect(guardian.recipient_not_ignored?(other_user)).to eq(true)
+      end
+    end
+
+    context "when target user is ignored" do
+      before do
+        IgnoredUser.create!(user: user, ignored_user: other_user, expiring_at: 1.day.from_now)
+      end
+
+      it "returns false" do
+        expect(guardian.recipient_not_ignored?(other_user)).to eq(false)
+      end
+    end
+  end
+
+  describe "#recipient_allows_direct_messages?" do
+    fab!(:other_user) { Fabricate(:user) }
+    alias_matcher :be_able_to_receive_direct_message, :be_recipient_allows_direct_messages
+
+    context "when target user has disabled private messages" do
+      before { other_user.user_option.update(allow_private_messages: false) }
+
+      it "returns false" do
+        expect(guardian).not_to be_able_to_receive_direct_message(other_user)
+      end
+    end
+
+    context "when target user is suspended" do
+      before do
+        UserSuspender.new(
+          other_user,
+          suspended_till: 1.day.from_now,
+          by_user: Discourse.system_user,
+          reason: "spam",
+        ).suspend
+      end
+
+      it "returns false" do
+        expect(guardian).not_to be_able_to_receive_direct_message(other_user)
+      end
+    end
+
+    context "when sender is ignored by target user" do
+      before do
+        IgnoredUser.create!(user: other_user, ignored_user: user, expiring_at: 1.day.from_now)
+      end
+
+      it "returns false" do
+        expect(guardian).not_to be_able_to_receive_direct_message(other_user)
+      end
+    end
+
+    context "when sender is muted by target user" do
+      before { MutedUser.create!(user: other_user, muted_user: user) }
+
+      it "returns false" do
+        expect(guardian).not_to be_able_to_receive_direct_message(other_user)
+      end
+    end
+
+    context "when staff is muted by target user" do
+      before { MutedUser.create!(user: other_user, muted_user: staff) }
+
+      it "returns true" do
+        expect(staff_guardian).to be_able_to_receive_direct_message(other_user)
+      end
+    end
+
+    context "when staff is ignored by target user" do
+      before do
+        IgnoredUser.create!(user: other_user, ignored_user: staff, expiring_at: 1.day.from_now)
+      end
+
+      it "returns true" do
+        expect(staff_guardian).to be_able_to_receive_direct_message(other_user)
       end
     end
   end

plugins/chat/spec/requests/chat/api/messages_controller_spec.rb

@@ -361,8 +361,8 @@ RSpec.describe Chat::Api::ChannelMessagesController do
     end
 
     describe "for direct message" do
-      fab!(:user1) { Fabricate(:user) }
-      fab!(:user2) { Fabricate(:user) }
+      fab!(:user1) { Fabricate(:user, refresh_auto_groups: true) }
+      fab!(:user2) { Fabricate(:user, refresh_auto_groups: true) }
       fab!(:chatable) { Fabricate(:direct_message, users: [user1, user2]) }
       fab!(:direct_message_channel) { Fabricate(:direct_message_channel, chatable: chatable) }
 

plugins/chat/spec/services/chat/channel/policy/message_creation_spec.rb

@@ -3,7 +3,7 @@
 RSpec.describe Chat::Channel::Policy::MessageCreation do
   subject(:policy) { described_class.new(context) }
 
-  fab!(:user)
+  fab!(:user) { Fabricate(:user, refresh_auto_groups: true) }
 
   let(:guardian) { user.guardian }
   let(:context) { Service::Base::Context.build(channel: channel, guardian: guardian) }
@@ -11,8 +11,13 @@ RSpec.describe Chat::Channel::Policy::MessageCreation do
   describe "#call" do
     subject(:result) { policy.call }
 
-    context "when channel is a direct message one" do
-      fab!(:channel) { Fabricate(:direct_message_channel) }
+    context "when direct message channel" do
+      fab!(:channel) { Fabricate(:direct_message_channel, users: [user, Fabricate(:user)]) }
+
+      before do
+        SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:everyone]
+        SiteSetting.direct_message_enabled_groups = Group::AUTO_GROUPS[:everyone]
+      end
 
       context "when user can't create a message in this channel" do
         before { channel.closed!(Discourse.system_user) }
@@ -23,8 +28,8 @@ RSpec.describe Chat::Channel::Policy::MessageCreation do
           end
         end
 
-        context "when user can create direct messages" do
-          before { user.groups << Group.find(Group::AUTO_GROUPS[:trust_level_1]) }
+        context "when admin can create direct messages" do
+          before { user.grant_admin! }
 
           it "returns true" do
             expect(result).to be_truthy
@@ -37,9 +42,23 @@ RSpec.describe Chat::Channel::Policy::MessageCreation do
           expect(result).to be_truthy
         end
       end
+
+      context "when user can't send direct messages to the channel" do
+        before { channel.chatable.users.delete(user) }
+
+        it "returns false" do
+          expect(result).to be_falsey
+        end
+      end
+
+      context "when user can send direct messages to the channel" do
+        it "returns true" do
+          expect(result).to be_truthy
+        end
+      end
     end
 
-    context "when channel is a category one" do
+    context "when category channel" do
       fab!(:channel) { Fabricate(:chat_channel) }
 
       context "when user can't create a message in this channel" do
@@ -62,11 +81,82 @@ RSpec.describe Chat::Channel::Policy::MessageCreation do
     subject(:reason) { policy.reason }
 
     context "when channel is a direct message one" do
-      fab!(:channel) { Fabricate(:direct_message_channel) }
+      fab!(:other_user) { Fabricate(:user) }
+      fab!(:channel) { Fabricate(:direct_message_channel, users: [user, other_user]) }
+
+      before { SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:everyone] }
 
       it "returns a message related to direct messages" do
         expect(reason).to eq(I18n.t("chat.errors.user_cannot_send_direct_messages"))
       end
+
+      context "when sender does not allow direct messages" do
+        before { user.user_option.update!(allow_private_messages: false) }
+
+        it "returns a proper message" do
+          expect(reason).to eq(I18n.t("chat.errors.actor_disallowed_dms"))
+        end
+      end
+
+      context "when sender is not able to send messages to the channel" do
+        before { channel.chatable.users.delete(user) }
+
+        it "returns a proper message" do
+          expect(reason).to eq(I18n.t("chat.errors.user_cannot_send_direct_messages"))
+        end
+      end
+
+      context "when sender is muting the target user" do
+        before { MutedUser.create!(user: user, muted_user: other_user) }
+
+        it "returns a proper message" do
+          expect(reason).to eq(
+            I18n.t("chat.errors.actor_muting_target_user", username: other_user.username),
+          )
+        end
+      end
+
+      context "when sender is ignoring the target user" do
+        before do
+          IgnoredUser.create!(user: user, ignored_user: other_user, expiring_at: 1.day.from_now)
+        end
+
+        it "returns a proper message" do
+          expect(reason).to eq(
+            I18n.t("chat.errors.actor_ignoring_target_user", username: other_user.username),
+          )
+        end
+      end
+
+      context "when recipient is not able to chat" do
+        before { other_user.user_option.update!(chat_enabled: false) }
+
+        it "returns a proper message" do
+          expect(reason).to eq(I18n.t("chat.errors.not_reachable", username: other_user.username))
+        end
+      end
+
+      context "when sender is muted by the recipient" do
+        before { MutedUser.create!(user: other_user, muted_user: user) }
+
+        it "returns a proper message" do
+          expect(reason).to eq(
+            I18n.t("chat.errors.not_accepting_dms", username: other_user.username),
+          )
+        end
+      end
+
+      context "when sender is ignored by the recipient" do
+        before do
+          IgnoredUser.create!(user: other_user, ignored_user: user, expiring_at: 1.day.from_now)
+        end
+
+        it "returns a proper message" do
+          expect(reason).to eq(
+            I18n.t("chat.errors.not_accepting_dms", username: other_user.username),
+          )
+        end
+      end
     end
 
     context "when channel is a category one" do

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

@@ -311,8 +311,6 @@ RSpec.describe Chat::CreateMessage do
           end
 
           context "when user can join channel" do
-            before { user.groups << Group.find(Group::AUTO_GROUPS[:trust_level_1]) }
-
             context "when user can't create a message in the channel" do
               before { channel.closed!(Discourse.system_user) }