From 752085a6f88cd3ce75ecc7a64afe064a0e4f9e35 Mon Sep 17 00:00:00 2001 From: davida marion Date: Mon, 25 Nov 2024 13:37:41 -0500 Subject: [PATCH 1/2] Fix bug when signature is params --- Gemfile.lock | 2 +- lib/saml_idp/version.rb | 2 +- lib/saml_idp/xml_security.rb | 4 +++- spec/lib/saml_idp/request_spec.rb | 10 ++++++++++ spec/support/saml_request_macros.rb | 6 +++++- 5 files changed, 20 insertions(+), 4 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 11f2eeac..c18e1a4f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - saml_idp (0.23.1.pre.18f) + saml_idp (0.23.3.pre.18f) activesupport builder faraday diff --git a/lib/saml_idp/version.rb b/lib/saml_idp/version.rb index e0cb5383..c7cd3504 100644 --- a/lib/saml_idp/version.rb +++ b/lib/saml_idp/version.rb @@ -1,3 +1,3 @@ module SamlIdp - VERSION = '0.23.2-18f'.freeze + VERSION = '0.23.3-18f'.freeze end diff --git a/lib/saml_idp/xml_security.rb b/lib/saml_idp/xml_security.rb index 5fe05508..233faea6 100644 --- a/lib/saml_idp/xml_security.rb +++ b/lib/saml_idp/xml_security.rb @@ -79,10 +79,12 @@ def validate_with_sha256(idp_certificate, options = {}) raise ValidationError.new('Request certificate not valid or registered', :request_cert_not_registered) end - validate_doc(request_cert, false, options) + validate_doc(Base64.encode64(idp_certificate.to_pem), false, options) end def request_cert + return false if cert_element.blank? + if cert_element.text.blank? raise ValidationError.new( 'Certificate element present in response (ds:X509Certificate) but evaluating to nil', diff --git a/spec/lib/saml_idp/request_spec.rb b/spec/lib/saml_idp/request_spec.rb index 409e2d75..3bd44d02 100644 --- a/spec/lib/saml_idp/request_spec.rb +++ b/spec/lib/saml_idp/request_spec.rb @@ -477,6 +477,16 @@ module SamlIdp expect(subject.sha256_validation_matching_cert).to eq cert end + describe 'when the request is not embedded' do + let(:security_overrides) {{ embed_sign: false }} + let(:params) { encoded_request.symbolize_keys! } + subject { described_class.from_deflated_request(params[:SAMLRequest], get_params: params) } + + it 'returns the cert' do + expect(subject.sha256_validation_matching_cert).to eq cert + end + end + context 'when the signature algorithm is not right' do let(:security_overrides) do { diff --git a/spec/support/saml_request_macros.rb b/spec/support/saml_request_macros.rb index e059eae8..c1ee66bd 100644 --- a/spec/support/saml_request_macros.rb +++ b/spec/support/saml_request_macros.rb @@ -10,7 +10,11 @@ def custom_saml_request(overrides: {}, security_overrides: {}, signed: true) ) ) - CGI.unescape(auth_url.split('=').last) + if security_overrides[:embed_sign] == false + Rack::Utils.parse_nested_query URI(auth_url).query + else + CGI.unescape(auth_url.split('=').last) + end end def custom_logout_request(overrides: {}, security_overrides: {}) From f249deb2e1a6739623b59baad3ad5823b8b3049c Mon Sep 17 00:00:00 2001 From: "Davida (she/they)" Date: Mon, 25 Nov 2024 13:55:14 -0500 Subject: [PATCH 2/2] Update spec/support/saml_request_macros.rb Co-authored-by: Zach Margolis --- spec/support/saml_request_macros.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/support/saml_request_macros.rb b/spec/support/saml_request_macros.rb index c1ee66bd..b786b55e 100644 --- a/spec/support/saml_request_macros.rb +++ b/spec/support/saml_request_macros.rb @@ -11,7 +11,7 @@ def custom_saml_request(overrides: {}, security_overrides: {}, signed: true) ) if security_overrides[:embed_sign] == false - Rack::Utils.parse_nested_query URI(auth_url).query + Rack::Utils.parse_nested_query(URI(auth_url).query) else CGI.unescape(auth_url.split('=').last) end