From 8b2bdf6ee5a18e247e98d67d7e81605cc45542c6 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Tue, 21 May 2024 13:44:47 +0000 Subject: [PATCH] Bug 26176: Rename AutoLocation to StaffLoginRestrictBranchByIP This patch sets AutoLocation to be called StaffLoginRestrictBranchByIP. The new name is chosen to reflect the new pref StaffLoginBranchBasedOnIP. Also this patch corrects the order of sysprefs in installer file. To test: Follow test plans on bug 36665 and bug 35890 and confirm that the preferences continue to work as expected Confirm the descriptions of the prefs in the staff interface match the behaviors expected Signed-off-by: Jonathan Druart Signed-off-by: Martin Renvoize Signed-off-by: Katrin Fischer --- C4/Auth.pm | 12 +++++----- C4/UsageStats.pm | 2 +- .../data/mysql/atomicupdate/bug_26176.pl | 18 ++++++++++++++ installer/data/mysql/mandatory/sysprefs.sql | 2 +- .../intranet-tmpl/prog/en/includes/header.inc | 4 ++-- .../en/modules/admin/preferences/admin.pref | 4 ++-- t/db_dependent/Auth.t | 24 +++++++++---------- 7 files changed, 42 insertions(+), 24 deletions(-) create mode 100755 installer/data/mysql/atomicupdate/bug_26176.pl diff --git a/C4/Auth.pm b/C4/Auth.pm index a1019737c0..8f7789c697 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -498,7 +498,7 @@ sub get_template_and_user { advancedMARCEditor => C4::Context->preference("advancedMARCEditor"), AllowMultipleCovers => C4::Context->preference('AllowMultipleCovers'), AmazonCoverImages => C4::Context->preference("AmazonCoverImages"), - AutoLocation => C4::Context->preference("AutoLocation"), + StaffLoginRestrictBranchByIP => C4::Context->preference("StaffLoginRestrictBranchByIP"), can_see_cataloguing_module => haspermission( $user, get_cataloguing_page_permissions() ) ? 1 : 0, canreservefromotherbranches => C4::Context->preference('canreservefromotherbranches'), EasyAnalyticalRecords => C4::Context->preference('EasyAnalyticalRecords'), @@ -1216,7 +1216,7 @@ sub checkauth { } if ( $type ne 'opac' ) { my $branches = { map { $_->branchcode => $_->unblessed } Koha::Libraries->search->as_list }; - if ( C4::Context->preference('AutoLocation') ) { + if ( C4::Context->preference('StaffLoginRestrictBranchByIP') ) { # we have to check they are coming from the right ip range my $domain = $branches->{$branchcode}->{'branchip'}; $domain =~ s|\.\*||g; @@ -1238,12 +1238,12 @@ sub checkauth { # If StaffLoginBranchBasedOnIP is enabled we will try to find a branch # matching your ip, regardless of the choice you have passed in ( - !C4::Context->preference('AutoLocation') + !C4::Context->preference('StaffLoginRestrictBranchByIP') && C4::Context->preference('StaffLoginBranchBasedOnIP') ) - # When AutoLocation is enabled we will not choose a branch matching IP + # When StaffLoginRestrictBranchByIP is enabled we will not choose a branch matching IP # if your selected branch has no IP set - || ( C4::Context->preference('AutoLocation') + || ( C4::Context->preference('StaffLoginRestrictBranchByIP') && $auth_state ne 'failed' && $branches->{$branchcode}->{'branchip'} ) ) @@ -1444,7 +1444,7 @@ sub checkauth { IntranetUserCSS => C4::Context->preference("IntranetUserCSS"), IntranetUserJS => C4::Context->preference("IntranetUserJS"), IndependentBranches => C4::Context->preference("IndependentBranches"), - AutoLocation => C4::Context->preference("AutoLocation"), + StaffLoginRestrictBranchByIP => C4::Context->preference("StaffLoginRestrictBranchByIP"), wrongip => $info{'wrongip'}, PatronSelfRegistration => C4::Context->preference("PatronSelfRegistration"), PatronSelfRegistrationDefaultCategory => C4::Context->preference("PatronSelfRegistrationDefaultCategory"), diff --git a/C4/UsageStats.pm b/C4/UsageStats.pm index 56877dd8af..f45a3d4a85 100644 --- a/C4/UsageStats.pm +++ b/C4/UsageStats.pm @@ -146,7 +146,7 @@ sub _shared_preferences { noItemTypeImages OpacNoItemTypeImages virtualshelves - AutoLocation + StaffLoginRestrictBranchByIP IndependentBranches SessionStorage Persona diff --git a/installer/data/mysql/atomicupdate/bug_26176.pl b/installer/data/mysql/atomicupdate/bug_26176.pl new file mode 100755 index 0000000000..fb675cfe38 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_26176.pl @@ -0,0 +1,18 @@ +use Modern::Perl; +use Koha::Installer::Output qw(say_warning say_failure say_success say_info); + +return { + bug_number => "26176", + description => "Rename AutoLocation to StaffLoginRestrictBranchByIP", + up => sub { + my ($args) = @_; + my ( $dbh, $out ) = @$args{qw(dbh out)}; + + $dbh->do(q{ + UPDATE systempreferences SET variable = "StaffLoginRestrictBranchByIP" + WHERE variable = "AutoLocation" + }); + + say $out "Renamed system preference 'AutoLocation' to 'StaffLoginRestrictBranchByIP'"; + }, +}; diff --git a/installer/data/mysql/mandatory/sysprefs.sql b/installer/data/mysql/mandatory/sysprefs.sql index 6f70632031..8d8676be5d 100644 --- a/installer/data/mysql/mandatory/sysprefs.sql +++ b/installer/data/mysql/mandatory/sysprefs.sql @@ -90,7 +90,6 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('AutoCreditNumber', '', '', 'Automatically generate a number for account credits', 'Choice'), ('AutoEmailNewUser','0',NULL,'Send an email to newly created patrons.','YesNo'), ('AutoLinkBiblios','0',NULL,'If enabled, link biblio to authorities on creation and edit','YesNo'), -('AutoLocation','0',NULL,'If ON, IP authentication is enabled, blocking access to the staff interface from unauthorized IP addresses','YesNo'), ('AutomaticCheckinAutoFill','0',NULL,'Automatically fill the next hold with an automatic check in.','YesNo'), ('AutomaticConfirmTransfer','0',NULL,'Defines whether transfers should be automatically confirmed at checkin if modal dismissed','YesNo'), ('AutomaticItemReturn','1',NULL,'If ON, Koha will automatically set up a transfer of this item to its homebranch','YesNo'), @@ -738,6 +737,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('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', '1','', 'Set the logged in library for the user based on their current IP','YesNo'), +('StaffLoginRestrictBranchByIP','0',NULL,'If ON, IP authentication is enabled, blocking access to the staff interface from unauthorized IP addresses based on branch','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/includes/header.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/header.inc index 78cacbd6ed..0a58182e55 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/header.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/header.inc @@ -95,7 +95,7 @@ [% SET is_superlibrarian = CAN_user_superlibrarian ? 'is_superlibrarian' : '' %] [% logged_in_user.userid | html %] [% logged_in_user.categorycode | html %] - [% IF ( AutoLocation ) %] + [% IF ( StaffLoginRestrictBranchByIP ) %] [% Branches.GetLoggedInBranchname | html %] @@ -133,7 +133,7 @@ [% logged_in_user.userid | html %]
  • - [% IF ( AutoLocation ) %] + [% IF ( StaffLoginRestrictBranchByIP ) %] [% Branches.GetLoggedInBranchname | html %] 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 d2de2364f0..a55a0e6e21 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 @@ -92,7 +92,7 @@ Administration: - Adding d will specify it in days, e.g. 1d is timeout of one day. - - "Limit the libraries staff can select at login to those where their computer's IP address is within the library's specified range or to libraries without an IP restriction: " - - pref: AutoLocation + - pref: StaffLoginRestrictBranchByIP default: 0 choices: 1: "Yes" @@ -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. In the event of multiple branches with matching IPs, branchcode (alphabetically) will be the tie breaker.' + - 'This setting will be overridden by StaffLoginRestrictBranchByIP 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: " diff --git a/t/db_dependent/Auth.t b/t/db_dependent/Auth.t index bf092f8727..a2bc705742 100755 --- a/t/db_dependent/Auth.t +++ b/t/db_dependent/Auth.t @@ -1298,8 +1298,8 @@ subtest 'StaffLoginBranchBasedOnIP' => sub { $schema->storage->txn_begin; - t::lib::Mocks::mock_preference( 'AutoLocation', 0 ); - t::lib::Mocks::mock_preference( 'StaffLoginBranchBasedOnIP', 0 ); + t::lib::Mocks::mock_preference( 'StaffLoginRestrictBranchByIP', 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" } } ); @@ -1331,16 +1331,16 @@ subtest 'StaffLoginBranchBasedOnIP' => sub { $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 ); + # StaffLoginRestrictBranchByIP overrides StaffLoginBranchBasedOnIP + t::lib::Mocks::mock_preference( 'StaffLoginRestrictBranchByIP', 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" + "StaffLoginRestrictBranchByIP prevents StaffLoginBranchBasedOnIP from logging user in to another branch" ); - t::lib::Mocks::mock_preference( 'AutoLocation', 0 ); + t::lib::Mocks::mock_preference( 'StaffLoginRestrictBranchByIP', 0 ); my $other_branch = $builder->build_object( { class => 'Koha::Libraries', @@ -1357,13 +1357,13 @@ subtest 'StaffLoginBranchBasedOnIP' => sub { }; -subtest 'AutoLocation' => sub { +subtest 'StaffLoginRestrictBranchByIP' => sub { plan tests => 12; $schema->storage->txn_begin; - t::lib::Mocks::mock_preference( 'AutoLocation', 0 ); + t::lib::Mocks::mock_preference( 'StaffLoginRestrictBranchByIP', 0 ); my $patron = $builder->build_object( { class => 'Koha::Patrons', value => { flags => 1 } } ); my $password = 'password'; @@ -1381,12 +1381,12 @@ subtest 'AutoLocation' => sub { $ENV{REMOTE_ADDR} = '127.0.0.1'; my ( $userid, $cookie, $sessionID, $flags ) = C4::Auth::checkauth( $cgi, 0, { catalogue => 1 }, 'intranet' ); - is( $userid, $patron->userid, "Standard login without AutoLocation" ); + is( $userid, $patron->userid, "Standard login without StaffLoginRestrictBranchByIP" ); my $template; - t::lib::Mocks::mock_preference( 'AutoLocation', 1 ); + t::lib::Mocks::mock_preference( 'StaffLoginRestrictBranchByIP', 1 ); - # AutoLocation: "Require staff to log in from a computer in the IP address range specified by their library (if any)" + # StaffLoginRestrictBranchByIP: "Require staff to log in from a computer in the IP address range specified by their library (if any)" $patron->library->branchip('')->store; # There is none, allow access from anywhere ( $userid, $cookie, $sessionID, $flags, $template ) = C4::Auth::checkauth( $cgi, 0, { catalogue => 1 }, 'intranet' ); @@ -1423,7 +1423,7 @@ subtest 'AutoLocation' => sub { $session = C4::Auth::get_session($sessionID); is( $session->param('branch'), $other_library->branchcode, - "AutoLocation allows specifying a branch as long as the IP matches" + "StaffLoginRestrictBranchByIP allows specifying a branch as long as the IP matches" ); $other_library->branchip('129.0.0.1')->store; -- 2.39.5