From 3732e782bdda635e452ffca7d75daa1ff0de4fc8 Mon Sep 17 00:00:00 2001
From: Lari Taskula <lari.taskula@hypernova.fi>
Date: Wed, 18 Sep 2024 08:39:48 +0000
Subject: [PATCH] [24.05.x] Bug 28097: 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 <victor@tuxayo.net>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
---
 Koha/REST/V1/Patrons/Password.pm         | 50 ++++++++++--------------
 api/v1/swagger/paths/public_patrons.yaml |  2 -
 t/db_dependent/api/v1/patrons_password.t | 16 +++++++-
 3 files changed, 35 insertions(+), 33 deletions(-)

diff --git a/Koha/REST/V1/Patrons/Password.pm b/Koha/REST/V1/Patrons/Password.pm
index 77614ead5b..24522a3dc0 100644
--- a/Koha/REST/V1/Patrons/Password.pm
+++ b/Koha/REST/V1/Patrons/Password.pm
@@ -91,34 +91,25 @@ 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 ) ) {
+        unless ( checkpw_internal( $user->userid, $old_password ) ) {
             return $c->render(
                 status  => 400,
                 openapi => { error => "Invalid password" }
@@ -126,19 +117,20 @@ sub set_public {
         }
 
         ## Change password
-        $user->set_password({ password => $password });
+        $user->set_password( { password => $password } );
 
         return $c->render( status => 200, openapi => "" );
-    }
-    catch {
+    } catch {
         if ( blessed $_ and $_->isa('Koha::Exceptions::Password') ) {
             return $c->render(
                 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 8f79a4b81b..75e6669a0a 100755
--- a/t/db_dependent/api/v1/patrons_password.t
+++ b/t/db_dependent/api/v1/patrons_password.t
@@ -117,7 +117,7 @@ subtest 'set() (authorized user tests)' => sub {
 
 subtest 'set_public() (unprivileged user tests)' => sub {
 
-    plan tests => 18;
+    plan tests => 21;
 
     $schema->storage->txn_begin;
 
@@ -162,6 +162,18 @@ subtest 'set_public() (unprivileged user tests)' => sub {
         }
     );
 
+    $t->post_ok(
+        "/api/v1/public/patrons/" . $patron->id . "/password" => json => {
+            password          => $new_password,
+            password_repeated => $new_password,
+            old_password      => $password
+        }
+    )->status_is(401)->json_is(
+        '/error',
+        "Authentication failure."
+    );
+
+
     t::lib::Mocks::mock_preference( 'OpacPasswordChange', 1 );
 
     $t->post_ok(
@@ -174,7 +186,7 @@ subtest 'set_public() (unprivileged user tests)' => sub {
           }
     )->status_is(403)
       ->json_is( '/error',
-        "Authorization failure. Missing required permission(s)." );
+        "Changing other patron's password is forbidden" );
 
     $t->post_ok(
             "//$userid:$password@/api/v1/public/patrons/"
-- 
2.34.1