mirror of
https://github.com/openfoodfoundation/openfoodnetwork
synced 2026-01-24 20:36:49 +00:00
Simplify SMTP auth method selection
Instead of using the auth method name, let's just not supply username and password when we don't want to authenticate. The two affected servers AU and CA don't have credentials set anyway. This is compatible. The specs needed changing though.
This commit is contained in:
@@ -20,18 +20,14 @@ module Spree
|
||||
private
|
||||
|
||||
def mail_server_settings
|
||||
settings = if need_authentication?
|
||||
basic_settings.merge(user_credentials)
|
||||
else
|
||||
basic_settings
|
||||
end
|
||||
settings = basic_settings.merge(user_credentials)
|
||||
|
||||
settings.merge(enable_starttls_auto: secure_connection?)
|
||||
end
|
||||
|
||||
def user_credentials
|
||||
{ user_name: Config.smtp_username,
|
||||
password: Config.smtp_password }
|
||||
{ user_name: Config.smtp_username.presence,
|
||||
password: Config.smtp_password.presence }
|
||||
end
|
||||
|
||||
def basic_settings
|
||||
@@ -42,13 +38,10 @@ module Spree
|
||||
end
|
||||
|
||||
def authentication
|
||||
return unless need_authentication?
|
||||
|
||||
Config.mail_auth_type.presence
|
||||
end
|
||||
|
||||
def need_authentication?
|
||||
Config.mail_auth_type != 'None'
|
||||
# "None" is an option in the UI but not a real authentication type.
|
||||
# We should remove it from our host configurations but I'm keeping
|
||||
# this check for backwards-compatibility for now.
|
||||
Config.mail_auth_type.presence unless Config.mail_auth_type == "None"
|
||||
end
|
||||
|
||||
def secure_connection?
|
||||
|
||||
@@ -8,12 +8,12 @@ module Spree
|
||||
let!(:subject) { MailSettings.new }
|
||||
|
||||
context "overrides appplication defaults" do
|
||||
context "authentication method is none" do
|
||||
context "authentication method is login" do
|
||||
before do
|
||||
Config.mail_host = "smtp.example.com"
|
||||
Config.mail_domain = "example.com"
|
||||
Config.mail_port = 123
|
||||
Config.mail_auth_type = MailSettings::SECURE_CONNECTION_TYPES[0]
|
||||
Config.mail_auth_type = "login"
|
||||
Config.smtp_username = "schof"
|
||||
Config.smtp_password = "hellospree!"
|
||||
Config.secure_connection_type = "TLS"
|
||||
@@ -23,31 +23,22 @@ module Spree
|
||||
it { expect(ActionMailer::Base.smtp_settings[:address]).to eq "smtp.example.com" }
|
||||
it { expect(ActionMailer::Base.smtp_settings[:domain]).to eq "example.com" }
|
||||
it { expect(ActionMailer::Base.smtp_settings[:port]).to eq 123 }
|
||||
it { expect(ActionMailer::Base.smtp_settings[:authentication]).to eq nil }
|
||||
it { expect(ActionMailer::Base.smtp_settings[:authentication]).to eq "login" }
|
||||
it { expect(ActionMailer::Base.smtp_settings[:enable_starttls_auto]).to be_truthy }
|
||||
|
||||
it "doesnt touch user name config" do
|
||||
expect(ActionMailer::Base.smtp_settings[:user_name]).to be_nil
|
||||
end
|
||||
|
||||
it "doesnt touch password config" do
|
||||
expect(ActionMailer::Base.smtp_settings[:password]).to be_nil
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context "when mail_auth_type is other than none" do
|
||||
before do
|
||||
Config.mail_auth_type = "login"
|
||||
Config.smtp_username = "schof"
|
||||
Config.smtp_password = "hellospree!"
|
||||
subject.override!
|
||||
end
|
||||
|
||||
context "overrides user credentials" do
|
||||
it { expect(ActionMailer::Base.smtp_settings[:user_name]).to eq "schof" }
|
||||
it { expect(ActionMailer::Base.smtp_settings[:password]).to eq "hellospree!" }
|
||||
end
|
||||
|
||||
context "authentication method is none" do
|
||||
before do
|
||||
Config.mail_auth_type = "None"
|
||||
subject.override!
|
||||
end
|
||||
|
||||
it "doesn't store 'None' as auth method" do
|
||||
expect(ActionMailer::Base.smtp_settings[:authentication]).to eq nil
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user