From bfd6b2a75219ee27cd852c267fbda6b29bc2d2d2 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Tue, 21 May 2024 12:44:25 +0000 Subject: [PATCH] Bug 36908: Sort branches based on branchcode This adds a sort based on branchcode, it's a fallback for an edge case that should be rare so I think is acceptable, as long as documented. System preference descriptions are updated as well --- C4/Auth.pm | 5 +++-- .../en/modules/admin/preferences/admin.pref | 6 +++--- t/db_dependent/Auth.t | 17 +++++++++++++---- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/C4/Auth.pm b/C4/Auth.pm index b313687b2f0..9adf500476e 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -1214,8 +1214,8 @@ sub checkauth { $register_id = $register->id if ($register); $register_name = $register->name if ($register); } - my $branches = { map { $_->branchcode => $_->unblessed } Koha::Libraries->search->as_list }; if ( $type ne 'opac' ) { + my $branches = { map { $_->branchcode => $_->unblessed } Koha::Libraries->search->as_list }; if ( C4::Context->preference('AutoLocation') ) { # we have to check they are coming from the right ip range my $domain = $branches->{$branchcode}->{'branchip'}; @@ -1242,7 +1242,8 @@ sub checkauth { || ( C4::Context->preference('AutoLocation') && $auth_state ne 'failed' ) ) { - foreach my $br ( uniq( $branchcode, keys %$branches ) ) { + my @branchcodes = sort { lc $a cmp lc $b } keys %$branches; + foreach my $br ( uniq( $branchcode, @branchcodes ) ) { # now we work with the treatment of ip my $domain = $branches->{$br}->{'branchip'}; diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref index 685486d6c10..164bd564528 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref @@ -91,14 +91,14 @@ Administration: class: integer - Adding d will specify it in days, e.g. 1d is timeout of one day. - - - "Require staff to log in from a computer in the IP address range specified by their library (if any): " + - "Require staff to log in from a computer in the IP address range specified by their library or the branch chosen at login (if any): " - pref: AutoLocation default: 0 choices: 1: "Yes" 0: "No" - Link to library administration - - 'This setting will override the StaffLoginBranchBasedOnIP system preference.' + - 'Staff can only choose thier branch on login if they have "loggedinlibrary" permission. This setting will override the StaffLoginBranchBasedOnIP system preference. In the event of multiple branches with matching IPs, branchcode (alphabetically) will be the tie breaker.' - - "Enable check for change in remote IP address for session security: " - pref: SessionRestrictionByIP @@ -115,7 +115,7 @@ Administration: 1: "Yes" 0: "No" - "Note: If IPs overlap, the first found match will be used." - - 'This setting will be overridden by AutoLocation system preference.' + - 'This setting will be overridden by AutoLocation system preference. In the event of multiple branches with matching IPs, branchcode (alphabetically) will be the tie breaker.' # PostgreSQL is supported by CGI::Session but not by Koha. - - "Storage of login session information: " diff --git a/t/db_dependent/Auth.t b/t/db_dependent/Auth.t index 5e7a18246cc..153b91a5d6b 100755 --- a/t/db_dependent/Auth.t +++ b/t/db_dependent/Auth.t @@ -1341,11 +1341,15 @@ subtest 'StaffLoginBranchBasedOnIP' => sub { ); t::lib::Mocks::mock_preference( 'AutoLocation', 0 ); - my $other_branch = $builder->build_object( { class => 'Koha::Libraries', value => { branchip => "127.0.0.1" } } ); + my $other_branch = $builder->build_object( + { class => 'Koha::Libraries', value => { branchip => "127.0.0.1", branchcode => "z" . $branch->branchcode } } ); ( $userid, $cookie, $sessionID, $flags ) = C4::Auth::checkauth( $cgi, 0, { catalogue => 1 }, 'intranet' ); is( $userid, $patron->userid, "User successfully logged in" ); $session = C4::Auth::get_session($sessionID); - is( $session->param('branch'), $branch->branchcode, "Logged in branch is set based which branch when two libraries have same IP?" ); + is( + $session->param('branch'), $branch->branchcode, + "Logged in branch is set based which branch when two libraries have same IP?" + ); }; @@ -1427,8 +1431,13 @@ subtest 'AutoLocation' => sub { $ENV{REMOTE_ADDR} = '129.0.0.1'; # Set current IP to match other_branch $cgi->param( 'branch', undef ); # Do not pass a branch $patron->library->branchip('')->store; # Unset user branch IP, to allow IP matching on any branch - # Add a second branch with same IP - my $another_library = $builder->build_object( { class => 'Koha::Libraries', value => { branchip => '129.0.0.1' } } ); + # Add a second branch with same IP + my $another_library = $builder->build_object( + { + class => 'Koha::Libraries', + value => { branchip => '129.0.0.1', branchcode => "z" . $other_library->branchcode } + } + ); ( $userid, $cookie, $sessionID, $flags, $template ) = C4::Auth::checkauth( $cgi, 0, { catalogue => 1 }, 'intranet', undef, undef, { do_not_print => 1 } ); $session = C4::Auth::get_session($sessionID); -- 2.39.2