From ea8d655efbcb218c1c23c29ecb1d57e29f488741 Mon Sep 17 00:00:00 2001
From: Nick Clemens <nick@bywatersolutions.com>
Date: Fri, 26 Jan 2024 14:10:01 +0000
Subject: [PATCH] Bug 35518: Check authentication and set userenv before
 fetching userenv variables

Currently we get the userenv before we have set it correctly for the session

To test:
 1 - Sign in as a user with fast cataloging permission
 2 - Bring up a patron, type gibberish into barcode field to get a fast cataloging link
 3 - Check the link, it should have your current signed in barcode
 4 - Sign in to a different browser with a different user and at a different branch
 5 - Bring up a aptron in circulation and type gibberish into barcode field to get a fast cataloging link
 6 - It may have your branch, but it may also have the other user's branch from the other window
 7 - Keep entering gibberish to get a link until one user has the correct branch
 8 - Then switch to the other browser, and keep entering gibberish, watch the branchcode change
 9 - Apply patch, restart all
10 - Test switching between browsers. generating fast cataloging links
11 - Users should now consistently have the correct branch

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
---
 circ/circulation.pl | 79 +++++++++++++++++++++++----------------------
 1 file changed, 41 insertions(+), 38 deletions(-)

diff --git a/circ/circulation.pl b/circ/circulation.pl
index da5e2b566ae..f81102aec14 100755
--- a/circ/circulation.pl
+++ b/circ/circulation.pl
@@ -63,46 +63,10 @@ use List::MoreUtils qw( uniq );
 #
 my $query = CGI->new;
 
-my $override_high_holds     = $query->param('override_high_holds');
-my $override_high_holds_tmp = $query->param('override_high_holds_tmp');
-
-my $sessionID = $query->cookie("CGISESSID") ;
-my $session = get_session($sessionID);
-
-my $userenv = C4::Context->userenv;
-my $branch  = $userenv->{'branch'} // '';
-my $desk_id = $userenv->{"desk_id"} || '';
-
+my $borrowernumber = $query->param('borrowernumber');
 my $barcodes = [];
 my $barcode =  $query->param('barcode');
-my $findborrower;
-my $autoswitched;
-my $borrowernumber = $query->param('borrowernumber');
-
-if (C4::Context->preference("AutoSwitchPatron") && $barcode) {
-    my $new_barcode = $barcode;
-    Koha::Plugins->call( 'patron_barcode_transform', \$new_barcode );
-    if (Koha::Patrons->search( { cardnumber => $new_barcode} )->count() > 0) {
-        $findborrower = $barcode;
-        undef $barcode;
-        undef $borrowernumber;
-        $autoswitched = 1;
-    }
-}
-$findborrower ||= $query->param('findborrower') || q{};
-$findborrower =~ s|,| |g;
-
-if ( $query->param('confirm_hold') ) {
-    my $reserve_id          = $query->param('confirm_hold');
-    my $hold_branch         = $query->param('hold_branch');
-    my $hold_itemnumber     = $query->param('hold_itemnumber');
-    my $hold_borrowernumber = $query->param('hold_borrowernumber');
-    my $diffBranchSend      = ( $branch ne $hold_branch );
 
-    # diffBranchSend tells ModReserveAffect whether document is expected in this library or not,
-    # i.e., whether to apply waiting status
-    ModReserveAffect( $hold_itemnumber, $hold_borrowernumber, $diffBranchSend, $reserve_id, $desk_id );
-}
 
 # Barcode given by user could be '0'
 if ( $barcode || ( defined($barcode) && $barcode eq '0' ) ) {
@@ -121,7 +85,6 @@ if ( $barcode || ( defined($barcode) && $barcode eq '0' ) ) {
         @$barcodes = $query->multi_param('barcodes');
     }
 }
-
 $barcodes = [ uniq @$barcodes ];
 
 my $template_name = q|circ/circulation.tt|;
@@ -147,6 +110,46 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user (
         flagsrequired   => { circulate => 'circulate_remaining_permissions' },
     }
 );
+
+my $override_high_holds     = $query->param('override_high_holds');
+my $override_high_holds_tmp = $query->param('override_high_holds_tmp');
+
+my $sessionID = $query->cookie("CGISESSID") ;
+my $session = get_session($sessionID);
+
+my $userenv = C4::Context->userenv;
+my $branch  = $userenv->{'branch'} // '';
+my $desk_id = $userenv->{"desk_id"} || '';
+
+my $findborrower;
+my $autoswitched;
+
+if (C4::Context->preference("AutoSwitchPatron") && $barcode) {
+    my $new_barcode = $barcode;
+    Koha::Plugins->call( 'patron_barcode_transform', \$new_barcode );
+    if (Koha::Patrons->search( { cardnumber => $new_barcode} )->count() > 0) {
+        $findborrower = $barcode;
+        undef $barcode;
+        undef $borrowernumber;
+        $autoswitched = 1;
+    }
+}
+$findborrower ||= $query->param('findborrower') || q{};
+$findborrower =~ s|,| |g;
+
+if ( $query->param('confirm_hold') ) {
+    my $reserve_id          = $query->param('confirm_hold');
+    my $hold_branch         = $query->param('hold_branch');
+    my $hold_itemnumber     = $query->param('hold_itemnumber');
+    my $hold_borrowernumber = $query->param('hold_borrowernumber');
+    my $diffBranchSend      = ( $branch ne $hold_branch );
+
+    # diffBranchSend tells ModReserveAffect whether document is expected in this library or not,
+    # i.e., whether to apply waiting status
+    ModReserveAffect( $hold_itemnumber, $hold_borrowernumber, $diffBranchSend, $reserve_id, $desk_id );
+}
+
+
 my $logged_in_user = Koha::Patrons->find( $loggedinuser );
 
 my $force_allow_issue = $query->param('forceallow') || 0;
-- 
2.43.0