Merge branch 'master' of github.com:onelogin/ruby-saml

Author: brendon

.gitignore

@@ -9,3 +9,4 @@ lib/Lib.iml
 test/Test.iml
 .rvmrc
 *.gem
+.bundle

Gemfile

@@ -4,10 +4,12 @@ gemspec
 
 group :test do
   gem "ruby-debug", "~> 0.10.4", :require => nil, :platforms => :ruby_18
-  gem "debugger",   "~> 1.1.1",  :require => nil, :platforms => :ruby_19
-  gem "shoulda"
-  gem "rake"
-  gem "mocha"
-  gem "nokogiri", ">= 1.5.0"
-  gem "timecop"
+  gem "debugger",   "~> 1.1",    :require => nil, :platforms => :ruby_19
+  gem "shoulda",    "~> 2.11"
+  gem "rake",       "~> 10"
+  gem "mocha",      "~> 0.14"
+  gem "nokogiri",   "~> 1.5"
+  gem "timecop",    "<= 0.6.0"
+  gem "systemu",    "~> 2"
+  gem "rspec",      "~> 2"
 end

README.md

@@ -118,6 +118,20 @@ to the IdP settings.
   end
 ```
 
+## Clock Drift
+
+Server clocks tend to drift naturally. If during validation of the response you get the error "Current time is earlier than NotBefore condition" then this may be due to clock differences between your system and that of the Identity Provider.
+
+First, ensure that both systems synchronize their clocks, using for example the industry standard [Network Time Protocol (NTP)](http://en.wikipedia.org/wiki/Network_Time_Protocol).
+
+Even then you may experience intermittent issues though, because the clock of the Identity Provider may drift slightly ahead of your system clocks. To allow for a small amount of clock drift you can initialize the response passing in an option named `:allowed_clock_drift`. Its value must be given in a number (and/or fraction) of seconds. The value given is added to the current time at which the response is validated before it's tested against the `NotBefore` assertion. For example:
+
+```ruby
+  response = Onelogin::Saml::Response.new(params[:SAMLResponse], :allowed_clock_drift => 1)
+```
+
+Make sure to keep the value as comfortably small as possible to keep security risks to a minimum.
+
 ## Note on Patches/Pull Requests
 
 * Fork the project.

lib/onelogin/ruby-saml/authrequest.rb

@@ -36,7 +36,7 @@ def create(settings, params = {})
       def create_authentication_xml_doc(settings)
         uuid = "_" + UUID.new.generate
         time = Time.now.utc.strftime("%Y-%m-%dT%H:%M:%SZ")
-        # Create AuthnRequest root element using REXML 
+        # Create AuthnRequest root element using REXML
         request_doc = REXML::Document.new
 
         root = request_doc.add_element "samlp:AuthnRequest", { "xmlns:samlp" => "urn:oasis:names:tc:SAML:2.0:protocol" }
@@ -45,6 +45,7 @@ def create_authentication_xml_doc(settings)
         root.attributes['Version'] = "2.0"
         root.attributes['Destination'] = settings.idp_sso_target_url unless settings.idp_sso_target_url.nil?
         root.attributes['IsPassive'] = settings.passive unless settings.passive.nil?
+        root.attributes['ProtocolBinding'] = settings.protocol_binding unless settings.protocol_binding.nil?
 
         # Conditionally defined elements based on settings
         if settings.assertion_consumer_service_url != nil
@@ -55,7 +56,7 @@ def create_authentication_xml_doc(settings)
           issuer.text = settings.issuer
         end
         if settings.name_identifier_format != nil
-          root.add_element "samlp:NameIDPolicy", { 
+          root.add_element "samlp:NameIDPolicy", {
               "xmlns:samlp" => "urn:oasis:names:tc:SAML:2.0:protocol",
               # Might want to make AllowCreate a setting?
               "AllowCreate" => "true",
@@ -64,14 +65,14 @@ def create_authentication_xml_doc(settings)
         end
 
         # BUG fix here -- if an authn_context is defined, add the tags with an "exact"
-        # match required for authentication to succeed.  If this is not defined, 
+        # match required for authentication to succeed.  If this is not defined,
         # the IdP will choose default rules for authentication.  (Shibboleth IdP)
         if settings.authn_context != nil
-          requested_context = root.add_element "samlp:RequestedAuthnContext", { 
+          requested_context = root.add_element "samlp:RequestedAuthnContext", {
             "xmlns:samlp" => "urn:oasis:names:tc:SAML:2.0:protocol",
             "Comparison" => "exact",
           }
-          class_ref = requested_context.add_element "saml:AuthnContextClassRef", { 
+          class_ref = requested_context.add_element "saml:AuthnContextClassRef", {
             "xmlns:saml" => "urn:oasis:names:tc:SAML:2.0:assertion",
           }
           class_ref.text = settings.authn_context

lib/onelogin/ruby-saml/response.rb

@@ -78,7 +78,7 @@ def session_expires_at
           parse_time(node, "SessionNotOnOrAfter")
         end
       end
-      
+
       # Checks the status of the response for a "Success" code
       def success?
         @status_code ||= begin
@@ -92,6 +92,14 @@ def conditions
         @conditions ||= xpath_first_from_signed_assertion('/a:Conditions')
       end
 
+      def not_before
+        @not_before ||= parse_time(conditions, "NotBefore")
+      end
+
+      def not_on_or_after
+        @not_on_or_after ||= parse_time(conditions, "NotOnOrAfter")
+      end
+
       def issuer
         @issuer ||= begin
           node = REXML::XPath.first(document, "/p:Response/a:Issuer", { "p" => PROTOCOL, "a" => ASSERTION })
@@ -110,7 +118,7 @@ def validate(soft = true)
         validate_structure(soft)      &&
         validate_response_state(soft) &&
         validate_conditions(soft)     &&
-        document.validate(get_fingerprint, soft) && 
+        document.validate_document(get_fingerprint, soft) &&
         success?
       end
 
@@ -161,16 +169,14 @@ def validate_conditions(soft = true)
         return true if conditions.nil?
         return true if options[:skip_conditions]
 
-        if (not_before = parse_time(conditions, "NotBefore"))
-          if Time.now.utc < not_before
-            return soft ? false : validation_error("Current time is earlier than NotBefore condition")
-          end
+        now = Time.now.utc
+
+        if not_before && (now + (options[:allowed_clock_drift] || 0)) < not_before
+          return soft ? false : validation_error("Current time is earlier than NotBefore condition")
         end
 
-        if (not_on_or_after = parse_time(conditions, "NotOnOrAfter"))
-          if Time.now.utc >= not_on_or_after
-            return soft ? false : validation_error("Current time is on or after NotOnOrAfter condition")
-          end
+        if not_on_or_after && now >= not_on_or_after
+          return soft ? false : validation_error("Current time is on or after NotOnOrAfter condition")
         end
 
         true

lib/onelogin/ruby-saml/settings.rb

@@ -18,9 +18,10 @@ def initialize(overrides = {})
       attr_accessor :compress_request
       attr_accessor :double_quote_xml_attribute_values
       attr_accessor :passive
+      attr_accessor :protocol_binding
 
       private
-      
+
       DEFAULTS = {:compress_request => true, :double_quote_xml_attribute_values => false}
     end
   end

lib/onelogin/ruby-saml/version.rb

@@ -1,5 +1,5 @@
 module Onelogin
   module Saml
-    VERSION = '0.7.2'
+    VERSION = '0.7.3'
   end
 end

lib/xml_security.rb

@@ -44,7 +44,7 @@ def initialize(response)
       extract_signed_element_id
     end
 
-    def validate(idp_cert_fingerprint, soft = true)
+    def validate_document(idp_cert_fingerprint, soft = true)
       # get cert from response
       cert_element = REXML::XPath.first(self, "//ds:X509Certificate", { "ds"=>DSIG })
       raise Onelogin::Saml::ValidationError.new("Certificate element missing in response (ds:X509Certificate)") unless cert_element
@@ -59,10 +59,10 @@ def validate(idp_cert_fingerprint, soft = true)
         return soft ? false : (raise Onelogin::Saml::ValidationError.new("Fingerprint mismatch"))
       end
 
-      validate_doc(base64_cert, soft)
+      validate_signature(base64_cert, soft)
     end
 
-    def validate_doc(base64_cert, soft = true)
+    def validate_signature(base64_cert, soft = true)
       # validate references
 
       # check for inclusive namespaces

ruby-saml.gemspec

@@ -24,5 +24,5 @@ Gem::Specification.new do |s|
   s.test_files = `git ls-files test/*`.split("\n")
 
   s.add_runtime_dependency("uuid", ["~> 2.3"])
-  s.add_runtime_dependency("nokogiri", [">= 1.5.0"])
+  s.add_runtime_dependency("nokogiri", ["~> 1.5.0"])
 end

test/response_test.rb

@@ -120,7 +120,7 @@ class RubySamlTest < Test::Unit::TestCase
         settings = Onelogin::Saml::Settings.new
         response.settings = settings
         settings.idp_cert_fingerprint = "28:74:9B:E8:1F:E8:10:9C:A8:7C:A9:C3:E3:C5:01:6C:92:1C:B4:BA"
-        XMLSecurity::SignedDocument.any_instance.expects(:validate_doc).returns(true)
+        XMLSecurity::SignedDocument.any_instance.expects(:validate_signature).returns(true)
         assert response.validate!
       end
 
@@ -184,6 +184,17 @@ class RubySamlTest < Test::Unit::TestCase
         response = Onelogin::Saml::Response.new(response_document_5)
         assert response.send(:validate_conditions, true)
       end
+
+      should "optionally allow for clock drift" do
+        # The NotBefore condition in the document is 2011-06-14T18:21:01.516Z
+        Time.stubs(:now).returns(Time.parse("2011-06-14T18:21:01Z"))
+        response = Onelogin::Saml::Response.new(response_document_5, :allowed_clock_drift => 0.515)
+        assert !response.send(:validate_conditions, true)
+
+        Time.stubs(:now).returns(Time.parse("2011-06-14T18:21:01Z"))
+        response = Onelogin::Saml::Response.new(response_document_5, :allowed_clock_drift => 0.516)
+        assert response.send(:validate_conditions, true)
+      end
     end
 
     context "#attributes" do
@@ -229,13 +240,13 @@ class RubySamlTest < Test::Unit::TestCase
         response = Onelogin::Saml::Response.new(response_document)
         assert_equal "https://app.onelogin.com/saml/metadata/13590", response.issuer
       end
-      
+
       should "return the issuer inside the response" do
         response = Onelogin::Saml::Response.new(response_document_2)
         assert_equal "wibble", response.issuer
       end
     end
-    
+
     context "#success" do
       should "find a status code that says success" do
         response = Onelogin::Saml::Response.new(response_document)