Set EmailsOnPush reply-to address to committer email when enabled.emailsonpush-replyto

4 files changed, 40 insertions, 12 deletions

diff --git a/CHANGELOG b/CHANGELOG
index f133cb24780..ab13f8903ea 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,7 @@
 Please view this file on the master branch, on stable branches it's out of date.
 
 v 7.10.0 (unreleased)
+  - Set EmailsOnPush reply-to address to committer email when enabled.
   - Fix broken file browsing with a submodule that contains a relative link (Stan Hu)
   - Fix persistent XSS vulnerability around profile website URLs.
   - Fix project import URL regex to prevent arbitary local repos from being imported.
diff --git a/app/mailers/emails/projects.rb b/app/mailers/emails/projects.rb
index 20a863c3742..d15123e7311 100644
--- a/app/mailers/emails/projects.rb
+++ b/app/mailers/emails/projects.rb
@@ -84,9 +84,17 @@ module Emails
 
       @disable_footer = true
 
-      mail(from: sender(author_id, send_from_committer_email),
-           to: recipient,
-           subject: @subject)
+      reply_to = 
+        if send_from_committer_email && can_send_from_user_email?(@author)
+          @author.email
+        else
+          Gitlab.config.gitlab.email_reply_to
+        end
+
+      mail(from:      sender(author_id, send_from_committer_email),
+           reply_to:  reply_to,
+           to:        recipient,
+           subject:   @subject)
     end
   end
 end
diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb
index 7c8b37029d1..2c0d451511f 100644
--- a/app/mailers/notify.rb
+++ b/app/mailers/notify.rb
@@ -60,20 +60,24 @@ class Notify < ActionMailer::Base
     address
   end
 
+  def can_send_from_user_email?(sender)
+    sender_domain = sender.email.split("@").last
+    self.class.allowed_email_domains.include?(sender_domain)
+  end
+
   # Return an email address that displays the name of the sender.
   # Only the displayed name changes; the actual email address is always the same.
   def sender(sender_id, send_from_user_email = false)
-    if sender = User.find(sender_id)
-      address = default_sender_address
-      address.display_name = sender.name
+    return unless sender = User.find(sender_id)
+    
+    address = default_sender_address
+    address.display_name = sender.name
 
-      sender_domain = sender.email.split("@").last
-      if send_from_user_email && self.class.allowed_email_domains.include?(sender_domain)
-        address.address = sender.email
-      end
-
-      address.format
+    if send_from_user_email && can_send_from_user_email?(sender)
+      address.address = sender.email
     end
+
+    address.format
   end
 
   # Look up a User by their ID and return their email address
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index 6bea0321cb9..b297fbd5119 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -725,6 +725,11 @@ describe Notify do
           sender = subject.header[:from].addrs[0]
           expect(sender.address).to eq(user.email)
         end
+
+        it "is set to reply to the committer email" do
+          sender = subject.header[:reply_to].addrs[0]
+          expect(sender.address).to eq(user.email)
+        end
       end
 
       context "when the committer email domain is not completely within the GitLab domain" do
@@ -738,6 +743,11 @@ describe Notify do
           sender = subject.header[:from].addrs[0]
           expect(sender.address).to eq(gitlab_sender)
         end
+
+        it "is set to reply to the default email" do
+          sender = subject.header[:reply_to].addrs[0]
+          expect(sender.address).to eq(gitlab_sender_reply_to)
+        end
       end
 
       context "when the committer email domain is outside the GitLab domain" do
@@ -751,6 +761,11 @@ describe Notify do
           sender = subject.header[:from].addrs[0]
           expect(sender.address).to eq(gitlab_sender)
         end
+
+        it "is set to reply to the default email" do
+          sender = subject.header[:reply_to].addrs[0]
+          expect(sender.address).to eq(gitlab_sender_reply_to)
+        end
       end
     end
   end