From 06d0c2612624a0140d6f1e34b820900e2ee75c01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 29 Sep 2022 19:38:18 +0200 Subject: [PATCH] Allow having different SMTP settings per tenant Right now this is configured using the `secrets.yml` file, which is the file we've used in the past to configure SMTP settings. Note that, in the `current_secrets` method, the `if default?` condition is there so in single-tenant applications it returns the exact same object as `Rails.application.secrets`, and it makes it immediately clear for developers reading the code. We're also caching the tenant secrets (using `||=`) so they behave the same way as Rails secrets; for this to work properly 100% of the time (for example, in tests) we need to expire these cached secrets whenever the Rails secrets change. A similar `unless Tenant.default?` condition is present in the ApplicationMailer because there's a chance some CONSUL installations might not be using secrets to define the SMTP settings(they might be using environment variables, for example) and so in this case we don't want to force settings based on the secrets.yml file because it would break the application. The structure of the SMTP settings in the secrets file should be: ``` production: tenants: name_of_the_tenant_subdomain: smtp_settings: address: (...) ``` --- app/mailers/application_mailer.rb | 7 ++++ app/models/tenant.rb | 18 ++++++++++ config/secrets.yml.example | 24 +++++++++++++ spec/mailers/mailer_spec.rb | 36 ++++++++++++++++++++ spec/models/tenant_spec.rb | 56 +++++++++++++++++++++++++++++++ 5 files changed, 141 insertions(+) diff --git a/app/mailers/application_mailer.rb b/app/mailers/application_mailer.rb index 4451e348e..1fc1410e0 100644 --- a/app/mailers/application_mailer.rb +++ b/app/mailers/application_mailer.rb @@ -3,6 +3,7 @@ class ApplicationMailer < ActionMailer::Base default from: proc { "#{Setting["mailer_from_name"]} <#{Setting["mailer_from_address"]}>" } layout "mailer" before_action :set_asset_host + after_action :set_smtp_settings def default_url_options Tenant.current_url_options @@ -11,4 +12,10 @@ class ApplicationMailer < ActionMailer::Base def set_asset_host self.asset_host ||= root_url.delete_suffix("/") end + + def set_smtp_settings + unless Tenant.default? + mail.delivery_method.settings.merge!(Tenant.current_secrets.smtp_settings.to_h) + end + end end diff --git a/app/models/tenant.rb b/app/models/tenant.rb index 87b4a2c63..dbe00b335 100644 --- a/app/models/tenant.rb +++ b/app/models/tenant.rb @@ -53,6 +53,24 @@ class Tenant < ApplicationRecord end end + def self.current_secrets + if default? + Rails.application.secrets + else + @secrets ||= {} + @cached_rails_secrets ||= Rails.application.secrets + + if @cached_rails_secrets != Rails.application.secrets + @secrets = {} + @cached_rails_secrets = nil + end + + @secrets[current_schema] ||= Rails.application.secrets.merge( + Rails.application.secrets.dig(:tenants, current_schema.to_sym).to_h + ) + end + end + def self.default? current_schema == "public" end diff --git a/config/secrets.yml.example b/config/secrets.yml.example index 432ecc587..6d93ec4ab 100644 --- a/config/secrets.yml.example +++ b/config/secrets.yml.example @@ -50,6 +50,14 @@ staging: managers_url: "" managers_application_key: "" multitenancy: false + tenants: + # If you've enabled multitenancy, you can overwrite secrets for a + # specific tenant with: + # + # my_tenant_subdomain: + # secret_key: my_secret_value + # + # Currently you can overwrite SMTP settings. <<: *maps <<: *apis @@ -77,6 +85,14 @@ preproduction: managers_url: "" managers_application_key: "" multitenancy: false + tenants: + # If you've enabled multitenancy, you can overwrite secrets for a + # specific tenant with: + # + # my_tenant_subdomain: + # secret_key: my_secret_value + # + # Currently you can overwrite SMTP settings. twitter_key: "" twitter_secret: "" facebook_key: "" @@ -109,6 +125,14 @@ production: managers_url: "" managers_application_key: "" multitenancy: false + tenants: + # If you've enabled multitenancy, you can overwrite secrets for a + # specific tenant with: + # + # my_tenant_subdomain: + # secret_key: my_secret_value + # + # Currently you can overwrite SMTP settings. twitter_key: "" twitter_secret: "" facebook_key: "" diff --git a/spec/mailers/mailer_spec.rb b/spec/mailers/mailer_spec.rb index 3db6083c8..af991075c 100644 --- a/spec/mailers/mailer_spec.rb +++ b/spec/mailers/mailer_spec.rb @@ -69,5 +69,41 @@ describe Mailer do expect(body).to match "href=\"http://delay.consul.dev/" expect(body).to match "src=\"http://delay.consul.dev/" end + + describe "SMTP settings" do + let(:default_settings) { { address: "mail.consul.dev", username: "main" } } + let(:super_settings) { { address: "super.consul.dev", username: "super" } } + + before do + allow(Rails.application).to receive(:secrets).and_return(ActiveSupport::OrderedOptions.new.merge( + smtp_settings: default_settings, + tenants: { + supermailer: { smtp_settings: super_settings } + } + )) + end + + it "does not overwrite the settings for the default tenant" do + Mailer.user_invite("test@consul.dev").deliver_now + + expect(ActionMailer::Base.deliveries.last.delivery_method.settings).to eq({}) + end + + it "uses specific secret settings for tenants overwriting them" do + allow(Tenant).to receive(:current_schema).and_return("supermailer") + + Mailer.user_invite("test@consul.dev").deliver_now + + expect(ActionMailer::Base.deliveries.last.delivery_method.settings).to eq super_settings + end + + it "uses the default secret settings for other tenants" do + allow(Tenant).to receive(:current_schema).and_return("ultramailer") + + Mailer.user_invite("test@consul.dev").deliver_now + + expect(ActionMailer::Base.deliveries.last.delivery_method.settings).to eq default_settings + end + end end end diff --git a/spec/models/tenant_spec.rb b/spec/models/tenant_spec.rb index 60a658ffa..0d5cdfc8b 100644 --- a/spec/models/tenant_spec.rb +++ b/spec/models/tenant_spec.rb @@ -153,6 +153,62 @@ describe Tenant do end end + describe ".current_secrets" do + context "same secrets for all tenants" do + before do + allow(Rails.application).to receive(:secrets).and_return(ActiveSupport::OrderedOptions.new.merge( + star: "Sun", + volume: "Medium" + )) + end + + it "returns the default secrets for the default tenant" do + allow(Tenant).to receive(:current_schema).and_return("public") + + expect(Tenant.current_secrets.star).to eq "Sun" + expect(Tenant.current_secrets.volume).to eq "Medium" + end + + it "returns the default secrets for other tenants" do + allow(Tenant).to receive(:current_schema).and_return("earth") + + expect(Tenant.current_secrets.star).to eq "Sun" + expect(Tenant.current_secrets.volume).to eq "Medium" + end + end + + context "tenant overwriting secrets" do + before do + allow(Rails.application).to receive(:secrets).and_return(ActiveSupport::OrderedOptions.new.merge( + star: "Sun", + volume: "Medium", + tenants: { proxima: { star: "Alpha Centauri" }} + )) + end + + it "returns the default secrets for the default tenant" do + allow(Tenant).to receive(:current_schema).and_return("public") + + expect(Tenant.current_secrets.star).to eq "Sun" + expect(Tenant.current_secrets.volume).to eq "Medium" + end + + it "returns the overwritten secrets for tenants overwriting them" do + allow(Tenant).to receive(:current_schema).and_return("proxima") + + expect(Tenant.current_secrets.star).to eq "Alpha Centauri" + expect(Tenant.current_secrets.volume).to eq "Medium" + end + + it "returns the default secrets for other tenants" do + allow(Tenant).to receive(:current_schema).and_return("earth") + + expect(Tenant.current_secrets.star).to eq "Sun" + expect(Tenant.current_secrets.volume).to eq "Medium" + end + end + end + describe ".run_on_each" do it "runs the code on all tenants, including the default one" do create(:tenant, schema: "andromeda")