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>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
This commit is contained in:
Kyle Hall 2023-12-01 06:29:19 -05:00 committed by Katrin Fischer
parent 11e919cc19
commit 2b54d3c82b
Signed by: kfischer
GPG key ID: 0EF6E2C03357A834
3 changed files with 67 additions and 57 deletions

View file

@ -1940,29 +1940,31 @@ sub checkpw {
$type = 'opac' unless $type;
# Get shibboleth login attribute
my $shib = C4::Context->config('useshibboleth') && shib_ok();
my $shib = C4::Context->config('useshibboleth') && shib_ok();
my $shib_login = $shib ? get_login_shib() : undef;
my @return;
my $patron;
if ( defined $userid ){
$patron = Koha::Patrons->find({ userid => $userid });
$patron = Koha::Patrons->find({ cardnumber => $userid }) unless $patron;
if ( defined $userid ) {
$patron = Koha::Patrons->find( { userid => $userid } );
$patron = Koha::Patrons->find( { cardnumber => $userid } ) unless $patron;
}
my $check_internal_as_fallback = 0;
my $passwd_ok = 0;
my $passwd_ok = 0;
# Note: checkpw_* routines returns:
# 1 if auth is ok
# 0 if auth is nok
# -1 if user bind failed (LDAP only)
if ( $patron and ( $patron->account_locked ) ) {
if ( $patron and ( $patron->account_locked ) ) {
# Nothing to check, account is locked
} elsif ($ldap && defined($password)) {
} elsif ( $ldap && defined($password) ) {
my ( $retval, $retcard, $retuserid );
( $retval, $retcard, $retuserid, $patron ) = checkpw_ldap(@_); # EXTERNAL AUTH
if ( $retval == 1 ) {
@return = ( $retval, $retcard, $retuserid, $patron );
@return = ( $retval, $retcard, $retuserid, $patron );
$passwd_ok = 1;
}
$check_internal_as_fallback = 1 if $retval == 0;
@ -1971,9 +1973,10 @@ sub checkpw {
# In case of a CAS authentication, we use the ticket instead of the password
my $ticket = $query->param('ticket');
$query->delete('ticket'); # remove ticket to come back to original URL
my ( $retval, $retcard, $retuserid, $cas_ticket, $patron ) = checkpw_cas( $ticket, $query, $type ); # EXTERNAL AUTH
if ( $retval ) {
$query->delete('ticket'); # remove ticket to come back to original URL
my ( $retval, $retcard, $retuserid, $cas_ticket, $patron ) =
checkpw_cas( $ticket, $query, $type ); # EXTERNAL AUTH
if ($retval) {
@return = ( $retval, $retcard, $retuserid, $patron, $cas_ticket );
} else {
@return = (0);
@ -1992,8 +1995,9 @@ sub checkpw {
# Then, we check if it matches a valid koha user
if ($shib_login) {
my ( $retval, $retcard, $retuserid, $patron ) = C4::Auth_with_shibboleth::checkpw_shib($shib_login); # EXTERNAL AUTH
if ( $retval ) {
my ( $retval, $retcard, $retuserid, $patron ) =
C4::Auth_with_shibboleth::checkpw_shib($shib_login); # EXTERNAL AUTH
if ($retval) {
@return = ( $retval, $retcard, $retuserid, $patron );
}
$passwd_ok = $retval;
@ -2003,27 +2007,27 @@ sub checkpw {
}
# INTERNAL AUTH
if ( $check_internal_as_fallback ) {
@return = checkpw_internal( $userid, $password, $no_set_userenv);
push(@return, $patron);
$passwd_ok = 1 if $return[0] > 0; # 1 or 2
if ($check_internal_as_fallback) {
@return = checkpw_internal( $userid, $password, $no_set_userenv );
push( @return, $patron );
$passwd_ok = 1 if $return[0] > 0; # 1 or 2
}
if( $patron ) {
if ( $passwd_ok ) {
$patron->update({ login_attempts => 0 });
if( $patron->password_expired ){
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 });
} elsif ( !$patron->account_locked ) {
$patron->update( { login_attempts => $patron->login_attempts + 1 } );
}
}
# Optionally log success or failure
if( $patron && $passwd_ok && C4::Context->preference('AuthSuccessLog') ) {
if ( $patron && $passwd_ok && C4::Context->preference('AuthSuccessLog') ) {
logaction( 'AUTH', 'SUCCESS', $patron->id, "Valid password for $userid", $type );
} elsif( !$passwd_ok && C4::Context->preference('AuthFailureLog') ) {
} elsif ( !$passwd_ok && C4::Context->preference('AuthFailureLog') ) {
logaction( 'AUTH', 'FAILURE', $patron ? $patron->id : 0, "Wrong password for $userid", $type );
}

View file

@ -117,13 +117,13 @@ sub checkpw_cas {
# we should store the CAS ticekt too, we need this for single logout https://apereo.github.io/cas/4.2.x/protocol/CAS-Protocol-Specification.html#233-single-logout
# Does it match one of our users ?
my $dbh = C4::Context->dbh;
my $patron = Koha::Patrons->find({ userid => $userid });
if ( $patron ) {
my $dbh = C4::Context->dbh;
my $patron = Koha::Patrons->find( { userid => $userid } );
if ($patron) {
return ( 1, $patron->cardnumber, $patron->userid, $ticket, $patron );
}
$patron = Koha::Patrons->find({ cardnumber => $userid });
if ( $patron ) {
$patron = Koha::Patrons->find( { cardnumber => $userid } );
if ($patron) {
return ( 1, $patron->cardnumber, $patron->userid, $ticket, $patron );
}

View file

@ -1052,41 +1052,47 @@ subtest 'Bug 34893: ILS-DI can return the wrong patron for AuthenticatePatron' =
my $plain_password = 'tomasito';
$builder->build({
source => 'Borrower',
value => {
cardnumber => undef,
$builder->build(
{
source => 'Borrower',
value => {
cardnumber => undef,
}
}
});
);
my $borrower0 = $builder->build({
source => 'Borrower',
value => {
cardnumber => "cardnumber1",
userid => undef,
password => Koha::AuthUtils::hash_password( $plain_password ),
lastseen => "2001-01-01 12:34:56"
my $borrower0 = $builder->build(
{
source => 'Borrower',
value => {
cardnumber => "cardnumber1",
userid => undef,
password => Koha::AuthUtils::hash_password($plain_password),
lastseen => "2001-01-01 12:34:56"
}
}
});
);
my $borrower = $builder->build({
source => 'Borrower',
value => {
cardnumber => "cardnumber2",
userid => undef,
password => Koha::AuthUtils::hash_password( $plain_password ),
lastseen => "2001-01-01 12:34:56"
my $borrower = $builder->build(
{
source => 'Borrower',
value => {
cardnumber => "cardnumber2",
userid => undef,
password => Koha::AuthUtils::hash_password($plain_password),
lastseen => "2001-01-01 12:34:56"
}
}
});
);
my $query = CGI->new;
$query->param( 'username', $borrower->{cardnumber});
$query->param( 'password', $plain_password);
$query->param( 'username', $borrower->{cardnumber} );
$query->param( 'password', $plain_password );
my $reply = C4::ILSDI::Services::AuthenticatePatron( $query );
is( $reply->{id}, $borrower->{borrowernumber}, "userid and password - Patron authenticated" );
is( $reply->{code}, undef, "Error code undef");
my $seen_patron = Koha::Patrons->find({ borrowernumber => $reply->{id} });
my $reply = C4::ILSDI::Services::AuthenticatePatron($query);
is( $reply->{id}, $borrower->{borrowernumber}, "userid and password - Patron authenticated" );
is( $reply->{code}, undef, "Error code undef" );
my $seen_patron = Koha::Patrons->find( { borrowernumber => $reply->{id} } );
$schema->storage->txn_rollback;
};