Bugzilla – Attachment 106764 Details for
Bug 23634
Privilege escalation vulnerability for staff users with 'edit_borrowers' permission and 'OpacResetPassword' enabled
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 23634: (QA follow-up) Catch all email cases in API
Bug-23634-QA-follow-up-Catch-all-email-cases-in-AP.patch (text/plain), 5.10 KB, created by
Martin Renvoize (ashimema)
on 2020-07-10 09:27:07 UTC
(
hide
)
Description:
Bug 23634: (QA follow-up) Catch all email cases in API
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2020-07-10 09:27:07 UTC
Size:
5.10 KB
patch
obsolete
>From 10998828d2f6bd52144b161f592ad0a2f20fd57f Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@ptfs-europe.com> >Date: Fri, 10 Jul 2020 09:38:31 +0100 >Subject: [PATCH] Bug 23634: (QA follow-up) Catch all email cases in API > >The API was only catching the primary email change case, but we need to >catch email, emailpro and B_email. > >We were also not accounting for any of the emails (on PUT or from the >DB) being undefined. >--- > Koha/REST/V1/Patrons.pm | 17 +++++++++++++-- > t/db_dependent/api/v1/patrons.t | 37 ++++++++++++++++++++++++++++++++- > 2 files changed, 51 insertions(+), 3 deletions(-) > >diff --git a/Koha/REST/V1/Patrons.pm b/Koha/REST/V1/Patrons.pm >index 591a170404..a2fc2a6c8e 100644 >--- a/Koha/REST/V1/Patrons.pm >+++ b/Koha/REST/V1/Patrons.pm >@@ -209,10 +209,23 @@ sub update { > my $user = $c->stash('koha.user'); > > if ( $patron->is_superlibrarian and !$user->is_superlibrarian ) { >+ my $put_email = $body->{email} // qw{}; >+ my $db_email = $patron->email // qw{}; >+ my $put_email_pro = $body->{secondary_email} // qw{}; >+ my $db_email_pro = $patron->emailpro // qw{}; >+ my $put_email_B = $body->{altaddress_email} // qw{}; >+ my $db_email_B = $patron->B_email // qw{}; >+ > return $c->render( > status => 403, >- openapi => { error => "Not enough privileges to change a superlibrarian's email" } >- ) if $body->{email} ne $patron->email ; >+ openapi => { >+ error => >+ "Not enough privileges to change a superlibrarian's email" >+ } >+ ) >+ if ($put_email ne $db_email) >+ || ($put_email_pro ne $db_email_pro) >+ || ($put_email_B ne $db_email_B); > } > > $patron->set_from_api($c->validation->param('body'))->store; >diff --git a/t/db_dependent/api/v1/patrons.t b/t/db_dependent/api/v1/patrons.t >index a70fbc9625..8a2295865b 100644 >--- a/t/db_dependent/api/v1/patrons.t >+++ b/t/db_dependent/api/v1/patrons.t >@@ -222,7 +222,7 @@ subtest 'update() tests' => sub { > $schema->storage->txn_rollback; > > subtest 'librarian access tests' => sub { >- plan tests => 25; >+ plan tests => 40; > > $schema->storage->txn_begin; > >@@ -341,11 +341,46 @@ subtest 'update() tests' => sub { > $newpatron->{userid} = $superlibrarian->userid; > $newpatron->{email} = 'nosense@no.no'; > >+ # attempt to update > $authorized_patron->flags( 2**4 )->store; # borrowers flag = 4 > $t->put_ok( "//$userid:$password@/api/v1/patrons/" . $superlibrarian->borrowernumber => json => $newpatron ) > ->status_is(403, "Non-superlibrarian user change of superlibrarian email forbidden") > ->json_is( { error => "Not enough privileges to change a superlibrarian's email" } ); > >+ # attempt to unset >+ $newpatron->{email} = undef; >+ $t->put_ok( "//$userid:$password@/api/v1/patrons/" . $superlibrarian->borrowernumber => json => $newpatron ) >+ ->status_is(403, "Non-superlibrarian user change of superlibrarian email forbidden") >+ ->json_is( { error => "Not enough privileges to change a superlibrarian's email to undefined" } ); >+ >+ $newpatron->{email} = $superlibrarian->email; >+ $newpatron->{secondary_email} = 'nonsense@no.no'; >+ >+ # attempt to update >+ $t->put_ok( "//$userid:$password@/api/v1/patrons/" . $superlibrarian->borrowernumber => json => $newpatron ) >+ ->status_is(403, "Non-superlibrarian user change of superlibrarian secondary_email forbidden") >+ ->json_is( { error => "Not enough privileges to change a superlibrarian's secondary_email" } ); >+ >+ # attempt to unset >+ $newpatron->{secondary_email} = undef; >+ $t->put_ok( "//$userid:$password@/api/v1/patrons/" . $superlibrarian->borrowernumber => json => $newpatron ) >+ ->status_is(403, "Non-superlibrarian user change of superlibrarian secondary_email forbidden") >+ ->json_is( { error => "Not enough privileges to change a superlibrarian's secondary_email to undefined" } ); >+ >+ $newpatron->{secondary_email} = $superlibrarian->emailpro; >+ $newpatron->{altaddress_email} = 'nonsense@no.no'; >+ >+ # attempt to update >+ $t->put_ok( "//$userid:$password@/api/v1/patrons/" . $superlibrarian->borrowernumber => json => $newpatron ) >+ ->status_is(403, "Non-superlibrarian user change of superlibrarian altaddress_email forbidden") >+ ->json_is( { error => "Not enough privileges to change a superlibrarian's altaddress_email" } ); >+ >+ # attempt to unset >+ $newpatron->{altaddress_email} = undef; >+ $t->put_ok( "//$userid:$password@/api/v1/patrons/" . $superlibrarian->borrowernumber => json => $newpatron ) >+ ->status_is(403, "Non-superlibrarian user change of superlibrarian altaddress_email forbidden") >+ ->json_is( { error => "Not enough privileges to change a superlibrarian's altaddress_email to undefined" } ); >+ > $schema->storage->txn_rollback; > }; > }; >-- >2.20.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 23634
:
92932
|
92998
|
95563
|
95564
|
95574
|
95575
|
95576
|
95577
|
95582
|
97986
|
97987
|
97988
|
97989
|
97990
|
106159
|
106160
|
106161
|
106162
|
106163
|
106723
|
106724
|
106725
|
106726
|
106727
|
106764
|
106826
|
106837
|
106838
|
106839
|
106840
|
106841
|
106842
|
106843
|
106844
|
106979
|
106980
|
106981
|
106982
|
106983
|
106984
|
106985
|
106986
|
109582