Merge pull request #98 from onelogin/error_messages

Error reporting for diagnostics

Author: Rob Fletcher

lib/onelogin/ruby-saml/response.rb

@@ -13,16 +13,18 @@ class Response
 
       # TODO: This should probably be ctor initialized too... WDYT?
       attr_accessor :settings
+      attr_accessor :errors
 
       attr_reader :options
       attr_reader :response
       attr_reader :document
 
       def initialize(response, options = {})
+        @errors = []
         raise ArgumentError.new("Response cannot be nil") if response.nil?
         @options  = options
         @response = (response =~ /^</) ? response : Base64.decode64(response)
-        @document = XMLSecurity::SignedDocument.new(@response)
+        @document = XMLSecurity::SignedDocument.new(@response, @errors)
       end
 
       def is_valid?
@@ -33,6 +35,10 @@ def validate!
         validate(false)
       end
 
+      def errors
+        @errors
+      end
+
       # The value of the user identifier as designated by the initialization request response
       def name_id
         @name_id ||= begin
@@ -153,9 +159,14 @@ def validate_structure(soft = true)
           @xml = Nokogiri::XML(self.document.to_s)
         end
         if soft
-          @schema.validate(@xml).map{ return false }
+          @schema.validate(@xml).map{
+            @errors << "Schema validation failed";
+            return false 
+          }
         else
-          @schema.validate(@xml).map{ |error| validation_error("#{error.message}\n\n#{@xml.to_s}") }
+          @schema.validate(@xml).map{ |error| @errors << "#{error.message}\n\n#{@xml.to_s}";
+            validation_error("#{error.message}\n\n#{@xml.to_s}")
+          }
         end
       end
 
@@ -197,10 +208,12 @@ def validate_conditions(soft = true)
         now = Time.now.utc
 
         if not_before && (now + (options[:allowed_clock_drift] || 0)) < not_before
+          @errors << "Current time is earlier than NotBefore condition #{(now + (options[:allowed_clock_drift] || 0))} < #{not_before})"
           return soft ? false : validation_error("Current time is earlier than NotBefore condition")
         end
 
         if not_on_or_after && now >= not_on_or_after
+          @errors << "Current time is on or after NotOnOrAfter condition (#{now} >= #{not_on_or_after})"
           return soft ? false : validation_error("Current time is on or after NotOnOrAfter condition")
         end
 

lib/xml_security.rb

@@ -38,9 +38,11 @@ class SignedDocument < REXML::Document
     DSIG = "http://www.w3.org/2000/09/xmldsig#"
 
     attr_accessor :signed_element_id
+    attr_accessor :errors
 
-    def initialize(response)
+    def initialize(response, errors = [])
       super(response)
+      @errors = errors
       extract_signed_element_id
     end
 
@@ -62,6 +64,7 @@ def validate_document(idp_cert_fingerprint, soft = true)
       fingerprint = Digest::SHA1.hexdigest(cert.to_der)
 
       if fingerprint != idp_cert_fingerprint.gsub(/[^a-zA-Z0-9]/,"").downcase
+        @errors << "Fingerprint mismatch"
         return soft ? false : (raise OneLogin::RubySaml::ValidationError.new("Fingerprint mismatch"))
       end
 
@@ -108,6 +111,7 @@ def validate_signature(base64_cert, soft = true)
         digest_value                  = Base64.decode64(REXML::XPath.first(ref, "//ds:DigestValue", {"ds"=>DSIG}).text)
 
         unless digests_match?(hash, digest_value)
+          @errors << "Digest mismatch"
           return soft ? false : (raise OneLogin::RubySaml::ValidationError.new("Digest mismatch"))
         end
       end
@@ -123,6 +127,7 @@ def validate_signature(base64_cert, soft = true)
       signature_algorithm     = algorithm(REXML::XPath.first(signed_info_element, "//ds:SignatureMethod", {"ds"=>DSIG}))
 
       unless cert.public_key.verify(signature_algorithm.new, signature, canon_string)
+        @errors << "Key validation error"
         return soft ? false : (raise OneLogin::RubySaml::ValidationError.new("Key validation error"))
       end
 

test/response_test.rb

@@ -53,6 +53,14 @@ class RubySamlTest < Test::Unit::TestCase
       end
     end
 
+    context "#validate_structure" do
+      should "raise when encountering a condition that prevents the document from being valid" do
+        response = OneLogin::RubySaml::Response.new(response_document_2)
+        response.send(:validate_structure)
+        assert response.errors.include? "Schema validation failed"
+      end
+    end
+
     context "#is_valid?" do
       should "return false when response is initialized with blank data" do
         response = OneLogin::RubySaml::Response.new('')

test/xml_security_test.rb

@@ -40,13 +40,15 @@ class XmlSecurityTest < Test::Unit::TestCase
         @document.validate_document("no:fi:ng:er:pr:in:t", false)
       end
       assert_equal("Fingerprint mismatch", exception.message)
+      assert @document.errors.include? "Fingerprint mismatch"
     end
 
     should "should raise Digest mismatch" do
       exception = assert_raise(OneLogin::RubySaml::ValidationError) do
         @document.validate_signature(@base64cert, false)
       end
       assert_equal("Digest mismatch", exception.message)
+      assert @document.errors.include? "Digest mismatch"
     end
 
     should "should raise Key validation error" do
@@ -59,6 +61,7 @@ class XmlSecurityTest < Test::Unit::TestCase
         document.validate_signature(base64cert, false)
       end
       assert_equal("Key validation error", exception.message)
+      assert document.errors.include? "Key validation error"
     end
 
     should "correctly obtain the digest method with alternate namespace declaration" do