Bug 34893 - ILS-DI can return the wrong patron for AuthenticatePatron
Summary: ILS-DI can return the wrong patron for AuthenticatePatron
Status: Pushed to oldoldoldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Web services (show other bugs)
Version: 23.11
Hardware: All All
: P5 - low blocker (vote)
Assignee: Kyle M Hall
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on:
Blocks: 36034 36161 36575
  Show dependency treegraph
 
Reported: 2023-09-22 18:19 UTC by Kyle M Hall
Modified: 2024-05-03 11:12 UTC (History)
16 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00,23.11.02,23.05.08,22.11.14,22.05.18


Attachments
Bug 34893: ILS-DI can return the wrong patron for AuthenticatePatron (7.89 KB, patch)
2023-09-22 18:25 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 34893: ILS-DI can return the wrong patron for AuthenticatePatron (7.99 KB, patch)
2023-09-22 18:29 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 34893: ILS-DI can return the wrong patron for AuthenticatePatron (7.96 KB, patch)
2023-11-28 18:49 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 34893: ILS-DI can return the wrong patron for AuthenticatePatron (8.21 KB, patch)
2023-11-28 22:13 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 34893: Add checkpw change to REST API (1.64 KB, patch)
2023-11-28 23:05 UTC, David Cook
Details | Diff | Splinter Review
Bug 34893: Add unit tests (2.20 KB, patch)
2023-11-29 17:22 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 34893: (QA follow-up) Tidy code for qa script (9.05 KB, patch)
2023-12-01 11:44 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 34893: Add unit tests (2.25 KB, patch)
2023-12-03 23:31 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 34893: ILS-DI can return the wrong patron for AuthenticatePatron (8.21 KB, patch)
2023-12-03 23:31 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 34893: Add checkpw change to REST API (1.70 KB, patch)
2023-12-03 23:31 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 34893: (QA follow-up) Tidy code for qa script (9.11 KB, patch)
2023-12-03 23:31 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 34893: Add unit tests (2.31 KB, patch)
2023-12-13 12:53 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 34893: ILS-DI can return the wrong patron for AuthenticatePatron (8.28 KB, patch)
2023-12-13 12:53 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 34893: Add checkpw change to REST API (1.75 KB, patch)
2023-12-13 12:53 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 34893: (QA follow-up) Tidy code for qa script (9.17 KB, patch)
2023-12-13 12:53 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 34893: Unit tests for C4::Auth::checkpw (10.95 KB, patch)
2023-12-15 13:58 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 34893: Add unit tests (2.31 KB, patch)
2024-01-17 12:11 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 34893: ILS-DI can return the wrong patron for AuthenticatePatron (8.28 KB, patch)
2024-01-17 12:11 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 34893: Add checkpw change to REST API (1.83 KB, patch)
2024-01-17 12:12 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 34893: (QA follow-up) Tidy code for qa script (9.17 KB, patch)
2024-01-17 12:12 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 34893: Unit tests for C4::Auth::checkpw (10.95 KB, patch)
2024-01-17 12:12 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 34893: [22.05] Add a Koha::Patron->update_lastseen method (969 bytes, patch)
2024-01-24 10:01 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 34893: [22.05] (follow-up) change number of tests (726 bytes, patch)
2024-03-27 01:05 UTC, wainuiwitikapark
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2023-09-22 18:19:55 UTC
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.
Comment 1 Kyle M Hall 2023-09-22 18:25:49 UTC
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!
Comment 2 Kyle M Hall 2023-09-22 18:29:27 UTC
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
Comment 3 Kyle M Hall 2023-09-22 18:41:27 UTC
As a corollary to this, we should probably alter borrowers.userid to be NOT NULL and UNIQUE.
Comment 4 Liz Rea 2023-09-22 18:53:04 UTC
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.
Comment 5 Victor Grousset/tuxayo 2023-11-27 02:00:51 UTC
> 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.
Comment 6 David Cook 2023-11-27 02:28:41 UTC
(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.
Comment 7 David Cook 2023-11-27 02:35:35 UTC
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...
Comment 8 Victor Grousset/tuxayo 2023-11-27 23:03:37 UTC
(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
Comment 9 Kyle M Hall 2023-11-28 18:49:34 UTC
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
Comment 10 Victor Grousset/tuxayo 2023-11-28 22:13:50 UTC
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>
Comment 11 Victor Grousset/tuxayo 2023-11-28 22:14:05 UTC
It works! :D

Changed test plan to reproduce consistently.
fixed t/Auth_with_shibboleth.t => t/db_dependent/Auth_with_shibboleth.t
Comment 12 David Cook 2023-11-28 22:41:58 UTC
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.
Comment 13 David Cook 2023-11-28 23:05:41 UTC
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
Comment 14 Jonathan Druart 2023-11-29 07:02:35 UTC
No test, failing QA.
Comment 15 Jonathan Druart 2023-11-29 07:07:44 UTC
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?
Comment 16 Kyle M Hall 2023-11-29 17:21:48 UTC
> 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!

👍

> 
> 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.
Comment 17 Kyle M Hall 2023-11-29 17:22:07 UTC
Created attachment 159356 [details] [review]
Bug 34893: Add unit tests
Comment 18 David Cook 2023-11-29 22:25:52 UTC
(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...)
Comment 19 Marcel de Rooy 2023-12-01 07:24:11 UTC
(In reply to Jonathan Druart from comment #15)
> 2. No tests, we are in C4::Auth! Test everything!

This is a QA blocker.
Comment 20 Marcel de Rooy 2023-12-01 07:24:47 UTC
 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)
Comment 21 Kyle M Hall 2023-12-01 11:23:47 UTC
(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? :)
Comment 22 Kyle M Hall 2023-12-01 11:44:04 UTC
Created attachment 159441 [details] [review]
Bug 34893: (QA follow-up) Tidy code for qa script
Comment 23 Victor Grousset/tuxayo 2023-12-02 01:25:26 UTC
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 🙃!!!

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
Comment 24 David Cook 2023-12-03 22:48:17 UTC
(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.
Comment 25 Victor Grousset/tuxayo 2023-12-03 23:31:03 UTC
Created attachment 159506 [details] [review]
Bug 34893: Add unit tests

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 26 Victor Grousset/tuxayo 2023-12-03 23:31:07 UTC
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>
Comment 27 Victor Grousset/tuxayo 2023-12-03 23:31:11 UTC
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>
Comment 28 Victor Grousset/tuxayo 2023-12-03 23:31:14 UTC
Created attachment 159509 [details] [review]
Bug 34893: (QA follow-up) Tidy code for qa script

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 29 Victor Grousset/tuxayo 2023-12-03 23:32:19 UTC
(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)
Comment 30 Tomás Cohen Arazi 2023-12-13 12:53:11 UTC
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>
Comment 31 Tomás Cohen Arazi 2023-12-13 12:53:15 UTC
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>
Comment 32 Tomás Cohen Arazi 2023-12-13 12:53:19 UTC
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>
Comment 33 Tomás Cohen Arazi 2023-12-13 12:53:23 UTC
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>
Comment 34 Tomás Cohen Arazi 2023-12-13 13:06:58 UTC
Kyle: I think we need actual tests for the C4::Auth::checkpw() behavior change.
Comment 35 Tomás Cohen Arazi 2023-12-14 19:11:18 UTC
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.
Comment 36 Tomás Cohen Arazi 2023-12-15 13:58:41 UTC
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>
Comment 37 Tomás Cohen Arazi 2023-12-15 14:00:32 UTC
(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.
Comment 38 Katrin Fischer 2024-01-17 12:04:17 UTC
This no longer applies cleanly, could I get a speedy rebase please?
Comment 39 Tomás Cohen Arazi 2024-01-17 12:11:32 UTC
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>
Comment 40 Tomás Cohen Arazi 2024-01-17 12:11:58 UTC
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>
Comment 41 Tomás Cohen Arazi 2024-01-17 12:12:18 UTC
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>
Comment 42 Tomás Cohen Arazi 2024-01-17 12:12:45 UTC
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>
Comment 43 Tomás Cohen Arazi 2024-01-17 12:12:58 UTC
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>
Comment 44 Tomás Cohen Arazi 2024-01-17 12:14:04 UTC
Rebased on top of bug 35204 which is in master.
Comment 45 wainuiwitikapark 2024-01-24 08:31:43 UTC
Backported patched to 22.05.x-security

Having trouble testing though if anyone is able to help me please
Comment 46 Aleisha Amohia 2024-01-24 10:01:38 UTC
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!
Comment 47 Aleisha Amohia 2024-01-24 10:34:34 UTC
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
Comment 48 Jonathan Druart 2024-01-24 11:23:58 UTC
(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...
Comment 49 wainuiwitikapark 2024-01-24 21:05:59 UTC
(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
Comment 50 wainuiwitikapark 2024-01-24 23:41:20 UTC
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
Comment 51 Jonathan Druart 2024-02-13 14:41:52 UTC
Please see [Koha-devel] Session corruption in koha-testing-docker master

Seems related to this change.
Comment 52 Jonathan Druart 2024-02-13 15:49:49 UTC
(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.
Comment 53 wainuiwitikapark 2024-03-27 01:05:32 UTC
Created attachment 163964 [details] [review]
Bug 34893: [22.05] (follow-up) change number of tests
Comment 54 wainuiwitikapark 2024-03-27 01:06:30 UTC
(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_/
Comment 55 Victor Grousset/tuxayo 2024-03-27 04:22:47 UTC
> 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.