From fff3ff11c0109e14586c924dc96cd35a8b907d64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Guitaut?= Date: Tue, 7 Jun 2022 16:58:04 +0200 Subject: [PATCH] FIX: Make disabling TLS in mail possible again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Following the Rails 7 upgrade, the `DISCOURSE_SMTP_ENABLE_START_TLS` setting doesn’t work anymore. This is because Rails upgraded the `net-smtp` gem to the 0.3.1 version which enables `starttls` by default. The `mail` gem doesn’t support this new behavior yet and doesn’t know how to disable TLS. This should be fixed in an upcoming release. Meanwhile applying this patch allows us to get back the previous behavior which is expected by many. --- Gemfile | 2 +- config/environments/production.rb | 2 +- lib/freedom_patches/mail_disable_starttls.rb | 21 +++++++++++++++++ .../mail_disable_starttls_spec.rb | 23 +++++++++++++++++++ 4 files changed, 46 insertions(+), 2 deletions(-) create mode 100644 lib/freedom_patches/mail_disable_starttls.rb create mode 100644 spec/lib/freedom_patches/mail_disable_starttls_spec.rb diff --git a/Gemfile b/Gemfile index e35e37c8104..f7271515eb0 100644 --- a/Gemfile +++ b/Gemfile @@ -41,7 +41,7 @@ gem 'actionview_precompiler', require: false gem 'seed-fu' -gem 'mail', git: 'https://github.com/discourse/mail.git', require: false +gem 'mail', git: 'https://github.com/discourse/mail.git' gem 'mini_mime' gem 'mini_suffix' diff --git a/config/environments/production.rb b/config/environments/production.rb index a523888a8d5..5f05f6e9b33 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -41,7 +41,7 @@ Discourse::Application.configure do settings[:tls] = true end - config.action_mailer.smtp_settings = settings.reject { |_, y| y.nil? } + config.action_mailer.smtp_settings = settings.compact else config.action_mailer.delivery_method = :sendmail config.action_mailer.sendmail_settings = { arguments: '-i' } diff --git a/lib/freedom_patches/mail_disable_starttls.rb b/lib/freedom_patches/mail_disable_starttls.rb new file mode 100644 index 00000000000..ccdd9116dd9 --- /dev/null +++ b/lib/freedom_patches/mail_disable_starttls.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +# Patch from +# https://github.com/rails/rails/issues/44698#issuecomment-1069675285 to enable +# previous behavior with Net::SMTP regarding TLS. +# +# This should be fixed in an upcoming release of the Mail gem (probably 2.8), +# when this patch is merged: https://github.com/mikel/mail/pull/1435 +module FreedomPatches::MailDisableStarttls + def build_smtp_session + super.tap do |smtp| + unless settings[:enable_starttls_auto] + if smtp.respond_to?(:disable_starttls) + smtp.disable_starttls + end + end + end + end + + ::Mail::SMTP.prepend(self) +end diff --git a/spec/lib/freedom_patches/mail_disable_starttls_spec.rb b/spec/lib/freedom_patches/mail_disable_starttls_spec.rb new file mode 100644 index 00000000000..454c0372556 --- /dev/null +++ b/spec/lib/freedom_patches/mail_disable_starttls_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +describe FreedomPatches::MailDisableStarttls do + subject(:smtp_session) { smtp.build_smtp_session } + + let(:smtp) { Mail::SMTP.new(options) } + + context "when the starttls option is not provided" do + let(:options) { {} } + + it "doesn't disable starttls" do + expect(smtp_session).to be_starttls + end + end + + context "when the starttls option is set to `false`" do + let(:options) { { enable_starttls_auto: false } } + + it "properly disables starttls" do + expect(smtp_session).not_to be_starttls + end + end +end