From 654db485fb9c9364098c5a5835de2de21aaea15f Mon Sep 17 00:00:00 2001 From: Stuart Owen Date: Fri, 8 Nov 2024 14:56:58 +0000 Subject: [PATCH] remove authentication, user_name and password keys if set to blank or nil #2048 --- app/controllers/admin_controller.rb | 4 ++-- lib/seek/config.rb | 2 +- test/functional/admin_controller_test.rb | 25 +++++++++++++++++++++--- 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb index b1e4da4c8e..d0054568d8 100644 --- a/app/controllers/admin_controller.rb +++ b/app/controllers/admin_controller.rb @@ -539,11 +539,11 @@ def test_email_configuration smtp_hash_new = { address: params[:address], enable_starttls_auto: params[:enable_starttls_auto] == '1', domain: params[:domain], - authentication: params[:authentication], + authentication: (params[:authentication].blank? ? nil : params[:authentication] ), user_name: (params[:smtp_user_name].blank? ? nil : params[:smtp_user_name]), password: (params[:smtp_password].blank? ? nil : params[:smtp_password]) } smtp_hash_new[:port] = params[:port] if only_integer params[:port], 'port' - ActionMailer::Base.smtp_settings = smtp_hash_new + ActionMailer::Base.smtp_settings = smtp_hash_new.compact raise_delivery_errors_setting = ActionMailer::Base.raise_delivery_errors ActionMailer::Base.raise_delivery_errors = true begin diff --git a/lib/seek/config.rb b/lib/seek/config.rb index 7a18fcef3f..870ec28b82 100644 --- a/lib/seek/config.rb +++ b/lib/seek/config.rb @@ -45,7 +45,7 @@ def smtp_propagate new_hash[key.to_sym] = smtp_hash[key] end - ActionMailer::Base.smtp_settings = new_hash + ActionMailer::Base.smtp_settings = new_hash.compact end def bioportal_api_key_propagate diff --git a/test/functional/admin_controller_test.rb b/test/functional/admin_controller_test.rb index e28453ef84..06e844ea28 100644 --- a/test/functional/admin_controller_test.rb +++ b/test/functional/admin_controller_test.rb @@ -89,7 +89,7 @@ def setup with_config_value(:smtp, { address: '255.255.255.255', 'address' => '0.0.0.0' }) do assert_equal 'Hash', Seek::Config.smtp.class.name - post :update_features_enabled, params: { email_enabled: '1', address: '127.0.0.1', port: '25', domain: 'email.example.com', authentication: 'plain', smtp_user_name: '', smtp_password: '', enable_starttls_auto: '1' } + post :update_features_enabled, params: { email_enabled: '1', address: '127.0.0.1', port: '25', domain: 'email.example.com', authentication: 'plain', smtp_user_name: 'fred', smtp_password: 'bbb', enable_starttls_auto: '1' } assert_equal 'ActiveSupport::HashWithIndifferentAccess', Seek::Config.smtp.class.name assert Seek::Config.email_enabled @@ -99,13 +99,32 @@ def setup assert_equal '25', mailer_settings[:port] assert_equal 'email.example.com', mailer_settings[:domain] assert_equal 'plain', mailer_settings[:authentication] - assert_nil mailer_settings[:user_name] - assert_nil mailer_settings[:password] + assert_equal 'fred', mailer_settings[:user_name] + assert_equal 'bbb', mailer_settings[:password] assert mailer_settings[:enable_starttls_auto] end end end + test 'update SMTP settings nil authentication details removed' do + with_config_value(:email_enabled, false) do + with_config_value(:smtp, { address: '255.255.255.255', 'address' => '0.0.0.0' }) do + assert_equal 'Hash', Seek::Config.smtp.class.name + + post :update_features_enabled, params: { email_enabled: '1', address: '127.0.0.1', port: '25', domain: 'email.example.com', authentication: '', smtp_user_name: '', smtp_password: '', enable_starttls_auto: '0' } + + mailer_settings = ActionMailer::Base.smtp_settings + assert_equal '127.0.0.1', mailer_settings[:address] + assert_equal '25', mailer_settings[:port] + assert_equal 'email.example.com', mailer_settings[:domain] + refute mailer_settings[:enable_starttls_auto] + refute mailer_settings.has_key?(:authentication) + refute mailer_settings.has_key?(:user_name) + refute mailer_settings.has_key?(:password) + end + end + end + test 'should read SMTP setting as a HashWithIndifferentAccess' do with_config_value(:smtp, { address: '255.255.255.255', 'domain' => 'email.example.com' }) do assert_equal 'Hash', Seek::Config.smtp.class.name