Merge 5869522 into 252d3a3

LibreCat · Nov 5, 2019 · 8d3cbe0 · 8d3cbe0
2 parents 252d3a3 + 5869522
commit 8d3cbe0
Show file tree

Hide file tree

Showing 8 changed files with 88 additions and 99 deletions.
diff --git a/bin/app.pl b/bin/app.pl
@@ -65,12 +65,15 @@
     Dancer->dance(Dancer::Request->new(env => $_[0]));
 });
 
+$app = $app->to_app();
+
 # mojo app
+my $mojo_app;
 { 
     my $server = Mojo::Server::PSGI->new;
     my $script = Path::Tiny->new(__FILE__)->sibling('mojo_app.pl')->stringify;
     $server->load_app($script);
-    $app->add($server->to_psgi_app);
+    $mojo_app = $server->to_psgi_app;
 }
 
 # setup sessions
@@ -90,35 +93,55 @@
 my $session_sso = is_array_ref( $config->{session_sso} ) ? $config->{session_sso} : [];
 
 builder {
-    enable '+Dancer::Middleware::Rebase', base => $uri_base, strip => 0 if is_string( $uri_base );
-    enable_if { $_[0]->{REMOTE_ADDR} eq '127.0.0.1' }
-        "Plack::Middleware::ReverseProxy";
-    enable 'Deflater';
-    enable 'Session',
-        store => require_package( $session_store_package )->new( %$session_store_options ),
-        state => require_package( $session_state_package )->new( %$session_state_options );
-    enable 'MethodOverride';
 
-    for my $as ( @$auth_sso ) {
+    mount "/" => builder {
 
-        mount $as->{path} => require_package( $as->{package}, "Plack::Auth::SSO" )->new(
+        #Note: do not put these middleware inside any other path than "/"
+        enable "+Dancer::Middleware::Rebase", base => $uri_base, strip => 0 if is_string( $uri_base );
+        enable_if { $_[0]->{REMOTE_ADDR} eq "127.0.0.1" }
+            "Plack::Middleware::ReverseProxy";
 
-            %{ $as->{options} || {} },
-            uri_base => $uri_base
+        builder {
 
-        )->to_app();
+            #MOJO APP
+            mount "/api" => $mojo_app;
 
-    }
-    for my $as ( @$session_sso ) {
+            #DANCER APP
+            mount "/" => builder {
 
-        mount $as->{path} => require_package( $as->{package}, "LibreCat::Auth::SSO" )->new(
+                enable "Deflater";
+                enable "Session",
+                    store => require_package( $session_store_package )->new( %$session_store_options ),
+                    state => require_package( $session_state_package )->new( %$session_state_options );
+                enable "MethodOverride";
 
-            %{ $as->{options} || {} },
-            uri_base => $uri_base
+                for my $as ( @$auth_sso ) {
 
-        )->to_app();
+                    mount $as->{path} => require_package( $as->{package}, "Plack::Auth::SSO" )->new(
 
-    }
+                        %{ $as->{options} || {} },
+                        uri_base => $uri_base
+
+                    )->to_app();
+
+                }
+                for my $as ( @$session_sso ) {
+
+                    mount $as->{path} => require_package( $as->{package}, "LibreCat::Auth::SSO" )->new(
+
+                        %{ $as->{options} || {} },
+                        uri_base => $uri_base
+
+                    )->to_app();
+
+                }
+
+                $app;
+
+            };
+
+        };
+
+    };
 
-    mount '/' => $app->to_app;
 };
diff --git a/config/files.yml b/config/files.yml
@@ -75,11 +75,3 @@ access_thumbnailer:
         files: *filestore_settings
         access: *accessstore_settings
         tmpdir: data/tmp
-
-# Which ip-ranges can access the file api (not for end-users)
-api:
-    buffer_size: 8192
-    access:
-        ip_range:
-            - 127.0.0.1
-            - 10.0.0.0/8
diff --git a/config/permissions.yml b/config/permissions.yml
@@ -17,8 +17,6 @@ handlers:
     login: internal
     # role: requires a valid login and a user in a specific role
     role: internal
