Only access SAML single sign-on URL when necessary

We were calling `parse_remote_to_hash` in the Devise initializer, which
runs when the application starts.

That meant that, if we got an exception when calling that method, the
application wouldn't start. We got exceptions if the single sign-on
(SSO) URL isn't available or we aren't providing the right credentials.

So we're moving the call to `parse_remote_to_hash` to
`OmniauthTenantSetup`, which is only called when actually trying to sign
in with SAML.

Since we're moving the code there, we're also unifying the code so SAML
settings are configured the same way for the main tenant and other
tenants, like we did for OpenID Connect in commit c3b523290.

In order to keep the existing behavior, we're caching the result of
`parse_remote_to_hash` in an instance variable. Not sure about the
advantages and disadvantages of doing so over parsing the remote URL
metadata on every SAML-related request.

Note that the SAML tests in `OmniauthTenantSetup` use the `stub_secrets`
method. But this method is called after the application has started,
meaning it doesn't stub calls to `Rails.application.secrets` in
`config/initializers/`. So, before this commit, the code that parsed the
IDP metadata URL wasn't executed in the tests. Since now we've moved the
code but we don't want to depend on external URLs when running the
tests, we need to stub the call to the external URL. Since we're now
stubbing the call, we're adding expectations in the tests to check that
we correctly use the settings returned in that call.

app/lib/omniauth_tenant_setup.rb

@@ -42,14 +42,30 @@ module OmniauthTenantSetup
       end
 
       def saml_auth(env, sp_entity_id, idp_metadata_url, idp_sso_service_url)
-        unless Tenant.default?
-          strategy = env["omniauth.strategy"]
+        env["omniauth.strategy"].options.merge!(
+          saml_settings(sp_entity_id, idp_metadata_url, idp_sso_service_url)
+        )
+      end
 
-          strategy.options[:sp_entity_id] = sp_entity_id if sp_entity_id.present?
-          strategy.options[:idp_sso_service_url] = idp_sso_service_url if idp_sso_service_url.present?
+      def saml_settings(sp_entity_id, idp_metadata_url, idp_sso_service_url)
+        remote_saml_settings(idp_metadata_url).tap do |settings|
+          settings[:sp_entity_id] = sp_entity_id if sp_entity_id.present?
+          settings[:idp_sso_service_url] = idp_sso_service_url if idp_sso_service_url.present?
+          settings[:allowed_clock_drift] = 1.minute
         end
       end
 
+      def remote_saml_settings(idp_metadata_url)
+        return {} if idp_metadata_url.blank?
+
+        @remote_saml_settings ||= {}
+        @remote_saml_settings[idp_metadata_url] ||= parsed_saml_metadata(idp_metadata_url)
+      end
+
+      def parsed_saml_metadata(idp_metadata_url)
+        OneLogin::RubySaml::IdpMetadataParser.new.parse_remote_to_hash(idp_metadata_url)
+      end
+
       def oidc_auth(env, client_id, client_secret, issuer)
         strategy = env["omniauth.strategy"]
 

config/initializers/devise.rb

@@ -286,16 +286,7 @@ Devise.setup do |config|
                   Rails.application.secrets.wordpress_oauth2_secret,
                   client_options: { site: Rails.application.secrets.wordpress_oauth2_site },
                   setup: ->(env) { OmniauthTenantSetup.wordpress_oauth2(env) }
-  saml_settings = {}
-  if Rails.application.secrets.saml_idp_metadata_url.present?
-    idp_metadata_parser = OneLogin::RubySaml::IdpMetadataParser.new
-    saml_settings = idp_metadata_parser.parse_remote_to_hash(Rails.application.secrets.saml_idp_metadata_url)
-    saml_settings[:idp_sso_service_url] = Rails.application.secrets.saml_idp_sso_service_url
-    saml_settings[:sp_entity_id] = Rails.application.secrets.saml_sp_entity_id
-    saml_settings[:allowed_clock_drift] = 1.minute
-  end
-  config.omniauth :saml, saml_settings.merge(setup: ->(env) { OmniauthTenantSetup.saml(env) })
-
+  config.omniauth :saml, setup: ->(env) { OmniauthTenantSetup.saml(env) }
   config.omniauth :openid_connect,
                   name: :oidc,
                   scope: [:openid, :email, :profile],

spec/lib/omniauth_tenant_setup_spec.rb

@@ -2,6 +2,12 @@ require "rails_helper"
 
 describe OmniauthTenantSetup do
   describe "#saml" do
+    before do
+      allow(OmniauthTenantSetup).to receive(:parsed_saml_metadata) do |idp_metadata_url|
+        { idp_entity_id: idp_metadata_url.gsub("metadata", "entityid") }
+      end
+    end
+
     it "uses different secrets for different tenants" do
       create(:tenant, schema: "mars")
       create(:tenant, schema: "venus")
@@ -35,6 +41,7 @@ describe OmniauthTenantSetup do
 
         expect(mars_strategy_options[:sp_entity_id]).to eq "https://mars.consul.dev/saml/metadata"
         expect(mars_strategy_options[:idp_sso_service_url]).to eq "https://mars-idp.example.com/sso"
+        expect(mars_strategy_options[:idp_entity_id]).to eq "https://mars-idp.example.com/entityid"
       end
 
       Tenant.switch("venus") do
@@ -48,6 +55,7 @@ describe OmniauthTenantSetup do
 
         expect(venus_strategy_options[:sp_entity_id]).to eq "https://venus.consul.dev/saml/metadata"
         expect(venus_strategy_options[:idp_sso_service_url]).to eq "https://venus-idp.example.com/sso"
+        expect(venus_strategy_options[:idp_entity_id]).to eq "https://venus-idp.example.com/entityid"
       end
     end
 
@@ -78,6 +86,7 @@ describe OmniauthTenantSetup do
 
         expect(earth_strategy_options[:sp_entity_id]).to eq "https://default.consul.dev/saml/metadata"
         expect(earth_strategy_options[:idp_sso_service_url]).to eq "https://default-idp.example.com/sso"
+        expect(earth_strategy_options[:idp_entity_id]).to eq "https://default-idp.example.com/entityid"
       end
     end
   end