Merge branch '2364-fallback-to-in-reply-to-header' into 'master'

Fall back to In-Reply-To and References headers when sub-addressing is not available

_Originally opened at !3024 by @dabit._

- - -

Fixes #2364 

Summary of the changes:
- No more need to have the `%{key}` placeholder in the `incoming_email.address`
- The fallback message id format is `reply-[key]@[gitlab_host]` (reminder: it doesn't have to be a real email address)
- The fallback message id that includes the reply key is added to both `References` header
- Documentation for the "Reply by email" feature updated

See merge request !3305

CHANGELOG

@@ -74,6 +74,7 @@ v 8.6.0
   - Fix wiki search results point to raw source (Hiroyuki Sato)
   - Don't load all of GitLab in mail_room
   - Add information about `image` and `services` field at `job` level in the `.gitlab-ci.yml` documentation (Pat Turner)
+  - Fall back to `In-Reply-To` and `References` headers when sub-addressing is not available (David Padilla)
   - HTTP error pages work independently from location and config (Artem Sidorenko)
   - Update `omniauth-saml` to 1.5.0 to allow for custom response attributes to be set
   - Memoize @group in Admin::GroupsController (Yatish Mehta)

app/mailers/notify.rb

@@ -110,6 +110,10 @@ class Notify < BaseMailer
 
       headers['Reply-To'] = address
 
+      fallback_reply_message_id = "<reply-#{reply_key}@#{Gitlab.config.gitlab.host}>".freeze
+      headers['References'] ||= ''
+      headers['References'] << ' ' << fallback_reply_message_id
+
       @reply_by_email = true
     end
 

config/gitlab.yml.example

@@ -106,7 +106,7 @@ production: &base
     enabled: false
 
     # The email address including the `%{key}` placeholder that will be replaced to reference the item being replied to.
-    # The `%{key}` placeholder is added after the user part, after a `+` character, before the `@`.
+    # The placeholder can be omitted but if present, it must appear in the "user" part of the address (before the `@`).
     address: "gitlab-incoming+%{key}@gmail.com"
 
     # Email account username

config/mail_room.yml

@@ -17,7 +17,7 @@ if File.exists?(config_file)
   config['start_tls']  = false    if config['start_tls'].nil?
   config['mailbox']    = "inbox"  if config['mailbox'].nil?
 
-  if config['enabled'] && config['address'] && config['address'].include?('%{key}')
+  if config['enabled'] && config['address']
     redis_url = Gitlab::RedisConfig.new(rails_env).url
     %>
     -

doc/incoming_email/README.md

 # Reply by email
 
-GitLab can be set up to allow users to comment on issues and merge requests by replying to notification emails.
+GitLab can be set up to allow users to comment on issues and merge requests by
+replying to notification emails.
 
-## Get a mailbox
+## Requirement
 
-Reply by email requires an IMAP-enabled email account, with a provider or server that supports [email sub-addressing](https://en.wikipedia.org/wiki/Email_address#Sub-addressing). Sub-addressing is a feature where any email to `user+some_arbitrary_tag@example.com` will end up in the mailbox for `user@example.com`, and is supported by providers such as Gmail, Google Apps, Yahoo! Mail, Outlook.com and iCloud, as well as the Postfix mail server which you can run on-premises.
+Reply by email requires an IMAP-enabled email account. GitLab allows you to use
+three strategies for this feature:
+- using email sub-addressing
+- using a dedicated email address
+- using a catch-all mailbox
 
-If you want to use Gmail / Google Apps with Reply by email, make sure you have [IMAP access enabled](https://support.google.com/mail/troubleshooter/1668960?hl=en#ts=1665018) and [allow less secure apps to access the account](https://support.google.com/accounts/answer/6010255).
+### Email sub-addressing
 
-To set up a basic Postfix mail server with IMAP access on Ubuntu, follow [these instructions](./postfix.md).
+**If your provider or server supports email sub-addressing, we recommend using it.**
+
+[Sub-addressing](https://en.wikipedia.org/wiki/Email_address#Sub-addressing) is
+a feature where any email to `user+some_arbitrary_tag@example.com` will end up
+in the mailbox for `user@example.com`, and is supported by providers such as
+Gmail, Google Apps, Yahoo! Mail, Outlook.com and iCloud, as well as the Postfix
+mail server which you can run on-premises.
+
+### Dedicated email address
+
+This solution is really simple to set up: you just have to create an email
+address dedicated to receive your users' replies to GitLab notifications.
+
+### Catch-all mailbox
+
+A [catch-all mailbox](https://en.wikipedia.org/wiki/Catch-all) for a domain will
+"catch all" the emails addressed to the domain that do not exist in the mail
+server.
+
+## How it works?
+
+### 1. GitLab sends a notification email
+
+When GitLab sends a notification and Reply by email is enabled, the `Reply-To`
+header is set to the address defined in your GitLab configuration, with the
+`%{key}` placeholder (if present) replaced by a specific "reply key". In
+addition, this "reply key" is also added to the `References` header.
+
+### 2. You reply to the notification email
+
+When you reply to the notification email, your email client will:
+
+- send the email to the `Reply-To` address it got from the notification email
+- set the `In-Reply-To` header to the value of the `Message-ID` header from the
+  notification email
+- set the `References` header to the value of the `Message-ID` plus the value of
+  the notification email's `References` header.
+
+### 3. GitLab receives your reply to the notification email
+
+When GitLab receives your reply, it will look for the "reply key" in the
+following headers, in this order:
+
+1. the `To` header
+1. the `References` header
+
+If it finds a reply key, it will be able to leave your reply as a comment on
+the entity the notification was about (issue, merge request, commit...).
+
+For more details about the `Message-ID`, `In-Reply-To`, and `References headers`,
+please consult [RFC 5322](https://tools.ietf.org/html/rfc5322#section-3.6.4).
 
 ## Set it up
 
+If you want to use Gmail / Google Apps with Reply by email, make sure you have
+[IMAP access enabled](https://support.google.com/mail/troubleshooter/1668960?hl=en#ts=1665018)
+and [allowed less secure apps to access the account](https://support.google.com/accounts/answer/6010255).
+
+To set up a basic Postfix mail server with IMAP access on Ubuntu, follow
+[these instructions](./postfix.md).
+
 ### Omnibus package installations
 
-1. Find the `incoming_email` section in `/etc/gitlab/gitlab.rb`, enable the feature and fill in the details for your specific IMAP server and email account:
+1. Find the `incoming_email` section in `/etc/gitlab/gitlab.rb`, enable the
+  feature and fill in the details for your specific IMAP server and email account:
 
     ```ruby
     # Configuration for Postfix mail server, assumes mailbox incoming@gitlab.example.com
     gitlab_rails['incoming_email_enabled'] = true
-    
-    # The email address including a placeholder for the key that references the item being replied to.
-    # The `%{key}` placeholder is added after the user part, before the `@`.
+
+    # The email address including the `%{key}` placeholder that will be replaced to reference the item being replied to.
+    # The placeholder can be omitted but if present, it must appear in the "user" part of the address (before the `@`).
     gitlab_rails['incoming_email_address'] = "incoming+%{key}@gitlab.example.com"
-    
+
     # Email account username
     # With third party providers, this is usually the full email address.
     # With self-hosted email servers, this is usually the user part of the email address.
     gitlab_rails['incoming_email_email'] = "incoming"
     # Email account password
     gitlab_rails['incoming_email_password'] = "[REDACTED]"
-    
+
     # IMAP server host
     gitlab_rails['incoming_email_host'] = "gitlab.example.com"
     # IMAP server port
@@ -47,18 +110,18 @@ To set up a basic Postfix mail server with IMAP access on Ubuntu, follow [these
     ```ruby
     # Configuration for Gmail / Google Apps, assumes mailbox gitlab-incoming@gmail.com
     gitlab_rails['incoming_email_enabled'] = true
-    
+
     # The email address including the `%{key}` placeholder that will be replaced to reference the item being replied to.
-    # The `%{key}` placeholder is added after the user part, after a `+` character, before the `@`.
+    # The placeholder can be omitted but if present, it must appear in the "user" part of the address (before the `@`).
     gitlab_rails['incoming_email_address'] = "gitlab-incoming+%{key}@gmail.com"
-    
+
     # Email account username
     # With third party providers, this is usually the full email address.
     # With self-hosted email servers, this is usually the user part of the email address.
     gitlab_rails['incoming_email_email'] = "gitlab-incoming@gmail.com"
     # Email account password
     gitlab_rails['incoming_email_password'] = "[REDACTED]"
-    
+
     # IMAP server host
     gitlab_rails['incoming_email_host'] = "imap.gmail.com"
     # IMAP server port
@@ -72,8 +135,6 @@ To set up a basic Postfix mail server with IMAP access on Ubuntu, follow [these
     gitlab_rails['incoming_email_mailbox_name'] = "inbox"
     ```
 
-    As mentioned, the part after `+` in the address is ignored, and any email sent here will end up in the mailbox for `incoming@gitlab.example.com`/`gitlab-incoming@gmail.com`.
-
 1. Reconfigure GitLab and restart mailroom for the changes to take effect:
 
     ```sh
@@ -97,7 +158,8 @@ To set up a basic Postfix mail server with IMAP access on Ubuntu, follow [these
     cd /home/git/gitlab
     ```
 
-1. Find the `incoming_email` section in `config/gitlab.yml`, enable the feature and fill in the details for your specific IMAP server and email account:
+1. Find the `incoming_email` section in `config/gitlab.yml`, enable the feature
+  and fill in the details for your specific IMAP server and email account:
 
     ```sh
     sudo editor config/gitlab.yml
@@ -109,7 +171,7 @@ To set up a basic Postfix mail server with IMAP access on Ubuntu, follow [these
       enabled: true
 
       # The email address including the `%{key}` placeholder that will be replaced to reference the item being replied to.
-      # The `%{key}` placeholder is added after the user part, after a `+` character, before the `@`.
+      # The placeholder can be omitted but if present, it must appear in the "user" part of the address (before the `@`).
       address: "incoming+%{key}@gitlab.example.com"
 
       # Email account username
@@ -138,7 +200,7 @@ To set up a basic Postfix mail server with IMAP access on Ubuntu, follow [these
       enabled: true
 
       # The email address including the `%{key}` placeholder that will be replaced to reference the item being replied to.
-      # The `%{key}` placeholder is added after the user part, after a `+` character, before the `@`.
+      # The placeholder can be omitted but if present, it must appear in the "user" part of the address (before the `@`).
       address: "gitlab-incoming+%{key}@gmail.com"
 
       # Email account username
@@ -161,8 +223,6 @@ To set up a basic Postfix mail server with IMAP access on Ubuntu, follow [these
       mailbox: "inbox"
     ```
 
-    As mentioned, the part after `+` in the address is ignored, and any email sent here will end up in the mailbox for `incoming@gitlab.example.com`/`gitlab-incoming@gmail.com`.
-
 1. Enable `mail_room` in the init script at `/etc/default/gitlab`:
 
     ```sh
@@ -195,8 +255,8 @@ To set up a basic Postfix mail server with IMAP access on Ubuntu, follow [these
     incoming_email:
       enabled: true
 
-      # The email address including a placeholder for the key that references the item being replied to.
-      # The `%{key}` placeholder is added after the user part, before the `@`.
+      # The email address including the `%{key}` placeholder that will be replaced to reference the item being replied to.
+      # The placeholder can be omitted but if present, it must appear in the "user" part of the address (before the `@`).
       address: "gitlab-incoming+%{key}@gmail.com"
 
       # Email account username

lib/gitlab/email/receiver.rb

@@ -63,6 +63,10 @@ module Gitlab
       end
 
       def reply_key
+        key_from_to_header || key_from_additional_headers
+      end
+
+      def key_from_to_header
         key = nil
         message.to.each do |address|
           key = Gitlab::IncomingEmail.key_from_address(address)
@@ -72,6 +76,17 @@ module Gitlab
         key
       end
 
+      def key_from_additional_headers
+        reply_key = nil
+
+        Array(message.references).each do |message_id|
+          reply_key = Gitlab::IncomingEmail.key_from_fallback_reply_message_id(message_id)
+          break if reply_key
+        end
+
+        reply_key
+      end
+
       def sent_notification
         return nil unless reply_key
 

lib/gitlab/incoming_email.rb

 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

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? ... "
 

spec/fixtures/emails/reply_without_subaddressing_and_key_inside_references.eml

+Return-Path: <jake@adventuretime.ooo>
+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: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for <reply@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
+Date: Thu, 13 Jun 2013 17:03:48 -0400
+From: Jake the Dog <jake@adventuretime.ooo>
+To: reply@appmail.adventuretime.ooo
+Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com>
+In-Reply-To: <issue_1@localhost>
+References: <issue_1@localhost> <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost>
+Subject: re: [Discourse Meta] eviltrout posted in 'Adventure Time Sux'
+Mime-Version: 1.0
+Content-Type: text/plain;
+ charset=ISO-8859-1
+Content-Transfer-Encoding: 7bit
+X-Sieve: CMU Sieve 2.2
+X-Received: by 10.0.0.1 with SMTP id n7mr11234144ipb.85.1371157428600; Thu,
+ 13 Jun 2013 14:03:48 -0700 (PDT)
+X-Scanned-By: MIMEDefang 2.69 on IPv6:2001:470:1d:165::1
+
+I could not disagree more. I am obviously biased but adventure time is the
+greatest show ever created. Everyone should watch it.
+
+- Jake out
+
+
+On Sun, Jun 9, 2013 at 1:39 PM, eviltrout via Discourse Meta
+<reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo> wrote:
+>
+>
+>
+> eviltrout posted in 'Adventure Time Sux' on Discourse Meta:
+>
+> ---
+> hey guys everyone knows adventure time sucks!
+>
+> ---
+> Please visit this link to respond: http://localhost:3000/t/adventure-time-sux/1234/3
+>
+> To unsubscribe from these emails, visit your [user preferences](http://localhost:3000/user_preferences).
+>

spec/fixtures/emails/valid_reply.eml

@@ -7,6 +7,8 @@ Date: Thu, 13 Jun 2013 17:03:48 -0400
 From: Jake the Dog <jake@adventuretime.ooo>
 To: reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo
 Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com>
+In-Reply-To: <issue_1@localhost>
+References: <issue_1@localhost> <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost>
 Subject: re: [Discourse Meta] eviltrout posted in 'Adventure Time Sux'
 Mime-Version: 1.0
 Content-Type: text/plain;
@@ -37,4 +39,4 @@ On Sun, Jun 9, 2013 at 1:39 PM, eviltrout via Discourse Meta
 > Please visit this link to respond: http://localhost:3000/t/adventure-time-sux/1234/3
 >
 > To unsubscribe from these emails, visit your [user preferences](http://localhost:3000/user_preferences).
->
\ No newline at end of file
+>

spec/lib/gitlab/email/receiver_spec.rb

@@ -3,6 +3,7 @@ require "spec_helper"
 describe Gitlab::Email::Receiver, lib: true do
   before do
     stub_incoming_email_setting(enabled: true, address: "reply+%{key}@appmail.adventuretime.ooo")
+    stub_config_setting(host: 'localhost')
   end
 
   let(:reply_key) { "59d8df8370b7e95c5a49fbf86aeb2c93" }
@@ -137,5 +138,27 @@ describe Gitlab::Email::Receiver, lib: true do
 
       expect(note.note).to include(markdown)
     end
+
+    context 'when sub-addressing is not supported' do
+      before do
+        stub_incoming_email_setting(enabled: true, address: nil)
+      end
+
+      shared_examples 'an email that contains a reply key' do |header|
+        it "fetches the reply key from the #{header} header and creates a comment" do
+          expect { receiver.execute }.to change { noteable.notes.count }.by(1)
+          note = noteable.notes.last
+
+          expect(note.author).to eq(sent_notification.recipient)
+          expect(note.note).to include('I could not disagree more.')
+        end
+      end
+
+      context 'reply key is in the References header' do
+        let(:email_raw) { fixture_file('emails/reply_without_subaddressing_and_key_inside_references.eml') }
+
+        it_behaves_like 'an email that contains a reply key', 'References'
+      end
+    end
   end
 end

spec/lib/gitlab/incoming_email_spec.rb

@@ -7,24 +7,8 @@ describe Gitlab::IncomingEmail, lib: true do
         stub_incoming_email_setting(enabled: true)
       end
 
-      context "when the address is valid" do
-        before do
-          stub_incoming_email_setting(address: "replies+%{key}@example.com")
-        end
-
-        it "returns true" do
-          expect(described_class.enabled?).to be_truthy
-        end
-      end
-
-      context "when the address is invalid" do
-        before do
-          stub_incoming_email_setting(address: "replies@example.com")
-        end
-
-        it "returns false" do
-          expect(described_class.enabled?).to be_falsey
-        end
+      it 'returns true' do
+        expect(described_class.enabled?).to be_truthy
       end
     end
 
@@ -58,4 +42,10 @@ describe Gitlab::IncomingEmail, lib: true do
       expect(described_class.key_from_address("replies+key@example.com")).to eq("key")
     end
   end
+
+  context 'self.key_from_fallback_reply_message_id' do
+    it 'returns reply key' do
+      expect(described_class.key_from_fallback_reply_message_id('reply-key@localhost')).to eq('key')
+    end
+  end
 end

spec/mailers/notify_spec.rb

@@ -35,7 +35,9 @@ describe Notify do
           subject { Notify.new_issue_email(issue.assignee_id, issue.id) }
 
           it_behaves_like 'an assignee email'
-          it_behaves_like 'an email starting a new thread', 'issue'
+          it_behaves_like 'an email starting a new thread with reply-by-email enabled' do
+            let(:model) { issue }
+          end
           it_behaves_like 'it should show Gmail Actions View Issue link'
           it_behaves_like 'an unsubscribeable thread'
 
@@ -73,9 +75,11 @@ describe Notify do
           subject { Notify.reassigned_issue_email(recipient.id, issue.id, previous_assignee.id, current_user.id) }
 
           it_behaves_like 'a multiple recipients email'
-          it_behaves_like 'an answer to an existing thread', 'issue'
+          it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do
+            let(:model) { issue }
+          end
           it_behaves_like 'it should show Gmail Actions View Issue link'
-          it_behaves_like "an unsubscribeable thread"
+          it_behaves_like 'an unsubscribeable thread'
 
           it 'is sent as the author' do
             sender = subject.header[:from].addrs[0]
@@ -104,7 +108,9 @@ describe Notify do
           subject { Notify.relabeled_issue_email(recipient.id, issue.id, %w[foo bar baz], current_user.id) }
 
           it_behaves_like 'a multiple recipients email'
-          it_behaves_like 'an answer to an existing thread', 'issue'
+          it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do
+            let(:model) { issue }
+          end
           it_behaves_like 'it should show Gmail Actions View Issue link'
           it_behaves_like 'a user cannot unsubscribe through footer link'
           it_behaves_like 'an email with a labels subscriptions link in its footer'
@@ -132,7 +138,9 @@ describe Notify do
           let(:status) { 'closed' }
           subject { Notify.issue_status_changed_email(recipient.id, issue.id, status, current_user.id) }
 
-          it_behaves_like 'an answer to an existing thread', 'issue'
+          it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do
+            let(:model) { issue }
+          end
           it_behaves_like 'it should show Gmail Actions View Issue link'
           it_behaves_like 'an unsubscribeable thread'
 
@@ -163,7 +171,9 @@ describe Notify do
           let(:new_issue) { create(:issue) }
           subject { Notify.issue_moved_email(recipient, issue, new_issue, current_user) }
 
-          it_behaves_like 'an answer to an existing thread', 'issue'
+          it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do
+            let(:model) { issue }
+          end
           it_behaves_like 'it should show Gmail Actions View Issue link'
           it_behaves_like 'an unsubscribeable thread'
 
@@ -196,9 +206,11 @@ describe Notify do
           subject { Notify.new_merge_request_email(merge_request.assignee_id, merge_request.id) }
 
           it_behaves_like 'an assignee email'
-          it_behaves_like 'an email starting a new thread', 'merge_request'
+          it_behaves_like 'an email starting a new thread with reply-by-email enabled' do
+            let(:model) { merge_request }
+          end
           it_behaves_like 'it should show Gmail Actions View Merge request link'
-          it_behaves_like "an unsubscribeable thread"
+          it_behaves_like 'an unsubscribeable thread'
 
           it 'has the correct subject' do
             is_expected.to have_subject /#{merge_request.title} \(##{merge_request.iid}\)/
@@ -216,10 +228,6 @@ describe Notify do
             is_expected.to have_body_text /#{merge_request.target_branch}/
           end
 
-          it 'has the correct message-id set' do
-            is_expected.to have_header 'Message-ID', "<merge_request_#{merge_request.id}@#{Gitlab.config.gitlab.host}>"
-          end
-
           context 'when enabled email_author_in_body' do
             before do
               allow(current_application_settings).to receive(:email_author_in_body).and_return(true)
@@ -247,7 +255,9 @@ describe Notify do
           subject { Notify.reassigned_merge_request_email(recipient.id, merge_request.id, previous_assignee.id, current_user.id) }
 
           it_behaves_like 'a multiple recipients email'
-          it_behaves_like 'an answer to an existing thread', 'merge_request'
+          it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do
+            let(:model) { merge_request }
+          end
           it_behaves_like 'it should show Gmail Actions View Merge request link'
           it_behaves_like "an unsubscribeable thread"
 
@@ -278,7 +288,9 @@ describe Notify do
           subject { Notify.relabeled_merge_request_email(recipient.id, merge_request.id, %w[foo bar baz], current_user.id) }
 
           it_behaves_like 'a multiple recipients email'
-          it_behaves_like 'an answer to an existing thread', 'merge_request'
+          it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do
+            let(:model) { merge_request }
+          end
           it_behaves_like 'it should show Gmail Actions View Merge request link'
           it_behaves_like 'a user cannot unsubscribe through footer link'
           it_behaves_like 'an email with a labels subscriptions link in its footer'
@@ -306,9 +318,11 @@ describe Notify do
           let(:status) { 'reopened' }
           subject { Notify.merge_request_status_email(recipient.id, merge_request.id, status, current_user.id) }
 
-          it_behaves_like 'an answer to an existing thread', 'merge_request'
+          it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do
+            let(:model) { merge_request }
+          end
           it_behaves_like 'it should show Gmail Actions View Merge request link'
-          it_behaves_like "an unsubscribeable thread"
+          it_behaves_like 'an unsubscribeable thread'
 
           it 'is sent as the author' do
             sender = subject.header[:from].addrs[0]
@@ -337,9 +351,11 @@ describe Notify do
           subject { Notify.merged_merge_request_email(recipient.id, merge_request.id, merge_author.id) }
 
           it_behaves_like 'a multiple recipients email'
-          it_behaves_like 'an answer to an existing thread', 'merge_request'
+          it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do
+            let(:model) { merge_request }
+          end
           it_behaves_like 'it should show Gmail Actions View Merge request link'
-          it_behaves_like "an unsubscribeable thread"
+          it_behaves_like 'an unsubscribeable thread'
 
           it 'is sent as the merge author' do
             sender = subject.header[:from].addrs[0]
@@ -456,9 +472,11 @@ describe Notify do
         subject { Notify.note_commit_email(recipient.id, note.id) }
 
         it_behaves_like 'a note email'
-        it_behaves_like 'an answer to an existing thread', 'commit'
+        it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do
+          let(:model) { commit }
+        end
         it_behaves_like 'it should show Gmail Actions View Commit link'
-        it_behaves_like "a user cannot unsubscribe through footer link"
+        it_behaves_like 'a user cannot unsubscribe through footer link'
 
         it 'has the correct subject' do
           is_expected.to have_subject /#{commit.title} \(#{commit.short_id}\)/
@@ -477,7 +495,9 @@ describe Notify do
         subject { Notify.note_merge_request_email(recipient.id, note.id) }
 
         it_behaves_like 'a note email'
-        it_behaves_like 'an answer to an existing thread', 'merge_request'
+        it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do
+          let(:model) { merge_request }
+        end
         it_behaves_like 'it should show Gmail Actions View Merge request link'
         it_behaves_like 'an unsubscribeable thread'
 
@@ -498,7 +518,9 @@ describe Notify do
         subject { Notify.note_issue_email(recipient.id, note.id) }
 
         it_behaves_like 'a note email'
-        it_behaves_like 'an answer to an existing thread', 'issue'
+        it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do
+          let(:model) { issue }
+        end
         it_behaves_like 'it should show Gmail Actions View Issue link'
         it_behaves_like 'an unsubscribeable thread'
 

spec/mailers/shared/notify.rb

@@ -10,6 +10,13 @@ shared_context 'gitlab email notification' do
     ActionMailer::Base.deliveries.clear
     email = recipient.emails.create(email: "notifications@example.com")
     recipient.update_attribute(:notification_email, email.email)
+    stub_incoming_email_setting(enabled: true, address: "reply+%{key}@#{Gitlab.config.gitlab.host}")
+  end
+end
+
+shared_context 'reply-by-email is enabled with incoming address without %{key}' do
+  before do
+    stub_incoming_email_setting(enabled: true, address: "reply@#{Gitlab.config.gitlab.host}")
   end
 end
 
@@ -46,25 +53,76 @@ shared_examples 'an email with X-GitLab headers containing project details' do
   end
 end
 
-shared_examples 'an email starting a new thread' do |message_id_prefix|
-  include_examples 'an email with X-GitLab headers containing project details'
+shared_examples 'a new thread email with reply-by-email enabled' do
+  let(:regex) { /\A<reply\-(.*)@#{Gitlab.config.gitlab.host}>\Z/ }
+
+  it 'has a Message-ID header' do
+    is_expected.to have_header 'Message-ID', "<#{model.class.model_name.singular_route_key}_#{model.id}@#{Gitlab.config.gitlab.host}>"
+  end
 
-  it 'has a discussion identifier' do
-    is_expected.to have_header 'Message-ID',  /<#{message_id_prefix}(.*)@#{Gitlab.config.gitlab.host}>/
+  it 'has a References header' do
+    is_expected.to have_header 'References', regex
   end
 end
 
-shared_examples 'an answer to an existing thread' do |thread_id_prefix|
+shared_examples 'a thread answer email with reply-by-email enabled' do
   include_examples 'an email with X-GitLab headers containing project details'
+  let(:regex) { /\A<#{model.class.model_name.singular_route_key}_#{model.id}@#{Gitlab.config.gitlab.host}> <reply\-(.*)@#{Gitlab.config.gitlab.host}>\Z/ }
+
+  it 'has a Message-ID header' do
+    is_expected.to have_header 'Message-ID', /\A<(.*)@#{Gitlab.config.gitlab.host}>\Z/
+  end
+
+  it 'has a In-Reply-To header' do
+    is_expected.to have_header 'In-Reply-To', "<#{model.class.model_name.singular_route_key}_#{model.id}@#{Gitlab.config.gitlab.host}>"
+  end
+
+  it 'has a References header' do
+    is_expected.to have_header 'References', regex
+  end
 
   it 'has a subject that begins with Re: ' do
     is_expected.to have_subject /^Re: /
   end
+end
+
+shared_examples 'an email starting a new thread with reply-by-email enabled' do
+  include_examples 'an email with X-GitLab headers containing project details'
+  include_examples 'a new thread email with reply-by-email enabled'
+
+  context 'when reply-by-email is enabled with incoming address with %{key}' do
+    it 'has a Reply-To header' do
+      is_expected.to have_header 'Reply-To', /<reply+(.*)@#{Gitlab.config.gitlab.host}>\Z/
+    end
+  end
+
+  context 'when reply-by-email is enabled with incoming address without %{key}' do
+    include_context 'reply-by-email is enabled with incoming address without %{key}'
+    include_examples 'a new thread email with reply-by-email enabled'
+
+    it 'has a Reply-To header' do
+      is_expected.to have_header 'Reply-To', /<reply@#{Gitlab.config.gitlab.host}>\Z/
+    end
+  end
+end
+
+shared_examples 'an answer to an existing thread with reply-by-email enabled' do
+  include_examples 'an email with X-GitLab headers containing project details'
+  include_examples 'a thread answer email with reply-by-email enabled'
+
+  context 'when reply-by-email is enabled with incoming address with %{key}' do
+    it 'has a Reply-To header' do
+      is_expected.to have_header 'Reply-To', /<reply+(.*)@#{Gitlab.config.gitlab.host}>\Z/
+    end
+  end
+
+  context 'when reply-by-email is enabled with incoming address without %{key}' do
+    include_context 'reply-by-email is enabled with incoming address without %{key}'
+    include_examples 'a thread answer email with reply-by-email enabled'
 
-  it 'has headers that reference an existing thread' do
-    is_expected.to have_header 'Message-ID',  /<(.*)@#{Gitlab.config.gitlab.host}>/
-    is_expected.to have_header 'References',  /<#{thread_id_prefix}(.*)@#{Gitlab.config.gitlab.host}>/
-    is_expected.to have_header 'In-Reply-To', /<#{thread_id_prefix}(.*)@#{Gitlab.config.gitlab.host}>/
+    it 'has a Reply-To header' do
+      is_expected.to have_header 'Reply-To', /<reply@#{Gitlab.config.gitlab.host}>\Z/
+    end
   end
 end