Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

FIX: when getting a reply by email, ensure it's by the same user

This commit is contained in:
Régis Hanol 2015-12-16 00:43:05 +01:00
parent 1e7850fa90
commit 4bb31daa2e
4 changed files with 106 additions and 69 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
app/jobs/scheduled/poll_mailbox.rb
View File

@ -62,7 +62,6 @@ module Jobs
message_template = :email_reject_post_error_specified message_template = :email_reject_post_error_specified
template_args[:post_error] = e.message template_args[:post_error] = e.message
end end
else else
message_template = nil message_template = nil
end end

108
lib/email/receiver.rb
View File

@ -18,6 +18,8 @@ module Email
class AutoGeneratedEmailError < ProcessingError; end class AutoGeneratedEmailError < ProcessingError; end
class EmailLogNotFound < ProcessingError; end class EmailLogNotFound < ProcessingError; end
class InvalidPost < ProcessingError; end class InvalidPost < ProcessingError; end
class ReplyUserNotFoundError < ProcessingError; end
class ReplyUserNotMatchingError < ProcessingError; end
attr_reader :body, :email_log attr_reader :body, :email_log
@ -29,71 +31,78 @@ module Email
def process def process
raise EmptyEmailError if @raw.blank? raise EmptyEmailError if @raw.blank?
message = Mail.new(@raw) @message = Mail.new(@raw)
body = parse_body(message) raise AutoGeneratedEmailError if @message.header.to_s =~ /auto-(replied|generated)/
@body = parse_body(@message)
dest_info = { type: :invalid, obj: nil }
# 'smtp_envelope_to' is a combination of: to, cc and bcc fields # 'smtp_envelope_to' is a combination of: to, cc and bcc fields
message.smtp_envelope_to.each do |to_address| # prioriziting the `:reply` types
dest_info = check_address(to_address) dest_infos = @message.smtp_envelope_to
break if dest_info[:type] != :invalid .map { |to_address| check_address(to_address) }
.compact
.sort do |a, b|
if a[:type] == :reply && b[:type] != :reply
1
elsif a[:type] != :reply && b[:type] == :reply
-1
else
0
end
end end
raise BadDestinationAddress if dest_info[:type] == :invalid raise BadDestinationAddress if dest_infos.empty?
raise AutoGeneratedEmailError if message.header.to_s =~ /auto-generated/ || message.header.to_s =~ /auto-replied/
# TODO get to a state where we can remove this
@message = message
@body = body
from = @message[:from].address_list.addresses.first from = @message[:from].address_list.addresses.first
user_email = "#{from.local}@#{from.domain}" user_email = "#{from.local}@#{from.domain}"
user_name = from.display_name user_name = from.display_name
@user = User.find_by_email(user_email) # TODO: deal with suspended/inactive users
user = User.find_by_email(user_email)
# TODO: take advantage of all the "TO"s
dest_info = dest_infos[0]
case dest_info[:type] case dest_info[:type]
when :group when :group
group = dest_info[:obj] group = dest_info[:obj]
if @user.blank? if user.blank?
if SiteSetting.allow_staged_accounts if SiteSetting.allow_staged_accounts
@user = create_staged_account(user_email, user_name) user = create_staged_account(user_email, user_name)
else else
wrap_body_in_quote(user_email) wrap_body_in_quote(user_email)
@user = Discourse.system_user user = Discourse.system_user
end end
end end
raise UserNotFoundError if @user.blank? create_new_topic(user, archetype: Archetype.private_message, target_group_names: [group.name])
raise UserNotSufficientTrustLevelError.new(@user) unless @user.has_trust_level?(TrustLevel[SiteSetting.email_in_min_trust.to_i])
create_new_topic(archetype: Archetype.private_message, target_group_names: [group.name])
when :category when :category
category = dest_info[:obj] category = dest_info[:obj]
if @user.blank? && category.email_in_allow_strangers if user.blank? && category.email_in_allow_strangers
if SiteSetting.allow_staged_accounts if SiteSetting.allow_staged_accounts
@user = create_staged_account(user_email) user = create_staged_account(user_email)
else else
wrap_body_in_quote(user_email) wrap_body_in_quote(user_email)
@user = Discourse.system_user user = Discourse.system_user
end end
end end
raise UserNotFoundError if @user.blank? raise UserNotFoundError if user.blank?
raise UserNotSufficientTrustLevelError.new(@user) unless category.email_in_allow_strangers || @user.has_trust_level?(TrustLevel[SiteSetting.email_in_min_trust.to_i]) raise UserNotSufficientTrustLevelError.new(user) unless category.email_in_allow_strangers || user.has_trust_level?(TrustLevel[SiteSetting.email_in_min_trust.to_i])
create_new_topic(category: category.id) create_new_topic(user, category: category.id)
when :reply when :reply
@email_log = dest_info[:obj] @email_log = dest_info[:obj]
raise EmailLogNotFound if @email_log.blank? raise EmailLogNotFound if @email_log.blank?
raise TopicNotFoundError if Topic.find_by_id(@email_log.topic_id).nil? raise TopicNotFoundError if Topic.find_by_id(@email_log.topic_id).nil?
raise TopicClosedError if Topic.find_by_id(@email_log.topic_id).closed? raise TopicClosedError if Topic.find_by_id(@email_log.topic_id).closed?
raise ReplyUserNotFoundError if user.blank?
raise ReplyUserNotMatchingError if @email_log.user_id != user.id
create_reply create_reply(@email_log)
end end
rescue Encoding::UndefinedConversionError, Encoding::InvalidByteSequenceError => e rescue Encoding::UndefinedConversionError, Encoding::InvalidByteSequenceError => e
@ -110,37 +119,36 @@ module Email
end end
def check_address(address) def check_address(address)
# only check groups/categories when 'email_in' is enabled # only check for a group/category when 'email_in' is enabled
if SiteSetting.email_in if SiteSetting.email_in
group = Group.find_by_email(address) group = Group.find_by_email(address)
return { type: :group, obj: group } if group return { address: address, type: :group, obj: group } if group
category = Category.find_by_email(address) category = Category.find_by_email(address)
return { type: :category, obj: category } if category return { address: address, type: :category, obj: category } if category
end end
regex = Regexp.escape(SiteSetting.reply_by_email_address) match = reply_by_email_address_regex.match(address)
regex = regex.gsub(Regexp.escape('%{reply_key}'), "(.*)")
regex = Regexp.new(regex)
match = regex.match address
if match && match[1].present? if match && match[1].present?
reply_key = match[1] email_log = EmailLog.for(match[1])
email_log = EmailLog.for(reply_key) return { address: address, type: :reply, obj: email_log }
return { type: :reply, obj: email_log } end
end end
{ type: :invalid, obj: nil } def reply_by_email_address_regex
@reply_by_email_address_regex ||= Regexp.new Regexp.escape(SiteSetting.reply_by_email_address)
.gsub(Regexp.escape("%{reply_key}"), "([[:xdigit:]]{32})")
end end
def parse_body(message) def parse_body(message)
body = select_body message body = select_body(message)
encoding = body.encoding encoding = body.encoding
raise EmptyEmailError if body.strip.blank? raise EmptyEmailError if body.strip.blank?
body = discourse_email_trimmer body body = discourse_email_trimmer(body)
raise EmptyEmailError if body.strip.blank? raise EmptyEmailError if body.strip.blank?
body = DiscourseEmailParser.parse_reply body body = DiscourseEmailParser.parse_reply(body)
raise EmptyEmailError if body.strip.blank? raise EmptyEmailError if body.strip.blank?
body.force_encoding(encoding).encode("UTF-8") body.force_encoding(encoding).encode("UTF-8")
@ -187,7 +195,7 @@ module Email
nil nil
end end
REPLYING_HEADER_LABELS = ['From', 'Sent', 'To', 'Subject', 'Reply To', 'Cc', 'Bcc', 'Date'] REPLYING_HEADER_LABELS = ['From', 'Sent', 'To', 'Subject', 'In-Reply-To', 'Cc', 'Bcc', 'Date']
REPLYING_HEADER_REGEX = Regexp.union(REPLYING_HEADER_LABELS.map { |lbl| "#{lbl}:" }) REPLYING_HEADER_REGEX = Regexp.union(REPLYING_HEADER_LABELS.map { |lbl| "#{lbl}:" })
def line_is_quote?(l) def line_is_quote?(l)
@ -230,25 +238,25 @@ module Email
@body = "[quote=\"#{user_email}\"]\n#{@body}\n[/quote]" @body = "[quote=\"#{user_email}\"]\n#{@body}\n[/quote]"
end end
def create_reply def create_reply(email_log)
create_post_with_attachments(@email_log.user, create_post_with_attachments(email_log.user,
raw: @body, raw: @body,
topic_id: @email_log.topic_id, topic_id: email_log.topic_id,
reply_to_post_number: @email_log.post.post_number) reply_to_post_number: email_log.post.post_number)
end end
def create_new_topic(topic_options={}) def create_new_topic(user, topic_options={})
topic_options[:raw] = @body topic_options[:raw] = @body
topic_options[:title] = @message.subject topic_options[:title] = @message.subject
result = create_post_with_attachments(@user, topic_options) result = create_post_with_attachments(user, topic_options)
topic_id = result.post.present? ? result.post.topic_id : nil topic_id = result.post.present? ? result.post.topic_id : nil
EmailLog.create( EmailLog.create(
email_type: "topic_via_incoming_email", email_type: "topic_via_incoming_email",
to_address: @message.from.first, # pick from address because we want the user's email to_address: user.email,
topic_id: topic_id, topic_id: topic_id,
user_id: @user.id, user_id: user.id,
) )
result result

