Merge pull request #58 from squarooticus/master

Fix for response condition validation based on responses I'm getting from OneLogin
SAML-Toolkits · Jan 2, 2013 · 25173fe · 25173fe
2 parents bae45bc + c7e959e
commit 25173fe
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 16 deletions.
diff --git a/Gemfile b/Gemfile
@@ -9,4 +9,5 @@ group :test do
   gem "rake"
   gem "mocha"
   gem "nokogiri"
+  gem "timecop"
 end
diff --git a/lib/onelogin/ruby-saml/response.rb b/lib/onelogin/ruby-saml/response.rb
@@ -36,16 +36,14 @@ def validate!
       # The value of the user identifier as designated by the initialization request response
       def name_id
         @name_id ||= begin
-          node = REXML::XPath.first(document, "/p:Response/a:Assertion[@ID='#{document.signed_element_id}']/a:Subject/a:NameID", { "p" => PROTOCOL, "a" => ASSERTION })
-          node ||=  REXML::XPath.first(document, "/p:Response[@ID='#{document.signed_element_id}']/a:Assertion/a:Subject/a:NameID", { "p" => PROTOCOL, "a" => ASSERTION })
+          node = xpath_first_from_signed_assertion('/a:Subject/a:NameID')
           node.nil? ? nil : node.text
         end
       end
 
       def sessionindex
         @sessionindex ||= begin
-          node = REXML::XPath.first(document, "/p:Response/a:Assertion[@ID='#{document.signed_element_id}']/a:AuthnStatement", { "p" => PROTOCOL, "a" => ASSERTION })
-          node ||=  REXML::XPath.first(document, "/p:Response[@ID='#{document.signed_element_id}']/a:Assertion/a:AuthnStatement", { "p" => PROTOCOL, "a" => ASSERTION })
+          node = xpath_first_from_signed_assertion('/a:AuthnStatement')
           node.nil? ? nil : node.attributes['SessionIndex']
         end
       end
@@ -55,7 +53,7 @@ def attributes
         @attr_statements ||= begin
           result = {}
 
-          stmt_element = REXML::XPath.first(document, "/p:Response/a:Assertion/a:AttributeStatement", { "p" => PROTOCOL, "a" => ASSERTION })
+          stmt_element = xpath_first_from_signed_assertion('/a:AttributeStatement')
           return {} if stmt_element.nil?
 
           stmt_element.elements.each do |attr_element|
@@ -76,7 +74,7 @@ def attributes
       # When this user session should expire at latest
       def session_expires_at
         @expires_at ||= begin
-          node = REXML::XPath.first(document, "/p:Response/a:Assertion/a:AuthnStatement", { "p" => PROTOCOL, "a" => ASSERTION })
+          node = xpath_first_from_signed_assertion('/a:AuthnStatement')
           parse_time(node, "SessionNotOnOrAfter")
         end
       end
@@ -91,15 +89,13 @@ def success?
 
       # Conditions (if any) for the assertion to run
       def conditions
-        @conditions ||= begin
-          REXML::XPath.first(document, "/p:Response/a:Assertion[@ID='#{document.signed_element_id}']/a:Conditions", { "p" => PROTOCOL, "a" => ASSERTION })
-        end
+        @conditions ||= xpath_first_from_signed_assertion('/a:Conditions')
       end
 
       def issuer
         @issuer ||= begin
           node = REXML::XPath.first(document, "/p:Response/a:Issuer", { "p" => PROTOCOL, "a" => ASSERTION })
-          node ||= REXML::XPath.first(document, "/p:Response/a:Assertion/a:Issuer", { "p" => PROTOCOL, "a" => ASSERTION })
+          node ||= xpath_first_from_signed_assertion('/a:Issuer')
           node.nil? ? nil : node.text
         end
       end
@@ -146,6 +142,12 @@ def validate_response_state(soft = true)
         true
       end
 
+      def xpath_first_from_signed_assertion(subelt=nil)
+        node = REXML::XPath.first(document, "/p:Response/a:Assertion[@ID='#{document.signed_element_id}']#{subelt}", { "p" => PROTOCOL, "a" => ASSERTION })
+        node ||= REXML::XPath.first(document, "/p:Response[@ID='#{document.signed_element_id}']/a:Assertion#{subelt}", { "p" => PROTOCOL, "a" => ASSERTION })
+        node
+      end
+
       def get_fingerprint
         if settings.idp_cert
           cert = OpenSSL::X509::Certificate.new(settings.idp_cert)

diff --git a/test/test_helper.rb b/test/test_helper.rb
@@ -3,6 +3,7 @@
 require 'shoulda'
 require 'mocha'
 require 'ruby-debug'
+require 'timecop'
 
 $LOAD_PATH.unshift(File.join(File.dirname(__FILE__), '..', 'lib'))
 $LOAD_PATH.unshift(File.dirname(__FILE__))

diff --git a/test/xml_security_test.rb b/test/xml_security_test.rb
@@ -119,12 +119,29 @@ class XmlSecurityTest < Test::Unit::TestCase
     end
 
     context "StarfieldTMS" do
-      should "be able to validate a response" do
-        response = Onelogin::Saml::Response.new(fixture(:starfield_response))
-        response.settings = Onelogin::Saml::Settings.new(
-          :idp_cert_fingerprint => "8D:BA:53:8E:A3:B6:F9:F1:69:6C:BB:D9:D8:BD:41:B3:AC:4F:9D:4D"
-        )
-        assert response.validate!
+      setup do
+        @response = Onelogin::Saml::Response.new(fixture(:starfield_response))
+        @response.settings = Onelogin::Saml::Settings.new(
+                                                          :idp_cert_fingerprint => "8D:BA:53:8E:A3:B6:F9:F1:69:6C:BB:D9:D8:BD:41:B3:AC:4F:9D:4D"
+                                                          )
+      end
+
+      should "be able to validate a good response" do
+        Timecop.freeze Time.parse('2012-11-28 17:55:00 UTC') do
+          assert @response.validate!
+        end
+      end
+
+      should "fail before response is valid" do
+        Timecop.freeze Time.parse('2012-11-20 17:55:00 UTC') do
+          assert ! @response.is_valid?
+        end
+      end
+
+      should "fail after response expires" do
+        Timecop.freeze Time.parse('2012-11-30 17:55:00 UTC') do
+          assert ! @response.is_valid?
+        end
       end
     end