Commit fa9cfb3

mo <mo.khan@gmail.com>
2018-02-15 19:24:49
use nokogiri instead of xml hash.
1 parent 97f541e
lib/saml/kit/locales/en.yml
@@ -26,6 +26,7 @@ en:
         invalid_response_to: "must match request id."
         invalid_version: "must be 2.0."
         unregistered: "must originate from registered identity provider."
+        must_contain_single_assertion: "must contain a single Assertion."
       Signature:
         certificate: "Not valid before %{not_before}. Not valid after %{not_after}."
         digest_value: "is invalid."
lib/saml/kit/assertion.rb
@@ -9,9 +9,10 @@ module Saml
       attr_reader :name
       attr_accessor :occurred_at
 
-      def initialize(xml_hash, configuration: Saml::Kit.configuration)
+      def initialize(node, configuration: Saml::Kit.configuration)
         @name = "Assertion"
-        @xml_hash = xml_hash
+        @node = node
+        @xml_hash = hash_from(node)['Response'] || {}
         @configuration = configuration
         @occurred_at = Time.current
       end
@@ -30,7 +31,7 @@ module Saml
 
       def signature
         xml_hash = assertion.fetch('Signature', nil)
-        xml_hash ? Signature.new(xml_hash) : nil
+        xml_hash ? Signature.new(at_xpath('//ds:Signature')) : nil
       end
 
       def expired?(now = occurred_at)
@@ -39,6 +40,7 @@ module Saml
 
       def active?(now = occurred_at)
         drifted_started_at = started_at - configuration.clock_drift.to_i.seconds
+        puts [now.iso8601, drifted_started_at.iso8601, expired?(now)].inspect
         now > drifted_started_at && !expired?(now)
       end
 
@@ -71,7 +73,7 @@ module Saml
       end
 
       def encrypted?
-        @xml_hash.fetch('Response', {}).fetch('EncryptedAssertion', nil).present?
+        @xml_hash.fetch('EncryptedAssertion', nil).present?
       end
 
       def present?
@@ -87,11 +89,11 @@ module Saml
           if encrypted?
             private_keys = configuration.private_keys(use: :encryption)
             decryptor = ::Xml::Kit::Decryption.new(private_keys: private_keys)
-            decrypted = decryptor.decrypt_hash(@xml_hash['Response']['EncryptedAssertion'])
+            decrypted = decryptor.decrypt_hash(@xml_hash['EncryptedAssertion'])
             Saml::Kit.logger.debug(decrypted)
             Hash.from_xml(decrypted)['Assertion']
           else
-            result = @xml_hash.fetch('Response', {}).fetch('Assertion', {})
+            result = @xml_hash.fetch('Assertion', {})
             return result if result.is_a?(Hash)
 
             errors[:assertion] << error_message(:must_contain_single_assertion)
@@ -116,6 +118,15 @@ module Saml
         return if active?
         errors[:base] << error_message(:expired)
       end
+
+      def at_xpath(xpath)
+        @node.at_xpath(xpath, Saml::Kit::Document::NAMESPACES)
+      end
+
+      def hash_from(node)
+        return {} if node.nil?
+        Hash.from_xml(node.document.root.to_s) || {}
+      end
     end
   end
 end
lib/saml/kit/document.rb
@@ -79,6 +79,11 @@ module Saml
         to_nokogiri.at_xpath(xpath, NAMESPACES)
       end
 
+      # @!visibility private
+      def search(xpath)
+        to_nokogiri.search(xpath, NAMESPACES)
+      end
+
       def to_s
         to_xml
       end
lib/saml/kit/response.rb
@@ -8,6 +8,7 @@ module Saml
       def_delegators :assertion, :name_id, :[], :attributes
 
       validate :must_be_valid_assertion
+      validate :must_contain_single_assertion
 
       def initialize(xml, request_id: nil, configuration: Saml::Kit.configuration)
         @request_id = request_id
@@ -15,7 +16,16 @@ module Saml
       end
 
       def assertion
-        @assertion ||= Saml::Kit::Assertion.new(to_h, configuration: @configuration)
+        @assertion ||= 
+          begin
+            node = at_xpath(
+              [
+                '/samlp:Response/saml:Assertion',
+                '/samlp:Response/saml:EncryptedAssertion'
+              ].join('|')
+            )
+            Saml::Kit::Assertion.new(node, configuration: @configuration)
+          end
       end
 
       private
@@ -26,6 +36,20 @@ module Saml
           self.errors[attribute] << error
         end
       end
