From d61465cce29cb019173f4db7b31cf912d409917d Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Mon, 26 Feb 2024 14:40:04 +0000 Subject: [PATCH] Bug 36161: Fix ISLDI AuthenticatePatron when LDAP enabled Test Plan: 1) Enable ILSDI: http://localhost:8081/cgi-bin/koha/admin/preferences.pl?tab=web_services#web_services_ILS-DI 2) Enable ldap (not for real) by adding/editing to koha-conf: 1 ldaps://nha.dog belong.to.us 3) Try to authenticate a patron (generate your correct credentials: http://localhost:8080/cgi-bin/koha/ilsdi.pl?service=AuthenticatePatron&username=edna&password=acosta 4) Can't call method "update_lastseen" on an undefined value at /kohadevbox/koha/C4/ILSDI/Services.pm line 400 5) Apply this patch 6) Restart all the things! 7) Repeat step 3 8) No errors! 9) Set to a valid ldap server 10) Verify LDAP authentication still functions as usual --- C4/Auth.pm | 43 ++++++++++--------------------------- C4/Auth_with_ldap.pm | 51 +++++++++++++++----------------------------- 2 files changed, 28 insertions(+), 66 deletions(-) diff --git a/C4/Auth.pm b/C4/Auth.pm index ca4ee0c8af4..26d54a5a127 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -2012,7 +2012,6 @@ sub checkpw { # INTERNAL AUTH if ($check_internal_as_fallback) { @return = checkpw_internal( $userid, $password, $no_set_userenv ); - push( @return, $patron ); $passwd_ok = 1 if $return[0] > 0; # 1 or 2 } @@ -2041,43 +2040,23 @@ sub checkpw_internal { my ( $userid, $password, $no_set_userenv ) = @_; $password = Encode::encode( 'UTF-8', $password ) - if Encode::is_utf8($password); + if Encode::is_utf8($password); - my $dbh = C4::Context->dbh; - my $sth = - $dbh->prepare( - "select password,cardnumber,borrowernumber,userid,firstname,surname,borrowers.branchcode,branches.branchname,flags from borrowers join branches on borrowers.branchcode=branches.branchcode where userid=?" - ); - $sth->execute($userid); - if ( $sth->rows ) { - my ( $stored_hash, $cardnumber, $borrowernumber, $userid, $firstname, - $surname, $branchcode, $branchname, $flags ) - = $sth->fetchrow; - - if ( checkpw_hash( $password, $stored_hash ) ) { + my $patron = Koha::Patrons->find( { userid => $userid } ) || Koha::Patrons->find( { cardnumber => $userid } ); - C4::Context->set_userenv( "$borrowernumber", $userid, $cardnumber, - $firstname, $surname, $branchcode, $branchname, $flags ) unless $no_set_userenv; - return 1, $cardnumber, $userid; - } - } - $sth = - $dbh->prepare( - "select password,cardnumber,borrowernumber,userid,firstname,surname,borrowers.branchcode,branches.branchname,flags from borrowers join branches on borrowers.branchcode=branches.branchcode where cardnumber=?" - ); - $sth->execute($userid); - if ( $sth->rows ) { - my ( $stored_hash, $cardnumber, $borrowernumber, $userid, $firstname, - $surname, $branchcode, $branchname, $flags ) - = $sth->fetchrow; + if ($patron) { + if ( checkpw_hash( $password, $patron->password ) ) { + my $borrowernumber = $patron->id; - if ( checkpw_hash( $password, $stored_hash ) ) { + C4::Context->set_userenv( + "$borrowernumber", $patron->userid, $patron->cardnumber, + $patron->firstname, $patron->surname, $patron->branchcode, $patron->library->branchname, $patron->flags + ) unless $no_set_userenv; - C4::Context->set_userenv( $borrowernumber, $userid, $cardnumber, - $firstname, $surname, $branchcode, $branchname, $flags ) unless $no_set_userenv; - return 1, $cardnumber, $userid; + return ( 1, $patron->cardnumber, $patron->userid, $patron ); } } + return 0; } diff --git a/C4/Auth_with_ldap.pm b/C4/Auth_with_ldap.pm index 9b8b5d2e57c..a96016452a1 100644 --- a/C4/Auth_with_ldap.pm +++ b/C4/Auth_with_ldap.pm @@ -202,22 +202,22 @@ sub checkpw_ldap { # But we still have work to do. See perldoc below for detailed breakdown. my (%borrower); - my ($borrowernumber,$cardnumber,$local_userid,$savedpw) = exists_local($userid); + my $patron = Koha::Patrons->find( { userid => $userid } ) || Koha::Patrons->find( { cardnumber => $userid } ); - my $patron; - if (( $borrowernumber and $config{update} ) or - (!$borrowernumber and $config{replicate}) ) { + if (( $patron and $config{update} ) or + (!$patron and $config{replicate}) ) { + my $cardnumber = $patron ? $patron->cardnumber : q{}; %borrower = ldap_entry_2_hash($userldapentry,$cardnumber); - #warn "checkpw_ldap received \%borrower w/ " . keys(%borrower), " keys: ", join(' ', keys %borrower), "\n"; } - if ($borrowernumber) { - if ($config{update}) { # A1, B1 - my $c2 = &update_local($local_userid,$password,$borrowernumber,\%borrower) || ''; - ($cardnumber eq $c2) or warn "update_local returned cardnumber '$c2' instead of '$cardnumber'"; - } else { # C1, D1 - # maybe update just the password? - return(1, $cardnumber, $local_userid); + if ($patron) { + if ( $config{update} ) { # A1, B1 + my $c2 = &update_local( $patron->userid, $password, $patron->id, \%borrower ) || ''; + ( $patron->cardnumber eq $c2 ) + or warn "update_local returned cardnumber '$c2' instead of '$patron->cardnumber'"; + } else { # C1, D1 + # maybe update just the password? + return ( 1, $patron->cardnumber, $patron->userid, $patron ); } } elsif ($config{replicate}) { # A2, C2 my @columns = Koha::Patrons->columns; @@ -227,10 +227,10 @@ sub checkpw_ldap { } )->store; die "Insert of new patron failed" unless $patron; - $borrowernumber = $patron->borrowernumber; + $patron->discard_changes; C4::Members::Messaging::SetMessagingPreferencesFromDefaults( { - borrowernumber => $borrowernumber, + borrowernumber => $patron->id, categorycode => $borrower{'categorycode'} } ); @@ -268,7 +268,7 @@ sub checkpw_ldap { } else { return 0; # B2, D2 } - if (C4::Context->preference('ExtendedPatronAttributes') && $borrowernumber && ($config{update} ||$config{replicate})) { + if (C4::Context->preference('ExtendedPatronAttributes') && $patron->id && ($config{update} ||$config{replicate})) { my $library_id = C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef; my $attribute_types = Koha::Patron::Attribute::Types->search_with_library_limits({}, {}, $library_id); while ( my $attribute_type = $attribute_types->next ) { @@ -276,7 +276,7 @@ sub checkpw_ldap { unless (exists($borrower{$code}) && $borrower{$code} !~ m/^\s*$/ ) { next; } - $patron = Koha::Patrons->find($borrowernumber); + $patron = Koha::Patrons->find($patron->id); if ( $patron ) { # Should not be needed, but we are in C4::Auth LDAP... eval { my $attribute = Koha::Patron::Attribute->new({code => $code, attribute => $borrower{$code}}); @@ -288,7 +288,7 @@ sub checkpw_ldap { } } } - return(1, $cardnumber, $userid, $patron); + return ( 1, $patron->cardnumber, $userid, $patron ); } # Pass LDAP entry object and local cardnumber (userid). @@ -344,23 +344,6 @@ sub ldap_entry_2_hash { return %borrower; } -sub exists_local { - my $arg = shift; - my $dbh = C4::Context->dbh; - my $select = "SELECT borrowernumber,cardnumber,userid,password FROM borrowers "; - - my $sth = $dbh->prepare("$select WHERE userid=?"); # was cardnumber=? - $sth->execute($arg); - #warn "Userid '$arg' exists_local? %s\n", $sth->rows; - ($sth->rows == 1) and return $sth->fetchrow; - - $sth = $dbh->prepare("$select WHERE cardnumber=?"); - $sth->execute($arg); - #warn "Cardnumber '$arg' exists_local? %s\n", $sth->rows; - ($sth->rows == 1) and return $sth->fetchrow; - return 0; -} - # This function performs a password update, given the userid, borrowerid, # and digested password. It will verify that things are correct and return the # borrowers cardnumber. The idea is that it is used to keep the local -- 2.30.2