Bug 25327 - Cannot access API spec
Summary: Cannot access API spec
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on: 25045
Blocks: 25411
  Show dependency treegraph
 
Reported: 2020-04-30 12:22 UTC by Nick Clemens
Modified: 2020-07-02 22:37 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.07


Attachments
Bug 25327: Regression tests (1.05 KB, patch)
2020-05-04 18:50 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 25327: Do not call authenticate_api_request to render the spec (1.76 KB, patch)
2020-05-04 18:50 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 25327: Regression tests (1.10 KB, patch)
2020-05-07 11:04 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25327: Do not call authenticate_api_request to render the spec (1.82 KB, patch)
2020-05-07 11:04 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25327: Regression tests (1.17 KB, patch)
2020-05-07 11:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 25327: Do not call authenticate_api_request to render the spec (1.89 KB, patch)
2020-05-07 11:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
[19.11.x] Bug 25327: Regression tests (1.15 KB, patch)
2020-06-19 12:44 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
[19.11.x] Bug 25327: Do not call authenticate_api_request to render the spec (1.90 KB, patch)
2020-06-19 12:45 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 Nick Clemens 2020-04-30 12:22:06 UTC
On matser, when I try to visit the API spec:
http://localhost:8080/api/v1/.html
http://localhost:8081/api/v1/.html

I get an error:
	
error	"Authentication failure."
Comment 1 Tomás Cohen Arazi 2020-04-30 13:34:34 UTC
This is caused by bug 25045. Do you think the spec should be subject to this check? Or always available? I vote for the latter, just gathering opinions while I write the solution.
Comment 2 Jonathan Druart 2020-04-30 13:39:57 UTC
Always available makes sense to me.
Comment 3 Tomás Cohen Arazi 2020-05-04 18:50:33 UTC
Created attachment 104318 [details] [review]
Bug 25327: Regression tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 4 Tomás Cohen Arazi 2020-05-04 18:50:37 UTC
Created attachment 104319 [details] [review]
Bug 25327: Do not call authenticate_api_request to render the spec

The original code for Koha::REST::V1::Auth::under called
authenticate_api_request when requesting the API spec. This didn't make
sense, and recent changes on what conditions are tested for public
routes, broke the route.

We could add another condition, but it really doesn't make sense to call
authenticate_api_request if it should be publicly available in any
configuration, as discussed on the bug.

This patch adds a trivial check and the requested route, and lets the
request through in any case in 'under'.

To test:
1. Point your browser to:
   http://kohadev.myDNSname.org:8080/api/v1/
   http://kohadev.myDNSname.org:8080/api/v1/.html
=> FAIL: In both cases you get an authorization error.
2. Apply the regression tests patch
3. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/auth.t
=> FAIL: The tests reflect the situation, and fail
4. Apply this patch
5. Repeat 1 and 3
=> SUCCESS: All good!
6. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 5 Nick Clemens 2020-05-07 11:04:07 UTC
Created attachment 104489 [details] [review]
Bug 25327: Regression tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 6 Nick Clemens 2020-05-07 11:04:11 UTC
Created attachment 104490 [details] [review]
Bug 25327: Do not call authenticate_api_request to render the spec

The original code for Koha::REST::V1::Auth::under called
authenticate_api_request when requesting the API spec. This didn't make
sense, and recent changes on what conditions are tested for public
routes, broke the route.

We could add another condition, but it really doesn't make sense to call
authenticate_api_request if it should be publicly available in any
configuration, as discussed on the bug.

This patch adds a trivial check and the requested route, and lets the
request through in any case in 'under'.

To test:
1. Point your browser to:
   http://kohadev.myDNSname.org:8080/api/v1/
   http://kohadev.myDNSname.org:8080/api/v1/.html
=> FAIL: In both cases you get an authorization error.
2. Apply the regression tests patch
3. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/auth.t
=> FAIL: The tests reflect the situation, and fail
4. Apply this patch
5. Repeat 1 and 3
=> SUCCESS: All good!
6. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 7 Jonathan Druart 2020-05-07 11:36:02 UTC
Created attachment 104494 [details] [review]
Bug 25327: Regression tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 8 Jonathan Druart 2020-05-07 11:36:05 UTC
Created attachment 104495 [details] [review]
Bug 25327: Do not call authenticate_api_request to render the spec

The original code for Koha::REST::V1::Auth::under called
authenticate_api_request when requesting the API spec. This didn't make
sense, and recent changes on what conditions are tested for public
routes, broke the route.

We could add another condition, but it really doesn't make sense to call
authenticate_api_request if it should be publicly available in any
configuration, as discussed on the bug.

This patch adds a trivial check and the requested route, and lets the
request through in any case in 'under'.

To test:
1. Point your browser to:
   http://kohadev.myDNSname.org:8080/api/v1/
   http://kohadev.myDNSname.org:8080/api/v1/.html
=> FAIL: In both cases you get an authorization error.
2. Apply the regression tests patch
3. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/auth.t
=> FAIL: The tests reflect the situation, and fail
4. Apply this patch
5. Repeat 1 and 3
=> SUCCESS: All good!
6. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 9 Martin Renvoize 2020-05-11 09:01:33 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 10 Joy Nelson 2020-05-11 22:36:41 UTC
not backported to 19.11 - missing dependencies
Comment 11 Tomás Cohen Arazi 2020-06-19 12:44:59 UTC
Created attachment 106082 [details] [review]
[19.11.x] Bug 25327: Regression tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 12 Tomás Cohen Arazi 2020-06-19 12:45:04 UTC
Created attachment 106083 [details] [review]
[19.11.x] Bug 25327: Do not call authenticate_api_request to render the spec

The original code for Koha::REST::V1::Auth::under called
authenticate_api_request when requesting the API spec. This didn't make
sense, and recent changes on what conditions are tested for public
routes, broke the route.

We could add another condition, but it really doesn't make sense to call
authenticate_api_request if it should be publicly available in any
configuration, as discussed on the bug.

This patch adds a trivial check and the requested route, and lets the
request through in any case in 'under'.

To test:
1. Point your browser to:
   http://kohadev.myDNSname.org:8080/api/v1/
   http://kohadev.myDNSname.org:8080/api/v1/.html
=> FAIL: In both cases you get an authorization error.
2. Apply the regression tests patch
3. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/auth.t
=> FAIL: The tests reflect the situation, and fail
4. Apply this patch
5. Repeat 1 and 3
=> SUCCESS: All good!
6. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 13 Aleisha Amohia 2020-06-22 02:30:20 UTC
backported to 19.11.x for 19.11.07
Comment 14 Victor Grousset/tuxayo 2020-07-02 22:37:19 UTC
Missing dependencies for 19.05.x, can't backport.