Attachment 57286 Details for Bug 17479 – [SIGNED-OFF] Bug 17479: Store information on owner access into $c->stash

[patch] [SIGNED-OFF] Bug 17479: Store information on owner access into $c->stash

SIGNED-OFF-Bug-17479-Store-information-on-owner-ac.patch (text/plain), 6.46 KB, created by Josef Moravec on 2016-11-07 21:06:28 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Josef Moravec

Created: 2016-11-07 21:06:28 UTC

Size: 6.46 KB

patch

obsolete

>From 73068b03e4d36800c8906f995bcdbd470700058f Mon Sep 17 00:00:00 2001
>From: Lari Taskula <lari.taskula@jns.fi>
>Date: Fri, 21 Oct 2016 12:57:18 +0300
>Subject: [PATCH] [SIGNED-OFF] 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
>
>Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
>---
> 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;
>+}
>-- 
>2.1.4

Actions: View | Diff | Splinter Review

Attachments on bug 17479: 56720 | 57256 | 57286 | 61174