From 3a0d6f5d07b914ab03f9ae3c56b033158bd91130 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 --- 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 608650937f..4b0bc1bbe7 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -1234,7 +1234,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 06ebba4511..d4c4acde1b 100644 --- a/installer/data/mysql/mandatory/sysprefs.sql +++ b/installer/data/mysql/mandatory/sysprefs.sql @@ -737,6 +737,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 2b5c78c69f..685486d6c1 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 e66fe24bf8..f50740e189 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt @@ -158,6 +158,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 c66b2a469a..71d4a63c26 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 => 21; +use Test::More tests => 22; 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