@@ -, +, @@ --- C4/Auth.pm | 5 ++- .../en/modules/admin/preferences/admin.pref | 6 +-- t/db_dependent/Auth.t | 44 +++++++++++++++---- 3 files changed, 41 insertions(+), 14 deletions(-) --- a/C4/Auth.pm +++ a/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'}; @@ -1248,7 +1248,8 @@ sub checkauth { && $branches->{$branchcode}->{'branchip'} ) ) { - 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'}; --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref +++ a/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: " --- a/t/db_dependent/Auth.t +++ a/t/db_dependent/Auth.t @@ -1341,17 +1341,25 @@ 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 => substr( "z" . $branch->branchcode, 0, 10 ) } + } + ); ( $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?" + ); }; subtest 'AutoLocation' => sub { - plan tests => 11; + plan tests => 12; $schema->storage->txn_begin; @@ -1428,19 +1436,37 @@ subtest 'AutoLocation' => sub { ( $userid, $cookie, $sessionID, $flags, $template ) = C4::Auth::checkauth( $cgi, 0, { catalogue => 1 }, 'intranet' ); $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" ); + 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 + $cgi->param( 'branch', '' ); # 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 => substr( "z" . $other_library->branchcode, 0, 10 ) } + } + ); ( $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?" + $session->param('branch'), $patron->library->branchcode, + "When user branch has no IP, and no branch chosen, user is logged in to their homebranch" + ); + + $cgi->param( 'branch', $another_library->branchcode ) + ; # Choose branch with duplicate IP and alphabetically later 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); + is( + $session->param('branch'), $another_library->branchcode, + "When there is an IP conflict, we use the chosen branch if it matches" ); $schema->storage->txn_rollback; --