From 8528009910534ca4c29342265abc2e993bb12f7d Mon Sep 17 00:00:00 2001 From: David Cook Date: Wed, 1 Sep 2021 06:31:00 +0000 Subject: [PATCH] [Alternate] Bug 28929: Filter user input through allowlist for patrons This patch creates public and staff allowlists that are used to filter the user input from the Staff Interface, OPAC, and OPAC self-registration. The lists are hard-coded into modules, but they could be generated by other means in the future to make them more suited to user context. Test plan: 0. The following should be done for the Staff Interface, a logged in OPAC user, and a OPAC self-registration user 1. Go to a patron create/edit screen 2. Using F12 dev tools in your browser, add an input for "flags" or "borrower_flags" depending on the other inputs available on that screen. Give it a value of "1". 3. Submit the change 4. Check the Staff Interface or database to confirm that the flags have not been set to 1 for that user Additional test plan: 1. prove t/Koha/Patron/Allowlist.t 2. Note that all tests pass --- Koha/Patron/Allowlist.pm | 54 +++++++++++++++++++++++++++++++ Koha/Patron/Allowlist/Public.pm | 65 ++++++++++++++++++++++++++++++++++++++ Koha/Patron/Allowlist/Staff.pm | 70 +++++++++++++++++++++++++++++++++++++++++ members/memberentry.pl | 6 ++++ opac/opac-memberentry.pl | 12 ++++++- t/Koha/Patron/Allowlist.t | 48 ++++++++++++++++++++++++++++ 6 files changed, 254 insertions(+), 1 deletion(-) create mode 100644 Koha/Patron/Allowlist.pm create mode 100644 Koha/Patron/Allowlist/Public.pm create mode 100644 Koha/Patron/Allowlist/Staff.pm create mode 100644 t/Koha/Patron/Allowlist.t diff --git a/Koha/Patron/Allowlist.pm b/Koha/Patron/Allowlist.pm new file mode 100644 index 0000000000..bb01a071be --- /dev/null +++ b/Koha/Patron/Allowlist.pm @@ -0,0 +1,54 @@ +package Koha::Patron::Allowlist; + +use Modern::Perl; + +=head1 NAME + +Koha::Patron::Allowlist - Allowlist implementation for Koha::Patron + +=head1 API + +=head2 Class Methods + +=head3 new + +=cut + +sub new { + my ($class, $args) = @_; + $args = {} unless defined $args; + my $self = bless ($args, $class); + $self->{ui_fields} = $self->_get_ui_fields() unless $self->{ui_fields}; + return $self; +} + +=head3 apply + +Apply an allowlist to input data + +=cut + +sub apply { + my ( $self, $params ) = @_; + my $blocked = {}; + my $ui_fields = $self->{ui_fields}; + if ($ui_fields && ref $ui_fields eq 'HASH'){ + my $input = $params->{input}; + if ( $input && ref $input eq 'HASH' ){ + my @keys = keys %$input; + foreach my $key (@keys){ + if ( ! $ui_fields->{ $key } ){ + #NOTE: We capture the deleted data so that it can be used for logging purposes + $blocked->{$key} = delete $input->{ $key }; + } + } + } + } + return $blocked; +} + +sub _get_ui_fields { + return undef; +} + +1; diff --git a/Koha/Patron/Allowlist/Public.pm b/Koha/Patron/Allowlist/Public.pm new file mode 100644 index 0000000000..b63daf16d3 --- /dev/null +++ b/Koha/Patron/Allowlist/Public.pm @@ -0,0 +1,65 @@ +package Koha::Patron::Allowlist::Public; + +use parent 'Koha::Patron::Allowlist'; + +sub _get_ui_fields { + my $ui_fields = { + 'cardnumber' => 1, + 'surname' => 1, + 'firstname' => 1, + 'title' => 1, + 'othernames' => 1, + 'initials' => 1, + 'streetnumber' => 1, + 'streettype' => 1, + 'address' => 1, + 'address2' => 1, + 'city' => 1, + 'state' => 1, + 'zipcode' => 1, + 'country' => 1, + 'email' => 1, + 'phone' => 1, + 'mobile' => 1, + 'fax' => 1, + 'emailpro' => 1, + 'phonepro' => 1, + 'B_streetnumber' => 1, + 'B_streettype' => 1, + 'B_address' => 1, + 'B_address2' => 1, + 'B_city' => 1, + 'B_state' => 1, + 'B_zipcode' => 1, + 'B_country' => 1, + 'B_email' => 1, + 'B_phone' => 1, + 'dateofbirth' => 1, + 'dateenrolled' => undef, + 'dateexpiry' => undef, + 'branchcode' => 1, + 'categorycode' => 1, + 'contactname' => 1, + 'contactfirstname' => 1, + 'borrowernotes' => undef, + 'sex' => 1, + 'password' => 1, + 'userid' => 1, + 'opacnote' => undef, + 'contactnote' => 1, + 'altcontactfirstname' => 1, + 'altcontactsurname' => 1, + 'altcontactaddress1' => 1, + 'altcontactaddress2' => 1, + 'altcontactaddress3' => 1, + 'altcontactstate' => 1, + 'altcontactzipcode' => 1, + 'altcontactcountry' => 1, + 'altcontactphone' => 1, + 'smsalertnumber' => 1, + 'primary_contact_method' => 1, + }; + return $ui_fields; +} + +1; diff --git a/Koha/Patron/Allowlist/Staff.pm b/Koha/Patron/Allowlist/Staff.pm new file mode 100644 index 0000000000..2135c6ae83 --- /dev/null +++ b/Koha/Patron/Allowlist/Staff.pm @@ -0,0 +1,70 @@ +package Koha::Patron::Allowlist::Staff; + +use parent 'Koha::Patron::Allowlist'; + +sub _get_ui_fields { + my $ui_fields = { + 'cardnumber' => 1, + 'surname' => 1, + 'firstname' => 1, + 'title' => 1, + 'othernames' => 1, + 'initials' => 1, + 'streetnumber' => 1, + 'streettype' => 1, + 'address' => 1, + 'address2' => 1, + 'city' => 1, + 'state' => 1, + 'zipcode' => 1, + 'country' => 1, + 'email' => 1, + 'phone' => 1, + 'mobile' => 1, + 'fax' => 1, + 'emailpro' => 1, + 'phonepro' => 1, + 'B_streetnumber' => 1, + 'B_streettype' => 1, + 'B_address' => 1, + 'B_address2' => 1, + 'B_city' => 1, + 'B_state' => 1, + 'B_zipcode' => 1, + 'B_country' => 1, + 'B_email' => 1, + 'B_phone' => 1, + 'dateofbirth' => 1, + 'dateenrolled' => 1, + 'dateexpiry' => 1, + 'branchcode' => 1, + 'categorycode' => 1, + 'contactname' => 1, + 'contactfirstname' => 1, + 'borrowernotes' => 1, + 'sex' => 1, + 'password' => 1, + 'userid' => 1, + 'opacnote' => 1, + 'contactnote' => 1, + 'altcontactfirstname' => 1, + 'altcontactsurname' => 1, + 'altcontactaddress1' => 1, + 'altcontactaddress2' => 1, + 'altcontactaddress3' => 1, + 'altcontactstate' => 1, + 'altcontactzipcode' => 1, + 'altcontactcountry' => 1, + 'altcontactphone' => 1, + 'smsalertnumber' => 1, + 'primary_contact_method' => 1, + 'autorenew_checkouts' => 1, + 'sort1' => 1, + 'sort2' => 1, + 'gonenoaddress' => 1, + 'lost' => 1, + }; + return $ui_fields; +} + +1; diff --git a/members/memberentry.pl b/members/memberentry.pl index 5844b9c3f9..8e20372334 100755 --- a/members/memberentry.pl +++ b/members/memberentry.pl @@ -46,6 +46,10 @@ use Koha::Patron::HouseboundRoles; use Koha::Token; use Email::Valid; use Koha::SMS::Providers; +use Koha::Patron::Allowlist::Staff; + +my $ui_allowlist = Koha::Patron::Allowlist::Staff->new(); + my $input = CGI->new; my %data; @@ -429,6 +433,8 @@ if ( defined $sms ) { $newdata{smsalertnumber} = $sms; } +$ui_allowlist->apply({ input => \%newdata }); + ### Error checks should happen before this line. $nok = $nok || scalar(@errors); if ((!$nok) and $nodouble and ($op eq 'insert' or $op eq 'save')){ diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl index 8945cc339b..16c422f609 100755 --- a/opac/opac-memberentry.pl +++ b/opac/opac-memberentry.pl @@ -45,6 +45,11 @@ use Koha::Patron::Modifications; use Koha::Patron::Categories; use Koha::Token; use Koha::AuthorisedValues; + +use Koha::Patron::Allowlist::Public; + +my $ui_allowlist = Koha::Patron::Allowlist::Public->new(); + my $cgi = CGI->new; my $dbh = C4::Context->dbh; @@ -127,7 +132,11 @@ if ( $action eq 'create' ) { my @empty_mandatory_fields = (CheckMandatoryFields( \%borrower, $action ), CheckMandatoryAttributes( \%borrower, $attributes ) ); my $invalidformfields = CheckForInvalidFields(\%borrower); + my $consent_dt = delete $borrower{gdpr_proc_consent}; delete $borrower{'password2'}; + + $ui_allowlist->apply({ input => \%borrower }); + my $cardnumber_error_code; if ( !grep { $_ eq 'cardnumber' } @empty_mandatory_fields ) { # No point in checking the cardnumber if it's missing and mandatory, it'll just generate a @@ -218,7 +227,6 @@ if ( $action eq 'create' ) { ); $borrower{password} ||= Koha::AuthUtils::generate_password(Koha::Patron::Categories->find($borrower{categorycode})); - my $consent_dt = delete $borrower{gdpr_proc_consent}; my $patron = Koha::Patron->new( \%borrower )->store; Koha::Patron::Consent->new({ borrowernumber => $patron->borrowernumber, type => 'GDPR_PROCESSING', given_on => $consent_dt })->store if $consent_dt; if ( $patron ) { @@ -266,6 +274,8 @@ elsif ( $action eq 'update' ) { # Send back the data to the template %borrower = ( %$borrower, %borrower ); + $ui_allowlist->apply({ input => \%borrower }); + if (@empty_mandatory_fields || @$invalidformfields) { $template->param( empty_mandatory_fields => \@empty_mandatory_fields, diff --git a/t/Koha/Patron/Allowlist.t b/t/Koha/Patron/Allowlist.t new file mode 100644 index 0000000000..7a1cd7e4cf --- /dev/null +++ b/t/Koha/Patron/Allowlist.t @@ -0,0 +1,48 @@ +#!/usr/bin/perl + +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see . + +use Modern::Perl; + +use Test::More tests => 4; + +use Test::MockModule; +use Test::Exception; + +use t::lib::Mocks; + +use_ok('Koha::Patron::Allowlist::Staff'); +use_ok('Koha::Patron::Allowlist::Public'); + +subtest 'Staff Allowlist' => sub { + plan tests => 3; + my $input = { firstname => 'test firstname', surname => 'test surname', flags => '1' }; + my $allowlist = Koha::Patron::Allowlist::Staff->new(); + $allowlist->apply({ input => $input, }); + ok( $input->{firstname} eq 'test firstname', 'firstname preserved' ); + ok( $input->{surname} eq 'test surname', 'surname preserved' ); + is( $input->{flags}, undef, 'flags filtered' ); +}; + +subtest 'Public Allowlist' => sub { + plan tests => 3; + my $input = { firstname => 'test firstname', surname => 'test surname', flags => '1' }; + my $allowlist = Koha::Patron::Allowlist::Public->new(); + $allowlist->apply({ input => $input, }); + ok( $input->{firstname} eq 'test firstname', 'firstname preserved' ); + ok( $input->{surname} eq 'test surname', 'surname preserved' ); + is( $input->{flags}, undef, 'flags filtered' ); +}; -- 2.11.0