Swap methods

Author: johnnyshields

lib/onelogin/ruby-saml/metadata.rb

@@ -64,7 +64,7 @@ def add_sp_sso_element(root, settings)
 
       # Add KeyDescriptor elements for SP certificates.
       def add_sp_certificates(sp_sso, settings)
-        certs = settings.get_active_sp_certs
+        certs = settings.get_sp_certs
 
         certs[:signing].each { |cert, _| add_sp_cert_element(sp_sso, cert, :signing) }
 

lib/onelogin/ruby-saml/settings.rb

@@ -205,24 +205,16 @@ def get_idp_cert_multi
         certs
       end
 
-      # @return [Hash<Symbol, Array<Array<OpenSSL::X509::Certificate, OpenSSL::PKey::RSA>>>]
-      #   Build the SP certificates and private keys from the settings. Returns all
-      #   certificates and private keys, even if they are expired.
-      def get_sp_certs
-        validate_sp_certs_params!
-        get_sp_certs_multi || get_sp_certs_single
-      end
-
       # @return [Hash<Symbol, Array<Array<OpenSSL::X509::Certificate, OpenSSL::PKey::RSA>>>]
       #   Build the SP certificates and private keys from the settings. If
       #   check_sp_cert_expiration is true, only returns certificates and private keys
       #   that are not expired.
-      def get_active_sp_certs
-        certs = get_sp_certs
+      def get_sp_certs
+        certs = get_all_sp_certs
         return certs unless security[:check_sp_cert_expiration]
 
         active_certs = { signing: [], encryption: [] }
-        get_sp_certs.each do |use, pairs|
+        certs.each do |use, pairs|
           next if pairs.empty?
 
           pairs = pairs.select { |cert, _| !cert || OneLogin::RubySaml::Utils.is_cert_active(cert) }
@@ -236,7 +228,7 @@ def get_active_sp_certs
       # @return [Array<OpenSSL::X509::Certificate, OpenSSL::PKey::RSA>]
       #   The SP signing certificate and private key.
       def get_sp_signing_pair
-        get_active_sp_certs[:signing].first
+        get_sp_certs[:signing].first
       end
 
       # @return [OpenSSL::X509::Certificate] The SP signing certificate.
@@ -257,7 +249,7 @@ def get_sp_signing_key
 
       # @return [Array<OpenSSL::PKey::RSA>] The SP decryption keys.
       def get_sp_decryption_keys
-        ary = get_active_sp_certs[:encryption].map { |pair| pair[1] }
+        ary = get_sp_certs[:encryption].map { |pair| pair[1] }
         ary.compact!
         ary.uniq!(&:to_pem)
         ary.freeze
@@ -310,6 +302,14 @@ def get_binding(value)
 
       private
 
+      # @return [Hash<Symbol, Array<Array<OpenSSL::X509::Certificate, OpenSSL::PKey::RSA>>>]
+      #   Build the SP certificates and private keys from the settings. Returns all
+      #   certificates and private keys, even if they are expired.
+      def get_all_sp_certs
+        validate_sp_certs_params!
+        get_sp_certs_multi || get_sp_certs_single
+      end
+
       # Validate certificate, certificate_new, private_key, and sp_cert_multi params.
       def validate_sp_certs_params!
         multi    = sp_cert_multi   && !sp_cert_multi.empty?

test/settings_test.rb

@@ -439,7 +439,7 @@ class SettingsTest < Minitest::Test
       end
     end
 
-    describe "#get_sp_certs" do
+    describe "#get_sp_certs (base cases)" do
       let(:cert_text1) { ruby_saml_cert_text }
       let(:cert_text2) { ruby_saml_cert2.to_pem }
       let(:cert_text3) { CertificateHelper.generate_cert.to_pem }
@@ -593,7 +593,7 @@ class SettingsTest < Minitest::Test
       end
     end
 
-    describe "#get_active_sp_certs" do
+    describe "#get_sp_certs" do
       let(:valid_pair) { CertificateHelper.generate_pair_hash }
       let(:early_pair) { CertificateHelper.generate_pair_hash(not_before: Time.now + 60) }
       let(:expired_pair) { CertificateHelper.generate_pair_hash(not_after: Time.now - 60) }
@@ -602,7 +602,7 @@ class SettingsTest < Minitest::Test
         @settings.security = { check_sp_cert_expiration: false }
         @settings.sp_cert_multi = { signing: [valid_pair, expired_pair], encryption: [valid_pair, early_pair] }
 
-        actual = @settings.get_active_sp_certs
+        actual = @settings.get_sp_certs
         expected_signing = [valid_pair, expired_pair].map(&:values)
         expected_encryption = [valid_pair, early_pair].map(&:values)
         assert_equal expected_signing, actual[:signing].map {|ary| ary.map(&:to_pem) }
@@ -613,7 +613,7 @@ class SettingsTest < Minitest::Test
         @settings.security = { check_sp_cert_expiration: true }
         @settings.sp_cert_multi = { signing: [valid_pair, expired_pair], encryption: [valid_pair, early_pair] }
 
-        actual = @settings.get_active_sp_certs
+        actual = @settings.get_sp_certs
         expected_active = [valid_pair].map(&:values)
         assert_equal expected_active, actual[:signing].map {|ary| ary.map(&:to_pem) }
         assert_equal expected_active, actual[:encryption].map {|ary| ary.map(&:to_pem) }
@@ -624,7 +624,7 @@ class SettingsTest < Minitest::Test
         @settings.sp_cert_multi = { signing: [expired_pair], encryption: [valid_pair] }
 
         assert_raises OneLogin::RubySaml::ValidationError do
-          @settings.get_active_sp_certs
+          @settings.get_sp_certs
         end
       end
 
@@ -633,14 +633,14 @@ class SettingsTest < Minitest::Test
         @settings.sp_cert_multi = { signing: [valid_pair], encryption: [expired_pair] }
 
         assert_raises OneLogin::RubySaml::ValidationError do
-          @settings.get_active_sp_certs
+          @settings.get_sp_certs
         end
       end
 
       it "returns empty arrays for signing and encryption if no pairs are present" do
         @settings.sp_cert_multi = { signing: [], encryption: [] }
 
-        actual = @settings.get_active_sp_certs
+        actual = @settings.get_sp_certs
         assert_empty actual[:signing]
         assert_empty actual[:encryption]
       end