Merge branch 'improve-omniauth-ldap-failure-log-message' into 'master'

Improves log message when invalid credentials are used

See merge request !11

Author: Douwe Maan

CHANGELOG

@@ -1,2 +1,5 @@
+## 2.0.4
+  - Improve log message when invalid credentials are used
+
 ## 2.0.3
   - Protects against wrong request method call to callback

lib/omniauth-ldap/version.rb

@@ -1,5 +1,5 @@
 module OmniAuth
   module LDAP
-    VERSION = "2.0.3"
+    VERSION = "2.0.4"
   end
 end

lib/omniauth/strategies/ldap.rb

@@ -4,6 +4,9 @@ module OmniAuth
   module Strategies
     class LDAP
       include OmniAuth::Strategy
+
+      InvalidCredentialsError = Class.new(StandardError)
+
       @@config = {
         'name' => 'cn',
         'first_name' => 'givenName',
@@ -45,7 +48,9 @@ def callback_phase
         begin
           @ldap_user_info = @adaptor.bind_as(:filter => filter(@adaptor), :size => 1, :password => request['password'])
 
-          return fail!(:invalid_credentials) if !@ldap_user_info
+          unless @ldap_user_info
+            return fail!(:invalid_credentials, InvalidCredentialsError.new("Invalid credentials for #{request['username']}"))
+          end
 
           @user_info = self.class.map_user(@@config, @ldap_user_info)
           super
@@ -54,7 +59,7 @@ def callback_phase
         end
       end
 
-      def filter adaptor
+      def filter(adaptor)
         if adaptor.filter and !adaptor.filter.empty?
           username = Net::LDAP::Filter.escape(@options[:name_proc].call(request['username']))
           Net::LDAP::Filter.construct(adaptor.filter % { username: username })

spec/omniauth/strategies/ldap_spec.rb

@@ -69,8 +69,10 @@ class MyLdapProvider < OmniAuth::Strategies::LDAP; end
 
       it 'should redirect to error page' do
         post('/auth/ldap/callback', {:username => 'ping', :password => 'password'})
-        last_response.should be_redirect
-        last_response.headers['Location'].should =~ %r{invalid_credentials}
+
+        expect(last_response).to be_redirect
+        expect(last_response.headers['Location']).to match('invalid_credentials')
+        expect(last_request.env['omniauth.error'].message).to eq('Invalid credentials for ping')
       end
 
       it 'should redirect to error page when there is exception' do
@@ -132,17 +134,19 @@ class MyLdapProvider < OmniAuth::Strategies::LDAP; end
           it 'should redirect to error page' do
             post('/auth/ldap/callback', {:username => 'ping', :password => 'password'})
 
-            last_response.should be_redirect
-            last_response.headers['Location'].should =~ %r{invalid_credentials}
+            expect(last_response).to be_redirect
+            expect(last_response.headers['Location']).to match('invalid_credentials')
+            expect(last_request.env['omniauth.error'].message).to eq('Invalid credentials for ping')
           end
           context 'and filter is set' do
             it 'should bind with filter' do
               @adaptor.stub(:filter).and_return('uid=%{username}')
               Net::LDAP::Filter.should_receive(:construct).with('uid=ping')
               post('/auth/ldap/callback', {:username => 'ping', :password => 'password'})
 
-              last_response.should be_redirect
-              last_response.headers['Location'].should =~ %r{invalid_credentials}
+              expect(last_response).to be_redirect
+              expect(last_response.headers['Location']).to match('invalid_credentials')
+              expect(last_request.env['omniauth.error'].message).to eq('Invalid credentials for ping')
             end
           end