-    # api_access: requires api credentials (see filestore.api.access config)
-    api_access: internal
     # no_access: this route is denied for all
     no_access: internal
     # default: no login or role requires
@@ -69,8 +67,6 @@ routes:
 
     - [ 'GET'  , '/librecat/export' , 'login'  ]
 
-    - [ 'ANY'  , '/librecat/api'                 , 'api_access' ]
-
     # catch all for security reasons
     - [ 'ANY'  , '/librecat'                     , 'login' ]
 

diff --git a/lib/LibreCat/App.pm b/lib/LibreCat/App.pm
@@ -12,7 +12,6 @@ use LibreCat qw(user);
 use Dancer qw(:syntax);
 use LibreCat::App::Search;       # the frontend
 use LibreCat::App::Catalogue;    # the backend
-use LibreCat::App::Api;          # the api
 use LibreCat::App::Helper;
 use Dancer::Plugin::Auth::Tiny;
 
@@ -32,15 +31,14 @@ hook before => sub {
         login      => _login_route($conf),
         redirect   => _redirect_route($conf),
         role       => _role_route($conf),
-        api_access => _api_route($conf),
         no_access  => sub {
             return redirect uri_for('/access_denied');
         },
         default => sub { },
     };
 
     for my $h (keys %{$conf->{handlers}}) {
-        next if $h =~ m{^(login|redirect|role|api_access|no_access|default)$};
+        next if $h =~ m{^(login|redirect|role|no_access|default)$};
         my $package_name = $conf->{handlers}->{$h};
 
         h->log->info("loading $package_name for $h");
@@ -130,32 +128,6 @@ sub _role_route {
     };
 }
 
