From aa1c4c0281039749fa024687049b32b375d78ec6 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Tue, 21 May 2024 12:26:34 +0000 Subject: [PATCH] Bug 36908: Additional unit tests to identify flaw when two branches have same IP This could be considered a configuration flaw, but when: StaffLoginBranchBasedOnIP enabled and not AutoLocation or AutoLocation enabledand no IP set in user's branch AND two branches have the same IP set the user can be logged in randomly to one of the matching branches. These test often pass, but will also randomly fail Easier to verify with a one liner demonstrating current code: perl -e 'use Koha::Libraries; use List::MoreUtils qw(uniq); my $branches = { map { $_->branchcode => $_->unblessed } Koha::Libraries->search->as_list }; my $branchcode="CPL"; warn Data::Dumper::Dumper( uniq( $branchcode, keys %$branches ));' Signed-off-by: Jonathan Druart Signed-off-by: Martin Renvoize Signed-off-by: Katrin Fischer (cherry picked from commit 7e8803537254ec950c64327bece8091e6cf49499) Signed-off-by: Fridolin Somers --- t/db_dependent/Auth.t | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/t/db_dependent/Auth.t b/t/db_dependent/Auth.t index e16f683710..3fcf328d61 100755 --- a/t/db_dependent/Auth.t +++ b/t/db_dependent/Auth.t @@ -1294,7 +1294,7 @@ subtest 'checkpw() return values tests' => sub { subtest 'StaffLoginBranchBasedOnIP' => sub { - plan tests => 5; + plan tests => 7; $schema->storage->txn_begin; @@ -1340,11 +1340,18 @@ subtest 'StaffLoginBranchBasedOnIP' => sub { "AutoLocation prevents StaffLoginBranchBasedOnIP from logging user in to another branch" ); + t::lib::Mocks::mock_preference( 'AutoLocation', 0 ); + my $other_branch = $builder->build_object( { class => 'Koha::Libraries', value => { branchip => "127.0.0.1" } } ); + ( $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?" ); + }; subtest 'AutoLocation' => sub { - plan tests => 9; + plan tests => 11; $schema->storage->txn_begin; @@ -1423,6 +1430,19 @@ subtest 'AutoLocation' => sub { $session = C4::Auth::get_session($sessionID); is( $session->param('branch'), $noip_library->branchcode, "When a branch with no IP set is chosen, we respect the choice regardless of current IP" ); + $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' } } ); + ( $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); + is( + $session->param('branch'), $other_library->branchcode, + "Which branch is chosen when home branch has no IP and more than 1 library matches?" + ); + $schema->storage->txn_rollback; }; -- 2.39.5