Bugzilla – Attachment 124479 Details for
Bug 28948
Add a /public counterpart for the libraries REST endpoints
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 28948: Add a generic way to handle API privileged access attributes deny-list
Bug-28948-Add-a-generic-way-to-handle-API-privileg.patch (text/plain), 8.55 KB, created by
Martin Renvoize (ashimema)
on 2021-09-03 15:08:43 UTC
(
hide
)
Description:
Bug 28948: Add a generic way to handle API privileged access attributes deny-list
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2021-09-03 15:08:43 UTC
Size:
8.55 KB
patch
obsolete
>From 9c80f3e9de89a7098e6e52f07530b4a3d32af751 Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >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 <martin.renvoize@ptfs-europe.com> >--- > 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 a1e10e846a..da9838ccce 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 554b631741..2567b850f2 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,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
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 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