[Bug 35905] New: Missing permission check for "record batch delete" from reports
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35905 Bug ID: 35905 Summary: Missing permission check for "record batch delete" from reports Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: minor Priority: P5 - low Component: Reports Assignee: koha-bugs@lists.koha-community.org Reporter: katrin.fischer@bsz-bw.de QA Contact: testopia@bugs.koha-community.org If the user has reports permission, but is missing the "batch record delete" permission, it will still show up as link on top of the reports table. To test: * Create a report like: select biblionumber from biblio limit 10 * Run the report * Verify you have the batch delete option on top of the table * Re-run with a user that is not a super librarian or has the permission * Verify the link still shows, but leads to a 'permission denied' when clicked -- 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=35905 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- 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=35905 --- Comment #1 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 169260 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=169260&action=edit Bug 35905: Add permission checks to report results Test plan: 1. Apply patch 2. Create a report like: select biblionumber from biblio limit 10 2. Run the report 3. Verify batch operations dropdown contains batch modification and deletion 4. Run the report again as a user that has only catalogue and reports permissions 5. Confirm the batch operations drop down only includes Add to list 6. Confirm the records drop down only includes View record -- 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=35905 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch CC| |blawlor@clamsnet.org Assignee|koha-bugs@lists.koha-commun |blawlor@clamsnet.org |ity.org | -- 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=35905 Roman Dolny <roman.dolny@jezuici.pl> 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=35905 Roman Dolny <roman.dolny@jezuici.pl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #169260|0 |1 is obsolete| | --- Comment #2 from Roman Dolny <roman.dolny@jezuici.pl> --- Created attachment 169263 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=169263&action=edit Bug 35905: Add permission checks to report results Test plan: 1. Apply patch 2. Create a report like: select biblionumber from biblio limit 10 2. Run the report 3. Verify batch operations dropdown contains batch modification and deletion 4. Run the report again as a user that has only catalogue and reports permissions 5. Confirm the batch operations drop down only includes Add to list 6. Confirm the records drop down only includes View record Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35905 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35905 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #169263|0 |1 is obsolete| | --- Comment #3 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 170440 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=170440&action=edit Bug 35905: Add permission checks to report results Test plan: 1. Apply patch 2. Create a report like: select biblionumber from biblio limit 10 2. Run the report 3. Verify batch operations dropdown contains batch modification and deletion 4. Run the report again as a user that has only catalogue and reports permissions 5. Confirm the batch operations drop down only includes Add to list 6. Confirm the records drop down only includes View record Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> 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=35905 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | 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=35905 --- Comment #4 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- If I read this correctly, this seems not quite right: [% IF ( CAN_user_editcatalogue_edit_catalogue || CAN_user_editcatalogue_edit_items ) %] For "Edit record". I believe the edit_items is not right here. The page seems only be accessible with either fast_cataloging or edit_catalogue. I think edit_catalogue is the one here. Can you please revise? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35905 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35905 --- Comment #5 from Brendan Lawlor <blawlor@clamsnet.org> --- (In reply to Katrin Fischer from comment #4)
If I read this correctly, this seems not quite right:
[% IF ( CAN_user_editcatalogue_edit_catalogue || CAN_user_editcatalogue_edit_items ) %]
For "Edit record".
I believe the edit_items is not right here. The page seems only be accessible with either fast_cataloging or edit_catalogue. I think edit_catalogue is the one here.
Can you please revise?
Thanks Katrin! You are right there is something off here. I think it may need some more logic to handle all the different links that are automatically rendered in report results. I added edit_items here for reports that have the itemnumber in them. Item numbers link to /cataloguing/additem.pl?op=edititem&itemnumber=1 where i think edit_items is maybe the least permission needed to access that page, but I'm not sure if that's exactly correct either. I just realized that the same issue exists for reports with brrowernumber or cardnumber in them too. It's been a while since I looked at this code in detail, but I will give it another shot. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35905 --- Comment #6 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- What we could also consider: Fixing the batch link here and moving the other permission checks to another report if that makes things easier. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35905 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35905 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #170440|0 |1 is obsolete| | --- Comment #7 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 170634 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=170634&action=edit Bug 35905: Add permission checks to report results Test plan: 1. Apply patch 2. Create a report like: select biblionumber from biblio limit 10 2. Run the report 3. Verify batch operations dropdown contains batch modification and deletion 4. Run the report again as a user that has only catalogue and reports permissions 5. Confirm the batch operations drop down only includes Add to list 6. Confirm the records drop down only includes View record Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> 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=35905 --- Comment #8 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 170635 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=170635&action=edit Bug 35905: (follow-up) Add permission checks to report results -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35905 --- Comment #9 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 170636 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=170636&action=edit Bug 35905: (follow-up) Add permission checks to report reuslts To test: 1. Create the following two reports a) select borrowernumber, cardnumber from borrowers limit 100 b) select itemnumber, biblionumber from items limit 100 2. Create a user with only catalogue and reports permissions 3. Run each report as the user with minimal permissions 3. Verify the batch operations button is disabled 4. Verify there is no link to edit patron in borrowernumber columns 5. Verify there is no link to edit record in itemnumber and biblionumber columns 6. Add the following permissions one at a time and re run the reports tools_items_batchmod tools_items_batchdel editcatalogue_edit_catalogue editcatalogue_edit_items tools_edit_patrons borrowers_edit_borrowers 7. Verify the batch operations button is enabled and displays the correct links 8. Verify the links to edit patron and edit record are displayed correctly -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35905 --- Comment #10 from Brendan Lawlor <blawlor@clamsnet.org> --- This update fixes the permission checks for all batch operations and edit links. I think technically a user could still hit a unauthorized page if they don't have permissions to view patron records or to circulate, but that's probably less of an issue. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35905 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35905 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|Trivial patch |Small patch Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35905 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #170634|0 |1 is obsolete| | Attachment #170635|0 |1 is obsolete| | Attachment #170636|0 |1 is obsolete| | --- Comment #11 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 171105 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=171105&action=edit Bug 35905: Add permission checks to report results There are some links on the report results page that the user may not have access to This patch adds permission checks to the report results page to: 1. Disable the batch operations button or remove its dropdown items 2. Remove automatic links from the itemnumber, biblionumber, borrowernumber, and cardnumber columns 3. Remove the 'Hide/Show data menus' button When the user doen't have permission to those actions To test: 1. Create the following two reports a) select borrowernumber, cardnumber from borrowers limit 100 b) select itemnumber, biblionumber from items limit 100 2. Create a user with only catalogue and reports permissions 3. Run each report as the user with minimal permissions 3. Verify the batch operations button is disabled 4. Verify there is no link to edit, view or checkout patron in borrowernumber columns 5. Verify there is no link to edit record in itemnumber and biblionumber columns 6. Add the following permissions one at a time and re run the reports tools_items_batchmod tools_items_batchdel tools_records_batchmod tools_records_batchdel tools_edit_patrons editcatalogue_edit_catalogue editcatalogue_edit_items borrowers_edit_borrowers 7. Verify the batch operations button is enabled and displays the correct links 8. Verify the links in borrowernumber and cardnumber columns to view, edit and checkout patron are displayed correctly 9. Verify the links in itemnumber and biblionumber columns to edit record are displayed correctly -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35905 --- Comment #12 from Brendan Lawlor <blawlor@clamsnet.org> --- I ended up adding a checks circulate_remaining_permissions so I believe this now covers all links on the reports results page that a user might not have permission to access. But in the test plan I forgot to mention testing circulate_remaining_permissions for the checkout links -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35905 Olivier V <ovezina@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |ovezina@inlibro.com --- Comment #13 from Olivier V <ovezina@inlibro.com> --- In the batch operations button, i am still getting permission issues with the Patron card creation. If i don't give the label creation permission and click on the button, i am redirected to a login page saying i don't have the permission to access the page. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35905 --- Comment #14 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Olivier V from comment #13)
In the batch operations button, i am still getting permission issues with the Patron card creation. If i don't give the label creation permission and click on the button, i am redirected to a login page saying i don't have the permission to access the page.
On first glance that seems the correct bahvior. Patron cards use the label creator permission. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35905 --- Comment #15 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Katrin Fischer from comment #14)
(In reply to Olivier V from comment #13)
In the batch operations button, i am still getting permission issues with the Patron card creation. If i don't give the label creation permission and click on the button, i am redirected to a login page saying i don't have the permission to access the page.
On first glance that seems the correct bahvior. Patron cards use the label creator permission.
OH! I see what you mean, but I think that's outside of scope here and should go on a separate bug :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35905 --- Comment #16 from Brendan Lawlor <blawlor@clamsnet.org> --- For the patron card creation, I wasn't sure what the correct permission was. When I was testing I think it worked with the permission tools_edit_patrons. If the least permission for that is actually tools_label_creator I can update the patch for that. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35905 Olivier V <ovezina@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #171105|0 |1 is obsolete| | --- Comment #17 from Olivier V <ovezina@inlibro.com> --- Created attachment 171492 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=171492&action=edit Bug 35905: Add permission checks to report results There are some links on the report results page that the user may not have access to This patch adds permission checks to the report results page to: 1. Disable the batch operations button or remove its dropdown items 2. Remove automatic links from the itemnumber, biblionumber, borrowernumber, and cardnumber columns 3. Remove the 'Hide/Show data menus' button When the user doen't have permission to those actions To test: 1. Create the following two reports a) select borrowernumber, cardnumber from borrowers limit 100 b) select itemnumber, biblionumber from items limit 100 2. Create a user with only catalogue and reports permissions 3. Run each report as the user with minimal permissions 3. Verify the batch operations button is disabled 4. Verify there is no link to edit, view or checkout patron in borrowernumber columns 5. Verify there is no link to edit record in itemnumber and biblionumber columns 6. Add the following permissions one at a time and re run the reports tools_items_batchmod tools_items_batchdel tools_records_batchmod tools_records_batchdel tools_edit_patrons editcatalogue_edit_catalogue editcatalogue_edit_items borrowers_edit_borrowers 7. Verify the batch operations button is enabled and displays the correct links 8. Verify the links in borrowernumber and cardnumber columns to view, edit and checkout patron are displayed correctly 9. Verify the links in itemnumber and biblionumber columns to edit record are displayed correctly Signed-off-by: Olivier V <olivier.vezina@inLibro.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35905 Olivier V <ovezina@inlibro.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=35905 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #18 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- + [% IF ( batch_biblionumbers || batch_itemnumbers ) || ( batch_cardnumbers || batch_borrowernumbers && (CAN_user_borrowers_list_borrowers || CAN_user_borrowers_edit_borrowers || CAN_user_circulate_circulate_remaining_permissions ) ) %] Please explain this change. Why didnt we have batch_borrowernumbers before? Are you missing parentheses around batch_card OR batch_borrower ? - [% ELSIF header_row.$place.cell == 'cardnumber' || header_types.item(header_row.$place.cell) == 'cardnumber' %] + [% ELSIF CAN_user_circulate_circulate_remaining_permissions && header_row.$place.cell == 'cardnumber' || header_types.item(header_row.$place.cell) == 'cardnumber' %] Are we missing parentheses here ? -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org