In large library systems, it can be inconvenient to allow a staff member to edit circulation rules for all libraries. A permission that allows editing only circ rules for the staff member's home library would help.
Created attachment 66681 [details] [review] Bug 15520: Add permission to restrict circ rules editing to own library Test plan: 1) Ensure that you have four users: a) A superlibrarian b) A user with all `parameters` permissions (the toplevel `parameters` box is checked). c) A user with the `manage_circ_rules` permission (and, of course, `catalogue`). d) A user with the `manage_circ_rules`, `manage_circ_rules_restricted` and `catalogue` permissions. 2) As all four users, load the "Circulation and fine rules" administration page (admin/smart-rules.pl). 3) The page should be unchanged for the first three users. It should be possible to view and edit the circ rules for all libraries. 4) The last (restricted) user should only be able to view and edit the circ rules for their own library.
Created attachment 67177 [details] [review] Bug 15520: Add permission to restrict circ rules editing to own library Test plan: 1) Ensure that you have four users: a) A superlibrarian b) A user with all `parameters` permissions (the toplevel `parameters` box is checked). c) A user with the `manage_circ_rules` permission (and, of course, `catalogue`). d) A user with the `manage_circ_rules`, `manage_circ_rules_restricted` and `catalogue` permissions. 2) As all four users, load the "Circulation and fine rules" administration page (admin/smart-rules.pl). 3) The page should be unchanged for the first three users. It should be possible to view and edit the circ rules for all libraries. 4) The last (restricted) user should only be able to view and edit the circ rules for their own library. Followed test plan, patch worked as described Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Created attachment 70387 [details] [review] Bug 15520: Add permission to restrict circ rules editing to own library Test plan: 1) Ensure that you have four users: a) A superlibrarian b) A user with all `parameters` permissions (the toplevel `parameters` box is checked). c) A user with the `manage_circ_rules` permission (and, of course, `catalogue`). d) A user with the `manage_circ_rules`, `manage_circ_rules_restricted` and `catalogue` permissions. 2) As all four users, load the "Circulation and fine rules" administration page (admin/smart-rules.pl). 3) The page should be unchanged for the first three users. It should be possible to view and edit the circ rules for all libraries. 4) The last (restricted) user should only be able to view and edit the circ rules for their own library. Followed test plan, patch worked as described Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
I have fixed an easy conflict with commit 971591d4ee8576d6782d32610bbae7cfe4674231 Bug 19029: Add JavaScript security question for cloning circ rules - <input type="submit" value="Clone" /> + <input type="submit" id="clone_rules" value="Clone" />
This did not work in my test. a) A superlibrarian - This user could only set circ rules for their own library b) A user with all `parameters` permissions (the toplevel `parameters` box is checked). - This user could only set circ rules for their own library c) A user with the `manage_circ_rules` permission (and, of course, `catalogue`). - This user could set circ rules for all libraries. d) A user with the `manage_circ_rules`, `manage_circ_rules_restricted` and `catalogue` permissions. - This user could only set circ rules for their own library
Never mind, my db update didn't run correctly. This looks like it's (still) working correctly.
Jesse, this is working good, but I have found some small issues though: The new permission option is added only for existing installation, we need this in installer data too. Would be good to have test for the new functionality of haspermission sub.
Created attachment 72240 [details] [review] Bug 15520: Add permission to restrict circ rules editing to own library Test plan: 1) Ensure that you have four users: a) A superlibrarian b) A user with all `parameters` permissions (the toplevel `parameters` box is checked). c) A user with the `manage_circ_rules` permission (and, of course, `catalogue`). d) A user with the `manage_circ_rules`, `manage_circ_rules_restricted` and `catalogue` permissions. 2) As all four users, load the "Circulation and fine rules" administration page (admin/smart-rules.pl). 3) The page should be unchanged for the first three users. It should be possible to view and edit the circ rules for all libraries. 4) The last (restricted) user should only be able to view and edit the circ rules for their own library. Followed test plan, patch worked as described Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Created attachment 72241 [details] [review] Bug 15520: (QA follow-up) Add tests, missing installer data
Comment on attachment 72240 [details] [review] Bug 15520: Add permission to restrict circ rules editing to own library Review of attachment 72240 [details] [review]: ----------------------------------------------------------------- ::: admin/smart-rules.pl @@ +63,5 @@ > + > +my $uid = Koha::Patrons->find( $loggedinuser )->userid; > +my $restricted_to_own_library = $uid && haspermission( $uid, { parameters => 'manage_circ_rules_restricted' }, { no_inherit => 1 } ); > +$template->param( restricted_to_own_library => $restricted_to_own_library ); > +$branch = C4::Context::mybranch() if $restricted_to_own_library; This is not the logged in user's home library, it is the library where the user is logged in now - so when user change library, he/she could change another's library rules!
But couldn't they change the home library in their records too? I think in general we got a problem there, that you can get around the limits, maybe the logged-in library is more helpful?
(In reply to Josef Moravec from comment #10) > This is not the logged in user's home library, it is the library where the > user is logged in now - so when user change library, he/she could change > another's library rules! I (In reply to Katrin Fischer from comment #11) > But couldn't they change the home library in their records too? > I think in general we got a problem there, that you can get around the > limits, maybe the logged-in library is more helpful? I think this should be the user's logged in library - for extra protection I think the library would implement AutoLocation and limit logged in site by ip so this would require some extra steps but would also be a first level guard against accidentally altering other rules
> I (In reply to Katrin Fischer from comment #11) > > But couldn't they change the home library in their records too? > > I think in general we got a problem there, that you can get around the > > limits, maybe the logged-in library is more helpful? > > I think this should be the user's logged in library - for extra protection I > think the library would implement AutoLocation and limit logged in site by > ip so this would require some extra steps but would also be a first level > guard against accidentally altering other rules A lot of the public and smaller libraries here don't have static IP addresses, so AutoLocation is not good. But it's a problem we should solve on this bug report! I think the new permission adds an extra barrier as a good first step and it will prevent people from accidentally editing the wrong library's rules.
it's NOT a problem we should solve here... (sorry, typo)
Yes, that would work, but it smells a bit :) For instance a superlibrarian user will have CAN_manage_circ_rules_restricted set in templates. Why not simply do the reverse, like view_borrower_infos_from_any_libraries, and name it manage_circ_rules_from_any_libraries (will need to be set on update for patrons with manage_circ_rules). It would remove the new parameter and avoid a permission to restrict, which could add side-effects (maybe not now, but certainly later).
(In reply to Jonathan Druart from comment #15) > Yes, that would work, but it smells a bit :) > > For instance a superlibrarian user will have > CAN_manage_circ_rules_restricted set in templates. > > Why not simply do the reverse, like view_borrower_infos_from_any_libraries, > and name it manage_circ_rules_from_any_libraries (will need to be set on > update for patrons with manage_circ_rules). > It would remove the new parameter and avoid a permission to restrict, which > could add side-effects (maybe not now, but certainly later). It does make more sense to me
(In reply to Jonathan Druart from comment #15) > Yes, that would work, but it smells a bit :) > > For instance a superlibrarian user will have > CAN_manage_circ_rules_restricted set in templates. > > Why not simply do the reverse, like view_borrower_infos_from_any_libraries, > and name it manage_circ_rules_from_any_libraries (will need to be set on > update for patrons with manage_circ_rules). > It would remove the new parameter and avoid a permission to restrict, which > could add side-effects (maybe not now, but certainly later). I agree with Jonathan's proposal. We can remove this later once the permissions system is based on the library hierarchies :-D
Created attachment 77202 [details] [review] Bug 15520: Add permission to restrict circ rules editing to own library Test plan: 1) Ensure that you have four users: a) A superlibrarian b) A user with all `parameters` permissions (the toplevel `parameters` box is checked). c) A user with the `manage_circ_rules` permission (and, of course, `catalogue`). d) A user with the `manage_circ_rules`, `manage_circ_rules_restricted` and `catalogue` permissions. 2) As all four users, load the "Circulation and fine rules" administration page (admin/smart-rules.pl). 3) The page should be unchanged for the first three users. It should be possible to view and edit the circ rules for all libraries. 4) The last (restricted) user should only be able to view and edit the circ rules for their own library. Amended by JD: In a second version of this patch manage_circ_rules_restricted has been replaced by manage_circ_rules_from_any_libraries and 'no_inherit' related code has been removed
Created attachment 77203 [details] [review] Bug 15520: Rename permission to manage_circ_rules_from_any_libraries
I have removed unnecessary code from the first version and provide a second patch to rename the permission as well as adjust the code. Ready for testing.
Created attachment 77219 [details] [review] Bug 15520: Add permission to restrict circ rules editing to own library Test plan: 1) Ensure that you have four users: a) A superlibrarian b) A user with all `parameters` permissions (the toplevel `parameters` box is checked). c) A user with the `manage_circ_rules` permission (and, of course, `catalogue`). d) A user with the `manage_circ_rules`, `manage_circ_rules_restricted` and `catalogue` permissions. 2) As all four users, load the "Circulation and fine rules" administration page (admin/smart-rules.pl). 3) The page should be unchanged for the first three users. It should be possible to view and edit the circ rules for all libraries. 4) The last (restricted) user should only be able to view and edit the circ rules for their own library. Amended by JD: In a second version of this patch manage_circ_rules_restricted has been replaced by manage_circ_rules_from_any_libraries and 'no_inherit' related code has been removed
Created attachment 77220 [details] [review] Bug 15520: Rename permission to manage_circ_rules_from_any_libraries
Issues in Biblibre Sandbox 1: Some problems occurred applying patches from bug 15520: <h1>Something went wrong !</h1>Applying: Bug 15520: Add permission to restrict circ rules editing to own library Using index info to reconstruct a base tree... M admin/smart-rules.pl M koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt Auto-merging admin/smart-rules.pl error: Failed to merge in the changes. Patch failed at 0001 Bug 15520: Add permission to restrict circ rules editing to own library The copy of the patch that failed is found in: .git/rebase-apply/patch When you have resolved this problem run git bz apply --continue. If you would prefer to skip this patch, instead run git bz apply --skip. To restore the original branch and stop patching run git bz apply --abort. Bug 15520 - Add more granular permission for only editing own library's circ rules 77219 - Bug 15520: Add permission to restrict circ rules editing to own library 77220 - Bug 15520: Rename permission to manage_circ_rules_from_any_libraries Apply? [(y)es, (n)o, (i)nteractive] Patch left in /tmp/Bug-15520-Add-permission-to-restrict-circ-rules-ed-GgcAa1.patch .
Created attachment 78275 [details] [review] Bug 15520: Add permission to restrict circ rules editing to own library Test plan: 1) Ensure that you have four users: a) A superlibrarian b) A user with all `parameters` permissions (the toplevel `parameters` box is checked). c) A user with the `manage_circ_rules` permission (and, of course, `catalogue`). d) A user with the `manage_circ_rules`, `manage_circ_rules_restricted` and `catalogue` permissions. 2) As all four users, load the "Circulation and fine rules" administration page (admin/smart-rules.pl). 3) The page should be unchanged for the first three users. It should be possible to view and edit the circ rules for all libraries. 4) The last (restricted) user should only be able to view and edit the circ rules for their own library. Amended by JD: In a second version of this patch manage_circ_rules_restricted has been replaced by manage_circ_rules_from_any_libraries and 'no_inherit' related code has been removed
Created attachment 78276 [details] [review] Bug 15520: Rename permission to manage_circ_rules_from_any_libraries
Applying: Bug 15520: Add permission to restrict circ rules editing to own library Using index info to reconstruct a base tree... M admin/smart-rules.pl M koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt Auto-merging admin/smart-rules.pl CONFLICT (content): Merge conflict in admin/smart-rules.pl Failed to merge in the changes. Patch failed at 0001 Bug 15520: Add permission to restrict circ rules editing to own library The copy of the patch that failed is found in: /home/vagrant/kohaclone/.git/rebase-apply/patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-15520-Add-permission-to-restrict-circ-rules-ed-IQNrNs.patch Lisette
Created attachment 78722 [details] [review] Bug 15520: Add permission to restrict circ rules editing to own library Test plan: 1) Ensure that you have four users: a) A superlibrarian b) A user with all `parameters` permissions (the toplevel `parameters` box is checked). c) A user with the `manage_circ_rules` permission (and, of course, `catalogue`). d) A user with the `manage_circ_rules`, `manage_circ_rules_restricted` and `catalogue` permissions. 2) As all four users, load the "Circulation and fine rules" administration page (admin/smart-rules.pl). 3) The page should be unchanged for the first three users. It should be possible to view and edit the circ rules for all libraries. 4) The last (restricted) user should only be able to view and edit the circ rules for their own library. Amended by JD: In a second version of this patch manage_circ_rules_restricted has been replaced by manage_circ_rules_from_any_libraries and 'no_inherit' related code has been removed
Created attachment 78723 [details] [review] Bug 15520: Rename permission to manage_circ_rules_from_any_libraries
Restricted user was able to see rules for all libraries and when they tried to edit it, the following error occurred: No property note for Koha::IssuingRule at /usr/share/perl5/Exception/Class/Base.pm line 73 Lisette
(In reply to Lisette Scheer from comment #29) > Restricted user was able to see rules for all libraries and when they tried > to edit it, the following error occurred: > No property note for Koha::IssuingRule at > /usr/share/perl5/Exception/Class/Base.pm line 73 > > Lisette I think comment was for bug 12365.
Created attachment 78735 [details] [review] Bug 15520: Add permission to restrict circ rules editing to own library Test plan: 1) Ensure that you have four users: a) A superlibrarian b) A user with all `parameters` permissions (the toplevel `parameters` box is checked). c) A user with the `manage_circ_rules` permission (and, of course, `catalogue`). d) A user with the `manage_circ_rules`, `manage_circ_rules_restricted` and `catalogue` permissions. 2) As all four users, load the "Circulation and fine rules" administration page (admin/smart-rules.pl). 3) The page should be unchanged for the first three users. It should be possible to view and edit the circ rules for all libraries. 4) The last (restricted) user should only be able to view and edit the circ rules for their own library. Amended by JD: In a second version of this patch manage_circ_rules_restricted has been replaced by manage_circ_rules_from_any_libraries and 'no_inherit' related code has been removed
Created attachment 78736 [details] [review] Bug 15520: Rename permission to manage_circ_rules_from_any_libraries Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>
Created attachment 79780 [details] [review] Bug 15520: Add permission to restrict circ rules editing to own library Test plan: 1) Ensure that you have four users: a) A superlibrarian b) A user with all `parameters` permissions (the toplevel `parameters` box is checked). c) A user with the `manage_circ_rules` permission (and, of course, `catalogue`). d) A user with the `manage_circ_rules`, `manage_circ_rules_restricted` and `catalogue` permissions. 2) As all four users, load the "Circulation and fine rules" administration page (admin/smart-rules.pl). 3) The page should be unchanged for the first three users. It should be possible to view and edit the circ rules for all libraries. 4) The last (restricted) user should only be able to view and edit the circ rules for their own library. Amended by JD: In a second version of this patch manage_circ_rules_restricted has been replaced by manage_circ_rules_from_any_libraries and 'no_inherit' related code has been removed Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 79781 [details] [review] Bug 15520: Rename permission to manage_circ_rules_from_any_libraries Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 79782 [details] [review] Bug 15520: (QA follow-up) Fix renaming of circ rules permission Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Awesome work all! Pushed to master for 18.11
Enhancement, will not be backported to 18.05.x series.