Bugzilla – Attachment 161872 Details for
Bug 28657
Expand branches.branchip to allow for multiple space separated IP ranges
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 28657: expand branches.branchip to allow for multiple ip ranges.
0001-Bug-28657-expand-branches.branchip-to-allow-for-mult.patch (text/plain), 13.29 KB, created by
Michael Hafen
on 2024-02-07 23:49:41 UTC
(
hide
)
Description:
Bug 28657: expand branches.branchip to allow for multiple ip ranges.
Filename:
MIME Type:
Creator:
Michael Hafen
Created:
2024-02-07 23:49:41 UTC
Size:
13.29 KB
patch
obsolete
>From a8fcfebc359701933ac8f98dfeaf32f210cf5ed5 Mon Sep 17 00:00:00 2001 >From: Michael Hafen <michael.hafen@washk12.org> >Date: Fri, 2 Jul 2021 11:03:08 -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). >Change note in admin/branches.tt under branchip field to reflect changes to branchip handling. >Bug 28657: (QA follow-up) Return 1 if no range, improve unit tests >Bug 28657: (Follow-up for QA) Tidy the code and make it more consident > 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. > >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 | 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 > >diff --git a/C4/Auth.pm b/C4/Auth.pm >index d38a254d4a..f0e7d1a2ff 100644 >--- a/C4/Auth.pm >+++ b/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 { >diff --git a/Koha/Library.pm b/Koha/Library.pm >index decfa13617..cf15f94540 100644 >--- a/Koha/Library.pm >+++ b/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) { >diff --git a/installer/data/mysql/atomicupdate/bug_28657.pl b/installer/data/mysql/atomicupdate/bug_28657.pl >new file mode 100755 >index 0000000000..08382b266a >--- /dev/null >+++ b/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 '*'"; >+ }, >+}; >diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql >index 16310f0f44..db47669e8a 100644 >--- a/installer/data/mysql/kohastructure.sql >+++ b/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', >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 6fdb1ad4f1..04a775d4eb 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/branches.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/branches.tt >@@ -249,8 +249,8 @@ > </li> > <li> > <label for="branchip">IP: </label> >- <input type="text" name="branchip" id="branchip" size="15" maxlength="15" value="[% library.branchip | html %]" /> >- <div class="hint">Can be entered as a single IP, or a subnet such as 192.168.1.*</div> >+ <input type="text" name="branchip" id="branchip" size="60" value="[% library.branchip | html %]" /> >+ <div class="hint">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.</div> > </li> > <li> > <label for="marcorgcode">MARC organization code</label> >diff --git a/t/Auth.t b/t/Auth.t >index d5b2f1e7fa..6881734583 100755 >--- a/t/Auth.t >+++ b/t/Auth.t >@@ -16,22 +16,24 @@ > # along with Koha; if not, see <http://www.gnu.org/licenses>. > > 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.' ); >-- >2.34.1 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 28657
:
122572
|
134126
|
134173
|
149412
|
151484
|
151485
|
156582
| 161872