Skip to content

Commit 5ab35a0

Browse files
committed
1 parent 12b2ff6 commit 5ab35a0

File tree

4 files changed

+126
-19
lines changed

4 files changed

+126
-19
lines changed

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ Please file a bug if you notice a violation of semantic versioning.
2525
- Support for SCRIPT_NAME for proper URL generation
2626
- behind certain proxies/load balancers, or
2727
- under a subdirectory
28+
- Password Policy for LDAP Directories
29+
- password_policy: true|false (default: false)
30+
- on authentication failure, if the server returns password policy controls, the info will be included in the failure message
31+
- https://datatracker.ietf.org/doc/html/draft-behera-ldap-password-policy-11
2832

2933
### Changed
3034

lib/omniauth-ldap/adaptor.rb

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,9 @@ def bind_as(args = {})
118118
dn = rs.first.dn
119119
if dn
120120
password = args[:password]
121-
method = args[:method] || @method
122121
password = password.call if password.respond_to?(:call)
123122

124-
bind_args = if method == "sasl"
123+
bind_args = if @bind_method == :sasl
125124
sasl_auths({username: dn, password: password}).first
126125
else
127126
{
@@ -133,27 +132,13 @@ def bind_as(args = {})
133132

134133
# Optionally request LDAP Password Policy control (RFC Draft - de facto standard)
135134
if @password_policy
136-
# Best-effort: if specific control class is available use it; otherwise request by OID
137-
control = begin
138-
if defined?(Net::LDAP::Control::PasswordPolicy)
139-
Net::LDAP::Control::PasswordPolicy.new
140-
elsif defined?(Net::LDAP::Controls) && defined?(Net::LDAP::Controls::PasswordPolicy)
141-
Net::LDAP::Controls::PasswordPolicy.new
142-
elsif defined?(Net::LDAP::Control) && Net::LDAP::Control.respond_to?(:new)
143-
# Arguments: oid, critical, value
144-
Net::LDAP::Control.new("1.3.6.1.4.1.42.2.27.8.5.1", true, nil)
145-
else
146-
# Fallback to a simple hash - useful for testing with stubs
147-
{oid: "1.3.6.1.4.1.42.2.27.8.5.1", criticality: true, value: nil}
148-
end
149-
rescue StandardError
150-
{oid: "1.3.6.1.4.1.42.2.27.8.5.1", criticality: true, value: nil}
151-
end
135+
# Always request by OID using a simple hash; avoids depending on gem-specific control classes
136+
control = {oid: "1.3.6.1.4.1.42.2.27.8.5.1", criticality: true, value: nil}
152137
if bind_args.is_a?(Hash)
153138
bind_args = bind_args.merge({controls: [control]})
154139
else
155140
# Some Net::LDAP versions allow passing a block for SASL only; ensure we still can add controls if hash
156-
# If not a Hash, we can't merge; rely on server default behavior.
141+
# When not a Hash, we can't merge; rely on server default behavior.
157142
end
158143
end
159144

spec/omniauth-ldap/adaptor_spec.rb

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,4 +242,59 @@
242242
end
243243
end
244244
end
245+
246+
describe "password policy support" do
247+
let(:args) { {filter: Net::LDAP::Filter.eq("sAMAccountName", "u"), password: "p", size: 1} }
248+
let(:entry) { Struct.new(:dn).new("cn=u,dc=example,dc=com") }
249+
let(:ppolicy_oid) { "1.3.6.1.4.1.42.2.27.8.5.1" }
250+
251+
def mock_conn(opts = {})
252+
search_result = opts[:search_result]
253+
bind_result = opts.key?(:bind_result) ? opts[:bind_result] : true
254+
op_result_controls = opts[:op_result_controls] || []
255+
256+
conn = double("ldap connection")
257+
allow(conn).to receive(:open).and_yield(conn)
258+
allow(conn).to receive(:search).with(args).and_return(search_result)
259+
allow(conn).to receive(:bind) do |bind_args|
260+
@last_bind_args = bind_args
261+
bind_result
262+
end
263+
op_result = Struct.new(:controls).new(op_result_controls)
264+
allow(conn).to receive(:get_operation_result).and_return(op_result)
265+
conn
266+
end
267+
268+
it "passes a hash control with Password Policy OID and captures response control" do
269+
adaptor = described_class.new(host: "127.0.0.1", port: 389, encryption: "plain", base: "dc=example,dc=com", uid: "sAMAccountName", password_policy: true)
270+
# Response control from server (as a minimal struct exposing oid)
271+
server_ctrl = Struct.new(:oid).new(ppolicy_oid)
272+
adaptor.instance_variable_set(:@connection, mock_conn(search_result: [entry], op_result_controls: [server_ctrl]))
273+
274+
expect(adaptor.bind_as(args)).to eq entry
275+
expect(@last_bind_args[:controls].first).to include(oid: ppolicy_oid)
276+
expect(adaptor.last_password_policy_response.oid).to eq(ppolicy_oid)
277+
end
278+
279+
it "handles hash-shaped response controls from server" do
280+
adaptor = described_class.new(host: "127.0.0.1", port: 389, encryption: "plain", base: "dc=example,dc=com", uid: "sAMAccountName", password_policy: true)
281+
hash_ctrl = {oid: ppolicy_oid}
282+
adaptor.instance_variable_set(:@connection, mock_conn(search_result: [entry], op_result_controls: [hash_ctrl]))
283+
284+
expect(adaptor.bind_as(args)).to eq entry
285+
expect(@last_bind_args[:controls].first).to include(oid: ppolicy_oid)
286+
expect(adaptor.last_password_policy_response).to eq(hash_ctrl)
287+
end
288+
289+
it "attaches controls for SASL binds" do
290+
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)
291+
ctrl = Struct.new(:oid).new(ppolicy_oid)
292+
adaptor.instance_variable_set(:@connection, mock_conn(search_result: [entry], op_result_controls: [ctrl]))
293+
294+
expect(adaptor.bind_as(args)).to eq entry
295+
expect(@last_bind_args).to include(method: :sasl)
296+
expect(@last_bind_args[:controls].first).to include(oid: ppolicy_oid)
297+
expect(adaptor.last_password_policy_response.oid).to eq(ppolicy_oid)
298+
end
299+
end
245300
end

