Merge pull request #458 from davidlibrera/refactor/warnings

Remove ruby 2.4+ warnings

Author: pitbulk

.travis.yml

@@ -8,6 +8,7 @@ rvm:
   - 2.2.0
   - 2.3.0
   - 2.4.0
+  - 2.5.0
   - ree
   - jruby-1.7.21
   - jruby-9.0.0.0

lib/onelogin/ruby-saml/attribute_service.rb

@@ -16,7 +16,7 @@ def initialize
       end
 
       def configure(&block)
-        instance_eval &block
+        instance_eval(&block)
       end
 
       # @return [Boolean] True if the AttributeService object has been initialized and set with the required values

lib/onelogin/ruby-saml/logging.rb

@@ -7,7 +7,10 @@ class Logging
       DEFAULT_LOGGER = ::Logger.new(STDOUT)
 
       def self.logger
-        @logger || (defined?(::Rails) && Rails.respond_to?(:logger) && Rails.logger) || DEFAULT_LOGGER
+        @logger ||= begin
+                      (defined?(::Rails) && Rails.respond_to?(:logger) && Rails.logger) ||
+                        DEFAULT_LOGGER
+                    end
       end
 
       def self.logger=(logger)

lib/onelogin/ruby-saml/logoutresponse.rb

@@ -24,7 +24,7 @@ class Logoutresponse < SamlMessage
       # Constructs the Logout Response. A Logout Response Object that is an extension of the SamlMessage class.
       # @param response  [String] A UUEncoded logout response from the IdP.
       # @param settings  [OneLogin::RubySaml::Settings|nil] Toolkit settings
-      # @param options   [Hash] Extra parameters. 
+      # @param options   [Hash] Extra parameters.
       #                    :matches_request_id It will validate that the logout response matches the ID of the request.
       #                    :get_params GET Parameters, including the SAMLResponse
       #                    :relax_signature_validation to accept signatures if no idp certificate registered on settings
@@ -50,7 +50,7 @@ def initialize(response, settings = nil, options = {})
       # Checks if the Status has the "Success" code
       # @return [Boolean] True if the StatusCode is Sucess
       # @raise [ValidationError] if soft == false and validation fails
-      # 
+      #
       def success?
         unless status_code == "urn:oasis:names:tc:SAML:2.0:status:Success"
           return append_error("Bad status code. Expected <urn:oasis:names:tc:SAML:2.0:status:Success>, but was: <#@status_code>")
@@ -146,7 +146,7 @@ def validate_success_status
 
       # Validates the Logout Response against the specified schema.
       # @return [Boolean] True if the XML is valid, otherwise False if soft=True
-      # @raise [ValidationError] if soft == false and validation fails 
+      # @raise [ValidationError] if soft == false and validation fails
       #
       def validate_structure
         unless valid_saml?(document, soft)
@@ -205,7 +205,7 @@ def valid_issuer?
       # Validates the Signature if it exists and the GET parameters are provided
       # @return [Boolean] True if not contains a Signature or if the Signature is valid, otherwise False if soft=True
       # @raise [ValidationError] if soft == false and validation fails
-      #      
+      #
       def validate_signature
         return true unless !options.nil?
         return true unless options.has_key? :get_params
@@ -240,9 +240,9 @@ def validate_signature
           )
         else
           valid = false
-          idp_certs[:signing].each do |idp_cert|
+          idp_certs[:signing].each do |signing_idp_cert|
             valid = OneLogin::RubySaml::Utils.verify_signature(
-              :cert         => idp_cert,
+              :cert         => signing_idp_cert,
               :sig_alg      => options[:get_params]['SigAlg'],
               :signature    => options[:get_params]['Signature'],
               :query_string => query_string
@@ -257,7 +257,7 @@ def validate_signature
           error_msg = "Invalid Signature on Logout Response"
           return append_error(error_msg)
         end
-        true        
+        true
       end
 
     end

lib/onelogin/ruby-saml/response.rb

@@ -171,9 +171,10 @@ def attributes
                 # identify the subject in an SP rather than email or other less opaque attributes
                 # NameQualifier, if present is prefixed with a "/" to the value
                 else
-                 REXML::XPath.match(e,'a:NameID', { "a" => ASSERTION }).collect{|n|
-                    (n.attributes['NameQualifier'] ? n.attributes['NameQualifier'] +"/" : '') + Utils.element_text(n)
-                  }
+                  REXML::XPath.match(e,'a:NameID', { "a" => ASSERTION }).collect do |n|
+                    base_path = n.attributes['NameQualifier'] ? "#{n.attributes['NameQualifier']}/" : ''
+                    base_path + Utils.element_text(n)
+                  end
                 end
               }
 
