Attachment 56544 Details for Bug 17428 – Bug 17428: Just some refactoring

[patch] Bug 17428: Just some refactoring

Bug-17428-Just-some-refactoring.patch (text/plain), 2.70 KB, created by Jonathan Druart on 2016-10-14 14:21:09 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2016-10-14 14:21:09 UTC

Size: 2.70 KB

patch

obsolete

>From 6dd3d0a2b98b12661fb44b94dd2d875b94858ccc Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>Date: Fri, 14 Oct 2016 14:23:20 +0100
>Subject: [PATCH] Bug 17428: Just some refactoring
>
>The idea is to make the code more readable moving the code to its own
>subroutine
>---
> Koha/REST/V1.pm | 32 +++++++++++++++++++++-----------
> 1 file changed, 21 insertions(+), 11 deletions(-)
>
>diff --git a/Koha/REST/V1.pm b/Koha/REST/V1.pm
>index 35d2bfb..4fa0df3 100644
>--- a/Koha/REST/V1.pm
>+++ b/Koha/REST/V1.pm
>@@ -91,14 +91,8 @@ sub authenticate_api_request {
>         ) if $cookie and $action_spec->{'x-koha-authorization'};
>     }
> 
>-    # Check for malformed query parameters
>-    my @errors;
>-    my %valid_parameters = map { $_->{name} => 1 if $_->{in} eq 'query' } @{$action_spec->{parameters}};
>-    my $existing_params = $c->req->query_params->to_hash;
>-    for my $param ( keys %{$existing_params} ) {
>-      push @errors, { path => "/query/".$param, message => 'Malformed query string' } unless exists $valid_parameters{$param};
>-    }
>-    return $c->render_swagger({},\@errors,400) if @errors;
>+    my @errors = validate_parameters( $c, $action_spec );
>+    return $c->render_swagger({}, \@errors, 400) if @errors;
> 
>     return $next->($c) unless $action_spec->{'x-koha-authorization'};
>     unless ($user) {
>@@ -107,9 +101,11 @@ sub authenticate_api_request {
> 
>     my $authorization = $action_spec->{'x-koha-authorization'};
>     my $permissions = $authorization->{'permissions'};
>-    return $next->($c) if C4::Auth::haspermission($user->userid, $permissions);
>-    return $next->($c) if allow_owner($c, $authorization, $user);
>-    return $next->($c) if allow_guarantor($c, $authorization, $user);
>+    if ( C4::Auth::haspermission($user->userid, $permissions)
>+        or allow_owner($c, $authorization, $user)
>+        or allow_guarantor($c, $authorization, $user) ) {
>+        return $next->($c)
>+    }
>     return $c->render_swagger(
>         { error => "Authorization failure. Missing required permission(s).",
>           required_permissions => $permissions },
>@@ -118,6 +114,20 @@ sub authenticate_api_request {
>     );
> }
> 
>+sub validate_parameters {
>+    my ( $c, $action_spec ) = @_;
>+
>+    # Check for malformed query parameters
>+    my @errors;
>+    my %valid_parameters = map { ( $_->{in} eq 'query' ) ? ( $_->{name} => 1 ) : () } @{ $action_spec->{parameters} };
>+    my $existing_params = $c->req->query_params->to_hash;
>+    for my $param ( keys %{$existing_params} ) {
>+        push @errors, { path => "/query/" . $param, message => 'Malformed query string' } unless exists $valid_parameters{$param};
>+    }
>+    return @errors;
>+}
>+
>+
> =head3 allow_owner
> 
> Allows access to object for its owner.
>-- 
>2.8.1

Actions: View | Diff | Splinter Review

Attachments on bug 17428: 56189 | 56190 | 56191 | 56226 | 56227 | 56228 | 56261 | 56314 | 56501 | 56544 | 56545 | 56548 | 56553 | 56554 | 56555 | 56556 | 56557 | 56558 | 56559 | 56724 | 56741 | 56800 | 56801 | 56802 | 56803 | 56804 | 56807 | 56808 | 56809 | 56810 | 56811 | 56812