use the now-standard timeout_protection for ldap binding

The previous code was a one-off written before timeout_protection
existed. This means that we'll now allow N timeouts (3 by default) in
the given period, rather than refusing to connect to the LDAP server
after just one timeout.

closes CNVS-8371

test plan:

* Configure an account to use ldap. Rather than setting up a real ldap
  server, it's sufficient for this testing to just use nc or another
  application to listen on the port you specify in the account config.
* Attempt to login to the account, and see canvas in your nc output.
  Allow it to timeout. Attempt again, and canvas will hit your "ldap
  server" again. After 3 timed out attempts, canvas will blacklist your
  server for 1 minute.
* Also verify that logging in with ldap still works against a real ldap.

Change-Id: I60293d01690be3cc24f57b8bcd5c6c52e23fc2a9
Reviewed-on: https://gerrit.instructure.com/24657
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
QA-Review: August Thornton <august@instructure.com>
Product-Review: Brian Palmer <brianp@instructure.com>

app/models/account_authorization_config.rb

@@ -374,32 +374,20 @@ class AccountAuthorizationConfig < ActiveRecord::Base
   def ldap_bind_result(unique_id, password_plaintext)
     return nil if password_plaintext.blank?
 
-    if self.last_timeout_failure.present?
-      failure_timeout = self.class.ldap_failure_wait_time.ago
-      if self.last_timeout_failure >= failure_timeout
-        # we've failed too recently, don't try again
-        return nil
-      end
+    default_timeout = Setting.get('ldap_timelimit', 5.seconds.to_s).to_f
+
+    Canvas.timeout_protection("ldap:#{self.global_id}",
+                              raise_on_timeout: true,
+                              fallback_timeout_length: default_timeout) do
+      ldap = self.ldap_connection
+      filter = self.ldap_filter(unique_id)
+      ldap.bind_as(:base => ldap.base, :filter => filter, :password => password_plaintext)
     end
-
-    timelimit = Setting.get('ldap_timelimit', 5.seconds.to_s).to_f
-
-    begin
-      Timeout.timeout(timelimit) do
-        ldap = self.ldap_connection
-        filter = self.ldap_filter(unique_id)
-        res = ldap.bind_as(:base => ldap.base, :filter => filter, :password => password_plaintext)
-        return res if res
-      end
-    rescue Net::LDAP::LdapError
-      ErrorReport.log_exception(:ldap, $!, :account => self.account)
-    rescue Timeout::Error
-      ErrorReport.log_exception(:ldap, $!, :account => self.account)
+  rescue => e
+    ErrorReport.log_exception(:ldap, e, :account => self.account)
+    if e.is_a?(Timeout::Error)
       self.update_attribute(:last_timeout_failure, Time.now)
-    rescue
-      ErrorReport.log_exception(:ldap, $!, :account => self.account)
     end
-
     return nil
   end
 

app/views/account_authorization_configs/_ldap_timeout_error.html.erb

@@ -4,7 +4,7 @@
   <td>
     <%= datetime_string(account_config.last_timeout_failure) %>
     <p>
-      <%= t :ldap_timeout_failure_help, "If Canvas times out communicating with the LDAP server, it will block further login attempts from using that LDAP server for %{time}.", :time => distance_of_time_in_words(AccountAuthorizationConfig.ldap_failure_wait_time) %>
+      <%= t :ldap_timeout_failure_help2, "If Canvas times out too many times communicating with the LDAP server, it will block further login attempts from using that LDAP server for %{time}.", :time => distance_of_time_in_words(AccountAuthorizationConfig.ldap_failure_wait_time) %>
     </p>
   </td>
 </tr>

lib/canvas.rb

@@ -142,12 +142,12 @@ module Canvas
       error_count = Canvas.redis.get(redis_key)
       if error_count.to_i >= cutoff
         Rails.logger.error("Skipping service call due to error count: #{service_name} #{error_count}")
-        raise "timeout_protection cutoff triggered" if options[:raise_on_timeout]
+        raise(Timeout::Error, "timeout_protection cutoff triggered") if options[:raise_on_timeout]
         return
       end
     end
 
-    timeout = (Setting.get_cached("service_#{service_name}_timeout", nil) || Setting.get_cached("service_generic_timeout", 15.seconds.to_s)).to_f
+    timeout = (Setting.get_cached("service_#{service_name}_timeout", nil) || options[:fallback_timeout_length] || Setting.get_cached("service_generic_timeout", 15.seconds.to_s)).to_f
     Timeout.timeout(timeout) do
       yield
     end

spec/lib/canvas_spec.rb

@@ -40,6 +40,17 @@ describe Canvas do
       lambda { Canvas.timeout_protection("spec", raise_on_timeout: true) {} }.should raise_error(Timeout::Error)
     end
 
+    it "should use the timeout argument over the generic default" do
+      Timeout.expects(:timeout).with(23)
+      Canvas.timeout_protection("foo", fallback_timeout_length: 23)
+    end
+
+    it "should use the settings timeout over the timeout argument" do
+      Setting.set("service_foo_timeout", "1")
+      Timeout.expects(:timeout).with(1)
+      Canvas.timeout_protection("foo", fallback_timeout_length: 23)
+    end
+
     if Canvas.redis_enabled?
       it "should skip calling the block after X failures" do
         Setting.set("service_spec_cutoff", "2")
@@ -63,7 +74,7 @@ describe Canvas do
 
       it "should raise on cutoff if raise_on_timeout option is specified" do
         Canvas.redis.set("service:timeouts:spec", 42)
-        lambda { Canvas.timeout_protection("spec", raise_on_timeout: true) {} }.should raise_error(RuntimeError)
+        lambda { Canvas.timeout_protection("spec", raise_on_timeout: true) {} }.should raise_error(Timeout::Error)
       end
     end
   end

spec/models/pseudonym_spec.rb

@@ -197,30 +197,9 @@ describe Pseudonym do
     it "should set last_timeout_failure on LDAP servers that timeout" do
       Net::LDAP.any_instance.expects(:bind_as).once.raises(Timeout::Error, "timed out")
       @pseudonym.ldap_bind_result('test').should be_false
-      ErrorReport.last.message.should match /Timeout::Error|timed out/ # 1.8/1.9 compat
+      ErrorReport.last.message.should match(/timed out/)
       @aac.reload.last_timeout_failure.should > 1.minute.ago
     end
-
-    it "should not attempt to bind if last_timeout_failure is set recently" do
-      # calling again should not attempt to bind
-      @aac.update_attribute(:last_timeout_failure, 5.seconds.ago)
-      Net::LDAP.any_instance.expects(:bind_as).never
-      @pseudonym.ldap_bind_result('test').should be_false
-
-      # updating the config should reset :last_timeout_failure
-      @aac.reload.update_attributes(:auth_port => 637)
-      @aac.last_timeout_failure.should be_nil
-      Net::LDAP.any_instance.expects(:bind_as).returns(true)
-      @pseudonym.reload
-      @pseudonym.ldap_bind_result('test').should be_true
-    end
-
-    it "should allow another attempt once last_timeout_failure is sufficiently in the past" do
-      @aac.update_attribute(:last_timeout_failure, 5.seconds.ago)
-      Setting.set('ldap_failure_wait_time', 2.seconds)
-      Net::LDAP.any_instance.expects(:bind_as).returns(true)
-      @pseudonym.ldap_bind_result('test').should be_true
-    end
   end
 
   it "should not error on malformed SSHA password" do