Bug 27947: (QA follow-up) Clarify permissions
Privileged routes need permissions defined. This patch adds the minimum required permission until there are article request-specific permissions in Koha: circulate: circulate_remaining_permissions It is also clarified that interacting with an article request from another patron, but having your own patron_id in the path would return 404 instead of 403, as technically the resource (an article request from the patron, identified.by the supplied id) doesn't exist. Tests are tweaked. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This commit is contained in:
parent
77506a474a
commit
9c4a1b19aa
|
|
@ -66,7 +66,7 @@ sub cancel {
|
|||
};
|
||||
}
|
||||
|
||||
=head3 patron_cancel
|
||||
=head3 patron_cancel (public route)
|
||||
|
||||
Controller function that handles cancelling a patron's Koha::ArticleRequest object
|
||||
|
||||
|
|
@ -84,6 +84,8 @@ sub patron_cancel {
|
|||
);
|
||||
}
|
||||
|
||||
# patron_id has been validated by the allow-owner check, so the following call to related
|
||||
# article requests covers the case of article requests not belonging to the patron
|
||||
my $article_request = $patron->article_requests->find( $c->validation->param('article_request_id') );
|
||||
|
||||
unless ( $article_request ) {
|
||||
|
|
@ -109,4 +111,4 @@ sub patron_cancel {
|
|||
};
|
||||
}
|
||||
|
||||
1;
|
||||
1;
|
||||
|
|
|
|||
|
|
@ -51,6 +51,9 @@
|
|||
description: Under maintenance
|
||||
schema:
|
||||
$ref: ../definitions.json#/error
|
||||
x-koha-authorization:
|
||||
permissions:
|
||||
circulate: circulate_remaining_permissions
|
||||
"/public/patrons/{patron_id}/article_requests/{article_request_id}":
|
||||
delete:
|
||||
x-mojo-to: ArticleRequests#patron_cancel
|
||||
|
|
|
|||
|
|
@ -40,7 +40,7 @@ subtest 'cancel() tests' => sub {
|
|||
my $authorized_patron = $builder->build_object(
|
||||
{
|
||||
class => 'Koha::Patrons',
|
||||
value => { flags => 1 }
|
||||
value => { flags => 2 ** 1 } # circulate flag = 1
|
||||
}
|
||||
);
|
||||
my $password = 'thePassword123';
|
||||
|
|
@ -80,7 +80,7 @@ subtest 'cancel() tests' => sub {
|
|||
|
||||
subtest 'patron_cancel() tests' => sub {
|
||||
|
||||
plan tests => 12;
|
||||
plan tests => 14;
|
||||
|
||||
t::lib::Mocks::mock_preference( 'RESTPublicAPI', 1 );
|
||||
t::lib::Mocks::mock_preference( 'RESTBasicAuth', 1 );
|
||||
|
|
@ -90,7 +90,7 @@ subtest 'patron_cancel() tests' => sub {
|
|||
my $patron = $builder->build_object(
|
||||
{
|
||||
class => 'Koha::Patrons',
|
||||
value => { privacy_guarantor_checkouts => 0 }
|
||||
value => { flags => 0 }
|
||||
}
|
||||
);
|
||||
my $password = 'thePassword123';
|
||||
|
|
@ -102,16 +102,21 @@ subtest 'patron_cancel() tests' => sub {
|
|||
my $deleted_article_request_id = $deleted_article_request->id;
|
||||
$deleted_article_request->delete;
|
||||
|
||||
my $another_patron = $builder->build_object({ class => 'Koha::Patrons' });
|
||||
my $another_patron_id = $another_patron->id;
|
||||
|
||||
$t->delete_ok("//$userid:$password@/api/v1/public/patrons/$another_patron_id/article_requests/$deleted_article_request_id")
|
||||
->status_is(403);
|
||||
|
||||
# delete non existent article request
|
||||
$t->delete_ok("//$userid:$password@/api/v1/public/patrons/$patron_id/article_requests/$deleted_article_request_id")
|
||||
->status_is(404)
|
||||
->json_is( { error => "Article request not found" } );
|
||||
|
||||
my $another_patron = $builder->build_object({ class => 'Koha::Patrons' });
|
||||
my $another_patron_id = $another_patron->id;
|
||||
|
||||
my $article_request_2 = $builder->build_object({ class => 'Koha::ArticleRequests', value => { borrowernumber => $another_patron_id } });
|
||||
|
||||
# delete another patron's request
|
||||
$t->delete_ok("//$userid:$password@/api/v1/public/patrons/$another_patron_id/article_requests/" . $article_request_2->id)
|
||||
->status_is(403)
|
||||
->json_is( '/error' => 'Authorization failure. Missing required permission(s).' );
|
||||
|
||||
my $another_article_request = $builder->build_object(
|
||||
{
|
||||
class => 'Koha::ArticleRequests',
|
||||
|
|
@ -119,9 +124,9 @@ subtest 'patron_cancel() tests' => sub {
|
|||
}
|
||||
);
|
||||
|
||||
$t->delete_ok("//$userid:$password@/api/v1/public/patrons/$patron_id/article_requests/$another_article_request")
|
||||
->status_is(403);
|
||||
|
||||
$t->delete_ok("//$userid:$password@/api/v1/public/patrons/$patron_id/article_requests/" . $another_article_request->id)
|
||||
->status_is(404)
|
||||
->json_is( { error => 'Article request not found' } );
|
||||
|
||||
my $article_request = $builder->build_object(
|
||||
{
|
||||
|
|
|
|||
Loading…
Reference in New Issue