Attachment 61174 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.92 KB, created by Lari Taskula on 2017-03-16 15:37:14 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Lari Taskula

Created: 2017-03-16 15:37:14 UTC

Size: 6.92 KB

patch

obsolete

>From 052c83709b124385a7c767017309f06a0feb1682 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/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

Actions: View | Diff | Splinter Review

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