Bugzilla – Attachment 159509 Details for
Bug 34893
ILS-DI can return the wrong patron for AuthenticatePatron
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 34893: (QA follow-up) Tidy code for qa script
Bug-34893-QA-follow-up-Tidy-code-for-qa-script.patch (text/plain), 9.11 KB, created by
Victor Grousset/tuxayo
on 2023-12-03 23:31:14 UTC
(
hide
)
Description:
Bug 34893: (QA follow-up) Tidy code for qa script
Filename:
MIME Type:
Creator:
Victor Grousset/tuxayo
Created:
2023-12-03 23:31:14 UTC
Size:
9.11 KB
patch
obsolete
>From d54a5a7e199bb7c8010a18ff63a8bcd75121db81 Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatersolutions.com> >Date: Fri, 1 Dec 2023 06:29:19 -0500 >Subject: [PATCH] Bug 34893: (QA follow-up) Tidy code for qa script > >Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> >--- > C4/Auth.pm | 54 +++++++++++++++-------------- > C4/Auth_with_cas.pm | 10 +++--- > t/db_dependent/ILSDI_Services.t | 60 ++++++++++++++++++--------------- > 3 files changed, 67 insertions(+), 57 deletions(-) > >diff --git a/C4/Auth.pm b/C4/Auth.pm >index 98e0d8198b..bcf65844a4 100644 >--- a/C4/Auth.pm >+++ b/C4/Auth.pm >@@ -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 ); > } > >diff --git a/C4/Auth_with_cas.pm b/C4/Auth_with_cas.pm >index e1cb1a97b2..68c2c48a47 100644 >--- a/C4/Auth_with_cas.pm >+++ b/C4/Auth_with_cas.pm >@@ -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 ); > } > >diff --git a/t/db_dependent/ILSDI_Services.t b/t/db_dependent/ILSDI_Services.t >index 9fb01180a4..442f1c2402 100755 >--- a/t/db_dependent/ILSDI_Services.t >+++ b/t/db_dependent/ILSDI_Services.t >@@ -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; > }; >-- >2.43.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 34893
:
156121
|
156123
|
159311
|
159314
|
159315
|
159356
|
159441
|
159506
|
159507
|
159508
|
159509
|
159797
|
159798
|
159799
|
159800
|
159893
|
161097
|
161098
|
161099
|
161100
|
161101
|
161324
|
163964