From fa78160a451b595204ee074bd794dd1739637840 Mon Sep 17 00:00:00 2001 From: Martin Renvoize 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. Signed-off-by: Tomas Cohen Arazi --- 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 591a170404c..a2fc2a6c8e2 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 a70fbc9625f..8a2295865bd 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.27.0