Attachment 124749 Details for Bug 28948 – Bug 28948: (follow-up) Use ::Allowlist structure

[patch] Bug 28948: (follow-up) Use ::Allowlist structure

Bug-28948-follow-up-Use-Allowlist-structure.patch (text/plain), 5.79 KB, created by Martin Renvoize (ashimema) on 2021-09-10 10:24:01 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2021-09-10 10:24:01 UTC

Size: 5.79 KB

patch

obsolete

>From 6de57b54a3e6e3a0202ad5082ff934c65e6c6a1e Mon Sep 17 00:00:00 2001
>From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>Date: Fri, 10 Sep 2021 11:21:08 +0100
>Subject: [PATCH] Bug 28948: (follow-up) Use ::Allowlist structure
>
>This patch updates the patchset to use the ::Allowlist structure
>introduced with bug 28935.  I think we need more work here to provide
>for 'read' and 'write' allowlists.. we also need a way to alter the
>allowlist from the Koha object when required as with the load time at
>to_api here we currently have no way to pass overrides to the allowlist.
>---
> Koha/Library.pm               | 15 ---------------
> Koha/Library/Allowlist.pm     | 22 ++++++++++++++++++++++
> Koha/Object.pm                | 25 +++++++++++++++++--------
> t/db_dependent/Koha/Object.t  |  4 +++-
> t/db_dependent/Koha/Objects.t |  4 +++-
> 5 files changed, 45 insertions(+), 25 deletions(-)
> create mode 100644 Koha/Library/Allowlist.pm
>
>diff --git a/Koha/Library.pm b/Koha/Library.pm
>index 125a11f1f1..fa98a4fc6e 100644
>--- a/Koha/Library.pm
>+++ b/Koha/Library.pm
>@@ -266,21 +266,6 @@ sub validate_hold_sibling {
>       ->count > 0;
> }
> 
>-=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 [
>-        'branchcode',     'branchname',     'branchaddress1',
>-        'branchaddress2', 'branchaddress3', 'branchzip',
>-        'branchcity',     'branchstate',    'branchcountry',
>-        'branchfax',      'branchemail',    'branchurl'
>-    ];
>-}
>-
> =head3 to_api_mapping
> 
> This method returns the mapping for representing a Koha::Library object
>diff --git a/Koha/Library/Allowlist.pm b/Koha/Library/Allowlist.pm
>new file mode 100644
>index 0000000000..c35fceab0a
>--- /dev/null
>+++ b/Koha/Library/Allowlist.pm
>@@ -0,0 +1,22 @@
>+package Koha::Patron::Allowlist;
>+
>+use Modern::Perl;
>+use parent 'Koha::Allowlist';
>+
>+sub load {
>+    my ($self) = @_;
>+    my @public_read = qw(
>+      branchcode     branchname     branchaddress1
>+      branchaddress2 branchaddress3 branchzip
>+      branchcity     branchstate    branchcountry
>+      branchfax      branchemail    branchurl );
>+    $self->add(@public_read);
>+
>+    my @staff_read = qw(
>+      branchillemail branchreplyto branchreturnpath
>+      branchip       branchnotes   marcorgcode
>+    );
>+    $self->add(@staff_read) if $self->{interface} eq 'staff';
>+}
>+
>+1;
>diff --git a/Koha/Object.pm b/Koha/Object.pm
>index 6ebf7870be..c845786518 100644
>--- a/Koha/Object.pm
>+++ b/Koha/Object.pm
>@@ -24,7 +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 Module::Load::Conditional qw( can_load );
> 
> use Koha::Database;
> use Koha::Exceptions::Object;
>@@ -552,15 +552,24 @@ Returns a representation of the object, suitable for API output.
> sub to_api {
>     my ( $self, $params ) = @_;
>     my $json_object = $self->TO_JSON;
>+    $params //= {};
> 
>     # 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};
>-        }
>+    # FIXME: All modules should have a corresponding allowlist eventually
>+    my $allowclass = ref($self) . '::Allowlist';
>+    if ( can_load( modules => { $allowclass => undef } ) ) {
>+        my $allowlist = $allowclass->new(
>+            { interface => $params->{public} ? 'opac' : 'staff' } );
>+        $allowlist->load;
>+        my $blocked = $allowlist->apply( { input => $json_object } );
>+        Koha::Logger->get->debug(
>+                ref($self)
>+              . "::to_api allowlist blocked fields: "
>+              . (
>+                join ', ',
>+                map { $_ . '=' . $blocked->{$_} } sort keys %$blocked
>+              )
>+        ) if keys %$blocked;
>     }
> 
>     my $to_api_mapping = $self->to_api_mapping;
>diff --git a/t/db_dependent/Koha/Object.t b/t/db_dependent/Koha/Object.t
>index 48a71db117..d89f2c8973 100755
>--- a/t/db_dependent/Koha/Object.t
>+++ b/t/db_dependent/Koha/Object.t
>@@ -31,6 +31,7 @@ use Koha::Database;
> use Koha::Acquisition::Orders;
> use Koha::DateUtils qw( dt_from_string );
> use Koha::Libraries;
>+use Koha::Library::Allowlist;
> use Koha::Patrons;
> use Koha::ApiKeys;
> 
>@@ -376,7 +377,8 @@ subtest "to_api() tests" => sub {
>     subtest 'unprivileged request tests' => sub {
> 
>         my @all_attrs = Koha::Libraries->columns();
>-        my $public_attrs = { map { $_ => 1 } @{ Koha::Library->public_read_list() } };
>+        my $allowlist = Koha::Library::Allowlist->new({ interface => 'opac' })->load();
>+        my $public_attrs = $allowlist->{_entries};
>         my $mapping = Koha::Library->to_api_mapping;
> 
>         plan tests => scalar @all_attrs * 2;
>diff --git a/t/db_dependent/Koha/Objects.t b/t/db_dependent/Koha/Objects.t
>index 34385dc7f4..e5168f41c4 100755
>--- a/t/db_dependent/Koha/Objects.t
>+++ b/t/db_dependent/Koha/Objects.t
>@@ -32,6 +32,7 @@ use Koha::Biblios;
> use Koha::Patron::Category;
> use Koha::Patron::Categories;
> use Koha::Patrons;
>+use Koha::Library::Allowlist;
> use Koha::Database;
> use Koha::DateUtils qw( dt_from_string );
> 
>@@ -401,7 +402,8 @@ subtest "to_api() tests" => sub {
>     subtest 'unprivileged request tests' => sub {
> 
>         my @all_attrs = Koha::Libraries->columns();
>-        my $public_attrs = { map { $_ => 1 } @{ Koha::Library->public_read_list() } };
>+        my $allowlist = Koha::Library::Allowlist->new({ interface => 'opac' })->load();
>+        my $public_attrs = $allowlist->{_entries};
>         my $mapping = Koha::Library->to_api_mapping;
> 
>         # Create sample libraries
>-- 
>2.20.1

Actions: View | Diff | Splinter Review

Attachments on bug 28948: 124479 | 124480 | 124481 | 124482 | 124483 | 124749 | 124911 | 124914 | 125901 | 125902 | 125903 | 125904 | 125905 | 125906 | 125907 | 125908 | 125911 | 125912 | 126111 | 126112 | 126113 | 126114 | 126115 | 126116 | 126117 | 126118 | 126119 | 126120 | 126121 | 126122 | 126123 | 126124 | 126125 | 126126 | 127054 | 127055