From 6f4632ef04365436079a78b3b0e23bd34536d972 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Fri, 17 May 2024 11:59:21 +0000 Subject: [PATCH] Bug 36665: Add option to set the staff user's logged in branch based on their current ip This patch adds a new system preference StaffLoginBranchBasedOnIP which restores the behaviour before bug 35918 of using the current IP to determine the user's logged in branchcode To test: 1 - Get your current ip 2 - Set that IP for a library in the administration section 3 - Find a user account assigned to a different library that can login to staff side 4 - Login to staff as that user, select 'My library' 5 - You are logged in to the user's branch 6 - Apply patch, restart all 7 - Log out and back in, selecting 'My library' 8 - You are logged in to the user's branch 9 - Enable new system preference StaffLoginBranchBasedOnIP 9 - Log out and back in, selecting a different branch, noting the new warning below the library selection 10 - You are logged in to the branch with the matching IP 11 - Log out and back in, selecting 'My library' 10 - You are logged in to the branch with the matching IP 11 - Change your logged in branch 12 - Verify the selection sticks and you can perform staff actions in the chosen branch 13 - Change the IP of the library to one that doesn't match yours 14 - Verify you can log out and log back in and that selected branch is respected when your IP doesn't match library IP Signed-off-by: Kristi Krueger Signed-off-by: Martin Renvoize Signed-off-by: Jonathan Druart Signed-off-by: Katrin Fischer (cherry picked from commit 3a0d6f5d07b914ab03f9ae3c56b033158bd91130) Signed-off-by: Fridolin Somers --- C4/Auth.pm | 9 +++- .../data/mysql/atomicupdate/bug_36665.pl | 21 ++++++++ installer/data/mysql/mandatory/sysprefs.sql | 1 + .../en/modules/admin/preferences/admin.pref | 10 ++++ .../intranet-tmpl/prog/en/modules/auth.tt | 3 ++ t/db_dependent/Auth.t | 52 ++++++++++++++++++- 6 files changed, 94 insertions(+), 2 deletions(-) create mode 100755 installer/data/mysql/atomicupdate/bug_36665.pl diff --git a/C4/Auth.pm b/C4/Auth.pm index d6f4d3b138..9bd47b0059 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -1214,7 +1214,14 @@ sub checkauth { } } - if ( C4::Context->preference('AutoLocation') && $auth_state ne 'failed' ) { + if ( + ( + !C4::Context->preference('AutoLocation') + && C4::Context->preference('StaffLoginBranchBasedOnIP') + ) + || ( C4::Context->preference('AutoLocation') && $auth_state ne 'failed' ) + ) + { foreach my $br ( uniq( $branchcode, keys %$branches ) ) { # now we work with the treatment of ip diff --git a/installer/data/mysql/atomicupdate/bug_36665.pl b/installer/data/mysql/atomicupdate/bug_36665.pl new file mode 100755 index 0000000000..951248b4d9 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_36665.pl @@ -0,0 +1,21 @@ +use Modern::Perl; +use Koha::Installer::Output qw(say_warning say_failure say_success say_info); + +return { + bug_number => "36665", + description => "Add StaffLoginBranchBasedOnIP", + up => sub { + my ($args) = @_; + my ( $dbh, $out ) = @$args{qw(dbh out)}; + + # Do you stuffs here + $dbh->do( + q{ + INSERT IGNORE INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `type` ) VALUES + ('StaffLoginBranchBasedOnIP', '0','', 'Set the logged in branch for the user based on their current IP','YesNo') + } + ); + + say $out "Added new system preference 'StaffLoginBranchBasedOnIP'"; + }, +}; diff --git a/installer/data/mysql/mandatory/sysprefs.sql b/installer/data/mysql/mandatory/sysprefs.sql index f7e845e3bd..fda8304b5d 100644 --- a/installer/data/mysql/mandatory/sysprefs.sql +++ b/installer/data/mysql/mandatory/sysprefs.sql @@ -724,6 +724,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('StaffHighlightedWords','1','','Highlight search terms on staff interface','YesNo'), ('StaffLangSelectorMode','footer','top|both|footer','Select the location to display the language selector in staff interface','Choice'), ('StaffLoginInstructions', '', NULL, 'HTML to go into the login box for the staff interface','Free'), +('StaffLoginBranchBasedOnIP', '0','', 'Set the logged in branch for the user based on their current IP','YesNo'), ('StaffSearchResultsDisplayBranch','holdingbranch','holdingbranch|homebranch','Controls the display of the home or holding branch for staff search results','Choice'), ('StaffSerialIssueDisplayCount','3','','Number of serial issues to display per subscription in the staff interface','Integer'), ('staffShibOnly','0','','If ON enables shibboleth only authentication for the staff client','YesNo'), 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 84238d0e53..a664499475 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 @@ -98,6 +98,7 @@ Administration: 1: "Yes" 0: "No" - Link to library administration + - 'This setting will override the StaffLoginBranchBasedOnIP system preference.' - - "Enable check for change in remote IP address for session security: " - pref: SessionRestrictionByIP @@ -106,6 +107,15 @@ Administration: 1: "Yes" 0: "No" - (Disable only when remote IP address changes frequently.) + - + - "Set the logged in branch for staff based on their current branch: " + - pref: StaffLoginBranchBasedOnIP + default: 0 + choices: + 1: "Yes" + 0: "No" + - "Note: If IPs overlap, the first found match will be used." + - 'This setting will be overridden by AutoLocation system preference.' # PostgreSQL is supported by CGI::Session but not by Koha. - - "Storage of login session information: " diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt index fe35c5cb64..a65a96fa3d 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt @@ -154,6 +154,9 @@ [% IF Koha.Preference('ForceLibrarySelection') %] Required [% END %] + [% IF Koha.Preference('StaffLoginBranchBasedOnIP')%] + Note: Your selection may be overridden if your current IP matches a specified IP for a branch in Koha + [% END %]

[% IF Koha.Preference('UseCirculationDesks') && Desks.all %] diff --git a/t/db_dependent/Auth.t b/t/db_dependent/Auth.t index 039b90ed8c..b59c1aa06c 100755 --- a/t/db_dependent/Auth.t +++ b/t/db_dependent/Auth.t @@ -7,7 +7,7 @@ use CGI qw ( -utf8 ); use Test::MockObject; use Test::MockModule; use List::MoreUtils qw/all any none/; -use Test::More tests => 20; +use Test::More tests => 21; use Test::Warn; use t::lib::Mocks; use t::lib::TestBuilder; @@ -1292,6 +1292,56 @@ subtest 'checkpw() return values tests' => sub { }; }; +subtest 'StaffLoginBranchBasedOnIP' => sub { + + plan tests => 5; + + $schema->storage->txn_begin; + + t::lib::Mocks::mock_preference( 'AutoLocation', 0 ); + t::lib::Mocks::mock_preference( 'StaffLoginBranchBasedOnIP', 0 ); + + my $patron = $builder->build_object( { class => 'Koha::Patrons', value => { flags => 1 } } ); + my $branch = $builder->build_object( { class => 'Koha::Libraries', value => { branchip => "127.0.0.1" } } ); + my $password = 'password'; + t::lib::Mocks::mock_preference( 'RequireStrongPassword', 0 ); + $patron->set_password( { password => $password } ); + + my $cgi_mock = Test::MockModule->new('CGI'); + $cgi_mock->mock( 'request_method', sub { return 'POST' } ); + my $cgi = CGI->new; + my $auth = Test::MockModule->new('C4::Auth'); + + # Simulating the login form submission + $cgi->param( 'login_userid', $patron->userid ); + $cgi->param( 'login_password', $password ); + + $ENV{REMOTE_ADDR} = '127.0.0.1'; + my ( $userid, $cookie, $sessionID, $flags ) = + C4::Auth::checkauth( $cgi, 0, { catalogue => 1 }, 'intranet' ); + is( $userid, $patron->userid, "User successfully logged in" ); + my $session = C4::Auth::get_session($sessionID); + is( $session->param('branch'), $patron->branchcode, "Logged in branch is set to the patron's branchcode" ); + + my $template; + t::lib::Mocks::mock_preference( 'StaffLoginBranchBasedOnIP', 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 on the IP from REMOTE_ADDR " ); + + # AutoLocation overrides StaffLoginBranchBasedOnIP + t::lib::Mocks::mock_preference( 'AutoLocation', 1 ); + ( $userid, $cookie, $sessionID, $flags, $template ) = + C4::Auth::checkauth( $cgi, 0, { catalogue => 1 }, 'intranet', undef, undef, { do_not_print => 1 } ); + is( + $template->{VARS}->{wrongip}, 1, + "AutoLocation prevents StaffLoginBranchBasedOnIP from logging user in to another branch" + ); + +}; + subtest 'AutoLocation' => sub { plan tests => 7; -- 2.39.5