From a68f6043f92f5a5fbb0cbdc90993adeff319d728 Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Tue, 2 Aug 2016 14:56:18 +0300 Subject: [PATCH] Bug 13895: Allow user to access their own checkouts and renew them Let user access their own checkouts and if OpacRenewalAllowed system preference is on, also let user to renew their checkouts. Test plan: 1. Open a browser tab on Koha staff and log in (to create CGISESSID cookie) with a Patron that has no permissions. This Patron will be referred as "your patron" or "your borrowernumber" below. 2. Go to http://yourlibrary/api/v1/checkouts?borrowernumber=XXX (replace XXX with your borrowernumber) and check you receive correct data 3. Go to http://yourlibrary/api/v1/checkouts?borrowernumber=XXX (replace XXX with someone else's borrowernumber) and check you get a permission error. 4. Go to http://yourlibrary/api/v1/checkouts/YYY (replace YYY with an existing checkout id of your Patron) and check you receive correct data 5. Go to http://yourlibrary/api/v1/checkouts/YYY (replace YYY with an existing checkout id of some other Patron) and check you get a permission error. 6. Send PUT request to http://yourlibrary/api/v1/checkouts/YYY, replace YYY with existing checkout id of some other Patron. You should get a permission error. 7. Set system preference OpacRenewalAllowed to 0. 8. Send PUT request to http://yourlibrary/api/v1/checkouts/YYY. YYY should be checkout id of checkout for your patron. You should get a permission error. 9. Set system preference OpacRenewalAllowed to 1. 10. Send PUT requests to http://yourlibrary/api/v1/checkouts/YYY until the maximum number of renewals is reached (you should have a 403 error). YYY should be checkout id of checkout for your patron. 11. Run unit tests in t/db_dependent/api/v1/checkouts.t Signed-off-by: Jiri Kozlovsky Wow, works like magic! :) Nice job! --- Koha/REST/V1/Checkout.pm | 27 ++++++++++++---- t/db_dependent/api/v1/checkouts.t | 67 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 85 insertions(+), 9 deletions(-) diff --git a/Koha/REST/V1/Checkout.pm b/Koha/REST/V1/Checkout.pm index 530e491..fb197dd 100644 --- a/Koha/REST/V1/Checkout.pm +++ b/Koha/REST/V1/Checkout.pm @@ -20,6 +20,7 @@ use Modern::Perl; use Mojo::Base 'Mojolicious::Controller'; use C4::Auth qw( haspermission ); +use C4::Context; use C4::Circulation; use Koha::Issues; @@ -27,7 +28,8 @@ sub list { my ($c, $args, $cb) = @_; my $user = $c->stash('koha.user'); - unless ($user && haspermission($user->userid, { circulate => "circulate_remaining_permissions" })) { + unless ($user && ($user->borrowernumber == $c->param('borrowernumber') + || haspermission($user->userid, { circulate => "circulate_remaining_permissions" }))) { return $c->$cb({error => "You don't have the required permission"}, 403); } @@ -43,9 +45,6 @@ sub get { my ($c, $args, $cb) = @_; my $user = $c->stash('koha.user'); - unless ($user && haspermission($user->userid, { circulate => "circulate_remaining_permissions" })) { - return $c->$cb({error => "You don't have the required permission"}, 403); - } my $checkout_id = $args->{checkout_id}; my $checkout = Koha::Issues->find($checkout_id); @@ -56,6 +55,13 @@ sub get { }, 404); } + my $borrowernumber = $checkout->borrowernumber; + + unless ($user && ( $user->borrowernumber == $borrowernumber + || haspermission($user->userid, { circulate => "circulate_remaining_permissions" }))) { + return $c->$cb({error => "You don't have the required permission"}, 403); + } + return $c->$cb($checkout->unblessed, 200); } @@ -63,9 +69,6 @@ sub renew { my ($c, $args, $cb) = @_; my $user = $c->stash('koha.user'); - unless ($user && haspermission($user->userid, { circulate => "circulate_remaining_permissions" })) { - return $c->$cb({error => "You don't have the required permission"}, 403); - } my $checkout_id = $args->{checkout_id}; my $checkout = Koha::Issues->find($checkout_id); @@ -79,6 +82,16 @@ sub renew { my $borrowernumber = $checkout->borrowernumber; my $itemnumber = $checkout->itemnumber; + my $OpacRenewalAllowed; + if ($user->borrowernumber == $borrowernumber) { + $OpacRenewalAllowed = C4::Context->preference('OpacRenewalAllowed'); + } + + unless ($user && ($OpacRenewalAllowed + || haspermission($user->userid, { circulate => "circulate_remaining_permissions" }))) { + return $c->$cb({error => "You don't have the required permission"}, 403); + } + my ($can_renew, $error) = C4::Circulation::CanBookBeRenewed( $borrowernumber, $itemnumber); diff --git a/t/db_dependent/api/v1/checkouts.t b/t/db_dependent/api/v1/checkouts.t index 4dbf62a..4db5a8f 100644 --- a/t/db_dependent/api/v1/checkouts.t +++ b/t/db_dependent/api/v1/checkouts.t @@ -17,9 +17,10 @@ use Modern::Perl; -use Test::More tests => 27; +use Test::More tests => 57; use Test::MockModule; use Test::Mojo; +use t::lib::Mocks; use t::lib::TestBuilder; use DateTime; @@ -59,8 +60,14 @@ $session->param('ip', '127.0.0.1'); $session->param('lasttime', time()); $session->flush; -my $borrower = $builder->build({ source => 'Borrower' }); +my $borrower = $builder->build({ source => 'Borrower', value => { flags => 0 } }); my $borrowernumber = $borrower->{borrowernumber}; +my $borrower_session = C4::Auth::get_session(''); +$borrower_session->param('number', $borrowernumber); +$borrower_session->param('id', $borrower->{ userid }); +$borrower_session->param('ip', '127.0.0.1'); +$borrower_session->param('lasttime', time()); +$borrower_session->flush; my $branchcode = $builder->build({ source => 'Branch' })->{ branchcode }; my $module = new Test::MockModule('C4::Context'); @@ -82,12 +89,15 @@ $t->request_ok($tx) my $biblionumber = create_biblio('RESTful Web APIs'); my $itemnumber1 = create_item($biblionumber, 'TEST000001'); my $itemnumber2 = create_item($biblionumber, 'TEST000002'); +my $itemnumber3 = create_item($biblionumber, 'TEST000003'); my $date_due = DateTime->now->add(weeks => 2); my $issue1 = C4::Circulation::AddIssue($borrower, 'TEST000001', $date_due); my $date_due1 = Koha::DateUtils::dt_from_string( $issue1->date_due ); my $issue2 = C4::Circulation::AddIssue($borrower, 'TEST000002', $date_due); my $date_due2 = Koha::DateUtils::dt_from_string( $issue2->date_due ); +my $issue3 = C4::Circulation::AddIssue($loggedinuser, 'TEST000003', $date_due); +my $date_due3 = Koha::DateUtils::dt_from_string( $issue3->date_due ); $tx = $t->ua->build_tx(GET => "/api/v1/checkouts?borrowernumber=$borrowernumber"); $tx->req->cookies({name => 'CGISESSID', value => $session->id}); @@ -101,6 +111,39 @@ $t->request_ok($tx) ->json_is('/1/date_due' => $date_due2->ymd . ' ' . $date_due2->hms) ->json_hasnt('/2'); +$tx = $t->ua->build_tx(GET => "/api/v1/checkouts/".$issue3->issue_id); +$tx->req->cookies({name => 'CGISESSID', value => $borrower_session->id}); +$t->request_ok($tx) + ->status_is(403) + ->json_is({ error => "You don't have the required permission" }); + +$tx = $t->ua->build_tx(GET => "/api/v1/checkouts?borrowernumber=".$loggedinuser->{borrowernumber}); +$tx->req->cookies({name => 'CGISESSID', value => $borrower_session->id}); +$t->request_ok($tx) + ->status_is(403) + ->json_is({ error => "You don't have the required permission" }); + +$tx = $t->ua->build_tx(GET => "/api/v1/checkouts?borrowernumber=$borrowernumber"); +$tx->req->cookies({name => 'CGISESSID', value => $borrower_session->id}); +$t->request_ok($tx) + ->status_is(200) + ->json_is('/0/borrowernumber' => $borrowernumber) + ->json_is('/0/itemnumber' => $itemnumber1) + ->json_is('/0/date_due' => $date_due1->ymd . ' ' . $date_due1->hms) + ->json_is('/1/borrowernumber' => $borrowernumber) + ->json_is('/1/itemnumber' => $itemnumber2) + ->json_is('/1/date_due' => $date_due2->ymd . ' ' . $date_due2->hms) + ->json_hasnt('/2'); + +$tx = $t->ua->build_tx(GET => "/api/v1/checkouts/" . $issue1->issue_id); +$tx->req->cookies({name => 'CGISESSID', value => $borrower_session->id}); +$t->request_ok($tx) + ->status_is(200) + ->json_is('/borrowernumber' => $borrowernumber) + ->json_is('/itemnumber' => $itemnumber1) + ->json_is('/date_due' => $date_due1->ymd . ' ' . $date_due1->hms) + ->json_hasnt('/1'); + $tx = $t->ua->build_tx(GET => "/api/v1/checkouts/" . $issue1->issue_id); $tx->req->cookies({name => 'CGISESSID', value => $session->id}); $t->request_ok($tx) @@ -127,6 +170,26 @@ $t->request_ok($tx) ->status_is(200) ->json_is('/date_due' => $expected_datedue->ymd . ' ' . $expected_datedue->hms); +$tx = $t->ua->build_tx(PUT => "/api/v1/checkouts/" . $issue3->issue_id); +$tx->req->cookies({name => 'CGISESSID', value => $borrower_session->id}); +$t->request_ok($tx) + ->status_is(403) + ->json_is({ error => "You don't have the required permission" }); + +t::lib::Mocks::mock_preference( "OpacRenewalAllowed", 0 ); +$tx = $t->ua->build_tx(PUT => "/api/v1/checkouts/" . $issue2->issue_id); +$tx->req->cookies({name => 'CGISESSID', value => $borrower_session->id}); +$t->request_ok($tx) + ->status_is(403) + ->json_is({ error => "You don't have the required permission" }); + +t::lib::Mocks::mock_preference( "OpacRenewalAllowed", 1 ); +$tx = $t->ua->build_tx(PUT => "/api/v1/checkouts/" . $issue2->issue_id); +$tx->req->cookies({name => 'CGISESSID', value => $borrower_session->id}); +$t->request_ok($tx) + ->status_is(200) + ->json_is('/date_due' => $expected_datedue->ymd . ' ' . $expected_datedue->hms); + $tx = $t->ua->build_tx(PUT => "/api/v1/checkouts/" . $issue1->issue_id); $tx->req->cookies({name => 'CGISESSID', value => $session->id}); $t->request_ok($tx) -- 2.1.4