@@ -, +, @@ ranges. remove branches.tt branchip input maxlength attribute. Have the db_rev match the kohastructure.sql for column collation and comment. Use new style db_rev skeleton. Fix comment C4::Auth. Have Koha::Library->store() watch for and remove '*' from branchip. Add a couple more test for full coverage. making sure one of the ranges includes your computers address. --- C4/Auth.pm | 51 +++++++++++-------- Koha/Library.pm | 6 +++ .../data/mysql/atomicupdate/bug_28657.pl | 26 ++++++++++ installer/data/mysql/kohastructure.sql | 2 +- .../prog/en/modules/admin/branches.tt | 4 +- t/Auth.t | 28 +++++----- 6 files changed, 80 insertions(+), 37 deletions(-) create mode 100755 installer/data/mysql/atomicupdate/bug_28657.pl --- a/C4/Auth.pm +++ a/C4/Auth.pm @@ -1198,17 +1198,20 @@ sub checkauth { # we have to check they are coming from the right ip range my $domain = $branches->{$branchcode}->{'branchip'}; - $domain =~ s|\.\*||g; - if ( $ip !~ /^$domain/ ) { + next unless ($domain); + if ( !in_iprange($domain) ) { $loggedin = 0; $auth_state = 'failed'; - $cookie = $cookie_mgr->replace_in_list( $cookie, $query->cookie( - -name => 'CGISESSID', - -value => '', - -HttpOnly => 1, - -secure => ( C4::Context->https_enabled() ? 1 : 0 ), - -sameSite => 'Lax', - )); + $cookie = $cookie_mgr->replace_in_list( + $cookie, + $query->cookie( + -name => 'CGISESSID', + -value => '', + -HttpOnly => 1, + -secure => ( C4::Context->https_enabled() ? 1 : 0 ), + -sameSite => 'Lax', + ) + ); $info{'wrongip'} = 1; } } @@ -1217,11 +1220,12 @@ 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 - $branchname = $branches->{$br}->{'branchname'}; + $branchname = $branches->{$br}->{'branchname'}; } } @@ -1658,18 +1662,19 @@ sub check_api_auth { if ( $query->param('branch') ) { $branchcode = $query->param('branch'); my $library = Koha::Libraries->find($branchcode); - $branchname = $library? $library->branchname: ''; + $branchname = $library ? $library->branchname: ''; } my $branches = { map { $_->branchcode => $_->unblessed } Koha::Libraries->search->as_list }; foreach my $br ( keys %$branches ) { # 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 - $branchname = $branches->{$br}->{'branchname'}; + $branchname = $branches->{$br}->{'branchname'}; } } $session->param( 'number', $borrowernumber ); @@ -2328,14 +2333,18 @@ Returns 1 if the remote address is in the provided iprange, or 0 otherwise. sub in_iprange { my ($iprange) = @_; - my $result = 1; - my @allowedipranges = $iprange ? split(' ', $iprange) : (); - if (scalar @allowedipranges > 0) { + return 1 unless $iprange; + my $result = 0; + + my @allowedipranges = $iprange ? split( ' ', $iprange ) : (); + if (@allowedipranges) { my @rangelist; - eval { @rangelist = Net::CIDR::range2cidr(@allowedipranges); }; return 0 if $@; - eval { $result = Net::CIDR::cidrlookup($ENV{'REMOTE_ADDR'}, @rangelist) } || Koha::Logger->get->warn('cidrlookup failed for ' . join(' ',@rangelist) ); - } - return $result ? 1 : 0; + eval { @rangelist = Net::CIDR::range2cidr(@allowedipranges); }; + return 0 if $@; + eval { $result = Net::CIDR::cidrlookup( $ENV{'REMOTE_ADDR'}, @rangelist ) } + || Koha::Logger->get->warn( 'cidrlookup failed for ' . join( ' ', @rangelist ) ); + } + return $result ? 1 : 0; } sub getborrowernumber { --- a/Koha/Library.pm +++ a/Koha/Library.pm @@ -58,6 +58,12 @@ sub store { $flush = 1 if ( $self_from_storage->branchname ne $self->branchname ); } + if ( $self->branchip =~ /\*/ ) { + my $branchip = $self->branchip; + $branchip =~ s/\*//g; + $branchip =~ s/\.{2,}/./g; + $self->branchip( $branchip ); + } $self = $self->SUPER::store; if ($flush) { --- a/installer/data/mysql/atomicupdate/bug_28657.pl +++ a/installer/data/mysql/atomicupdate/bug_28657.pl @@ -0,0 +1,26 @@ +use Modern::Perl; + +return { + bug_number => "28657", + description => "expand branches.branchip to allow for multiple ip ranges and remove '*'", + up => sub { + my ($args) = @_; + my ( $dbh, $out ) = @$args{qw(dbh out)}; + + $dbh->do( + q{ALTER TABLE `branches` MODIFY `branchip` mediumtext DEFAULT NULL COMMENT 'the IP address(s) for your library or branch'} + ); + say $out "Modify column 'branches.branchip' to mediumtext"; + + my $sth_select = $dbh->prepare(q{SELECT branchip,branchcode FROM branches WHERE branchip like '%*%'}); + my $sth_update = $dbh->prepare(q{UPDATE branches SET branchip = ? WHERE branchcode = ?}); + $sth_select->execute; + my $results = $sth_select->fetchall_arrayref( {} ); + foreach (@$results) { + $_->{'branchip'} =~ s/\*//g; + $_->{'branchip'} =~ s/\.{2,}/./g; + $sth_update->execute( $_->{'branchip'}, $_->{'branchcode'} ); + } + say $out "Update column 'branches.branchip' to remove '*'"; + }, +}; --- a/installer/data/mysql/kohastructure.sql +++ a/installer/data/mysql/kohastructure.sql @@ -1648,7 +1648,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', --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/branches.tt +++ a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/branches.tt @@ -249,8 +249,8 @@
  • - -
    Can be entered as a single IP, or a subnet such as 192.168.1.*
    + +
    Can be entered as a single IP, 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.
  • --- a/t/Auth.t +++ a/t/Auth.t @@ -16,22 +16,24 @@ # along with Koha; if not, see . use Modern::Perl; -use Test::More tests => 12; +use Test::More tests => 14; use Test::Warn; use C4::Auth qw( in_iprange ); $ENV{REMOTE_ADDR} = '192.168.1.30'; -ok(in_iprange("192.168.1.30"), 'simple single ip matching remote ip'); -ok(!in_iprange("192.168.1.31"), 'simple single ip not match remote ip'); -ok(in_iprange("192.168.1.1/24"), 'simple ip range/24 with remote ip in it'); -ok(!in_iprange("192.168.2.1/24"), 'simple ip range/24 with remote ip not in it'); -ok(in_iprange("192.168.2.1/16"), 'simple ip range/16 with remote ip in it'); -ok(!in_iprange("192.168.1.10-30"), 'invalidly represented IP range with remote ip in it'); -ok(in_iprange("192.168.1.10-192.168.1.30"), 'validly represented ip range with remote ip in it'); -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 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'); +is( 1, in_iprange("192.168.1.30"), 'simple single ip matching remote ip' ); +is( 0, in_iprange("192.168.1.31"), 'simple single ip not match remote ip' ); +is( 1, in_iprange("192.168.1.1/24"), 'simple ip range/24 with remote ip in it' ); +is( 0, in_iprange("192.168.2.1/24"), 'simple ip range/24 with remote ip not in it' ); +is( 1, in_iprange("192.168.2.1/16"), 'simple ip range/16 with remote ip in it' ); +is( 0, in_iprange("192.168.1.10-30"), 'invalidly represented IP range with remote ip in it' ); +is( 1, in_iprange("192.168.1.10-192.168.1.30"), 'validly represented ip range with remote ip in it' ); +is( 1, 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' ); +is( 0, 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" ); +is( 1, in_iprange(""), "blank list given, no preference set - implies everything goes through." ); +is( 1, in_iprange(), "no list given, no preference set - implies everything goes through." ); +is( 0, in_iprange("192.168.1.1/36"), 'simple invalid ip range/36 with remote ip in it' ); +is( 1, in_iprange("192.168.1."), 'simple ip range missing last octet with remote ip in it' ); +is( 1, in_iprange("192.168.1.*"), 'simple ip range with "*" with remote ip in it. Undocumented feature; this should not work, but it does.' ); --