spec/omniauth/strategies/ldap_spec.rb

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,12 @@ def make_env(path = "/auth/ldap", props = {})
8181
expect(last_response.body.scan("MyLdap Form").size).to be > 1
8282
end
8383

84+
it "redirects to callback when POST includes username and password" do
85+
post "/auth/ldap", {username: "alice", password: "secret"}, {"REQUEST_METHOD" => "POST"}
86+
expect(last_response).to be_redirect
87+
expect(last_response.headers["Location"]).to eq "http://example.org/auth/ldap/callback"
88+
end
89+
8490
context "when mounted under a subdirectory" do
8591
let(:sub_env) do
8692
make_env("/auth/ldap", {
@@ -201,6 +207,41 @@ def make_env(path = "/auth/ldap", props = {})
201207
expect(last_request.env["omniauth.error"].message).to eq("Invalid credentials for ping")
202208
end
203209

210+
it "attaches password policy info to env when enabled" do
211+
allow(@adaptor).to receive(:password_policy).and_return(true)
212+
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")
213+
op = double("op", code: 49, message: "Invalid Credentials")
214+
allow(@adaptor).to receive(:last_password_policy_response).and_return(ctrl)
215+
allow(@adaptor).to receive(:last_operation_result).and_return(op)
216+
217+
post("/auth/ldap/callback", {username: "ping", password: "password"})
218+
219+
expect(last_response).to be_redirect
220+
expect(last_response.headers["Location"]).to match("invalid_credentials")
221+
222+
policy = last_request.env["omniauth.ldap.password_policy"]
223+
expect(policy).to be_a(Hash)
224+
expect(policy[:error]).to eq(:passwordExpired)
225+
expect(policy[:time_before_expiration]).to eq(42)
226+
expect(policy[:grace_authns_remaining]).to eq(1)
227+
expect(policy[:oid]).to eq("1.3.6.1.4.1.42.2.27.8.5.1")
228+
expect(policy[:operation]).to eq({code: 49, message: "Invalid Credentials"})
229+
end
230+
231+
it "maps alternate policy fields (ppolicy_error, grace_logins_remaining)" do
232+
allow(@adaptor).to receive(:password_policy).and_return(true)
233+
ctrl = double("ppolicy", ppolicy_error: :accountLocked, grace_logins_remaining: 0, oid: "1.3.6.1.4.1.42.2.27.8.5.1")
234+
allow(@adaptor).to receive(:last_password_policy_response).and_return(ctrl)
235+
allow(@adaptor).to receive(:last_operation_result).and_return(nil)
236+
237+
post("/auth/ldap/callback", {username: "ping", password: "password"})
238+
239+
expect(last_response).to be_redirect
240+
policy = last_request.env["omniauth.ldap.password_policy"]
241+
expect(policy[:error]).to eq(:accountLocked)
242+
expect(policy[:grace_authns_remaining]).to eq(0)
243+
end
244+
204245
context "and filter is set" do
205246
it "binds with filter" do
206247
allow(@adaptor).to receive(:filter).and_return("uid=%{username}")
@@ -275,6 +316,28 @@ def make_env(path = "/auth/ldap", props = {})
275316
expect(last_response).not_to be_redirect
276317
end
277318

319+
it "attaches password policy env on success when enabled" do
320+
allow(@adaptor).to receive(:password_policy).and_return(true)
321+
ctrl = double("ppolicy", oid: "1.3.6.1.4.1.42.2.27.8.5.1")
322+
allow(@adaptor).to receive(:last_password_policy_response).and_return(ctrl)
323+
allow(@adaptor).to receive(:last_operation_result).and_return(nil)
324+
325+
post("/auth/ldap/callback", {username: "ping", password: "password"})
326+
327+
expect(last_response).not_to be_redirect
328+
policy = last_request.env["omniauth.ldap.password_policy"]
329+
expect(policy).to be_a(Hash)
330+
expect(policy[:oid]).to eq("1.3.6.1.4.1.42.2.27.8.5.1")
331+
expect(policy[:raw]).to eq(ctrl)
332+
end
333+
334+
it "uses equals filter when :filter is not configured" do
335+
allow(@adaptor).to receive(:filter).and_return(nil)
336+
expect(Net::LDAP::Filter).to receive(:equals).with(@adaptor.uid, "ping").and_call_original
337+
post("/auth/ldap/callback", {username: "ping", password: "password"})
338+
expect(last_response).not_to be_redirect
339+
end
340+
278341
context "and filter is set" do
279342
it "binds with filter" do
280343
allow(@adaptor).to receive(:filter).and_return("uid=%{username}")

0 commit comments

Comments
 (0)