+
+      def must_contain_single_assertion
+        nodes = search('/samlp:Response/saml:Assertion')
+        if nodes.count > 1
+          errors[:base] << error_message(:must_contain_single_assertion)
+        end
+      end
+
+      def assertion_nodes
+        search([
+          '/samlp:Response/saml:Assertion',
+          '/samlp:Response/saml:EncryptedAssertion'
+        ].join('|'))
+      end
     end
   end
 end
lib/saml/kit/signature.rb
@@ -11,12 +11,8 @@ module Saml
 
       def initialize(item)
         @name = "Signature"
-        if item.is_a?(Hash)
-          @xml_hash = item
-        else
-          @node = item
-          @xml_hash = @node ? Hash.from_xml(@node.to_s)["Signature"] : {}
-        end
+        @node = item
+        @xml_hash = @node ? Hash.from_xml(@node.to_s)["Signature"] : {}
       end
 
       # Returns the embedded X509 Certificate
@@ -52,8 +48,10 @@ module Saml
 
       def validate_certificate(now = Time.now.utc)
         if certificate.present? && !certificate.active?(now)
-          message = error_message(:certificate, not_before: certificate.not_before, not_after: certificate.not_after)
-          errors.add(:certificate, message)
+          errors.add(:certificate, error_message(:certificate,
+            not_before: certificate.not_before,
+            not_after: certificate.not_after
+          ))
         end
       end
     end
lib/saml/kit/trustable.rb
@@ -17,7 +17,7 @@ module Saml
       # @!visibility private
       def signature
         xml_hash = to_h.fetch(name, {}).fetch('Signature', nil)
-        xml_hash ? Signature.new(to_nokogiri.at_xpath('//ds:Signature', 'ds' => Xml::Kit::Namespaces::XMLDSIG)) : nil
+        xml_hash ? Signature.new(at_xpath('//ds:Signature')) : nil
       end
 
 
spec/saml/assertion_spec.rb
@@ -10,17 +10,29 @@ RSpec.describe Saml::Kit::Assertion do
     it 'is valid after a valid session window + drift' do
       now = Time.current
       travel_to now
-      xml_hash = {
-        'Response' => {
-          'Assertion' => {
-            'Conditions' => {
-              'NotBefore' => now.utc.iso8601,
-              'NotOnOrAfter' => configuration.session_timeout.since(now).iso8601,
-            }
-          }
-        }
-      }
-      subject = described_class.new(xml_hash, configuration: configuration)
+      not_on_or_after = configuration.session_timeout.since(now).iso8601
+      xml = <<-XML
+<Assertion xmlns="#{Saml::Kit::Namespaces::ASSERTION}" ID="#{Xml::Kit::Id.generate}" IssueInstant="#{now.iso8601}" Version="2.0">
+ <Issuer>#{FFaker::Internet.uri("https")}</Issuer>
+ <Subject>
+   <NameID Format="#{Saml::Kit::Namespaces::PERSISTENT}">#{SecureRandom.uuid}</NameID>
+   <SubjectConfirmation Method="#{Saml::Kit::Namespaces::BEARER}">
+     <SubjectConfirmationData InResponseTo="#{SecureRandom.uuid}" NotOnOrAfter="#{not_on_or_after}" Recipient="#{FFaker::Internet.uri("https")}"/>
+   </SubjectConfirmation>
+ </Subject>
+ <Conditions NotBefore="#{now.utc.iso8601}" NotOnOrAfter="#{not_on_or_after}">
+   <AudienceRestriction>
+     <Audience>#{FFaker::Internet.uri("https")}</Audience>
+   </AudienceRestriction>
+ </Conditions>
+ <AuthnStatement AuthnInstant="#{now.utc.iso8601}" SessionIndex="#{Xml::Kit::Id.generate}" SessionNotOnOrAfter="#{not_on_or_after}">
+   <AuthnContext>
+     <AuthnContextClassRef>#{Saml::Kit::Namespaces::PASSWORD}</AuthnContextClassRef>
+   </AuthnContext>
+ </AuthnStatement>
+</Assertion>
+XML
+      subject = described_class.new(Nokogiri::XML(xml), configuration: configuration)
       travel_to (configuration.clock_drift - 1.second).before(now)
       expect(subject).to be_active
       expect(subject).to_not be_expired
@@ -30,18 +42,30 @@ RSpec.describe Saml::Kit::Assertion do
       configuration.clock_drift = 30
       now = Time.current
       travel_to now
