FIX: Office365/Outlook auth method for group SMTP (#27854)

Both office365 and outlook SMTP servers need LOGIN
SMTP authentication instead of PLAIN (which is what
we are using by default). This commit uses that
unconditionally for these servers, and also makes
sure to use STARTTLS for them too.

app/services/email_settings_validator.rb

@@ -91,6 +91,7 @@ class EmailSettingsValidator
       end
 
       if username || password
+        authentication = provider_specific_authentication_overrides(host) if authentication.nil?
         authentication = (authentication || GlobalSetting.smtp_authentication)&.to_sym
         if !%i[plain login cram_md5].include?(authentication)
           raise ArgumentError, "Invalid authentication method. Must be plain, login, or cram_md5."
@@ -129,7 +130,11 @@ class EmailSettingsValidator
       smtp.enable_tls(ssl_context) if enable_tls
 
       smtp.open_timeout = 5
-      smtp.read_timeout = 5
+
+      # Some SMTP servers have a higher delay to respond with errors
+      # as a tarpit measure that slows down clients who are sending "bad" commands.
+      # 10s is the minimum, we might need to increase this in the future.
+      smtp.read_timeout = 10
 
       smtp.start(domain, username, password, authentication)
       smtp.finish
@@ -176,10 +181,19 @@ class EmailSettingsValidator
     raise err
   end
 
+  # Ideally we (or net-smtp) would automatically detect the correct authentication
+  # method, but this is sufficient for our purposes because we know certain providers
+  # need certain authentication methods. This may need to change when we start to
+  # use XOAUTH2 for SMTP.
+  def self.provider_specific_authentication_overrides(host)
+    return :login if %w[smtp.office365.com smtp-mail.outlook.com].include?(host)
+    :plain
+  end
+
   def self.provider_specific_ssl_overrides(host, port, enable_tls, enable_starttls_auto)
-    # Gmail acts weirdly if you do not use the correct combinations of
+    # Certain mail servers act weirdly if you do not use the correct combinations of
     # TLS settings based on the port, we clean these up here for the user.
-    if host == "smtp.gmail.com"
+    if %w[smtp.gmail.com smtp.office365.com smtp-mail.outlook.com].include?(host)
       if port.to_i == 587
         enable_starttls_auto = true
         enable_tls = false

spec/services/email_settings_validator_spec.rb

@@ -275,6 +275,52 @@ RSpec.describe EmailSettingsValidator do
       }.to raise_error(ArgumentError)
     end
 
+    it "corrects tls settings for gmail based on port 587" do
+      net_smtp_stub.expects(:enable_starttls_auto).once
+      net_smtp_stub.expects(:enable_tls).never
+      described_class.validate_smtp(
+        host: host,
+        port: 587,
+        username: username,
+        password: password,
+        domain: domain,
+      )
+    end
+
+    it "corrects tls settings for gmail based on port 465" do
+      net_smtp_stub.expects(:enable_starttls_auto).never
+      net_smtp_stub.expects(:enable_tls).once
+      described_class.validate_smtp(
+        host: host,
+        port: 465,
+        username: username,
+        password: password,
+        domain: domain,
+      )
+    end
+
+    it "corrects authentication method to login for office365" do
+      net_smtp_stub.expects(:start).with("office365.com", username, password, :login)
+      described_class.validate_smtp(
+        host: "smtp.office365.com",
+        port: port,
+        username: username,
+        password: password,
+        domain: "office365.com",
+      )
+    end
+
+    it "corrects authentication method to login for outlook" do
+      net_smtp_stub.expects(:start).with("outlook.com", username, password, :login)
+      described_class.validate_smtp(
+        host: "smtp-mail.outlook.com",
+        port: port,
+        username: username,
+        password: password,
+        domain: "outlook.com",
+      )
+    end
+
     context "when the domain is not provided" do
       let(:domain) { nil }
       it "gets the domain from the host" do