From 2d3a19a5ee8d04d546ff5ea0d18ebeaeedd5734c Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Fri, 3 Sep 2021 12:18:03 +0100 Subject: [PATCH] Bug 27358: (QA follow-up) Convert to allow-list This patch converts the code to use an allow-list as aposed to a deny-list. This is more 'fail safe' than requireing maintanence of a deny-list. We also switch to using db fields names for the list as aposed to api mapped names. This way, the list can be re-used for non-api related sanitising if required. Signed-off-by: Martin Renvoize --- Koha/Item.pm | 48 +++++++++++++++++++++++++++++++----------------- Koha/Object.pm | 20 +++++++++++--------- 2 files changed, 42 insertions(+), 26 deletions(-) diff --git a/Koha/Item.pm b/Koha/Item.pm index d42af2bb86..4ac511be2d 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -1104,6 +1104,37 @@ sub _set_found_trigger { return $self; } +=head3 public_read_list + +This method returns the list of publically readable database fields for both API and UI output purposes + +=cut + +sub public_read_list { + return [ + 'itemnumber', 'biblionumber', + 'biblioitemnumber', 'barcode', + 'dateaccessioned', 'booksellerid', + 'homebranch', 'price', + 'replacementprice', 'replacementpricedate', + 'datelastborrowed', 'datelastseen', + 'stack', 'notforloan', + 'damaged', 'damaged_on', + 'itemlost', 'itemlost_on', + 'withdrawn', 'withdrawn_on', + 'itemcallnumber', 'coded_location_qualifier', + 'renewals', 'restricted', + 'itemnotes', 'holdingbranch', + 'timestamp', 'location', + 'permanent_location', 'cn_source', + 'cn_sort', 'ccode', + 'materials', 'uri', + 'itype', 'enumchron', + 'copynumber', 'stocknumber', + 'new_status' + ]; +} + =head3 to_api_mapping This method returns the mapping for representing a Koha::Item object @@ -1160,23 +1191,6 @@ sub to_api_mapping { }; } -=head3 api_privileged_attrs - -This method returns the list of privileged access-only attributes. This is used -by $item->to_api($params) to render the object correctly, based on the passed I<$params>. - -=cut - -sub api_privileged_attrs { - return [ - 'checked_out_date', - 'checkouts_count', - 'holds_count', - 'internal_notes', - 'extended_subfields', - ]; -} - =head3 itemtype my $itemtype = $item->itemtype; diff --git a/Koha/Object.pm b/Koha/Object.pm index 1c2355aa81..d6894e5a61 100644 --- a/Koha/Object.pm +++ b/Koha/Object.pm @@ -24,6 +24,7 @@ use Carp qw( croak ); use Mojo::JSON; use Scalar::Util qw( blessed looks_like_number ); use Try::Tiny qw( catch try ); +use List::MoreUtils qw( any ); use Koha::Database; use Koha::Exceptions::Object; @@ -552,6 +553,16 @@ sub to_api { my ( $self, $params ) = @_; my $json_object = $self->TO_JSON; + # Remove forbidden attributes if required + # FIXME: We should eventually require public_read_list in all objects and drop the conditional here. + if ( $params->{public} + and $self->can('public_read_list') ) + { + for my $field ( keys %{$json_object} ) { + delete $json_object->{$field} unless any { $_ eq $field } $self->public_read_list; + } + } + my $to_api_mapping = $self->to_api_mapping; # Rename attributes if there's a mapping @@ -573,15 +584,6 @@ sub to_api { } } - # Remove forbidden attributes if required - if ( $params->{public} - and $self->can('api_privileged_attrs') ) - { - foreach my $privileged_attr ( @{ $self->api_privileged_attrs } ) { - delete $json_object->{$privileged_attr}; - } - } - # Make sure we duplicate the $params variable to avoid # breaking calls in a loop (Koha::Objects->to_api) $params = {%$params}; -- 2.20.1