-sub _api_route {
-    my $conf = shift;
-    sub {
-        my $role = shift // '';
-        if (_ip_match(request->address)) {
-
-            # ok
-        }
-        elsif (session->{role} && $role eq session->{role}) {
-
-            # ok
-        }
-        else {
-            return return redirect uri_for('/access_denied');
-        }
-    };
-}
-
-sub _ip_match {
-    my $ip       = shift;
-    my $access   = h->config->{filestore}->{api}->{access} // {};
-    my $ip_range = $access->{ip_range} // [];
-
-    h->within_ip_range($ip, $ip_range);
-}
-
 # custom authenticate routine
 sub _authenticate {
     my ($username, $password) = @_;

diff --git a/lib/LibreCat/Application.pm b/lib/LibreCat/Application.pm
@@ -19,15 +19,18 @@ sub startup {
     # hardcoded for now
     $self->plugin('LibreCat::Api');
 
-    $self->plugin('TemplateToolkit');
-
-    push @{$self->renderer->paths}, @{librecat->template_paths};
-
     $r->any(
         '/*whatever' => {whatever => ''} => sub {
-            my $c        = shift;
-            my $whatever = $c->param('whatever');
-            $c->render(template => '404', handler => 'tt2', status => 404);
+            $_[0]->render(
+                json => {
+                    errors => [{
+                        status => 404,
+                        id => "route_not_found",
+                        title => "route not found"
+                    }]
+                },
+                status => 404
+            );
         }
     );
 

diff --git a/lib/Mojolicious/Plugin/LibreCat/Api.pm b/lib/Mojolicious/Plugin/LibreCat/Api.pm
@@ -14,7 +14,8 @@ sub register {
     my $token_secret = librecat->config->{api}{v1}{token_secret};
     my $r = $app->routes;
 
-    my $api = $r->any("/api/v1");
+    #Note: path /api only known by bin/app.pl
+    my $api = $r->any("/v1");
 
     $api->get('/openapi.yml')->to('api#openapi_yml');
     $api->get('/openapi.json')->to('api#openapi_json');
@@ -47,18 +48,23 @@ sub register {
 
             ## In Mojolicious HEAD requests are considered equal to GET,
             ## but content will not be sent with the response even if it is present.
+            # GET /api/v1/:model/:id
             $model_api->get('/:id')->to('#show', model => $model)
                 ->name($model);
 
+            # DELETE /api/v1/:model/:id
             $model_api->delete('/:id')->to('#remove', model => $model)
                 ->name($model);
 
+            # PUT /api/v1/:model/:id
             $model_api->put('/:id')->to('#update', model => $model)
                 ->name($model);
 
+            # PUT /api/v1/:model/:id
             $model_api->patch('/:id')->to('#update_fields', model => $model)
                 ->name($model);
 
+            # POST /api/v1/:model
             $model_api->post->to('#create', model => $model)->name($model);
 
             if (librecat->$model->does("LibreCat::Model::Plugin::Versioning"))

diff --git a/t/api/librecat-api.t b/t/api/librecat-api.t
@@ -34,28 +34,28 @@ my $token = librecat->token->encode({foo => 'bar'});
 my $t = Test::Mojo->new('LibreCat::Application');
 
 subtest "get documentation" => sub {
-    $t->get_ok('/api/v1/openapi.json')->status_is(200)->json_has('/basePath');
-    $t->get_ok('/api/v1/openapi.json')->status_is(200);
+    $t->get_ok('/v1/openapi.json')->status_is(200)->json_has('/basePath');
+    $t->get_ok('/v1/openapi.json')->status_is(200);
 };
 
 subtest "authentication" => sub {
-    $t->get_ok('/api/v1/user/1')->status_is(401);
+    $t->get_ok('/v1/user/1')->status_is(401);
 
-    $t->get_ok('/api/v1/user/1' => {Authorization => 'invalid-key'})
+    $t->get_ok('/v1/user/1' => {Authorization => 'invalid-key'})
         ->status_is(401)->json_has('/errors');
 
     # authorization ok, but not user in DB
-    $t->get_ok('/api/v1/user/1' => {Authorization => $token})->status_is(404)
+    $t->get_ok('/v1/user/1' => {Authorization => $token})->status_is(404)
         ->json_has('/errors');
 };
 
 subtest "invalid model" => sub {
-    $t->get_ok('/api/v1/ugly/123' => {Authorization => $token})
+    $t->get_ok('/v1/ugly/123' => {Authorization => $token})
         ->status_is(404);
 };
 
 subtest "get non-existent user" => sub {
-    $t->get_ok('/api/v1/user/91919192882' => {Authorization => $token})
+    $t->get_ok('/v1/user/91919192882' => {Authorization => $token})
         ->status_is(404)->json_has('/errors')
         ->json_is('/errors/0/title', 'user 91919192882 not found');
 };
@@ -64,22 +64,22 @@ subtest "add/get/delete user" => sub {
     my $user = Catmandu->importer('YAML', file => "t/records/valid-user.yml")
         ->first;
 
-    $t->post_ok('/api/v1/user' => {Authorization => $token} => json => $user)
+    $t->post_ok('/v1/user' => {Authorization => $token} => json => $user)
         ->status_is(201)->json_is('/data/id', 999111999);
 
-    $t->get_ok('/api/v1/user/999111999' => {Authorization => $token})
+    $t->get_ok('/v1/user/999111999' => {Authorization => $token})
         ->status_is(200)->json_has('/data/attributes')
         ->json_is('/data/id',                   999111999)
         ->json_is('/data/attributes/full_name', 'User, Test');
 
-    $t->get_ok('/api/v1/user/999111999/versions' => {Authorization => $token})
+    $t->get_ok('/v1/user/999111999/versions' => {Authorization => $token})
         ->status_is(404);
 
     $t->get_ok(
-        '/api/v1/user/999111999/version/2' => {Authorization => $token})
+        '/v1/user/999111999/version/2' => {Authorization => $token})
         ->status_is(404);
 
-    $t->delete_ok('/api/v1/user/999111999' => {Authorization => $token})
+    $t->delete_ok('/v1/user/999111999' => {Authorization => $token})
         ->status_is(200)->json_has('/data/attributes')
         ->json_is('/data/id'                => 999111999)
         ->json_is('/data/attributes/status' => 'deleted');
@@ -92,13 +92,13 @@ subtest "add invalid user" => sub {
         = Catmandu->importer('YAML', file => "t/records/invalid-user.yml")
         ->first;
 
-    $t->post_ok('/api/v1/user' => {Authorization => $token} => json => $user)
+    $t->post_ok('/v1/user' => {Authorization => $token} => json => $user)
         ->status_is(400)->json_has('/errors');
 };
 
 subtest "get non-existent publication" => sub {
 
-    $t->get_ok('/api/v1/publication/101010101' => {Authorization => $token})
+    $t->get_ok('/v1/publication/101010101' => {Authorization => $token})
         ->status_is(404)->json_has('/errors')
         ->json_is('/errors/0/title', 'publication 101010101 not found');
 };
@@ -108,52 +108,52 @@ subtest "add/get/delete publication" => sub {
         file => "t/records/valid-publication.yml")->first;
 
     $t->post_ok(
-        '/api/v1/publication' => {Authorization => $token} => json => $pub)
+        '/v1/publication' => {Authorization => $token} => json => $pub)
         ->status_is(201)->json_is('/data/id', 999999999);
 
-    $t->put_ok('/api/v1/publication/999999999' => {Authorization => $token} =>
+    $t->put_ok('/v1/publication/999999999' => {Authorization => $token} =>
             json => $pub)->status_is(200)->json_is('/data/id', 999999999);
 
     # Change the pub record id and check for errors
     $pub->{_id} = '1234567890';
 
-    $t->put_ok('/api/v1/publication/999999999' => {Authorization => $token} =>
+    $t->put_ok('/v1/publication/999999999' => {Authorization => $token} =>
             json => $pub)->status_is(400)->json_is(
         '/errors/0/validation_error/0',
         "id in request and data don't match"
             );
 
-    $t->get_ok('/api/v1/publication/999999999' => {Authorization => $token})
+    $t->get_ok('/v1/publication/999999999' => {Authorization => $token})
         ->status_is(200)->json_has('/data/attributes')
         ->json_is('/data/id',             999999999)
         ->json_is('/data/attributes/doi', '10.1093/jxb/erv066');
 
     $t->patch_ok(
-        '/api/v1/publication/999999999' => {Authorization => $token} =>
+        '/v1/publication/999999999' => {Authorization => $token} =>
             json => {title => 'Test patch request'})->status_is(200)
         ->json_is('/data/id',               999999999)
         ->json_is('/data/attributes/title', 'Test patch request');
 
     # Patch with a wrong id
     $t->patch_ok(
-        '/api/v1/publication/999999999' => {Authorization => $token} =>
+        '/v1/publication/999999999' => {Authorization => $token} =>
             json => {_id => '1234567890'})->status_is(400)->json_is(
         '/errors/0/validation_error/0',
         "id in request and data don't match"
             );
 
     $t->get_ok(
-        '/api/v1/publication/999999999/versions' => {Authorization => $token})
+        '/v1/publication/999999999/versions' => {Authorization => $token})
         ->status_is(200)->json_is('/data/id', 999999999)
         ->json_is('/data/attributes/0/_version', '3');
 
-    $t->get_ok('/api/v1/publication/999999999/version/1' =>
+    $t->get_ok('/v1/publication/999999999/version/1' =>
             {Authorization => $token})->status_is(200)
         ->json_is('/data/id',                  999999999)
         ->json_is('/data/attributes/_version', '1');
 
     $t->delete_ok(
-        '/api/v1/publication/999999999' => {Authorization => $token})
+        '/v1/publication/999999999' => {Authorization => $token})
         ->status_is(200)->json_has('/data/attributes')
         ->json_is('/data/id'                => 999999999)
         ->json_is('/data/attributes/status' => 'deleted');
@@ -162,8 +162,8 @@ subtest "add/get/delete publication" => sub {
 };
 
 subtest "not_found" => sub {
-    $t->get_ok('/api/v1/projication' => {Authorization => $token})
-        ->status_is(404)->content_like(qr/Page not found \(404\)/);
+    $t->get_ok('/v1/projication' => {Authorization => $token})
+        ->status_is(404)->json_is('/errors',[{ status => 404, id => "route_not_found", title => "route not found" }]);
 };
 
 done_testing;