Merge pull request #79 from lawrencepit/not_before

Allow for clock drift.

Author: stouset

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/response.rb

@@ -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 })
@@ -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

test/response_test.rb

@@ -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