From f6b62919d247ea9b4b24796408e4014cf1218fa3 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 df3d519c139..414a5a41e31 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")
@@ -221,6 +229,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 26d4dcffbe9..b0f675814c9 100644
--- a/spec/components/email/sender_spec.rb
+++ b/spec/components/email/sender_spec.rb
@@ -49,6 +49,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