From 0bf11b036a5222459614d1619e5b5ac7b2722507 Mon Sep 17 00:00:00 2001 From: = <=> Date: Wed, 5 Oct 2011 16:47:21 -0400 Subject: [PATCH] Bug #6979 I removed several lines of code in the checkpw_ldap subroutine where LDAP authentication takes place, in the "else" part of the conditional that checks for the auth_by_bind config parameter. I added several lines to check whether the user can log in to LDAP using their DN and the password supplied in the login form. If they are able to bind, login contiues as normal and the LDAP attributes can be harvested as normal if the update options are turned on. The routine that was in place was failing because it was trying to check against a non-existent LDAP entry attribute called 'userpassword'. Instead of checking against a 'userpassword' attribute, the routine really should be checking to make sure the user can actually bind to LDAP. That's what I set up, and it is a safer way to test authentication against LDAP. --- C4/Auth_with_ldap.pm | 31 +++++++++++++++++++------------ 1 files changed, 19 insertions(+), 12 deletions(-) diff --git a/C4/Auth_with_ldap.pm b/C4/Auth_with_ldap.pm index 5baa07f..f10e049 100644 --- a/C4/Auth_with_ldap.pm +++ b/C4/Auth_with_ldap.pm @@ -105,6 +105,8 @@ sub checkpw_ldap { my $db = Net::LDAP->new(\@hosts); #$debug and $db->debug(5); my $userldapentry; + + # first, LDAP authentication if ( $ldap->{auth_by_bind} ) { my $principal_name = $ldap->{principal_name}; if ($principal_name and $principal_name =~ /\%/) { @@ -118,15 +120,15 @@ sub checkpw_ldap { return 0; } - # FIXME dpavlin -- we really need $userldapentry leater on even if using auth_by_bind! + # FIXME dpavlin -- we really need $userldapentry leater on even if using auth_by_bind! - # BUG #5094 - # 2010-08-04 JeremyC - # a $userldapentry is only needed if either updating or replicating are enabled - if($config{update} or $config{replicate}) { - my $search = search_method($db, $userid) or return 0; # warnings are in the sub - $userldapentry = $search->shift_entry; - } + # BUG #5094 + # 2010-08-04 JeremyC + # a $userldapentry is only needed if either updating or replicating are enabled + if($config{update} or $config{replicate}) { + my $search = search_method($db, $userid) or return 0; # warnings are in the sub + $userldapentry = $search->shift_entry; + } } else { my $res = ($config{anonymous}) ? $db->bind : $db->bind($ldapname, password=>$ldappassword); @@ -136,10 +138,15 @@ sub checkpw_ldap { } my $search = search_method($db, $userid) or return 0; # warnings are in the sub $userldapentry = $search->shift_entry; - my $cmpmesg = $db->compare( $userldapentry, attr=>'userpassword', value => $password ); - if ($cmpmesg->code != 6) { - warn "LDAP Auth rejected : invalid password for user '$userid'. " . description($cmpmesg); - return 0; + + # BUG 6979 + # 2011-09-29 Robert Fox (rfox2@nd.edu) + # Fix for password comparison bug + my $dn = $userldapentry->dn; + my $user_ldap_bind_ret = $db->bind($dn, password => $password); + if ($user_ldap_bind_ret->code) { + warn "LDAP Auth rejected : invalid password for user '$userid'. " . description($user_ldap_bind_ret); + return 0; } } -- 1.7.6