From 9f9bc2299e11ff81ee64305559132c5e4278f340 Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Wed, 18 Sep 2024 08:39:48 +0000 Subject: [PATCH] Bug 28907: [With corrected bug id] REST - Remove allow-owner from public password route To test: 1. prove t/db_dependent/api/v1/patrons_password.t 2. Apply patch 3. prove t/db_dependent/api/v1/patrons_password.t Observe success in both cases. https://bugs.koha-community.org/show_bug.cgi?id=28907 Signed-off-by: Victor Grousset/tuxayo Signed-off-by: Marcel de Rooy --- Koha/REST/V1/Patrons/Password.pm | 39 +++++++++++------------- api/v1/swagger/paths/public_patrons.yaml | 2 -- t/db_dependent/api/v1/patrons_password.t | 23 ++++++++++---- 3 files changed, 35 insertions(+), 29 deletions(-) diff --git a/Koha/REST/V1/Patrons/Password.pm b/Koha/REST/V1/Patrons/Password.pm index 1c117d0d64..a6c69d4fe7 100644 --- a/Koha/REST/V1/Patrons/Password.pm +++ b/Koha/REST/V1/Patrons/Password.pm @@ -90,29 +90,24 @@ sub set_public { my $user = $c->stash('koha.user'); - unless ( $user->borrowernumber == $patron_id ) { - return $c->render( - status => 403, - openapi => { error => "Changing other patron's password is forbidden" } - ); - } + return try { + $c->auth->public($patron_id); - unless ( $user->category->effective_change_password ) { - return $c->render( - status => 403, - openapi => { error => "Changing password is forbidden" } - ); - } + unless ( $user->category->effective_change_password ) { + return $c->render( + status => 403, + openapi => { error => "Changing password is forbidden" } + ); + } - my $old_password = $body->{old_password}; - my $password = $body->{password}; - my $password_2 = $body->{password_repeated}; + my $old_password = $body->{old_password}; + my $password = $body->{password}; + my $password_2 = $body->{password_repeated}; - unless ( $password eq $password_2 ) { - return $c->render( status => 400, openapi => { error => "Passwords don't match" } ); - } + unless ( $password eq $password_2 ) { + return $c->render( status => 400, openapi => { error => "Passwords don't match" } ); + } - return try { unless ( checkpw_internal( $user->userid, $old_password ) ) { return $c->render( status => 400, @@ -130,9 +125,11 @@ sub set_public { status => 400, openapi => { error => "$_" } ); + } elsif ( blessed $_ and $_->isa('Koha::Exceptions::REST::Public::Unauthorized') ) { + return $c->render( status => 403, json => { error => "Changing other patron's password is forbidden" } ); + } else { + $c->unhandled_exception($_); } - - $c->unhandled_exception($_); }; } diff --git a/api/v1/swagger/paths/public_patrons.yaml b/api/v1/swagger/paths/public_patrons.yaml index 2f2526e827..9a987ee0ed 100644 --- a/api/v1/swagger/paths/public_patrons.yaml +++ b/api/v1/swagger/paths/public_patrons.yaml @@ -60,8 +60,6 @@ description: Under maintenance schema: $ref: "../swagger.yaml#/definitions/error" - x-koha-authorization: - allow-owner: true "/public/patrons/{patron_id}/checkouts": post: x-mojo-to: Checkouts#add diff --git a/t/db_dependent/api/v1/patrons_password.t b/t/db_dependent/api/v1/patrons_password.t index 1551a36150..e9886932bf 100755 --- a/t/db_dependent/api/v1/patrons_password.t +++ b/t/db_dependent/api/v1/patrons_password.t @@ -102,7 +102,7 @@ subtest 'set() (authorized user tests)' => sub { subtest 'set_public() (unprivileged user tests)' => sub { - plan tests => 18; + plan tests => 21; $schema->storage->txn_begin; @@ -141,19 +141,30 @@ subtest 'set_public() (unprivileged user tests)' => sub { } )->status_is(403)->json_is( { error => 'Changing password is forbidden' } ); - t::lib::Mocks::mock_preference( 'OpacPasswordChange', 1 ); - $t->post_ok( - "//$userid:$password@/api/v1/public/patrons/" . $other_patron->id . "/password" => json => { + "/api/v1/public/patrons/" . $patron->id . "/password" => json => { password => $new_password, password_repeated => $new_password, old_password => $password } - )->status_is(403)->json_is( + )->status_is(401)->json_is( '/error', - "Authorization failure. Missing required permission(s)." + "Authentication failure." ); + + t::lib::Mocks::mock_preference( 'OpacPasswordChange', 1 ); + + $t->post_ok( + "//$userid:$password@/api/v1/public/patrons/" . $other_patron->id . "/password" => json => { + password => $new_password, + password_repeated => $new_password, + old_password => $password + } + )->status_is(403) + ->json_is( '/error', + "Changing other patron's password is forbidden" ); + $t->post_ok( "//$userid:$password@/api/v1/public/patrons/" . $patron->id . "/password" => json => { password => $new_password, -- 2.48.1