FIX: Limit new and existent staged users for email topics (#17970)

The maximum_staged_users_per_email site setting controls how many
staged users will be invited to the topic created from an incoming
email. Previously, it counted only the new staged users.
This commit is contained in:
Bianca Nenciu 2022-08-18 18:19:20 +03:00 committed by GitHub
parent b082f459c9
commit 707034bc75
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 32 additions and 15 deletions

View File

@ -55,6 +55,7 @@ module Email
def initialize(mail_string, opts = {}) def initialize(mail_string, opts = {})
raise EmptyEmailError if mail_string.blank? raise EmptyEmailError if mail_string.blank?
@staged_users = [] @staged_users = []
@created_staged_users = []
@raw_email = mail_string @raw_email = mail_string
COMMON_ENCODINGS.each do |encoding| COMMON_ENCODINGS.each do |encoding|
@ -92,7 +93,7 @@ module Email
post post
rescue Exception => e rescue Exception => e
@incoming_email.update_columns(error: e.class.name) if @incoming_email @incoming_email.update_columns(error: e.class.name) if @incoming_email
delete_staged_users delete_created_staged_users
raise raise
end end
end end
@ -698,11 +699,9 @@ module Email
end end
end end
def find_or_create_user(email, display_name, raise_on_failed_create: false) def find_or_create_user(email, display_name, raise_on_failed_create: false, user: nil)
user = nil
User.transaction do User.transaction do
user = User.find_by_email(email) user ||= User.find_by_email(email)
if user.nil? && SiteSetting.enable_staged_users if user.nil? && SiteSetting.enable_staged_users
raise EmailNotAllowed unless EmailValidator.allowed?(email) raise EmailNotAllowed unless EmailValidator.allowed?(email)
@ -715,12 +714,14 @@ module Email
name: display_name.presence || User.suggest_name(email), name: display_name.presence || User.suggest_name(email),
staged: true staged: true
) )
@staged_users << user @created_staged_users << user
rescue PG::UniqueViolation, ActiveRecord::RecordNotUnique, ActiveRecord::RecordInvalid rescue PG::UniqueViolation, ActiveRecord::RecordNotUnique, ActiveRecord::RecordInvalid
raise if raise_on_failed_create raise if raise_on_failed_create
user = nil user = nil
end end
end end
@staged_users << user if user&.staged?
end end
user user
@ -1358,6 +1359,8 @@ module Email
end end
def add_other_addresses(post, sender, mail_object) def add_other_addresses(post, sender, mail_object)
max_staged_users_post = nil
%i(to cc bcc).each do |d| %i(to cc bcc).each do |d|
next if mail_object[d].blank? next if mail_object[d].blank?
@ -1367,18 +1370,22 @@ module Email
email = address_field.address.downcase email = address_field.address.downcase
display_name = address_field.display_name.try(:to_s) display_name = address_field.display_name.try(:to_s)
next unless email["@"] next unless email["@"]
if should_invite?(email) if should_invite?(email)
user = find_or_create_user(email, display_name) user = User.find_by_email(email)
# cap number of staged users created per email
if (!user || user.staged) && @staged_users.count >= SiteSetting.maximum_staged_users_per_email
max_staged_users_post ||= post.topic.add_moderator_post(sender, I18n.t("emails.incoming.maximum_staged_user_per_email_reached"), import_mode: @opts[:import_mode])
next
end
user = find_or_create_user(email, display_name, user: user)
if user && can_invite?(post.topic, user) if user && can_invite?(post.topic, user)
post.topic.topic_allowed_users.create!(user_id: user.id) post.topic.topic_allowed_users.create!(user_id: user.id)
TopicUser.auto_notification_for_staging(user.id, post.topic_id, TopicUser.notification_reasons[:auto_watch]) TopicUser.auto_notification_for_staging(user.id, post.topic_id, TopicUser.notification_reasons[:auto_watch])
post.topic.add_small_action(sender, "invited_user", user.username, import_mode: @opts[:import_mode]) post.topic.add_small_action(sender, "invited_user", user.username, import_mode: @opts[:import_mode])
end end
# cap number of staged users created per email
if @staged_users.count > SiteSetting.maximum_staged_users_per_email
post.topic.add_moderator_post(sender, I18n.t("emails.incoming.maximum_staged_user_per_email_reached"), import_mode: @opts[:import_mode])
return
end
end end
rescue ActiveRecord::RecordInvalid, EmailNotAllowed rescue ActiveRecord::RecordInvalid, EmailNotAllowed
# don't care if user already allowed or the user's email address is not allowed # don't care if user already allowed or the user's email address is not allowed
@ -1413,8 +1420,8 @@ module Email
end end
end end
def delete_staged_users def delete_created_staged_users
@staged_users.each do |user| @created_staged_users.each do |user|
if @incoming_email.user&.id == user.id if @incoming_email.user&.id == user.id
@incoming_email.update_columns(user_id: nil) @incoming_email.update_columns(user_id: nil)
end end

View File

@ -879,7 +879,17 @@ RSpec.describe Email::Receiver do
it "cap the number of staged users created per email" do it "cap the number of staged users created per email" do
SiteSetting.maximum_staged_users_per_email = 1 SiteSetting.maximum_staged_users_per_email = 1
expect { process(:cc) }.to change(Topic, :count) expect { process(:cc) }.to change(Topic, :count).by(1)
.and change(User, :count).by(1)
expect(Topic.last.ordered_posts[-1].post_type).to eq(Post.types[:moderator_action])
end
it "cap the number of staged users existing per email" do
Fabricate(:user, email: "discourse@bar.com", staged: true) # from
Fabricate(:user, email: "someone@else.com", staged: true) # to
SiteSetting.maximum_staged_users_per_email = 1
expect { process(:cc) }.to change(Topic, :count).and not_change(User, :count)
expect(Topic.last.ordered_posts[-1].post_type).to eq(Post.types[:moderator_action]) expect(Topic.last.ordered_posts[-1].post_type).to eq(Post.types[:moderator_action])
end end