diff --git a/CHANGELOG b/CHANGELOG index 1698fd8f5b70d8410de9134adaae17db7eadc06f..25eeb24b49965614591b46f02da2f57b40bb83b1 100644 --- a/CHANGELOG +++ b/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) diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index 8cbc9eefc7b73e6dea9b4dd1160272959d2d38e2..826e5f96fa1ed5dc0ccba6a6a541ba002699229f 100644 --- a/app/mailers/notify.rb +++ b/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 diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 500b745f55e9a83127d7cee20153a38405ca19c9..fb1c3476f65b4383aac8c332fef81d35f24e212c 100644 --- a/config/gitlab.yml.example +++ b/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 diff --git a/config/mail_room.yml b/config/mail_room.yml index aed55f74eab5e5acb1e19cc5e1489dfc559b1644..60257329f3ebf7b2128ffdb64c3fb0f36cfd53ff 100644 --- a/config/mail_room.yml +++ b/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 %> - diff --git a/doc/incoming_email/README.md b/doc/incoming_email/README.md index 4cfb840294391e837107d489ec24f68a0acd9d64..5a9a158287741814e3ec1450290e2a5b23110570 100644 --- a/doc/incoming_email/README.md +++ b/doc/incoming_email/README.md @@ -1,36 +1,99 @@ # 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 diff --git a/lib/gitlab/email/receiver.rb b/lib/gitlab/email/receiver.rb index d4b6f6d120db2bc78173296fcbc4aa51a380c48d..97ef9851d71c7240a0f3197815c2d28dad1f3a9d 100644 --- a/lib/gitlab/email/receiver.rb +++ b/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 diff --git a/lib/gitlab/incoming_email.rb b/lib/gitlab/incoming_email.rb index 9068d79c95e29a0ee1011209ca0434fb3e2516c0..8ce9d32abe043b615e957ee20ddc7a40a6248ff8 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 27ed57efe555c69011f63425b1935acfd84728b9..effb8eb60012b3c619da2f37c3e51d0fc4f6d60e 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? ... " diff --git a/spec/fixtures/emails/reply_without_subaddressing_and_key_inside_references.eml b/spec/fixtures/emails/reply_without_subaddressing_and_key_inside_references.eml new file mode 100644 index 0000000000000000000000000000000000000000..39d5cefbc2ae8b0206eb45e7d0133c943ee9c4a0 --- /dev/null +++ b/spec/fixtures/emails/reply_without_subaddressing_and_key_inside_references.eml @@ -0,0 +1,42 @@ +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). +> diff --git a/spec/fixtures/emails/valid_reply.eml b/spec/fixtures/emails/valid_reply.eml index 1e696389954ca0b2ca63fe4a0f4dfd5853f5e61a..980e10a8812512db980ae6de78038153566497f4 100644 --- a/spec/fixtures/emails/valid_reply.eml +++ b/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 +> diff --git a/spec/lib/gitlab/email/receiver_spec.rb b/spec/lib/gitlab/email/receiver_spec.rb index abe179cd4af160d27f5bff11e688ba62d03ce11b..36267faeb93b66eb8966a9b75dec7aa6149a41a7 100644 --- a/spec/lib/gitlab/email/receiver_spec.rb +++ b/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 diff --git a/spec/lib/gitlab/incoming_email_spec.rb b/spec/lib/gitlab/incoming_email_spec.rb index bcdba8d4c1220d1d0d0aa25f6def9a2f3020ca30..afb3e26f8fbbf15db3556a8fc41febebf3df970d 100644 --- a/spec/lib/gitlab/incoming_email_spec.rb +++ b/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 diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 9b47acfe0cd1027f5023a8fc63a1cfb7496188b2..631b5094f4206a4c39a761bc7863aee9fe87efb2 100644 --- a/spec/mailers/notify_spec.rb +++ b/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' diff --git a/spec/mailers/shared/notify.rb b/spec/mailers/shared/notify.rb index 6019af544d3f79bf7187d28794e995a5ef19ce47..56a6dbf96f99bfa56e330c36acb006ffc7068a18 100644 --- a/spec/mailers/shared/notify.rb +++ b/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