Refactor error handling; allow multiple error messages with soft

Author: vincentwoo

lib/onelogin/ruby-saml/error_handling.rb

@@ -0,0 +1,27 @@
+require "onelogin/ruby-saml/validation_error"
+
+module OneLogin
+  module RubySaml
+    module ErrorHandling
+      attr_accessor :errors
+
+      # Append the cause to the errors array, and based on the value of soft, return false or raise
+      # an exception. soft_override is provided as a means of overriding the object's notion of
+      # soft for just this invocation.
+      def append_error(error_msg, soft_override = nil)
+        @errors << error_msg
+
+        unless soft_override.nil? ? soft : soft_override
+          raise ValidationError.new(error_msg)
+        end
+
+        false
+      end
+
+      # Reset the errors array
+      def reset_errors!
+        @errors = []
+      end
+    end
+  end
+end

lib/onelogin/ruby-saml/logoutresponse.rb

@@ -1,5 +1,6 @@
 require "xml_security"
 require "onelogin/ruby-saml/saml_message"
+require "onelogin/ruby-saml/error_handling"
 
 require "time"
 
@@ -10,13 +11,11 @@ module RubySaml
     # SAML2 Logout Response (SLO IdP initiated, Parser)
     #
     class Logoutresponse < SamlMessage
+      include ErrorHandling
 
       # OneLogin::RubySaml::Settings Toolkit settings
       attr_accessor :settings
 
-      # Array with the causes
-      attr_accessor :errors
-
       attr_reader :document
       attr_reader :response
       attr_reader :options
@@ -47,18 +46,6 @@ def initialize(response, settings = nil, options = {})
         @document = XMLSecurity::SignedDocument.new(@response)
       end
 
-      # Append the cause to the errors array, and based on the value of soft, return false or raise
-      # an exception
-      def append_error(error_msg)
-        @errors << error_msg
-        return soft ? false : validation_error(error_msg)
-      end
-
-      # Reset the errors array
-      def reset_errors!
-        @errors = []
-      end
-
       # Checks if the Status has the "Success" code
       # @return [Boolean] True if the StatusCode is Sucess
       # @raise [ValidationError] if soft == false and validation fails
@@ -123,12 +110,14 @@ def status_message
       def validate
         reset_errors!
 
-        valid_state? &&
-        validate_success_status &&
-        validate_structure &&
-        valid_in_response_to? &&
-        valid_issuer? &&
+        valid_state?
+        validate_success_status
+        validate_structure
+        valid_in_response_to?
+        valid_issuer?
         validate_signature
+
+        @errors.empty?
       end
 
       private

lib/onelogin/ruby-saml/response.rb

@@ -11,6 +11,8 @@ module RubySaml
     # SAML2 Authentication Response. SAML Response
     #
     class Response < SamlMessage
+      include ErrorHandling
+
       ASSERTION = "urn:oasis:names:tc:SAML:2.0:assertion"
       PROTOCOL  = "urn:oasis:names:tc:SAML:2.0:protocol"
       DSIG      = "http://www.w3.org/2000/09/xmldsig#"
@@ -21,9 +23,6 @@ class Response < SamlMessage
       # OneLogin::RubySaml::Settings Toolkit settings
       attr_accessor :settings
 
-      # Array with the causes [Array of strings]
-      attr_accessor :errors
-
       attr_reader :document
       attr_reader :decrypted_document
       attr_reader :response
@@ -39,16 +38,15 @@ class Response < SamlMessage
       #                          or :matches_request_id that will validate that the response matches the ID of the request,
       #                          or skip the subject confirmation validation with the :skip_subject_confirmation option
       def initialize(response, options = {})
-        @errors = []
-
         raise ArgumentError.new("Response cannot be nil") if response.nil?
-        @options = options
 
+        @errors = []
+        @options = options
         @soft = true
-        if !options.empty? && !options[:settings].nil?
+        unless options[:settings].nil?
           @settings = options[:settings]
-          if !options[:settings].soft.nil? 
-            @soft = options[:settings].soft
+          unless @settings.soft.nil?
+            @soft = @settings.soft
           end
         end
 
@@ -60,18 +58,6 @@ def initialize(response, options = {})
         end
       end
 
-      # Append the cause to the errors array, and based on the value of soft, return false or raise
-      # an exception
-      def append_error(error_msg)
-        @errors << error_msg
-        return soft ? false : validation_error(error_msg)
-      end
-
-      # Reset the errors array
-      def reset_errors!
-        @errors = []
-      end
-
       # Validates the SAML Response with the default values (soft = true)
       # @return [Boolean] TRUE if the SAML Response is valid
       #
@@ -284,21 +270,23 @@ def allowed_clock_drift
       def validate
         reset_errors!
 
-        validate_response_state &&
-        validate_version &&
-        validate_id &&
-        validate_success_status &&
-        validate_num_assertion &&
-        validate_no_encrypted_attributes &&
-        validate_signed_elements &&
-        validate_structure &&
-        validate_in_response_to &&
-        validate_conditions &&
-        validate_audience &&
-        validate_issuer &&
-        validate_session_expiration &&
-        validate_subject_confirmation &&
+        return false unless validate_response_state
+        validate_version
+        validate_id
+        validate_success_status
+        validate_num_assertion
+        validate_no_encrypted_attributes
+        validate_signed_elements
+        validate_structure
+        validate_in_response_to
+        validate_conditions
+        validate_audience
+        validate_issuer
+        validate_session_expiration
+        validate_subject_confirmation
         validate_signature
+
+        @errors.empty?
       end
 
 
