Bug 643: QA Followup - server-side verification of permissions

Only allow $force_allow=1 if the logged in user has permissions.

TEST PLAN
---------
Attempt to intentionally override the checkout by passing an
appropriately handcrafted URL.
-- Regardless of the force_allow value, it should be not allowed
   for those lacking the force_checkout permission.

NOTE: I didn't test this. I figured Marc Veron could do that. :)
      (Sorry, couldn't easily get git bz to work with the accent)

Signed-off-by: Nick Clemens <nick@quecheelibrary.org>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

diff --git a/circ/circulation.pl b/circ/circulation.pl
index 131c8b0..c920787 100755 (executable)
--- a/circ/circulation.pl
+++ b/circ/circulation.pl
@@ -27,7 +27,7 @@ use warnings;
 use CGI;
 use C4::Output;
 use C4::Print;
-use C4::Auth qw/:DEFAULT get_session/;
+use C4::Auth qw/:DEFAULT get_session haspermission/;
 use C4::Dates qw/format_date/;
 use C4::Branch; # GetBranches
 use C4::Koha;   # GetPrinter
@@ -98,6 +98,9 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user (
 my $branches = GetBranches();
 
 my $force_allow_issue = $query->param('forceallow') || 0;
+if (!C4::Auth::haspermission( C4::Context->userenv->{id} , { circulate => 'force_checkout' } )) {
+    $force_allow_issue = 0;
+}
 
 my @failedrenews = $query->param('failedrenew');    # expected to be itemnumbers
 our %renew_failed = ();