From 9f218fc184894d61c10f738c59bce97780f06e25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Thu, 17 Mar 2016 20:03:51 +0100 Subject: 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 --- CHANGELOG | 1 + app/mailers/notify.rb | 12 +-- config/gitlab.yml.example | 2 +- config/mail_room.yml | 2 +- doc/incoming_email/README.md | 108 ++++++++++++++++----- lib/gitlab/email/receiver.rb | 10 +- lib/gitlab/incoming_email.rb | 16 +-- lib/tasks/gitlab/check.rake | 15 --- spec/fixtures/emails/key_in_headers_reply.eml | 42 -------- ...out_subaddressing_and_key_inside_references.eml | 42 ++++++++ spec/fixtures/emails/valid_reply.eml | 4 +- spec/lib/gitlab/email/receiver_spec.rb | 26 +++-- spec/lib/gitlab/incoming_email_spec.rb | 26 ++--- spec/mailers/notify_spec.rb | 70 ++++++++----- spec/mailers/shared/notify.rb | 77 +++++++++++++-- 15 files changed, 288 insertions(+), 165 deletions(-) delete mode 100644 spec/fixtures/emails/key_in_headers_reply.eml create mode 100644 spec/fixtures/emails/reply_without_subaddressing_and_key_inside_references.eml diff --git a/CHANGELOG b/CHANGELOG index 5d9f4961ef5..a9c38c16d12 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -46,6 +46,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 e7331d88517..826e5f96fa1 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 = "".freeze + headers['References'] ||= '' + headers['References'] << ' ' << fallback_reply_message_id + @reply_by_email = true end @@ -121,17 +125,11 @@ class Notify < BaseMailer # # See: mail_answer_thread def mail_new_thread(model, headers = {}) - headers['Message-ID'] = message_reply_id - headers['In-Reply-To'] = message_id(model) - headers['References'] = message_id(model) + headers['Message-ID'] = message_id(model) mail_thread(model, headers) end - def message_reply_id - Gitlab.config.incoming_email["address"].gsub("%{key}", reply_key) - end - # Send an email that responds to an existing conversation thread, # with headers suitable for grouping by thread in email clients. # diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 500b745f55e..fb1c3476f65 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 aed55f74eab..60257329f3e 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 4cfb8402943..5a9a1582877 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 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? ... " diff --git a/spec/fixtures/emails/key_in_headers_reply.eml b/spec/fixtures/emails/key_in_headers_reply.eml deleted file mode 100644 index 8f288775d0a..00000000000 --- a/spec/fixtures/emails/key_in_headers_reply.eml +++ /dev/null @@ -1,42 +0,0 @@ -Return-Path: -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 ; Thu, 13 Jun 2013 17:03:50 -0400 -Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for ; Thu, 13 Jun 2013 14:03:48 -0700 -Received: by 10.0.0.1 with HTTP; Thu, 13 Jun 2013 14:03:48 -0700 -In-Reply-To: -References: -Date: Thu, 13 Jun 2013 17:03:48 -0400 -From: Jake the Dog -To: reply@appmail.adventuretime.ooo -Message-ID: -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 - 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/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 00000000000..39d5cefbc2a --- /dev/null +++ b/spec/fixtures/emails/reply_without_subaddressing_and_key_inside_references.eml @@ -0,0 +1,42 @@ +Return-Path: +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 ; Thu, 13 Jun 2013 17:03:50 -0400 +Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for ; 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 +To: reply@appmail.adventuretime.ooo +Message-ID: +In-Reply-To: +References: +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 + 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 e2b974174bd..980e10a8812 100644 --- a/spec/fixtures/emails/valid_reply.eml +++ b/spec/fixtures/emails/valid_reply.eml @@ -3,12 +3,12 @@ Received: from iceking.adventuretime.ooo ([unix socket]) by iceking (Cyrus v2.2. 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 ; Thu, 13 Jun 2013 17:03:50 -0400 Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for ; Thu, 13 Jun 2013 14:03:48 -0700 Received: by 10.0.0.1 with HTTP; Thu, 13 Jun 2013 14:03:48 -0700 -In-Reply-To: -References: Date: Thu, 13 Jun 2013 17:03:48 -0400 From: Jake the Dog To: reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo Message-ID: +In-Reply-To: +References: Subject: re: [Discourse Meta] eviltrout posted in 'Adventure Time Sux' Mime-Version: 1.0 Content-Type: text/plain; diff --git a/spec/lib/gitlab/email/receiver_spec.rb b/spec/lib/gitlab/email/receiver_spec.rb index 13fd83ec7ba..36267faeb93 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" } @@ -138,17 +139,26 @@ describe Gitlab::Email::Receiver, lib: true do expect(note.note).to include(markdown) end - context "when the reply key is in the In-Reply-To header" do - let(:email_raw) { fixture_file("emails/key_in_headers_reply.eml") } + context 'when sub-addressing is not supported' do + before do + stub_incoming_email_setting(enabled: true, address: nil) + end - it "creates a comment" do - expect { receiver.execute }.to change { noteable.notes.count }.by(1) - note = noteable.notes.last + 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.") + 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 bcdba8d4c12..afb3e26f8fb 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 0f3de33f361..631b5094f42 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,14 +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', // - end - - it 'has the correct references set' do - is_expected.to have_header 'References', "" - end - context 'when enabled email_author_in_body' do before do allow(current_application_settings).to receive(:email_author_in_body).and_return(true) @@ -251,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" @@ -282,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' @@ -310,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] @@ -341,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] @@ -460,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}\)/ @@ -481,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' @@ -502,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 eafc5e5044b..56a6dbf96f9 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,26 +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\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', // - is_expected.to have_header 'References', /<#{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}> \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', /\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', /\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', /\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', /\Z/ + end end end -- cgit v1.2.1