From 21dcfb4075f44c4ddfeebc61e7445582529516aa 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 ));' --- t/db_dependent/Auth.t | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/t/db_dependent/Auth.t b/t/db_dependent/Auth.t index d1876bfeec7..5e7a18246cc 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 => 10; $schema->storage->txn_begin; @@ -1416,6 +1423,20 @@ subtest 'AutoLocation' => sub { C4::Auth::checkauth( $cgi, 0, { catalogue => 1 }, 'intranet', undef, undef, { do_not_print => 1 } ); is( $template->{VARS}->{wrongip}, 1, "Login denied when branch specified and IP does not match branch 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.2