diff --git a/lib/mail/network/delivery_methods/smtp.rb b/lib/mail/network/delivery_methods/smtp.rb index 7287e4764..7ecdd19e6 100644 --- a/lib/mail/network/delivery_methods/smtp.rb +++ b/lib/mail/network/delivery_methods/smtp.rb @@ -3,18 +3,18 @@ module Mail # == Sending Email with SMTP - # + # # Mail allows you to send emails using SMTP. This is done by wrapping Net::SMTP in # an easy to use manner. - # + # # === Sending via SMTP server on Localhost - # + # # Sending locally (to a postfix or sendmail server running on localhost) requires # no special setup. Just to Mail.deliver &block or message.deliver! and it will # be sent in this method. - # + # # === Sending via MobileMe - # + # # Mail.defaults do # delivery_method :smtp, { :address => "smtp.me.com", # :port => 587, @@ -22,11 +22,11 @@ module Mail # :user_name => '', # :password => '', # :authentication => 'plain', - # :enable_starttls_auto => true } + # :enable_starttls => :auto } # end - # + # # === Sending via GMail - # + # # Mail.defaults do # delivery_method :smtp, { :address => "smtp.gmail.com", # :port => 587, @@ -34,9 +34,17 @@ module Mail # :user_name => '', # :password => '', # :authentication => 'plain', - # :enable_starttls_auto => true } + # :enable_starttls => :auto } # end # + # === Configuring TLS/SSL and STARTTLS + # + # A few remarks: + # - when enabling `tls` (or `ssl`), setting (truthy values for) either `enable_starttls` or `enable_starttls_auto` will raise an ArgumentError as TLS and STARTTLS are mutually exclusive. + # - to configure STARTTLS, use the `enable_starttls`-flag (instead of a combination of `enable_starttls` and `enable_starttls_auto`). Acceptable values are `:always`, `:auto` and `false`. + # - when providing a truthy value for `enable_starttls`, the `enable_starttls_auto`-flag will be ignored. + # - when none of `tls`, `ssl`, `enable_starttls` or `enable_starttls_auto` is set, the fallback will be `enable_starttls` `:auto`. + # # === Certificate verification # # When using TLS, some mail servers provide certificates that are self-signed @@ -48,30 +56,30 @@ module Mail # verify mode constant (OpenSSL::SSL::VERIFY_NONE, OpenSSL::SSL::VERIFY_PEER), # or a string containing the name of an OpenSSL verify mode (none, peer). # - # === Others - # + # === Others + # # Feel free to send me other examples that were tricky - # + # # === Delivering the email - # + # # Once you have the settings right, sending the email is done by: - # + # # Mail.deliver do # to 'mikel@test.lindsaar.net' # from 'ada@test.lindsaar.net' # subject 'testing sendmail' # body 'testing sendmail' # end - # + # # Or by calling deliver on a Mail message - # + # # mail = Mail.new do # to 'mikel@test.lindsaar.net' # from 'ada@test.lindsaar.net' # subject 'testing sendmail' # body 'testing sendmail' # end - # + # # mail.deliver! class SMTP attr_accessor :settings @@ -84,7 +92,7 @@ class SMTP :password => nil, :authentication => nil, :enable_starttls => nil, - :enable_starttls_auto => true, + :enable_starttls_auto => nil, :openssl_verify_mode => nil, :ssl => nil, :tls => nil, @@ -105,39 +113,66 @@ def deliver!(mail) end private + # `k` is said to be provided when `settings` has a non-nil value for `k`. + def setting_provided?(k) + !settings[k].nil? + end + + # Yields one of `:always`, `:auto` or `false` based on `enable_starttls` and `enable_starttls_auto` flags. + # Yields `false` when `smtp_tls?`. + # Else defaults to `:auto` when neither `enable_starttls*` flag is provided. + # Providing a truthy value for `enable_starttls` will ignore `enable_starttls_auto`. + def smtp_starttls + return false if smtp_tls? + + if setting_provided?(:enable_starttls) && settings[:enable_starttls] + # enable_starttls: provided and truthy + case settings[:enable_starttls] + when :auto then :auto + when :always then :always + else + :always + end + else + # enable_starttls: not provided or false + if setting_provided?(:enable_starttls_auto) + settings[:enable_starttls_auto] ? :auto : false + else + # enable_starttls_auto: not provided + # enable_starttls: when provided then false + # use :auto when neither enable_starttls* provided + setting_provided?(:enable_starttls) ? false : :auto + end + end + end + + def smtp_tls? + (setting_provided?(:tls) && settings[:tls]) || (setting_provided?(:ssl) && settings[:ssl]) + end + def start_smtp_session(&block) build_smtp_session.start(settings[:domain], settings[:user_name], settings[:password], settings[:authentication], &block) end def build_smtp_session + if smtp_tls? && (settings[:enable_starttls] || settings[:enable_starttls_auto]) + raise ArgumentError, ":enable_starttls and :tls are mutually exclusive. Set :tls if you're on an SMTPS connection. Set :enable_starttls if you're on an SMTP connection and using STARTTLS for secure TLS upgrade." + end + Net::SMTP.new(settings[:address], settings[:port]).tap do |smtp| - tls = settings[:tls] || settings[:ssl] - if !tls.nil? - case tls - when true - smtp.enable_tls(ssl_context) - when false - smtp.disable_tls - else - raise ArgumentError, "Unrecognized :tls value #{settings[:tls].inspect}; expected true, false, or nil" - end - elsif settings.include?(:enable_starttls) && !settings[:enable_starttls].nil? - case settings[:enable_starttls] - when true + if smtp_tls? + smtp.disable_starttls + smtp.enable_tls(ssl_context) + else + smtp.disable_tls + + case smtp_starttls + when :always smtp.enable_starttls(ssl_context) - when false - smtp.disable_starttls - else - raise ArgumentError, "Unrecognized :enable_starttls value #{settings[:enable_starttls].inspect}; expected true, false, or nil" - end - elsif settings.include?(:enable_starttls_auto) && !settings[:enable_starttls_auto].nil? - case settings[:enable_starttls_auto] - when true + when :auto smtp.enable_starttls_auto(ssl_context) - when false - smtp.disable_starttls else - raise ArgumentError, "Unrecognized :enable_starttls_auto value #{settings[:enable_starttls_auto].inspect}; expected true, false, or nil" + smtp.disable_starttls end end diff --git a/spec/mail/network/delivery_methods/smtp_spec.rb b/spec/mail/network/delivery_methods/smtp_spec.rb index f7550fa70..89262777b 100644 --- a/spec/mail/network/delivery_methods/smtp_spec.rb +++ b/spec/mail/network/delivery_methods/smtp_spec.rb @@ -2,7 +2,7 @@ # frozen_string_literal: true require 'spec_helper' -describe "SMTP Delivery Method" do +RSpec.describe "SMTP Delivery Method" do before(:each) do MockSMTP.reset @@ -202,14 +202,7 @@ def redefine_verify_none(new_value) to 'ada@test.lindsaar.net' subject 'Re: No way!' body 'Yeah sure' - delivery_method :smtp, { :address => "localhost", - :port => 25, - :domain => 'localhost.localdomain', - :user_name => nil, - :password => nil, - :authentication => nil, - :enable_starttls => true } - + delivery_method :smtp, { :enable_starttls => true } end message.deliver! @@ -217,41 +210,29 @@ def redefine_verify_none(new_value) expect(MockSMTP.starttls).to eq :always end - it 'should allow disabling automatic STARTTLS' do + it 'should allow forcing STARTTLS via enable_starttls: :always (overriding :enable_starttls_auto)' do message = Mail.new do from 'mikel@test.lindsaar.net' to 'ada@test.lindsaar.net' subject 'Re: No way!' body 'Yeah sure' - delivery_method :smtp, { :address => "localhost", - :port => 25, - :domain => 'localhost.localdomain', - :user_name => nil, - :password => nil, - :authentication => nil, - :enable_starttls => false } - + delivery_method :smtp, { :enable_starttls => :always, + :enable_starttls_auto => true } end message.deliver! - expect(MockSMTP.starttls).to eq false + expect(MockSMTP.starttls).to eq :always end - it 'should allow forcing STARTTLS auto' do + it 'should allow detecting STARTTLS via enable_starttls: :auto (overriding :enable_starttls_auto)' do message = Mail.new do from 'mikel@test.lindsaar.net' to 'ada@test.lindsaar.net' subject 'Re: No way!' body 'Yeah sure' - delivery_method :smtp, { :address => "localhost", - :port => 25, - :domain => 'localhost.localdomain', - :user_name => nil, - :password => nil, - :authentication => nil, - :enable_starttls_auto => true } - + delivery_method :smtp, { :enable_starttls => :auto, + :enable_starttls_auto => false } end message.deliver! @@ -259,26 +240,33 @@ def redefine_verify_none(new_value) expect(MockSMTP.starttls).to eq :auto end - it 'should allow disabling automatic STARTTLS auto' do + it 'should allow disabling automatic STARTTLS' do message = Mail.new do from 'mikel@test.lindsaar.net' to 'ada@test.lindsaar.net' subject 'Re: No way!' body 'Yeah sure' - delivery_method :smtp, { :address => "localhost", - :port => 25, - :domain => 'localhost.localdomain', - :user_name => nil, - :password => nil, - :authentication => nil, - :enable_starttls_auto => false } - + delivery_method :smtp, { :enable_starttls => false } end message.deliver! expect(MockSMTP.starttls).to eq false end + + it 'raises when setting STARTTLS with tls' do + message = Mail.new do + from 'mikel@test.lindsaar.net' + to 'ada@test.lindsaar.net' + subject 'Re: No way!' + body 'Yeah sure' + delivery_method :smtp, { :tls => true, :enable_starttls => :always } + end + + expect { + message.deliver! + }.to raise_error(ArgumentError, /:enable_starttls and :tls are mutually exclusive/) + end end describe "SMTP Envelope" do @@ -325,7 +313,7 @@ def redefine_verify_none(new_value) end.to raise_error(ArgumentError, 'SMTP From address may not be blank: nil') end - it "should raise an error if no recipient if defined" do + it "should raise an error if no recipient is defined" do mail = Mail.new do from "from@somemail.com" subject "Email with no recipient"