From 176763cfa7e47cddbafa929525c235bd5b26a249 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 (cherry picked from commit 6f4632ef04365436079a78b3b0e23bd34536d972) Signed-off-by: Lucas Gass --- 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 | 50 +++++++++++++++++++ 6 files changed, 93 insertions(+), 1 deletion(-) create mode 100755 installer/data/mysql/atomicupdate/bug_36665.pl diff --git a/C4/Auth.pm b/C4/Auth.pm index 850b75dd7b..c04ad626a6 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -1215,7 +1215,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 93d1a5598b..2f8ba2b750 100644 --- a/installer/data/mysql/mandatory/sysprefs.sql +++ b/installer/data/mysql/mandatory/sysprefs.sql @@ -690,6 +690,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'), ('StaticHoldsQueueWeight','0',NULL,'Specify a list of library location codes separated by commas -- the list of codes will be traversed and weighted with first values given higher weight for holds fulfillment -- alternatively, if RandomizeHoldsQueueWeight is set, the list will be randomly selective','Integer'), 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 c25ee18962..e86b1a04b5 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 a51a3f8399..90691feea1 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt @@ -125,6 +125,9 @@ [% 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 82187e67ff..7c8e0bb58b 100755 --- a/t/db_dependent/Auth.t +++ b/t/db_dependent/Auth.t @@ -1335,6 +1335,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