@@ -221,8 +222,8 @@ def status_code
                 "/p:Response/p:Status/p:StatusCode/p:StatusCode",
                 { "p" => PROTOCOL }
               )
-              statuses = nodes.collect do |node|
-                node.attributes["Value"]
+              statuses = nodes.collect do |inner_node|
+                inner_node.attributes["Value"]
               end
               extra_code = statuses.join(" | ")
               if extra_code
@@ -288,7 +289,6 @@ def issuers
             raise ValidationError.new(error_msg)
           end
 
-          doc = decrypted_document.nil? ? document : decrypted_document
           issuer_assertion_nodes = xpath_from_signed_assertion("/a:Issuer")
           unless issuer_assertion_nodes.size == 1
             error_msg = "Issuer of the Assertion not found or multiple."

lib/onelogin/ruby-saml/saml_message.rb

@@ -62,7 +62,7 @@ def id(document)
       # @param document [REXML::Document] The message that will be validated
       # @param soft [Boolean] soft Enable or Disable the soft mode (In order to raise exceptions when the message is invalid or not)
       # @return [Boolean] True if the XML is valid, otherwise False, if soft=True
-      # @raise [ValidationError] if soft == false and validation fails 
+      # @raise [ValidationError] if soft == false and validation fails
       #
       def valid_saml?(document, soft = true)
         begin
@@ -74,9 +74,9 @@ def valid_saml?(document, soft = true)
           raise ValidationError.new("XML load failed: #{error.message}")
         end
 
-        SamlMessage.schema.validate(xml).map do |error|
+        SamlMessage.schema.validate(xml).map do |schema_error|
           return false if soft
-          raise ValidationError.new("#{error.message}\n\n#{xml.to_s}")
+          raise ValidationError.new("#{schema_error.message}\n\n#{xml.to_s}")
         end
       end
 

lib/onelogin/ruby-saml/slo_logoutrequest.rb

@@ -24,9 +24,9 @@ class SloLogoutrequest < SamlMessage
 
       # Constructs the Logout Request. A Logout Request Object that is an extension of the SamlMessage class.
       # @param request [String] A UUEncoded Logout Request from the IdP.
-      # @param options [Hash]  :settings to provide the OneLogin::RubySaml::Settings object 
+      # @param options [Hash]  :settings to provide the OneLogin::RubySaml::Settings object
       #                        Or :allowed_clock_drift for the logout request validation process to allow a clock drift when checking dates with
-      #                        Or :relax_signature_validation to accept signatures if no idp certificate registered on settings 
+      #                        Or :relax_signature_validation to accept signatures if no idp certificate registered on settings
       #
       # @raise [ArgumentError] If Request is nil
       #
@@ -192,7 +192,7 @@ def validate_not_on_or_after
 
       # Validates the Logout Request against the specified schema.
       # @return [Boolean] True if the XML is valid, otherwise False if soft=True
-      # @raise [ValidationError] if soft == false and validation fails 
+      # @raise [ValidationError] if soft == false and validation fails
       #
       def validate_structure
         unless valid_saml?(document, soft)
@@ -202,7 +202,7 @@ def validate_structure
         true
       end
 
-      # Validates that the Logout Request provided in the initialization is not empty, 
+      # Validates that the Logout Request provided in the initialization is not empty,
       # @return [Boolean] True if the required info is found, otherwise False if soft=True
       # @raise [ValidationError] if soft == false and validation fails
       #
@@ -289,9 +289,9 @@ def validate_signature
           )
         else
           valid = false