@@ -585,9 +573,8 @@ def validate_signature
         )
         doc = (response_signed || decrypted_document.nil?) ? document : decrypted_document
 
-        unless fingerprint && doc.validate_document(fingerprint, :fingerprint_alg => settings.idp_cert_fingerprint_algorithm)
-          error_msg = "Invalid Signature on SAML Response"
-          return append_error(error_msg)
+        unless fingerprint && doc.validate_document(fingerprint, true, :fingerprint_alg => settings.idp_cert_fingerprint_algorithm)
+          return append_error("Invalid Signature on SAML Response")
         end
 
         true
@@ -641,7 +628,7 @@ def xpath_from_signed_assertion(subelt=nil)
       #
       def generate_decrypted_document
         if settings.nil? || !settings.get_sp_key
-          validation_error('An EncryptedAssertion found and no SP private key found on the settings to decrypt it. Be sure you provided the :settings parameter at the initialize method')
+          raise ValidationError.new('An EncryptedAssertion found and no SP private key found on the settings to decrypt it. Be sure you provided the :settings parameter at the initialize method')
         end
 
         # Marshal at Ruby 1.8.7 throw an Exception
@@ -707,7 +694,7 @@ def decrypt_nameid(encryptedid_node)
       #
       def decrypt_element(encrypt_node, rgrex)
         if settings.nil? || !settings.get_sp_key
-          return validation_error('An ' + encrypt_node.name + ' found and no SP private key found on the settings to decrypt it')
+          raise ValidationError.new('An ' + encrypt_node.name + ' found and no SP private key found on the settings to decrypt it')
         end
 
         elem_plaintext = OneLogin::RubySaml::Utils.decrypt_data(encrypt_node, settings.get_sp_key)

lib/onelogin/ruby-saml/saml_message.rb

@@ -69,23 +69,15 @@ def valid_saml?(document, soft = true)
           end
         rescue Exception => error
           return false if soft
-          validation_error("XML load failed: #{error.message}")
+          raise ValidationError.new("XML load failed: #{error.message}")
         end
 
         SamlMessage.schema.validate(xml).map do |error|
           return false if soft
-          validation_error("#{error.message}\n\n#{xml.to_s}")
+          raise ValidationError.new("#{error.message}\n\n#{xml.to_s}")
         end
       end
 
-      # Raise a ValidationError with the provided message
-      # @param message [String] Message of the exception
-      # @raise [ValidationError]
-      #
-      def validation_error(message)
-        raise ValidationError.new(message)
-      end
-
       private
 
       # Base64 decode and try also to inflate a SAML Message

lib/onelogin/ruby-saml/slo_logoutrequest.rb

@@ -11,13 +11,11 @@ module RubySaml
     # SAML2 Logout Request (SLO IdP initiated, Parser)
     #
     class SloLogoutrequest < SamlMessage
+      include ErrorHandling
 
       # OneLogin::RubySaml::Settings Toolkit settings
       attr_accessor :settings
 
-      # Array with the causes [Array of strings]
-      attr_accessor :errors
-
       attr_reader :document
       attr_reader :request
       attr_reader :options
@@ -32,34 +30,22 @@ class SloLogoutrequest < SamlMessage
       # @raise [ArgumentError] If Request is nil
       #
       def initialize(request, options = {})
-        @errors = []
         raise ArgumentError.new("Request cannot be nil") if request.nil?
-        @options  = options
 
+        @errors = []
+        @options = options
         @soft = true
-        if !options.empty? && !options[:settings].nil?
+        unless options[:settings].nil?
           @settings = options[:settings]
-          if !options[:settings].soft.nil? 
-            @soft = options[:settings].soft
+          unless @settings.soft.nil?
+            @soft = @settings.soft
           end
         end
 
         @request = decode_raw_saml(request)
         @document = REXML::Document.new(@request)
       end
 
-      # Append the cause to the errors array, and based on the value of soft, return false or raise
-      # an exception
-      def append_error(error_msg)
-        @errors << error_msg
-        return soft ? false : validation_error(error_msg)
-      end
-
-      # Reset the errors array
-      def reset_errors!
-        @errors = []
-      end
-
       # Validates the Logout Request with the default values (soft = true)
       # @return [Boolean] TRUE if the Logout Request is valid
       #
@@ -138,13 +124,15 @@ def session_indexes
       def validate
         reset_errors!
 
-        validate_request_state &&
-        validate_id &&
-        validate_version &&        
-        validate_structure &&
-        validate_not_on_or_after &&
-        validate_issuer &&
+        validate_request_state
+        validate_id
+        validate_version
+        validate_structure
+        validate_not_on_or_after
+        validate_issuer
         validate_signature
+
+        @errors.empty?
       end
 
       # Validates that the Logout Request contains an ID
@@ -213,7 +201,7 @@ def validate_request_state
       # @raise [ValidationError] if soft == false and validation fails
       #
       def validate_issuer
-        return true if settings.idp_entity_id.nil? || issuer.nil?
+        return true if settings.nil? || settings.idp_entity_id.nil? || issuer.nil?
 
         unless URI.parse(issuer) == URI.parse(settings.idp_entity_id)
           return append_error("Doesn't match the issuer, expected: <#{settings.idp_entity_id}>, but was: <#{issuer}>")
@@ -230,7 +218,7 @@ def validate_signature
         return true if options.nil?
         return true unless options.has_key? :get_params
         return true unless options[:get_params].has_key? 'Signature'
-        return true if settings.nil? || settings.get_idp_cert.nil?
+        return true if settings.get_idp_cert.nil?
 
         query_string = OneLogin::RubySaml::Utils.build_query(
           :type        => 'SAMLRequest',