From ed47d59b9022587e376fd951cc22bc1b9416f06e 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 --- 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 43b040e263d..a721da0a8ba 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.34.1