50
spec/components/email/receiver_spec.rb
View File

@ -262,6 +262,7 @@ This is a link http://example.com"
let(:reply_key) { raise "Override this in a lower describe block" } let(:reply_key) { raise "Override this in a lower describe block" }
let(:email_raw) { raise "Override this in a lower describe block" } let(:email_raw) { raise "Override this in a lower describe block" }
# ---- # ----
let(:to) { SiteSetting.reply_by_email_address.gsub("%{reply_key}", reply_key) }
let(:receiver) { Email::Receiver.new(email_raw) } let(:receiver) { Email::Receiver.new(email_raw) }
let(:post) { create_post } let(:post) { create_post }
let(:topic) { post.topic } let(:topic) { post.topic }
@ -286,7 +287,7 @@ This is a link http://example.com"
describe "valid_reply.eml" do describe "valid_reply.eml" do
let!(:reply_key) { '59d8df8370b7e95c5a49fbf86aeb2c93' } let!(:reply_key) { '59d8df8370b7e95c5a49fbf86aeb2c93' }
let!(:email_raw) { fixture_file("emails/valid_reply.eml") } let!(:email_raw) { fill_email(fixture_file("emails/valid_reply.eml"), replying_user_email, to) }
it "creates a post with the correct content" do it "creates a post with the correct content" do
start_count = topic.posts.count start_count = topic.posts.count
@ -296,7 +297,6 @@ This is a link http://example.com"
expect(topic.posts.count).to eq(start_count + 1) expect(topic.posts.count).to eq(start_count + 1)
created_post = topic.posts.last created_post = topic.posts.last
expect(created_post.via_email).to eq(true) expect(created_post.via_email).to eq(true)
expect(created_post.raw_email).to eq(fixture_file("emails/valid_reply.eml"))
expect(created_post.cooked.strip).to eq(fixture_file("emails/valid_reply.cooked").strip) expect(created_post.cooked.strip).to eq(fixture_file("emails/valid_reply.cooked").strip)
end end
end end
@ -386,6 +386,7 @@ This is a link http://example.com"
describe "posting reply to a closed topic" do describe "posting reply to a closed topic" do
let(:reply_key) { raise "Override this in a lower describe block" } let(:reply_key) { raise "Override this in a lower describe block" }
let(:email_raw) { raise "Override this in a lower describe block" } let(:email_raw) { raise "Override this in a lower describe block" }
let(:to) { SiteSetting.reply_by_email_address.gsub("%{reply_key}", reply_key) }
let(:receiver) { Email::Receiver.new(email_raw) } let(:receiver) { Email::Receiver.new(email_raw) }
let(:topic) { Fabricate(:topic, closed: true) } let(:topic) { Fabricate(:topic, closed: true) }
let(:post) { Fabricate(:post, topic: topic, post_number: 1) } let(:post) { Fabricate(:post, topic: topic, post_number: 1) }
@ -407,7 +408,7 @@ This is a link http://example.com"
describe "should not create post" do describe "should not create post" do
let!(:reply_key) { '59d8df8370b7e95c5a49fbf86aeb2c93' } let!(:reply_key) { '59d8df8370b7e95c5a49fbf86aeb2c93' }
let!(:email_raw) { fixture_file("emails/valid_reply.eml") } let!(:email_raw) { fill_email(fixture_file("emails/valid_reply.eml"), replying_user_email, to) }
it "raises a TopicClosedError" do it "raises a TopicClosedError" do
expect { receiver.process }.to raise_error(Email::Receiver::TopicClosedError) expect { receiver.process }.to raise_error(Email::Receiver::TopicClosedError)
end end
@ -417,6 +418,7 @@ This is a link http://example.com"
describe "posting reply to a deleted topic" do describe "posting reply to a deleted topic" do
let(:reply_key) { raise "Override this in a lower describe block" } let(:reply_key) { raise "Override this in a lower describe block" }
let(:email_raw) { raise "Override this in a lower describe block" } let(:email_raw) { raise "Override this in a lower describe block" }
let(:to) { SiteSetting.reply_by_email_address.gsub("%{reply_key}", reply_key) }
let(:receiver) { Email::Receiver.new(email_raw) } let(:receiver) { Email::Receiver.new(email_raw) }
let(:deleted_topic) { Fabricate(:deleted_topic) } let(:deleted_topic) { Fabricate(:deleted_topic) }
let(:post) { Fabricate(:post, topic: deleted_topic, post_number: 1) } let(:post) { Fabricate(:post, topic: deleted_topic, post_number: 1) }
@ -438,7 +440,7 @@ This is a link http://example.com"
describe "should not create post" do describe "should not create post" do
let!(:reply_key) { '59d8df8370b7e95c5a49fbf86aeb2c93' } let!(:reply_key) { '59d8df8370b7e95c5a49fbf86aeb2c93' }
let!(:email_raw) { fixture_file("emails/valid_reply.eml") } let!(:email_raw) { fill_email(fixture_file("emails/valid_reply.eml"), replying_user_email, to) }
it "raises a TopicNotFoundError" do it "raises a TopicNotFoundError" do
expect { receiver.process }.to raise_error(Email::Receiver::TopicNotFoundError) expect { receiver.process }.to raise_error(Email::Receiver::TopicNotFoundError)
end end
@ -499,16 +501,16 @@ This is a link http://example.com"
describe "with a valid email" do describe "with a valid email" do
let(:reply_key) { "59d8df8370b7e95c5a49fbf86aeb2c93" } let(:reply_key) { "59d8df8370b7e95c5a49fbf86aeb2c93" }
let(:to) { SiteSetting.reply_by_email_address.gsub("%{reply_key}", reply_key) } let(:to) { SiteSetting.reply_by_email_address.gsub("%{reply_key}", reply_key) }
let(:user_email) { "test@test.com" }
let(:user) { Fabricate(:user, email: user_email, trust_level: 2)}
let(:post) { create_post(user: user) }
let(:valid_reply) { let(:valid_reply) {
reply = fixture_file("emails/valid_reply.eml") reply = fixture_file("emails/valid_reply.eml")
to = SiteSetting.reply_by_email_address.gsub("%{reply_key}", reply_key) fill_email(reply, user.email, to)
fill_email(reply, "test@test.com", to)
} }
let(:receiver) { Email::Receiver.new(valid_reply) } let(:receiver) { Email::Receiver.new(valid_reply) }
let(:post) { create_post }
let(:user) { post.user }
let(:email_log) { EmailLog.new(reply_key: reply_key, let(:email_log) { EmailLog.new(reply_key: reply_key,
post_id: post.id, post_id: post.id,
topic_id: post.topic_id, topic_id: post.topic_id,
@ -516,7 +518,7 @@ This is a link http://example.com"
post: post, post: post,
user: user, user: user,
email_type: 'test', email_type: 'test',
to_address: 'test@test.com' to_address: user.email
) } ) }
let(:reply_body) { let(:reply_body) {
"I could not disagree more. I am obviously biased but adventure time is the "I could not disagree more. I am obviously biased but adventure time is the
@ -527,7 +529,7 @@ greatest show ever created. Everyone should watch it.
describe "with an email log" do describe "with an email log" do
it "extracts data" do it "extracts data" do
expect{ receiver.process }.to raise_error(Email::Receiver::EmailLogNotFound) expect { receiver.process }.to raise_error(Email::Receiver::EmailLogNotFound)
email_log.save! email_log.save!
receiver.process receiver.process
@ -540,6 +542,34 @@ greatest show ever created. Everyone should watch it.
end end
describe "with a valid email from a different user" do
let(:reply_key) { SecureRandom.hex(16) }
let(:to) { SiteSetting.reply_by_email_address.gsub("%{reply_key}", reply_key) }
let(:user) { Fabricate(:user, email: "test@test.com", trust_level: 2)}
let(:post) { create_post(user: user) }
let!(:email_log) { EmailLog.create(reply_key: reply_key,
post_id: post.id,
topic_id: post.topic_id,
user_id: post.user_id,
post: post,
user: user,
email_type: 'test',
to_address: user.email) }
it "raises ReplyUserNotFoundError when user doesn't exist" do
reply = fill_email(fixture_file("emails/valid_reply.eml"), "unknown@user.com", to)
receiver = Email::Receiver.new(reply)
expect { receiver.process }.to raise_error(Email::Receiver::ReplyUserNotFoundError)
end
it "raises ReplyUserNotMatchingError when user is not matching the reply key" do
another_user = Fabricate(:user, email: "existing@user.com")
reply = fill_email(fixture_file("emails/valid_reply.eml"), another_user.email, to)
receiver = Email::Receiver.new(reply)
expect { receiver.process }.to raise_error(Email::Receiver::ReplyUserNotMatchingError)
end
end
describe "processes an email to a category" do describe "processes an email to a category" do
let(:to) { "some@email.com" } let(:to) { "some@email.com" }

6
spec/fixtures/emails/valid_reply.eml vendored
View File

@ -1,11 +1,11 @@
Return-Path: <jake@adventuretime.ooo> Return-Path: <FROM>
Received: from iceking.adventuretime.ooo ([unix socket]) by iceking (Cyrus v2.2.13-Debian-2.2.13-19+squeeze3) with LMTPA; Thu, 13 Jun 2013 17:03:50 -0400 Received: from iceking.adventuretime.ooo ([unix socket]) by iceking (Cyrus v2.2.13-Debian-2.2.13-19+squeeze3) with LMTPA; Thu, 13 Jun 2013 17:03:50 -0400
Received: from mail-ie0-x234.google.com (mail-ie0-x234.google.com [IPv6:2607:f8b0:4001:c03::234]) by iceking.adventuretime.ooo (8.14.3/8.14.3/Debian-9.4) with ESMTP id r5DL3nFJ016967 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for <reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 17:03:50 -0400 Received: from mail-ie0-x234.google.com (mail-ie0-x234.google.com [IPv6:2607:f8b0:4001:c03::234]) by iceking.adventuretime.ooo (8.14.3/8.14.3/Debian-9.4) with ESMTP id r5DL3nFJ016967 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for <reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 17:03:50 -0400
Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for <reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 14:03:48 -0700 Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for <reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 14:03:48 -0700
Received: by 10.0.0.1 with HTTP; Thu, 13 Jun 2013 14:03:48 -0700 Received: by 10.0.0.1 with HTTP; Thu, 13 Jun 2013 14:03:48 -0700
Date: Thu, 13 Jun 2013 17:03:48 -0400 Date: Thu, 13 Jun 2013 17:03:48 -0400
From: Jake the Dog <jake@adventuretime.ooo> From: FROM
To: reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo To: TO
Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com> Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com>
Subject: re: [Discourse Meta] eviltrout posted in 'Adventure Time Sux' Subject: re: [Discourse Meta] eviltrout posted in 'Adventure Time Sux'
Mime-Version: 1.0 Mime-Version: 1.0