FEATURE: add indication if incoming email attachment was rejected and inform sender about it (#6376)

* FEATURE: add indication if incoming email attachment was rejected and inform sender about it

* include errors for rejected attachments in email

* don't send warning email to staged users

* use user object instead of user_id in add_attachments method

config/locales/server.en.yml

@@ -88,6 +88,7 @@ en:
       maximum_staged_user_per_email_reached: "Reached maximum number of staged users created per email."
       no_subject: "(no subject)"
       no_body: "(no body)"
+      missing_attachment: '(Attachment %{filename} is missing)'
       errors:
         empty_email_error: "Happens when the raw mail we received was blank."
         no_message_id_error: "Happens when the mail has no 'Message-Id' header."
@@ -2609,6 +2610,17 @@ en:
 
         There was an unrecognized error while processing your email and it wasn't posted. You should try again, or [contact a staff member](%{base_url}/about).
 
+    email_reject_attachment:
+      title: "Email Attachment Rejected"
+      subject_template: "[%{email_prefix}] Email issue -- Attachment Rejected"
+      text_body_template:  |
+        Unfortunately some attachments in your email message to %{destination} (titled %{former_title}) were rejected.
+
+        Details:
+        %{rejected_errors}
+
+        If you believe this is an error, [contact a staff member](%{base_url}/about).
+
     email_error_notification:
       title: "Email Error Notification"
       subject_template: "[%{email_prefix}] Email issue -- POP authentication error"

lib/email/receiver.rb

@@ -878,12 +878,13 @@ module Email
 
     def create_post_with_attachments(options = {})
       # deal with attachments
-      options[:raw] = add_attachments(options[:raw], options[:user].id, options)
+      options[:raw] = add_attachments(options[:raw], options[:user], options)
 
       create_post(options)
     end
 
-    def add_attachments(raw, user_id, options = {})
+    def add_attachments(raw, user, options = {})
+      rejected_attachments = []
       attachments.each do |attachment|
         tmp = Tempfile.new(["discourse-email-attachment", File.extname(attachment.filename)])
         begin
@@ -891,7 +892,7 @@ module Email
           File.open(tmp.path, "w+b") { |f| f.write attachment.body.decoded }
           # create the upload for the user
           opts = { for_group_message: options[:is_group_message] }
-          upload = UploadCreator.new(tmp, attachment.filename, opts).create_for(user_id)
+          upload = UploadCreator.new(tmp, attachment.filename, opts).create_for(user.id)
           if upload&.valid?
             # try to inline images
             if attachment.content_type&.start_with?("image/")
@@ -905,15 +906,39 @@ module Email
             else
               raw << "\n\n#{attachment_markdown(upload)}\n\n"
             end
+          else
+            rejected_attachments << upload
+            raw << "\n\n#{I18n.t('emails.incoming.missing_attachment', filename: upload.original_filename)}\n\n"
           end
         ensure
           tmp&.close!
         end
       end
+      notify_about_rejected_attachment(rejected_attachments) if rejected_attachments.present? && !user.staged?
 
       raw
     end
 
+    def notify_about_rejected_attachment(attachments)
+      errors = []
+
+      attachments.each do |a|
+        error = a.errors.messages.values[0][0]
+        errors << "#{a.original_filename}: #{error}"
+      end
+
+      message = Mail::Message.new(@mail)
+      template_args = {
+        former_title: message.subject,
+        destination: message.to,
+        site_name: SiteSetting.title,
+        rejected_errors: errors.join("\n")
+      }
+
+      client_message = RejectionMailer.send_rejection(:email_reject_attachment, message.from, template_args)
+      Email::Sender.new(client_message, :email_reject_attachment).send
+    end
+
     def attachment_markdown(upload)
       if FileHelper.is_supported_image?(upload.original_filename)
         "<img src='#{upload.url}' width='#{upload.width}' height='#{upload.height}'>"

spec/components/email/receiver_spec.rb

@@ -465,10 +465,24 @@ describe Email::Receiver do
       SiteSetting.authorized_extensions = "txt"
       expect { process(:attached_txt_file) }.to change { topic.posts.count }
       expect(topic.posts.last.raw).to match(/text\.txt/)
+    end
 
-      SiteSetting.authorized_extensions = "csv"
+    context "when attachment is rejected" do
-      expect { process(:attached_txt_file_2) }.to change { topic.posts.count }
+      it "sends out the warning email" do
-      expect(topic.posts.last.raw).to_not match(/text\.txt/)
+        expect { process(:attached_txt_file) }.to change { EmailLog.count }.by(1)
+        expect(EmailLog.last.email_type).to eq("email_reject_attachment")
+      end
+
+      it "doesn't send out the warning email if sender is staged user" do
+        user.update_columns(staged: true)
+        expect { process(:attached_txt_file) }.not_to change { EmailLog.count }
+      end
+
+      it "creates the post with attachment missing message" do
+        missing_attachment_regex = Regexp.escape(I18n.t('emails.incoming.missing_attachment', filename: "text.txt"))
+        expect { process(:attached_txt_file) }.to change { topic.posts.count }
+        expect(topic.posts.last.raw).to match(/#{missing_attachment_regex}/)
+      end
     end
 
     it "supports emails with just an attachment" do