Bugzilla – Attachment 98061 Details for
Bug 22522
API authentication breaks with updated Mojolicious version
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 22522: Add support for current Mojolicious and related packages
Bug-22522-Add-support-for-current-Mojolicious-and-.patch (text/plain), 8.36 KB, created by
Ere Maijala
on 2020-01-29 11:25:12 UTC
(
hide
)
Description:
Bug 22522: Add support for current Mojolicious and related packages
Filename:
MIME Type:
Creator:
Ere Maijala
Created:
2020-01-29 11:25:12 UTC
Size:
8.36 KB
patch
obsolete
>From 59e2606a9794af7ecf5c14c1e085825982e6986f Mon Sep 17 00:00:00 2001 >From: Ere Maijala <ere.maijala@helsinki.fi> >Date: Wed, 29 Jan 2020 13:24:15 +0200 >Subject: [PATCH] Bug 22522: Add support for current Mojolicious and related > packages > >This patch allows tests to succeed with the following versions: >JSON::Validator 3.18 >Mojolicious 8.32 >Mojolicious::Plugin::OpenAPI 2.21 > >Also Mojolicious::Plugin::OpenAPI version 1.17 and later 1.x versions now work. > >Calling valid_input in under() would cause ' Use of uninitialized value $_[2] ' in more recent OpenAPI plugins, so that was changed too. As far as I can see this does not affect authorization. >--- > Koha/REST/V1/Auth.pm | 10 +++--- > t/db_dependent/Koha/REST/Plugin/Objects.t | 35 ++++++++++++------- > .../Koha/REST/Plugin/PluginRoutes.t | 16 ++++++--- > 3 files changed, 41 insertions(+), 20 deletions(-) > >diff --git a/Koha/REST/V1/Auth.pm b/Koha/REST/V1/Auth.pm >index 57aea11905..85fc5602e0 100644 >--- a/Koha/REST/V1/Auth.pm >+++ b/Koha/REST/V1/Auth.pm >@@ -55,14 +55,14 @@ This subroutine is called before every request to API. > =cut > > sub under { >- my $c = shift->openapi->valid_input or return;; >+ my ( $c ) = @_; > > my $status = 0; > > try { > > # /api/v1/{namespace} >- my $namespace = $c->req->url->to_abs->path->[2]; >+ my $namespace = $c->req->url->to_abs->path->[2] // ''; > > if ( $namespace eq 'public' > and !C4::Context->preference('RESTPublicAPI') ) >@@ -136,7 +136,10 @@ sub authenticate_api_request { > > my $user; > >- my $spec = $c->match->endpoint->pattern->defaults->{'openapi.op_spec'}; >+ # The following supports retrieval of spec with Mojolicious::Plugin::OpenAPI@1.17 and later (first one) >+ # and older versions (second one). >+ # TODO: remove the latter 'openapi.op_spec' if minimum version is bumped to at least 1.17. >+ my $spec = $c->openapi->spec || $c->match->endpoint->pattern->defaults->{'openapi.op_spec'}; > > $c->stash_embed({ spec => $spec }); > >@@ -205,7 +208,6 @@ sub authenticate_api_request { > if ($status eq "ok") { > my $session = get_session($sessionID); > $user = Koha::Patrons->find($session->param('number')); >- # $c->stash('koha.user' => $user); > } > elsif ($status eq "maintenance") { > Koha::Exceptions::UnderMaintenance->throw( >diff --git a/t/db_dependent/Koha/REST/Plugin/Objects.t b/t/db_dependent/Koha/REST/Plugin/Objects.t >index 2e162b21bc..ea18a5c3f0 100644 >--- a/t/db_dependent/Koha/REST/Plugin/Objects.t >+++ b/t/db_dependent/Koha/REST/Plugin/Objects.t >@@ -58,14 +58,15 @@ my $builder = t::lib::TestBuilder->new; > > subtest 'objects.search helper' => sub { > >- plan tests => 34; >+ plan tests => 38; > > $schema->storage->txn_begin; > >- # Remove existing cities to have more control on the search restuls >+ # Remove existing cities to have more control on the search results > Koha::Cities->delete; > >- # Create two sample patrons that match the query >+ # Create three sample cities that match the query. This makes sure we >+ # always have a "next" link regardless of Mojolicious::Plugin::OpenAPI version. > $builder->build_object({ > class => 'Koha::Cities', > value => { >@@ -78,10 +79,16 @@ subtest 'objects.search helper' => sub { > city_name => 'Manuela' > } > }); >+ $builder->build_object({ >+ class => 'Koha::Cities', >+ value => { >+ city_name => 'Manuelab' >+ } >+ }); > > $t->get_ok('/cities?name=manuel&_per_page=1&_page=1') > ->status_is(200) >- ->header_like( 'Link' => qr/<http:\/\/.*\?.*&_page=2.*>; rel="next",/ ) >+ ->header_like( 'Link' => qr/<http:\/\/.*[\?&]_page=2.*>; rel="next",/ ) > ->json_has('/0') > ->json_hasnt('/1') > ->json_is('/0/name' => 'Manuel'); >@@ -94,16 +101,18 @@ subtest 'objects.search helper' => sub { > }); > > # _match=starts_with >- $t->get_ok('/cities?name=manuel&_per_page=3&_page=1&_match=starts_with') >+ $t->get_ok('/cities?name=manuel&_per_page=4&_page=1&_match=starts_with') > ->status_is(200) > ->json_has('/0') > ->json_has('/1') >- ->json_hasnt('/2') >+ ->json_has('/2') >+ ->json_hasnt('/3') > ->json_is('/0/name' => 'Manuel') >- ->json_is('/1/name' => 'Manuela'); >+ ->json_is('/1/name' => 'Manuela') >+ ->json_is('/2/name' => 'Manuelab'); > > # _match=ends_with >- $t->get_ok('/cities?name=manuel&_per_page=3&_page=1&_match=ends_with') >+ $t->get_ok('/cities?name=manuel&_per_page=4&_page=1&_match=ends_with') > ->status_is(200) > ->json_has('/0') > ->json_has('/1') >@@ -112,22 +121,24 @@ subtest 'objects.search helper' => sub { > ->json_is('/1/name' => 'Emanuel'); > > # _match=exact >- $t->get_ok('/cities?name=manuel&_per_page=3&_page=1&_match=exact') >+ $t->get_ok('/cities?name=manuel&_per_page=4&_page=1&_match=exact') > ->status_is(200) > ->json_has('/0') > ->json_hasnt('/1') > ->json_is('/0/name' => 'Manuel'); > > # _match=contains >- $t->get_ok('/cities?name=manuel&_per_page=3&_page=1&_match=contains') >+ $t->get_ok('/cities?name=manuel&_per_page=4&_page=1&_match=contains') > ->status_is(200) > ->json_has('/0') > ->json_has('/1') > ->json_has('/2') >- ->json_hasnt('/3') >+ ->json_has('/3') >+ ->json_hasnt('/4') > ->json_is('/0/name' => 'Manuel') > ->json_is('/1/name' => 'Manuela') >- ->json_is('/2/name' => 'Emanuel'); >+ ->json_is('/2/name' => 'Manuelab') >+ ->json_is('/3/name' => 'Emanuel'); > > $schema->storage->txn_rollback; > }; >diff --git a/t/db_dependent/Koha/REST/Plugin/PluginRoutes.t b/t/db_dependent/Koha/REST/Plugin/PluginRoutes.t >index 197a28d016..6444d8bc64 100644 >--- a/t/db_dependent/Koha/REST/Plugin/PluginRoutes.t >+++ b/t/db_dependent/Koha/REST/Plugin/PluginRoutes.t >@@ -50,6 +50,8 @@ subtest 'Bad plugins tests' => sub { > t::lib::Mocks::mock_config( 'enable_plugins', 1 ); > t::lib::Mocks::mock_preference( 'UseKohaPlugins', 1 ); > >+ # remove any existing plugins that might interfere >+ Koha::Plugins::Methods->search->delete; > my $plugins = Koha::Plugins->new; > $plugins->InstallPlugins; > >@@ -66,8 +68,10 @@ subtest 'Bad plugins tests' => sub { > 'Bad plugins raise warning'; > > my $routes = get_defined_routes($t); >- ok( !exists $routes->{'/contrib/badass/patrons/(:patron_id)/bother_wrong'}, 'Route doesn\'t exist' ); >- ok( exists $routes->{'/contrib/testplugin/patrons/(:patron_id)/bother'}, 'Route exists' ); >+ # Support placeholders () and <> (latter style used starting with Mojolicious::Plugin::OpenAPI@1.28) >+ # TODO: remove () if minimum version is bumped to at least 1.28. >+ ok( !exists $routes->{'/contrib/badass/patrons/(:patron_id)/bother_wrong'} && !exists $routes->{'/contrib/badass/patrons/<:patron_id>/bother_wrong'}, 'Route doesn\'t exist' ); >+ ok( exists $routes->{'/contrib/testplugin/patrons/(:patron_id>)/bother'} || exists $routes->{'/contrib/testplugin/patrons/<:patron_id>/bother'}, 'Route exists' ); > > $schema->storage->txn_rollback; > }; >@@ -98,7 +102,9 @@ subtest 'Disabled plugins tests' => sub { > my $t = Test::Mojo->new('Koha::REST::V1'); > > my $routes = get_defined_routes($t); >- ok( !exists $routes->{'/contrib/testplugin/patrons/(:patron_id)/bother'}, >+ # Support placeholders () and <> (latter style used starting with Mojolicious::Plugin::OpenAPI@1.28) >+ # TODO: remove () if minimum version is bumped to at least 1.28. >+ ok( !exists $routes->{'/contrib/testplugin/patrons/(:patron_id)/bother'} && !exists $routes->{'/contrib/testplugin/patrons/<:patron_id>/bother'}, > 'Plugin disabled, route not defined' ); > > $good_plugin->enable; >@@ -106,7 +112,9 @@ subtest 'Disabled plugins tests' => sub { > $t = Test::Mojo->new('Koha::REST::V1'); > $routes = get_defined_routes($t); > >- ok( exists $routes->{'/contrib/testplugin/patrons/(:patron_id)/bother'}, >+ # Support placeholders () and <> (latter style used starting with Mojolicious::Plugin::OpenAPI@1.28) >+ # TODO: remove () if minimum version is bumped to at least 1.28. >+ ok( exists $routes->{'/contrib/testplugin/patrons/(:patron_id)/bother'} || exists $routes->{'/contrib/testplugin/patrons/<:patron_id>/bother'}, > 'Plugin enabled, route defined' ); > > $schema->storage->txn_rollback; >-- >2.17.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 22522
:
86756
|
93592
|
98061
|
98089
|
98372
|
98557
|
98558
|
98559
|
98665
|
98666
|
98667
|
99181
|
99182
|
99183
|
105591
|
105639
|
105657
|
105658
|
105659
|
105692