Imagine we have a set of users. Some of those users have a NULL userid. We then call AuthenticatePatron from ILS-DI for a patron with a NULL userid, but a valid cardnumber. We call checkpw, which returns the cardnumber and userid. We then call Koha::Patrons->find on the userid *which is null*, meaning the borrowernumber returned is not the correct one, but instead the earliest patron inserted into the database that has a NULL userid.
Created attachment 156121 [details] [review] Bug 34893: ILS-DI can return the wrong patron for AuthenticatePatron Imagine we have a set of users. Some of those users have a NULL userid. We then call AuthenticatePatron from ILS-DI for a patron with a NULL userid, but a valid cardnumber. We call checkpw, which returns the cardnumber and userid. We then call Koha::Patrons->find on the userid *which is null*, meaning the borrowernumber returned is not the correct one, but instead the earliest patron inserted into the database that has a NULL userid. Test Plan: 1) Create a new patron ( we assume there are existing patrons as well ) 2) From the database cli, set the patron's userid to null 3) Run this query: update borrowers set userid = null; 3) Call AuthenticatePatron with username being the patron's cardnumber, and password being the password you set for that patron 4) Note you get back a borrowernumber for a different patron 5) Apply this patch 6) Restart all the things! 7) Call AuthenticatePatron again 8) Note you get the correct borrowernumber!
Created attachment 156123 [details] [review] Bug 34893: ILS-DI can return the wrong patron for AuthenticatePatron Imagine we have a set of users. Some of those users have a NULL userid. We then call AuthenticatePatron from ILS-DI for a patron with a NULL userid, but a valid cardnumber. We call checkpw, which returns the cardnumber and userid. We then call Koha::Patrons->find on the userid *which is null*, meaning the borrowernumber returned is not the correct one, but instead the earliest patron inserted into the database that has a NULL userid. Test Plan: 1) Create a new patron ( we assume there are existing patrons as well ) 2) From the database cli, set the patron's userid to null 3) Run this query: update borrowers set userid = null; 3) Call AuthenticatePatron with username being the patron's cardnumber, and password being the password you set for that patron 4) Note you get back a borrowernumber for a different patron 5) Apply this patch 6) Restart all the things! 7) Call AuthenticatePatron again 8) Note you get the correct borrowernumber! 9) prove t/Auth.t t/db_dependent/Auth_with_ldap.t t/Auth_with_shibboleth.t t/db_dependent/Auth_with_cas.t
As a corollary to this, we should probably alter borrowers.userid to be NOT NULL and UNIQUE.
Just want to point out here that a perfect storm has to happen to expose this bug: - you must have more than one NULL or empty userid in your catalog, which is not possible to achieve from within the Koha interface - the changes to the userid's will likely have been done through the database backend, potentially during migration time or through some other requested process (mad scheme alert) The risk this bug exposes is if an external party is not checking to make sure that the returned cardnumber or userid from ILS-DI is the same as the one they asked for, depending on how they then ask for user information, an end user could potentially see the information of the wrong Koha borrower.
> you must have more than one NULL or empty userid in your catalog, > which is not possible to achieve from within the Koha interface It there a place to find older DB migrations than update22to30.pl? To find when userid was introduced how was it populated? Oh wait, that doesn't tell when the staff interface started to use firstname and surname to ensure userid had a value. After some deep archeology: Koha 2.0 introduced userid and password in borrowers table. https://gitlab.com/koha-community/Koha/-/blame/1f39fe386dbd67cb5577e52855041f7597393db0/misc/koha.mysql#L502 And something like two years later, the code to put a default value for userid ($onefirstnameletter.$fivesurnameletter) was introduced. (it says login, but it's supposed to be userid (to match DB), a subsequent commit fixes that ^^") https://gitlab.com/koha-community/Koha/-/blame/97182dceb2ce9d23e82c57cd91fc1f6c1d8bf382/members/memberentry.pl#L135 So very old Koha instances are expected to have multiple NULL userid. For all/most of their old patrons. That tells that likely the migration to Koha 2.0 should have set userid to NULL for all users, since it was the default column value and the UI didn't put a default value. It would be weird for the migration to populate it. > The risk this bug exposes is if an external party is not checking to > make sure that the returned cardnumber or userid from ILS-DI is the same as the one they asked for Isn't that is likely how most integrations would work? Not sure there are much that doublecheck cardnumber is the same. And userid not null. The likely check would be the returned id!=NULL (borrowernumber) returned by AuthenticatePatron. Since it will be used for calls like GetPatronInfo, HoldItem, etc > , depending on how they then ask for user information, an end user could potentially > see the information of the wrong Koha borrower. Ah yes, thanks Liz, that seem to be the main security issue, good thing it's random, not targetable. Other stuff would be bugs, like holds placed for the other patron. Anyway that doesn't look like a blocker severe bug. It's still bad though, nice find Kyle! > 4) Note you get back a borrowernumber for a different patron Is there a way to reproduce that consistently? I only once got the wrong id returned. Multiple refreshes and a service restart still give me the correct id. Same on another patron. So I can't apply the patch and see that it effectively fixes the issue. I don't know by which luck the first call managed to get me the wrong id.
(In reply to Liz Rea from comment #4) > Just want to point out here that a perfect storm has to happen to expose > this bug: > > - you must have more than one NULL or empty userid in your catalog, which is > not possible to achieve from within the Koha interface > - the changes to the userid's will likely have been done through the > database backend, potentially during migration time or through some other > requested process (mad scheme alert) I'm not sure that's true. In /cgi-bin/koha/tools/import_borrowers.pl the only required fields are "branchcode" and "categorycode", so it could be very easy to have a NULL/empty userid in a catalogue. Autoadd by SSO/LDAP could also add patrons with null/empty userid I believe.
I think that we'll probably need some additional unit tests / updated unit tests for this change. I haven't tested this yet but I don't quite understand this change when glancing at the code. Putting this one on my radar though...
(In reply to Victor Grousset/tuxayo from comment #5) > Is there a way to reproduce that consistently? I only once got the wrong id > returned. Multiple refreshes and a service restart still give me the correct > id. Same on another patron. So I can't apply the patch and see that it > effectively fixes the issue. I don't know by which luck the first call > managed to get me the wrong id. I think I got a way: it happens (wrong id returned) once per patron since the start of KTD. Test Plan: 1) Create a new patron ( we assume there are existing patrons as well ) 2) Give two other patrons a userid and a password 3) From the database cli, set all patrons's userid to null Run this query: update borrowers set userid = null; 4) Call AuthenticatePatron with username being the 1st patron cardnumber, and password being the password you set for that patron http://localhost:8080/cgi-bin/koha/ilsdi.pl?service=AuthenticatePatron&username=kohacard&password=koha 5) Note you get back a borrowernumber for a different patron. Refresh the page and the number is correct. 6) Do the same with the 2nd patron. Same issue at 1st and correct number after. 7) Apply this patch 8) Restart all the things! 9) Do the same with the 3rd patron. 10) Note you get the correct borrowernumber! :D 11) prove t/Auth.t t/db_dependent/Auth_with_ldap.t t/Auth_with_shibboleth.t t/db_dependent/Auth_with_cas.t But I couldn't test the end, patch doesn't apply :P
Created attachment 159311 [details] [review] Bug 34893: ILS-DI can return the wrong patron for AuthenticatePatron Imagine we have a set of users. Some of those users have a NULL userid. We then call AuthenticatePatron from ILS-DI for a patron with a NULL userid, but a valid cardnumber. We call checkpw, which returns the cardnumber and userid. We then call Koha::Patrons->find on the userid *which is null*, meaning the borrowernumber returned is not the correct one, but instead the earliest patron inserted into the database that has a NULL userid. Test Plan: 1) Create a new patron ( we assume there are existing patrons as well ) 2) From the database cli, set the patron's userid to null 3) Run this query: update borrowers set userid = null; 3) Call AuthenticatePatron with username being the patron's cardnumber, and password being the password you set for that patron 4) Note you get back a borrowernumber for a different patron 5) Apply this patch 6) Restart all the things! 7) Call AuthenticatePatron again 8) Note you get the correct borrowernumber! 9) prove t/Auth.t t/db_dependent/Auth_with_ldap.t t/Auth_with_shibboleth.t t/db_dependent/Auth_with_cas.t
Created attachment 159314 [details] [review] Bug 34893: ILS-DI can return the wrong patron for AuthenticatePatron Imagine we have a set of users. Some of those users have a NULL userid. We then call AuthenticatePatron from ILS-DI for a patron with a NULL userid, but a valid cardnumber. We call checkpw, which returns the cardnumber and userid. We then call Koha::Patrons->find on the userid *which is null*, meaning the borrowernumber returned is not the correct one, but instead the earliest patron inserted into the database that has a NULL userid. Test Plan: 1) Give three patrons a userid and a password 2) From the database cli, set all patrons's userid to null Run this query: update borrowers set userid = null; 3) Call AuthenticatePatron with username being the 1st patron cardnumber, and password being the password you set for that patron http://localhost:8080/cgi-bin/koha/ilsdi.pl?service=AuthenticatePatron&username=kohacard&password=koha 4) Note you get back a borrowernumber for a different patron. Refresh the page and the number is correct. 5) Do the same with the 2nd patron. Same issue at 1st and correct number after. 6) Apply this patch 7) Restart all the things! 8) Do the same with the 3rd patron. 9) Note you get the correct borrowernumber! :D 10) prove t/Auth.t t/db_dependent/Auth_with_ldap.t t/Auth_with_shibboleth.t t/db_dependent/Auth_with_cas.t Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
It works! :D Changed test plan to reproduce consistently. fixed t/Auth_with_shibboleth.t => t/db_dependent/Auth_with_shibboleth.t
I'm not sure if /api/v1/auth/password/validation is affected by the problem, but Koha/REST/V1/Auth/Password.pm does use C4::Auth::checkpw and looks up patron based on the cardnumber returned by C4::Auth::checkpw (which in hindsight is super problematic since lots of libraries don't use cardnumbers). So I think we need to update there too.
Created attachment 159315 [details] [review] Bug 34893: Add checkpw change to REST API This patch adds the checkpw return value change to the REST API route for validating user identifiers and password. Test plan: 0. Apply patch 1. prove t/db_dependent/api/v1/password_validation.t Bonus points: 1. koha-plack --reload kohadev 2. Enable syspref RESTBasicAuth 3. curl -XPOST -H "Content-Type: application/json" \ -u <staff_userid>:<staff_password> \ -d '{"identifier":"<cardnumber>","password":"<password>"}' \ http://localhost:8081/api/v1/auth/password/validation 4. Validation doesn't fail. It gives you cardnumber, patron_id, userid
No test, failing QA.
1. The maintenance cost on stuff like: ( $retval, $retcard, $retuserid, $patron ) = checkpw_ldap(@_); my ( $retval, $retcard, $retuserid, $cas_ticket, $patron ) = checkpw_cas( $ticket, $query, $type ); # EXTERNAL AUTH Patron is 4th param for ldap, but 5th for cas... 2. No tests, we are in C4::Auth! Test everything! 3. We are returning the id and the object, could you clarify? Do you really need both?
> 1. The maintenance cost on stuff like: > ( $retval, $retcard, $retuserid, $patron ) = checkpw_ldap(@_); > my ( $retval, $retcard, $retuserid, $cas_ticket, $patron ) = checkpw_cas( > $ticket, $query, $type ); # EXTERNAL AUTH I would be happy to change the return values to a hashref in a followup bug. > Patron is 4th param for ldap, but 5th for cas... It's the last param for all of them > > 2. No tests, we are in C4::Auth! Test everything! [U+1F44D] > > 3. We are returning the id and the object, could you clarify? Do you really > need both? Least changes necessary. We don't want to risk breaking something that might use the id. Refactoring this code should be on a followup bug.
Created attachment 159356 [details] [review] Bug 34893: Add unit tests
(In reply to Jonathan Druart from comment #14) > No test, failing QA. I initially thought that as well, but in theory the existing unit tests should catch regressions for most of the changes. (That said, I'm not 100% confident in the changes myself, so I wasn't keen to QA without doing a lot more investigating...)
(In reply to Jonathan Druart from comment #15) > 2. No tests, we are in C4::Auth! Test everything! This is a QA blocker.
WARN C4/Auth.pm WARN tidiness The file is less tidy than before (bad/messy lines before: 437, now: 440) WARN C4/Auth_with_cas.pm WARN tidiness The file is less tidy than before (bad/messy lines before: 35, now: 40) WARN t/db_dependent/ILSDI_Services.t WARN tidiness The file is less tidy than before (bad/messy lines before: 349, now: 376)
(In reply to Marcel de Rooy from comment #19) > (In reply to Jonathan Druart from comment #15) > > 2. No tests, we are in C4::Auth! Test everything! > > This is a QA blocker. Did you notice I already added tests before you wrote this? :)
Created attachment 159441 [details] [review] Bug 34893: (QA follow-up) Tidy code for qa script
After playing with the patch order, it is confirmed that "Bug 34893: Add unit tests" works as expected. As for "Bug 34893: Add checkpw change to REST API" I tried the complete test plan. And I can't reproduce the issue. With or without the patches, I get something super weird: update borrowers set userid = null where borrowernumber = 5; curl -XPOST -H "Content-Type: application/json" \ -u koha:koha \ -d '{"identifier":"test","password":"test"}' \ http://localhost:8081/api/v1/auth/password/validation response: > {"cardnumber":"testcard","patron_id":5,"userid":"test.test"} The call to /auth/password/validation causes the userid to be set from first name and surname [U+1F643]!!! I checked before and after with select userid from borrowers where borrowernumber=5 And redid the process multiple times and that's really what happens. Test Plan: 1. Enable syspref RESTBasicAuth 2. Pick patron that can login to staff interface (note the borrowernumber) Not your usual staff patron, otherwise you will lock yourself outside on next step. 3. update borrowers set userid = null where borrowernumber = XXX; 4. curl -XPOST -H "Content-Type: application/json" \ -u <YOUR_staff_userid>:<YOUR_staff_password> \ -d '{"identifier":"<TEST_patron_cardnumber>","password":"<TEST_patron_password>"}' \ http://localhost:8081/api/v1/auth/password/validation 5. It returns your patron with the userid autogenerated from first name and surname 6. ⁉️⁉️⁉️ So it seems that another bug is blocking exploiting the REST API and testing the fix. options: A. New ticket and this one depends on it B. Fix the bug here C. There is another way to test the rest API fix. D. It's obvious from the code change that the vulnerability is fixed and the above manual testing still shows that there is no regression on the feature (plus the test suite) E. The above test plan has a big mistake so the conclusion don't make any sense
(In reply to Victor Grousset/tuxayo from comment #23) > So it seems that another bug is blocking exploiting the REST API and testing > the fix. > > options: > A. New ticket and this one depends on it > B. Fix the bug here > C. There is another way to test the rest API fix. > D. It's obvious from the code change that the vulnerability is fixed and the > above manual testing still shows that there is no regression on the feature > (plus the test suite) > E. The above test plan has a big mistake so the conclusion don't make any > sense Why do you think it's a bug? If Koha::Patron doesn't have a userid, it will generate one when it's stored. This has been in Koha::Patron for a while. C4::Auth::checkpw updates "login_attempts" for Koha::Patron at the end of the function. For "Add checkpw change to REST API", I wasn't trying to fix the same problem as Kyle since the REST API used cardnumber rather than userid for the lookup. A null userid wouldn't have made a difference for the REST API. Rather, I was updating the checkpw() usage to be consistent with the changes he made in the other modules.
Created attachment 159506 [details] [review] Bug 34893: Add unit tests Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 159507 [details] [review] Bug 34893: ILS-DI can return the wrong patron for AuthenticatePatron Imagine we have a set of users. Some of those users have a NULL userid. We then call AuthenticatePatron from ILS-DI for a patron with a NULL userid, but a valid cardnumber. We call checkpw, which returns the cardnumber and userid. We then call Koha::Patrons->find on the userid *which is null*, meaning the borrowernumber returned is not the correct one, but instead the earliest patron inserted into the database that has a NULL userid. Test Plan: 1) Give three patrons a userid and a password 2) From the database cli, set all patrons's userid to null Run this query: update borrowers set userid = null; 3) Call AuthenticatePatron with username being the 1st patron cardnumber, and password being the password you set for that patron http://localhost:8080/cgi-bin/koha/ilsdi.pl?service=AuthenticatePatron&username=kohacard&password=koha 4) Note you get back a borrowernumber for a different patron. Refresh the page and the number is correct. 5) Do the same with the 2nd patron. Same issue at 1st and correct number after. 6) Apply this patch 7) Restart all the things! 8) Do the same with the 3rd patron. 9) Note you get the correct borrowernumber! :D 10) prove t/Auth.t t/db_dependent/Auth_with_ldap.t t/Auth_with_shibboleth.t t/db_dependent/Auth_with_cas.t Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 159508 [details] [review] Bug 34893: Add checkpw change to REST API This patch adds the checkpw return value change to the REST API route for validating user identifiers and password. Test plan: 0. Apply patch 1. prove t/db_dependent/api/v1/password_validation.t Bonus points: 1. koha-plack --reload kohadev 2. Enable syspref RESTBasicAuth 3. curl -XPOST -H "Content-Type: application/json" \ -u <staff_userid>:<staff_password> \ -d '{"identifier":"<cardnumber>","password":"<password>"}' \ http://localhost:8081/api/v1/auth/password/validation 4. Validation doesn't fail. It gives you cardnumber, patron_id, userid Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 159509 [details] [review] Bug 34893: (QA follow-up) Tidy code for qa script Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
(In reply to David Cook from comment #24) > Why do you think it's a bug? > > If Koha::Patron doesn't have a userid, it will generate one when it's > stored. This has been in Koha::Patron for a while. Oh, ok, that was weird for /auth/password/validation to trigger that side effect. Option E then! ^^ > C4::Auth::checkpw updates "login_attempts" for Koha::Patron at the end of > the function. > > For "Add checkpw change to REST API", I wasn't trying to fix the same > problem as Kyle since the REST API used cardnumber rather than userid for > the lookup. A null userid wouldn't have made a difference for the REST API. > Rather, I was updating the checkpw() usage to be consistent with the changes > he made in the other modules. Ok then all works as advertised. Back to signed off :) (it was already but it needed retesting for the new patches)
Created attachment 159797 [details] [review] Bug 34893: Add unit tests Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 159798 [details] [review] Bug 34893: ILS-DI can return the wrong patron for AuthenticatePatron Imagine we have a set of users. Some of those users have a NULL userid. We then call AuthenticatePatron from ILS-DI for a patron with a NULL userid, but a valid cardnumber. We call checkpw, which returns the cardnumber and userid. We then call Koha::Patrons->find on the userid *which is null*, meaning the borrowernumber returned is not the correct one, but instead the earliest patron inserted into the database that has a NULL userid. Test Plan: 1) Give three patrons a userid and a password 2) From the database cli, set all patrons's userid to null Run this query: update borrowers set userid = null; 3) Call AuthenticatePatron with username being the 1st patron cardnumber, and password being the password you set for that patron http://localhost:8080/cgi-bin/koha/ilsdi.pl?service=AuthenticatePatron&username=kohacard&password=koha 4) Note you get back a borrowernumber for a different patron. Refresh the page and the number is correct. 5) Do the same with the 2nd patron. Same issue at 1st and correct number after. 6) Apply this patch 7) Restart all the things! 8) Do the same with the 3rd patron. 9) Note you get the correct borrowernumber! :D 10) prove t/Auth.t t/db_dependent/Auth_with_ldap.t t/Auth_with_shibboleth.t t/db_dependent/Auth_with_cas.t Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 159799 [details] [review] Bug 34893: Add checkpw change to REST API This patch adds the checkpw return value change to the REST API route for validating user identifiers and password. Test plan: 0. Apply patch 1. prove t/db_dependent/api/v1/password_validation.t Bonus points: 1. koha-plack --reload kohadev 2. Enable syspref RESTBasicAuth 3. curl -XPOST -H "Content-Type: application/json" \ -u <staff_userid>:<staff_password> \ -d '{"identifier":"<cardnumber>","password":"<password>"}' \ http://localhost:8081/api/v1/auth/password/validation 4. Validation doesn't fail. It gives you cardnumber, patron_id, userid Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 159800 [details] [review] Bug 34893: (QA follow-up) Tidy code for qa script Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Kyle: I think we need actual tests for the C4::Auth::checkpw() behavior change.
While trying to write tests for this change, I noticed that the last part of the checkpw(): if ($patron) { if ($passwd_ok) { $patron->update( { login_attempts => 0 } ); if ( $patron->password_expired ) { @return = (-2); } } elsif ( !$patron->account_locked ) { $patron->update( { login_attempts => $patron->login_attempts + 1 } ); } } is performing updates on the $patron object. In particular, if $passwd_ok is true the login_attempts gets updated. BUT in the CAS and SHIB use cases, the returned $patron from (the respective) checkpw_cas() and checkpw_shib() methods is scoped to the `if` in which they are called. In the CAS use case it's fine (just redundant, CAS code querying the patron again... the same way) but on the SHIB one, there's the option to set a custom matchpoint... which could potentially be, say, the email field. In this case, we cannot be sure both the global and the scoped $patron objects represent the same patron on the DB.
Created attachment 159893 [details] [review] Bug 34893: Unit tests for C4::Auth::checkpw This patch introduces some tests on the current (and new) behavior for the `checkpw` function. I needed it to better understand if an edge case was actually possible (it wasn't). Found a really minor annoyance for the internal check with expired password not returning the $patron object for consistency with the other use cases. I think this method deserves (at least) changing the return value to a sane data structure. But that's not target for backporting to stable releases. So a separate bug. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
(In reply to Tomás Cohen Arazi from comment #35) > In the CAS use case it's fine (just redundant, CAS code querying the patron > again... the same way) but on the SHIB one, there's the option to set a > custom matchpoint... which could potentially be, say, the email field. In Found this not to be an issue, as the identifier parameter on the shibboleth use case is an empty string as all data comes from Shibboleth itself.
This no longer applies cleanly, could I get a speedy rebase please?
Created attachment 161097 [details] [review] Bug 34893: Add unit tests Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 161098 [details] [review] Bug 34893: ILS-DI can return the wrong patron for AuthenticatePatron Imagine we have a set of users. Some of those users have a NULL userid. We then call AuthenticatePatron from ILS-DI for a patron with a NULL userid, but a valid cardnumber. We call checkpw, which returns the cardnumber and userid. We then call Koha::Patrons->find on the userid *which is null*, meaning the borrowernumber returned is not the correct one, but instead the earliest patron inserted into the database that has a NULL userid. Test Plan: 1) Give three patrons a userid and a password 2) From the database cli, set all patrons's userid to null Run this query: update borrowers set userid = null; 3) Call AuthenticatePatron with username being the 1st patron cardnumber, and password being the password you set for that patron http://localhost:8080/cgi-bin/koha/ilsdi.pl?service=AuthenticatePatron&username=kohacard&password=koha 4) Note you get back a borrowernumber for a different patron. Refresh the page and the number is correct. 5) Do the same with the 2nd patron. Same issue at 1st and correct number after. 6) Apply this patch 7) Restart all the things! 8) Do the same with the 3rd patron. 9) Note you get the correct borrowernumber! :D 10) prove t/Auth.t t/db_dependent/Auth_with_ldap.t t/Auth_with_shibboleth.t t/db_dependent/Auth_with_cas.t Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 161099 [details] [review] Bug 34893: Add checkpw change to REST API This patch adds the checkpw return value change to the REST API route for validating user identifiers and password. Test plan: 0. Apply patch 1. prove t/db_dependent/api/v1/password_validation.t Bonus points: 1. koha-plack --reload kohadev 2. Enable syspref RESTBasicAuth 3. curl -XPOST -H "Content-Type: application/json" \ -u <staff_userid>:<staff_password> \ -d '{"identifier":"<cardnumber>","password":"<password>"}' \ http://localhost:8081/api/v1/auth/password/validation 4. Validation doesn't fail. It gives you cardnumber, patron_id, userid Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 161100 [details] [review] Bug 34893: (QA follow-up) Tidy code for qa script Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 161101 [details] [review] Bug 34893: Unit tests for C4::Auth::checkpw This patch introduces some tests on the current (and new) behavior for the `checkpw` function. I needed it to better understand if an edge case was actually possible (it wasn't). Found a really minor annoyance for the internal check with expired password not returning the $patron object for consistency with the other use cases. I think this method deserves (at least) changing the return value to a sane data structure. But that's not target for backporting to stable releases. So a separate bug. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Rebased on top of bug 35204 which is in master.
Backported patched to 22.05.x-security Having trouble testing though if anyone is able to help me please
Created attachment 161324 [details] [review] Bug 34893: [22.05] Add a Koha::Patron->update_lastseen method Without this patch, we get an error trace whe calling AuthenticatePatron The method Koha::Patron->update_lastseen is not covered by tests!
Hi all We have pushed these patches plus the 22.05 follow-up to the 22.05 security branch, but we are getting failing tests and aren't sure how to fix them. kohadev-koha@kohadevbox:koha((3417a7052a5...))$ prove t/Auth.t t/db_dependent/Auth_with_ldap.t t/Auth_with_shibboleth.t t/db_dependent/Auth_with_cas.t t/db_dependent/Auth.t t/db_dependent/api/v1/password_validation.t t/Auth.t ..................................... ok t/db_dependent/Auth_with_ldap.t .............. 1/4 Subroutine ldapserver_error redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 45, <DATA> line 960. Subroutine description redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 80, <DATA> line 960. Subroutine search_method redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 87, <DATA> line 960. Subroutine checkpw_ldap redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 111, <DATA> line 960. Subroutine ldap_entry_2_hash redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 261, <DATA> line 960. Subroutine exists_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 309, <DATA> line 960. Subroutine _do_changepassword redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 335, <DATA> line 960. Subroutine update_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 366, <DATA> line 960. Subroutine ldapserver_error redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 45, <DATA> line 960. Subroutine description redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 80, <DATA> line 960. Subroutine search_method redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 87, <DATA> line 960. Subroutine checkpw_ldap redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 111, <DATA> line 960. Subroutine ldap_entry_2_hash redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 261, <DATA> line 960. Subroutine exists_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 309, <DATA> line 960. Subroutine _do_changepassword redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 335, <DATA> line 960. Subroutine update_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 366, <DATA> line 960. Subroutine ldapserver_error redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 45, <DATA> line 960. Subroutine description redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 80, <DATA> line 960. Subroutine search_method redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 87, <DATA> line 960. Subroutine checkpw_ldap redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 111, <DATA> line 960. Subroutine ldap_entry_2_hash redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 261, <DATA> line 960. Subroutine exists_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 309, <DATA> line 960. Subroutine _do_changepassword redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 335, <DATA> line 960. Subroutine update_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 366, <DATA> line 960. Subroutine ldapserver_error redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 45. Subroutine description redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 80. Subroutine search_method redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 87. Subroutine checkpw_ldap redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 111. Subroutine ldap_entry_2_hash redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 261. Subroutine exists_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 309. Subroutine _do_changepassword redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 335. Subroutine update_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 366. Subroutine ldapserver_error redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 45. Subroutine description redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 80. Subroutine search_method redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 87. Subroutine checkpw_ldap redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 111. Subroutine ldap_entry_2_hash redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 261. Subroutine exists_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 309. Subroutine _do_changepassword redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 335. Subroutine update_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 366. Subroutine ldapserver_error redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 45. Subroutine description redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 80. Subroutine search_method redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 87. Subroutine checkpw_ldap redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 111. Subroutine ldap_entry_2_hash redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 261. Subroutine exists_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 309. Subroutine _do_changepassword redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 335. Subroutine update_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 366. Subroutine ldapserver_error redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 45. Subroutine description redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 80. Subroutine search_method redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 87. Subroutine checkpw_ldap redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 111. Subroutine ldap_entry_2_hash redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 261. Subroutine exists_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 309. Subroutine _do_changepassword redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 335. Subroutine update_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 366. Subroutine ldapserver_error redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 45. Subroutine description redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 80. Subroutine search_method redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 87. Subroutine checkpw_ldap redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 111. Subroutine ldap_entry_2_hash redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 261. Subroutine exists_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 309. Subroutine _do_changepassword redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 335. Subroutine update_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 366. Subroutine ldapserver_error redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 45. Subroutine description redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 80. Subroutine search_method redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 87. Subroutine checkpw_ldap redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 111. Subroutine ldap_entry_2_hash redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 261. Subroutine exists_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 309. Subroutine _do_changepassword redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 335. Subroutine update_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 366. Subroutine ldapserver_error redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 45. Subroutine description redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 80. Subroutine search_method redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 87. Subroutine checkpw_ldap redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 111. Subroutine ldap_entry_2_hash redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 261. Subroutine exists_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 309. Subroutine _do_changepassword redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 335. Subroutine update_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 366. Subroutine ldapserver_error redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 45. Subroutine description redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 80. Subroutine search_method redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 87. Subroutine checkpw_ldap redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 111. Subroutine ldap_entry_2_hash redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 261. Subroutine exists_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 309. Subroutine _do_changepassword redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 335. Subroutine update_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 366. t/db_dependent/Auth_with_ldap.t .............. ok t/Auth_with_shibboleth.t ..................... ok t/db_dependent/Auth_with_cas.t ............... ok t/db_dependent/Auth.t ........................ 1/17 Use of uninitialized value $return in numeric gt (>) at /kohadevbox/koha/C4/Auth.pm line 1158. WARNING: The key derivation method "opensslv1" is deprecated. Using -pbkdf=>'pbkdf2' would be better. Pass -nodeprecate=>1 to inhibit this message. at /kohadevbox/koha/Koha/Patron.pm line 2169. t/db_dependent/Auth.t ........................ 15/17 # No tests run! # Failed test 'No tests run for subtest "Internal check tests"' # at t/db_dependent/Auth.t line 1010. # Looks like you planned 3 tests but ran 1. # Looks like you failed 1 test of 1 run. # Failed test 'checkpw() return values tests' # at t/db_dependent/Auth.t line 1148. Can't locate object method "prepare" via package "ePbNiHVXA8SXmNiUYlABl7X_Bvz5eVDlV63G7m2VlFElfda9J80fWIlZQvEJIm6Km" (perhaps you forgot to load "ePbNiHVXA8SXmNiUYlABl7X_Bvz5eVDlV63G7m2VlFElfda9J80fWIlZQvEJIm6Km"?) at /kohadevbox/koha/C4/Auth.pm line 2001. # Looks like your test exited with 11 just after 17. t/db_dependent/Auth.t ........................ Dubious, test returned 11 (wstat 2816, 0xb00) Failed 1/17 subtests Cannot detect source of 't/db_dependent/api/v1/password_validation.t'! at /usr/share/perl/5.32/TAP/Parser/IteratorFactory.pm line 256. TAP::Parser::IteratorFactory::detect_source(TAP::Parser::IteratorFactory=HASH(0x557f31f8c9e0), TAP::Parser::Source=HASH(0x557f31f97dc8)) called at /usr/share/perl/5.32/TAP/Parser/IteratorFactory.pm line 211 TAP::Parser::IteratorFactory::make_iterator(TAP::Parser::IteratorFactory=HASH(0x557f31f8c9e0), TAP::Parser::Source=HASH(0x557f31f97dc8)) called at /usr/share/perl/5.32/TAP/Parser.pm line 472 TAP::Parser::_initialize(TAP::Parser=HASH(0x557f31f8cc50), HASH(0x557f31d48bd0)) called at /usr/share/perl/5.32/TAP/Object.pm line 55 TAP::Object::new("TAP::Parser", HASH(0x557f31d48bd0)) called at /usr/share/perl/5.32/TAP/Object.pm line 130 TAP::Object::_construct(TAP::Harness=HASH(0x557f3157fad0), "TAP::Parser", HASH(0x557f31d48bd0)) called at /usr/share/perl/5.32/TAP/Harness.pm line 852 TAP::Harness::make_parser(TAP::Harness=HASH(0x557f3157fad0), TAP::Parser::Scheduler::Job=HASH(0x557f31d35178)) called at /usr/share/perl/5.32/TAP/Harness.pm line 651 TAP::Harness::_aggregate_single(TAP::Harness=HASH(0x557f3157fad0), TAP::Parser::Aggregator=HASH(0x557f319ff2e8), TAP::Parser::Scheduler=HASH(0x557f31adeb28)) called at /usr/share/perl/5.32/TAP/Harness.pm line 743 TAP::Harness::aggregate_tests(TAP::Harness=HASH(0x557f3157fad0), TAP::Parser::Aggregator=HASH(0x557f319ff2e8), "t/Auth.t", "t/db_dependent/Auth_with_ldap.t", "t/Auth_with_shibboleth.t", "t/db_dependent/Auth_with_cas.t", "t/db_dependent/Auth.t", "t/db_dependent/api/v1/password_validation.t") called at /usr/share/perl/5.32/TAP/Harness.pm line 558 TAP::Harness::__ANON__() called at /usr/share/perl/5.32/TAP/Harness.pm line 571 TAP::Harness::runtests(TAP::Harness=HASH(0x557f3157fad0), "t/Auth.t", "t/db_dependent/Auth_with_ldap.t", "t/Auth_with_shibboleth.t", "t/db_dependent/Auth_with_cas.t", "t/db_dependent/Auth.t", "t/db_dependent/api/v1/password_validation.t") called at /usr/share/perl/5.32/App/Prove.pm line 548 App::Prove::_runtests(App::Prove=HASH(0x557f31549538), HASH(0x557f319c6230), "t/Auth.t", "t/db_dependent/Auth_with_ldap.t", "t/Auth_with_shibboleth.t", "t/db_dependent/Auth_with_cas.t", "t/db_dependent/Auth.t", "t/db_dependent/api/v1/password_validation.t") called at /usr/share/perl/5.32/App/Prove.pm line 506 App::Prove::run(App::Prove=HASH(0x557f31549538)) called at /usr/bin/prove line 13
(In reply to Aleisha Amohia from comment #47) > Can't locate object method "prepare" via package > "ePbNiHVXA8SXmNiUYlABl7X_Bvz5eVDlV63G7m2VlFElfda9J80fWIlZQvEJIm6Km" (perhaps > you forgot to load > "ePbNiHVXA8SXmNiUYlABl7X_Bvz5eVDlV63G7m2VlFElfda9J80fWIlZQvEJIm6Km"?) at > /kohadevbox/koha/C4/Auth.pm line 2001. $dbh is no longer a parameter. 1896 sub checkpw { 1897 my ( $dbh, $userid, $password, $query, $type, $no_set_userenv ) = @_; vs + my @return = checkpw( $patron->userid, $password, undef, ); You may consider backporting bug 27342. But all this will end badly I am sure...
(In reply to Jonathan Druart from comment #48) > (In reply to Aleisha Amohia from comment #47) > > Can't locate object method "prepare" via package > > "ePbNiHVXA8SXmNiUYlABl7X_Bvz5eVDlV63G7m2VlFElfda9J80fWIlZQvEJIm6Km" (perhaps > > you forgot to load > > "ePbNiHVXA8SXmNiUYlABl7X_Bvz5eVDlV63G7m2VlFElfda9J80fWIlZQvEJIm6Km"?) at > > /kohadevbox/koha/C4/Auth.pm line 2001. > > $dbh is no longer a parameter. > > > 1896 sub checkpw { > 1897 my ( $dbh, $userid, $password, $query, $type, $no_set_userenv ) = > @_; > > vs > > + my @return = checkpw( $patron->userid, $password, undef, ); > > You may consider backporting bug 27342. But all this will end badly I am > sure... Thanks for pointing that out, we have now backported bug 27342 to 22.05.x The test output is now: kohadev-koha@kohadevbox:koha((2895060ac8...))$ prove t/Auth.t t/db_dependent/Auth_with_ldap.t t/Auth_with_shibboleth.t t/db_dependent/Auth_with_cas.t t/db_dependent/Auth.t t/db_dependent/api/v1/password_validation.t t/Auth.t ..................................... ok t/db_dependent/Auth_with_ldap.t .............. 1/4 Subroutine ldapserver_error redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 45, <DATA> line 960. Subroutine description redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 80, <DATA> line 960. Subroutine search_method redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 87, <DATA> line 960. Subroutine checkpw_ldap redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 111, <DATA> line 960. Subroutine ldap_entry_2_hash redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 261, <DATA> line 960. Subroutine exists_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 309, <DATA> line 960. Subroutine _do_changepassword redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 335, <DATA> line 960. Subroutine update_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 366, <DATA> line 960. Subroutine ldapserver_error redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 45, <DATA> line 960. Subroutine description redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 80, <DATA> line 960. Subroutine search_method redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 87, <DATA> line 960. Subroutine checkpw_ldap redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 111, <DATA> line 960. Subroutine ldap_entry_2_hash redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 261, <DATA> line 960. Subroutine exists_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 309, <DATA> line 960. Subroutine _do_changepassword redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 335, <DATA> line 960. Subroutine update_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 366, <DATA> line 960. Subroutine ldapserver_error redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 45, <DATA> line 960. Subroutine description redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 80, <DATA> line 960. Subroutine search_method redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 87, <DATA> line 960. Subroutine checkpw_ldap redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 111, <DATA> line 960. Subroutine ldap_entry_2_hash redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 261, <DATA> line 960. Subroutine exists_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 309, <DATA> line 960. Subroutine _do_changepassword redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 335, <DATA> line 960. Subroutine update_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 366, <DATA> line 960. Subroutine ldapserver_error redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 45. Subroutine description redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 80. Subroutine search_method redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 87. Subroutine checkpw_ldap redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 111. Subroutine ldap_entry_2_hash redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 261. Subroutine exists_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 309. Subroutine _do_changepassword redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 335. Subroutine update_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 366. Subroutine ldapserver_error redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 45. Subroutine description redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 80. Subroutine search_method redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 87. Subroutine checkpw_ldap redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 111. Subroutine ldap_entry_2_hash redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 261. Subroutine exists_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 309. Subroutine _do_changepassword redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 335. Subroutine update_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 366. Subroutine ldapserver_error redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 45. Subroutine description redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 80. Subroutine search_method redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 87. Subroutine checkpw_ldap redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 111. Subroutine ldap_entry_2_hash redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 261. Subroutine exists_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 309. Subroutine _do_changepassword redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 335. Subroutine update_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 366. Subroutine ldapserver_error redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 45. Subroutine description redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 80. Subroutine search_method redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 87. Subroutine checkpw_ldap redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 111. Subroutine ldap_entry_2_hash redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 261. Subroutine exists_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 309. Subroutine _do_changepassword redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 335. Subroutine update_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 366. Subroutine ldapserver_error redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 45. Subroutine description redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 80. Subroutine search_method redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 87. Subroutine checkpw_ldap redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 111. Subroutine ldap_entry_2_hash redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 261. Subroutine exists_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 309. Subroutine _do_changepassword redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 335. Subroutine update_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 366. Subroutine ldapserver_error redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 45. Subroutine description redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 80. Subroutine search_method redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 87. Subroutine checkpw_ldap redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 111. Subroutine ldap_entry_2_hash redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 261. Subroutine exists_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 309. Subroutine _do_changepassword redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 335. Subroutine update_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 366. Subroutine ldapserver_error redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 45. Subroutine description redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 80. Subroutine search_method redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 87. Subroutine checkpw_ldap redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 111. Subroutine ldap_entry_2_hash redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 261. Subroutine exists_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 309. Subroutine _do_changepassword redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 335. Subroutine update_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 366. Subroutine ldapserver_error redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 45. Subroutine description redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 80. Subroutine search_method redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 87. Subroutine checkpw_ldap redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 111. Subroutine ldap_entry_2_hash redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 261. Subroutine exists_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 309. Subroutine _do_changepassword redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 335. Subroutine update_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 366. t/db_dependent/Auth_with_ldap.t .............. ok t/Auth_with_shibboleth.t ..................... ok t/db_dependent/Auth_with_cas.t ............... ok t/db_dependent/Auth.t ........................ 1/17 Use of uninitialized value $return in numeric gt (>) at /kohadevbox/koha/C4/Auth.pm line 1156. WARNING: The key derivation method "opensslv1" is deprecated. Using -pbkdf=>'pbkdf2' would be better. Pass -nodeprecate=>1 to inhibit this message. at /kohadevbox/koha/Koha/Patron.pm line 2169. t/db_dependent/Auth.t ........................ ok Cannot detect source of 't/db_dependent/api/v1/password_validation.t'! at /usr/share/perl/5.32/TAP/Parser/IteratorFactory.pm line 256. TAP::Parser::IteratorFactory::detect_source(TAP::Parser::IteratorFactory=HASH(0x55588017a4f0), TAP::Parser::Source=HASH(0x555880167648)) called at /usr/share/perl/5.32/TAP/Parser/IteratorFactory.pm line 211 TAP::Parser::IteratorFactory::make_iterator(TAP::Parser::IteratorFactory=HASH(0x55588017a4f0), TAP::Parser::Source=HASH(0x555880167648)) called at /usr/share/perl/5.32/TAP/Parser.pm line 472 TAP::Parser::_initialize(TAP::Parser=HASH(0x55588005bb98), HASH(0x55587ff19b28)) called at /usr/share/perl/5.32/TAP/Object.pm line 55 TAP::Object::new("TAP::Parser", HASH(0x55587ff19b28)) called at /usr/share/perl/5.32/TAP/Object.pm line 130 TAP::Object::_construct(TAP::Harness=HASH(0x55587f74e538), "TAP::Parser", HASH(0x55587ff19b28)) called at /usr/share/perl/5.32/TAP/Harness.pm line 852 TAP::Harness::make_parser(TAP::Harness=HASH(0x55587f74e538), TAP::Parser::Scheduler::Job=HASH(0x55587ff03c70)) called at /usr/share/perl/5.32/TAP/Harness.pm line 651 TAP::Harness::_aggregate_single(TAP::Harness=HASH(0x55587f74e538), TAP::Parser::Aggregator=HASH(0x55587fba39f0), TAP::Parser::Scheduler=HASH(0x55587fcac4b0)) called at /usr/share/perl/5.32/TAP/Harness.pm line 743 TAP::Harness::aggregate_tests(TAP::Harness=HASH(0x55587f74e538), TAP::Parser::Aggregator=HASH(0x55587fba39f0), "t/Auth.t", "t/db_dependent/Auth_with_ldap.t", "t/Auth_with_shibboleth.t", "t/db_dependent/Auth_with_cas.t", "t/db_dependent/Auth.t", "t/db_dependent/api/v1/password_validation.t") called at /usr/share/perl/5.32/TAP/Harness.pm line 558 TAP::Harness::__ANON__() called at /usr/share/perl/5.32/TAP/Harness.pm line 571 TAP::Harness::runtests(TAP::Harness=HASH(0x55587f74e538), "t/Auth.t", "t/db_dependent/Auth_with_ldap.t", "t/Auth_with_shibboleth.t", "t/db_dependent/Auth_with_cas.t", "t/db_dependent/Auth.t", "t/db_dependent/api/v1/password_validation.t") called at /usr/share/perl/5.32/App/Prove.pm line 548 App::Prove::_runtests(App::Prove=HASH(0x55587f718538), HASH(0x55587fb93158), "t/Auth.t", "t/db_dependent/Auth_with_ldap.t", "t/Auth_with_shibboleth.t", "t/db_dependent/Auth_with_cas.t", "t/db_dependent/Auth.t", "t/db_dependent/api/v1/password_validation.t") called at /usr/share/perl/5.32/App/Prove.pm line 506 App::Prove::run(App::Prove=HASH(0x55587f718538)) called at /usr/bin/prove line 13
kohadev-koha@kohadevbox:koha((2895060ac8...))$ prove t/Auth.t t/db_dependent/Auth_with_ldap.t t/Auth_with_shibboleth.t t/db_dependent/Auth_with_cas.t t/db_dependent/Auth.t t/Auth.t ......................... ok t/db_dependent/Auth_with_ldap.t .. 1/4 Subroutine ldapserver_error redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 45, <DATA> line 960. Subroutine description redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 80, <DATA> line 960. Subroutine search_method redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 87, <DATA> line 960. Subroutine checkpw_ldap redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 111, <DATA> line 960. Subroutine ldap_entry_2_hash redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 261, <DATA> line 960. Subroutine exists_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 309, <DATA> line 960. Subroutine _do_changepassword redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 335, <DATA> line 960. Subroutine update_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 366, <DATA> line 960. Subroutine ldapserver_error redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 45, <DATA> line 960. Subroutine description redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 80, <DATA> line 960. Subroutine search_method redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 87, <DATA> line 960. Subroutine checkpw_ldap redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 111, <DATA> line 960. Subroutine ldap_entry_2_hash redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 261, <DATA> line 960. Subroutine exists_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 309, <DATA> line 960. Subroutine _do_changepassword redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 335, <DATA> line 960. Subroutine update_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 366, <DATA> line 960. Subroutine ldapserver_error redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 45, <DATA> line 960. Subroutine description redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 80, <DATA> line 960. Subroutine search_method redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 87, <DATA> line 960. Subroutine checkpw_ldap redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 111, <DATA> line 960. Subroutine ldap_entry_2_hash redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 261, <DATA> line 960. Subroutine exists_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 309, <DATA> line 960. Subroutine _do_changepassword redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 335, <DATA> line 960. Subroutine update_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 366, <DATA> line 960. Subroutine ldapserver_error redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 45. Subroutine description redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 80. Subroutine search_method redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 87. Subroutine checkpw_ldap redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 111. Subroutine ldap_entry_2_hash redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 261. Subroutine exists_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 309. Subroutine _do_changepassword redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 335. Subroutine update_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 366. Subroutine ldapserver_error redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 45. Subroutine description redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 80. Subroutine search_method redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 87. Subroutine checkpw_ldap redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 111. Subroutine ldap_entry_2_hash redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 261. Subroutine exists_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 309. Subroutine _do_changepassword redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 335. Subroutine update_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 366. Subroutine ldapserver_error redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 45. Subroutine description redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 80. Subroutine search_method redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 87. Subroutine checkpw_ldap redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 111. Subroutine ldap_entry_2_hash redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 261. Subroutine exists_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 309. Subroutine _do_changepassword redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 335. Subroutine update_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 366. Subroutine ldapserver_error redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 45. Subroutine description redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 80. Subroutine search_method redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 87. Subroutine checkpw_ldap redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 111. Subroutine ldap_entry_2_hash redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 261. Subroutine exists_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 309. Subroutine _do_changepassword redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 335. Subroutine update_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 366. Subroutine ldapserver_error redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 45. Subroutine description redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 80. Subroutine search_method redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 87. Subroutine checkpw_ldap redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 111. Subroutine ldap_entry_2_hash redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 261. Subroutine exists_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 309. Subroutine _do_changepassword redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 335. Subroutine update_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 366. Subroutine ldapserver_error redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 45. Subroutine description redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 80. Subroutine search_method redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 87. Subroutine checkpw_ldap redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 111. Subroutine ldap_entry_2_hash redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 261. Subroutine exists_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 309. Subroutine _do_changepassword redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 335. Subroutine update_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 366. Subroutine ldapserver_error redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 45. Subroutine description redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 80. Subroutine search_method redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 87. Subroutine checkpw_ldap redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 111. Subroutine ldap_entry_2_hash redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 261. Subroutine exists_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 309. Subroutine _do_changepassword redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 335. Subroutine update_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 366. Subroutine ldapserver_error redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 45. Subroutine description redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 80. Subroutine search_method redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 87. Subroutine checkpw_ldap redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 111. Subroutine ldap_entry_2_hash redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 261. Subroutine exists_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 309. Subroutine _do_changepassword redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 335. Subroutine update_local redefined at /kohadevbox/koha/C4/Auth_with_ldap.pm line 366. t/db_dependent/Auth_with_ldap.t .. ok t/Auth_with_shibboleth.t ......... ok t/db_dependent/Auth_with_cas.t ... ok t/db_dependent/Auth.t ............ 1/17 Use of uninitialized value $return in numeric gt (>) at /kohadevbox/koha/C4/Auth.pm line 1156. WARNING: The key derivation method "opensslv1" is deprecated. Using -pbkdf=>'pbkdf2' would be better. Pass -nodeprecate=>1 to inhibit this message. at /kohadevbox/koha/Koha/Patron.pm line 2169. t/db_dependent/Auth.t ............ ok All tests successful. Files=5, Tests=56, 15 wallclock secs ( 0.05 usr 0.01 sys + 12.60 cusr 1.32 csys = 13.98 CPU) Result: PASS
Please see [Koha-devel] Session corruption in koha-testing-docker master Seems related to this change.
(In reply to Jonathan Druart from comment #51) > Please see [Koha-devel] Session corruption in koha-testing-docker master > > Seems related to this change. This is bug 36034.
Created attachment 163964 [details] [review] Bug 34893: [22.05] (follow-up) change number of tests
(In reply to wainuiwitikapark from comment #53) > Created attachment 163964 [details] [review] [review] > Bug 34893: [22.05] (follow-up) change number of tests Added a new patch to change number of tests so my jenkins build passes https://jenkins.koha-community.org/job/Koha_Sec/job/Koha_22.05/lastCompletedBuild/testReport/(root)/t_db_dependent_ILSDI_Services_t/Number_of_runned_tests_does_not_match_plan___376_/
> Added a new patch to change number of tests so my jenkins build passes Ah yes, that often happens. I don't know if in the lasts cycles something changed but if not: when that happened to me I would just commit to the branch and not bother attaching a follow-up in the ticket. And if caught that when running the tests locally when backporting, I would append the patch that changed the test. If that can help you getting rid quicker of those test number issues.