Bugzilla – Attachment 46659 Details for
Bug 6979
LDAP authentication fails during password comparison
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 6979 - Update tests
Bug-6979---Update-tests.patch (text/plain), 8.69 KB, created by
Alex Arnaud
on 2016-01-14 16:49:39 UTC
(
hide
)
Description:
Bug 6979 - Update tests
Filename:
MIME Type:
Creator:
Alex Arnaud
Created:
2016-01-14 16:49:39 UTC
Size:
8.69 KB
patch
obsolete
>From efd62e4979d1ee7165142376a6912e89c4fca69b Mon Sep 17 00:00:00 2001 >From: Alex Arnaud <alex.arnaud@biblibre.com> >Date: Thu, 14 Jan 2016 17:48:56 +0100 >Subject: [PATCH] Bug 6979 - Update tests > >--- > C4/Auth_with_ldap.pm | 5 ++- > t/db_dependent/Auth_with_ldap.t | 87 ++++++++++++++++----------------------- > 2 files changed, 39 insertions(+), 53 deletions(-) > >diff --git a/C4/Auth_with_ldap.pm b/C4/Auth_with_ldap.pm >index 295c8fc..5458245 100644 >--- a/C4/Auth_with_ldap.pm >+++ b/C4/Auth_with_ldap.pm >@@ -60,6 +60,7 @@ my $prefhost = $ldap->{hostname} or die ldapserver_error('hostname'); > my $base = $ldap->{base} or die ldapserver_error('base'); > $ldapname = $ldap->{user} ; > $ldappassword = $ldap->{pass} ; >+$ldap->{anonymous_bind} = 1 unless $ldapname && $ldappassword; > our %mapping = %{$ldap->{mapping}}; # FIXME dpavlin -- don't die because of || (); from 6eaf8511c70eb82d797c941ef528f4310a15e9f9 > my @mapkeys = keys %mapping; > $debug and print STDERR "Got ", scalar(@mapkeys), " ldap mapkeys ( total ): ", join ' ', @mapkeys, "\n"; >@@ -67,7 +68,6 @@ $debug and print STDERR "Got ", scalar(@mapkeys), " ldap mapkeys ( total ): ", > $debug and print STDERR "Got ", scalar(@mapkeys), " ldap mapkeys (populated): ", join ' ', @mapkeys, "\n"; > > my %config = ( >- anonymous => ($ldapname and $ldappassword) ? 0 : 1, > replicate => defined($ldap->{replicate}) ? $ldap->{replicate} : 1, # add from LDAP to Koha database for new user > update => defined($ldap->{update} ) ? $ldap->{update} : 1, # update from LDAP to Koha database for existing user > ); >@@ -167,7 +167,7 @@ sub checkpw_ldap { > $userldapentry = $search->shift_entry; > } > } else { >- my $res = ($config{anonymous}) ? $db->bind : $db->bind($ldapname, password=>$ldappassword); >+ my $res = ($ldap->{anonymous_bind}) ? $db->bind : $db->bind($ldapname, password=>$ldappassword); > if ($res->code) { # connection refused > warn "LDAP bind failed as ldapuser " . ($ldapname || '[ANONYMOUS]') . ": " . description($res); > return 0; >@@ -180,6 +180,7 @@ sub checkpw_ldap { > warn "LDAP Auth rejected : invalid password for user '$userid'. " . description($user_ldap_bind_ret); > return -1; > } >+ > } > > # To get here, LDAP has accepted our user's login attempt. >diff --git a/t/db_dependent/Auth_with_ldap.t b/t/db_dependent/Auth_with_ldap.t >index 4982248..eb0b932 100755 >--- a/t/db_dependent/Auth_with_ldap.t >+++ b/t/db_dependent/Auth_with_ldap.t >@@ -37,11 +37,11 @@ my $anonymous_bind = 1; > # Variables controlling LDAP behaviour > my $desired_authentication_result = 'success'; > my $desired_connection_result = 'error'; >-my $desired_bind_result = 'error'; >+my $desired_admin_bind_result = 'error'; > my $desired_compare_result = 'error'; > my $desired_search_result = 'error'; > my $desired_count_result = 1; >-my $non_anonymous_bind_result = 'error'; >+my $desired_bind_result = 'error'; > my $ret; > > # Mock the context module >@@ -90,7 +90,7 @@ subtest "checkpw_ldap tests" => sub { > > $desired_authentication_result = 'success'; > $anonymous_bind = 1; >- $desired_bind_result = 'error'; >+ $desired_admin_bind_result = 'error'; > $desired_search_result = 'error'; > reload_ldap_module(); > >@@ -103,15 +103,15 @@ subtest "checkpw_ldap tests" => sub { > > $desired_authentication_result = 'success'; > $anonymous_bind = 1; >- $desired_bind_result = 'success'; >+ $desired_admin_bind_result = 'success'; > $desired_search_result = 'success'; > $desired_count_result = 0; # user auth problem >- $non_anonymous_bind_result = 'success'; >+ $desired_bind_result = 'success'; > reload_ldap_module(); > is ( C4::Auth_with_ldap::checkpw_ldap( $dbh, 'hola', password => 'hey' ), > 0, "checkpw_ldap returns 0 if user lookup returns 0"); > >- $non_anonymous_bind_result = 'error'; >+ $desired_bind_result = 'error'; > reload_ldap_module(); > > warning_like { $ret = C4::Auth_with_ldap::checkpw_ldap( >@@ -123,10 +123,10 @@ subtest "checkpw_ldap tests" => sub { > # regression tests for bug 12831 > $desired_authentication_result = 'error'; > $anonymous_bind = 0; >- $desired_bind_result = 'error'; >+ $desired_admin_bind_result = 'error'; > $desired_search_result = 'success'; > $desired_count_result = 0; # user auth problem >- $non_anonymous_bind_result = 'error'; >+ $desired_bind_result = 'error'; > reload_ldap_module(); > > warning_like { $ret = C4::Auth_with_ldap::checkpw_ldap( >@@ -145,8 +145,8 @@ subtest "checkpw_ldap tests" => sub { > > # Anonymous bind > $anonymous_bind = 1; >- $desired_bind_result = 'error'; >- $non_anonymous_bind_result = 'error'; >+ $desired_admin_bind_result = 'error'; >+ $desired_bind_result = 'error'; > reload_ldap_module(); > > warning_like { $ret = C4::Auth_with_ldap::checkpw_ldap( >@@ -156,9 +156,8 @@ subtest "checkpw_ldap tests" => sub { > is ( $ret, 0, "checkpw_ldap returns 0 if bind fails"); > > $anonymous_bind = 1; >- $desired_bind_result = 'success'; >- $non_anonymous_bind_result = 'success'; >- $desired_compare_result = 'error'; >+ $desired_admin_bind_result = 'success'; >+ $desired_bind_result = 'error'; > reload_ldap_module(); > > warning_like { $ret = C4::Auth_with_ldap::checkpw_ldap( >@@ -169,9 +168,8 @@ subtest "checkpw_ldap tests" => sub { > > # Non-anonymous bind > $anonymous_bind = 0; >- $desired_bind_result = 'success'; >- $non_anonymous_bind_result = 'error'; >- $desired_compare_result = 'dont care'; >+ $desired_admin_bind_result = 'error'; >+ $desired_bind_result = 'error'; > reload_ldap_module(); > > warning_like { $ret = C4::Auth_with_ldap::checkpw_ldap( >@@ -181,9 +179,8 @@ subtest "checkpw_ldap tests" => sub { > is ( $ret, 0, "checkpw_ldap returns 0 if bind fails"); > > $anonymous_bind = 0; >- $desired_bind_result = 'success'; >- $non_anonymous_bind_result = 'success'; >- $desired_compare_result = 'error'; >+ $desired_admin_bind_result = 'success'; >+ $desired_bind_result = 'error'; > reload_ldap_module(); > > warning_like { $ret = C4::Auth_with_ldap::checkpw_ldap( >@@ -265,41 +262,19 @@ sub mock_net_ldap { > my $mocked_ldap = Test::MockObject->new(); > > $mocked_ldap->mock( 'bind', sub { >- >- my @args = @_; >- my $mocked_message; >- >- if ( $#args > 1 ) { >- # Args passed => non-anonymous bind >- if ( $non_anonymous_bind_result eq 'error' ) { >- return mock_net_ldap_message(1,1,'error_name','error_text'); >- } else { >- return mock_net_ldap_message(0,0,'',''); >- } >- } else { >- $mocked_message = mock_net_ldap_message( >- ($desired_bind_result eq 'error' ) ? 1 : 0, # code >- ($desired_bind_result eq 'error' ) ? 1 : 0, # error >- ($desired_bind_result eq 'error' ) ? 'error_name' : 0, # error_name >- ($desired_bind_result eq 'error' ) ? 'error_text' : 0 # error_text >+ if (is_admin_bind(@_)) { >+ return mock_net_ldap_message( >+ ($desired_admin_bind_result eq 'error' ) ? 1 : 0, # code >+ ($desired_admin_bind_result eq 'error' ) ? 1 : 0, # error >+ ($desired_admin_bind_result eq 'error' ) ? 'error_name' : 0, # error_name >+ ($desired_admin_bind_result eq 'error' ) ? 'error_text' : 0 # error_text > ); >- } >- >- return $mocked_message; >- }); >- >- $mocked_ldap->mock( 'compare', sub { >- >- my $mocked_message; >- >- if ( $desired_compare_result eq 'error' ) { >- $mocked_message = mock_net_ldap_message(1,1,'error_name','error_text'); > } else { >- # we expect return code 6 for success >- $mocked_message = mock_net_ldap_message(6,0,'',''); >+ if ( $desired_bind_result eq 'error' ) { >+ return mock_net_ldap_message(1,1,'error_name','error_text'); >+ } >+ return mock_net_ldap_message(0,0,'',''); > } >- >- return $mocked_message; > }); > > $mocked_ldap->mock( 'search', sub { >@@ -374,6 +349,16 @@ sub reload_ldap_module { > C4::Auth_with_ldap->import; > } > >+sub is_admin_bind { >+ my @args = @_; >+ >+ if ($#args <= 1 || $args[1] eq 'cn=Manager,dc=metavore,dc=com') { >+ return 1; >+ } >+ >+ return 0; >+} >+ > $dbh->rollback; > > 1; >-- >1.7.10.4
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 6979
:
5723
|
46646
|
46654
|
46659
|
46698
|
46863
|
57480
|
57481
|
57482
|
57483
|
57963
|
57964
|
57965
|
57966
|
61295
|
61296
|
61297
|
61298
|
61310
|
61311
|
61312
|
61313