[Bug 27358] New: Add GET /public/biblios/:biblio_id/items
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 Bug ID: 27358 Summary: Add GET /public/biblios/:biblio_id/items Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: REST API Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com Let's add a public route to get items. This requires a way to filter out attributes that are not meant for public access. -- 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=27358 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | Depends on| |27352 CC| |jonathan.druart@bugs.koha-c | |ommunity.org, | |katrin.fischer@bsz-bw.de, | |lucas@bywatersolutions.com, | |martin.renvoize@ptfs-europe | |.com --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- I will need advise on the fields to hide for public access :-D Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27352 [Bug 27352] Add GET /biblios/:biblio_id/items -- 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=27358 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |24254 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24254 [Bug 24254] Add Koha::Items->filter_by_visible_in_opac -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |agustinmoyano@theke.io, | |nick@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 114922 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114922&action=edit Bug 27358: Add a generic way to handle API privileged access attributes deny-list This patch introduces a way for Koha::Object(s)->to_api to filter out attributes that require privileged access. It is done in a way that the 'public' parameter is recursively passed to nested objects in recursive to_api() calls. This way, Koha::Object-based classes can determine how they will render depending on this parameter. For example, for implementing a route for fetching an item looks like: GET /items The controller will look like: my $item = Koha::Items->find( $c->validation->param('item_id') ); return $c->render( status => 200, openapi => $item->to_api ); Implementing an unprivileged (public) route would look like: GET /public/items/:item_id The controller will look like: my $item = Koha::Items->find( $c->validation->param('item_id') ); return $c->render( status => 200, openapi => $item->to_api({ public => 1 }) ); To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Object*.t => SUCCESS: Tests pass (i.e. current behaviour is kept, new behaviour passes the tests) 3. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114922|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=27358 --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 114935 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114935&action=edit Bug 27358: Add a generic way to handle API privileged access attributes deny-list This patch introduces a way for Koha::Object(s)->to_api to filter out attributes that require privileged access. It is done in a way that the 'public' parameter is recursively passed to nested objects in recursive to_api() calls. This way, Koha::Object-based classes can determine how they will render depending on this parameter. For example, for implementing a route for fetching an item looks like: GET /items The controller will look like: my $item = Koha::Items->find( $c->validation->param('item_id') ); return $c->render( status => 200, openapi => $item->to_api ); Implementing an unprivileged (public) route would look like: GET /public/items/:item_id The controller will look like: my $item = Koha::Items->find( $c->validation->param('item_id') ); return $c->render( status => 200, openapi => $item->to_api({ public => 1 }) ); To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Object*.t => SUCCESS: Tests pass (i.e. current behaviour is kept, new behaviour passes the tests) 3. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 --- Comment #4 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 114936 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114936&action=edit Bug 27358: Make is_public stashed on public routes This patch makes the API authentication code stash the 'is_public' value when public routes are hit. This will be particularly useful to have $c->objects->search generically pass this info down to the ->to_api method. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t => SUCCESS: Tests pass! When a public route is reached, the controller has the 'is_public' value stashed 3. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 --- Comment #5 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 114937 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114937&action=edit Bug 27358: Teach objects.search about public requests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 --- Comment #6 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 114938 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114938&action=edit Bug 27358: Add GET /public/biblios/:biblio_id/items This patch introduces a route to fetch items belonging to a biblio. It is expected to return the 'public' representation of the Koha::Item objects. It is also enforcing the visibility rules, by using Koha::Items->filter_by_visible_in_opac. To test: 1. Apply this patches 2. Run: $ kshell k$ prove t/db_dependent/api/v1/biblios.t => SUCCESS: Test pass and they cover all the cases! 3. Try your favourite REST tool against the new route. 4. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 --- Comment #7 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 114939 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114939&action=edit Bug 27358: Unit tests for public items retrieval -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 --- Comment #8 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Tomas, if I understood correctly you wanted me to check on what fields should not be exposed on the public route? One is easy: itemnotes_nonpublic We also have libraries requesting to hide the acq related information (price fields, booksellerid, etc.) and other fields describing internal processes too. Maybe it would be easier to work backwards on what fields make the most sense to expose? Database fields (not the names for the API) itemnumber biblionumber homebranch holdingbranch location collectioncode itemcallnumber copynumber enumchron barcode dateaccessioned (for finding new things) itemnotes onloan uri itype Not sure how useful these would be as they require a solution for resolving the internal codes into something more useful: notforloan damaged itemlost withdrawn restricted It might make more sense to replace these by a more general availability route? I think we had done some work to specify things like "can be renewed", "can be checked out", "can be placed on hold" etc. A more difficult one: sometimes strictly internal, sometimes visible in the OPAC: materials How will we be handling more_subfields_xml? As it can contain about anything, probably best to hide by default. Could it make sense to fall back on the "visible in OPAC" setting in the frameworks here? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 --- Comment #9 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Hi Katrina, I like both ideas: - A core list of fields that should be returned - Filter the rest out based on the frameworks About more_subfields_xml, they are not present in the current implementation. But we could do something about that too. with 'x-koha-embed' and a method that translates the XML into a hash, and filtering them using the frameworks. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 Bug 27358 depends on bug 24254, which changed state. Bug 24254 Summary: Add Koha::Items->filter_by_visible_in_opac https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24254 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 Bug 27358 depends on bug 24254, which changed state. Bug 24254 Summary: Add Koha::Items->filter_by_visible_in_opac https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24254 What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |REOPENED Resolution|FIXED |--- -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- This looks pretty clever and lays a nice foundation for other routes of a similar nature. Testing :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114935|0 |1 is obsolete| | --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 116187 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116187&action=edit Bug 27358: Add a generic way to handle API privileged access attributes deny-list This patch introduces a way for Koha::Object(s)->to_api to filter out attributes that require privileged access. It is done in a way that the 'public' parameter is recursively passed to nested objects in recursive to_api() calls. This way, Koha::Object-based classes can determine how they will render depending on this parameter. For example, for implementing a route for fetching an item looks like: GET /items The controller will look like: my $item = Koha::Items->find( $c->validation->param('item_id') ); return $c->render( status => 200, openapi => $item->to_api ); Implementing an unprivileged (public) route would look like: GET /public/items/:item_id The controller will look like: my $item = Koha::Items->find( $c->validation->param('item_id') ); return $c->render( status => 200, openapi => $item->to_api({ public => 1 }) ); To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Object*.t => SUCCESS: Tests pass (i.e. current behaviour is kept, new behaviour passes the tests) 3. Sign off :-D Signed-off-by: Martin Renvoize <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=27358 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114936|0 |1 is obsolete| | --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 116188 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116188&action=edit Bug 27358: Make is_public stashed on public routes This patch makes the API authentication code stash the 'is_public' value when public routes are hit. This will be particularly useful to have $c->objects->search generically pass this info down to the ->to_api method. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t => SUCCESS: Tests pass! When a public route is reached, the controller has the 'is_public' value stashed 3. Sign off :-D Signed-off-by: Martin Renvoize <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=27358 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114937|0 |1 is obsolete| | --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 116189 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116189&action=edit Bug 27358: Teach objects.search about public requests Signed-off-by: Martin Renvoize <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=27358 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114938|0 |1 is obsolete| | --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 116190 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116190&action=edit Bug 27358: Add GET /public/biblios/:biblio_id/items This patch introduces a route to fetch items belonging to a biblio. It is expected to return the 'public' representation of the Koha::Item objects. It is also enforcing the visibility rules, by using Koha::Items->filter_by_visible_in_opac. To test: 1. Apply this patches 2. Run: $ kshell k$ prove t/db_dependent/api/v1/biblios.t => SUCCESS: Test pass and they cover all the cases! 3. Try your favourite REST tool against the new route. 4. Sign off :-D Signed-off-by: Martin Renvoize <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=27358 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114939|0 |1 is obsolete| | --- Comment #15 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 116191 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116191&action=edit Bug 27358: Unit tests for public items retrieval Signed-off-by: Martin Renvoize <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=27358 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #16 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- This works well and I like it.. signing off.. However, I have one security/qa comment... I think it would be best to reverse the logic here.. Instead of the `BlockList` approach, we have with `api_privileged_attrs` I think we should instead implement an `AllowList` to 'fail safe' if someone forgets to add a field to the list: `api_public_atts` perhaps? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 --- Comment #17 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I am a little worried about the short list here: +sub api_privileged_attrs { + return [ + 'checked_out_date', + 'checkouts_count', + 'holds_count', + 'internal_notes', + 'extended_subfields', + ]; +} + Can you help me? Just wondering if it also uses the framework visibility, then I'd be happy already :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 --- Comment #18 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Hmmm, I hadn't considered the framework visibility at all... I suppose that could work with the infrastructure here.. I still think we should convert the generic handling to be an AllowList approach, but the list method could easily be looked up from the frameworks... I think? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 Rickard Lindfors <rickard.lindfors@sh.se> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |rickard.lindfors@sh.se -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116187|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=27358 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116188|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=27358 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116189|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=27358 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116190|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=27358 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116191|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=27358 --- Comment #19 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 123843 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123843&action=edit Bug 27358: Add a generic way to handle API privileged access attributes deny-list This patch introduces a way for Koha::Object(s)->to_api to filter out attributes that require privileged access. It is done in a way that the 'public' parameter is recursively passed to nested objects in recursive to_api() calls. This way, Koha::Object-based classes can determine how they will render depending on this parameter. For example, for implementing a route for fetching an item looks like: GET /items The controller will look like: my $item = Koha::Items->find( $c->validation->param('item_id') ); return $c->render( status => 200, openapi => $item->to_api ); Implementing an unprivileged (public) route would look like: GET /public/items/:item_id The controller will look like: my $item = Koha::Items->find( $c->validation->param('item_id') ); return $c->render( status => 200, openapi => $item->to_api({ public => 1 }) ); To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Object*.t => SUCCESS: Tests pass (i.e. current behaviour is kept, new behaviour passes the tests) 3. Sign off :-D Signed-off-by: Martin Renvoize <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=27358 --- Comment #20 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 123844 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123844&action=edit Bug 27358: Make is_public stashed on public routes This patch makes the API authentication code stash the 'is_public' value when public routes are hit. This will be particularly useful to have $c->objects->search generically pass this info down to the ->to_api method. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t => SUCCESS: Tests pass! When a public route is reached, the controller has the 'is_public' value stashed 3. Sign off :-D Signed-off-by: Martin Renvoize <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=27358 --- Comment #21 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 123845 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123845&action=edit Bug 27358: Teach objects.search about public requests Signed-off-by: Martin Renvoize <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=27358 --- Comment #22 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 123846 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123846&action=edit Bug 27358: Add GET /public/biblios/:biblio_id/items This patch introduces a route to fetch items belonging to a biblio. It is expected to return the 'public' representation of the Koha::Item objects. It is also enforcing the visibility rules, by using Koha::Items->filter_by_visible_in_opac. To test: 1. Apply this patches 2. Run: $ kshell k$ prove t/db_dependent/api/v1/biblios.t => SUCCESS: Test pass and they cover all the cases! 3. Try your favourite REST tool against the new route. 4. Sign off :-D Signed-off-by: Martin Renvoize <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=27358 --- Comment #23 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 123847 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123847&action=edit Bug 27358: Unit tests for public items retrieval Signed-off-by: Martin Renvoize <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=27358 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff --- Comment #24 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Katrin Fischer from comment #17)
I am a little worried about the short list here:
+sub api_privileged_attrs { + return [ + 'checked_out_date', + 'checkouts_count', + 'holds_count', + 'internal_notes', + 'extended_subfields', + ]; +} +
Can you help me? Just wondering if it also uses the framework visibility, then I'd be happy already :)
If we leave more_subfields_xml/frameworks out of the item representation (we have plans for that), would y'all help me refine this deny-list for the items? I have just rebased this work and it still works nicely. If I don't get feedback in a few days, I will move the 'public' layer work to another (simpler) table, so other devs see the benefit from this and can work on top of it. My feeling is we can have a list of 'hidden in opac' attributes from the 'items' table, and then we can sort visibility in the views. I might be wrong, though. Looking for feedback. Please PM me if you feel like there's a good use case that could be simpler than this (I'm thinking accountlines). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 --- Comment #25 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- (In reply to Tomás Cohen Arazi from comment #24)
(In reply to Katrin Fischer from comment #17)
I am a little worried about the short list here:
+sub api_privileged_attrs { + return [ + 'checked_out_date', + 'checkouts_count', + 'holds_count', + 'internal_notes', + 'extended_subfields', + ]; +} +
Can you help me? Just wondering if it also uses the framework visibility, then I'd be happy already :)
If we leave more_subfields_xml/frameworks out of the item representation (we have plans for that), would y'all help me refine this deny-list for the items?
I still think we should switch from 'deny-list' to 'allow-list'.. security by default ;)
I have just rebased this work and it still works nicely. If I don't get feedback in a few days, I will move the 'public' layer work to another (simpler) table, so other devs see the benefit from this and can work on top of it.
Hmm, I don't think it would be a bad idea to move the core idea to another, simpler, endpoint/table so other work can be based upon it.
My feeling is we can have a list of 'hidden in opac' attributes from the 'items' table, and then we can sort visibility in the views. I might be wrong, though. Looking for feedback.
Hmm, not sure I understand this one.. do you mean expose fields in the API and only use the 'hidden in opac' options for the final display.. I can see a use case for that.. but I can also see people complaining that some hidden fields are still publically available if you know how to use the API.
Please PM me if you feel like there's a good use case that could be simpler than this (I'm thinking accountlines).
Accountlines could work.. though I still have a way to go regarding the api's there. Questions like '/credits vs /debits vs /lines' and how embeds should work for offsets and things. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #123843|0 |1 is obsolete| | --- Comment #26 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 124454 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124454&action=edit Bug 27358: Add a generic way to handle API privileged access attributes deny-list This patch introduces a way for Koha::Object(s)->to_api to filter out attributes that require privileged access. It is done in a way that the 'public' parameter is recursively passed to nested objects in recursive to_api() calls. This way, Koha::Object-based classes can determine how they will render depending on this parameter. For example, for implementing a route for fetching an item looks like: GET /items The controller will look like: my $item = Koha::Items->find( $c->validation->param('item_id') ); return $c->render( status => 200, openapi => $item->to_api ); Implementing an unprivileged (public) route would look like: GET /public/items/:item_id The controller will look like: my $item = Koha::Items->find( $c->validation->param('item_id') ); return $c->render( status => 200, openapi => $item->to_api({ public => 1 }) ); To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Object*.t => SUCCESS: Tests pass (i.e. current behaviour is kept, new behaviour passes the tests) 3. Sign off :-D Signed-off-by: Martin Renvoize <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=27358 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #123844|0 |1 is obsolete| | --- Comment #27 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 124455 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124455&action=edit Bug 27358: Make is_public stashed on public routes This patch makes the API authentication code stash the 'is_public' value when public routes are hit. This will be particularly useful to have $c->objects->search generically pass this info down to the ->to_api method. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t => SUCCESS: Tests pass! When a public route is reached, the controller has the 'is_public' value stashed 3. Sign off :-D Signed-off-by: Martin Renvoize <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=27358 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #123845|0 |1 is obsolete| | --- Comment #28 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 124456 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124456&action=edit Bug 27358: Teach objects.search about public requests Signed-off-by: Martin Renvoize <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=27358 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #123846|0 |1 is obsolete| | --- Comment #29 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 124457 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124457&action=edit Bug 27358: Add GET /public/biblios/:biblio_id/items This patch introduces a route to fetch items belonging to a biblio. It is expected to return the 'public' representation of the Koha::Item objects. It is also enforcing the visibility rules, by using Koha::Items->filter_by_visible_in_opac. To test: 1. Apply this patches 2. Run: $ kshell k$ prove t/db_dependent/api/v1/biblios.t => SUCCESS: Test pass and they cover all the cases! 3. Try your favourite REST tool against the new route. 4. Sign off :-D Signed-off-by: Martin Renvoize <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=27358 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #123847|0 |1 is obsolete| | --- Comment #30 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 124458 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124458&action=edit Bug 27358: Unit tests for public items retrieval Signed-off-by: Martin Renvoize <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=27358 --- Comment #31 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 124459 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124459&action=edit Bug 27358: (QA follow-up) Convert to allow-list This patch converts the code to use an allow-list as aposed to a deny-list. This is more 'fail safe' than requireing maintanence of a deny-list. We also switch to using db fields names for the list as aposed to api mapped names. This way, the list can be re-used for non-api related sanitising if required. Signed-off-by: Martin Renvoize <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=27358 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=28948 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 --- Comment #32 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Cloned the foundational code to bug 28948 so we can keep this public allow list stuff moving. Lets come back to this once that's pushed and just add the API route and additional filter by framework visibility stuff in this one. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |28948 See Also|https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=28948 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28948 [Bug 28948] Add a /public counterpart for the libraries REST endpoints -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124454|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=27358 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124455|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=27358 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124456|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=27358 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124459|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=27358 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124457|0 |1 is obsolete| | --- Comment #33 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 126156 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=126156&action=edit Bug 27358: Add GET /public/biblios/:biblio_id/items This patch introduces a route to fetch items belonging to a biblio. It is expected to return the 'public' representation of the Koha::Item objects. It is also enforcing the visibility rules, by using Koha::Items->filter_by_visible_in_opac. To test: 1. Apply this patches 2. Run: $ kshell k$ prove t/db_dependent/api/v1/biblios.t => SUCCESS: Test pass and they cover all the cases! 3. Try your favourite REST tool against the new route. 4. Sign off :-D Signed-off-by: Martin Renvoize <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=27358 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124458|0 |1 is obsolete| | --- Comment #34 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 126157 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=126157&action=edit Bug 27358: Unit tests for public items retrieval Signed-off-by: Martin Renvoize <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=27358 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #126157|0 |1 is obsolete| | --- Comment #35 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 126158 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=126158&action=edit Bug 27358: Unit tests for public items retrieval Signed-off-by: Martin Renvoize <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=27358 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #126156|0 |1 is obsolete| | --- Comment #36 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 126159 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=126159&action=edit Bug 27358: Add GET /public/biblios/:biblio_id/items This patch introduces a route to fetch items belonging to a biblio. It is expected to return the 'public' representation of the Koha::Item objects. It is also enforcing the visibility rules, by using Koha::Items->filter_by_visible_in_opac. To test: 1. Apply this patches 2. Run: $ kshell k$ prove t/db_dependent/api/v1/biblios.t => SUCCESS: Test pass and they cover all the cases! 3. Try your favourite REST tool against the new route. 4. Sign off :-D Signed-off-by: Martin Renvoize <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=27358 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #126158|0 |1 is obsolete| | --- Comment #37 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 126164 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=126164&action=edit Bug 27358: Unit tests for public items retrieval Signed-off-by: Martin Renvoize <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=27358 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #126159|0 |1 is obsolete| | --- Comment #38 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 126165 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=126165&action=edit Bug 27358: Add GET /public/biblios/:biblio_id/items This patch introduces a route to fetch items belonging to a biblio. It is expected to return the 'public' representation of the Koha::Item objects. It is also enforcing the visibility rules, by using Koha::Items->filter_by_visible_in_opac. To test: 1. Apply this patches 2. Run: $ kshell k$ prove t/db_dependent/api/v1/biblios.t => SUCCESS: Test pass and they cover all the cases! 3. Try your favourite REST tool against the new route. 4. Sign off :-D Signed-off-by: Martin Renvoize <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=27358 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 --- Comment #39 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Bug updated to reflect dependence on bug 28948 where we took the base code from here for a public allow list implementation and refined it using a simpler case, libraries. Allowlist taken from Katrins list in comment 8 as I agree with that list. We leave out more_subfields_xml so it can be handled separately, likely as an embed. All looking good here now for the next QA round.. Signing off. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |28854 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28854 [Bug 28854] [ALTERNATIVE] Add ability to create bundles of items -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 Bug 27358 depends on bug 24254, which changed state. Bug 24254 Summary: Add Koha::Items->filter_by_visible_in_opac https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24254 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldoldstable |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #126164|0 |1 is obsolete| | --- Comment #40 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 126971 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=126971&action=edit Bug 27358: Unit tests for public items retrieval Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #126165|0 |1 is obsolete| | --- Comment #41 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 126972 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=126972&action=edit Bug 27358: Add GET /public/biblios/:biblio_id/items This patch introduces a route to fetch items belonging to a biblio. It is expected to return the 'public' representation of the Koha::Item objects. It is also enforcing the visibility rules, by using Koha::Items->filter_by_visible_in_opac. To test: 1. Apply this patches 2. Run: $ kshell k$ prove t/db_dependent/api/v1/biblios.t => SUCCESS: Test pass and they cover all the cases! 3. Try your favourite REST tool against the new route. 4. Sign off :-D Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact| |m.de.rooy@rijksmuseum.nl CC| |m.de.rooy@rijksmuseum.nl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |21.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 --- Comment #42 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Pushed to master for 21.11, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 --- Comment #43 from Rickard Lindfors <rickard.lindfors@sh.se> --- Great job! Would it be possible to backport to 20.05 as well? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 Bug 27358 depends on bug 28948, which changed state. Bug 28948 Summary: Add a /public counterpart for the libraries REST endpoints https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28948 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27358 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |RESOLVED CC| |kyle@bywatersolutions.com Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org