[Bug 22522] New: ILL API breaks with updated Mojolicious version
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 Bug ID: 22522 Summary: ILL API breaks with updated Mojolicious version Change sponsored?: --- Product: Koha Version: 18.05 Hardware: All OS: Linux Status: NEW Severity: normal Priority: P5 - low Component: ILL Assignee: koha-bugs@lists.koha-community.org Reporter: jose-mario.monteiro-santos@inlibro.com CC: andrew.isherwood@ptfs-europe.com, philippe.blouin@inlibro.com Target Milestone: --- When we tried to setup the ILL module using FreeForm as its backend, the ILL requests page in our staff client would not load any of the requests. We would get an error message saying that there was a problem with missing content in the Datable. Upon investigation, we discovered that the calls made to the API were returning "Malformed query" errors. When validating the query, Auth.pm checks for authorized parameters in openapi.op_spec, which wasn't defined anywhere in our case. After a quick discussion with Andrew Isherwood on IRC, we downgraded Mojolicious (from 8.02 to 7.21), JSON::Validator (from 3.02 to 0.97) and Mojolicious::Plugin::OpenAPI (from 2.08 to 1.15) and everything worked. Isn't this problematic ? If these modules aren't installed or don't meet the minimum version required by Koha, cpan will install the most recent versions and the API won't work. While this was experienced with the ILL module, I assume that other API calls can and will break in the same way. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org, | |katrin.fischer@bsz-bw.de --- Comment #1 from Andrew Isherwood <andrew.isherwood@ptfs-europe.com> --- I agree with José-Mario, if Mojolicious is introducing breaking changes like this we should be pinning the version to be installed. Looking at the release changelog (https://github.com/mojolicious/mojo/blob/master/Changes) 7.21 is now over two years old, so we should probably be making gradual moves forward on that too. It seems to be a very fast moving project, so pinning the max version is probably the best thing to do in the short term. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|ILL API breaks with updated |API authentication breaks |Mojolicious version |with updated Mojolicious | |version -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |josef.moravec@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #2 from José-Mario Monteiro-Santos <jose-mario.monteiro-santos@inlibro.com> --- I've noticed today that with newer Mojolicious versions, the authentication is basically skipped as x-koha-authorization is never defined. It seems that this makes it so all endpoints do not require authorization to be accessed. This is a major security flaw, since anybody can for example access patron's information. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #3 from José-Mario Monteiro-Santos <jose-mario.monteiro-santos@inlibro.com> --- Created attachment 86756 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=86756&action=edit Bug 22522 - Update API specs' access in Auth.pm With newer versions of Mojolicious and its plugins, endpoints' specs could no longer be accessed, thus bypassing authorization checks and failing to validate query parameters. Test plan: 1. Without being logged in to Koha, access an endpoint directly (such as /api/v1/patrons/{patron_id}) 2. Notice results are received (which is bad since we're not authenticated) 3. Try again with an endpoint that accepts query parameters (such as /api/v1/patrons?firstname=something) 4. Notice that the query is not accepted (even with correct parameters) 5. Apply the patch 6. Repeat step 1 7. Notice that the access is denied 8. Login as a user with proper access rights 9. Repeat step 1 10. Notice that you can now get results 11. Repeat step 3 12. Notice that the query is now accepted 13. Repeat step 3 but with an absurd parameter 14. Notice the query is correctly rejected 15. Ideally, check if other API calls were not broken -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 José-Mario Monteiro-Santos <jose-mario.monteiro-santos@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #86756|Bug 22522 - Update API |Fix for newer description|specs' access in Auth.pm |Mojolicious/OpenAPI | |versions --- Comment #4 from José-Mario Monteiro-Santos <jose-mario.monteiro-santos@inlibro.com> --- Comment on attachment 86756 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=86756 Fix for newer Mojolicious/OpenAPI versions
From 9398e9ce1ddb545b031c4accad4148f743eddadd Mon Sep 17 00:00:00 2001 From: Jose-Mario Monteiro-Santos <jose-mario.monteiro-santos@inLibro.com> Date: Tue, 19 Mar 2019 11:55:45 -0400 Subject: [PATCH] Bug 22522 - Update API specs' access in Auth.pm
With newer versions of Mojolicious and its plugins, endpoints' specs could no longer be accessed, thus bypassing authorization checks and failing to validate query parameters.
Test plan: 1. Without being logged in to Koha, access an endpoint directly (such as /api/v1/patrons/{patron_id}) 2. Notice results are received (which is bad since we're not authenticated) 3. Try again with an endpoint that accepts query parameters (such as /api/v1/patrons?firstname=something) 4. Notice that the query is not accepted (even with correct parameters)
5. Apply the patch
6. Repeat step 1 7. Notice that the access is denied 8. Login as a user with proper access rights 9. Repeat step 1 10. Notice that you can now get results 11. Repeat step 3 12. Notice that the query is now accepted 13. Repeat step 3 but with an absurd parameter 14. Notice the query is correctly rejected
15. Ideally, check if other API calls were not broken --- 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 53c6bac..d0ecd13 100644 --- a/Koha/REST/V1/Auth.pm +++ b/Koha/REST/V1/Auth.pm @@ -130,7 +130,7 @@ sub authenticate_api_request {
my $user;
- my $spec = $c->match->endpoint->pattern->defaults->{'openapi.op_spec'}; + my $spec = $c->openapi->spec; my $authorization = $spec->{'x-koha-authorization'};
my $authorization_header = $c->req->headers->authorization; -- 2.7.4
-- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #5 from José-Mario Monteiro-Santos <jose-mario.monteiro-santos@inlibro.com> --- Slightly modifying Auth.pm restores the original functionnality. It may be useful as a temporary fix for those who have this problem. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 Magnus Enger <magnus@libriotech.no> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |magnus@libriotech.no Status|NEW |Needs Signoff --- Comment #6 from Magnus Enger <magnus@libriotech.no> --- Looks like this should be “needs sign off”? -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 Magnus Enger <magnus@libriotech.no> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #7 from Magnus Enger <magnus@libriotech.no> --- Created attachment 93592 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=93592&action=edit Bug 22522 - Update API specs' access in Auth.pm With newer versions of Mojolicious and its plugins, endpoints' specs could no longer be accessed, thus bypassing authorization checks and failing to validate query parameters. Test plan: 1. Without being logged in to Koha, access an endpoint directly (such as /api/v1/patrons/{patron_id}) 2. Notice results are received (which is bad since we're not authenticated) 3. Try again with an endpoint that accepts query parameters (such as /api/v1/patrons?firstname=something) 4. Notice that the query is not accepted (even with correct parameters) 5. Apply the patch 6. Repeat step 1 7. Notice that the access is denied 8. Login as a user with proper access rights 9. Repeat step 1 10. Notice that you can now get results 11. Repeat step 3 12. Notice that the query is now accepted 13. Repeat step 3 but with an absurd parameter 14. Notice the query is correctly rejected 15. Ideally, check if other API calls were not broken Signed-off-by: Magnus Enger <magnus@libriotech.no> Upgraded modules with: $ sudo cpanm Mojolicious JSON::Validator Mojolicious::Plugin::OpenAPI@2.14 Before the patch data is returned without being logged in. Not good! After the patch: {"error":"Authentication failure."} -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |jose-mario.monteiro-santos@ |ity.org |inlibro.com --- Comment #8 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Is 18.05 here correct or shoudl this be a higher version? -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 Blou <philippe.blouin@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|18.05 |master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #9 from Blou <philippe.blouin@inlibro.com> --- I changed it to master. We applied the patch to our own 19.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 Magnus Enger <magnus@libriotech.no> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |critical Component|ILL |REST api --- Comment #10 from Magnus Enger <magnus@libriotech.no> --- This fixes a major security problem, I'm upping the importance and changing the component from ILL to "REST API". -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #11 from Magnus Enger <magnus@libriotech.no> --- *** Bug 23864 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Group|Koha security | Version|unspecified |master Product|Koha security |Koha Component|Koha |Architecture, internals, | |and plumbing -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #49 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- (In reply to Mason James from comment #40)
(In reply to Ere Maijala from comment #35)
All the mentioned tests should now pass. I'll run a full test suite with Kohadevbox to verify.
hi folks all tests are passing for me on debian 11,10,9,8
debian 9 and 8 are using mojo-7 and openapi-1 modules debian 11 and 10 are using mojo-8 and openapi-2 modules
great work Ere!!! :p
So... where are we at with D10 packaging.. I'm confused.. How did you install Mojolicious::Plugin::OpenAPI on Buster for testing the above scenario's.. directly with cpan? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #50 from Ere Maijala <ere.maijala@helsinki.fi> --- Ues, directly with CPAN. I don't really have enough experience with packaging. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #51 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Brill, me too. For now I've added it to the cpan list in our koha-testing-docker Dockerfile for Buster. We should look at packaging options now I think as it's not great to require people to pull dependancies via cpan.. I'll open a big for both that and more formally fixing our dependancy versions. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au Priority|P5 - low |P1 - high --- Comment #52 from David Cook <dcook@prosentient.com.au> --- Cheers Ere and Martin! (In reply to Blou from comment #9)
I changed it to master.
We applied the patch to our own 19.05
I've patched our own 18.11 and 19.11 with Ere's fix, so I wouldn't be surprised if it affects 18.05 as well. (How far back are we supporting these days?) It would be good to see this get pushed to the stable branches ASAP. While this problem shouldn't affect anyone using Debian packages, I'm sure there are lots of people around the world who aren't using them. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 Joy Nelson <joy@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |joy@bywatersolutions.com --- Comment #53 from Joy Nelson <joy@bywatersolutions.com> --- does not apply to 19.11.x branch. please rebase if needed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|RM_priority | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #54 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- All the API related tests are failing for 18.11 and 19.05 (maybe 19.11 as well, I have just requested a new run 19.11_D9 #156). Could that be related to this change? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com --- Comment #55 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #54)
All the API related tests are failing for 18.11 and 19.05 (maybe 19.11 as well, I have just requested a new run 19.11_D9 #156).
Could that be related to this change?
I think so. At least, they are failing for me on koha-testing-docker, presumably because the Mojo version is too new. - my $spec = $c->match->endpoint->pattern->defaults->{'openapi.op_spec'}; + my $spec = $c->openapi->spec || $c->match->endpoint->pattern->defaults->{'openapi.op_spec'}; Fixes it for me. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #56 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 105591 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=105591&action=edit [19.11.x] Bug 22522: Fail back to ->openapi->spec -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #57 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Kyle, using master-bionic and 19.11.x + your patch I get: t/db_dependent/api/v1/acquisitions_funds.t ............. 1/13 # Failed test '401 Unauthorized' # at t/db_dependent/api/v1/acquisitions_funds.t line 67. # got: '200' # expected: '401' Use of uninitialized value $_[2] in hash element at /usr/share/perl5/Mojolicious/Plugin/OpenAPI.pm line 275. And lot of similar failures -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #58 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I think we need all the Auth.pm changes in the first patch on this series: -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #59 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 105639 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=105639&action=edit Bug 22522: Add support for current Mojolicious in 19.11 This patch allows tests to succeed with the following versions: JSON::Validator 3.18 Mojolicious 8.32 Mojolicious::Plugin::OpenAPI 2.21 Also Mojolicious::Plugin::OpenAPI version 1.17 and later 1.x versions now work. Calling valid_input in under() would cause ' Use of uninitialized value $_[2] ' in more recent OpenAPI plugins, so that was changed too. As far as I can see this does not affect authorization. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #60 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I believe my alternative to Kyle's should do the trick.. it's literally the REST/V1/Auth.pm changes from the first patch here, just without all the test updates included. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #105591|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #61 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- still not working but it's better. Only 3 are failing now: t/db_dependent/api/v1/libraries.t t/db_dependent/api/v1/patrons.t t/db_dependent/api/v1/patrons_password.t with things like # Failed test 'has value for JSON Pointer "ARRAY(0x558849c58998)"' # at t/db_dependent/api/v1/libraries.t line 92. # Looks like you failed 17 tests of 51. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #62 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I've been looking at this and in reality.. I think what we really need is to just backport the whole set correctly.. I'm not sure what stopped it from being backported in the first place to be honest. I've had a go at backporting locally and it applies with minor conflicts to resolve on the 19.11 branch pretty easily. (all of it, including all the tests fixes).. The problem I'm having however, is that running all the tests to check they're now working against a KOHA_IMAGE=19.11-bionic koha-testing-docker crashes my dev server entirely for unknown reasons at the moment.. hence I've not submitted the set on mass yet. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #63 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Martin Renvoize from comment #62)
I've been looking at this and in reality.. I think what we really need is to just backport the whole set correctly.. I'm not sure what stopped it from being backported in the first place to be honest.
I've had a go at backporting locally and it applies with minor conflicts to resolve on the 19.11 branch pretty easily. (all of it, including all the tests fixes).. The problem I'm having however, is that running all the tests to check they're now working against a KOHA_IMAGE=19.11-bionic koha-testing-docker crashes my dev server entirely for unknown reasons at the moment.. hence I've not submitted the set on mass yet.
Are you by chance running it with CPAN and COVERAGE? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #64 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nope.. `KOHA_IMAGE=19.11-bionic docker-compose -f docker-compose.yml -f docker-compose.mariadb_d9.yml -p koha up --force-recreate` Then `kshell`, `kshell`, `prove -v t/db_dependant/api/v1/*` It get's about 2/3rds through then the host OS runs out of memory and starts OOMing, starting with ssh.. and finally it all just falls over and I have to restart my dev box. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #65 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- https://gitlab.com/mrenvoize/Koha/-/commits/bug_22522 is my 19.11 branch with backported patches applied if it helps. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #105639|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #66 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 105657 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=105657&action=edit Bug 22522: [19.11.x] Add support for current Mojolicious and related packages This patch allows tests to succeed with the following versions: JSON::Validator 3.18 Mojolicious 8.32 Mojolicious::Plugin::OpenAPI 2.21 Also Mojolicious::Plugin::OpenAPI version 1.17 and later 1.x versions now work. Calling valid_input in under() would cause ' Use of uninitialized value $_[2] ' in more recent OpenAPI plugins, so that was changed too. As far as I can see this does not affect authorization. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #67 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 105658 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=105658&action=edit Bug 22522: [19.11.x] Fix route typo Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #68 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 105659 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=105659&action=edit Bug 22522: [19.11.x] Fix several REST API tests Fixes among others the invalid use of json_has() which caused broken tests to pass with older Mojolicious versions. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #69 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I have tested Martin's remote branch on master-bionic and 19.11-bionic: all tests pass! Aleisha, could you push those 3 patches (the 19.11 ones) and see how behave Jenkins? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #70 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Awesome.. glad to hear it worked well for your Jonathan.. I think my OOM's were local.. seems my dev server was due of a little maintenance and I can confirm that test now pass for me too :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 Aleisha Amohia <aleisha@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |aleisha@catalyst.net.nz --- Comment #71 from Aleisha Amohia <aleisha@catalyst.net.nz> --- Hi all, the tests fail in Jenkins in 19.11. Output below: 10:41:09 koha_1 | DBD::mysql::st execute failed: Unknown column 'name' in 'where clause' [for Statement "SELECT COUNT( * ) FROM `cities` `me` WHERE ( `name` LIKE ? )" with ParamValues: 0="manuel%"] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1836. 10:41:09 koha_1 | 10:41:09 koha_1 | # Failed test '200 OK' 10:41:09 koha_1 | # at t/db_dependent/Koha/REST/Plugin/Objects.t line 155. 10:41:09 koha_1 | # got: '500' 10:41:09 koha_1 | # expected: '200' 10:41:09 koha_1 | 10:41:09 koha_1 | # Failed test 'has value for JSON Pointer "/0"' 10:41:09 koha_1 | # at t/db_dependent/Koha/REST/Plugin/Objects.t line 155. 10:41:09 koha_1 | 10:41:09 koha_1 | # Failed test 'has value for JSON Pointer "/1"' 10:41:09 koha_1 | # at t/db_dependent/Koha/REST/Plugin/Objects.t line 155. 10:41:09 koha_1 | 10:41:09 koha_1 | # Failed test 'has value for JSON Pointer "/2"' 10:41:09 koha_1 | # at t/db_dependent/Koha/REST/Plugin/Objects.t line 155. 10:41:09 koha_1 | 10:41:09 koha_1 | # Failed test 'exact match for JSON Pointer "/0/name"' 10:41:09 koha_1 | # at t/db_dependent/Koha/REST/Plugin/Objects.t line 155. 10:41:09 koha_1 | # got: undef 10:41:09 koha_1 | # expected: 'Manuel' 10:41:09 koha_1 | 10:41:09 koha_1 | # Failed test 'exact match for JSON Pointer "/1/name"' 10:41:09 koha_1 | # at t/db_dependent/Koha/REST/Plugin/Objects.t line 155. 10:41:09 koha_1 | # got: undef 10:41:09 koha_1 | # expected: 'Manuela' 10:41:09 koha_1 | 10:41:09 koha_1 | # Failed test 'exact match for JSON Pointer "/2/name"' 10:41:09 koha_1 | # at t/db_dependent/Koha/REST/Plugin/Objects.t line 155. 10:41:09 koha_1 | # got: undef 10:41:09 koha_1 | # expected: 'Manuelab' -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #72 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- 19.11: There is something wrong in the paths, and so plack does not start correctly. I have cherry-picked a07c5c153aaecc5cc8b0d1de5c53f184da5971b9 Bug 25279: Add general query parameters to the /cities route But there is still something wrong in the tests, name is not reversed with city_name but I did not find what was missing. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #73 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 105692 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=105692&action=edit [19.11.x] Bug 22522: Adjust tests to previous changes Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #74 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Jonathan Druart from comment #72)
19.11: There is something wrong in the paths, and so plack does not start correctly. I have cherry-picked a07c5c153aaecc5cc8b0d1de5c53f184da5971b9 Bug 25279: Add general query parameters to the /cities route
Forget about this. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #75 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Tomás Cohen Arazi from comment #73)
Created attachment 105692 [details] [review] [19.11.x] Bug 22522: Adjust tests to previous changes
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Aleisha, this one is read for you :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 Aleisha Amohia <aleisha@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable Version(s)|20.05.00 |20.05.00, 19.11.07 released in| | --- Comment #76 from Aleisha Amohia <aleisha@catalyst.net.nz> --- Jenkins is happy now :) backported to 19.11.x for 19.11.07 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 Aleisha Amohia <aleisha@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #77 from Victor Grousset/tuxayo <victor@tuxayo.net> --- I got a conflict when trying to backport to 19.05. (note the tests pass without these patches, no API issues on 19.05) I'm not sure about InstallPlugins, it doesn't exist in 19.05 vs 19.11. So I don't know how valid that is if I don't use it. About `Koha::Plugins::Methods->search->delete;` there is also uncertainly. (note that HEAD means 19.05.x, parent of 6eef54e940 means 19.11.x before the patch that I'm backporting, and the last section is the patch that I'm backporting) <<<<<<< HEAD my @plugins = Koha::Plugins->new->GetPlugins( { all => 1 } ); ||||||| parent of 6eef54e940... [SIGNED-OFF] Bug 22522: [19.11.x] Add support for current Mojolicious and related packages my $plugins = Koha::Plugins->new; $plugins->InstallPlugins; my @plugins = $plugins->GetPlugins( { all => 1 } ); ======= # remove any existing plugins that might interfere Koha::Plugins::Methods->search->delete; my $plugins = Koha::Plugins->new; $plugins->InstallPlugins; my @plugins = $plugins->GetPlugins( { all => 1 } );
> 6eef54e940... [SIGNED-OFF] Bug 22522: [19.11.x] Add support for current Mojolicious and related packages
It's only in PluginRoutes.t I could just try and see which tests pass... Next conflict! (note that HEAD means 19.05.x, parent of 095948c43f means 19.11.x before the patch that I'm backporting, and the last section is the patch that I'm backporting) t/db_dependent/api/v1/patrons.t <<<<<<< HEAD $tx = $t->ua->build_tx(PUT => "/api/v1/patrons/" . $patron_2->id => json => $newpatron); $tx->req->cookies({name => 'CGISESSID', value => $session_id}); $t->request_ok($tx) ->status_is(200, 'Patron updated successfully') ->json_has($newpatron); ||||||| parent of 095948c43f... [SIGNED-OFF] Bug 22522: [19.11.x] Fix several REST API tests $t->put_ok( "//$userid:$password@/api/v1/patrons/" . $patron_2->borrowernumber => json => $newpatron ) ->status_is(200, 'Patron updated successfully') ->json_has($newpatron); ======= my $result = $t->put_ok( "//$userid:$password@/api/v1/patrons/" . $patron_2->borrowernumber => json => $newpatron ) ->status_is(200, 'Patron updated successfully'); # Put back the RO attributes $newpatron->{patron_id} = $unauthorized_patron->to_api->{patron_id}; $newpatron->{restricted} = $unauthorized_patron->to_api->{restricted}; $newpatron->{anonymized} = $unauthorized_patron->to_api->{anonymized}; is_deeply($result->tx->res->json, $newpatron, 'Returned patron from update matches expected');
> 095948c43f... [SIGNED-OFF] Bug 22522: [19.11.x] Fix several REST API tests
I don't know if 19.11 supports that way to authenticate. The tests will tell. Another similar conflict. Tests: it doesn't work. That was to be expected, that would have been so lucky to just hammer it like that. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #78 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Aleisha Amohia from comment #76)
Jenkins is happy now :) backported to 19.11.x for 19.11.07
It seems that there is something wrong for D10 however: 19.11_D10 (#14) 18:34:00 koha_1 | # at t/00-load.t line 62. 18:34:00 koha_1 | # Tried to use 'Koha::REST::V1'. 18:34:00 koha_1 | # Error: Can't locate JSON/Validator/OpenAPI/Mojolicious.pm in @INC (you may need to install the JSON::Validator::OpenAPI::Mojolicious module) (@INC contains: /kohadevbox/koha /etc/perl /usr/local/lib/x86_64-linux-gnu/perl/5.28.1 /usr/local/share/perl/5.28.1 /usr/lib/x86_64-linux-gnu/perl5/5.28 /usr/share/perl5 /usr/lib/x86_64-linux-gnu/perl/5.28 /usr/share/perl/5.28 /usr/local/lib/site_perl /usr/lib/x86_64-linux-gnu/perl-base /var/lib/koha/kohadev/plugins) at /kohadevbox/koha/Koha/REST/V1.pm line 23. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #79 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I'm confused.. in the 19.11 D10 package we seem to be getting the following Mojolicious::Plugin::OpenAPI version: kohadev-koha@ecc849b0f6ab:/kohadevbox/koha$ apt-cache policy libmojolicious-plugin-openapi-perl libmojolicious-plugin-openapi-perl: Installed: 1.15-1~kohadev1 Candidate: 1.15-1~kohadev1 Version table: *** 1.15-1~kohadev1 500 500 http://debian.koha-community.org/koha 19.11/main amd64 Packages 100 /var/lib/dpkg/status This version predates the movement of code from the core plugin into JSON::Validator::OpenAPI::Mojolicious as as such we don't get that module installed.. which leads to the errors Jonathan mentions above.. did something go weird with the packaging here? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #80 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Even more confused.. seems we're using those versions elsewhere too.. so I'm not understanding how the tests are passing elsewhere but not no D10. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #81 from Mason James <mtj@kohaaloha.com> --- (In reply to Martin Renvoize from comment #80)
Even more confused.. seems we're using those versions elsewhere too.. so I'm not understanding how the tests are passing elsewhere but not no D10.
(In reply to Martin Renvoize from comment #79)
I'm confused.. in the 19.11 D10 package we seem to be getting the following Mojolicious::Plugin::OpenAPI version:
kohadev-koha@ecc849b0f6ab:/kohadevbox/koha$ apt-cache policy libmojolicious-plugin-openapi-perl libmojolicious-plugin-openapi-perl: Installed: 1.15-1~kohadev1 Candidate: 1.15-1~kohadev1 Version table: *** 1.15-1~kohadev1 500 500 http://debian.koha-community.org/koha 19.11/main amd64 Packages 100 /var/lib/dpkg/status
This version predates the movement of code from the core plugin into JSON::Validator::OpenAPI::Mojolicious as as such we don't get that module installed.. which leads to the errors Jonathan mentions above.. did something go weird with the packaging here?
hi Martin this behaviour is actually expected the 'koha on debian-10' APT fixes have currently only been applied to the koha dev and 20.05 suites, and not the 19.11 and 19.05 suites appling the fixes for koha 19.11 on debian 10, will actually break koha 19.11 on debian 8 i recommend that we fix koha 19.11 on debian 10, with the acceptance that koha 19.11 on debian 8 will break the following patches are required to be applied to the 19.11.x branch for this to happen https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25633 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25591 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25633 NOTE: users running 19.11 on older debian versions (oldoldstable and older) can continue to use the koha 19.11 packages by using the following /etc/apt/sources.d/koha line... deb http://debian.koha-community.org/koha 19.11 main OS_VERSION replacing OS_VERSION with jessie, xenial, bionic -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #82 from Aleisha Amohia <aleisha@catalyst.net.nz> --- (In reply to Mason James from comment #81)
the following patches are required to be applied to the 19.11.x branch for this to happen
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25633 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25591
Thanks all I have backported these two to 19.11.x for 19.11.07 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #83 from Aleisha Amohia <aleisha@catalyst.net.nz> --- (In reply to Mason James from comment #81)
hi Martin this behaviour is actually expected
the 'koha on debian-10' APT fixes have currently only been applied to the koha dev and 20.05 suites, and not the 19.11 and 19.05 suites appling the fixes for koha 19.11 on debian 10, will actually break koha 19.11 on debian 8
i recommend that we fix koha 19.11 on debian 10, with the acceptance that koha 19.11 on debian 8 will break
Noting that, as expected, the 19.11_D8 node is broken -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 Andrew Isherwood <andrew.isherwood@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|andrew.isherwood@ptfs-europ | |e.com | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=23653 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522 --- Comment #84 from Victor Grousset/tuxayo <victor@tuxayo.net> --- (In reply to Magnus Enger from comment #10)
This fixes a major security problem, I'm upping the importance and changing the component from ILL to "REST API".
Wow, I missed that. After a quick test I confirm that I can't do this (on 19.05 and 18.11):
Test plan: 1. Without being logged in to Koha, access an endpoint directly (such as /api/v1/patrons/{patron_id}) 2. Notice results are received (which is bad since we're not authenticated)
-- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org