Merge pull request #106 from omniauth/feat/timeout

Author: pboling

.rubocop_gradual.lock

@@ -1,6 +1,6 @@
 {
-  "lib/omniauth-ldap/adaptor.rb:3212021924": [
-    [65, 7, 413, "Style/ClassMethodsDefinitions: Use `class << self` to define a class method.", 105664470]
+  "lib/omniauth-ldap/adaptor.rb:3925200886": [
+    [68, 7, 413, "Style/ClassMethodsDefinitions: Use `class << self` to define a class method.", 105664470]
   ],
   "spec/integration/middleware_spec.rb:4142891586": [
     [3, 16, 39, "RSpec/DescribeClass: The first argument to describe should be the class or module being tested.", 638096201],
@@ -12,10 +12,10 @@
     [4, 3, 12, "RSpec/BeforeAfterAll: Beware of using `before(:all)` as it may cause state to leak between tests. If you are using `rspec-rails`, and `use_transactional_fixtures` is enabled, then records created in `before(:all)` are not automatically rolled back.", 86334566],
     [70, 16, 5, "RSpec/ExpectActual: Provide the actual value you are testing to `expect(...)`.", 237881235]
   ],
-  "spec/omniauth-ldap/adaptor_spec.rb:321639549": [
+  "spec/omniauth-ldap/adaptor_spec.rb:1245381318": [
     [3, 1, 38, "RSpec/SpecFilePathFormat: Spec path should end with `omni_auth/ldap/adaptor*_spec.rb`.", 1973618936],
-    [225, 9, 26, "RSpec/StubbedMock: Prefer `allow` over `expect` when configuring a response.", 1924417310],
-    [226, 9, 26, "RSpec/StubbedMock: Prefer `allow` over `expect` when configuring a response.", 1924417310]
+    [239, 9, 26, "RSpec/StubbedMock: Prefer `allow` over `expect` when configuring a response.", 1924417310],
+    [240, 9, 26, "RSpec/StubbedMock: Prefer `allow` over `expect` when configuring a response.", 1924417310]
   ],
   "spec/omniauth/adaptor_spec.rb:1168013709": [
     [3, 1, 38, "RSpec/SpecFilePathFormat: Spec path should end with `omni_auth/ldap/adaptor*_spec.rb`.", 1973618936],

README.md

@@ -63,6 +63,9 @@ use OmniAuth::Strategies::LDAP,
   name_proc: proc { |name| name.gsub(/@.*$/, "") },
   bind_dn: "default_bind_dn",
   password: "password",
+  # Optional timeouts (seconds)
+  connect_timeout: 3,
+  read_timeout: 7,
   tls_options: {
     ssl_version: "TLSv1_2",
     ciphers: ["AES-128-CBC", "AES-128-CBC-HMAC-SHA1", "AES-128-CBC-HMAC-SHA256"],
@@ -194,6 +197,8 @@ The following options are available for configuring the OmniAuth LDAP strategy:
 - `:password_policy` - When true, the strategy will request the LDAP Password Policy response control (OID `1.3.6.1.4.1.42.2.27.8.5.1`) during the user bind. If the server supports it, the adaptor exposes:
   - `adaptor.last_operation_result` — the last Net::LDAP operation result object.
   - `adaptor.last_password_policy_response` — the matching password policy response control (implementation-specific object). This can indicate conditions such as password expired, account locked, reset required, or grace logins remaining (per the draft RFC).
+- `:connect_timeout` - Maximum time in seconds to wait when establishing the TCP connection to the LDAP server. Forwarded to `Net::LDAP`.
+- `:read_timeout` - Maximum time in seconds to wait for reads during LDAP operations (search/bind). Forwarded to `Net::LDAP`.
 
 Example enabling password policy:
 
@@ -719,7 +724,7 @@ To join the community or get help 👇️ Join the Discord.
 
 To say "thanks!" ☝️ Join the Discord or 👇️ send money.
 
-[![Sponsor me on GitHub Sponsors][🖇sponsor-bottom-img]][🖇sponsor] 💌 [![Sponsor me on Liberapay][⛳liberapay-bottom-img]][⛳liberapay-img] 💌 [![Donate on PayPal][🖇paypal-bottom-img]][🖇paypal-img]
+[![Sponsor me on GitHub Sponsors][🖇sponsor-bottom-img]][🖇sponsor] 💌 [![Sponsor me on Liberapay][⛳liberapay-bottom-img]][⛳liberapay] 💌 [![Donate on PayPal][🖇paypal-bottom-img]][🖇paypal]
 
 ### Please give the project a star ⭐ ♥.
 

lib/omniauth-ldap/adaptor.rb

@@ -32,6 +32,9 @@ class ConnectionError < StandardError; end
         :filter,
         :tls_options,
         :password_policy,
+        # Timeouts
+        :connect_timeout,
+        :read_timeout,
 
         # Deprecated
         :method,
@@ -89,6 +92,8 @@ def initialize(configuration = {})
           port: @port,
           encryption: encryption_options,
         }
+        # Remove passing timeouts here to avoid issues on older net-ldap versions.
+        # We'll set them after initialization if the connection responds to writers.
         @bind_method = if @try_sasl
           :sasl
         else
@@ -103,6 +108,21 @@ def initialize(configuration = {})
         }
         config[:auth] = @auth
         @connection = Net::LDAP.new(config)
+        # Apply optional timeout settings if supported by the installed net-ldap version
+        if !@connect_timeout.nil?
+          if @connection.respond_to?(:connect_timeout=)
+            @connection.connect_timeout = @connect_timeout
+          else
+            @connection.instance_variable_set(:@connect_timeout, @connect_timeout)
+          end
+        end
+        if !@read_timeout.nil?
+          if @connection.respond_to?(:read_timeout=)
+            @connection.read_timeout = @read_timeout
+          else
+            @connection.instance_variable_set(:@read_timeout, @read_timeout)
+          end
+        end
       end
 
       #:base => "dc=yourcompany, dc=com",

lib/omniauth/strategies/ldap.rb

@@ -46,6 +46,9 @@ class LDAP
       #                 standard Rack "HTTP_" variant automatically.
       option :header_auth, false
       option :header_name, "REMOTE_USER"
+      # Optional timeouts (forwarded to Net::LDAP when supported)
+      option :connect_timeout, nil
+      option :read_timeout, nil
 
       def request_phase
         # OmniAuth >= 2.0 expects the request phase to be POST-only for /auth/:provider.

spec/omniauth-ldap/adaptor_spec.rb

@@ -195,6 +195,20 @@
         expect(adaptor.connection.instance_variable_get(:@encryption)).to include method: :start_tls
       end
     end
+
+    context "when timeouts are configured" do
+      it "passes connect_timeout and read_timeout settings to Net::LDAP connection" do
+        adaptor = described_class.new(host: "192.168.1.145", encryption: "plain", base: "dc=example,dc=com", port: 389, uid: "uid", connect_timeout: 3, read_timeout: 7)
+        expect(adaptor.connection.instance_variable_get(:@connect_timeout)).to eq 3
+        expect(adaptor.connection.instance_variable_get(:@read_timeout)).to eq 7
+      end
+
+      it "omits timeout settings when not provided" do
+        adaptor = described_class.new(host: "192.168.1.145", encryption: "plain", base: "dc=example,dc=com", port: 389, uid: "uid")
+        expect(adaptor.connection.instance_variable_get(:@connect_timeout)).to be_nil
+        expect(adaptor.connection.instance_variable_get(:@read_timeout)).to be_nil
+      end
+    end
   end
 
   describe "bind_as" do