More specific error messages for request certificate validation

lib/onelogin/ruby-saml/response.rb

@@ -863,8 +863,12 @@ def validate_signature
           valid = false
           expired = false
           idp_certs[:signing].each do |idp_cert|
-            valid = doc.validate_document_with_cert(idp_cert)
+            valid = doc.validate_document_with_cert(idp_cert, true)
             if valid
+              # required to reset errors as there could be issues with previous certificates
+              doc.reset_errors!
+              reset_errors!
+
               if settings.security[:check_idp_cert_expiration]
                 if OneLogin::RubySaml::Utils.is_cert_expired(idp_cert)
                   expired = true

lib/xml_security.rb

@@ -241,7 +241,7 @@ def validate_document(idp_cert_fingerprint, soft = true, options = {})
       validate_signature(base64_cert, soft)
     end
 
-    def validate_document_with_cert(idp_cert)
+    def validate_document_with_cert(idp_cert, soft = true)
       # get cert from response
       cert_element = REXML::XPath.first(
         self,
@@ -255,12 +255,12 @@ def validate_document_with_cert(idp_cert)
         begin
           cert = OpenSSL::X509::Certificate.new(cert_text)
         rescue OpenSSL::X509::CertificateError => _e
-          return append_error("Certificate Error", soft)
+          return append_error("Document certificate error", soft)
         end
 
         # check saml response cert matches provided idp cert
         if idp_cert.to_pem != cert.to_pem
-          return false
+          return append_error("Document certificate does not match idp certificate", soft)
         end
       else
         base64_cert = Base64.encode64(idp_cert.to_pem)

test/xml_security_test.rb

@@ -395,25 +395,51 @@ class XmlSecurityTest < Minitest::Test
     end
 
     describe '#validate_document_with_cert' do
+      let(:document_data) { read_response('response_with_signed_message_and_assertion.xml') }
+      let(:document) { OneLogin::RubySaml::Response.new(document_data).document }
+      let(:idp_cert) { OpenSSL::X509::Certificate.new(ruby_saml_cert_text) }
+      let(:fingerprint) { '4b68c453c7d994aad9025c99d5efcf566287fe8d' }
+
       describe 'with valid document ' do
-        describe 'when response has cert' do
-          let(:document_data) { read_response('response_with_signed_message_and_assertion.xml') }
-          let(:document) { OneLogin::RubySaml::Response.new(document_data).document }
-          let(:idp_cert) { OpenSSL::X509::Certificate.new(ruby_saml_cert_text) }
-          let(:fingerprint) { '4b68c453c7d994aad9025c99d5efcf566287fe8d' }
+        it 'is valid' do
+          assert document.validate_document_with_cert(idp_cert), 'Document should be valid'
+        end
+      end
 
-          it 'is valid' do
-            assert document.validate_document_with_cert(idp_cert), 'Document should be valid'
-          end
+      describe 'when response has no cert but you have local cert' do
+        let(:document_data) { response_document_valid_signed_without_x509certificate }
+
+        it 'is valid' do
+          assert document.validate_document_with_cert(idp_cert), 'Document should be valid'
         end
-
-        describe 'when response has no cert but you have local cert' do
-          let(:document) { OneLogin::RubySaml::Response.new(response_document_valid_signed_without_x509certificate).document }
-          let(:idp_cert) { OpenSSL::X509::Certificate.new(ruby_saml_cert_text) }
+      end
 
-          it 'is valid' do
-            assert document.validate_document_with_cert(idp_cert), 'Document should be valid'
+      describe 'when response cert is invalid' do
+        let(:document_data) do
+          contents = read_response('response_with_signed_message_and_assertion.xml')
+          contents.sub(/<ds:X509Certificate>.*<\/ds:X509Certificate>/,
+                       "<ds:X509Certificate>an-invalid-certificate</ds:X509Certificate>")
+        end
+
+        it 'is not valid' do
+          assert !document.validate_document_with_cert(idp_cert), 'Document should be valid'
+          assert_equal(["Document certificate error"], document.errors)
+        end
+      end
+
+      describe 'when response cert is different from idp cert' do
+        let(:idp_cert) { OpenSSL::X509::Certificate.new(ruby_saml_cert_text2) }
+
+        it 'is not valid' do
+          exception = assert_raises(OneLogin::RubySaml::ValidationError) do
+            document.validate_document_with_cert(idp_cert, false)
           end
+          assert_equal("Document certificate does not match idp certificate", exception.message)
+        end
+
+        it 'is not valid (soft = true)' do
+          document.validate_document_with_cert(idp_cert)
+          assert_equal(["Document certificate does not match idp certificate"], document.errors)
         end
       end
     end