Bugzilla – Attachment 106844 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) Our PUT is really a PATCH
Bug-23634-QA-follow-up-Our-PUT-is-really-a-PATCH.patch (text/plain), 3.26 KB, created by
Tomás Cohen Arazi (tcohen)
on 2020-07-13 15:36:26 UTC
(
hide
)
Description:
Bug 23634: (QA follow-up) Our PUT is really a PATCH
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2020-07-13 15:36:26 UTC
Size:
3.26 KB
patch
obsolete
>From fe871f6483450fb528045324c88186df0ddd6b2e Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >Date: Mon, 13 Jul 2020 12:25:13 -0300 >Subject: [PATCH] Bug 23634: (QA follow-up) Our PUT is really a PATCH > >This patch makes the controller not expect that there will always be all >the email fields. So it now checks if an email field was passed, and >changed, and renders the error if that stands. > >To test: >1. Run: > $ kshell > k$ prove t/db_dependent/api/v1/patrons.t >=> FAIL: Tests written by Nick highlight a problem >2. Apply this patch >3. Repeat 1 >=> SUCCESS: Problems solved >4. Sign off :-D > >Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> >--- > Koha/REST/V1/Patrons.pm | 46 +++++++++++++++++++++++++---------------- > 1 file changed, 28 insertions(+), 18 deletions(-) > >diff --git a/Koha/REST/V1/Patrons.pm b/Koha/REST/V1/Patrons.pm >index a2fc2a6c8e2..04e7a31f640 100644 >--- a/Koha/REST/V1/Patrons.pm >+++ b/Koha/REST/V1/Patrons.pm >@@ -208,24 +208,34 @@ sub update { > my $body = $c->validation->param('body'); > 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 ($put_email ne $db_email) >- || ($put_email_pro ne $db_email_pro) >- || ($put_email_B ne $db_email_B); >+ if ( >+ $patron->is_superlibrarian >+ and !$user->is_superlibrarian >+ and ( exists $body->{email} >+ or exists $body->{secondary_email} >+ or exists $body->{altaddress_email} ) >+ ) >+ { >+ foreach my $email_field ( qw(email secondary_email altaddress_email) ) { >+ my $exists_email = exists $body->{$email_field}; >+ next unless $exists_email; >+ >+ # exists, verify if we are asked to change it >+ my $put_email = $body->{$email_field}; >+ # As of writing this patch, 'email' is the only unmapped field >+ # (i.e. it preserves its name, hence this fallback) >+ my $db_email_field = $patron->to_api_mapping->{$email_field} // 'email'; >+ my $db_email = $patron->$db_email_field; >+ >+ return $c->render( >+ status => 403, >+ openapi => { error => "Not enough privileges to change a superlibrarian's email" } >+ ) >+ unless ( !defined $put_email and !defined $db_email ) >+ or ( defined $put_email >+ and defined $db_email >+ and $put_email eq $db_email ); >+ } > } > > $patron->set_from_api($c->validation->param('body'))->store; >-- >2.27.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 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