Finalize tests and fixes

SAML-Toolkits · Nov 25, 2023 · 49f8f82 · 49f8f82
1 parent 19c2932
commit 49f8f82
Show file tree

Hide file tree

Showing 8 changed files with 275 additions and 22 deletions.
diff --git a/lib/onelogin/ruby-saml/authrequest.rb b/lib/onelogin/ruby-saml/authrequest.rb
@@ -72,8 +72,9 @@ def create_params(settings, params={})
         request = deflate(request) if settings.compress_request
         base64_request = encode(request)
         request_params = {"SAMLRequest" => base64_request}
+        sp_signing_key = settings.get_sp_signing_key
 
-        if settings.idp_sso_service_binding == Utils::BINDINGS[:redirect] && settings.security[:authn_requests_signed] && settings.private_key
+        if settings.idp_sso_service_binding == Utils::BINDINGS[:redirect] && settings.security[:authn_requests_signed] && sp_signing_key
           params['SigAlg'] = settings.security[:signature_method]
           url_string = OneLogin::RubySaml::Utils.build_query(
             :type => 'SAMLRequest',
@@ -82,7 +83,7 @@ def create_params(settings, params={})
             :sig_alg => params['SigAlg']
           )
           sign_algorithm = XMLSecurity::BaseDocument.new.algorithm(settings.security[:signature_method])
-          signature = settings.get_sp_signing_key.sign(sign_algorithm.new, url_string)
+          signature = sp_signing_key.sign(sign_algorithm.new, url_string)
           params['Signature'] = encode(signature)
         end
 
@@ -179,8 +180,8 @@ def create_xml_document(settings)
       end
 
       def sign_document(document, settings)
-        if settings.idp_sso_service_binding == Utils::BINDINGS[:post] && settings.security[:authn_requests_signed] && settings.private_key && settings.certificate
-          cert, private_key = settings.get_sp_signing_pair
+        cert, private_key = settings.get_sp_signing_pair
+        if settings.idp_sso_service_binding == Utils::BINDINGS[:post] && settings.security[:authn_requests_signed] && private_key && cert
           document.sign_document(private_key, cert, settings.security[:signature_method], settings.security[:digest_method])
         end
 

diff --git a/lib/onelogin/ruby-saml/logoutrequest.rb b/lib/onelogin/ruby-saml/logoutrequest.rb
@@ -69,8 +69,9 @@ def create_params(settings, params={})
         request = deflate(request) if settings.compress_request
         base64_request = encode(request)
         request_params = {"SAMLRequest" => base64_request}
+        sp_signing_key = settings.get_sp_signing_key
 
