From 3f2653a14d92758c8e31904cb79306ff981c09cb Mon Sep 17 00:00:00 2001
From: Mac Siri <mac@forem.com>
Date: Mon, 24 Jan 2022 14:11:50 -0500
Subject: [PATCH] Prefer custom SMTP config over Sendgrid (#16216)

---
 app/models/forem_instance.rb            |  2 +-
 app/models/settings/smtp.rb             | 16 +++++++++++---
 spec/mailers/application_mailer_spec.rb |  1 +
 spec/models/forem_instance_spec.rb      |  3 ++-
 spec/models/settings/smtp_spec.rb       | 29 ++++++++++++++++++++-----
 spec/support/seeds/seeds_e2e.rb         |  1 +
 6 files changed, 41 insertions(+), 11 deletions(-)

diff --git a/app/models/forem_instance.rb b/app/models/forem_instance.rb
index 3eb6334f2..59dcf4fc4 100644
--- a/app/models/forem_instance.rb
+++ b/app/models/forem_instance.rb
@@ -25,7 +25,7 @@ class ForemInstance
   end
 
   def self.smtp_enabled?
-    (Settings::SMTP.user_name.present? && Settings::SMTP.password.present?) || ENV["SENDGRID_API_KEY"].present?
+    Settings::SMTP.provided_minimum_settings? || ENV["SENDGRID_API_KEY"].present?
   end
 
   def self.invitation_only?
diff --git a/app/models/settings/smtp.rb b/app/models/settings/smtp.rb
index ecddac0bc..da2e8b246 100644
--- a/app/models/settings/smtp.rb
+++ b/app/models/settings/smtp.rb
@@ -14,14 +14,24 @@ module Settings
 
     class << self
       def settings
-        return sendgrid_settings if ENV["SENDGRID_API_KEY"].present?
+        if provided_minimum_settings?
+          custom_provider_settings
+        else
+          fallback_sendgrid_settings
+        end
+      end
 
-        keys.index_with { |k| public_send(k) }.symbolize_keys
+      def provided_minimum_settings?
+        address.present? && user_name.present? && password.present?
       end
 
       private
 
-      def sendgrid_settings
+      def custom_provider_settings
+        to_h
+      end
+
+      def fallback_sendgrid_settings
         {
           address: "smtp.sendgrid.net",
           port: 587,
diff --git a/spec/mailers/application_mailer_spec.rb b/spec/mailers/application_mailer_spec.rb
index 91a1974e9..d383e1a59 100644
--- a/spec/mailers/application_mailer_spec.rb
+++ b/spec/mailers/application_mailer_spec.rb
@@ -23,6 +23,7 @@ RSpec.describe ApplicationMailer, type: :mailer do
     end
 
     it "sets proper SMTP credential during callback" do
+      Settings::SMTP.address = "smtp.google.com"
       Settings::SMTP.user_name = Faker::Internet.username
       Settings::SMTP.password = Faker::Internet.password
       email.deliver_now
diff --git a/spec/models/forem_instance_spec.rb b/spec/models/forem_instance_spec.rb
index 95f1e80f9..b8f3a4b74 100644
--- a/spec/models/forem_instance_spec.rb
+++ b/spec/models/forem_instance_spec.rb
@@ -82,7 +82,8 @@ RSpec.describe ForemInstance, type: :model do
       expect(described_class.smtp_enabled?).to be(false)
     end
 
-    it "returns true if user_name and password are present" do
+    it "returns true if provided_minimum_settings?" do
+      allow(Settings::SMTP).to receive(:address).and_return("address")
       allow(Settings::SMTP).to receive(:user_name).and_return("something")
       allow(Settings::SMTP).to receive(:password).and_return("something")
 
diff --git a/spec/models/settings/smtp_spec.rb b/spec/models/settings/smtp_spec.rb
index f901bc94f..2c05b208f 100644
--- a/spec/models/settings/smtp_spec.rb
+++ b/spec/models/settings/smtp_spec.rb
@@ -1,16 +1,18 @@
 require "rails_helper"
 
 RSpec.describe Settings::SMTP do
+  let(:key) { "something" }
+
+  before { ENV["SENDGRID_API_KEY"] = key }
+
   after do
     described_class.clear_cache
+    ENV["SENDGRID_API_KEY"] = nil
   end
 
   describe "::settings" do
-    it "use default sendgrid config if SENDGRID_API_KEY is available" do
-      key = "something"
+    it "use falback sendgrid settings if address is not provided" do
       domain = "test.com"
-      allow(ApplicationConfig).to receive(:[]).with("SENDGRID_API_KEY").and_return(key)
-      ENV["SENDGRID_API_KEY"] = "something"
       allow(Settings::General).to receive(:app_domain).and_return(domain)
 
       expect(described_class.settings).to eq({
@@ -21,10 +23,9 @@ RSpec.describe Settings::SMTP do
                                                password: key,
                                                domain: domain
                                              })
-      ENV["SENDGRID_API_KEY"] = nil
     end
 
-    it "uses Settings::SMTP config if SENDGRID_API_KEY is not available" do
+    it "uses Settings::SMTP config if address is provided" do
       described_class.address = "smtp.google.com"
       described_class.port = 25
       described_class.authentication = "plain"
@@ -42,4 +43,20 @@ RSpec.describe Settings::SMTP do
                                              })
     end
   end
+
+  describe "::provided_minimum_settings?" do
+    it "returns true if addess, user_name, and password are provided" do
+      described_class.address = "smtp.google.com"
+      described_class.user_name = "username"
+      described_class.password = "password"
+
+      expect(described_class.provided_minimum_settings?).to be true
+    end
+
+    it "returns false if one of addess, user_name, or password is missing" do
+      described_class.address = "smtp.google.com"
+
+      expect(described_class.provided_minimum_settings?).to be false
+    end
+  end
 end
diff --git a/spec/support/seeds/seeds_e2e.rb b/spec/support/seeds/seeds_e2e.rb
index 8b93a30d7..8b96d95db 100644
--- a/spec/support/seeds/seeds_e2e.rb
+++ b/spec/support/seeds/seeds_e2e.rb
@@ -12,6 +12,7 @@ seeder = Seeder.new
 Settings::UserExperience.public = true
 Settings::General.waiting_on_first_user = false
 Settings::Authentication.allow_email_password_registration = true
+Settings::SMTP.address = "smtp.website.com"
 Settings::SMTP.user_name = "username"
 Settings::SMTP.password = "password"