From 94b258deda1b09a1991d1dba3bc29d14225f1c53 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Tue, 20 Dec 2022 13:11:14 +1000 Subject: [PATCH] FIX: TL0 could not message group with everyone messageable_level (#19525) The commits e62e93f83a77adfa80b38fbfecf82bbee14e12fe and d6bd4ad7ee1d3bf43ba0a2bb1948f8f306b6177a caused a regression to the behaviour added for https://meta.discourse.org/t/allow-tl0-to-write-messages-to-staff-group-not-to-other-members-or-non-staff/124335, which allowed a user to message a group with the messageable_level set to Everyone even if they were TL0 (or otherwise did not reach the appropriate trust level). This commit fixes the issue and adjusts the spec to reflect the real scenario. c.f. https://meta.discourse.org/t/tl0-cant-message-groups-with-messageable-level-everyone-recession/249205 --- lib/guardian.rb | 11 ++++++++--- spec/lib/guardian_spec.rb | 4 ++-- spec/requests/groups_controller_spec.rb | 10 ++++++++++ 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/lib/guardian.rb b/lib/guardian.rb index 83387fc7bae..e6a4ad76ac1 100644 --- a/lib/guardian.rb +++ b/lib/guardian.rb @@ -472,15 +472,20 @@ class Guardian from_system = @user.is_system_user? # Must be a valid target - (target_is_group || target_is_user) && + return false if !(target_is_group || target_is_user) + + # Users can send messages to certain groups with the `everyone` messageable_level + # even if they are not in personal_message_enabled_groups + group_is_messageable = target_is_group && Group.messageable(@user).where(id: target.id).exists? + # 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) && + (can_send_private_messages?(notify_moderators: notify_moderators) || group_is_messageable) && # User disabled private message (is_staff? || target_is_group || target.user_option.allow_private_messages) && # Can't send PMs to suspended users (is_staff? || target_is_group || !target.suspended?) && # Check group messageable level - (from_system || target_is_user || Group.messageable(@user).where(id: target.id).exists? || notify_moderators) && + (from_system || target_is_user || group_is_messageable || 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 22d3b85f617..f386344328e 100644 --- a/spec/lib/guardian_spec.rb +++ b/spec/lib/guardian_spec.rb @@ -364,9 +364,9 @@ RSpec.describe Guardian do end end - it "allows TL0 to message group with messageable_level = everyone" do + it "allows TL0 to message group with messageable_level = everyone regardless of personal_message_enabled_groups" do group.update!(messageable_level: Group::ALIAS_LEVELS[:everyone]) - SiteSetting.personal_message_enabled_groups = Group::AUTO_GROUPS[:trust_level_0] + SiteSetting.personal_message_enabled_groups = Group::AUTO_GROUPS[:trust_level_1] expect(Guardian.new(trust_level_0).can_send_private_message?(group)).to eq(true) expect(Guardian.new(user).can_send_private_message?(group)).to eq(true) end diff --git a/spec/requests/groups_controller_spec.rb b/spec/requests/groups_controller_spec.rb index 493a4cc599b..33ff0daf1c5 100644 --- a/spec/requests/groups_controller_spec.rb +++ b/spec/requests/groups_controller_spec.rb @@ -639,6 +639,16 @@ RSpec.describe GroupsController do get "/groups/#{group.name}/messageable.json" expect(response.status).to eq(200) + body = response.parsed_body + expect(body["messageable"]).to eq(true) + + group.update!( + messageable_level: Group::ALIAS_LEVELS[:only_admins], + ) + + get "/groups/#{group.name}/messageable.json" + expect(response.status).to eq(200) + body = response.parsed_body expect(body["messageable"]).to eq(false) end