Bug 25411 - Plugin routes cannot have anonymous access
Summary: Plugin routes cannot have anonymous access
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Jonathan Druart
URL:
Keywords:
: 25484 (view as bug list)
Depends on: 25327
Blocks: 24003 26322
  Show dependency treegraph
 
Reported: 2020-05-07 11:52 UTC by Tomás Cohen Arazi
Modified: 2021-12-13 21:09 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.07


Attachments
Bug 25411: Regression tests (9.35 KB, patch)
2020-05-15 20:15 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 25411: Add special handling for public plugin routes (2.14 KB, patch)
2020-05-15 20:15 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 25411: Regression tests (9.40 KB, patch)
2020-05-16 04:52 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 25411: Add special handling for public plugin routes (2.19 KB, patch)
2020-05-16 04:52 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 25411: Regression tests (9.47 KB, patch)
2020-05-20 15:14 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 25411: Add special handling for public plugin routes (2.26 KB, patch)
2020-05-20 15:14 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 25411: Group conditions in if elsif (791 bytes, patch)
2020-05-20 15:14 UTC, Jonathan Druart
Details | Diff | Splinter Review
[19.11.x] Bug 25411: Regression tests (8.36 KB, patch)
2020-06-19 12:52 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
[19.11.x] Bug 25411: Add special handling for public plugin routes (2.27 KB, patch)
2020-06-19 12:52 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2020-05-07 11:52:05 UTC
This is a consequence of a big change introduced by bug 25045
Comment 1 Tomás Cohen Arazi 2020-05-15 18:49:01 UTC
*** Bug 25484 has been marked as a duplicate of this bug. ***
Comment 2 Tomás Cohen Arazi 2020-05-15 20:15:31 UTC
Created attachment 104968 [details] [review]
Bug 25411: Regression tests

This patch adds tests the new RESTPublicAnonymousRequests syspref
doesn't apply to routes added by plugins.

It is up to the plugin developer to handle those conditions.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/REST/Plugin/PluginRoutes.t
=> FAIL: Notice the tests fail
Comment 3 Tomás Cohen Arazi 2020-05-15 20:15:36 UTC
Created attachment 104969 [details] [review]
Bug 25411: Add special handling for public plugin routes

This patch implements the required logic in the API code so plugins are
not affected by the new RESTPublicAnonymousRequests system preference.
It is up to the plugin develpers to handle this

To test:
1. Apply the tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/REST/Plugin/PluginRoutes.t
=> FAIL: Notice the tests fail
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass! Yay! Not bad for a friday evening!
5. Sign off :-D

Sponsored-by: ByWater Solutions
Comment 4 Victor Grousset/tuxayo 2020-05-16 04:52:28 UTC
Created attachment 104978 [details] [review]
Bug 25411: Regression tests

This patch adds tests the new RESTPublicAnonymousRequests syspref
doesn't apply to routes added by plugins.

It is up to the plugin developer to handle those conditions.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/REST/Plugin/PluginRoutes.t
=> FAIL: Notice the tests fail

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 5 Victor Grousset/tuxayo 2020-05-16 04:52:31 UTC
Created attachment 104979 [details] [review]
Bug 25411: Add special handling for public plugin routes

This patch implements the required logic in the API code so plugins are
not affected by the new RESTPublicAnonymousRequests system preference.
It is up to the plugin develpers to handle this

To test:
1. Apply the tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/REST/Plugin/PluginRoutes.t
=> FAIL: Notice the tests fail
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass! Yay! Not bad for a friday evening!
5. Sign off :-D

Sponsored-by: ByWater Solutions
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 6 Victor Grousset/tuxayo 2020-05-16 04:53:20 UTC
It works, and hooray for tests first :)
Comment 7 Victor Grousset/tuxayo 2020-05-16 04:53:52 UTC
Whoops, wrong status.
Comment 8 Jonathan Druart 2020-05-20 15:14:45 UTC
Created attachment 105155 [details] [review]
Bug 25411: Regression tests

