Bug 18890

Summary: REST API: Fix fetching operation spec in route chain
Product: Koha Reporter: Lari Taskula <lari.taskula>
Component: REST APIAssignee: Lari Taskula <lari.taskula>
Status: RESOLVED MOVED QA Contact:
Severity: enhancement    
Priority: P5 - low CC: jonathan.druart, mtompset, tomascohen
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on: 18137    
Bug Blocks:    
Attachments: Bug 18890: REST API: Fix fetching operation spec in route chain
Bug 18890: Fix cities.t
Bug 18890: Fix holds.t
Bug 18890: REST API: Fix fetching operation spec in route chain
Bug 18890: Fix cities.t
Bug 18890: Fix holds.t

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.