From f90c8fac7c0945bd491a53be7c8239f81be7c41f Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Fri, 14 Oct 2016 14:26:58 +0100 Subject: [PATCH] Bug 17428: 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 ;) --- 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.8.1