From f4939aae26bb0db99f6a47b6aa647ac5758f1e15 Mon Sep 17 00:00:00 2001 From: Lari Taskula 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 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. --- 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 0e3c73e21d..0d8742d974 100755 --- a/t/db_dependent/api/v1/patrons.t +++ b/t/db_dependent/api/v1/patrons.t @@ -1140,7 +1140,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 ); @@ -1148,6 +1148,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; @@ -1155,6 +1156,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.' ); @@ -1178,7 +1187,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 ); @@ -1186,11 +1195,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.34.1