From f9e4ca497c0904ffbc837fc59c43dd69c4401fef Mon Sep 17 00:00:00 2001 From: johnnyshields <27655+johnnyshields@users.noreply.github.com> Date: Sun, 26 Nov 2023 16:22:58 +0900 Subject: [PATCH] Swap methods --- lib/onelogin/ruby-saml/metadata.rb | 2 +- lib/onelogin/ruby-saml/settings.rb | 26 +++++++++++++------------- test/settings_test.rb | 14 +++++++------- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/lib/onelogin/ruby-saml/metadata.rb b/lib/onelogin/ruby-saml/metadata.rb index 6cfe159a..fed96b67 100644 --- a/lib/onelogin/ruby-saml/metadata.rb +++ b/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) } diff --git a/lib/onelogin/ruby-saml/settings.rb b/lib/onelogin/ruby-saml/settings.rb index 20850af9..401732ac 100644 --- a/lib/onelogin/ruby-saml/settings.rb +++ b/lib/onelogin/ruby-saml/settings.rb @@ -205,24 +205,16 @@ def get_idp_cert_multi certs end - # @return [Hash>>] - # 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>>] # 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] # 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] 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>>] + # 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? diff --git a/test/settings_test.rb b/test/settings_test.rb index 91d3f941..827fe041 100644 --- a/test/settings_test.rb +++ b/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