From 400bc60ffbfd39df2e775c33a3bd6fabe5c417b1 Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Wed, 18 Sep 2024 13:35:06 +0000 Subject: [PATCH] Bug 28907: REST - Drop support for allow-owner functionality ...and allow-guarantor functionality. Replaced by $c->auth->public($patron_id) and/or $c->auth->public_guarantor($patron_id), where $patron_id is the patron's id that owns the requested resource. Old method, was applicable to both privileged and public routes: api/v1/swagger/paths/route.yaml x-koha-authorization: allow-owner: true allow-guarantor: true New method, use public routes with no x-koha-authorization: GET /public/route/{patron_id} Koha/REST/V1/Controller#public_action: sub public_action { my $c = shift->openapi->valid_input or return; my $patron_id = $c->param( 'patron_id' ); try { # Throws an exception that will render a response of 401 if not # authenticated and 403 if trying to access another user's resources $c->auth->public($patron_id); #or $c->auth->public_guarantor($patron_id) ... # other code ... } catch { $c->unhandled_exception($_); } } Another example of retrieving $patron_id when patron_id is not a request parameter: GET /public/another/object/{another_object_id} my $patron_id = Another::Object->find($another_object_id)->borrowernumber; try { # 403 if $another_object_id does not belong to API user $c->auth->public($patron_id); ... --- Koha/REST/V1/Auth.pm | 156 +------------------------------------------ 1 file changed, 1 insertion(+), 155 deletions(-) diff --git a/Koha/REST/V1/Auth.pm b/Koha/REST/V1/Auth.pm index c10e81cf8a..b63d4e0140 100644 --- a/Koha/REST/V1/Auth.pm +++ b/Koha/REST/V1/Auth.pm @@ -319,9 +319,7 @@ sub authenticate_api_request { my $permissions = $authorization->{'permissions'}; # Check if the user is authorized - if ( ( defined($permissions) and haspermission($user->userid, $permissions) ) - or allow_owner($c, $authorization, $user) - or allow_guarantor($c, $authorization, $user) ) { + if ( ( defined($permissions) and haspermission($user->userid, $permissions) ) ) { validate_query_parameters( $c, $spec ); @@ -360,158 +358,6 @@ sub validate_query_parameters { ) if @errors; } -=head3 allow_owner - -Allows access to object for its owner. - -There are endpoints that should allow access for the object owner even if they -do not have the required permission, e.g. access an own reserve. This can be -achieved by defining the operation as follows: - -"/holds/{reserve_id}": { - "get": { - ..., - "x-koha-authorization": { - "allow-owner": true, - "permissions": { - "borrowers": "1" - } - } - } -} - -=cut - -sub allow_owner { - my ($c, $authorization, $user) = @_; - - return unless $authorization->{'allow-owner'}; - - return check_object_ownership($c, $user) if $user and $c; -} - -=head3 allow_guarantor - -Same as "allow_owner", but checks if the object is owned by one of C<$user>'s -guarantees. - -=cut - -sub allow_guarantor { - my ($c, $authorization, $user) = @_; - - if (!$c || !$user || !$authorization || !$authorization->{'allow-guarantor'}){ - return; - } - - my $guarantees = $user->guarantee_relationships->guarantees->as_list; - foreach my $guarantee (@{$guarantees}) { - return 1 if check_object_ownership($c, $guarantee); - } -} - -=head3 check_object_ownership - -Determines ownership of an object from request parameters. - -As introducing an endpoint that allows access for object's owner; if the -parameter that will be used to determine ownership is not already inside -$parameters, add a new subroutine that checks the ownership and extend -$parameters to contain a key with parameter_name and a value of a subref to -the subroutine that you created. - -=cut - -sub check_object_ownership { - my ($c, $user) = @_; - - return if not $c or not $user; - - my $parameters = { - accountlines_id => \&_object_ownership_by_accountlines_id, - borrowernumber => \&_object_ownership_by_patron_id, - patron_id => \&_object_ownership_by_patron_id, - checkout_id => \&_object_ownership_by_checkout_id, - reserve_id => \&_object_ownership_by_reserve_id, - }; - - foreach my $param ( keys %{ $parameters } ) { - my $check_ownership = $parameters->{$param}; - if ($c->stash($param)) { - return &$check_ownership($c, $user, $c->stash($param)); - } - elsif ($c->param($param)) { - return &$check_ownership($c, $user, $c->param($param)); - } - elsif ($c->match->stack->[-1]->{$param}) { - return &$check_ownership($c, $user, $c->match->stack->[-1]->{$param}); - } - elsif ($c->req->json && $c->req->json->{$param}) { - return 1 if &$check_ownership($c, $user, $c->req->json->{$param}); - } - } -} - -=head3 _object_ownership_by_accountlines_id - -Finds a Koha::Account::Line-object by C<$accountlines_id> and checks if it -belongs to C<$user>. - -=cut - -sub _object_ownership_by_accountlines_id { - my ($c, $user, $accountlines_id) = @_; - - my $accountline = Koha::Account::Lines->find($accountlines_id); - return $accountline && $user->borrowernumber == $accountline->borrowernumber; -} - -=head3 _object_ownership_by_borrowernumber - -Compares C<$borrowernumber> to currently logged in C<$user>. - -=cut - -sub _object_ownership_by_patron_id { - my ($c, $user, $patron_id) = @_; - - return $user->borrowernumber == $patron_id; -} - -=head3 _object_ownership_by_checkout_id - -First, attempts to find a Koha::Checkout-object by C<$issue_id>. If we find one, -compare its borrowernumber to currently logged in C<$user>. However, if an issue -is not found, attempt to find a Koha::Old::Checkout-object instead and compare its -borrowernumber to currently logged in C<$user>. - -=cut - -sub _object_ownership_by_checkout_id { - my ($c, $user, $issue_id) = @_; - - my $issue = Koha::Checkouts->find($issue_id); - $issue = Koha::Old::Checkouts->find($issue_id) unless $issue; - return $issue && $issue->borrowernumber - && $user->borrowernumber == $issue->borrowernumber; -} - -=head3 _object_ownership_by_reserve_id - -Finds a Koha::Hold-object by C<$reserve_id> and checks if it -belongs to C<$user>. - -TODO: Also compare against old_reserves - -=cut - -sub _object_ownership_by_reserve_id { - my ($c, $user, $reserve_id) = @_; - - my $reserve = Koha::Holds->find($reserve_id); - return $reserve && $user->borrowernumber == $reserve->borrowernumber; -} - =head3 _basic_auth Internal method that performs Basic authentication. -- 2.34.1