[Bug 36161] New: ISLDI AuthenticatePatron broken when LDAP enabled and auth falls back to internal connection
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36161 Bug ID: 36161 Summary: ISLDI AuthenticatePatron broken when LDAP enabled and auth falls back to internal connection Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: critical Priority: P5 - low Component: Authentication Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: dpavlin@rot13.org bug 34893 passes a patron object back from authentication, however, when LDAP is enabled we define the patron from the return of checkpw_ldap When the ldap auth is a failure, we fallback to internal, however, we don't redefine the patron object In the code: 1965 if ( defined $userid ) { 1966 $patron = Koha::Patrons->find( { userid => $userid } ); 1967 $patron = Koha::Patrons->find( { cardnumber => $userid } ) unless $patron; 1968 } . . . 1980 } elsif ( $ldap && defined($password) ) { 1981 my ( $retval, $retcard, $retuserid ); 1982 ( $retval, $retcard, $retuserid, $patron ) = checkpw_ldap(@_); # EXTERNAL AUTH . . . 2026 # INTERNAL AUTH 2027 if ($check_internal_as_fallback) { 2028 @return = checkpw_internal( $userid, $password, $no_set_userenv ); 2029 push( @return, $patron ); 2030 $passwd_ok = 1 if $return[0] > 0; # 1 or 2 2031 } line 1982 undefs, line 2029 returns it In ilsdi: 397 my ($status, $cardnumber, $userid, $patron) = C4::Auth::checkpw( $username, $password ); 398 if ( $status == 1 ) { 399 # Track the login 400 $patron->update_lastseen('connection'); 401 return { id => $patron->borrowernumber }; 402 } We die on undefined patron -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36161 --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- To recreate: 1 - Enable ILSDI: http://localhost:8081/cgi-bin/koha/admin/preferences.pl?tab=web_services#web... 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 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36161 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |34893 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34893 [Bug 34893] ILS-DI can return the wrong patron for AuthenticatePatron -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36161 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36161 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36161 --- Comment #2 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 162458 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162458&action=edit 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... 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 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36161 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |kyle@bywatersolutions.com |ity.org | CC| |kyle@bywatersolutions.com -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36161 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #162458|0 |1 is obsolete| | --- Comment #3 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 162460 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162460&action=edit 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... 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36161 Magnus Enger <magnus@libriotech.no> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |magnus@libriotech.no Summary|ISLDI AuthenticatePatron |ILS-DI AuthenticatePatron |broken when LDAP enabled |broken when LDAP enabled |and auth falls back to |and auth falls back to |internal connection |internal connection -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36161 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com Status|Needs Signoff |Failed QA --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I'm afraid t/db_dependent/Auth.t is failing with this patch... could you check it out? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36161 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36161 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=36575 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36161 --- Comment #5 from Nick Clemens <nick@bywatersolutions.com> --- Currently the checkpw_internal gets patron by userid and check the password if a patron is found, then it gets patron by cardnumber and checks password. This patch reduces that to a single check, get patron by either field, then check password - it's going to muddy the case where a username/cardnumber is shared bug 14323 aims to disallow, but I think we need to reconcile this one and 36575 - can you compare them Kyle? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36161 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |36575 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36161 --- Comment #6 from Kyle M Hall <kyle@bywatersolutions.com> --- We believe bug 36575 may take care of this. Let's revisit it after that's been pushed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36161 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=36928 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36161 Bug 36161 depends on bug 36575, which changed state. Bug 36575 Summary: Wrong patron can be returned for API validation route https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36575 What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36161 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|martin.renvoize@ptfs-europe | |.com | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36161 --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Little prompt to ask if this is still valid or can perhaps now be closed instead? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36161 --- Comment #8 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Martin Renvoize from comment #7)
Little prompt to ask if this is still valid or can perhaps now be closed instead?
ILS-DI is completely broken right now I believe (CSRF related), so not sure if this can be tested. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36161 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |RESOLVED Resolution|--- |FIXED --- Comment #9 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Katrin Fischer from comment #8)
(In reply to Martin Renvoize from comment #7)
Little prompt to ask if this is still valid or can perhaps now be closed instead?
ILS-DI is completely broken right now I believe (CSRF related), so not sure if this can be tested.
We think bug 36575 has resolved this issue! We can always reopen if we find that is not the case. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36161 Bug 36161 depends on bug 34893, which changed state. Bug 34893 Summary: ILS-DI can return the wrong patron for AuthenticatePatron https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34893 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldoldoldstable |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36161 Developer Inside <amar@ourlib.in> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |amar@ourlib.in --- Comment #10 from Developer Inside <amar@ourlib.in> --- I just want to confirm whether the changes or bug fix from the attached patch (https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162460&action=diff ) have been incorporated into the main GitHub repository or version 25.05. We are unable to see the changes in our current setup. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org