Skip to content

Commit

Permalink
Swap methods
Browse files Browse the repository at this point in the history
  • Loading branch information
johnnyshields committed Nov 26, 2023
1 parent 1efb89d commit f9e4ca4
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 21 deletions.
2 changes: 1 addition & 1 deletion lib/onelogin/ruby-saml/metadata.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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) }

Expand Down
26 changes: 13 additions & 13 deletions lib/onelogin/ruby-saml/settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
Expand All @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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?
Expand Down
14 changes: 7 additions & 7 deletions test/settings_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down Expand Up @@ -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) }
Expand All @@ -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) }
Expand All @@ -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) }
Expand All @@ -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

Expand All @@ -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
Expand Down

0 comments on commit f9e4ca4

Please sign in to comment.