Bugzilla – Attachment 167279 Details for
Bug 36575
Wrong patron can be returned for API validation route
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 36575: (QA follow-up)
Bug-36575-QA-follow-up.patch (text/plain), 7.61 KB, created by
Tomás Cohen Arazi (tcohen)
on 2024-05-30 16:54:45 UTC
(
hide
)
Description:
Bug 36575: (QA follow-up)
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2024-05-30 16:54:45 UTC
Size:
7.61 KB
patch
obsolete
>From e67519d6e5912df8ef1f16e7d4231d08f5b9d82d Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@ptfs-europe.com> >Date: Thu, 11 Apr 2024 12:18:30 +0200 >Subject: [PATCH] Bug 36575: (QA follow-up) > >Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> > >Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> > >Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> >--- > 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 bf8ccd35c80..3f23d848e48 100644 >--- a/C4/Auth.pm >+++ b/C4/Auth.pm >@@ -2007,8 +2007,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 { >@@ -2040,22 +2039,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 bb485ac7ab6..e41668605f4 100755 >--- a/t/db_dependent/Auth.t >+++ b/t/db_dependent/Auth.t >@@ -1570,23 +1570,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.45.1
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 36575
:
164668
|
164670
|
164671
|
165133
|
165134
|
165467
|
165468
|
165469
|
165470
|
165471
|
165908
|
166493
|
166494
|
166495
|
166496
|
166497
|
166498
|
167276
|
167277
|
167278
| 167279 |
167280
|
167281
|
167282
|
167283