From ca1b065e61328b99456f24b1d216414c936770e4 Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Fri, 21 Oct 2016 12:57:18 +0300 Subject: [PATCH] Bug 17479: Store information on owner access into $c->stash There are two ways of accessing a resource via REST API; either: - you have the required permission - you do not have the permission but you are owner of the object, e.g. you want to GET your own patron information In many cases we want to perform additional operations if the user is accessing his own object. Usually this additional operation is checking a system preference. Example: Patron wants to update his own patron information via REST API. We have to check OPACPatronDetails system preference for this. If it is on, we should forward the changes for approval from a librarian. Currently, in controller, we can check this opac-like access by checking that the user does not have permissions and that the patron he is accessing is himself. This would require another haspermission() call. Instead, we could set a flag into $c->stash in Koha/REST/V1.pm in the case of ownership access. After this, in controller, we only need to check $c->stash for this flag. To test: 1. Apply patch 2. Run t/db_dependent/api/v1/ownerflag.t 3. Observe it pass --- Koha/REST/V1.pm | 9 ++- t/db_dependent/api/v1/ownerflag.t | 117 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+), 2 deletions(-) create mode 100644 t/db_dependent/api/v1/ownerflag.t diff --git a/Koha/REST/V1.pm b/Koha/REST/V1.pm index 76ae180..9e0664b 100644 --- a/Koha/REST/V1.pm +++ b/Koha/REST/V1.pm @@ -110,13 +110,18 @@ sub authenticate_api_request { my $permissions = $authorization->{'permissions'}; # Check if the user is authorized + my ($owner_access, $guarantor_access); if ( C4::Auth::haspermission($user->userid, $permissions) - or allow_owner($c, $authorization, $user) - or allow_guarantor($c, $authorization, $user) ) { + or $owner_access = allow_owner($c, $authorization, $user) + or $guarantor_access = allow_guarantor($c, $authorization, $user) ) { # Return the query errors if exist return $c->render_swagger({}, \@query_errors, 400) if @query_errors; + # Store information on owner/guarantor access + $c->stash('is_owner_access', 1) if $owner_access; + $c->stash('is_guarantor_access', 1) if $guarantor_access; + # Everything is ok return $next->($c) } diff --git a/t/db_dependent/api/v1/ownerflag.t b/t/db_dependent/api/v1/ownerflag.t new file mode 100644 index 0000000..c8d4c60 --- /dev/null +++ b/t/db_dependent/api/v1/ownerflag.t @@ -0,0 +1,117 @@ +#!/usr/bin/env perl + +# Copyright 2016 Koha-Suomi Oy +# +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it under the +# terms of the GNU General Public License as published by the Free Software +# Foundation; either version 3 of the License, or (at your option) any later +# version. +# +# Koha is distributed in the hope that it will be useful, but WITHOUT ANY +# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR +# A PARTICULAR PURPOSE. See the GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License along +# with Koha; if not, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +use Modern::Perl; + +use Test::More tests => 9; +use Test::Mojo; +use Test::MockModule; +use t::lib::TestBuilder; + +use C4::Auth; +use C4::Context; + +use Koha::Database; +use Koha::Patron; + +my $mock = Test::MockModule->new('Koha::REST::V1::Patron'); +$mock->mock(get => sub { + my ($c, $args, $cb) = @_; + + # return 404 because it has a generic response schema that is unlikely to change + return $c->$cb({ error => "is_owner_access" }, 404) if $c->stash('is_owner_access'); + return $c->$cb({ error => "is_guarantor_access" }, 404) if $c->stash('is_guarantor_access'); + return $c->$cb({ error => "librarian_access" }, 404); +}); +my $builder = t::lib::TestBuilder->new(); + +my $dbh = C4::Context->dbh; +$dbh->{AutoCommit} = 0; +$dbh->{RaiseError} = 1; + +$ENV{REMOTE_ADDR} = '127.0.0.1'; +my $t = Test::Mojo->new('Koha::REST::V1'); + +my $categorycode = $builder->build({ source => 'Category' })->{ categorycode }; +my $branchcode = $builder->build({ source => 'Branch' })->{ branchcode }; +my $guarantor = $builder->build({ + source => 'Borrower', + value => { + branchcode => $branchcode, + categorycode => $categorycode, + flags => 0, + } +}); +my $borrower = $builder->build({ + source => 'Borrower', + value => { + branchcode => $branchcode, + categorycode => $categorycode, + flags => 0, + guarantorid => $guarantor->{borrowernumber}, + } +}); +my $librarian = $builder->build({ + source => 'Borrower', + value => { + branchcode => $branchcode, + categorycode => $categorycode, + flags => 16, + } +}); + +my $session = create_session($borrower); +my $session2 = create_session($guarantor); +my $lib_session = create_session($librarian); + +# User without permissions, but is the owner of the object +my $tx = $t->ua->build_tx(GET => "/api/v1/patrons/" . $borrower->{borrowernumber}); +$tx->req->cookies({name => 'CGISESSID', value => $session->id}); +$t->request_ok($tx) + ->status_is(404) + ->json_is('/error', 'is_owner_access'); + +# User without permissions, but is the guarantor of the owner of the object +$tx = $t->ua->build_tx(GET => "/api/v1/patrons/" . $borrower->{borrowernumber}); +$tx->req->cookies({name => 'CGISESSID', value => $session2->id}); +$t->request_ok($tx) + ->status_is(404) + ->json_is('/error', 'is_guarantor_access'); + +# User with permissions +$tx = $t->ua->build_tx(GET => "/api/v1/patrons/" . $librarian->{borrowernumber}); +$tx->req->cookies({name => 'CGISESSID', value => $lib_session->id}); +$t->request_ok($tx) + ->status_is(404) + ->json_is('/error', 'librarian_access'); + +$dbh->rollback; + +sub create_session { + my ($patron) = @_; + + my $session = C4::Auth::get_session(''); + $session->param('number', $patron->{ borrowernumber }); + $session->param('id', $patron->{ userid }); + $session->param('ip', '127.0.0.1'); + $session->param('lasttime', time()); + $session->flush; + + return $session; +} -- 1.9.1