Merge pull request #109 from omniauth/feat/ldap-error-on-misconfiguration

Author: pboling

.rubocop_gradual.lock

@@ -1,5 +1,5 @@
 {
-  "lib/omniauth-ldap/adaptor.rb:3925200886": [
+  "lib/omniauth-ldap/adaptor.rb:928056405": [
     [68, 7, 413, "Style/ClassMethodsDefinitions: Use `class << self` to define a class method.", 105664470]
   ],
   "spec/integration/middleware_spec.rb:2185613788": [
@@ -12,7 +12,7 @@
     [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:1245381318": [
+  "spec/omniauth-ldap/adaptor_spec.rb:1839434473": [
     [3, 1, 38, "RSpec/SpecFilePathFormat: Spec path should end with `omni_auth/ldap/adaptor*_spec.rb`.", 1973618936],
     [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]

CHANGELOG.md

@@ -31,6 +31,8 @@ Please file a bug if you notice a violation of semantic versioning.
   - https://datatracker.ietf.org/doc/html/draft-behera-ldap-password-policy-11
 - Support for JSON bodies
 - Support custom LDAP attributes mapping
+- Raise a distinct error when LDAP server is unreachable
+  - Previously raised an invalid credentials authentication failure error, which is technically incorrect
 
 ### Changed
 

lib/omniauth-ldap/adaptor.rb

@@ -134,6 +134,8 @@ def bind_as(args = {})
         @last_password_policy_response = nil
         @connection.open do |me|
           rs = me.search(args)
+          raise ConnectionError.new("LDAP search operation failed") unless rs
+
           if rs && rs.first
             dn = rs.first.dn
             if dn

spec/omniauth-ldap/adaptor_spec.rb

@@ -310,5 +310,14 @@ def mock_conn(opts = {})
       expect(@last_bind_args[:controls].first).to include(oid: ppolicy_oid)
       expect(adaptor.last_password_policy_response.oid).to eq(ppolicy_oid)
     end
+
+    it "raises a ConnectionError if the bind fails" do
+      adaptor = described_class.new({host: "192.168.1.126", method: "plain", base: "dc=score, dc=local", port: 389, uid: "sAMAccountName", bind_dn: "bind_dn", password: "password"})
+      allow(adaptor.connection).to receive(:open).and_yield(adaptor.connection)
+      # Net::LDAP#search returns nil if the operation was not successful
+      allow(adaptor.connection).to receive(:search).with(args).and_return(nil)
+      expect(adaptor.connection).not_to receive(:bind)
+      expect { adaptor.bind_as(args) }.to raise_error described_class::ConnectionError
+    end
   end
 end