Users without the edit_catalogue permission can delete the record if no items remain from the batch item deletion tool. To test, 1. Create a user with the following permissions: edit_any_item, edit_items, items_batchdel, items_batchmod. 2. When logged in as that user, use the batch item deletion tool to delete a few items. 3. Select “Delete records if no items remain” and “Delete selected items.” When the job has processed, notice that X item(s) deleted. X record(s) deleted. The record deletion shouldn’t be possible without the edit_catalogue permission.
Created attachment 182010 [details] [review] Bug 39848: Restrict record deletion to staff who have edit_catalogue permission To test, 1. Create a user with the following permissions: edit_any_item, edit_items, items_batchdel, items_batchmod. 2. When logged in as that user, use the batch item deletion tool to delete a few items. 3. Select “Delete records if no items remain” and “Delete selected items.” When the job has processed, notice that X item(s) deleted. X record(s) deleted. 4. APPLY PATCH 5. With the same user repeat step 2. 6. This time there should be no option to 'Delete records if no items remain'. 7. Give the same use the Edit catalog (Modify bibliographic/holdings data) (edit_catalogue) permission. 8. Make sure you can successfully delete a record using the 'Delete records if no items remain'.
Created attachment 182030 [details] [review] Bug 39848: Restrict record deletion to staff who have edit_catalogue permission To test, 1. Create a user with the following permissions: edit_any_item, edit_items, items_batchdel, items_batchmod. 2. When logged in as that user, use the batch item deletion tool to delete a few items. 3. Select “Delete records if no items remain” and “Delete selected items.” When the job has processed, notice that X item(s) deleted. X record(s) deleted. 4. APPLY PATCH 5. With the same user repeat step 2. 6. This time there should be no option to 'Delete records if no items remain'. 7. Give the same use the Edit catalog (Modify bibliographic/holdings data) (edit_catalogue) permission. 8. Make sure you can successfully delete a record using the 'Delete records if no items remain'. Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Created attachment 182077 [details] [review] Bug 39848: Restrict record deletion to staff who have edit_catalogue permission To test, 1. Create a user with the following permissions: edit_any_item, edit_items, items_batchdel, items_batchmod. 2. When logged in as that user, use the batch item deletion tool to delete a few items. 3. Select “Delete records if no items remain” and “Delete selected items.” When the job has processed, notice that X item(s) deleted. X record(s) deleted. 4. APPLY PATCH 5. With the same user repeat step 2. 6. This time there should be no option to 'Delete records if no items remain'. 7. Give the same use the Edit catalog (Modify bibliographic/holdings data) (edit_catalogue) permission. 8. Make sure you can successfully delete a record using the 'Delete records if no items remain'. Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Updated description slightly - unauthorized would be a bit worse :)
Is it really enough to remove the checkbox here? Should we not also check for the flag in the script itself?
Pushed for 25.05! Well done everyone, thank you!
It seems I'm a bit late with my comment but we have used item batch deletion to delete bibliographic records on purpose. We do not want to give the edit_catalogue permission to everyone who needs to delete records because it also gives them the permission to edit records (wicht nowadays takes lots of professional competence). With this change we would need a new permission delete_catalogue (or delete_biblio_records) to obtain our current way of deleting bibliographic records.
(In reply to Anneli Österman from comment #7) > It seems I'm a bit late with my comment but we have used item batch deletion > to delete bibliographic records on purpose. We do not want to give the > edit_catalogue permission to everyone who needs to delete records because it > also gives them the permission to edit records (wicht nowadays takes lots of > professional competence). > > With this change we would need a new permission delete_catalogue (or > delete_biblio_records) to obtain our current way of deleting bibliographic > records. Hi Anneli, thanks for reaching out. I am sorry, that this creates a problem for your workflow and I can understand your point. I checked and was surprised to only find your new bug and no older requests for this. I'd like to find a solution that works for all use cases, but developing a new permission at this point in cycle is a bit tricky, as we'd have to change a lot of other places too. This has been marked as major, so for some libraries it's bad... and for you the change is bad. We are in a bit of a hard stop. What we could do is maybe halt off backporting for a bit?
This won't be an immediate problem for us because we do not update to newer version in a while (we have a quite slow update schedule except for security updates). Maybe the new permission has been added before our next update. I found bug https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35953 and commented on that one too. We will surely help with it - at the very least by testing it.
My worry is that since this is marked "major" it will be backported to all older releases - so it could hit you sooner with a security update.
I will discuss about this with colleagues tomorrow, they can make a more accurate assessment than I can. Thank you for your concern! :)
Sorry about the delay. We are fine with backporting. We will take our own actions if it is needed when the time comes. :)
(In reply to Anneli Österman from comment #12) > Sorry about the delay. We are fine with backporting. We will take our own > actions if it is needed when the time comes. :) +1
(In reply to Katrin Fischer from comment #5) > Is it really enough to remove the checkbox here? Should we not also check > for the flag in the script itself? What about this question?
(In reply to Katrin Fischer from comment #14) > (In reply to Katrin Fischer from comment #5) > > Is it really enough to remove the checkbox here? Should we not also check > > for the flag in the script itself? > > What about this question? I 100% agree, I opened Bug 39926 and assigned it to myself. I will get a patch up ASAP!
(In reply to Lucas Gass (lukeg) from comment #15) > (In reply to Katrin Fischer from comment #14) > > (In reply to Katrin Fischer from comment #5) > > > Is it really enough to remove the checkbox here? Should we not also check > > > for the flag in the script itself? > > > > What about this question? > > I 100% agree, I opened Bug 39926 and assigned it to myself. I will get a > patch up ASAP! Thanks!