From 928f504a2bdef4c672371bdc2d9dea870b5d255c Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Wed, 6 Jan 2021 16:16:01 -0300 Subject: [PATCH] Bug 28948: 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 library looks like: GET /libraries The controller will look like: my $library = Koha::Libraries->find( $c->validation->param('library_id') ); return $c->render( status => 200, openapi => $library->to_api ); Implementing an unprivileged (public) route would look like: GET /public/libraries/:library_id The controller will look like: my $library = Koha::Libraries->find( $c->validation->param('library_id') ); return $c->render( status => 200, openapi => $library->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/Library.pm | 19 ++++++++++++++++ Koha/Object.pm | 43 +++++++++++++++++++++++++++-------- t/db_dependent/Koha/Object.t | 22 +++++++++++++++++- t/db_dependent/Koha/Objects.t | 39 ++++++++++++++++++++++++++++++- 4 files changed, 112 insertions(+), 11 deletions(-) diff --git a/Koha/Library.pm b/Koha/Library.pm index 7ee84baca4..43fc86d838 100644 --- a/Koha/Library.pm +++ b/Koha/Library.pm @@ -258,6 +258,25 @@ sub to_api_mapping { }; } +=head3 api_privileged_attrs + +This method returns the list of privileged access-only attributes. This is used +by $library->to_api($params) to render the object correctly, based on the passed I<$params>. + +=cut + +sub api_privileged_attrs { + return [ + 'illemail', + 'reply_to_email', + 'return_path_email', + 'ip', + 'notes', + 'marc_org_code' + ]; +} + + =head3 get_hold_libraries Return all libraries (including self) that belong to the same hold groups diff --git a/Koha/Object.pm b/Koha/Object.pm index 6f55c3b486..1c2355aa81 100644 --- a/Koha/Object.pm +++ b/Koha/Object.pm @@ -538,6 +538,7 @@ sub prefetch_whitelist { ... } }, + public => 0|1, ... ] } @@ -572,7 +573,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} ) { @@ -591,20 +604,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 c3de0be3de..25643a8bb5 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::Library->api_privileged_attrs }; + + plan tests => scalar @privileged_attrs * 2; + + # Create a sample library + my $library = $builder->build_object( { class => 'Koha::Libraries' } ); + + my $unprivileged_representation = $library->to_api({ public => 1 }); + my $privileged_representation = $library->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 dd2e595f67..a3c9c446c6 100755 --- a/t/db_dependent/Koha/Objects.t +++ b/t/db_dependent/Koha/Objects.t @@ -315,7 +315,7 @@ subtest '->search() tests' => sub { subtest "to_api() tests" => sub { - plan tests => 18; + plan tests => 19; $schema->storage->txn_begin; @@ -398,6 +398,43 @@ subtest "to_api() tests" => sub { $i++; } + subtest 'unprivileged request tests' => sub { + + my @privileged_attrs = @{ Koha::Library->api_privileged_attrs }; + + # Create sample libraries + my $library_1 = $builder->build_object({ class => 'Koha::Libraries' }); + my $library_2 = $builder->build_object({ class => 'Koha::Libraries' }); + my $library_3 = $builder->build_object({ class => 'Koha::Libraries' }); + my $libraries = Koha::Libraries->search( + { + branchcode => { + '-in' => [ + $library_1->branchcode, $library_2->branchcode, + $library_3->branchcode + ] + } + } + ); + + plan tests => scalar @privileged_attrs * 2 * $libraries->count; + + my $libraries_unprivileged_representation = $libraries->to_api({ public => 1 }); + my $libraries_privileged_representation = $libraries->to_api(); + + for (my $i = 0; $i < $libraries->count; $i++) { + my $privileged_representation = $libraries_privileged_representation->[$i]; + my $unprivileged_representation = $libraries_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