-          idp_certs[:signing].each do |idp_cert|
+          idp_certs[:signing].each do |signing_idp_cert|
             valid = OneLogin::RubySaml::Utils.verify_signature(
-              :cert         => idp_cert,
+              :cert         => signing_idp_cert,
               :sig_alg      => options[:get_params]['SigAlg'],
               :signature    => options[:get_params]['Signature'],
               :query_string => query_string

lib/onelogin/ruby-saml/utils.rb

@@ -32,7 +32,9 @@ def self.format_cert(cert)
           formatted_cert.join("\n")
         else
           cert = cert.gsub(/\-{5}\s?(BEGIN|END) CERTIFICATE\s?\-{5}/, "")
-          cert = cert.gsub(/[\n\r\s]/, "")
+          cert = cert.gsub(/\r/, "")
+          cert = cert.gsub(/\n/, "")
+          cert = cert.gsub(/\s/, "")
           cert = cert.scan(/.{1,64}/)
           cert = cert.join("\n")
           "-----BEGIN CERTIFICATE-----\n#{cert}\n-----END CERTIFICATE-----"
@@ -51,7 +53,9 @@ def self.format_private_key(key)
         # is this an rsa key?
         rsa_key = key.match("RSA PRIVATE KEY")
         key = key.gsub(/\-{5}\s?(BEGIN|END)( RSA)? PRIVATE KEY\s?\-{5}/, "")
-        key = key.gsub(/[\n\r\s]/, "")
+        key = key.gsub(/\n/, "")
+        key = key.gsub(/\r/, "")
+        key = key.gsub(/\s/, "")
         key = key.scan(/.{1,64}/)
         key = key.join("\n")
         key_label = rsa_key ? "RSA PRIVATE KEY" : "PRIVATE KEY"
@@ -98,7 +102,7 @@ def self.build_query_from_raw_parts(params)
       # @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 ||= {}
 
@@ -107,7 +111,7 @@ def self.prepare_raw_get_params(rawparams, params)
         end
         if rawparams['SAMLResponse'].nil? && !params['SAMLResponse'].nil?
           rawparams['SAMLResponse'] = CGI.escape(params['SAMLResponse'])
-        end        
+        end
         if rawparams['RelayState'].nil? && !params['RelayState'].nil?
           rawparams['RelayState'] = CGI.escape(params['RelayState'])
         end
@@ -136,16 +140,16 @@ def self.verify_signature(params)
       # @param status_code [String] StatusCode value
       # @param status_message [Strig] StatusMessage value
       # @return [String] The status error message
-      def self.status_error_msg(error_msg, status_code = nil, status_message = nil)
-        unless status_code.nil?
-          if status_code.include? "|"
-            status_codes = status_code.split(' | ')
+      def self.status_error_msg(error_msg, raw_status_code = nil, status_message = nil)
+        unless raw_status_code.nil?
+          if raw_status_code.include? "|"
+            status_codes = raw_status_code.split(' | ')
             values = status_codes.collect do |status_code|
               status_code.split(':').last
             end
             printable_code = values.join(" => ")
           else
-            printable_code = status_code.split(':').last
+            printable_code = raw_status_code.split(':').last
           end
           error_msg << ', was ' + printable_code
         end

lib/xml_security.rb

@@ -91,7 +91,7 @@ class Document < BaseDocument
     ENVELOPED_SIG   = "http://www.w3.org/2000/09/xmldsig#enveloped-signature"
     INC_PREFIX_LIST = "#default samlp saml ds xs xsi md"
 
-    attr_accessor :uuid
+    attr_writer :uuid
 
     def uuid
       @uuid ||= begin
@@ -187,7 +187,7 @@ def compute_digest(document, digest_algorithm)
   class SignedDocument < BaseDocument
     include OneLogin::RubySaml::ErrorHandling
 
-    attr_accessor :signed_element_id
+    attr_writer :signed_element_id
 
     def initialize(response, errors = [])
       super(response)
@@ -211,7 +211,7 @@ def validate_document(idp_cert_fingerprint, soft = true, options = {})
         cert_text = Base64.decode64(base64_cert)
         begin
           cert = OpenSSL::X509::Certificate.new(cert_text)
-        rescue OpenSSL::X509::CertificateError => e
+        rescue OpenSSL::X509::CertificateError => _e
           return append_error("Certificate Error", soft)
         end
 
@@ -254,7 +254,7 @@ def validate_document_with_cert(idp_cert)
         cert_text = Base64.decode64(base64_cert)
         begin
           cert = OpenSSL::X509::Certificate.new(cert_text)
-        rescue OpenSSL::X509::CertificateError => e
+        rescue OpenSSL::X509::CertificateError => _e
           return append_error("Certificate Error", soft)
         end
 
@@ -318,10 +318,9 @@ def validate_signature(base64_cert, soft = true)
 
       # check digests
       ref = REXML::XPath.first(sig_element, "//ds:Reference", {"ds"=>DSIG})
-      uri = ref.attributes.get_attribute("URI").value
 
       hashed_element = document.at_xpath("//*[@ID=$id]", nil, { 'id' => extract_signed_element_id })
-      
+
       canon_algorithm = canon_algorithm REXML::XPath.first(
         ref,
         '//ds:CanonicalizationMethod',

ruby-saml.gemspec

@@ -44,7 +44,7 @@ Gem::Specification.new do |s|
   s.add_development_dependency('mocha',    '~> 0.14')
   s.add_development_dependency('rake',     '~> 10')
   s.add_development_dependency('shoulda',  '~> 2.11')
-  s.add_development_dependency('simplecov','~> 0.9.0')
+  s.add_development_dependency('simplecov')
   s.add_development_dependency('systemu',  '~> 2')
   s.add_development_dependency('timecop',  '<= 0.6.0')