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 <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
This commit is contained in:
Tomas Cohen Arazi 2025-07-21 13:52:57 -03:00 committed by Martin Renvoize
parent 91f3e51e1a
commit bf0ed60e73
No known key found for this signature in database
GPG Key ID: 2D86E0EAEC83711B
2 changed files with 6 additions and 17 deletions

View File

@ -2114,23 +2114,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;

View File

@ -1650,14 +1650,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' );