✨ Password Policy for LDAP Directories

- https://datatracker.ietf.org/doc/html/draft-behera-ldap-password-policy-11

Author: pboling

.rubocop_gradual.lock

@@ -1,9 +1,6 @@
 {
-  "lib/omniauth-ldap/adaptor.rb:715274645": [
-    [64, 7, 413, "Style/ClassMethodsDefinitions: Use `class << self` to define a class method.", 105664470],
-    [114, 17, 3, "Style/AndOr: Use `&&` instead of `and`.", 193409806],
-    [114, 30, 3, "Style/AndOr: Use `&&` instead of `and`.", 193409806],
-    [114, 37, 1, "Lint/AssignmentInCondition: Wrap assignment in parentheses if intentional", 177560]
+  "lib/omniauth-ldap/adaptor.rb:3212021924": [
+    [65, 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],
@@ -15,29 +12,25 @@
     [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:2715031579": [
+  "spec/omniauth-ldap/adaptor_spec.rb:321639549": [
     [3, 1, 38, "RSpec/SpecFilePathFormat: Spec path should end with `omni_auth/ldap/adaptor*_spec.rb`.", 1973618936],
-    [206, 7, 26, "RSpec/StubbedMock: Prefer `allow` over `expect` when configuring a response.", 1924417310],
-    [207, 7, 26, "RSpec/StubbedMock: Prefer `allow` over `expect` when configuring a response.", 1924417310],
-    [208, 7, 26, "RSpec/StubbedMock: Prefer `allow` over `expect` when configuring a response.", 1924417310],
-    [214, 7, 26, "RSpec/StubbedMock: Prefer `allow` over `expect` when configuring a response.", 1924417310],
-    [215, 7, 26, "RSpec/StubbedMock: Prefer `allow` over `expect` when configuring a response.", 1924417310],
-    [216, 7, 26, "RSpec/StubbedMock: Prefer `allow` over `expect` when configuring a response.", 1924417310]
+    [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]
   ],
   "spec/omniauth/adaptor_spec.rb:1168013709": [
     [3, 1, 38, "RSpec/SpecFilePathFormat: Spec path should end with `omni_auth/ldap/adaptor*_spec.rb`.", 1973618936],
     [46, 7, 38, "RSpec/AnyInstance: Avoid stubbing using `allow_any_instance_of`.", 3627954156],
     [47, 7, 38, "RSpec/AnyInstance: Avoid stubbing using `allow_any_instance_of`.", 3627954156],
     [84, 7, 48, "RSpec/AnyInstance: Avoid stubbing using `allow_any_instance_of`.", 2759780562]
   ],
-  "spec/omniauth/strategies/ldap_spec.rb:2044523926": [
-    [120, 13, 9, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 1130140517],
-    [175, 17, 28, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 3444838747],
-    [184, 17, 23, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 1584148894],
-    [195, 17, 32, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 1515076977],
-    [204, 19, 19, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 2526348694],
-    [230, 17, 56, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 2413495789],
-    [245, 13, 9, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 3182939526],
-    [278, 15, 19, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 2526348694]
+  "spec/omniauth/strategies/ldap_spec.rb:1834231975": [
+    [126, 13, 9, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 1130140517],
+    [181, 17, 28, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 3444838747],
+    [190, 17, 23, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 1584148894],
+    [201, 17, 32, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 1515076977],
+    [243, 19, 19, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 2526348694],
+    [269, 17, 56, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 2413495789],
+    [284, 13, 9, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 3182939526],
+    [338, 15, 19, "RSpec/ContextWording: Context description should match /^when\\b/, /^with\\b/, or /^without\\b/.", 2526348694]
   ]
 }

CHANGELOG.md

@@ -25,6 +25,10 @@ Please file a bug if you notice a violation of semantic versioning.
 - Support for SCRIPT_NAME for proper URL generation
   - behind certain proxies/load balancers, or
   - under a subdirectory
+- Password Policy for LDAP Directories
+  - password_policy: true|false (default: false)
+  - on authentication failure, if the server returns password policy controls, the info will be included in the failure message
+  - https://datatracker.ietf.org/doc/html/draft-behera-ldap-password-policy-11
 
 ### Changed
 

lib/omniauth-ldap/adaptor.rb

@@ -118,10 +118,9 @@ def bind_as(args = {})
             dn = rs.first.dn
             if dn
               password = args[:password]
-              method = args[:method] || @method
               password = password.call if password.respond_to?(:call)
 
-              bind_args = if method == "sasl"
+              bind_args = if @bind_method == :sasl
                 sasl_auths({username: dn, password: password}).first
               else
                 {
@@ -133,27 +132,13 @@ def bind_as(args = {})
 
               # Optionally request LDAP Password Policy control (RFC Draft - de facto standard)
               if @password_policy
-                # Best-effort: if specific control class is available use it; otherwise request by OID
-                control = begin
-                  if defined?(Net::LDAP::Control::PasswordPolicy)
-                    Net::LDAP::Control::PasswordPolicy.new
-                  elsif defined?(Net::LDAP::Controls) && defined?(Net::LDAP::Controls::PasswordPolicy)
-                    Net::LDAP::Controls::PasswordPolicy.new
-                  elsif defined?(Net::LDAP::Control) && Net::LDAP::Control.respond_to?(:new)
-                    # Arguments: oid, critical, value
-                    Net::LDAP::Control.new("1.3.6.1.4.1.42.2.27.8.5.1", true, nil)
-                  else
-                    # Fallback to a simple hash - useful for testing with stubs
-                    {oid: "1.3.6.1.4.1.42.2.27.8.5.1", criticality: true, value: nil}
-                  end
-                rescue StandardError
-                  {oid: "1.3.6.1.4.1.42.2.27.8.5.1", criticality: true, value: nil}
-                end
+                # Always request by OID using a simple hash; avoids depending on gem-specific control classes
+                control = {oid: "1.3.6.1.4.1.42.2.27.8.5.1", criticality: true, value: nil}
                 if bind_args.is_a?(Hash)
                   bind_args = bind_args.merge({controls: [control]})
                 else
                   # Some Net::LDAP versions allow passing a block for SASL only; ensure we still can add controls if hash
-                  # If not a Hash, we can't merge; rely on server default behavior.
+                  # When not a Hash, we can't merge; rely on server default behavior.
                 end
               end
 

spec/omniauth-ldap/adaptor_spec.rb

@@ -203,17 +203,17 @@
 
     it "binds simple" do
       adaptor = described_class.new({host: "192.168.1.126", encryption: "plain", base: "dc=score, dc=local", port: 389, uid: "sAMAccountName", bind_dn: "bind_dn", password: "password"})
-      expect(adaptor.connection).to receive(:open).and_yield(adaptor.connection)
-      expect(adaptor.connection).to receive(:search).with(args).and_return([rs])
-      expect(adaptor.connection).to receive(:bind).with({username: "new dn", password: args[:password], method: :simple}).and_return(true)
+      allow(adaptor.connection).to receive(:open).and_yield(adaptor.connection)
+      allow(adaptor.connection).to receive(:search).with(args).and_return([rs])
+      allow(adaptor.connection).to receive(:bind).with({username: "new dn", password: args[:password], method: :simple}).and_return(true)
       expect(adaptor.bind_as(args)).to eq rs
     end
 
     it "binds sasl" do
       adaptor = described_class.new({host: "192.168.1.145", encryption: "plain", base: "dc=intridea, dc=com", port: 389, uid: "sAMAccountName", try_sasl: true, sasl_mechanisms: ["GSS-SPNEGO"], bind_dn: "bind_dn", password: "password"})
-      expect(adaptor.connection).to receive(:open).and_yield(adaptor.connection)
-      expect(adaptor.connection).to receive(:search).with(args).and_return([rs])
-      expect(adaptor.connection).to receive(:bind).and_return(true)
+      allow(adaptor.connection).to receive(:open).and_yield(adaptor.connection)
+      allow(adaptor.connection).to receive(:search).with(args).and_return([rs])
+      allow(adaptor.connection).to receive(:bind).and_return(true)
       expect(adaptor.bind_as(args)).to eq rs
     end
 
@@ -242,4 +242,59 @@
       end
     end
   end
+
+  describe "password policy support" do
+    let(:args) { {filter: Net::LDAP::Filter.eq("sAMAccountName", "u"), password: "p", size: 1} }
+    let(:entry) { Struct.new(:dn).new("cn=u,dc=example,dc=com") }
+    let(:ppolicy_oid) { "1.3.6.1.4.1.42.2.27.8.5.1" }
+
+    def mock_conn(opts = {})
+      search_result = opts[:search_result]
+      bind_result = opts.key?(:bind_result) ? opts[:bind_result] : true
+      op_result_controls = opts[:op_result_controls] || []
+
+      conn = double("ldap connection")
+      allow(conn).to receive(:open).and_yield(conn)
+      allow(conn).to receive(:search).with(args).and_return(search_result)
+      allow(conn).to receive(:bind) do |bind_args|
+        @last_bind_args = bind_args
+        bind_result
+      end
+      op_result = Struct.new(:controls).new(op_result_controls)
+      allow(conn).to receive(:get_operation_result).and_return(op_result)
+      conn
+    end
+
+    it "passes a hash control with Password Policy OID and captures response control" do
+      adaptor = described_class.new(host: "127.0.0.1", port: 389, encryption: "plain", base: "dc=example,dc=com", uid: "sAMAccountName", password_policy: true)
+      # Response control from server (as a minimal struct exposing oid)
+      server_ctrl = Struct.new(:oid).new(ppolicy_oid)
+      adaptor.instance_variable_set(:@connection, mock_conn(search_result: [entry], op_result_controls: [server_ctrl]))
+
+      expect(adaptor.bind_as(args)).to eq entry
+      expect(@last_bind_args[:controls].first).to include(oid: ppolicy_oid)
+      expect(adaptor.last_password_policy_response.oid).to eq(ppolicy_oid)
+    end
+
+    it "handles hash-shaped response controls from server" do
+      adaptor = described_class.new(host: "127.0.0.1", port: 389, encryption: "plain", base: "dc=example,dc=com", uid: "sAMAccountName", password_policy: true)
+      hash_ctrl = {oid: ppolicy_oid}
+      adaptor.instance_variable_set(:@connection, mock_conn(search_result: [entry], op_result_controls: [hash_ctrl]))
+
+      expect(adaptor.bind_as(args)).to eq entry
+      expect(@last_bind_args[:controls].first).to include(oid: ppolicy_oid)
+      expect(adaptor.last_password_policy_response).to eq(hash_ctrl)
+    end
+
+    it "attaches controls for SASL binds" do
+      adaptor = described_class.new(host: "127.0.0.1", port: 389, encryption: "plain", base: "dc=example,dc=com", uid: "sAMAccountName", try_sasl: true, sasl_mechanisms: ["DIGEST-MD5"], bind_dn: "bind_dn", password: "bind_pw", password_policy: true)
+      ctrl = Struct.new(:oid).new(ppolicy_oid)
+      adaptor.instance_variable_set(:@connection, mock_conn(search_result: [entry], op_result_controls: [ctrl]))
+
+      expect(adaptor.bind_as(args)).to eq entry
+      expect(@last_bind_args).to include(method: :sasl)
+      expect(@last_bind_args[:controls].first).to include(oid: ppolicy_oid)
+      expect(adaptor.last_password_policy_response.oid).to eq(ppolicy_oid)
+    end
+  end
 end

spec/omniauth/strategies/ldap_spec.rb

@@ -81,6 +81,12 @@ def make_env(path = "/auth/ldap", props = {})
       expect(last_response.body.scan("MyLdap Form").size).to be > 1
     end
 
+    it "redirects to callback when POST includes username and password" do
+      post "/auth/ldap", {username: "alice", password: "secret"}, {"REQUEST_METHOD" => "POST"}
+      expect(last_response).to be_redirect
+      expect(last_response.headers["Location"]).to eq "http://example.org/auth/ldap/callback"
+    end
+
     context "when mounted under a subdirectory" do
       let(:sub_env) do
         make_env("/auth/ldap", {
@@ -201,6 +207,39 @@ def make_env(path = "/auth/ldap", props = {})
             expect(last_request.env["omniauth.error"].message).to eq("Invalid credentials for ping")
           end
 
+          it "attaches password policy info to env when enabled" do
+            allow(@adaptor).to receive(:password_policy).and_return(true)
+            ctrl = double("ppolicy", error: :passwordExpired, time_before_expiration: 42, grace_authns_remaining: 1, oid: "1.3.6.1.4.1.42.2.27.8.5.1")
+            op = double("op", code: 49, message: "Invalid Credentials")
+            allow(@adaptor).to receive_messages(last_password_policy_response: ctrl, last_operation_result: op)
+
+            post("/auth/ldap/callback", {username: "ping", password: "password"})
+
+            expect(last_response).to be_redirect
+            expect(last_response.headers["Location"]).to match("invalid_credentials")
+
+            policy = last_request.env["omniauth.ldap.password_policy"]
+            expect(policy).to be_a(Hash)
+            expect(policy[:error]).to eq(:passwordExpired)
+            expect(policy[:time_before_expiration]).to eq(42)
+            expect(policy[:grace_authns_remaining]).to eq(1)
+            expect(policy[:oid]).to eq("1.3.6.1.4.1.42.2.27.8.5.1")
+            expect(policy[:operation]).to eq({code: 49, message: "Invalid Credentials"})
+          end
+
+          it "maps alternate policy fields (ppolicy_error, grace_logins_remaining)" do
+            allow(@adaptor).to receive(:password_policy).and_return(true)
+            ctrl = double("ppolicy", ppolicy_error: :accountLocked, grace_logins_remaining: 0, oid: "1.3.6.1.4.1.42.2.27.8.5.1")
+            allow(@adaptor).to receive_messages(last_password_policy_response: ctrl, last_operation_result: nil)
+
+            post("/auth/ldap/callback", {username: "ping", password: "password"})
+
+            expect(last_response).to be_redirect
+            policy = last_request.env["omniauth.ldap.password_policy"]
+            expect(policy[:error]).to eq(:accountLocked)
+            expect(policy[:grace_authns_remaining]).to eq(0)
+          end
+
           context "and filter is set" do
             it "binds with filter" do
               allow(@adaptor).to receive(:filter).and_return("uid=%{username}")
@@ -275,6 +314,27 @@ def make_env(path = "/auth/ldap", props = {})
         expect(last_response).not_to be_redirect
       end
 
+      it "attaches password policy env on success when enabled" do
+        allow(@adaptor).to receive(:password_policy).and_return(true)
+        ctrl = double("ppolicy", oid: "1.3.6.1.4.1.42.2.27.8.5.1")
+        allow(@adaptor).to receive_messages(last_password_policy_response: ctrl, last_operation_result: nil)
+
+        post("/auth/ldap/callback", {username: "ping", password: "password"})
+
+        expect(last_response).not_to be_redirect
+        policy = last_request.env["omniauth.ldap.password_policy"]
+        expect(policy).to be_a(Hash)
+        expect(policy[:oid]).to eq("1.3.6.1.4.1.42.2.27.8.5.1")
+        expect(policy[:raw]).to eq(ctrl)
+      end
+
+      it "uses equals filter when :filter is not configured" do
+        allow(@adaptor).to receive(:filter).and_return(nil)
+        expect(Net::LDAP::Filter).to receive(:equals).with(@adaptor.uid, "ping").and_call_original
+        post("/auth/ldap/callback", {username: "ping", password: "password"})
+        expect(last_response).not_to be_redirect
+      end
+
       context "and filter is set" do
         it "binds with filter" do
           allow(@adaptor).to receive(:filter).and_return("uid=%{username}")