From 0a9b7f7d5ed73117e17274d3acd5e495e93d4c8d Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Mon, 21 Jul 2025 13:52:57 -0300 Subject: [PATCH] Bug 40286: Make C4::Auth::checkpw_internal use Koha::Patrons->find_by_identifier This patch does what the title says. The code change is clear to understand. The check for userid vs. cardnumber is done in the `find_by_identifier` method so no need to do it here. Note: When `find_by_identifier` was written, I picked some other place's logic (API password validation) so `userid` is used first for finding the patron. Because of this, tests in t/db_dependent/Auth.t need to be adjusted. I believe this is not an issue, as those tests should be removed once we move forward with bug 33905, entirely. To test: 1. Apply this patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Auth.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi --- C4/Auth.pm | 15 ++------------- t/db_dependent/Auth.t | 8 ++++---- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/C4/Auth.pm b/C4/Auth.pm index 069edd42116..dddfebeb6f6 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -2182,23 +2182,12 @@ Missing POD for checkpw_internal. =cut sub checkpw_internal { - my ( $userid, $password, $no_set_userenv ) = @_; + my ( $identifier, $password, $no_set_userenv ) = @_; $password = Encode::encode( 'UTF-8', $password ) if Encode::is_utf8($password); - my $patron = Koha::Patrons->find( { userid => $userid } ); - if ($patron) { - if ( checkpw_hash( $password, $patron->password ) ) { - my $borrowernumber = $patron->borrowernumber; - C4::Context->set_userenv( - "$borrowernumber", $patron->userid, $patron->cardnumber, - $patron->firstname, $patron->surname, $patron->branchcode, $patron->library->branchname, $patron->flags - ) unless $no_set_userenv; - return 1, $patron->cardnumber, $patron->userid, $patron; - } - } - $patron = Koha::Patrons->find( { cardnumber => $userid } ); + my $patron = Koha::Patrons->find_by_identifier($identifier); if ($patron) { if ( checkpw_hash( $password, $patron->password ) ) { my $borrowernumber = $patron->borrowernumber; diff --git a/t/db_dependent/Auth.t b/t/db_dependent/Auth.t index a2c0a77da7a..abef79d3205 100755 --- a/t/db_dependent/Auth.t +++ b/t/db_dependent/Auth.t @@ -1649,14 +1649,14 @@ subtest 'checkpw for users with shared cardnumber / userid ' => sub { my $patron_2 = $builder->build_object( { class => 'Koha::Patrons', value => { userid => $patron_1->cardnumber } } ); $patron_2->set_password( { password => "PasswordTwo" } ); - my ( $checkpw, $cardnumber, $userid, $patron ) = checkpw( $patron_1->cardnumber, "OnePassword", undef, undef, 1 ); - ok( $checkpw, 'checkpw returns true for right password when logging in via cardnumber' ); + my ( $checkpw, $cardnumber, $userid, $patron ) = checkpw( $patron_1->userid, "OnePassword", undef, undef, 1 ); + ok( $checkpw, 'checkpw returns true for right password when logging in via usrid' ); is( $cardnumber, $patron_1->cardnumber, 'checkpw returns correct cardnumber' ); is( $userid, $patron_1->userid, 'checkpw returns correct userid' ); is( $patron->id, $patron_1->id, 'checkpw returns correct patron' ); - ( $checkpw, $cardnumber, $userid, $patron ) = checkpw( $patron_2->userid, "PasswordTwo", undef, undef, 1 ); - ok( $checkpw, 'checkpw returns true for right password when logging in via userid' ); + ( $checkpw, $cardnumber, $userid, $patron ) = checkpw( $patron_2->cardnumber, "PasswordTwo", undef, undef, 1 ); + ok( $checkpw, 'checkpw returns true for right password when logging in via carndumber' ); is( $cardnumber, $patron_2->cardnumber, 'checkpw returns correct cardnumber' ); is( $userid, $patron_2->userid, 'checkpw returns correct userid' ); is( $patron->id, $patron_2->id, 'checkpw returns correct patron' ); -- 2.50.1