diff --git a/app/mailers/group_smtp_mailer.rb b/app/mailers/group_smtp_mailer.rb index 708e46043f5..7b6cc26b44d 100644 --- a/app/mailers/group_smtp_mailer.rb +++ b/app/mailers/group_smtp_mailer.rb @@ -15,7 +15,9 @@ class GroupSmtpMailer < ActionMailer::Base domain: from_group.email_username_domain, user_name: from_group.email_username, password: from_group.email_password, - authentication: GlobalSetting.smtp_authentication, + # NOTE: Might be better at some point to store this authentication method in the database + # against the group. + authentication: SmtpProviderOverrides.authentication_override(from_group.smtp_server), enable_starttls_auto: from_group.smtp_ssl, return_response: true, open_timeout: GlobalSetting.group_smtp_open_timeout, diff --git a/app/services/email_settings_validator.rb b/app/services/email_settings_validator.rb index c921b155af9..f7227ce2a9c 100644 --- a/app/services/email_settings_validator.rb +++ b/app/services/email_settings_validator.rb @@ -84,15 +84,15 @@ class EmailSettingsValidator ) begin port, enable_tls, enable_starttls_auto = - provider_specific_ssl_overrides(host, port, enable_tls, enable_starttls_auto) + SmtpProviderOverrides.ssl_override(host, port, enable_tls, enable_starttls_auto) if enable_tls && enable_starttls_auto raise ArgumentError, "TLS and STARTTLS are mutually exclusive" end if username || password - authentication = provider_specific_authentication_overrides(host) if authentication.nil? - authentication = (authentication || GlobalSetting.smtp_authentication)&.to_sym + authentication = SmtpProviderOverrides.authentication_override(host) if authentication.nil? + authentication = authentication.to_sym if !%i[plain login cram_md5].include?(authentication) raise ArgumentError, "Invalid authentication method. Must be plain, login, or cram_md5." end @@ -180,29 +180,4 @@ class EmailSettingsValidator end 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) - # 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 %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 - elsif port.to_i == 465 - enable_starttls_auto = false - enable_tls = true - end - end - - [port, enable_tls, enable_starttls_auto] - end end diff --git a/lib/smtp_provider_overrides.rb b/lib/smtp_provider_overrides.rb new file mode 100644 index 00000000000..0263ba4e06f --- /dev/null +++ b/lib/smtp_provider_overrides.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +class SmtpProviderOverrides + # 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.authentication_override(host) + return "login" if %w[smtp.office365.com smtp-mail.outlook.com].include?(host) + GlobalSetting.smtp_authentication + end + + def self.ssl_override(host, port, enable_tls, enable_starttls_auto) + # 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 %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 + elsif port.to_i == 465 + enable_starttls_auto = false + enable_tls = true + end + end + + [port, enable_tls, enable_starttls_auto] + end +end diff --git a/spec/mailers/group_smtp_mailer_spec.rb b/spec/mailers/group_smtp_mailer_spec.rb index 09bbf72489c..5d5f1de2656 100644 --- a/spec/mailers/group_smtp_mailer_spec.rb +++ b/spec/mailers/group_smtp_mailer_spec.rb @@ -135,6 +135,18 @@ RSpec.describe GroupSmtpMailer do ) end + it "uses the login SMTP authentication method for office365" do + group.update!(smtp_server: "smtp.office365.com") + mail = GroupSmtpMailer.send_mail(group, user.email, Fabricate(:post)) + expect(mail.delivery_method.settings[:authentication]).to eq("login") + end + + it "uses the login SMTP authentication method for outlook" do + group.update!(smtp_server: "smtp-mail.outlook.com") + mail = GroupSmtpMailer.send_mail(group, user.email, Fabricate(:post)) + expect(mail.delivery_method.settings[:authentication]).to eq("login") + end + context "when the site has a reply by email address configured" do before do SiteSetting.manual_polling_enabled = true