response.rb in saml2-3.1.3

- old
+ new

@@ -1,19 +1,17 @@
 # frozen_string_literal: true
 
-require 'nokogiri-xmlsec'
-require 'time'
+require "nokogiri-xmlsec"
+require "time"
 
-require 'saml2/assertion'
-require 'saml2/authn_statement'
-require 'saml2/status_response'
-require 'saml2/subject'
+require "saml2/assertion"
+require "saml2/authn_statement"
+require "saml2/status_response"
+require "saml2/subject"
 
 module SAML2
   class Response < StatusResponse
-    attr_reader :assertions
-
     # Respond to an {AuthnRequest}
     #
     # {AuthnRequest#resolve} needs to have been previously called on the {AuthnRequest}.
     # @param authn_request [AuthnRequest]
     # @param issuer [NameID]
@@ -96,41 +94,52 @@
     def validate(service_provider:,
                  identity_provider:,
                  verification_time: nil,
                  ignore_audience_condition: false)
       raise ArgumentError, "service_provider should be an Entity object" unless service_provider.is_a?(Entity)
-      raise ArgumentError, "service_provider should have at least one service_provider role" unless (sp = service_provider.service_providers.first)
 
+      unless (sp = service_provider.service_providers.first)
+        raise ArgumentError,
+              "service_provider should have at least one service_provider role"
+      end
+
       # validate the schema
       super()
       return errors unless errors.empty?
 
       if verification_time.nil?
         verification_time = Time.now.utc
         # they issued it in the (near) future according to our clock;
         # use their clock instead
-        verification_time = issue_instant if issue_instant > verification_time && issue_instant < verification_time + 5 * 60
+        if issue_instant > verification_time && issue_instant < verification_time + (5 * 60)
+          verification_time = issue_instant
+        end
       end
 
       # not finding the issuer is not exceptional
       if identity_provider.nil?
         errors << "could not find issuer of response"
         return errors
       end
 
       # getting the wrong data type is exceptional, and we should raise an error
       raise ArgumentError, "identity_provider should be an Entity object" unless identity_provider.is_a?(Entity)
-      raise ArgumentError, "identity_provider should have at least one identity_provider role" unless (idp = identity_provider.identity_providers.first)
 
+      unless (idp = identity_provider.identity_providers.first)
+        raise ArgumentError,
+              "identity_provider should have at least one identity_provider role"
+      end
+
       issuer = self.issuer || assertions.first&.issuer
       unless identity_provider.entity_id == issuer&.id
-        errors << "received unexpected message from '#{issuer&.id}'; expected it to be from '#{identity_provider.entity_id}'"
+        errors << "received unexpected message from '#{issuer&.id}'; " \
+                  "expected it to be from '#{identity_provider.entity_id}'"
         return errors
       end
 
-      certificates = idp.signing_keys.map(&:certificate).compact
-      keys = idp.signing_keys.map(&:key).compact
+      certificates = idp.signing_keys.filter_map(&:certificate)
+      keys = idp.signing_keys.filter_map(&:key)
       if idp.fingerprints.empty? && certificates.empty? && keys.empty?
         errors << "could not find certificate to validate message"
         return errors
       end
 
@@ -138,10 +147,11 @@
         unless (signature_errors = validate_signature(key: keys,
                                                       fingerprint: idp.fingerprints,
                                                       cert: certificates)).empty?
           return errors.concat(signature_errors)
         end
+
         response_signed = true
       end
 
       assertion = assertions.first
 
@@ -150,28 +160,31 @@
         unless (signature_errors = assertion.validate_signature(key: keys,
                                                                 fingerprint: idp.fingerprints,
                                                                 cert: certificates)).empty?
           return errors.concat(signature_errors)
         end
+
         assertion_signed = true
       end
 
-      find_decryption_key = ->(embedded_certificates) do
+      find_decryption_key = lambda do |embedded_certificates|
         key = nil
         embedded_certificates.each do |cert_info|
           cert = case cert_info
-                   when OpenSSL::X509::Certificate; cert_info
-                   when Hash; sp.encryption_keys.map(&:certificate).find { |c| c.serial == cert_info[:serial] }
+                 when OpenSSL::X509::Certificate then cert_info
+                 when Hash then sp.encryption_keys.map(&:certificate).find { |c| c.serial == cert_info[:serial] }
                  end
           next unless cert
+
           key = sp.private_keys.find { |k| cert.check_private_key(k) }
           break if key
         end
-        if !key
+        unless key
           # couldn't figure out which key to use; just try them all
           next sp.private_keys
         end
+
         key
       end
 
       unless sp.private_keys.empty?
         begin
@@ -206,27 +219,28 @@
       if assertion.signed? && !assertion_signed
         unless (signature_errors = assertion.validate_signature(fingerprint: idp.fingerprints,
                                                                 cert: certificates)).empty?
           return errors.concat(signature_errors)
         end
+
         assertion_signed = true
       end
 
       # only do our own issue instant validation if the assertion
       # doesn't mandate any
-      unless assertion.conditions&.not_on_or_after
-        if assertion.issue_instant +  5 * 60 < verification_time ||
-            assertion.issue_instant - 5 * 60 > verification_time
-          errors << "assertion not recently issued"
-          return errors
-        end
+      if !assertion.conditions&.not_on_or_after && (assertion.issue_instant + (5 * 60) < verification_time ||
+           assertion.issue_instant - (5 * 60) > verification_time)
+        errors << "assertion not recently issued"
+        return errors
       end
 
       if assertion.conditions &&
-          !(condition_errors = assertion.conditions.validate(verification_time: verification_time,
-                                                             audience: service_provider.entity_id,
-                                                             ignore_audience_condition: ignore_audience_condition)).empty?
+         !(condition_errors = assertion.conditions.validate(
+           verification_time: verification_time,
+           audience: service_provider.entity_id,
+           ignore_audience_condition: ignore_audience_condition
+         )).empty?
         return errors.concat(condition_errors)
       end
 
       if !response_signed && !assertion_signed
         errors << "neither response nor assertion were signed"
@@ -251,13 +265,11 @@
       errors
     end
 
     # @return [Array<Assertion>]
     def assertions
-      unless instance_variable_defined?(:@assertions)
-        @assertions = load_object_array(xml, 'saml:Assertion', Assertion)
-      end
+      @assertions = load_object_array(xml, "saml:Assertion", Assertion) unless instance_variable_defined?(:@assertions)
       @assertions
     end
 
     # (see Signable#sign)
     # Signs each assertion.
@@ -273,12 +285,12 @@
     end
 
     private
 
     def build(builder)
-      builder['samlp'].Response(
-        'xmlns:samlp' => Namespaces::SAMLP,
-        'xmlns:saml' => Namespaces::SAML
+      builder["samlp"].Response(
+        "xmlns:samlp" => Namespaces::SAMLP,
+        "xmlns:saml" => Namespaces::SAML
       ) do |response|
         super(response)
 
         assertions.each do |assertion|
           # we can't just call build, because it may already