-      xml_hash = {
-        'Response' => {
-          'Assertion' => {
-            'Conditions' => {
-              'NotBefore' => now.utc.iso8601,
-              'NotOnOrAfter' => configuration.session_timeout.since(now).iso8601,
-            }
-          }
-        }
-      }
-
-      subject = described_class.new(xml_hash, configuration: configuration)
+      not_before = now.utc.iso8601
+      not_after = configuration.session_timeout.since(now).iso8601
+      xml = <<-XML
+<Assertion xmlns="#{Saml::Kit::Namespaces::ASSERTION}" ID="#{Xml::Kit::Id.generate}" IssueInstant="#{now.iso8601}" Version="2.0">
+ <Issuer>#{FFaker::Internet.uri("https")}</Issuer>
+ <Subject>
+   <NameID Format="#{Saml::Kit::Namespaces::PERSISTENT}">#{SecureRandom.uuid}</NameID>
+   <SubjectConfirmation Method="#{Saml::Kit::Namespaces::BEARER}">
+     <SubjectConfirmationData InResponseTo="#{SecureRandom.uuid}" NotOnOrAfter="#{not_after}" Recipient="#{FFaker::Internet.uri("https")}"/>
+   </SubjectConfirmation>
+ </Subject>
+ <Conditions NotBefore="#{not_before}" NotOnOrAfter="#{not_after}">
+   <AudienceRestriction>
+     <Audience>#{FFaker::Internet.uri("https")}</Audience>
+   </AudienceRestriction>
+ </Conditions>
+ <AuthnStatement AuthnInstant="#{now.utc.iso8601}" SessionIndex="#{Xml::Kit::Id.generate}" SessionNotOnOrAfter="#{not_after}">
+   <AuthnContext>
+     <AuthnContextClassRef>#{Saml::Kit::Namespaces::PASSWORD}</AuthnContextClassRef>
+   </AuthnContext>
+ </AuthnStatement>
+</Assertion>
+XML
+      subject = described_class.new(Nokogiri::XML(xml), configuration: configuration)
       expect(subject).to be_active
       expect(subject).to_not be_expired
     end
@@ -49,14 +73,35 @@ RSpec.describe Saml::Kit::Assertion do
 
   describe "#present?" do
     it 'returns false when the assertion is empty' do
-      xml_hash = { 'Response' => { } }
-      subject = described_class.new(xml_hash)
+      subject = described_class.new(nil)
       expect(subject).to_not be_present
     end
 
     it 'returns true when the assertion is present' do
-      xml_hash = { 'Response' => { 'Assertion' => { 'Conditions' => { } } } }
-      subject = described_class.new(xml_hash)
+      not_before = Time.now.utc.iso8601
+      not_after = 10.minutes.from_now.iso8601
+      xml = <<-XML
+<Assertion xmlns="#{Saml::Kit::Namespaces::ASSERTION}" ID="#{Xml::Kit::Id.generate}" IssueInstant="#{Time.now.iso8601}" Version="2.0">
+ <Issuer>#{FFaker::Internet.uri("https")}</Issuer>
+ <Subject>
+   <NameID Format="#{Saml::Kit::Namespaces::PERSISTENT}">#{SecureRandom.uuid}</NameID>
+   <SubjectConfirmation Method="#{Saml::Kit::Namespaces::BEARER}">
+     <SubjectConfirmationData InResponseTo="#{SecureRandom.uuid}" NotOnOrAfter="#{not_after}" Recipient="#{FFaker::Internet.uri("https")}"/>
+   </SubjectConfirmation>
+ </Subject>
+ <Conditions NotBefore="#{not_before}" NotOnOrAfter="#{not_after}">
+   <AudienceRestriction>
+     <Audience>#{FFaker::Internet.uri("https")}</Audience>
+   </AudienceRestriction>
+ </Conditions>
+ <AuthnStatement AuthnInstant="#{Time.now.utc.iso8601}" SessionIndex="#{Xml::Kit::Id.generate}" SessionNotOnOrAfter="#{not_after}">
+   <AuthnContext>
+     <AuthnContextClassRef>#{Saml::Kit::Namespaces::PASSWORD}</AuthnContextClassRef>
+   </AuthnContext>
+ </AuthnStatement>
+</Assertion>
+XML
+      subject = described_class.new(Nokogiri::XML(xml))
       expect(subject).to be_present
     end
   end
spec/saml/response_spec.rb
@@ -227,7 +227,7 @@ RSpec.describe Saml::Kit::Response do
       end
       subject = described_class.new(xml)
       expect(subject).to_not be_valid
-      expect(subject.errors[:assertion]).to be_present
+      expect(subject.errors.full_messages).to include("must contain a single Assertion.")
     end
 
     it 'is invalid when the assertion has a signature and has been tampered with' do