Bugzilla – Attachment 56661 Details for
Bug 17445
REST API: Generic handling of malformed query parameters
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 17445: Move the params check after the authentication check
Bug-17445-Move-the-params-check-after-the-authenti.patch (text/plain), 2.60 KB, created by
Tomás Cohen Arazi (tcohen)
on 2016-10-19 13:20:26 UTC
(
hide
)
Description:
Bug 17445: Move the params check after the authentication check
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2016-10-19 13:20:26 UTC
Size:
2.60 KB
patch
obsolete
>From 26685b233dd3b079a530cbd34af3d4aa13b2e5ca Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Fri, 14 Oct 2016 14:26:58 +0100 >Subject: [PATCH] Bug 17445: Move the params check after the authentication > check > >If the user is not authorised to call this route, we would prefer to >raise a 403 instead of 400 > >Note that we wanted to submit tests for this change but the city code >does not let use do that (we are allowed to list/show cities even >without any permissions). The patrons.t is not complete enought and the >holds.t tests do not pass... > >Tomas plans to submit tests but we reach the end of the hackfest ;) > >Signed-off-by: Josef Moravec <josef.moravec@gmail.com> >Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> >--- > Koha/REST/V1.pm | 23 +++++++++++++++++++---- > 1 file changed, 19 insertions(+), 4 deletions(-) > >diff --git a/Koha/REST/V1.pm b/Koha/REST/V1.pm >index 4fa0df3..76ae180 100644 >--- a/Koha/REST/V1.pm >+++ b/Koha/REST/V1.pm >@@ -91,21 +91,36 @@ sub authenticate_api_request { > ) if $cookie and $action_spec->{'x-koha-authorization'}; > } > >- my @errors = validate_parameters( $c, $action_spec ); >- return $c->render_swagger({}, \@errors, 400) if @errors; > >- return $next->($c) unless $action_spec->{'x-koha-authorization'}; >+ # Then check the parameters >+ my @query_errors = validate_query_parameters( $c, $action_spec ); >+ >+ # We do not need any authorization >+ unless ( $action_spec->{'x-koha-authorization'} ) { >+ return @query_errors >+ ? $c->render_swagger({}, \@query_errors, 400) >+ : $next->($c); >+ } >+ > unless ($user) { > return $c->render_swagger({ error => "Authentication required." },{},401); > } > > my $authorization = $action_spec->{'x-koha-authorization'}; > my $permissions = $authorization->{'permissions'}; >+ >+ # Check if the user is authorized > if ( C4::Auth::haspermission($user->userid, $permissions) > or allow_owner($c, $authorization, $user) > or allow_guarantor($c, $authorization, $user) ) { >+ >+ # Return the query errors if exist >+ return $c->render_swagger({}, \@query_errors, 400) if @query_errors; >+ >+ # Everything is ok > return $next->($c) > } >+ > return $c->render_swagger( > { error => "Authorization failure. Missing required permission(s).", > required_permissions => $permissions }, >@@ -114,7 +129,7 @@ sub authenticate_api_request { > ); > } > >-sub validate_parameters { >+sub validate_query_parameters { > my ( $c, $action_spec ) = @_; > > # Check for malformed query parameters >-- >2.10.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 17445
:
56487
|
56488
|
56500
|
56546
|
56547
|
56634
|
56635
|
56636
|
56659
|
56660
| 56661