From 98e7359d25418a393913d016bc928df055341b76 Mon Sep 17 00:00:00 2001 From: Michael Hafen Date: Mon, 14 Jul 2025 18:03:43 -0600 Subject: [PATCH] Bug 28657: expand branches.branchip to allow for multiple ip ranges. Content-Type: text/plain; charset="utf-8" change branches.branchip to a mediumtext, and remove '*' from existing branchip settings. Change C4/Auth.pm from a regexp to using in_iprange() to check client address against branchip. (in_iprange() doesn't work with '*' in branchip.) Change in_iprange() to default to false (which is what the POD says it should do). Also, have it trim '*' in case someone adds some back later. Change tests to account for new default, and add a test for '*'. Change note in admin/branches.tt under branchip field to reflect changes to branchip handling. Test plan: 1. Turn on AutoLocation system preference 2. Change a branches branchip to multiple ip ranges seperated by a space, making sure one of the ranges includes your computers address. 3. Log out and try logging in. The location check will fail. 4. Apply patch. Restart plack if necessary. 5. Try logging in again. The location check will succeed. --- C4/Auth.pm | 15 +++++++----- Koha/Template/Plugin/Branches.pm | 6 ++--- api/v1/swagger/definitions/library.yaml | 2 +- .../data/mysql/atomicupdate/bug_28657.perl | 23 +++++++++++++++++++ installer/data/mysql/kohastructure.sql | 2 +- .../prog/en/modules/admin/branches.tt | 4 ++-- t/Auth.t | 12 ++++++---- 7 files changed, 46 insertions(+), 18 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_28657.perl diff --git a/C4/Auth.pm b/C4/Auth.pm index 2fa9e2bed7..231651d8da 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -1278,9 +1278,8 @@ sub checkauth { # we have to check they are coming from the right ip range my $domain = $branches->{$branchcode}->{'branchip'} // q{}; - $domain =~ s|\.\*||g; - $domain =~ s/\s+//g; - if ( $domain && $ip !~ /^$domain/ ) { + next unless ( $domain ); + if ( ! in_iprange($domain) ) { $cookie = $cookie_mgr->replace_in_list( $cookie, $query->cookie( @@ -1316,7 +1315,8 @@ sub checkauth { # now we work with the treatment of ip my $domain = $branches->{$br}->{'branchip'}; - if ( $domain && $ip =~ /^$domain/ ) { + next unless ( $domain ); + if ( in_iprange($domain) ) { $branchcode = $branches->{$br}->{'branchcode'}; # new op dev : add the branchname to the cookie @@ -1813,7 +1813,8 @@ sub check_api_auth { # now we work with the treatment of ip my $domain = $branches->{$br}->{'branchip'}; - if ( $domain && $ip =~ /^$domain/ ) { + next unless ( $domain ); + if ( in_iprange($domain) ) { $branchcode = $branches->{$br}->{'branchcode'}; # new op dev : add the branchname to the cookie @@ -2472,7 +2473,9 @@ Returns 1 if the remote address is in the provided iprange, or 0 otherwise. sub in_iprange { my ($iprange) = @_; - my $result = 1; + my $result = 0; + # FIXME remove '*' for backwards compatibility in branchip settings + $iprange =~ s|\*||g if ($iprange); my @allowedipranges = $iprange ? split( ' ', $iprange ) : (); if ( scalar @allowedipranges > 0 ) { my @rangelist; diff --git a/Koha/Template/Plugin/Branches.pm b/Koha/Template/Plugin/Branches.pm index 164bc8162b..acfb9c767d 100644 --- a/Koha/Template/Plugin/Branches.pm +++ b/Koha/Template/Plugin/Branches.pm @@ -25,6 +25,7 @@ use base qw( Template::Plugin ); use C4::Koha; use C4::Context; +use C4::Auth qw(in_iprange); use Koha::Cache::Memory::Lite; use Koha::Libraries; @@ -91,13 +92,10 @@ sub all { : Koha::Libraries->search_filtered( $search_params, { order_by => ['branchname'] } )->unblessed; if ($ip_limit) { - my $ip = $ENV{'REMOTE_ADDR'}; my @ip_libraries = (); for my $l (@$libraries) { my $domain = $l->{branchip} // ''; - $domain =~ s|\.\*||g; - $domain =~ s/\s+//g; - unless ( $domain && $ip !~ /^$domain/ ) { + unless ( $domain && ! in_iprange($domain) ) { push @ip_libraries, $l; } } diff --git a/api/v1/swagger/definitions/library.yaml b/api/v1/swagger/definitions/library.yaml index c4debe94b5..116de475a8 100644 --- a/api/v1/swagger/definitions/library.yaml +++ b/api/v1/swagger/definitions/library.yaml @@ -85,7 +85,7 @@ properties: - string - "null" description: the IP address for your library or branch - maxLength: 15 + maxLength: 120 notes: type: - string diff --git a/installer/data/mysql/atomicupdate/bug_28657.perl b/installer/data/mysql/atomicupdate/bug_28657.perl new file mode 100644 index 0000000000..6f2e3ae07b --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_28657.perl @@ -0,0 +1,23 @@ +$DBversion = 'XXX'; # will be replaced by the RM +if( CheckVersion( $DBversion ) ) { + $dbh->do( "ALTER TABLE `branches` MODIFY `branchip` mediumtext DEFAULT NULL COMMENT 'the IP address(s) for your library or branch'" ); + + my $sth = $dbh->prepare(" + SELECT branchip,branchcode + FROM branches + WHERE branchip like '%*%' + "); + $sth->execute; + my $results = $sth->fetchall_arrayref({}); + $sth = $dbh->prepare(" + UPDATE branches + SET branchip = ? + WHERE branchcode = ? + "); + foreach(@$results) { + $_->{branchip} =~ s|\*||g; + $sth->execute($_->{branchip}, $_->{branchcode}); + } + + NewVersion( $DBversion, 28657, "expand branches.branchip to allow for multiple ip ranges and remove '*'"); +} diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index 11018d4f7b..20a78876cd 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -1670,7 +1670,7 @@ CREATE TABLE `branches` ( `branchreturnpath` longtext DEFAULT NULL COMMENT 'the email to be used as Return-Path', `branchurl` longtext DEFAULT NULL COMMENT 'the URL for your library or branch''s website', `issuing` tinyint(4) DEFAULT NULL COMMENT 'unused in Koha', - `branchip` varchar(15) DEFAULT NULL COMMENT 'the IP address for your library or branch', + `branchip` mediumtext DEFAULT NULL COMMENT 'the IP address(s) for your library or branch', `branchnotes` longtext DEFAULT NULL COMMENT 'notes related to your library or branch', `geolocation` varchar(255) DEFAULT NULL COMMENT 'geolocation of your library', `marcorgcode` varchar(16) DEFAULT NULL COMMENT 'MARC Organization Code, see http://www.loc.gov/marc/organizations/orgshome.html, when empty defaults to syspref MARCOrgCode', diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/branches.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/branches.tt index 4098f59c3d..c53d5ed6c5 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/branches.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/branches.tt @@ -362,9 +362,9 @@
- +
-
Enter as a single IP address, or a subnet such as 192.168.1.*
+
Enter as a single IP address, an IP range such as 192.168.1. or 192.168.1.10-192.168.1.50 or 192.168.1.0/24, or multiple space separated IP addresses or ranges.
diff --git a/t/Auth.t b/t/Auth.t index d7eedee376..40221795dc 100755 --- a/t/Auth.t +++ b/t/Auth.t @@ -17,7 +17,7 @@ use Modern::Perl; use Test::NoWarnings; -use Test::More tests => 13; +use Test::More tests => 14; use Test::Warn; use C4::Auth qw( in_iprange ); @@ -35,10 +35,14 @@ ok( in_iprange("127.0.0.1 192.168.1.30 192.168.2.10-192.168.2.25"), 'multiple ips and ranges, including the remote ip' ); +ok( + in_iprange("127.0.0.1 192.168.*.* 192.168.2.10-192.168.2.25"), + 'multiple ips and ranges, including a "*" from old settings' +); ok( !in_iprange("127.0.0.1 8.8.8.8 192.168.2.1/24 192.168.3.1/24 192.168.1.1-192.168.1.29"), "multiple ip and ip ranges, with the remote ip in none of them" ); -ok( in_iprange(""), "blank list given, no preference set - implies everything goes through." ); -ok( in_iprange(), "no list given, no preference set - implies everything goes through." ); -ok( in_iprange("192.168.1.1/36"), 'simple invalid ip range/36 with remote ip in it' ); +ok( !in_iprange(""), "blank list given, no preference set - default to not in range." ); +ok( !in_iprange(), "no list given, no preference set - default to not in range." ); +ok( !in_iprange("192.168.1.1/36"), 'simple invalid ip range/36 with remote ip in it' ); -- 2.43.0