From 515ccd9c65b6b584a39172325a2fe91b77d39e26 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Thu, 11 Apr 2024 12:18:30 +0200 Subject: [PATCH] Bug 36575: (QA follow-up) Signed-off-by: Martin Renvoize Signed-off-by: Kyle M Hall --- C4/Auth.pm | 17 +++++++-------- t/db_dependent/Auth.t | 14 ++++++------ t/db_dependent/api/v1/password_validation.t | 24 ++++++++++++--------- 3 files changed, 29 insertions(+), 26 deletions(-) diff --git a/C4/Auth.pm b/C4/Auth.pm index d127d567fd7..747e87c327b 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -1990,8 +1990,7 @@ sub checkpw { my $ticket = $query->param('ticket'); $query->delete('ticket'); # remove ticket to come back to original URL my ( $retval, $retcard, $retuserid, $cas_ticket ); - ( $retval, $retcard, $retuserid, $cas_ticket, $patron ) = - checkpw_cas( $ticket, $query, $type ); # EXTERNAL AUTH + ( $retval, $retcard, $retuserid, $cas_ticket, $patron ) = checkpw_cas( $ticket, $query, $type ); # EXTERNAL AUTH if ($retval) { @return = ( $retval, $retcard, $retuserid, $patron, $cas_ticket ); } else { @@ -2023,22 +2022,22 @@ sub checkpw { $check_internal_as_fallback = 1; } - if ( $check_internal_as_fallback ){ - # INTERNAL AUTH - @return = checkpw_internal( $userid, $password, $no_set_userenv ); - $passwd_ok = 1 if $return[0] > 0; # 1 or 2 - $patron = Koha::Patrons->find({ cardnumber => $return[1] }) if $passwd_ok; + if ($check_internal_as_fallback) { + # INTERNAL AUTH + @return = checkpw_internal( $userid, $password, $no_set_userenv ); + $passwd_ok = 1 if $return[0] > 0; # 1 or 2 + $patron = Koha::Patrons->find( { cardnumber => $return[1] } ) if $passwd_ok; push @return, $patron if $patron; } - if ( defined $userid && !$patron ) { + if ( defined $userid && !$patron ) { $patron = Koha::Patrons->find( { userid => $userid } ); $patron = Koha::Patrons->find( { cardnumber => $userid } ) unless $patron; push @return, $patron if $check_internal_as_fallback; } if ($patron) { - if( $patron->account_locked ){ + if ( $patron->account_locked ) { @return = (); } elsif ($passwd_ok) { $patron->update( { login_attempts => 0 } ); diff --git a/t/db_dependent/Auth.t b/t/db_dependent/Auth.t index a8bfa6ea65b..dbd5c8011c4 100755 --- a/t/db_dependent/Auth.t +++ b/t/db_dependent/Auth.t @@ -1447,23 +1447,23 @@ subtest 'checkpw for users with shared cardnumber / userid ' => sub { plan tests => 8; t::lib::Mocks::mock_preference( 'RequireStrongPassword', 0 ); - my $library = $builder->build_object( { class => 'Koha::Libraries' } ); - my $patron_1 = $builder->build_object( { class => 'Koha::Patrons' } ); + my $library = $builder->build_object( { class => 'Koha::Libraries' } ); + my $patron_1 = $builder->build_object( { class => 'Koha::Patrons' } ); $patron_1->set_password( { password => "OnePassword" } ); - my $patron_2 = $builder->build_object( { class => 'Koha::Patrons', value => { userid => $patron_1->cardnumber } } ); + 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' ); 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' ); + 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' ); 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' ); + is( $userid, $patron_2->userid, 'checkpw returns correct userid' ); + is( $patron->id, $patron_2->id, 'checkpw returns correct patron' ); }; diff --git a/t/db_dependent/api/v1/password_validation.t b/t/db_dependent/api/v1/password_validation.t index 1aef7187008..e5d891200f4 100755 --- a/t/db_dependent/api/v1/password_validation.t +++ b/t/db_dependent/api/v1/password_validation.t @@ -248,7 +248,7 @@ subtest 'password validation - users with shared cardnumber / userid' => sub { my $patron_1 = $builder->build_object( { class => 'Koha::Patrons', - value => { } + value => {} } ); my $patron_password_1 = 'thePassword123'; @@ -269,7 +269,8 @@ subtest 'password validation - users with shared cardnumber / userid' => sub { }; $t->post_ok( "//$userid:$password@/api/v1/auth/password/validation" => json => $json )->status_is(201) - ->json_is({ cardnumber => $patron_1->cardnumber, patron_id => $patron_1->borrowernumber, userid => $patron_1->userid} ); + ->json_is( + { cardnumber => $patron_1->cardnumber, patron_id => $patron_1->borrowernumber, userid => $patron_1->userid } ); $json = { identifier => $patron_2->userid, @@ -277,23 +278,26 @@ subtest 'password validation - users with shared cardnumber / userid' => sub { }; $t->post_ok( "//$userid:$password@/api/v1/auth/password/validation" => json => $json )->status_is(201) - ->json_is({ cardnumber => $patron_2->cardnumber, patron_id => $patron_2->borrowernumber, userid => $patron_2->userid} ); + ->json_is( + { cardnumber => $patron_2->cardnumber, patron_id => $patron_2->borrowernumber, userid => $patron_2->userid } ); - my $json = { - userid => $patron_1->cardnumber, - password => $patron_password_1, + $json = { + userid => $patron_1->cardnumber, + password => $patron_password_1, }; $t->post_ok( "//$userid:$password@/api/v1/auth/password/validation" => json => $json )->status_is(201) - ->json_is({ cardnumber => $patron_1->cardnumber, patron_id => $patron_1->borrowernumber, userid => $patron_1->userid} ); + ->json_is( + { cardnumber => $patron_1->cardnumber, patron_id => $patron_1->borrowernumber, userid => $patron_1->userid } ); $json = { - userid => $patron_2->userid, - password => $patron_password_2, + userid => $patron_2->userid, + password => $patron_password_2, }; $t->post_ok( "//$userid:$password@/api/v1/auth/password/validation" => json => $json )->status_is(201) - ->json_is({ cardnumber => $patron_2->cardnumber, patron_id => $patron_2->borrowernumber, userid => $patron_2->userid} ); + ->json_is( + { cardnumber => $patron_2->cardnumber, patron_id => $patron_2->borrowernumber, userid => $patron_2->userid } ); $schema->storage->txn_rollback; }; -- 2.39.2