[Bug 38745] New: Add a proof of concept JSON-RPC driven endpoint for RPC style calls that don't fit REST ideals
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38745 Bug ID: 38745 Summary: Add a proof of concept JSON-RPC driven endpoint for RPC style calls that don't fit REST ideals Change sponsored?: --- Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Web services Assignee: koha-bugs@lists.koha-community.org Reporter: martin.renvoize@ptfs-europe.com QA Contact: testopia@bugs.koha-community.org We have a few places where RPC style calls to perform various actions makes sense.. thinking 'bulk' actions and also 'test' actions for things like the sftp connection checks I propose a distinct /rpc endpoint with a basic router built into the endpoint and oneOf being used to specify using the existing OpenAPI specifications. -- 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=38745 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38745 --- Comment #1 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Created attachment 175843 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=175843&action=edit Bug 38745: Add RPC Router This patch add an RPC controller under the REST API to act as a router for JSON-RPC 2.0 syle requests. -- 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=38745 --- Comment #2 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Created attachment 175844 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=175844&action=edit Bug 38745: Start adding a populate_empty_callnumbers RPC method This patch takes the populate_empty_callnumbers method as proposed on bug 38226 and modifies it for json-rpc expectations. -- 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=38745 --- Comment #3 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Very much a proof of concept for code and not usable yet -- 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=38745 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=30652 -- 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=38745 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=38226 -- 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=38745 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38745 --- Comment #4 from David Cook <dcook@prosentient.com.au> --- (In reply to Martin Renvoize (ashimema) from comment #3)
Very much a proof of concept for code and not usable yet
I'm mostly liking what I'm seeing! I've got one question and some feedback. __Question__: What would the "id" be in the payload? __Feedback__: Based off on our past conversations, I ended up making an RPC-like endpoint a couple months ago (I wanted to just re-index N number of biblios from third-party tools), and the process brought up two issues: authentication and authorization. Firstly, at a glance, it looks like you'd only be able to apply Koha permissions at the level of the RPC router, which won't be very fine-grained. I think this will likely cause problems for production/practical uses. Secondly, there's no validation of the action passed in the method, which means a caller could call any method for the target class (which is fortunately limited to Koha::REST::V1::, although that's still fairly coarse validation). I think we'd need to think up some further security controls here. -- 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=38745 --- Comment #5 from David Cook <dcook@prosentient.com.au> --- For my local one, I ended up creating a rpc_biblios endpoint with the intention that any user using that endpoint will have edit_catalogue permissions, and limiting methods to fall within that scope. It does mean it's less flexible/powerful than it could be but it's reasonably secure (within the context of Koha's existing permission system...) -- 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=38745 --- Comment #6 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #4)
I think we'd need to think up some further security controls here.
In all honesty, I think the API is calling out for this already. For instance, /api/v1/auth/password/validation needs a finer grained permission than it currently has. That's actually an endpoint that would probably work better as RPC too actually... -- 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=38745 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=39637 -- 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=38745 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- 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=38745 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38745 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38745 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |martin.renvoize@openfifth.c |ity.org |o.uk Status|Needs Signoff |Failed QA --- Comment #7 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Adding Martin as assignee, but not sure about the status of this bug: (In reply to Martin Renvoize (ashimema) from comment #3)
Very much a proof of concept for code and not usable yet
So should this be Needs Signoff or maybe something else? - Patches still apply - Unit tests are missing? - QA tests fail for tidiness [FAIL] Koha/REST/V1/RPC.pm FAIL forbidden_patterns forbidden pattern: Use https:// instead of http:// for URLs (line 18) FAIL tidiness File is not tidy, please run `perl misc/devel/tidy.pl Koha/REST/V1/RPC.pm` -- 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=38745 --- Comment #8 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 209593 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209593&action=edit Bug 38745: Add the /rpc route and spec Add a JSON-RPC 2.0 endpoint, POST /api/v1/rpc, for actions that do not fit the REST model, such as bulk actions. It is described in the API spec, and reports every failure as a JSON-RPC error object: parse error, invalid request (batch requests and notifications are not supported), method not found and invalid params. Swagger 2.0 has no oneOf, which the spec validator rejects, so the params schema and the required permissions of each method live in an x-rpc-methods extension of the rpc_request definition, to be replaced by a oneOf when the spec is migrated to OpenAPI 3 (bug 35613). The route only requires the user to be authenticated, including patrons who cannot log in to the staff interface, so they can find out which method names exist and what permissions they need, as they can from the API spec. The permissions are checked per method. CSRF: cookie session authentication is accepted like on the other routes, and CSRF protection for the REST API comes with bug 34451. Until that is applied, callers of /rpc using a session cookie are not protected. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38745 --- Comment #9 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 209594 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209594&action=edit Bug 38745: Extract authorize_user from authenticate_api_request The permission check at the end of authenticate_api_request is needed by the RPC endpoint, which has to check permissions per method rather than per route. Move it into a reusable authorize_user sub, with no change in behaviour. To test: 1. prove t/db_dependent/api/v1/auth.t t/db_dependent/api/v1/auth_authenticate_api_request.t Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38745 --- Comment #10 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 209595 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209595&action=edit Bug 38745: Add the RPC registry, permissions and error envelopes Methods are only reachable through the explicit Koha::RPC registry, never by resolving the requested name to a Perl package. The permissions of the method, from the spec, are checked before its params are validated, and all failures are returned as JSON-RPC error objects: permission denied (-32001, with the required permissions) and internal error (-32603, with the details only in the log). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38745 --- Comment #11 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 209596 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209596&action=edit Bug 38745: Add Koha::Biblio->populate_empty_callnumbers Move the logic of filling the empty callnumbers of the items of a biblio from its record out of the REST controller, into the biblio object, so it can be reused and tested without HTTP. It reports problems with typed exceptions: the item is not in the biblio, the itemcallnumber preference is not set, or the record has no callnumber. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38745 --- Comment #12 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 209597 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209597&action=edit Bug 38745: Add the biblios.populate_empty_callnumbers RPC method The first RPC method, ported from bug 38226: fill the empty callnumbers of the items of a biblio (or of a single item) from its record. It needs the edit_catalogue permission, and reports a missing biblio or item, a missing itemcallnumber configuration and an empty callnumber in the record as JSON-RPC errors. The proof of concept version of the method in the REST biblios controller is removed. To test: 1. Enable RESTBasicAuth, set itemcallnumber to 082ab and create a biblio with an 082 field and items without a callnumber. 2. POST to /api/v1/rpc as a user with edit_catalogue: {"jsonrpc":"2.0","method":"biblios.populate_empty_callnumbers", "params":{"biblio_id":<id>},"id":1} => the items have the callnumber, listed in the result. 3. Repeat as a user without edit_catalogue => permission denied error. 4. prove t/db_dependent/api/v1/rpc.t t/db_dependent/Koha/RPC.t t/db_dependent/Koha/Biblio.t Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38745 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #175843|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=38745 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #175844|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=38745 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #13 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- I have taken this proof of concept forward into a first real submission, rebased on current main, and obsoleted the two original patches. What the patches do: * Add POST /api/v1/rpc, a JSON-RPC 2.0 endpoint. Only methods in an explicit registry (Koha::RPC) can be called, there is no dispatch by Perl package name any more. * Each method declares its params schema and the permissions it requires in the API spec, and the router checks them per method (permissions first, so users without access learn nothing about the params). Swagger 2.0 has no oneOf and the spec validator rejects it, so this lives in an x-rpc-methods extension of the rpc_request definition, to become a oneOf once we migrate to OpenAPI 3 (bug 35613). * Every failure after authentication (parse error, invalid request, unknown method, invalid params, permission denied, method specific errors) is returned as a JSON-RPC error object. Batch requests and notifications are rejected for now. * authorize_user is extracted from authenticate_api_request so the permission check can be reused. * The first method is biblios.populate_empty_callnumbers, ported from bug 38226 with the logic moved into Koha::Biblio->populate_empty_callnumbers. This addresses the concerns in comment 4: permissions are applied per method rather than for the whole router, and methods are not resolved dynamically. A note on CSRF: cookie session authentication is accepted as on every other route. CSRF protection for the REST API is bug 34451, and until that is applied callers of /rpc using a session cookie are not protected. To test: apply the patches, enable RESTBasicAuth, set itemcallnumber to 082ab, create a biblio with an 082 field and items without a callnumber, and POST {"jsonrpc":"2.0","method":"biblios.populate_empty_callnumbers","params":{"biblio_id":<id>},"id":1} to /api/v1/rpc as a user with edit_catalogue. Repeat as a user without it. Tests: prove t/db_dependent/api/v1/rpc.t t/db_dependent/Koha/RPC.t t/db_dependent/Koha/Biblio.t t/db_dependent/api/v1/auth.t and xt/api.t. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38745 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38745 --- Comment #14 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- I have taken this proof of concept forward into a first real submission, rebased on current main, and obsoleted the two original patches. What the patches do: * Add POST /api/v1/rpc, a JSON-RPC 2.0 endpoint. Only methods in an explicit registry (Koha::RPC) can be called, there is no dispatch by Perl package name any more. * Each method declares its params schema and the permissions it requires in the API spec, and the router checks them per method (permissions first, so users without access learn nothing about the params). Swagger 2.0 has no oneOf and the spec validator rejects it, so this lives in an x-rpc-methods extension of the rpc_request definition, to become a oneOf once we migrate to OpenAPI 3 (bug 35613). * Every failure after authentication (parse error, invalid request, unknown method, invalid params, permission denied, method specific errors) is returned as a JSON-RPC error object. Batch requests and notifications are rejected for now. * authorize_user is extracted from authenticate_api_request so the permission check can be reused. * The first method is biblios.populate_empty_callnumbers, ported from bug 38226 with the logic moved into Koha::Biblio->populate_empty_callnumbers. This addresses the concerns in comment 4: permissions are applied per method rather than for the whole router, and methods are not resolved dynamically. To test: apply the patches, enable RESTBasicAuth, set itemcallnumber to 082ab, create a biblio with an 082 field and items without a callnumber, and POST {"jsonrpc":"2.0","method":"biblios.populate_empty_callnumbers","params":{"biblio_id":<id>},"id":1} to /api/v1/rpc as a user with edit_catalogue. Repeat as a user without it. Tests: prove t/db_dependent/api/v1/rpc.t t/db_dependent/Koha/RPC.t t/db_dependent/Koha/Biblio.t t/db_dependent/api/v1/auth.t and xt/api.t. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org