From 57ccb8bd36bd12a2c5b2c154ec6d15db723701b7 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Wed, 6 Jan 2021 16:16:01 -0300 Subject: [PATCH] Bug 27358: Add a generic way to handle API privileged access attributes deny-list This patch introduces a way for Koha::Object(s)->to_api to filter out attributes that require privileged access. It is done in a way that the 'public' parameter is recursively passed to nested objects in recursive to_api() calls. This way, Koha::Object-based classes can determine how they will render depending on this parameter. For example, for implementing a route for fetching an item looks like: GET /items The controller will look like: my $item = Koha::Items->find( $c->validation->param('item_id') ); return $c->render( status => 200, openapi => $item->to_api ); Implementing an unprivileged (public) route would look like: GET /public/items/:item_id The controller will look like: my $item = Koha::Items->find( $c->validation->param('item_id') ); return $c->render( status => 200, openapi => $item->to_api({ public => 1 }) ); To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Object*.t => SUCCESS: Tests pass (i.e. current behaviour is kept, new behaviour passes the tests) 3. Sign off :-D Signed-off-by: Martin Renvoize --- Koha/Item.pm | 17 ++++++++++++++ Koha/Object.pm | 43 +++++++++++++++++++++++++++-------- t/db_dependent/Koha/Object.t | 22 +++++++++++++++++- t/db_dependent/Koha/Objects.t | 30 +++++++++++++++++++++++- 4 files changed, 101 insertions(+), 11 deletions(-) diff --git a/Koha/Item.pm b/Koha/Item.pm index 5c9d86e49a..80b32e640f 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -1040,6 +1040,23 @@ 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 57b1429b89..df5898e2a9 100644 --- a/Koha/Object.pm +++ b/Koha/Object.pm @@ -539,6 +539,7 @@ sub prefetch_whitelist { ... } }, + public => 0|1, ... ] } @@ -573,7 +574,19 @@ sub to_api { } } - my $embeds = $params->{embed}; + # 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}; + my $embeds = delete $params->{embed}; if ($embeds) { foreach my $embed ( keys %{$embeds} ) { @@ -592,20 +605,30 @@ sub to_api { if ( defined $children and ref($children) eq 'ARRAY' ) { my @list = map { $self->_handle_to_api_child( - { child => $_, next => $next, curr => $curr } ) + { + child => $_, + next => $next, + curr => $curr, + params => $params + } + ) } @{$children}; $json_object->{$curr} = \@list; } else { $json_object->{$curr} = $self->_handle_to_api_child( - { child => $children, next => $next, curr => $curr } ); + { + child => $children, + next => $next, + curr => $curr, + params => $params + } + ); } } } } - - return $json_object; } @@ -850,9 +873,10 @@ sub _type { } sub _handle_to_api_child { my ($self, $args ) = @_; - my $child = $args->{child}; - my $next = $args->{next}; - my $curr = $args->{curr}; + my $child = $args->{child}; + my $next = $args->{next}; + my $curr = $args->{curr}; + my $params = $args->{params}; my $res; @@ -862,7 +886,8 @@ sub _handle_to_api_child { if defined $next and blessed $child and !$child->can('to_api'); if ( blessed $child ) { - $res = $child->to_api({ embed => $next }); + $params->{embed} = $next; + $res = $child->to_api($params); } else { $res = $child; diff --git a/t/db_dependent/Koha/Object.t b/t/db_dependent/Koha/Object.t index b78283ce25..2c0c1de57c 100755 --- a/t/db_dependent/Koha/Object.t +++ b/t/db_dependent/Koha/Object.t @@ -223,7 +223,7 @@ subtest 'TO_JSON tests' => sub { subtest "to_api() tests" => sub { - plan tests => 28; + plan tests => 29; $schema->storage->txn_begin; @@ -373,6 +373,26 @@ subtest "to_api() tests" => sub { 'Koha::Exceptions::Object::MethodNotCoveredByTests', 'Unknown method exception thrown if is_count not specified'; + subtest 'unprivileged request tests' => sub { + + my @privileged_attrs = @{ Koha::Item->api_privileged_attrs }; + + plan tests => scalar @privileged_attrs * 2; + + # Create a sample item + my $item = $builder->build_sample_item(); + + my $unprivileged_representation = $item->to_api({ public => 1 }); + my $privileged_representation = $item->to_api; + + foreach my $privileged_attr ( @privileged_attrs ) { + ok( exists $privileged_representation->{$privileged_attr}, + "Attribute $privileged_attr' is present" ); + ok( !exists $unprivileged_representation->{$privileged_attr}, + "Attribute '$privileged_attr' is not present" ); + } + }; + $schema->storage->txn_rollback; }; diff --git a/t/db_dependent/Koha/Objects.t b/t/db_dependent/Koha/Objects.t index afb00ee142..42c190172e 100755 --- a/t/db_dependent/Koha/Objects.t +++ b/t/db_dependent/Koha/Objects.t @@ -313,7 +313,7 @@ subtest '->search() tests' => sub { subtest "to_api() tests" => sub { - plan tests => 18; + plan tests => 19; $schema->storage->txn_begin; @@ -396,6 +396,34 @@ subtest "to_api() tests" => sub { $i++; } + subtest 'unprivileged request tests' => sub { + + my @privileged_attrs = @{ Koha::Item->api_privileged_attrs }; + + # Create sample items + my $biblio = $builder->build_sample_biblio; + my $item_1 = $builder->build_sample_item({ biblionumber => $biblio->id }); + my $item_2 = $builder->build_sample_item({ biblionumber => $biblio->id }); + my $item_3 = $builder->build_sample_item({ biblionumber => $biblio->id }); + + plan tests => scalar @privileged_attrs * 2 * $biblio->items->count; + + my $items_unprivileged_representation = $biblio->items->to_api({ public => 1 }); + my $items_privileged_representation = $biblio->items->to_api(); + + for (my $i = 0; $i < $biblio->items->count; $i++) { + my $privileged_representation = $items_privileged_representation->[$i]; + my $unprivileged_representation = $items_unprivileged_representation->[$i]; + foreach my $privileged_attr ( @privileged_attrs ) { + ok( exists $privileged_representation->{$privileged_attr}, + "Attribute $privileged_attr' is present" ); + ok( !exists $unprivileged_representation->{$privileged_attr}, + "Attribute '$privileged_attr' is not present" ); + } + } + }; + + $schema->storage->txn_rollback; }; -- 2.20.1