Bugzilla – Attachment 124422 Details for
Bug 28935
No filtering on patron's data on member entry pages (OPAC, self registration, staff interface)
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 28935: Move logic to Koha::Allowlist for reusability
Bug-28935-Move-logic-to-KohaAllowlist-for-reusabil.patch (text/plain), 9.66 KB, created by
Jonathan Druart
on 2021-09-02 09:39:21 UTC
(
hide
)
Description:
Bug 28935: Move logic to Koha::Allowlist for reusability
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2021-09-02 09:39:21 UTC
Size:
9.66 KB
patch
obsolete
>From 10a49a4dbd0f154c08b4ca6c15cd9cf8f3ced069 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Thu, 2 Sep 2021 11:37:45 +0200 >Subject: [PATCH] Bug 28935: Move logic to Koha::Allowlist for reusability > >--- > Koha/Allowlist.pm | 93 +++++++++++++++++++++++ > Koha/Patron/Allowlist.pm | 100 ++++++------------------- > Koha/Patron/Allowlist/Public.pm | 21 ------ > Koha/Patron/Allowlist/Staff.pm | 9 --- > members/memberentry.pl | 2 +- > opac/opac-memberentry.pl | 4 +- > t/db_dependent/Koha/Patron/Allowlist.t | 11 +-- > 7 files changed, 121 insertions(+), 119 deletions(-) > create mode 100644 Koha/Allowlist.pm > delete mode 100644 Koha/Patron/Allowlist/Public.pm > delete mode 100644 Koha/Patron/Allowlist/Staff.pm > >diff --git a/Koha/Allowlist.pm b/Koha/Allowlist.pm >new file mode 100644 >index 00000000000..d0c67b7758d >--- /dev/null >+++ b/Koha/Allowlist.pm >@@ -0,0 +1,93 @@ >+package Koha::Allowlist; >+ >+use Modern::Perl; >+use Array::Utils qw( array_minus ); >+ >+=head1 NAME >+ >+Koha::Allowlist - Allowlist implementation base class >+ >+=head1 API >+ >+=head2 Class Methods >+ >+ my $allow_list Koha::Allowlist->new({ interface => 'staff' }); >+ >+=head3 new >+ >+Constructor. >+ >+Interface must be 'staff' or 'opac' - default to 'opac' >+ >+=cut >+ >+sub new { >+ my ($class, $args) = @_; >+ $args = {} unless defined $args; >+ my $self = bless ($args, $class); >+ $self->{interface} = >+ ( $args->{interface} && $args->{interface} eq 'staff' ) >+ ? 'staff' >+ : 'opac'; >+ return $self; >+} >+ >+=head3 apply >+ >+ my $ui_fields = Koha::Allowlist->new({ interface => 'staff' })->apply({ input => $hashref, additional_deny_list => [qw( list of fields )] }); >+ >+ >+Apply an allowlist to input data. >+ >+=cut >+ >+sub apply { >+ my ( $self, $params ) = @_; >+ >+ my $input = $params->{input}; >+ my $additional_deny_list = $params->{additional_deny_list} || []; >+ >+ my $blocked = {}; >+ my $ui_fields = { map { $_ => 1 } $self->get_ui_fields() }; >+ return unless $ui_fields and %$ui_fields; >+ >+ delete $ui_fields->{$_} for @$additional_deny_list; >+ >+ return unless $input and %$input; >+ >+ my @keys = keys %$input; >+ foreach my $key (@keys){ >+ unless ( exists $ui_fields->{ $key } ) { >+ #NOTE: We capture the deleted data so that it can be used for logging purposes >+ $blocked->{$key} = delete $input->{ $key }; >+ } >+ } >+ >+ if ( %$blocked ) { >+ while ( my ($k, $v)=each %$blocked){ >+ # FIXME We should raise an exception from here >+ my @c = caller; >+ warn sprintf "Forbidden - Tried to modify '%s' with '%s' from %s", $k, $v, "@c"; >+ } >+ } >+ >+ return $blocked; >+} >+ >+=head3 get_ui_fields >+ >+ my $ui_fields = $self->get_ui_fields(); >+ my $ui_fields = Koha::Patron::Allowlist::Public->new->get_ui_fields >+ >+=cut >+ >+sub get_ui_fields { >+ my ($self) = @_; >+ my @all_fields = $self->_get_all_fields(); >+ my @global_deny_list = $self->_global_deny_list(); >+ my @deny_list = ( $self->{interface} eq 'staff' ) ? $self->_deny_list_staff() : $self->_deny_list_opac(); >+ my @global_allow_list = array_minus @all_fields, @global_deny_list; >+ return array_minus @global_allow_list, @deny_list; >+} >+ >+1; >diff --git a/Koha/Patron/Allowlist.pm b/Koha/Patron/Allowlist.pm >index 485aeb81e53..259d61c9283 100644 >--- a/Koha/Patron/Allowlist.pm >+++ b/Koha/Patron/Allowlist.pm >@@ -1,84 +1,6 @@ > package Koha::Patron::Allowlist; > >-use Modern::Perl; >-use Array::Utils qw( array_minus ); >- >-=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} = { map { $_ => 1 } $self->get_ui_fields() }; >- return $self; >-} >- >-=head3 apply >- >- my $ui_fields = Koha::Patron::Allowlist::Public->new->apply({ input => $hashref, additional_deny_list => [qw( list of fields )] }); >- >-Apply an allowlist to input data. >- >-=cut >- >-sub apply { >- my ( $self, $params ) = @_; >- >- my $input = $params->{input}; >- my $additional_deny_list = $params->{additional_deny_list} || []; >- >- my $blocked = {}; >- my $ui_fields = $self->{ui_fields}; >- return unless $ui_fields and %$ui_fields; >- >- delete $ui_fields->{$_} for @$additional_deny_list; >- >- return unless $input and %$input; >- >- my @keys = keys %$input; >- foreach my $key (@keys){ >- unless ( exists $ui_fields->{ $key } ) { >- #NOTE: We capture the deleted data so that it can be used for logging purposes >- $blocked->{$key} = delete $input->{ $key }; >- } >- } >- >- if ( %$blocked ) { >- while ( my ($k, $v)=each %$blocked){ >- # FIXME We should raise an exception from here >- my @c = caller; >- warn sprintf "Forbidden - Tried to modify '%s' with '%s' from %s", $k, $v, "@c"; >- } >- } >- >- return $blocked; >-} >- >-=head3 get_ui_fields >- >- my $ui_fields = $self->get_ui_fields(); >- my $ui_fields = Koha::Patron::Allowlist::Public->new->get_ui_fields >- >-=cut >- >-sub get_ui_fields { >- my ($self) = @_; >- my @all_fields = $self->_get_all_fields(); >- my @global_deny_list = $self->_global_deny_list(); >- my @deny_list = $self->_deny_list(); >- my @global_allow_list = array_minus @all_fields, @global_deny_list; >- return array_minus @global_allow_list, @deny_list; >-} >+use parent 'Koha::Allowlist'; > > # This must not be replaced by Koha::Patrons->columns > # We want developper to not forget to adjust it manually and add new attribute to the deny list if needed >@@ -184,4 +106,24 @@ sub _global_deny_list { > ); > } > >+sub _deny_list_opac { >+ return qw( >+ dateenrolled >+ dateexpiry >+ gonenoaddress >+ lost >+ borrowernotes >+ relationship >+ opacnote >+ sort1 >+ sort2 >+ sms_provider_id >+ autorenew_checkouts >+ ); >+} >+ >+sub _deny_list_staff { >+ return qw(); >+} >+ > 1; >diff --git a/Koha/Patron/Allowlist/Public.pm b/Koha/Patron/Allowlist/Public.pm >deleted file mode 100644 >index 01742779849..00000000000 >--- a/Koha/Patron/Allowlist/Public.pm >+++ /dev/null >@@ -1,21 +0,0 @@ >-package Koha::Patron::Allowlist::Public; >- >-use parent 'Koha::Patron::Allowlist'; >- >-sub _deny_list { >- return qw( >- dateenrolled >- dateexpiry >- gonenoaddress >- lost >- borrowernotes >- relationship >- opacnote >- sort1 >- sort2 >- sms_provider_id >- autorenew_checkouts >- ); >-} >- >-1; >diff --git a/Koha/Patron/Allowlist/Staff.pm b/Koha/Patron/Allowlist/Staff.pm >deleted file mode 100644 >index 906e167d82c..00000000000 >--- a/Koha/Patron/Allowlist/Staff.pm >+++ /dev/null >@@ -1,9 +0,0 @@ >-package Koha::Patron::Allowlist::Staff; >- >-use parent 'Koha::Patron::Allowlist'; >- >-sub _deny_list { >- return (); >-} >- >-1; >diff --git a/members/memberentry.pl b/members/memberentry.pl >index 6b63ee7650b..0fc3d2b022b 100755 >--- a/members/memberentry.pl >+++ b/members/memberentry.pl >@@ -48,7 +48,7 @@ use Email::Valid; > use Koha::SMS::Providers; > use Koha::Patron::Allowlist::Staff; > >-my $ui_allowlist = Koha::Patron::Allowlist::Staff->new(); >+my $ui_allowlist = Koha::Patron::Allowlist->new({ interface => 'staff' }); > > > my $input = CGI->new; >diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl >index c1f35fe63fe..3c3d9434aef 100755 >--- a/opac/opac-memberentry.pl >+++ b/opac/opac-memberentry.pl >@@ -46,9 +46,9 @@ use Koha::Patron::Categories; > use Koha::Token; > use Koha::AuthorisedValues; > >-use Koha::Patron::Allowlist::Public; >+use Koha::Patron::Allowlist; > >-my $ui_allowlist = Koha::Patron::Allowlist::Public->new(); >+my $ui_allowlist = Koha::Patron::Allowlist->new(); > > my $cgi = CGI->new; > my $dbh = C4::Context->dbh; >diff --git a/t/db_dependent/Koha/Patron/Allowlist.t b/t/db_dependent/Koha/Patron/Allowlist.t >index 5327f54e411..edec69c678e 100644 >--- a/t/db_dependent/Koha/Patron/Allowlist.t >+++ b/t/db_dependent/Koha/Patron/Allowlist.t >@@ -17,7 +17,7 @@ > > use Modern::Perl; > >-use Test::More tests => 6; >+use Test::More tests => 4; > > use Test::MockModule; > use Test::Exception; >@@ -28,9 +28,6 @@ use Koha::Patron::Allowlist; > > use t::lib::Mocks; > >-use_ok('Koha::Patron::Allowlist::Staff'); >-use_ok('Koha::Patron::Allowlist::Public'); >- > subtest '_get_all_fields' => sub { > my @columns = Koha::Patrons->columns; > is_deeply( >@@ -43,7 +40,7 @@ subtest '_get_all_fields' => sub { > subtest 'Staff Allowlist' => sub { > plan tests => 4; > my $input = { firstname => 'test firstname', surname => 'test surname', flags => '1' }; >- my $allowlist = Koha::Patron::Allowlist::Staff->new(); >+ my $allowlist = Koha::Patron::Allowlist->new({ interface => 'staff' }); > warning_like { > $allowlist->apply({ input => $input, }); > } qr{Forbidden - Tried to modify 'flags' with '1' from}; >@@ -55,7 +52,7 @@ subtest 'Staff Allowlist' => sub { > subtest 'Public Allowlist' => sub { > plan tests => 4; > my $input = { firstname => 'test firstname', surname => 'test surname', flags => '1' }; >- my $allowlist = Koha::Patron::Allowlist::Public->new(); >+ my $allowlist = Koha::Patron::Allowlist->new(); > warning_like { > $allowlist->apply({ input => $input, }); > } qr{Forbidden - Tried to modify 'flags' with '1' from}; >@@ -69,7 +66,7 @@ subtest 'additional_deny_list' => sub { > plan tests => 10; > > my $input = { firstname => 'test firstname', surname => 'test surname' }; >- my $allowlist = Koha::Patron::Allowlist::Public->new(); >+ my $allowlist = Koha::Patron::Allowlist->new(); > > $allowlist->apply({ > input => $input, >-- >2.25.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 28935
:
124326
|
124327
|
124350
|
124416
|
124418
|
124422
|
124425
|
124446
|
124447
|
124448
|
124449
|
124450
|
124461
|
124462
|
124499
|
124500
|
124501
|
124502
|
124503
|
124523
|
124524
|
124525
|
124526
|
124527
|
124528
|
124529
|
124530
|
124531
|
124532
|
124640
|
124641
|
124642
|
124643
|
124644
|
124645
|
124646
|
124647
|
124648
|
124649
|
124650
|
124730
|
124731
|
124732
|
124733
|
124734
|
124735
|
124736
|
124737
|
124738
|
124739
|
124740
|
124769
|
124784
|
124832
|
124833
|
124837
|
124838
|
124839
|
124840