From f63da1c146dbcd6c94e0df52e442b7699b4e6508 Mon Sep 17 00:00:00 2001
From: Martin Brennan <mjrbrennan@gmail.com>
Date: Thu, 8 Oct 2020 13:52:17 +1000
Subject: [PATCH] FIX: Confirm new email not sent for staff if email disabled
 with "non-staff" option (#10794)

See https://meta.discourse.org/t/email-address-change-confirmation-email-not-sent-but-every-other-notification-emails-are/165358

In short: with disable emails set to non-staff, email address change confirmation emails (those sent to the new address) are not sent for staff or admin members.

This was happening because we were looking up the staff user with the to_address of the email, but the to address was the new email address because we are sending a confirm email change email, and thus the user could not be found. We didn't need to do this anyway because we are passing the user into the Email::Sender class anyway.
---
 lib/email/sender.rb                  | 17 +++++++++++++++--
 spec/components/email/sender_spec.rb | 14 ++++++++++++++
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/lib/email/sender.rb b/lib/email/sender.rb
index 027d85893c7..0668d7ea5ae 100644
--- a/lib/email/sender.rb
+++ b/lib/email/sender.rb
@@ -12,7 +12,15 @@ require 'uri'
 require 'net/smtp'
 
 SMTP_CLIENT_ERRORS = [Net::SMTPFatalError, Net::SMTPSyntaxError]
-BYPASS_DISABLE_TYPES = ["admin_login", "test_message"]
+BYPASS_DISABLE_TYPES = %w(
+  admin_login
+  test_message
+  new_version
+  group_smtp
+  invite
+  download_backup_message
+  admin_confirmation_message
+)
 
 module Email
   class Sender
@@ -37,7 +45,7 @@ module Email
       return skip(SkippedEmailLog.reason_types[:sender_message_to_blank]) if @message.to.blank?
 
       if SiteSetting.disable_emails == "non-staff" && !bypass_disable
-        return unless User.find_by_email(to_address)&.staff?
+        return unless find_user&.staff?
       end
 
       return skip(SkippedEmailLog.reason_types[:sender_message_to_invalid]) if to_address.end_with?(".invalid")
@@ -232,6 +240,11 @@ module Email
       email_log
     end
 
+    def find_user
+      return @user if @user
+      User.find_by_email(to_address)
+    end
+
     def to_address
       @to_address ||= begin
         to = @message.try(:to)
diff --git a/spec/components/email/sender_spec.rb b/spec/components/email/sender_spec.rb
index d3c7741c4f5..6732bc1eacf 100644
--- a/spec/components/email/sender_spec.rb
+++ b/spec/components/email/sender_spec.rb
@@ -52,6 +52,20 @@ describe Email::Sender do
         message = Mail::Message.new(to: moderator.email, body: "hello")
         Email::Sender.new(message, :hello).send
       end
+
+      it "delivers mail to staff user when confirming new email if user is provided" do
+        Mail::Message.any_instance.expects(:deliver_now).once
+        Fabricate(:email_change_request, {
+          user: moderator,
+          new_email: "newemail@testmoderator.com",
+          old_email: moderator.email,
+          change_state: EmailChangeRequest.states[:authorizing_new]
+        })
+        message = Mail::Message.new(
+          to: "newemail@testmoderator.com", body: "hello"
+        )
+        Email::Sender.new(message, :confirm_new_email, moderator).send
+      end
     end
   end