Bugzilla – Attachment 162460 Details for
Bug 36161
ILS-DI AuthenticatePatron broken when LDAP enabled and auth falls back to internal connection
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 36161: Fix ISLDI AuthenticatePatron when LDAP enabled
Bug-36161-Fix-ISLDI-AuthenticatePatron-when-LDAP-e.patch (text/plain), 8.65 KB, created by
Kyle M Hall (khall)
on 2024-02-26 16:03:40 UTC
(
hide
)
Description:
Bug 36161: Fix ISLDI AuthenticatePatron when LDAP enabled
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2024-02-26 16:03:40 UTC
Size:
8.65 KB
patch
obsolete
>From d61465cce29cb019173f4db7b31cf912d409917d Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatersolutions.com> >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: ><useldapserver>1</useldapserver><!-- see C4::Auth_with_ldap for extra configs you must add if you want to turn this on --> > <ldapserver id="ldapserver"> > <hostname>ldaps://nha.dog</hostname> > <base>belong.to.us</base> > <mapping> > <userid is="SAMAccountname"></userid> > </mapping> > </ldapserver> >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
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 36161
:
162458
| 162460