Improve and finish the fallback to the In-Reply-To and References header for the reply-by-email feature

A few things to note:
- The IncomingEmail feature is now enabled even without a
  correctly-formatted sub-address
- Message-ID for new thread mail are kept the same so that subsequent
  notifications to this thread are grouped in the thread by the email
  service that receives the notification
  (i.e. In-Reply-To of the answer == Message-ID of the first thread message)
- To maximize our chance to be able to retrieve the reply key, we look
  for it in the In-Reply-To header and the References header
- The pattern for the fallback reply message id is "reply-[key]@[gitlab_host]"
- Improve docs thanks to Axil

3 files changed, 15 insertions, 26 deletions

diff --git a/lib/gitlab/email/receiver.rb b/lib/gitlab/email/receiver.rb
index d55bacde5b0..97ef9851d71 100644
--- a/lib/gitlab/email/receiver.rb
+++ b/lib/gitlab/email/receiver.rb
@@ -63,10 +63,10 @@ module Gitlab
       end
 
       def reply_key
-        key_from_to_address || key_from_in_reply_to_header
+        key_from_to_header || key_from_additional_headers
       end
 
-      def key_from_to_address
+      def key_from_to_header
         key = nil
         message.to.each do |address|
           key = Gitlab::IncomingEmail.key_from_address(address)
@@ -76,11 +76,11 @@ module Gitlab
         key
       end
 
-      def key_from_in_reply_to_header
+      def key_from_additional_headers
         reply_key = nil
 
-        message[:in_reply_to].message_ids.each do |message_id|
-          reply_key = Gitlab::IncomingEmail.key_from_address(message_id)
+        Array(message.references).each do |message_id|
+          reply_key = Gitlab::IncomingEmail.key_from_fallback_reply_message_id(message_id)
           break if reply_key
         end
 
diff --git a/lib/gitlab/incoming_email.rb b/lib/gitlab/incoming_email.rb
index 9068d79c95e..8ce9d32abe0 100644
--- a/lib/gitlab/incoming_email.rb
+++ b/lib/gitlab/incoming_email.rb
@@ -1,13 +1,10 @@
 module Gitlab
   module IncomingEmail
     class << self
-      def enabled?
-        config.enabled && address_formatted_correctly?
-      end
+      FALLBACK_REPLY_MESSAGE_ID_REGEX = /\Areply\-(.+)@#{Gitlab.config.gitlab.host}\Z/.freeze
 
-      def address_formatted_correctly?
-        config.address &&
-          config.address.include?("%{key}")
+      def enabled?
+        config.enabled && config.address
       end
 
       def reply_address(key)
@@ -24,6 +21,13 @@ module Gitlab
         match[1]
       end
 
+      def key_from_fallback_reply_message_id(message_id)
+        match = message_id.match(FALLBACK_REPLY_MESSAGE_ID_REGEX)
+        return unless match
+
+        match[1]
+      end
+
       def config
         Gitlab.config.incoming_email
       end
diff --git a/lib/tasks/gitlab/check.rake b/lib/tasks/gitlab/check.rake
index 27ed57efe55..effb8eb6001 100644
--- a/lib/tasks/gitlab/check.rake
+++ b/lib/tasks/gitlab/check.rake
@@ -623,7 +623,6 @@ namespace :gitlab do
       start_checking "Reply by email"
 
       if Gitlab.config.incoming_email.enabled
-        check_address_formatted_correctly
         check_imap_authentication
 
         if Rails.env.production?
@@ -643,20 +642,6 @@ namespace :gitlab do
     # Checks
     ########################
 
-    def check_address_formatted_correctly
-      print "Address formatted correctly? ... "
-
-      if Gitlab::IncomingEmail.address_formatted_correctly?
-        puts "yes".green
-      else
-        puts "no".red
-        try_fixing_it(
-          "Make sure that the address in config/gitlab.yml includes the '%{key}' placeholder."
-        )
-        fix_and_rerun
-      end
-    end
-
     def check_initd_configured_correctly
       print "Init.d configured correctly? ... "