This patch adds tests the new RESTPublicAnonymousRequests syspref
doesn't apply to routes added by plugins.

It is up to the plugin developer to handle those conditions.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/REST/Plugin/PluginRoutes.t
=> FAIL: Notice the tests fail

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 9 Jonathan Druart 2020-05-20 15:14:49 UTC
Created attachment 105156 [details] [review]
Bug 25411: Add special handling for public plugin routes

This patch implements the required logic in the API code so plugins are
not affected by the new RESTPublicAnonymousRequests system preference.
It is up to the plugin develpers to handle this

To test:
1. Apply the tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/REST/Plugin/PluginRoutes.t
=> FAIL: Notice the tests fail
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass! Yay! Not bad for a friday evening!
5. Sign off :-D

Sponsored-by: ByWater Solutions
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 10 Jonathan Druart 2020-05-20 15:14:53 UTC
Created attachment 105157 [details] [review]
Bug 25411: Group conditions in if elsif

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 11 Tomás Cohen Arazi 2020-05-20 17:14:45 UTC
(In reply to Jonathan Druart from comment #10)
> Created attachment 105157 [details] [review] [review]
> Bug 25411: Group conditions in if elsif
> 
> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Thanks, my initial implementation was a bit more complex (it involved forcing a new layout on the plugins side) and when I rolled it back I forgot this).
Comment 12 David Cook 2020-05-21 00:37:49 UTC
I've only skimmed the code, but it looks good at a glance.

For a second, I thought it was making all plugin API routes public, but then I noticed that's only if they don't have 'x-koha-authorization' defined. 

I wonder if there's anything we need or can do in the API documentation to make it more obvious what (plugin) routes require auth and which don't?
Comment 13 David Cook 2020-05-21 00:38:11 UTC
(In reply to David Cook from comment #12)
> I've only skimmed the code, but it looks good at a glance.
> 
> For a second, I thought it was making all plugin API routes public, but then
> I noticed that's only if they don't have 'x-koha-authorization' defined. 
> 
> I wonder if there's anything we need or can do in the API documentation to
> make it more obvious what (plugin) routes require auth and which don't?

Just a general question. Not directly related to this bug report per se...
Comment 14 Martin Renvoize 2020-05-22 08:33:54 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 15 Aleisha Amohia 2020-06-09 02:01:18 UTC
missing dependencies, not backported to 19.11.x
Comment 16 Tomás Cohen Arazi 2020-06-19 12:52:50 UTC
Created attachment 106084 [details] [review]
[19.11.x] Bug 25411: Regression tests

This patch adds tests the new RESTPublicAnonymousRequests syspref
doesn't apply to routes added by plugins.

It is up to the plugin developer to handle those conditions.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/REST/Plugin/PluginRoutes.t
=> FAIL: Notice the tests fail

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 17 Tomás Cohen Arazi 2020-06-19 12:52:56 UTC
Created attachment 106085 [details] [review]
[19.11.x] Bug 25411: Add special handling for public plugin routes

This patch implements the required logic in the API code so plugins are
not affected by the new RESTPublicAnonymousRequests system preference.
It is up to the plugin develpers to handle this

To test:
1. Apply the tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/REST/Plugin/PluginRoutes.t
=> FAIL: Notice the tests fail
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass! Yay! Not bad for a friday evening!
5. Sign off :-D

Sponsored-by: ByWater Solutions
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 18 Tomás Cohen Arazi 2020-06-19 13:04:51 UTC
Aleisha, only the [19.11.x] patches here.
Comment 19 Aleisha Amohia 2020-06-21 23:52:51 UTC
backported to 19.11.x for 19.11.07
Comment 20 Victor Grousset/tuxayo 2020-07-02 22:34:04 UTC
Missing dependencies for 19.05.x, can't backport.