Bugzilla – Attachment 76879 Details for
Bug 18947
Unexpected Active Directory LDAP authentication failure mode
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 18947 - LDAP: do not assume anonymous bind if no user or password
Bug-18947---LDAP-do-not-assume-anonymous-bind-if-n.patch (text/plain), 6.09 KB, created by
Martin Renvoize
on 2018-07-12 09:52:21 UTC
(
hide
)
Description:
Bug 18947 - LDAP: do not assume anonymous bind if no user or password
Filename:
MIME Type:
Creator:
Martin Renvoize
Created:
2018-07-12 09:52:21 UTC
Size:
6.09 KB
patch
obsolete
>From a82533cfa404f4ae0f67e18c9bbed67f17f43a63 Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >Date: Wed, 13 Dec 2017 13:27:36 +0000 >Subject: [PATCH] Bug 18947 - LDAP: do not assume anonymous bind if no user or > password > >To test: >Ideally tested on a working ldap server with bind by auth and no >anonymous bind >1 - Define an LDAP config with bind by auth >2 - Don't define user/pass >3 - Define anonymous_bind = 0 >4 - Attempt bind by auth >5 - Error is something like: >LDAP search failed to return object : XXXXXXXXX: LdapErr: XXXX-XXXXXX, > comment: In order to perform this operation a successful bind must > be completed on the connection., data 0, v2580 at > /usr/share/koha/lib/C4/Auth_with_ldap.pm line 102. >6 - Define user/pass >7 - Now bind by auth should work >8 - remove user/pass >9 - Apply patch >10 - Attempt again >11 - Bind by auth shoudl succeed > >prove -v t/db_dependent/Auth_with_ldap.t > >Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> >--- > C4/Auth_with_ldap.pm | 9 ++++----- > t/db_dependent/Auth_with_ldap.t | 25 +++++++++++++++++++++---- > 2 files changed, 25 insertions(+), 9 deletions(-) > >diff --git a/C4/Auth_with_ldap.pm b/C4/Auth_with_ldap.pm >index bbb1094229..70879b1c95 100644 >--- a/C4/Auth_with_ldap.pm >+++ b/C4/Auth_with_ldap.pm >@@ -60,7 +60,6 @@ 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"; >@@ -77,7 +76,7 @@ if(defined $ldap->{categorycode_mapping}) { > } > > my %config = ( >- anonymous => ($ldapname and $ldappassword) ? 0 : 1, >+ anonymous => defined ($ldap->{anonymous_bind}) ? $ldap->{anonymous_bind} : 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 > ); >@@ -128,7 +127,7 @@ sub checkpw_ldap { > # first, LDAP authentication > if ( $ldap->{auth_by_bind} ) { > my $principal_name; >- if ( $ldap->{anonymous_bind} ) { >+ if ( $config{anonymous} ) { > > # Perform an anonymous bind > my $res = $db->bind; >@@ -156,7 +155,7 @@ sub checkpw_ldap { > # Perform a LDAP bind for the given username using the matched DN > my $res = $db->bind( $principal_name, password => $password ); > if ( $res->code ) { >- if ( $ldap->{anonymous_bind} ) { >+ if ( $config{anonymous} ) { > # With anonymous_bind approach we can be sure we have found the correct user > # and that any 'code' response indicates a 'bad' user (be that blocked, banned > # or password changed). We should not fall back to local accounts in this case. >@@ -177,7 +176,7 @@ sub checkpw_ldap { > $userldapentry = $search->shift_entry; > } > } else { >- my $res = ($ldap->{anonymous_bind}) ? $db->bind : $db->bind($ldapname, password=>$ldappassword); >+ my $res = ($config{anonymous}) ? $db->bind : $db->bind($ldapname, password=>$ldappassword); > if ($res->code) { # connection refused > warn "LDAP bind failed as ldapuser " . ($ldapname || '[ANONYMOUS]') . ": " . description($res); > return 0; >diff --git a/t/db_dependent/Auth_with_ldap.t b/t/db_dependent/Auth_with_ldap.t >index 1a1c3d048f..e0c3c4532c 100755 >--- a/t/db_dependent/Auth_with_ldap.t >+++ b/t/db_dependent/Auth_with_ldap.t >@@ -41,6 +41,8 @@ my $update = 0; > my $replicate = 0; > my $auth_by_bind = 1; > my $anonymous_bind = 1; >+my $user = 'cn=Manager,dc=metavore,dc=com'; >+my $pass = 'metavore'; > > # Variables controlling LDAP behaviour > my $desired_authentication_result = 'success'; >@@ -143,7 +145,7 @@ subtest 'checkpw_ldap tests' => sub { > > subtest 'auth_by_bind = 1 tests' => sub { > >- plan tests => 9; >+ plan tests => 11; > > $auth_by_bind = 1; > >@@ -161,6 +163,19 @@ subtest 'checkpw_ldap tests' => sub { > 'checkpw_ldap prints correct warning if LDAP anonymous bind fails'; > is( $ret, 0, 'checkpw_ldap returns 0 if LDAP anonymous bind fails' ); > >+ $anonymous_bind = 0; >+ $user = undef; >+ $pass = undef; >+ reload_ldap_module(); >+ >+ warning_like { >+ $ret = C4::Auth_with_ldap::checkpw_ldap( $dbh, 'hola', >+ password => 'hey' ); >+ } >+ qr/LDAP bind failed as kohauser hola: LDAP error #1: error_name/, >+ 'checkpw_ldap prints correct warning if LDAP bind_by_auth fails'; >+ is( $ret, 0, 'checkpw_ldap returns 0 if LDAP bind_by_auth fails' ); >+ > $desired_authentication_result = 'success'; > $anonymous_bind = 1; > $desired_admin_bind_result = 'success'; >@@ -251,8 +266,10 @@ subtest 'checkpw_ldap tests' => sub { > > # Anonymous bind > $anonymous_bind = 1; >+ $user = 'cn=Manager,dc=metavore,dc=com'; >+ $pass = 'metavore'; > $desired_admin_bind_result = 'error'; >- $desired_bind_result = 'error'; >+ $desired_bind_result = 'error'; > reload_ldap_module(); > > warning_like { >@@ -361,11 +378,11 @@ sub mockedC4Config { > base => 'dc=metavore,dc=com', > hostname => 'localhost', > mapping => \%ldap_mapping, >- pass => 'metavore', >+ pass => $pass, > principal_name => '%s@my_domain.com', > replicate => $replicate, > update => $update, >- user => 'cn=Manager,dc=metavore,dc=com', >+ user => $user, > ); > return \%ldap_config; > } >-- >2.17.1
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 18947
:
69777
|
76879
|
78001