enqueue spam/dmarc failing emails instead of hiding (#8674)

* enqueue spam/dmarc failing emails instead of hiding

* add translations for dmarc/spam enqueued reasons

* unescape quote

* if email_in_authserv_id is blank return gray for all emails

config/locales/server.en.yml

@@ -2406,6 +2406,7 @@ en:
       fixed_primary_email: "Fixed primary email for staged user"
       same_ip_address: "Same IP address (%{ip_address}) as other users"
       inactive_user: "Inactive user"
+    email_in_spam_header: "User's first email was flagged as spam"
 
   reviewables_reminder:
     submitted:
@@ -4661,6 +4662,8 @@ en:
       category: "Posts in this category require manual approval by staff. See the category settings."
       must_approve_users: "All new users must be approved by staff. See `must_approve_users`."
       invite_only: "All new users should be invited. See `invite_only`."
+      email_auth_res_enqueue: "This email failed a DMARC check, it most likely isn't from whom it seems to be from. Check the raw email headers for more information."
+      email_spam: "This email was flagged as spam by the header defined in `email_in_spam_header`."
 
     actions:
       agree:

lib/email/authentication_results.rb

@@ -2,8 +2,6 @@
 
 module Email
   class AuthenticationResults
-    attr_reader :results
-
     VERDICT = Enum.new(
       :gray,
       :pass,
@@ -12,11 +10,16 @@ module Email
     )
 
     def initialize(headers)
-      authserv_id = SiteSetting.email_in_authserv_id
-      @results = Array(headers).map do |header|
+      @authserv_id = SiteSetting.email_in_authserv_id
+      @headers = headers
+      @verdict = :gray if @authserv_id.blank?
+    end
+
+    def results
+      @results ||= Array(@headers).map do |header|
         parse_header(header.to_s)
       end.filter do |result|
-        authserv_id.blank? || authserv_id == result[:authserv_id]
+        @authserv_id.blank? || @authserv_id == result[:authserv_id]
       end
     end
 
@@ -32,7 +35,7 @@ module Email
 
     def calc_action
       if verdict == :fail
-        :hide
+        :enqueue
       else
         :accept
       end
@@ -44,7 +47,7 @@ module Email
 
     def calc_dmarc
       verdict = VERDICT[:gray]
-      @results.each do |result|
+      results.each do |result|
         result[:resinfo].each do |resinfo|
           if resinfo[:method] == "dmarc"
             v = VERDICT[resinfo[:result].to_sym].to_i

lib/email/receiver.rb

@@ -160,7 +160,6 @@ module Email
         create_reply(user: user,
                      raw: body,
                      elided: elided,
-                     hidden_reason_id: hidden_reason_id,
                      post: post,
                      topic: post.topic,
                      skip_validations: user.staged?,
@@ -170,7 +169,7 @@ module Email
 
         destinations.each do |destination|
           begin
-            return process_destination(destination, user, body, elided, hidden_reason_id)
+            return process_destination(destination, user, body, elided)
           rescue => e
             first_exception ||= e
           end
@@ -195,17 +194,6 @@ module Email
       end
     end
 
-    def hidden_reason_id
-      @hidden_reason_id ||=
-        if is_spam?
-          Post.hidden_reasons[:email_spam_header_found]
-        elsif !sent_to_mailinglist_mirror? && auth_res_action == :hide
-          Post.hidden_reasons[:email_authentication_result_header]
-        else
-          nil
-        end
-    end
-
     def log_and_validate_user(user)
       @incoming_email.update_columns(user_id: user.id)
 
@@ -242,7 +230,6 @@ module Email
           create_reply(user: @from_user,
                        raw: body,
                        elided: elided,
-                       hidden_reason_id: hidden_reason_id,
                        post: post,
                        topic: topic,
                        skip_validations: true,
@@ -667,7 +654,7 @@ module Email
       nil
     end
 
-    def process_destination(destination, user, body, elided, hidden_reason_id)
+    def process_destination(destination, user, body, elided)
       return if SiteSetting.forwarded_emails_behaviour != "hide" &&
                 has_been_forwarded? &&
                 process_forwarded_email(destination, user)
@@ -679,7 +666,7 @@ module Email
         user ||= stage_from_user
 
         group = destination[:obj]
-        create_group_post(group, user, body, elided, hidden_reason_id)
+        create_group_post(group, user, body, elided)
 
       when :category
         category = destination[:obj]
@@ -693,7 +680,6 @@ module Email
         create_topic(user: user,
                      raw: body,
                      elided: elided,
-                     hidden_reason_id: hidden_reason_id,
                      title: subject,
                      category: category.id,
                      skip_validations: user.staged?)
@@ -713,7 +699,6 @@ module Email
         create_reply(user: user,
                      raw: body,
                      elided: elided,
-                     hidden_reason_id: hidden_reason_id,
                      post: post,
                      topic: post&.topic,
                      skip_validations: user.staged?,
@@ -721,7 +706,7 @@ module Email
       end
     end
 
-    def create_group_post(group, user, body, elided, hidden_reason_id)
+    def create_group_post(group, user, body, elided)
       message_ids = Email::Receiver.extract_reply_message_ids(@mail, max_message_id_count: 5)
       post_ids = []
 
@@ -739,7 +724,6 @@ module Email
         create_reply(user: user,
                      raw: body,
                      elided: elided,
-                     hidden_reason_id: hidden_reason_id,
                      post: post,
                      topic: post.topic,
                      skip_validations: true)
@@ -749,7 +733,6 @@ module Email
         create_topic(user: user,
                      raw: body,
                      elided: elided,
-                     hidden_reason_id: hidden_reason_id,
                      title: subject,
                      archetype: Archetype.private_message,
                      target_group_names: [group.name],
@@ -1135,6 +1118,10 @@ module Email
       if sent_to_mailinglist_mirror?
         options[:skip_validations] = true
         options[:skip_guardian] = true
+      else
+        options[:email_spam] = is_spam?
+        options[:first_post_checks] = true if is_spam?
+        options[:email_auth_res_action] = auth_res_action
       end
 
       user = options.delete(:user)

lib/new_post_manager.rb

@@ -80,8 +80,12 @@ class NewPostManager
   def self.post_needs_approval?(manager)
     user = manager.user
 
+    return :email_auth_res_enqueue if manager.args[:email_auth_res_action] == :enqueue
+
     return :skip if exempt_user?(user)
 
+    return :email_spam if manager.args[:email_spam]
+
     return :post_count if (
       user.trust_level <= TrustLevel.levels[:basic] &&
       user.post_count < SiteSetting.approve_post_count
@@ -157,6 +161,8 @@ class NewPostManager
       UserSilencer.silence(manager.user, Discourse.system_user, keep_posts: true, reason: I18n.t("user.new_user_typed_too_fast"))
     elsif matches_auto_silence_regex?(manager)
       UserSilencer.silence(manager.user, Discourse.system_user, keep_posts: true, reason: I18n.t("user.content_matches_auto_silence_regex"))
+    elsif reason == :email_spam && is_first_post?(manager)
+      UserSilencer.silence(manager.user, Discourse.system_user, keep_posts: true, reason: I18n.t("user.email_in_spam_header"))
     end
 
     result

spec/components/email/authentication_results_spec.rb

@@ -239,7 +239,7 @@ describe Email::AuthenticationResults do
 
         context "with a fail" do
           let(:headers) { "foobar.com; dmarc=fail" }
-          include_examples "is verdict", :fail
+          include_examples "is verdict", :gray
         end
 
         context "with a pass" do
@@ -277,10 +277,10 @@ describe Email::AuthenticationResults do
   end
 
   describe "#action" do
-    it "hides a fail verdict" do
+    it "enqueues a fail verdict" do
       results = described_class.new("")
       results.expects(:verdict).returns(:fail)
-      expect(results.action).to eq (:hide)
+      expect(results.action).to eq (:enqueue)
     end
 
     it "accepts a pass verdict" do

spec/components/email/receiver_spec.rb

@@ -979,59 +979,33 @@ describe Email::Receiver do
     it "creates hidden topic for X-Spam-Flag" do
       SiteSetting.email_in_spam_header = 'X-Spam-Flag'
 
-      Fabricate(:user, email: "existing@bar.com", trust_level: SiteSetting.email_in_min_trust)
-      expect { process(:spam_x_spam_flag) }.to change { Topic.count }.by(1) # Topic created
-
-      topic = Topic.last
-      expect(topic.visible).to eq(false)
-
-      post = Post.last
-      expect(post.hidden).to eq(true)
-      expect(post.hidden_at).not_to eq(nil)
-      expect(post.hidden_reason_id).to eq(Post.hidden_reasons[:email_spam_header_found])
+      user = Fabricate(:user, email: "existing@bar.com", trust_level: SiteSetting.email_in_min_trust)
+      expect { process(:spam_x_spam_flag) }.to change { ReviewableQueuedPost.count }.by(1)
+      expect(user.reload.silenced?).to be(true)
     end
 
     it "creates hidden topic for X-Spam-Status" do
       SiteSetting.email_in_spam_header = 'X-Spam-Status'
 
-      Fabricate(:user, email: "existing@bar.com", trust_level: SiteSetting.email_in_min_trust)
-      expect { process(:spam_x_spam_status) }.to change { Topic.count }.by(1) # Topic created
-
-      topic = Topic.last
-      expect(topic.visible).to eq(false)
-
-      post = Post.last
-      expect(post.hidden).to eq(true)
-      expect(post.hidden_at).not_to eq(nil)
-      expect(post.hidden_reason_id).to eq(Post.hidden_reasons[:email_spam_header_found])
+      user = Fabricate(:user, email: "existing@bar.com", trust_level: SiteSetting.email_in_min_trust)
+      expect { process(:spam_x_spam_status) }.to change { ReviewableQueuedPost.count }.by(1)
+      expect(user.reload.silenced?).to be(true)
     end
 
     it "creates hidden topic for X-SES-Spam-Verdict" do
       SiteSetting.email_in_spam_header = 'X-SES-Spam-Verdict'
 
-      Fabricate(:user, email: "existing@bar.com", trust_level: SiteSetting.email_in_min_trust)
-      expect { process(:spam_x_ses_spam_verdict) }.to change { Topic.count }.by(1) # Topic created
-
-      topic = Topic.last
-      expect(topic.visible).to eq(false)
-
-      post = Post.last
-      expect(post.hidden).to eq(true)
-      expect(post.hidden_at).not_to eq(nil)
-      expect(post.hidden_reason_id).to eq(Post.hidden_reasons[:email_spam_header_found])
+      user = Fabricate(:user, email: "existing@bar.com", trust_level: SiteSetting.email_in_min_trust)
+      expect { process(:spam_x_ses_spam_verdict) }.to change { ReviewableQueuedPost.count }.by(1)
+      expect(user.reload.silenced?).to be(true)
     end
 
     it "creates hidden topic for failed Authentication-Results header" do
-      Fabricate(:user, email: "existing@bar.com", trust_level: SiteSetting.email_in_min_trust)
-      expect { process(:dmarc_fail) }.to change { Topic.count }.by(1) # Topic created
+      SiteSetting.email_in_authserv_id = 'example.com'
 
-      topic = Topic.last
-      expect(topic.visible).to eq(false)
-
-      post = Post.last
-      expect(post.hidden).to eq(true)
-      expect(post.hidden_at).not_to eq(nil)
-      expect(post.hidden_reason_id).to eq(Post.hidden_reasons[:email_authentication_result_header])
+      user = Fabricate(:user, email: "existing@bar.com", trust_level: SiteSetting.email_in_min_trust)
+      expect { process(:dmarc_fail) }.to change { ReviewableQueuedPost.count }.by(1)
+      expect(user.reload.silenced?).to be(false)
     end
 
     it "adds the 'elided' part of the original message when always_show_trimmed_content is enabled" do

spec/components/new_post_manager_spec.rb

@@ -437,4 +437,103 @@ describe NewPostManager do
     end
   end
 
+  context "via email with a spam failure" do
+    let(:user) { Fabricate(:user) }
+    let(:admin) { Fabricate(:admin) }
+
+    it "silences users if its their first post" do
+      manager = NewPostManager.new(
+        user,
+        raw: 'this is emailed content',
+        via_email: true,
+        raw_email: 'raw email contents',
+        email_spam: true,
+        first_post_checks: true
+      )
+
+      result = manager.perform
+      expect(result.action).to eq(:enqueued)
+      expect(user.silenced?).to be(true)
+    end
+
+    it "doesn't silence or enqueue exempt users" do
+      manager = NewPostManager.new(
+        admin,
+        raw: 'this is emailed content',
+        via_email: true,
+        raw_email: 'raw email contents',
+        email_spam: true,
+        first_post_checks: true
+      )
+
+      result = manager.perform
+      expect(result.action).to eq(:create_post)
+      expect(admin.silenced?).to be(false)
+    end
+  end
+
+  context "via email with an authentication results failure" do
+    let(:user) { Fabricate(:user) }
+    let(:admin) { Fabricate(:admin) }
+
+    it "doesn't silence users" do
+      manager = NewPostManager.new(
+        user,
+        raw: 'this is emailed content',
+        via_email: true,
+        raw_email: 'raw email contents',
+        email_auth_res_action: :enqueue,
+        first_post_checks: true
+      )
+
+      result = manager.perform
+      expect(result.action).to eq(:enqueued)
+      expect(user.silenced?).to be(false)
+    end
+
+    it "still enqueues exempt users" do
+      manager = NewPostManager.new(
+        admin,
+        raw: 'this is emailed content',
+        via_email: true,
+        raw_email: 'raw email contents',
+        email_auth_res_action: :enqueue
+      )
+
+      result = manager.perform
+      expect(result.action).to eq(:enqueued)
+      expect(user.silenced?).to be(false)
+    end
+  end
+
+  context "private message via email" do
+    it "doesn't enqueue authentiation results failure" do
+      manager = NewPostManager.new(
+        topic.user,
+        raw: 'this is emailed content',
+        archetype: Archetype.private_message,
+        via_email: true,
+        raw_email: 'raw email contents',
+        email_auth_res_action: :enqueue
+      )
+
+      result = manager.perform
+      expect(result.action).to eq(:create_post)
+    end
+
+    it "doesn't enqueue spam failure" do
+      manager = NewPostManager.new(
+        topic.user,
+        raw: 'this is emailed content',
+        archetype: Archetype.private_message,
+        via_email: true,
+        raw_email: 'raw email contents',
+        email_spam: true
+      )
+
+      result = manager.perform
+      expect(result.action).to eq(:create_post)
+    end
+  end
+
 end