[Bug 17479] New: REST API: Save information on owner access
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17479 Bug ID: 17479 Summary: REST API: Save information on owner access Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Web services Assignee: koha-bugs@lists.koha-community.org Reporter: lari.taskula@jns.fi QA Contact: testopia@bugs.koha-community.org There are two ways of accessing a resource via REST API; either: - you have the required permission - you do not have the permission but you are owner of the object, e.g. you want to GET your own patron information In many cases we want to perform additional operations if the user is accessing his own object. Usually this additional operation is checking a system preference. Example: Patron wants to update his own patron information via REST API. We have to check OPACPatronDetails system preference for this. If it is on, we should forward the changes for approval from a librarian. Currently, in controller, we can check this opac-like access by checking that the user does not have permissions and that the patron he is accessing is himself. This would require another haspermission() call. Instead, we could set a flag into $c->stash in Koha/REST/V1.pm in the case of ownership access. After this, in controller, we only need to check $c->stash for this flag. -- 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=17479 Lari Taskula <lari.taskula@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |lari.taskula@jns.fi |ity.org | -- 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=17479 Lari Taskula <lari.taskula@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Depends on| |14868 Status|NEW |Needs Signoff Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14868 [Bug 14868] REST API: Swagger2-driven permission checking -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17479 --- Comment #1 from Lari Taskula <lari.taskula@jns.fi> --- Created attachment 56720 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56720&action=edit Bug 17479: Store information on owner access into $c->stash There are two ways of accessing a resource via REST API; either: - you have the required permission - you do not have the permission but you are owner of the object, e.g. you want to GET your own patron information In many cases we want to perform additional operations if the user is accessing his own object. Usually this additional operation is checking a system preference. Example: Patron wants to update his own patron information via REST API. We have to check OPACPatronDetails system preference for this. If it is on, we should forward the changes for approval from a librarian. Currently, in controller, we can check this opac-like access by checking that the user does not have permissions and that the patron he is accessing is himself. This would require another haspermission() call. Instead, we could set a flag into $c->stash in Koha/REST/V1.pm in the case of ownership access. After this, in controller, we only need to check $c->stash for this flag. To test: 1. Apply patch 2. Run t/db_dependent/api/v1/ownerflag.t 3. Observe it pass -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17479 Lari Taskula <lari.taskula@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |17565 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17565 [Bug 17565] REST API: Let user cancel reserve according to CanReserveBeCanceledFromOpac -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17479 Lari Taskula <lari.taskula@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56720|0 |1 is obsolete| | --- Comment #2 from Lari Taskula <lari.taskula@jns.fi> --- Created attachment 57256 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=57256&action=edit Bug 17479: Store information on owner access into $c->stash There are two ways of accessing a resource via REST API; either: - you have the required permission - you do not have the permission but you are owner of the object, e.g. you want to GET your own patron information In many cases we want to perform additional operations if the user is accessing his own object. Usually this additional operation is checking a system preference. Example: Patron wants to update his own patron information via REST API. We have to check OPACPatronDetails system preference for this. If it is on, we should forward the changes for approval from a librarian. Currently, in controller, we can check this opac-like access by checking that the user does not have permissions and that the patron he is accessing is himself. This would require another haspermission() call. Instead, we could set a flag into $c->stash in Koha/REST/V1.pm in the case of ownership access. After this, in controller, we only need to check $c->stash for this flag. To test: 1. Apply patch 2. Run t/db_dependent/api/v1/ownerflag.t 3. Observe it pass -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17479 Josef Moravec <josef.moravec@gmail.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=17479 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #57256|0 |1 is obsolete| | --- Comment #3 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 57286 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=57286&action=edit [SIGNED-OFF] Bug 17479: Store information on owner access into $c->stash There are two ways of accessing a resource via REST API; either: - you have the required permission - you do not have the permission but you are owner of the object, e.g. you want to GET your own patron information In many cases we want to perform additional operations if the user is accessing his own object. Usually this additional operation is checking a system preference. Example: Patron wants to update his own patron information via REST API. We have to check OPACPatronDetails system preference for this. If it is on, we should forward the changes for approval from a librarian. Currently, in controller, we can check this opac-like access by checking that the user does not have permissions and that the patron he is accessing is himself. This would require another haspermission() call. Instead, we could set a flag into $c->stash in Koha/REST/V1.pm in the case of ownership access. After this, in controller, we only need to check $c->stash for this flag. To test: 1. Apply patch 2. Run t/db_dependent/api/v1/ownerflag.t 3. Observe it pass Signed-off-by: Josef Moravec <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=17479 Jiri Kozlovsky <mail@jkozlovsky.cz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mail@jkozlovsky.cz --- Comment #4 from Jiri Kozlovsky <mail@jkozlovsky.cz> --- I like this flag approach! How about also adding the owned object to the stash? It is very common to perform duplicate search for that object (you may need some arbitrary column from it or to check if something has changes, etc ..) Now you create DB query when checking for ownership and then, the same query when the object itself is required from within the controller. After this implemented, there would always be only one DB query for the owned object. In the controller then, it'd be called $c->stash('owned_object'). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17479 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #5 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Just a dumb [hypothetical] question, since I am not that deep into Mojolicious etc. You only set the flag; you do not clear the flag. How do you make sure that setting this owner flag is not misused/abused later on? Is it possible that it is still on thru persistence? I could imagine that you would clear this flag in the else branch of the haspermission if? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17479 --- Comment #6 from Lari Taskula <lari.taskula@jns.fi> --- In Mojolicious, the stash, which we use for this flag, is a non-persistent storage only for the current request. So it will be cleared for the following requests and then set again. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17479 --- Comment #7 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Lari Taskula from comment #6)
In Mojolicious, the stash, which we use for this flag, is a non-persistent storage only for the current request. So it will be cleared for the following requests and then set again.
OK I suspected that. Altough clearing it in the else still seems safe.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17479 Lari Taskula <lari.taskula@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17479 Lari Taskula <lari.taskula@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #57286|0 |1 is obsolete| | --- Comment #8 from Lari Taskula <lari.taskula@jns.fi> --- Created attachment 61174 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61174&action=edit Bug 17479: Store information on owner access into $c->stash There are two ways of accessing a resource via REST API; either: - you have the required permission - you do not have the permission but you are owner of the object, e.g. you want to GET your own patron information In many cases we want to perform additional operations if the user is accessing his own object. Usually this additional operation is checking a system preference. Example: Patron wants to update his own patron information via REST API. We have to check OPACPatronDetails system preference for this. If it is on, we should forward the changes for approval from a librarian. Currently, in controller, we can check this opac-like access by checking that the user does not have permissions and that the patron he is accessing is himself. This would require another haspermission() call. Instead, we could set a flag into $c->stash in Koha/REST/V1.pm in the case of ownership access. After this, in controller, we only need to check $c->stash for this flag. To test: 1. Apply patch 2. Run t/db_dependent/api/v1/ownerflag.t 3. Observe it pass -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17479 Lari Taskula <lari.taskula@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |18137 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18137 [Bug 18137] REST API: Migrate from Mojolicious::Plugin::Swagger2 to Mojolicious::Plugin::OpenAPI -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17479 --- Comment #9 from Lari Taskula <lari.taskula@jns.fi> --- Rebased on top of Bug 18137. To test, first apply Bug 18137, then this one, and run tests. Removed sign-offs as the test file was heavily rewritten. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17479 --- Comment #10 from Jiri Kozlovsky <mail@jkozlovsky.cz> --- Lari, what do you think about storing owned object also? I think sparing one DB request for each owner / guarantor request is a good step forward. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17479 --- Comment #11 from Lari Taskula <lari.taskula@jns.fi> --- (In reply to Jiri Kozlovsky from comment #10)
Lari, what do you think about storing owned object also?
I think sparing one DB request for each owner / guarantor request is a good step forward.
True. I don't see why we should not also stash the owned object. Feel free to provide a follow-up :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17479 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA CC| |josef.moravec@gmail.com --- Comment #12 from Josef Moravec <josef.moravec@gmail.com> --- The test is failing: t/db_dependent/api/v1/ownerflag.t .. # Failed test 'exact match for JSON Pointer "/error"' # at t/db_dependent/api/v1/ownerflag.t line 61. # got: 'librarian_access' # expected: 'is_owner_access' # Looks like you failed 1 test of 3. t/db_dependent/api/v1/ownerflag.t .. 1/3 # Failed test 'without permission, owner of object tests' # at t/db_dependent/api/v1/ownerflag.t line 64. # Failed test 'exact match for JSON Pointer "/error"' # at t/db_dependent/api/v1/ownerflag.t line 73. # got: 'librarian_access' # expected: 'is_guarantor_access' # Looks like you failed 1 test of 3. # Failed test 'without permissions, guarantor of the owner of the object tests' # at t/db_dependent/api/v1/ownerflag.t line 76. # Looks like you failed 2 tests of 3. t/db_dependent/api/v1/ownerflag.t .. Dubious, test returned 2 (wstat 512, 0x200) Failed 2/3 subtests Test Summary Report ------------------- t/db_dependent/api/v1/ownerflag.t (Wstat: 512 Tests: 3 Failed: 2) Failed tests: 1-2 Non-zero exit status: 2 Files=1, Tests=3, 2 wallclock secs ( 0.02 usr 0.00 sys + 1.19 cusr 0.20 csys = 1.41 CPU) Result: FAIL -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17479 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Component|Web services |REST api CC| |jonathan.druart@bugs.koha-c | |ommunity.org QA Contact|testopia@bugs.koha-communit | |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17479 Bug 17479 depends on bug 18137, which changed state. Bug 18137 Summary: REST API: Migrate from Mojolicious::Plugin::Swagger2 to Mojolicious::Plugin::OpenAPI https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18137 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=17479 Michal Denar <black23@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |black23@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17479 Lari Taskula <lari.taskula@hypernova.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|lari.taskula@hypernova.fi |koha-bugs@lists.koha-commun | |ity.org --- Comment #13 from Lari Taskula <lari.taskula@hypernova.fi> --- I'm no longer able to work on this, so I'm setting assignee to default. Feel free to continue this work. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org