From 0c9f9e90f9e1a61bc1ee02bf02b937a6bae21fee Mon Sep 17 00:00:00 2001 From: Olli-Antti Kivilahti Date: Mon, 14 Sep 2015 15:20:20 +0300 Subject: [PATCH] [SIGNED-OFF] Bug 14868: Swagger2-driven Permission checking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A hasty downgrade from 13920, utilizing new features implemented by the Mojolicious::Plugin::Swagger2-author, to make it possible to implement more complex authentication/authorization scenarios with the Plugin. Define 'x-koha-permission' for the Swagger2 Operation Object, to automatically authorize against the required permissions. This way we immediately tell the API consumer in the Swagger2-definition, which permissions are needed to access defined resources. Also we don't need to maintain permissions in multiple locations and we can build a smart testing framework to help a lot in creating tests for the new REST API. Rebase notes from 2015-09-22 to 2016-06-20: - In patrons.t, change expected HTTP 403 to 401 when accessing the endpoint without authentication (401 = not authenticated, 403 = authenticated, but no permission). Signed-off-by: Olli-Antti Kivilahti My name is Olli-Antti Kivilahti and I approve this commit. We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now in production and I am certain we have a pretty good idea on how to work with the limitations of Swagger2.0 We participated in the development of the Mojolicious::Plugin::Swagger and know it well. We have made an extension to the plugin to provide full CORS support and have been building all our in-house features on the new REST API. Signed-off-by: Johanna Raisa My name is Johanna Räisä and I approve this commit. We have been using Swagger2.0-driven REST API in production successfully. --- Koha/REST/V1.pm | 53 ++++++++++++++++++++++++++++------------- api/v1/swagger/swagger.json | 1 + t/db_dependent/api/v1/patrons.t | 4 ++-- 3 files changed, 39 insertions(+), 19 deletions(-) diff --git a/Koha/REST/V1.pm b/Koha/REST/V1.pm index 6a1bc8c..c3f3ab6 100644 --- a/Koha/REST/V1.pm +++ b/Koha/REST/V1.pm @@ -20,7 +20,7 @@ use Mojo::Base 'Mojolicious'; use File::Find::Rule; -use C4::Auth qw( check_cookie_auth get_session ); +use C4::Auth qw( check_cookie_auth get_session haspermission ); use C4::Context; use Koha::Logger; use Koha::Patrons; @@ -28,21 +28,6 @@ use Koha::Patrons; sub startup { my $self = shift; - my $route = $self->routes->under->to( - cb => sub { - my $c = shift; - - my ($status, $sessionID) = check_cookie_auth($c->cookie('CGISESSID')); - if ($status eq "ok") { - my $session = get_session($sessionID); - my $user = Koha::Patrons->find($session->param('number')); - $c->stash('koha.user' => $user); - } - - return 1; - } - ); - # Force charset=utf8 in Content-Type header for JSON responses $self->types->type(json => 'application/json; charset=utf8'); @@ -54,7 +39,6 @@ sub startup { } $self->plugin(Swagger2 => { - route => $route, url => $self->home->rel_file("api/v1/swagger/swagger.min.json"), }); } @@ -102,4 +86,39 @@ sub _isMinificationRequired { return 1 if $latestModification < -C $pathToSwaggerMinJson; # compare modification time } +sub authenticate_api_request { + my ($next, $c, $action_spec) = @_; + + my ($session, $user); + my ($status, $sessionID) = check_cookie_auth($c->cookie('CGISESSID')); + if ($status eq "ok") { + $session = get_session($sessionID); + $user = Koha::Patrons->find($session->param('number')); + $c->stash('koha.user' => $user); + } + else { + return $c->render_swagger( + { error => "Invalid authorization key" }, + {}, + 401 + ); + } + + if ($action_spec->{'x-koha-permission'}) { + if (C4::Auth::haspermission($user->userid, $action_spec->{'x-koha-permission'})) { + return $next->($c); + } + else { + return $c->render_swagger( + { error => "No permission" }, + {}, + 403 + ); + } + } + else { + return $next->($c); + } +} + 1; diff --git a/api/v1/swagger/swagger.json b/api/v1/swagger/swagger.json index 249e71b..75fe161 100644 --- a/api/v1/swagger/swagger.json +++ b/api/v1/swagger/swagger.json @@ -13,6 +13,7 @@ } }, "basePath": "/api/v1", + "x-mojo-around-action": "Koha::REST::V1::authenticate_api_request", "paths": { "$ref": "paths.json" }, diff --git a/t/db_dependent/api/v1/patrons.t b/t/db_dependent/api/v1/patrons.t index 6bd4e1a..ded6f1f 100644 --- a/t/db_dependent/api/v1/patrons.t +++ b/t/db_dependent/api/v1/patrons.t @@ -47,10 +47,10 @@ my $borrower = $builder->build({ }); $t->get_ok('/api/v1/patrons') - ->status_is(403); + ->status_is(401); $t->get_ok("/api/v1/patrons/" . $borrower->{ borrowernumber }) - ->status_is(403); + ->status_is(401); my $loggedinuser = $builder->build({ source => 'Borrower', -- 1.9.1