From 052c83709b124385a7c767017309f06a0feb1682 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/Auth.pm | 9 ++- t/db_dependent/api/v1/ownerflag.t | 130 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 137 insertions(+), 2 deletions(-) create mode 100644 t/db_dependent/api/v1/ownerflag.t diff --git a/Koha/REST/V1/Auth.pm b/Koha/REST/V1/Auth.pm index 27727b8..b838b5c 100644 --- a/Koha/REST/V1/Auth.pm +++ b/Koha/REST/V1/Auth.pm @@ -149,12 +149,17 @@ sub authenticate_api_request { my $permissions = $authorization->{'permissions'}; # Check if the user is authorized + my ($owner_access, $guarantor_access); if ( 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) ) { validate_query_parameters( $c, $spec ); + # 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 1; } diff --git a/t/db_dependent/api/v1/ownerflag.t b/t/db_dependent/api/v1/ownerflag.t new file mode 100644 index 0000000..8aec82e --- /dev/null +++ b/t/db_dependent/api/v1/ownerflag.t @@ -0,0 +1,130 @@ +#!/usr/bin/env perl + +# Copyright 2017 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 => 3; +use Test::Mojo; +use t::lib::Mocks; +use t::lib::TestBuilder; + +use C4::Auth; +use C4::Context; + +use Koha::Database; +use Koha::Patrons; + +use Mojolicious::Lite; + +my $schema = Koha::Database->new->schema; +my $builder = t::lib::TestBuilder->new; + +# FIXME: sessionStorage defaults to mysql, but it seems to break transaction handling +# this affects the other REST api tests +t::lib::Mocks::mock_preference( 'SessionStorage', 'tmp' ); +my $mock = create_test_endpoint(); + +my $remote_address = '127.0.0.1'; +my $t = Test::Mojo->new('Koha::REST::V1'); + +$schema->storage->txn_begin; + +my ($patron, $session) = create_user_and_session(0); +my ($guarantor, $session2) = create_user_and_session(0); +Koha::Patrons->find($patron->{borrowernumber}) + ->guarantorid($guarantor->{borrowernumber}) + ->store; +my ($librarian, $lib_session) = create_user_and_session(16); + +subtest 'without permission, owner of object tests' => sub { + plan tests => 3; + + my $borrowernumber = $patron->{borrowernumber}; + my $tx = $t->ua->build_tx(GET => "/api/v1/patrons/$borrowernumber"); + $tx->req->cookies({name => 'CGISESSID', value => $session->id}); + $t->request_ok($tx) + ->status_is(418) + ->json_is('/error', 'is_owner_access'); +}; + + +subtest 'without permissions, guarantor of the owner of the object tests' => sub { + plan tests => 3; + + my $borrowernumber = $patron->{borrowernumber}; + my $tx = $t->ua->build_tx(GET => "/api/v1/patrons/$borrowernumber"); + $tx->req->cookies({name => 'CGISESSID', value => $session2->id}); + $t->request_ok($tx) + ->status_is(418) + ->json_is('/error', 'is_guarantor_access'); +}; + +subtest 'with permissions tests' => sub { + plan tests => 3; + + my $borrowernumber = $librarian->{borrowernumber}; + my $tx = $t->ua->build_tx(GET => "/api/v1/patrons/$borrowernumber"); + $tx->req->cookies({name => 'CGISESSID', value => $lib_session->id}); + $t->request_ok($tx) + ->status_is(418) + ->json_is('/error', 'librarian_access'); +}; + +$schema->storage->txn_rollback; + +sub create_test_endpoint { + # Mock Koha::REST::V1::Patron::get to read stash for owner access + my $mock = Test::MockModule->new('Koha::REST::V1::Patron'); + $mock->mock(get => sub { + my $c = shift; + if ($c->stash('is_owner_access')) { + return $c->render( status => 418, + json => { error => "is_owner_access" }); + } elsif ($c->stash('is_guarantor_access')) { + return $c->render( status => 418, + json => { error => "is_guarantor_access" }); + } + return $c->render( status => 418, + json => { error => "librarian_access" }); + }); + return $mock; +} + +sub create_user_and_session { + my ($flags) = @_; + + my $categorycode = $builder->build({ source => 'Category' })->{ categorycode }; + my $branchcode = $builder->build({ source => 'Branch' })->{ branchcode }; + my $patron = $builder->build({ + source => 'Borrower', + value => { + branchcode => $branchcode, + categorycode => $categorycode, + flags => $flags, + } + }); + 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 ($patron, $session); +} -- 1.9.1