FIX: prevents PM to large groups (#28681)

This commit introduces a new hidden site setting: `group_pm_user_limit`, default to `1000` which will raise an error when attempting to create a PM target a large group.
This commit is contained in:
Joffrey JAFFEUX 2024-09-03 12:08:14 +02:00 committed by GitHub
parent 80b9c280ba
commit e418f7056f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 84 additions and 4 deletions

View File

@ -464,7 +464,19 @@ class Group < ActiveRecord::Base
Group.auto_groups_between(:trust_level_0, :trust_level_4).to_a Group.auto_groups_between(:trust_level_0, :trust_level_4).to_a
end end
class GroupPmUserLimitExceededError < StandardError
end
def set_message_default_notification_levels!(topic, ignore_existing: false) def set_message_default_notification_levels!(topic, ignore_existing: false)
if user_count > SiteSetting.group_pm_user_limit
raise GroupPmUserLimitExceededError,
I18n.t(
"groups.errors.default_notification_level_users_limit",
count: SiteSetting.group_pm_user_limit,
group_name: name,
)
end
group_users group_users
.pluck(:user_id, :notification_level) .pluck(:user_id, :notification_level)
.each do |user_id, notification_level| .each do |user_id, notification_level|

View File

@ -612,6 +612,9 @@ en:
one: "%{count} user has been added to the group." one: "%{count} user has been added to the group."
other: "%{count} users have been added to the group." other: "%{count} users have been added to the group."
errors: errors:
default_notification_level_users_limit:
one: "Default notification level can't be set on groups larger than %{count} user. Incorrect group: %{group_name}"
other: "Default notification level can't be set on groups larger than %{count} users. Incorrect group: %{group_name}"
grant_trust_level_not_valid: "'%{trust_level}' is not a valid trust level." grant_trust_level_not_valid: "'%{trust_level}' is not a valid trust level."
can_not_modify_automatic: "You cannot modify an automatic group" can_not_modify_automatic: "You cannot modify an automatic group"
member_already_exist: member_already_exist:
@ -754,6 +757,7 @@ en:
reply_by_email_disabled: "Reply by email has been disabled." reply_by_email_disabled: "Reply by email has been disabled."
send_to_email_disabled: "Sorry, you cannot send personal messages to email." send_to_email_disabled: "Sorry, you cannot send personal messages to email."
target_user_not_found: "One of the users you are sending this message to could not be found." target_user_not_found: "One of the users you are sending this message to could not be found."
too_large_group: "The group: %{group_name}, has too many users to receive a message. Limit is %{limit}."
unable_to_update: "There was an error updating that topic." unable_to_update: "There was an error updating that topic."
unable_to_tag: "There was an error tagging the topic." unable_to_tag: "There was an error tagging the topic."
unable_to_unlist: "Sorry, you cannot create an unlisted topic." unable_to_unlist: "Sorry, you cannot create an unlisted topic."

View File

@ -1227,6 +1227,11 @@ posting:
max_form_template_content_length: max_form_template_content_length:
default: 5000 default: 5000
max: 150000 max: 150000
group_pm_user_limit:
default: 1000
type: integer
min: 1
hidden: true
email: email:
email_time_window_mins: email_time_window_mins:

View File

@ -15,14 +15,14 @@ module HasErrors
false false
end end
def rollback_with!(obj, error) def rollback_with!(obj, error, **kwargs)
obj.errors.add(:base, error) obj.errors.add(:base, error, **kwargs)
rollback_from_errors!(obj) rollback_from_errors!(obj)
end end
def rollback_from_errors!(obj) def rollback_from_errors!(obj)
add_errors_from(obj) add_errors_from(obj)
raise ActiveRecord::Rollback.new raise ActiveRecord::Rollback.new, obj.errors.full_messages.join("\n")
end end
def add_error(msg) def add_error(msg)

View File

@ -109,7 +109,18 @@ class TopicCreator
end end
topic.reload.topic_allowed_groups.each do |topic_allowed_group| topic.reload.topic_allowed_groups.each do |topic_allowed_group|
topic_allowed_group.group.set_message_default_notification_levels!(topic) group = topic_allowed_group.group
begin
group.set_message_default_notification_levels!(topic)
rescue Group::GroupPmUserLimitExceededError => e
rollback_with!(
topic,
:too_large_group,
group_name: group.name,
limit: SiteSetting.group_pm_user_limit,
)
end
end end
end end

View File

@ -563,6 +563,29 @@ RSpec.describe TopicCreator do
end end
end end
context "with too many users in a group" do
fab!(:group) { Fabricate(:group, messageable_level: Group::ALIAS_LEVELS[:everyone]) }
before do
SiteSetting.group_pm_user_limit = 1
Fabricate.times(2, :user).each { |user| group.add(user) }
pm_valid_attrs[:target_group_names] = group.name
end
it "fails with an error" do
expect do
TopicCreator.create(user, Guardian.new(admin), pm_valid_attrs)
end.to raise_error(
ActiveRecord::Rollback,
I18n.t(
"activerecord.errors.models.topic.attributes.base.too_large_group",
limit: SiteSetting.group_pm_user_limit,
group_name: group.name,
),
)
end
end
context "with to emails" do context "with to emails" do
it "works for staff" do it "works for staff" do
SiteSetting.send_email_messages_allowed_groups = "1|3" SiteSetting.send_email_messages_allowed_groups = "1|3"

View File

@ -94,6 +94,31 @@ RSpec.describe Group do
end end
end end
describe "#set_message_default_notification_levels!" do
context "with too many users in a group" do
fab!(:topic)
fab!(:large_group) { Fabricate(:group, messageable_level: Group::ALIAS_LEVELS[:everyone]) }
before do
SiteSetting.group_pm_user_limit = 1
Fabricate.times(2, :user).each { |user| large_group.add(user) }
end
it "raises a GroupPmUserLimitExceededError error" do
expect do
large_group.reload.set_message_default_notification_levels!(topic)
end.to raise_error(
Group::GroupPmUserLimitExceededError,
I18n.t(
"groups.errors.default_notification_level_users_limit",
count: SiteSetting.group_pm_user_limit,
group_name: large_group.name,
),
)
end
end
end
describe "#builtin" do describe "#builtin" do
context "when verifying enum sequence" do context "when verifying enum sequence" do
before { @builtin = Group.builtin } before { @builtin = Group.builtin }