Merge pull request #6113 from consuldemocracy/saml_on_demand

Only access SAML single sign-on URL when necessary

app/lib/omniauth_tenant_setup.rb

@@ -42,23 +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_metadata_url] = idp_metadata_url if idp_metadata_url.present?
-          strategy.options[:idp_sso_service_url] = idp_sso_service_url if idp_sso_service_url.present?
-
-          if strategy.options[:issuer].present? && sp_entity_id.present?
-            strategy.options[:issuer] = sp_entity_id
-          end
-
-          if strategy.options[:idp_metadata].present? && idp_metadata_url.present?
-            strategy.options[:idp_metadata] = idp_metadata_url
-          end
+      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")
@@ -34,8 +40,8 @@ describe OmniauthTenantSetup do
         mars_strategy_options = mars_env["omniauth.strategy"].options
 
         expect(mars_strategy_options[:sp_entity_id]).to eq "https://mars.consul.dev/saml/metadata"
-        expect(mars_strategy_options[:idp_metadata_url]).to eq "https://mars-idp.example.com/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,8 +54,8 @@ describe OmniauthTenantSetup do
         venus_strategy_options = venus_env["omniauth.strategy"].options
 
         expect(venus_strategy_options[:sp_entity_id]).to eq "https://venus.consul.dev/saml/metadata"
-        expect(venus_strategy_options[:idp_metadata_url]).to eq "https://venus-idp.example.com/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
 
@@ -79,8 +85,8 @@ describe OmniauthTenantSetup do
         earth_strategy_options = earth_env["omniauth.strategy"].options
 
         expect(earth_strategy_options[:sp_entity_id]).to eq "https://default.consul.dev/saml/metadata"
-        expect(earth_strategy_options[:idp_metadata_url]).to eq "https://default-idp.example.com/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