Minor refactor

Author: pitbulk

README.md

@@ -24,7 +24,7 @@ options = {
   },
 }
 slo_logout_request = OneLogin::RubySaml::SloLogoutrequest.new(query_params["SAMLRequest"], settings, options)
-raise "Uh oh!" unless slo_logout_request.is_valid?
+raise "Invalid Logout Request" unless slo_logout_request.is_valid?
 ```
 
 The old form is still supported for backward compatibility, but all Ruby SAML users should prefer `options[:raw_get_params]` where possible to ensure compatibility with other SAML implementations.

lib/onelogin/ruby-saml/logoutresponse.rb

@@ -211,32 +211,8 @@ def validate_signature
         return true unless options.has_key? :get_params
         return true unless options[:get_params].has_key? 'Signature'
 
-        # SAML specifies that the signature should be derived from a concatenation
-        # of URI-encoded values _as sent by the IDP_:
-        #
-        # > Further, note that URL-encoding is not canonical; that is, there are multiple legal encodings for a given
-        # > value. The relying party MUST therefore perform the verification step using the original URL-encoded
-        # > values it received on the query string. It is not sufficient to re-encode the parameters after they have been
-        # > processed by software because the resulting encoding may not match the signer's encoding.
-        #
-        # <http://docs.oasis-open.org/security/saml/v2.0/saml-bindings-2.0-os.pdf>
-        #
-        # If we don't have the original parts (for backward compatibility) required to correctly verify the signature,
-        # then fabricate them by re-encoding the parsed URI parameters, and hope that we're lucky enough to use
-        # the exact same URI-encoding as the IDP. (This is not the case if the IDP is ADFS!)
-        options[:raw_get_params] ||= {}
-        if options[:raw_get_params]['SAMLResponse'].nil? && !options[:get_params]['SAMLResponse'].nil?
-          options[:raw_get_params]['SAMLResponse'] = CGI.escape(options[:get_params]['SAMLResponse'])
-        end
-        if options[:raw_get_params]['RelayState'].nil? && !options[:get_params]['RelayState'].nil?
-          options[:raw_get_params]['RelayState'] = CGI.escape(options[:get_params]['RelayState'])
-        end
-        if options[:raw_get_params]['SigAlg'].nil? && !options[:get_params]['SigAlg'].nil?
-          options[:raw_get_params]['SigAlg'] = CGI.escape(options[:get_params]['SigAlg'])
-        end
+        options[:raw_get_params] = OneLogin::RubySaml::Utils.prepare_raw_get_params(options[:raw_get_params], options[:get_params])
 
-        # If we only received the raw version of SigAlg,
-        # then parse it back into the decoded params hash for convenience.
         if options[:get_params]['SigAlg'].nil? && !options[:raw_get_params]['SigAlg'].nil?
           options[:get_params]['SigAlg'] = CGI.unescape(options[:raw_get_params]['SigAlg'])
         end

lib/onelogin/ruby-saml/utils.rb

@@ -76,7 +76,6 @@ def self.build_query(params)
       end
 
       # Reconstruct a canonical query string from raw URI-encoded parts, to be used in verifying a signature
-      # sent by an IDP.
       #
       # @param params [Hash] Parameters to build the Query String
       # @option params [String] :type 'SAMLRequest' or 'SAMLResponse'
@@ -93,6 +92,32 @@ def self.build_query_from_raw_parts(params)
         url_string << "&SigAlg=#{raw_sig_alg}"
       end
 
+      # Prepare raw GET parameters (build them from normal parameters
+      # if not provided). 
+      #
+      # @param rawparams [Hash] Raw GET Parameters
+      # @param params [Hash] GET Parameters
+      # @return [Hash] New raw parameters
+      # 
+      def self.prepare_raw_get_params(rawparams, params)
+        rawparams ||= {}
+
+        if rawparams['SAMLRequest'].nil? && !params['SAMLRequest'].nil?
+          rawparams['SAMLRequest'] = CGI.escape(params['SAMLRequest'])
+        end
+        if rawparams['SAMLResponse'].nil? && !params['SAMLResponse'].nil?
+          rawparams['SAMLResponse'] = CGI.escape(params['SAMLResponse'])
+        end        
+        if rawparams['RelayState'].nil? && !params['RelayState'].nil?
+          rawparams['RelayState'] = CGI.escape(params['RelayState'])
+        end
+        if rawparams['SigAlg'].nil? && !params['SigAlg'].nil?
+          rawparams['SigAlg'] = CGI.escape(params['SigAlg'])
+        end
+
+        rawparams
+      end
+
       # Validate the Signature parameter sent on the HTTP-Redirect binding
       # @param params [Hash] Parameters to be used in the validation process
       # @option params [OpenSSL::X509::Certificate] cert The Identity provider public certtificate