Attachment 161707 Details for Bug 35518 – Bug 35518: Check authentication and set userenv before fetching userenv variables

[patch] Bug 35518: Check authentication and set userenv before fetching userenv variables

Bug-35518-Check-authentication-and-set-userenv-bef.patch (text/plain), 5.22 KB, created by David Nind on 2024-01-31 23:07:16 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: David Nind

Created: 2024-01-31 23:07:16 UTC

Size: 5.22 KB

patch

obsolete

>From db27433fa05e09a0c5a6a596658b796983ea4bb9 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>
>---
> circ/circulation.pl | 79 +++++++++++++++++++++++----------------------
> 1 file changed, 41 insertions(+), 38 deletions(-)
>
>diff --git a/circ/circulation.pl b/circ/circulation.pl
>index da5e2b566a..f81102aec1 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.30.2

Actions: View | Diff | Splinter Review

Attachments on bug 35518: 161539 | 161707 | 161929 | 161930 | 162216 | 162218