From 6296ae3d31915f9c521d2b9a04bf2070f501bf82 Mon Sep 17 00:00:00 2001 From: Leo McArdle Date: Wed, 7 Aug 2019 11:32:19 +0100 Subject: [PATCH] FEATURE: add setting to show content of forwarded emails in topics (#7935) --- config/locales/server.en.yml | 2 +- config/site_settings.yml | 8 +- ...orwarded_emails_behaviour_site_settings.rb | 35 ++++++++ lib/email/receiver.rb | 85 +++++++++++++------ spec/components/email/receiver_spec.rb | 69 +++++++++++++-- 5 files changed, 165 insertions(+), 34 deletions(-) create mode 100644 db/migrate/20190724162522_migrate_forwarded_emails_behaviour_site_settings.rb diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index c40cdf9ee5a..ba940c74ae9 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1811,7 +1811,7 @@ en: attachment_content_type_blacklist: "List of keywords used to blacklist attachments based on the content type." attachment_filename_blacklist: "List of keywords used to blacklist attachments based on the filename." - enable_forwarded_emails: "[BETA] Allow users to create a topic by forwarding an email in." + forwarded_emails_behaviour: "How to treat a forwarded email to Discourse" always_show_trimmed_content: "Always show trimmed part of incoming emails. WARNING: might reveal email addresses." private_email: "Don't include content from posts or topics in email title or email body. NOTE: also disables digest emails." email_total_attachment_size_limit_kb: "Max total size of files attached to outgoing emails. Set to 0 to disable sending of attachments." diff --git a/config/site_settings.yml b/config/site_settings.yml index ad28491b7d9..69655450336 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1022,7 +1022,13 @@ email: type: list default: "smime.p7s|signature.asc|winmail.dat" list_type: compact - enable_forwarded_emails: false + forwarded_emails_behaviour: + type: enum + default: hide + choices: + - hide + - quote + - create_replies always_show_trimmed_content: false private_email: false email_custom_template: diff --git a/db/migrate/20190724162522_migrate_forwarded_emails_behaviour_site_settings.rb b/db/migrate/20190724162522_migrate_forwarded_emails_behaviour_site_settings.rb new file mode 100644 index 00000000000..eee5c04dc8b --- /dev/null +++ b/db/migrate/20190724162522_migrate_forwarded_emails_behaviour_site_settings.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +class MigrateForwardedEmailsBehaviourSiteSettings < ActiveRecord::Migration[5.2] + def up + execute <<~SQL + UPDATE site_settings + SET name = 'forwarded_emails_behaviour', + data_type = 7, + value = 'create_replies' + WHERE name = 'enable_forwarded_emails' AND value = 't'; + SQL + + execute <<~SQL + DELETE + FROM site_settings + WHERE name = 'enable_forwarded_emails'; + SQL + end + + def down + execute <<~SQL + UPDATE site_settings + SET name = 'enable_forwarded_emails', + data_type = 5, + value = 't' + WHERE name = 'forwarded_emails_behaviour' AND value = 'create_replies'; + SQL + + execute <<~SQL + DELETE + FROM site_settings + WHERE name = 'forwarded_emails_behaviour'; + SQL + end +end diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb index e02ba46146e..418d8d2aade 100644 --- a/lib/email/receiver.rb +++ b/lib/email/receiver.rb @@ -660,7 +660,7 @@ module Email end def process_destination(destination, user, body, elided, hidden_reason_id) - return if SiteSetting.enable_forwarded_emails && + return if SiteSetting.forwarded_emails_behaviour != "hide" && has_been_forwarded? && process_forwarded_email(destination, user) @@ -800,27 +800,30 @@ module Email def process_forwarded_email(destination, user) user ||= stage_from_user - embedded = Mail.new(embedded_email_raw) - email, display_name = parse_from_field(embedded) - - return false if email.blank? || !email["@"] - - raw = try_to_encode(embedded.decoded, "UTF-8").presence || embedded.to_s - title = embedded.subject.presence || subject + case SiteSetting.forwarded_emails_behaviour + when "create_replies" + forwarded_email_create_replies(destination, user) + when "quote" + forwarded_email_quote_forwarded(destination, user) + else + false + end + end + def forwarded_email_create_topic(destination: , user: , raw: , title: , date: nil, embedded_user: nil) case destination[:type] when :group group = destination[:obj] - embedded_user = find_or_create_user(email, display_name) - post = create_topic(user: embedded_user, - raw: raw, - title: title, - archetype: Archetype.private_message, - target_usernames: [user.username], - target_group_names: [group.name], - is_group_message: true, - skip_validations: true, - created_at: embedded.date) + topic_user = embedded_user&.call || user + create_topic(user: topic_user, + raw: raw, + title: title, + archetype: Archetype.private_message, + target_usernames: [user.username], + target_group_names: [group.name], + is_group_message: true, + skip_validations: true, + created_at: date) when :category category = destination[:obj] @@ -828,16 +831,31 @@ module Email return false if user.staged? && !category.email_in_allow_strangers return false if !user.has_trust_level?(SiteSetting.email_in_min_trust) - embedded_user = find_or_create_user(email, display_name) - post = create_topic(user: embedded_user, - raw: raw, - title: title, - category: category.id, - skip_validations: embedded_user.staged?, - created_at: embedded.date) + topic_user = embedded_user&.call || user + create_topic(user: topic_user, + raw: raw, + title: title, + category: category.id, + skip_validations: topic_user.staged?, + created_at: date) else - return false + false end + end + + def forwarded_email_create_replies(destination, user) + embedded = Mail.new(embedded_email_raw) + email, display_name = parse_from_field(embedded) + + return false if email.blank? || !email["@"] + + post = forwarded_email_create_topic(destination: destination, + user: user, + raw: try_to_encode(embedded.decoded, "UTF-8").presence || embedded.to_s, + title: embedded.subject.presence || subject, + date: embedded.date, + embedded_user: lambda { find_or_create_user(email, display_name) }) + return false unless post if post&.topic # mark post as seen for the forwarder @@ -846,7 +864,7 @@ module Email # create reply when available if @before_embedded.present? post_type = Post.types[:regular] - post_type = Post.types[:whisper] if post.topic.private_message? && group.usernames[user.username] + post_type = Post.types[:whisper] if post.topic.private_message? && destination[:obj].usernames[user.username] create_reply(user: user, raw: @before_embedded, @@ -860,6 +878,19 @@ module Email true end + def forwarded_email_quote_forwarded(destination, user) + embedded = embedded_email_raw + raw = <<~EOF + #{@before_embedded} + + [quote] + #{PlainTextToMarkdown.new(embedded).to_markdown} + [/quote] + EOF + + return true if forwarded_email_create_topic(destination: destination, user: user, raw: raw, title: subject) + end + def self.reply_by_email_address_regex(extract_reply_key = true, include_verp = false) reply_addresses = [SiteSetting.reply_by_email_address] reply_addresses << (SiteSetting.alternative_reply_by_email_addresses.presence || "").split("|") diff --git a/spec/components/email/receiver_spec.rb b/spec/components/email/receiver_spec.rb index c31385d5caa..f563a7b8d96 100644 --- a/spec/components/email/receiver_spec.rb +++ b/spec/components/email/receiver_spec.rb @@ -708,6 +708,41 @@ describe Email::Receiver do end + shared_examples "creates topic with forwarded message as quote" do |destination, address| + it "creates topic with forwarded message as quote" do + expect { process(:forwarded_email_1) }.to change(Topic, :count) + + topic = Topic.last + if destination == :category + expect(topic.category).to eq(Category.where(email_in: address).first) + else + expect(topic.archetype).to eq(Archetype.private_message) + expect(topic.allowed_groups).to eq(Group.where(incoming_email: address)) + end + + post = Post.last + + expect(post.user.email).to eq("ba@bar.com") + expect(post.raw).to eq(<<~EOF.chomp + @team, can you have a look at this email below? + + [quote] + From: Some One <some@one\\.com> + To: Ba Bar <ba@bar\\.com> + Date: Mon, 1 Dec 2016 00:13:37 \\+0100 + Subject: Discoursing much? + + Hello Ba Bar, + + Discoursing much today? + + XoXo + [/quote] + EOF + ) + end + end + context "new message to a group" do fab!(:group) { Fabricate(:group, incoming_email: "team@bar.com|meat@bar.com") } @@ -818,10 +853,10 @@ describe Email::Receiver do expect(user.user_option.email_messages_level).to eq(UserOption.email_level_types[:always]) end - context "with forwarded emails enabled" do + context "with forwarded emails behaviour set to create replies" do before do Fabricate(:group, incoming_email: "some_group@bar.com") - SiteSetting.enable_forwarded_emails = true + SiteSetting.forwarded_emails_behaviour = "create_replies" end it "handles forwarded emails" do @@ -842,7 +877,7 @@ describe Email::Receiver do group.add(Fabricate(:user, email: "ba@bar.com")) group.save - SiteSetting.enable_forwarded_emails = true + SiteSetting.forwarded_emails_behaviour = "create_replies" expect { process(:forwarded_email_2) }.to change(Topic, :count) forwarded_post, last_post = *Post.last(2) @@ -863,6 +898,14 @@ describe Email::Receiver do end + context "with forwarded emails behaviour set to quote" do + before do + SiteSetting.forwarded_emails_behaviour = "quote" + end + + include_examples "creates topic with forwarded message as quote", :group, "team@bar.com|meat@bar.com" + end + context "when message sent to a group has no key and find_related_post_with_key is enabled" do let!(:topic) do process(:email_reply_1) @@ -1202,6 +1245,22 @@ describe Email::Receiver do include_examples "does not create staged users", :no_date, Email::Receiver::InvalidPost end + + context "with forwarded emails behaviour set to quote" do + before do + SiteSetting.forwarded_emails_behaviour = "quote" + end + + context "with a category which allows strangers" do + fab!(:category) { Fabricate(:category, email_in: "team@bar.com", email_in_allow_strangers: true) } + include_examples "creates topic with forwarded message as quote", :category, "team@bar.com" + end + + context "with a category which doesn't allow strangers" do + fab!(:category) { Fabricate(:category, email_in: "team@bar.com", email_in_allow_strangers: false) } + include_examples "cleans up staged users", :forwarded_email_1, Email::Receiver::StrangersNotAllowedError + end + end end context "email is a reply" do @@ -1220,9 +1279,9 @@ describe Email::Receiver do include_examples "does not create staged users", :reply_user_not_matching, Email::Receiver::ReplyUserNotMatchingError end - context "when forwarded emails are enabled" do + context "with forwarded emails behaviour set to create replies" do before do - SiteSetting.enable_forwarded_emails = true + SiteSetting.forwarded_emails_behaviour = "create_replies" end context "when a reply contains a forwareded email" do