From 68ed58d6fee5e86ed7ea7ae293170556deb9c1b0 Mon Sep 17 00:00:00 2001
From: Lari Taskula <lari.taskula@hypernova.fi>
Date: Wed, 18 Sep 2024 08:46:46 +0000
Subject: [PATCH] Bug 28907: REST - Remove allow-owner from public guarantors
can see charges and checkouts
Content-Type: text/plain; charset=utf-8
To test:
1. prove t/db_dependent/api/v1/patrons.t
2. Apply patch
3. prove t/db_dependent/api/v1/patrons.t
Observe success in both cases.
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
---
Koha/REST/V1/Patrons.pm | 4 ++++
api/v1/swagger/paths/public_patrons.yaml | 4 ----
t/db_dependent/api/v1/patrons.t | 21 +++++++++++++++++++--
3 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/Koha/REST/V1/Patrons.pm b/Koha/REST/V1/Patrons.pm
index ca620109a0..555d3a171a 100644
--- a/Koha/REST/V1/Patrons.pm
+++ b/Koha/REST/V1/Patrons.pm
@@ -456,6 +456,8 @@ sub guarantors_can_see_charges {
my $c = shift->openapi->valid_input or return;
return try {
+ $c->auth->public( $c->param('patron_id') );
+
if ( C4::Context->preference('AllowPatronToSetFinesVisibilityForGuarantor') ) {
my $patron = $c->stash( 'koha.user' );
my $privacy_setting = ($c->req->json->{allowed}) ? 1 : 0;
@@ -492,6 +494,8 @@ sub guarantors_can_see_checkouts {
my $c = shift->openapi->valid_input or return;
return try {
+ $c->auth->public( $c->param('patron_id') );
+
if ( C4::Context->preference('AllowPatronToSetCheckoutsVisibilityForGuarantor') ) {
my $patron = $c->stash( 'koha.user' );
my $privacy_setting = ( $c->req->json->{allowed} ) ? 1 : 0;
diff --git a/api/v1/swagger/paths/public_patrons.yaml b/api/v1/swagger/paths/public_patrons.yaml
index 0d1e48c6ca..774533a445 100644
--- a/api/v1/swagger/paths/public_patrons.yaml
+++ b/api/v1/swagger/paths/public_patrons.yaml
@@ -182,8 +182,6 @@
description: Under maintenance
schema:
$ref: "../swagger.yaml#/definitions/error"
- x-koha-authorization:
- allow-owner: true
"/public/patrons/{patron_id}/guarantors/can_see_checkouts":
put:
x-mojo-to: Patrons#guarantors_can_see_checkouts
@@ -236,8 +234,6 @@
description: Under maintenance
schema:
$ref: "../swagger.yaml#/definitions/error"
- x-koha-authorization:
- allow-owner: true
"/public/patrons/{patron_id}/holds/{hold_id}":
delete:
x-mojo-to: Patrons::Holds#delete_public
diff --git a/t/db_dependent/api/v1/patrons.t b/t/db_dependent/api/v1/patrons.t
index b311611670..303f865cfe 100755
--- a/t/db_dependent/api/v1/patrons.t
+++ b/t/db_dependent/api/v1/patrons.t
@@ -1160,7 +1160,7 @@ subtest 'delete() tests' => sub {
subtest 'guarantors_can_see_charges() tests' => sub {
- plan tests => 11;
+ plan tests => 17;
t::lib::Mocks::mock_preference( 'RESTPublicAPI', 1 );
t::lib::Mocks::mock_preference( 'RESTBasicAuth', 1 );
@@ -1168,6 +1168,7 @@ subtest 'guarantors_can_see_charges() tests' => sub {
$schema->storage->txn_begin;
my $patron = $builder->build_object({ class => 'Koha::Patrons', value => { privacy_guarantor_fines => 0 } });
+ my $other_patron_id = $builder->build_object( { class => 'Koha::Patrons' } )->borrowernumber;
my $password = 'thePassword123';
$patron->set_password({ password => $password, skip_validation => 1 });
my $userid = $patron->userid;
@@ -1175,6 +1176,14 @@ subtest 'guarantors_can_see_charges() tests' => sub {
t::lib::Mocks::mock_preference( 'AllowPatronToSetFinesVisibilityForGuarantor', 0 );
+ $t->put_ok(
+ "/api/v1/public/patrons/$other_patron_id/guarantors/can_see_charges" => json => { allowed => Mojo::JSON->true }
+ )->status_is(401)->json_is( { error => "Authentication failure." } );
+
+ $t->put_ok( "//$userid:$password@/api/v1/public/patrons/$other_patron_id/guarantors/can_see_charges" => json =>
+ { allowed => Mojo::JSON->true } )->status_is(403)
+ ->json_is( { error => "Unprivileged user cannot access another user's resources" } );
+
$t->put_ok( "//$userid:$password@/api/v1/public/patrons/$patron_id/guarantors/can_see_charges" => json => { allowed => Mojo::JSON->true } )
->status_is( 403 )
->json_is( '/error', 'The current configuration doesn\'t allow the requested action.' );
@@ -1198,7 +1207,7 @@ subtest 'guarantors_can_see_charges() tests' => sub {
subtest 'guarantors_can_see_checkouts() tests' => sub {
- plan tests => 11;
+ plan tests => 17;
t::lib::Mocks::mock_preference( 'RESTPublicAPI', 1 );
t::lib::Mocks::mock_preference( 'RESTBasicAuth', 1 );
@@ -1206,11 +1215,19 @@ subtest 'guarantors_can_see_checkouts() tests' => sub {
$schema->storage->txn_begin;
my $patron = $builder->build_object({ class => 'Koha::Patrons', value => { privacy_guarantor_checkouts => 0 } });
+ my $other_patron_id = $builder->build_object( { class => 'Koha::Patrons' } )->borrowernumber;
my $password = 'thePassword123';
$patron->set_password({ password => $password, skip_validation => 1 });
my $userid = $patron->userid;
my $patron_id = $patron->borrowernumber;
+ $t->put_ok( "/api/v1/public/patrons/$other_patron_id/guarantors/can_see_checkouts" => json =>
+ { allowed => Mojo::JSON->true } )->status_is(401)->json_is( { error => "Authentication failure." } );
+
+ $t->put_ok( "//$userid:$password@/api/v1/public/patrons/$other_patron_id/guarantors/can_see_checkouts" => json =>
+ { allowed => Mojo::JSON->true } )->status_is(403)
+ ->json_is( { error => "Unprivileged user cannot access another user's resources" } );
+
t::lib::Mocks::mock_preference( 'AllowPatronToSetCheckoutsVisibilityForGuarantor', 0 );
$t->put_ok( "//$userid:$password@/api/v1/public/patrons/$patron_id/guarantors/can_see_checkouts" => json => { allowed => Mojo::JSON->true } )
--
2.39.5