Attachment 56720 Details for Bug 17479 – Bug 17479: Store information on owner access into $c->stash

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

Bug-17479-Store-information-on-owner-access-into-c.patch (text/plain), 6.33 KB, created by Lari Taskula on 2016-10-21 10:41:07 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Lari Taskula

Created: 2016-10-21 10:41:07 UTC

Size: 6.33 KB

patch

obsolete

>From e0372b9580df5228acbeaedd14d3496396a73149 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] 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                   |  10 +++-
> t/db_dependent/api/v1/ownerflag.t | 117 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 125 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 d62ee23..720a10a 100644
>--- a/Koha/REST/V1.pm
>+++ b/Koha/REST/V1.pm
>@@ -99,8 +99,14 @@ sub authenticate_api_request {
>     my $authorization = $action_spec->{'x-koha-authorization'};
>     my $permissions = $authorization->{'permissions'};
>     return $next->($c) if C4::Auth::haspermission($user->userid, $permissions);
>-    return $next->($c) if allow_owner($c, $authorization, $user);
>-    return $next->($c) if allow_guarantor($c, $authorization, $user);
>+    if (allow_owner($c, $authorization, $user)) {
>+        $c->stash('is_owner_access', 1);
>+        return $next->($c);
>+    }
>+    elsif (allow_guarantor($c, $authorization, $user)) {
>+        $c->stash('is_guarantor_access', 1);
>+        return $next->($c);
>+    }
>     return $c->render_swagger(
>         { error => "Authorization failure. Missing required permission(s).",
>           required_permissions => $permissions },
>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

Actions: View | Diff | Splinter Review

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