Bug 18890 - REST API: Fix fetching operation spec in route chain
Summary: REST API: Fix fetching operation spec in route chain
Status: RESOLVED MOVED
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Lari Taskula
QA Contact:
URL:
Keywords:
Depends on: 18137
Blocks:
  Show dependency treegraph
 
Reported: 2017-07-03 12:20 UTC by Lari Taskula
Modified: 2023-09-30 20:52 UTC (History)
3 users (show)

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


Attachments
Bug 18890: REST API: Fix fetching operation spec in route chain (1.74 KB, patch)
2017-07-03 12:55 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 18890: Fix cities.t (1.96 KB, patch)
2017-07-03 14:52 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 18890: Fix holds.t (1.16 KB, patch)
2017-07-03 14:52 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 18890: REST API: Fix fetching operation spec in route chain (1.79 KB, patch)
2017-10-23 05:52 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 18890: Fix cities.t (2.01 KB, patch)
2017-10-23 05:52 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 18890: Fix holds.t (1.21 KB, patch)
2017-10-23 05:52 UTC, Mark Tompsett
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lari Taskula 2017-07-03 12:20:30 UTC
Mojolicious::Plugin::OpenAPI version 1.17 added a feature for fetching API spec in route chain. For us, this type of functionality is used in authentication with our own workaround introduced in Bug 18137. Our own workaround no longer works if M::P::OpenAPI version >=1.17. 

When upgrading Mojolicious::Plugin::OpenAPI dependency, this patch  will fix the issue.
Comment 1 Lari Taskula 2017-07-03 12:55:39 UTC
Created attachment 64774 [details] [review]
Bug 18890: REST API: Fix fetching operation spec in route chain

Mojolicious::Plugin::OpenAPI version 1.17 added a feature for fetching API spec
in route chain. For us, this type of functionality is used in authentication with
our own workaround introduced in Bug 18137. Our own workaround no longer works if
M::P::OpenAPI version >=1.17.

When upgrading Mojolicious::Plugin::OpenAPI dependency, this patch  will fix
the issue.

To test:
1. prove t/db_dependent/api/v1
Comment 2 Lari Taskula 2017-07-03 14:52:17 UTC
Created attachment 64777 [details] [review]
Bug 18890: Fix cities.t

The update also causes some tests authorization tests to fail with HTTP 400
response instead of the expected 401/403. It seems that the parameters were
not properly validated before and the tests were mistakenly passing.

To test:
1. prove t/db_dependent/api/v1/cities.t
Comment 3 Lari Taskula 2017-07-03 14:52:31 UTC
Created attachment 64778 [details] [review]
Bug 18890: Fix holds.t

The update also causes some tests authorization tests to fail with HTTP 400
response instead of the expected 401/403. It seems that the parameters were
not properly validated before and the tests were mistakenly passing.

To test:
1. prove t/db_dependent/api/v1/holds.t
Comment 4 Mark Tompsett 2017-10-23 05:52:10 UTC
Created attachment 68370 [details] [review]
Bug 18890: REST API: Fix fetching operation spec in route chain

Mojolicious::Plugin::OpenAPI version 1.17 added a feature for fetching API spec
in route chain. For us, this type of functionality is used in authentication with
our own workaround introduced in Bug 18137. Our own workaround no longer works if
M::P::OpenAPI version >=1.17.

When upgrading Mojolicious::Plugin::OpenAPI dependency, this patch  will fix
the issue.

To test:
1. prove t/db_dependent/api/v1

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 5 Mark Tompsett 2017-10-23 05:52:17 UTC
Created attachment 68371 [details] [review]
Bug 18890: Fix cities.t

The update also causes some tests authorization tests to fail with HTTP 400
response instead of the expected 401/403. It seems that the parameters were
not properly validated before and the tests were mistakenly passing.

To test:
1. prove t/db_dependent/api/v1/cities.t

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 6 Mark Tompsett 2017-10-23 05:52:22 UTC
Created attachment 68372 [details] [review]
Bug 18890: Fix holds.t

The update also causes some tests authorization tests to fail with HTTP 400
response instead of the expected 401/403. It seems that the parameters were
not properly validated before and the tests were mistakenly passing.

To test:
1. prove t/db_dependent/api/v1/holds.t

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 7 Mark Tompsett 2017-10-23 05:57:35 UTC
Had to cpan2deb Mojolicious::Plugin::OpenAPI and JSON::Validator on a Debian 8 old git install to test.

I tested cities and holds no problem.
acquisition_vendors has a similar 400 vs 403 issue, but I didn't look into it. I'll Fail QA and hope for a patch to that test too, as the test plan in comment #1 is what triggered that problem.
Comment 8 Katrin Fischer 2023-09-30 19:20:31 UTC
This is 6 years old now - is it still valid with the current implementation of our API?
Comment 9 Lari Taskula 2023-09-30 19:24:53 UTC
(In reply to Katrin Fischer from comment #8)
> This is 6 years old now - is it still valid with the current implementation
> of our API?

Not valid.

Already fixed by Bug 22522

Thanks for noticing this.
Comment 10 Katrin Fischer 2023-09-30 20:52:29 UTC
Thx for the feedback :) Closing.