From 4241305b759511ec47766887910ce6f01740be18 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Mon, 31 Aug 2020 09:36:56 -0300 Subject: [PATCH 1/2] Bug 26322: [19.11.x] Regression tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Tomas Cohen Arazi Signed-off-by: Joonas Kylmälä Signed-off-by: Martin Renvoize Signed-off-by: Aleisha Amohia --- .../Koha/REST/Plugin/PluginRoutes.t | 40 ++++++++++++++++--- 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/t/db_dependent/Koha/REST/Plugin/PluginRoutes.t b/t/db_dependent/Koha/REST/Plugin/PluginRoutes.t index 0779b11ce9..4ccfd28cf4 100644 --- a/t/db_dependent/Koha/REST/Plugin/PluginRoutes.t +++ b/t/db_dependent/Koha/REST/Plugin/PluginRoutes.t @@ -37,8 +37,10 @@ BEGIN { use Koha::Database; use Koha::Plugins; +use t::lib::TestBuilder; -my $schema = Koha::Database->new->schema; +my $schema = Koha::Database->new->schema; +my $builder = t::lib::TestBuilder->new; subtest 'Bad plugins tests' => sub { @@ -120,14 +122,16 @@ subtest 'Disabled plugins tests' => sub { $schema->storage->txn_rollback; }; -subtest 'Anonymous access routes plugins tests' => sub { +subtest 'Permissions and access to plugin routes tests' => sub { - plan tests => 9; + plan tests => 16; $schema->storage->txn_begin; # enable plugins t::lib::Mocks::mock_config( 'enable_plugins', 1 ); + # enable BASIC auth + t::lib::Mocks::mock_preference( 'RESTBasicAuth', 1 ); # remove any existing plugins that might interfere Koha::Plugins::Methods->search->delete; @@ -152,9 +156,9 @@ subtest 'Anonymous access routes plugins tests' => sub { C4::Context->set_preference( 'RESTPublicAnonymousRequests', 0 ); - $t->get_ok('/api/v1/contrib/testplugin/public/patrons/bother') - ->status_is(200, 'Plugin routes not affected by RESTPublicAnonymousRequests') - ->json_is( { bothered => Mojo::JSON->true } ); + $t->get_ok('/api/v1/contrib/testplugin/public/patrons/bother') + ->status_is(200, 'Plugin routes not affected by RESTPublicAnonymousRequests') + ->json_is( { bothered => Mojo::JSON->true } ); C4::Context->set_preference( 'RESTPublicAnonymousRequests', 1 ); @@ -162,6 +166,30 @@ subtest 'Anonymous access routes plugins tests' => sub { ->status_is(200, 'Plugin routes not affected by RESTPublicAnonymousRequests') ->json_is( { bothered => Mojo::JSON->true } ); + $t->get_ok('/api/v1/contrib/testplugin/patrons/bother') + ->status_is(401, 'Plugin routes honour permissions, anonymous access denied'); + + # Create a patron with permissions, but the wrong ones: 3 => parameters + my $librarian = $builder->build_object( + { + class => 'Koha::Patrons', + value => { flags => 2**3 } + } + ); + my $password = 'thePassword123'; + $librarian->set_password( { password => $password, skip_validation => 1 } ); + my $userid = $librarian->userid; + + $t->get_ok("//$userid:$password@/api/v1/contrib/testplugin/patrons/bother") + ->status_is(403, 'Plugin routes honour permissions, wrong permissions, access denied'); + + # Set the patron permissions to the right ones: 4 => borrowers + $librarian->set({ flags => 2 ** 4 })->store->discard_changes; + + $t->get_ok("//$userid:$password@/api/v1/contrib/testplugin/patrons/bother") + ->status_is(200, 'Plugin routes honour permissions, right permissions, access granted') + ->json_is( { bothered => Mojo::JSON->true } ); + $schema->storage->txn_rollback; }; From 57bb4ee95e6f144bf34fbf4b67e148e551343fee Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Mon, 31 Aug 2020 09:43:09 -0300 Subject: [PATCH 2/2] Bug 26322: [19.11.x] Permissions not checked correctly for plugins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch fixes the logic in a condition to address the fact that permissions are not checked for plugins. This was due to bad parenthesis pairing and the lack of good tests for this. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/REST/Plugin/PluginRoutes.t => FAIL: Tests fail because of bad logic 3. Apply this patch 4. Repeat (2) => SUCCESS: Tests pass! 5. Verify the tests cover the use cases that are needed: - Anonymous access - Real user with wrong permissions (parameters => 1) - Real user with right permissions (borrowers => 1) => SUCCESS: Needed use cases so we catch any regression are found 6. Sign off :-D Signed-off-by: Tomas Cohen Arazi Signed-off-by: Joonas Kylmälä Signed-off-by: Martin Renvoize Signed-off-by: Aleisha Amohia --- Koha/REST/V1/Auth.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Koha/REST/V1/Auth.pm b/Koha/REST/V1/Auth.pm index ca26530f61..cec55a0274 100644 --- a/Koha/REST/V1/Auth.pm +++ b/Koha/REST/V1/Auth.pm @@ -254,7 +254,7 @@ sub authenticate_api_request { if ( !$authorization and ( $params->{is_public} and ( C4::Context->preference('RESTPublicAnonymousRequests') or - $user) ) or $params->{is_plugin} ) { + $user) or $params->{is_plugin} ) ) { # We do not need any authorization # Check the parameters validate_query_parameters( $c, $spec );