diff --git a/app/serializers/current_user_serializer.rb b/app/serializers/current_user_serializer.rb index 0a607648e75..8fefa2c9d55 100644 --- a/app/serializers/current_user_serializer.rb +++ b/app/serializers/current_user_serializer.rb @@ -170,7 +170,7 @@ class CurrentUserSerializer < BasicUserSerializer end def can_send_private_messages - scope.can_send_private_message?(Discourse.system_user) + scope.can_send_private_messages? end def can_edit diff --git a/app/serializers/user_card_serializer.rb b/app/serializers/user_card_serializer.rb index a109e9ebbfb..5e208169c7a 100644 --- a/app/serializers/user_card_serializer.rb +++ b/app/serializers/user_card_serializer.rb @@ -141,7 +141,7 @@ class UserCardSerializer < BasicUserSerializer # Needed because 'send_private_message_to_user' will always return false # when the current user is being serialized def can_send_private_messages - scope.can_send_private_message?(Discourse.system_user) + scope.can_send_private_messages? end def can_send_private_message_to_user diff --git a/lib/guardian.rb b/lib/guardian.rb index 0dbdd1dec3c..254fc1eab4f 100644 --- a/lib/guardian.rb +++ b/lib/guardian.rb @@ -445,23 +445,42 @@ class Guardian can_send_private_message?(group) end - def can_send_private_message?(target, notify_moderators: false) - is_user = target.is_a?(User) - is_group = target.is_a?(Group) + ## + # This should be used as a general, but not definitive, check for whether + # the user can send private messages _generally_, which is mostly useful + # for changing the UI. + # + # Please otherwise use can_send_private_message?(target, notify_moderators) + # to check if a single target can be messaged. + def can_send_private_messages?(notify_moderators: false) from_system = @user.is_system_user? from_bot = @user.bot? - (is_group || is_user) && # User is authenticated authenticated? && + # User can send PMs, this can be covered by trust levels as well via AUTO_GROUPS + (is_staff? || from_bot || from_system || \ + (@user.in_any_groups?(SiteSetting.personal_message_enabled_groups_map)) || notify_moderators) + end + + ## + # This should be used as a final check for when a user is sending a message + # to a target user or group. + def can_send_private_message?(target, notify_moderators: false) + target_is_user = target.is_a?(User) + target_is_group = target.is_a?(Group) + from_system = @user.is_system_user? + + # Must be a valid target + (target_is_group || target_is_user) && + # User is authenticated and can send PMs, this can be covered by trust levels as well via AUTO_GROUPS + can_send_private_messages?(notify_moderators: notify_moderators) && # User disabled private message - (is_staff? || is_group || target.user_option.allow_private_messages) && - # User can send PMs, this can be covered by trust levels as well via AUTO_GROUPS - (is_staff? || from_bot || from_system || (@user.in_any_groups?(SiteSetting.personal_message_enabled_groups_map)) || notify_moderators) && + (is_staff? || target_is_group || target.user_option.allow_private_messages) && # Can't send PMs to suspended users - (is_staff? || is_group || !target.suspended?) && + (is_staff? || target_is_group || !target.suspended?) && # Check group messageable level - (from_system || is_user || Group.messageable(@user).where(id: target.id).exists? || notify_moderators) && + (from_system || target_is_user || Group.messageable(@user).where(id: target.id).exists? || notify_moderators) && # Silenced users can only send PM to staff (!is_silenced? || target.staff?) end diff --git a/spec/lib/guardian_spec.rb b/spec/lib/guardian_spec.rb index feed9f9dac2..d0b03bd0b03 100644 --- a/spec/lib/guardian_spec.rb +++ b/spec/lib/guardian_spec.rb @@ -420,6 +420,78 @@ RSpec.describe Guardian do end end + describe 'can_send_private_messages' do + fab!(:suspended_user) { Fabricate(:user, suspended_till: 1.week.from_now, suspended_at: 1.day.ago) } + + it "returns false when the user is nil" do + expect(Guardian.new(nil).can_send_private_messages?).to be_falsey + end + + it "disallows pms to other users if the user is not in the automated trust level used for personal_message_enabled_groups" do + SiteSetting.personal_message_enabled_groups = Group::AUTO_GROUPS[:trust_level_2] + user.update!(trust_level: TrustLevel[1]) + Group.user_trust_level_change!(user.id, TrustLevel[1]) + user.reload + expect(Guardian.new(user).can_send_private_messages?).to be_falsey + user.update!(trust_level: TrustLevel[2]) + Group.user_trust_level_change!(user.id, TrustLevel[2]) + user.reload + expect(Guardian.new(user).can_send_private_messages?).to be_truthy + end + + context "when personal_message_enabled_groups does contain the user" do + let(:group) { Fabricate(:group) } + before do + SiteSetting.personal_message_enabled_groups = group.id + end + + it "returns true" do + expect(Guardian.new(user).can_send_private_messages?).to be_falsey + GroupUser.create(user: user, group: group) + user.reload + expect(Guardian.new(user).can_send_private_messages?).to be_truthy + end + end + + context "when personal_message_enabled_groups does not contain the user" do + let(:group) { Fabricate(:group) } + before do + SiteSetting.personal_message_enabled_groups = group.id + end + + it "returns false if user is not staff member" do + expect(Guardian.new(trust_level_4).can_send_private_messages?).to be_falsey + GroupUser.create(user: trust_level_4, group: group) + trust_level_4.reload + expect(Guardian.new(trust_level_4).can_send_private_messages?).to be_truthy + end + + it "returns true for staff member" do + expect(Guardian.new(moderator).can_send_private_messages?).to be_truthy + expect(Guardian.new(admin).can_send_private_messages?).to be_truthy + end + + it "returns true for bot user" do + expect(Guardian.new(Fabricate(:bot)).can_send_private_messages?).to be_truthy + end + + it "returns true for system user" do + expect(Guardian.new(Discourse.system_user).can_send_private_messages?).to be_truthy + end + end + + context "when author is silenced" do + before do + user.silenced_till = 1.year.from_now + user.save + end + + it "returns true, since there is no target user, we do that check separately" do + expect(Guardian.new(user).can_send_private_messages?).to be_truthy + end + end + end + describe 'can_reply_as_new_topic' do fab!(:topic) { Fabricate(:topic) } fab!(:private_message) { Fabricate(:private_message_topic) }