-        if settings.idp_slo_service_binding == Utils::BINDINGS[:redirect] && settings.security[:logout_requests_signed] && settings.private_key
+        if settings.idp_slo_service_binding == Utils::BINDINGS[:redirect] && settings.security[:logout_requests_signed] && sp_signing_key
           params['SigAlg'] = settings.security[:signature_method]
           url_string = OneLogin::RubySaml::Utils.build_query(
             :type => 'SAMLRequest',
@@ -138,8 +139,8 @@ def create_xml_document(settings)
 
       def sign_document(document, settings)
         # embed signature
-        if settings.idp_slo_service_binding == Utils::BINDINGS[:post] && settings.security[:logout_requests_signed] && settings.private_key && settings.certificate
-          cert, private_key = settings.get_sp_signing_pair
+        cert, private_key = settings.get_sp_signing_pair
+        if settings.idp_slo_service_binding == Utils::BINDINGS[:post] && settings.security[:logout_requests_signed] && private_key && cert
           document.sign_document(private_key, cert, settings.security[:signature_method], settings.security[:digest_method])
         end
 

diff --git a/lib/onelogin/ruby-saml/slo_logoutresponse.rb b/lib/onelogin/ruby-saml/slo_logoutresponse.rb
@@ -78,17 +78,18 @@ def create_params(settings, request_id = nil, logout_message = nil, params = {},
         response = deflate(response) if settings.compress_response
         base64_response = encode(response)
         response_params = {"SAMLResponse" => base64_response}
+        sp_signing_key = settings.get_sp_signing_key
 
-        if settings.idp_slo_service_binding == Utils::BINDINGS[:redirect] && settings.security[:logout_responses_signed] && settings.private_key
-          params['SigAlg']    = settings.security[:signature_method]
+        if settings.idp_slo_service_binding == Utils::BINDINGS[:redirect] && settings.security[:logout_responses_signed] && sp_signing_key
+          params['SigAlg'] = settings.security[:signature_method]
           url_string = OneLogin::RubySaml::Utils.build_query(
             :type => 'SAMLResponse',
             :data => base64_response,
             :relay_state => relay_state,
             :sig_alg => params['SigAlg']
           )
           sign_algorithm = XMLSecurity::BaseDocument.new.algorithm(settings.security[:signature_method])
-          signature = settings.get_sp_signing_key.sign(sign_algorithm.new, url_string)
+          signature = sp_signing_key.sign(sign_algorithm.new, url_string)
           params['Signature'] = encode(signature)
         end
 
@@ -150,8 +151,8 @@ def create_xml_document(settings, request_id = nil, logout_message = nil, status
 
       def sign_document(document, settings)
         # embed signature
-        if settings.idp_slo_service_binding == Utils::BINDINGS[:post] && settings.private_key && settings.certificate
-          cert, private_key = settings.get_sp_signing_pair
+        cert, private_key = settings.get_sp_signing_pair
+        if settings.idp_slo_service_binding == Utils::BINDINGS[:post] && private_key && cert
           document.sign_document(private_key, cert, settings.security[:signature_method], settings.security[:digest_method])
         end
 

diff --git a/test/logoutrequest_test.rb b/test/logoutrequest_test.rb
@@ -110,7 +110,6 @@ class RequestTest < Minitest::Test
     end
 
     describe "signing with HTTP-POST binding" do
-
       before do
         settings.security[:logout_requests_signed] = true
         settings.idp_slo_service_binding = :post
@@ -153,7 +152,7 @@ class RequestTest < Minitest::Test
         assert_match %r[<ds:DigestMethod Algorithm='http://www.w3.org/2000/09/xmldsig#sha1'/>], inflated
       end
 
-      it "created a signed logout request" do
+      it "create a signed logout request" do
         settings.compress_request = true
 
         unauth_req = OneLogin::RubySaml::Logoutrequest.new
@@ -165,14 +164,24 @@ class RequestTest < Minitest::Test
         assert_match %r[<ds:DigestMethod Algorithm='http://www.w3.org/2000/09/xmldsig#sha1'/>], inflated
       end
 
+      it "create an uncompressed signed logout request" do
+        settings.compress_request = false
+
+        params = OneLogin::RubySaml::Logoutrequest.new.create_params(settings)
+        request_xml = Base64.decode64(params["SAMLRequest"])
+
+        assert_match %r[<ds:SignatureValue>([a-zA-Z0-9/+=]+)</ds:SignatureValue>], request_xml
+        assert_match %r[<ds:SignatureMethod Algorithm='http://www.w3.org/2000/09/xmldsig#rsa-sha1'/>], request_xml
+        assert_match %r[<ds:DigestMethod Algorithm='http://www.w3.org/2000/09/xmldsig#sha1'/>], request_xml
+      end
+
       it "create a signed logout request with 256 digest and signature method" do
         settings.compress_request = false
         settings.security[:signature_method] = XMLSecurity::Document::RSA_SHA256
         settings.security[:digest_method] = XMLSecurity::Document::SHA256
 
         params = OneLogin::RubySaml::Logoutrequest.new.create_params(settings)
         request_xml = Base64.decode64(params["SAMLRequest"])
-
         assert_match %r[<ds:SignatureValue>([a-zA-Z0-9/+=]+)</ds:SignatureValue>], request_xml
         assert_match %r[<ds:SignatureMethod Algorithm='http://www.w3.org/2001/04/xmldsig-more#rsa-sha256'/>], request_xml
         assert_match %r[<ds:DigestMethod Algorithm='http://www.w3.org/2001/04/xmlenc#sha256'/>], request_xml
@@ -190,6 +199,53 @@ class RequestTest < Minitest::Test
         assert_match %r[<ds:SignatureMethod Algorithm='http://www.w3.org/2001/04/xmldsig-more#rsa-sha384'/>], request_xml
         assert_match %r[<ds:DigestMethod Algorithm='http://www.w3.org/2001/04/xmlenc#sha512'/>], request_xml
       end
+
+      it "create a signed logout request using the first certificate and key" do
+        settings.compress_request = false
+        settings.certificate = nil
+        settings.private_key = nil
+        settings.sp_cert_multi = {
+          signing: [
+            { certificate: ruby_saml_cert_text, private_key: ruby_saml_key_text },
+            CertificateHelper.generate_pair_hash
+          ]
+        }
+
+        params = OneLogin::RubySaml::Logoutrequest.new.create_params(settings)
+        request_xml = Base64.decode64(params["SAMLRequest"])
+
+        assert_match %r[<ds:SignatureValue>([a-zA-Z0-9/+=]+)</ds:SignatureValue>], request_xml
+        assert_match %r[<ds:SignatureMethod Algorithm='http://www.w3.org/2000/09/xmldsig#rsa-sha1'/>], request_xml
+        assert_match %r[<ds:DigestMethod Algorithm='http://www.w3.org/2000/09/xmldsig#sha1'/>], request_xml
+      end
+
+      it "create a signed logout request using the first valid certificate and key when :check_sp_cert_expiration is true" do
+        settings.compress_request = false
+        settings.certificate = nil
+        settings.private_key = nil
+        settings.security[:check_sp_cert_expiration] = true
+        settings.sp_cert_multi = {
+          signing: [
+            { certificate: ruby_saml_cert_text, private_key: ruby_saml_key_text },
+            CertificateHelper.generate_pair_hash
+          ]
+        }
+
+        params = OneLogin::RubySaml::Logoutrequest.new.create_params(settings)
+        request_xml = Base64.decode64(params["SAMLRequest"])
+
+        assert_match %r[<ds:SignatureValue>([a-zA-Z0-9/+=]+)</ds:SignatureValue>], request_xml
+        assert_match %r[<ds:SignatureMethod Algorithm='http://www.w3.org/2000/09/xmldsig#rsa-sha1'/>], request_xml
+        assert_match %r[<ds:DigestMethod Algorithm='http://www.w3.org/2000/09/xmldsig#sha1'/>], request_xml
+      end
+
+      it "raises error when no valid certs and :check_sp_cert_expiration is true" do
+        settings.security[:check_sp_cert_expiration] = true
+
+        assert_raises(OneLogin::RubySaml::ValidationError, 'The SP certificate expired.') do
+          OneLogin::RubySaml::Logoutrequest.new.create_params(settings)
+        end
+      end
     end
 
     describe "signing with HTTP-Redirect binding" do
@@ -269,6 +325,41 @@ class RequestTest < Minitest::Test
         assert_equal signature_algorithm, OpenSSL::Digest::SHA512
         assert cert.public_key.verify(signature_algorithm.new, Base64.decode64(params['Signature']), query_string)
       end
+
+      it "create a signature parameter using the first certificate and key" do
+        settings.security[:signature_method] = XMLSecurity::Document::RSA_SHA1
+        settings.compress_request = false
+        settings.certificate = nil
+        settings.private_key = nil
+        settings.sp_cert_multi = {
+          signing: [
+            { certificate: ruby_saml_cert_text, private_key: ruby_saml_key_text },
+            CertificateHelper.generate_pair_hash
+          ]
+        }
+
+        params = OneLogin::RubySaml::Logoutrequest.new.create_params(settings, :RelayState => 'http://example.com')
+        assert params['SAMLRequest']
+        assert params[:RelayState]
+        assert params['Signature']
+        assert_equal params['SigAlg'], XMLSecurity::Document::RSA_SHA1
+
+        query_string = "SAMLRequest=#{CGI.escape(params['SAMLRequest'])}"
+        query_string << "&RelayState=#{CGI.escape(params[:RelayState])}"
+        query_string << "&SigAlg=#{CGI.escape(params['SigAlg'])}"
+
+        signature_algorithm = XMLSecurity::BaseDocument.new.algorithm(params['SigAlg'])
+        assert_equal signature_algorithm, OpenSSL::Digest::SHA1
+        assert cert.public_key.verify(signature_algorithm.new, Base64.decode64(params['Signature']), query_string)
+      end
+
+      it "raises error when no valid certs and :check_sp_cert_expiration is true" do
+        settings.security[:check_sp_cert_expiration] = true
+
+        assert_raises(OneLogin::RubySaml::ValidationError, 'The SP certificate expired.') do
+          OneLogin::RubySaml::Logoutrequest.new.create_params(settings, :RelayState => 'http://example.com')
+        end
+      end
     end
 
     describe "DEPRECATED: signing with HTTP-POST binding via :embed_sign" do

diff --git a/test/request_test.rb b/test/request_test.rb
@@ -268,6 +268,49 @@ class RequestTest < Minitest::Test
         assert_match %r[<ds:SignatureMethod Algorithm='http://www.w3.org/2001/04/xmldsig-more#rsa-sha256'/>], request_xml
         assert_match %r[<ds:DigestMethod Algorithm='http://www.w3.org/2001/04/xmlenc#sha512'/>], request_xml
       end
+
+      it "creates a signed request using the first certificate and key" do
+        settings.certificate = nil
+        settings.private_key = nil
+        settings.sp_cert_multi = {
+          signing: [
+            { certificate: ruby_saml_cert_text, private_key: ruby_saml_key_text },
+            CertificateHelper.generate_pair_hash
+          ]
+        }
+
+        params = OneLogin::RubySaml::Authrequest.new.create_params(settings)
+
+        request_xml = Base64.decode64(params["SAMLRequest"])
+        assert_match %r[<ds:SignatureValue>([a-zA-Z0-9/+=]+)</ds:SignatureValue>], request_xml
+        assert_match %r[<ds:SignatureMethod Algorithm='http://www.w3.org/2000/09/xmldsig#rsa-sha1'/>], request_xml
+      end
+
+      it "creates a signed request using the first valid certificate and key when :check_sp_cert_expiration is true" do
+        settings.certificate = nil
+        settings.private_key = nil
+        settings.security[:check_sp_cert_expiration] = true
+        settings.sp_cert_multi = {
+          signing: [
+            { certificate: ruby_saml_cert_text, private_key: ruby_saml_key_text },
+            CertificateHelper.generate_pair_hash
+          ]
+        }
+
+        params = OneLogin::RubySaml::Authrequest.new.create_params(settings)
+
+        request_xml = Base64.decode64(params["SAMLRequest"])
+        assert_match %r[<ds:SignatureValue>([a-zA-Z0-9/+=]+)</ds:SignatureValue>], request_xml
+        assert_match %r[<ds:SignatureMethod Algorithm='http://www.w3.org/2000/09/xmldsig#rsa-sha1'/>], request_xml
+      end
+
+      it "raises error when no valid certs and :check_sp_cert_expiration is true" do
+        settings.security[:check_sp_cert_expiration] = true
+
+        assert_raises(OneLogin::RubySaml::ValidationError, 'The SP certificate expired.') do
+          OneLogin::RubySaml::Authrequest.new.create_params(settings)
+        end
+      end
     end
 
     describe "#create_params signing with HTTP-Redirect binding" do
@@ -298,7 +341,6 @@ class RequestTest < Minitest::Test
 
         signature_algorithm = XMLSecurity::BaseDocument.new.algorithm(params['SigAlg'])
         assert_equal signature_algorithm, OpenSSL::Digest::SHA1
-
         assert cert.public_key.verify(signature_algorithm.new, Base64.decode64(params['Signature']), query_string)
       end
 
@@ -317,6 +359,41 @@ class RequestTest < Minitest::Test
         assert_equal signature_algorithm, OpenSSL::Digest::SHA256
         assert cert.public_key.verify(signature_algorithm.new, Base64.decode64(params['Signature']), query_string)
       end
+
+      it "create a signature parameter using the first certificate and key" do
+        settings.security[:signature_method] = XMLSecurity::Document::RSA_SHA1
+        settings.compress_request = false
+        settings.certificate = nil
+        settings.private_key = nil
+        settings.sp_cert_multi = {
+          signing: [
+            { certificate: ruby_saml_cert_text, private_key: ruby_saml_key_text },
+            CertificateHelper.generate_pair_hash
+          ]
+        }
+
+        params = OneLogin::RubySaml::Authrequest.new.create_params(settings, :RelayState => 'http://example.com')
+        assert params['SAMLRequest']
+        assert params[:RelayState]
+        assert params['Signature']
+        assert_equal params['SigAlg'], XMLSecurity::Document::RSA_SHA1
+
+        query_string = "SAMLRequest=#{CGI.escape(params['SAMLRequest'])}"
+        query_string << "&RelayState=#{CGI.escape(params[:RelayState])}"
+        query_string << "&SigAlg=#{CGI.escape(params['SigAlg'])}"
+
+        signature_algorithm = XMLSecurity::BaseDocument.new.algorithm(params['SigAlg'])
+        assert_equal signature_algorithm, OpenSSL::Digest::SHA1
+        assert cert.public_key.verify(signature_algorithm.new, Base64.decode64(params['Signature']), query_string)
+      end
+
+      it "raises error when no valid certs and :check_sp_cert_expiration is true" do
+        settings.security[:check_sp_cert_expiration] = true
+
+        assert_raises(OneLogin::RubySaml::ValidationError, 'The SP certificate expired.') do
+          OneLogin::RubySaml::Authrequest.new.create_params(settings, :RelayState => 'http://example.com')
+        end
+      end
     end
 
     it "create the saml:AuthnContextClassRef element correctly" do

diff --git a/test/response_test.rb b/test/response_test.rb
@@ -1374,7 +1374,7 @@ def generate_audience_error(expected, actual)
     end
 
     describe '#sign_document' do
-      it 'Sign an unsigned SAML Response XML and initiate the SAML object with it' do
+      it 'sign an unsigned SAML Response XML and initiate the SAML object with it' do
         xml = Base64.decode64(fixture("test_sign.xml"))
 
         document = XMLSecurity::Document.new(xml)
@@ -1404,11 +1404,9 @@ def generate_audience_error(expected, actual)
         @no_signed_assertion = OneLogin::RubySaml::Response.new(response_document_valid_signed, :settings => settings)
       end
 
-
       it 'returns false if :want_assertion_signed enabled and Assertion not signed' do
         assert !@no_signed_assertion.send(:validate_signed_elements)
         assert_includes @no_signed_assertion.errors, "The Assertion of the Response is not signed and the SP requires it"
-
       end
 
       it 'returns true if :want_assertion_signed enabled and Assertion is signed' do

diff --git a/test/slo_logoutrequest_test.rb b/test/slo_logoutrequest_test.rb
@@ -281,11 +281,13 @@ class RubySamlTest < Minitest::Test
         logout_request.settings.idp_entity_id = 'https://app.onelogin.com/saml/metadata/SOMEACCOUNT'
         assert logout_request.send(:validate_issuer)
       end
+
       it "return false when the issuer of the Logout Request does not match the IdP entityId" do
         logout_request.settings.idp_entity_id = 'http://idp.example.com/invalid'
         assert !logout_request.send(:validate_issuer)
         assert_includes logout_request.errors, "Doesn't match the issuer, expected: <#{logout_request.settings.idp_entity_id}>, but was: <https://app.onelogin.com/saml/metadata/SOMEACCOUNT>"
       end
+
       it "raise when the issuer of the Logout Request does not match the IdP entityId" do
         logout_request.settings.idp_entity_id = 'http://idp.example.com/invalid'
         logout_request.soft = false
@@ -552,7 +554,6 @@ class RubySamlTest < Minitest::Test
         logout_request_sign_test.settings = settings
         assert !logout_request_sign_test.send(:validate_signature)
         assert_includes logout_request_sign_test.errors, "Invalid Signature on Logout Request"
-
       end
     end
   end