[Bug 28882] New: Incorrect permissions check client-side
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28882 Bug ID: 28882 Summary: Incorrect permissions check client-side Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Patrons Assignee: koha-bugs@lists.koha-community.org Reporter: jonathan.druart+koha@gmail.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com -- 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=28882 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |20100, 22150 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20100 [Bug 20100] A non-superlibrarian should not be able to add superlibrarian privileges https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22150 [Bug 22150] Make it easier to unselect one member permission after selecting all -- 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=28882 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |jonathan.druart+koha@gmail. |ity.org |com Status|NEW |ASSIGNED Severity|enhancement |normal -- 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=28882 --- Comment #1 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 123972 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123972&action=edit Bug 28882: permission UI allows selection of superlibrarian permission With bug 20100 and 22150 we allow to select all the permissions but the superlibrarian permission, to ease the selection. And we also forbid a non-superlibrarian user to add superlibrarian permission. However there is something wrong in the JS code and it's possible to add the superlibrarian permission. The user is getting an ugly 500 and so the permission change is not done, but the UI checks must be fixed. To recreate: Login with a non-superlibrarian user Edit permission Clear all => You can select the "superlibrarian" permission Test plan: Login with a non-superlibrarian user Try to set the superlibrarian permissions to a user => not possible Try the select all/clear all => still cannot set the superlibrarian permission Work to be done: Login with a non-superlibrarian user Edit permissions for a superlibrarian user => You can remove it, then cannot add it back Should we allow removal of superlibrarian permission by non-superlibrarian user? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28882 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |In Discussion CC| |katrin.fischer@bsz-bw.de, | |m.de.rooy@rijksmuseum.nl, | |oleonard@myacpl.org --- Comment #2 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Can you have a look at the "work to be done" in the last patch please? What's your opinion? Did we discuss it already? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28882 --- Comment #3 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Work to be done: Login with a non-superlibrarian user Edit permissions for a superlibrarian user => You can remove it, then cannot add it back Should we allow removal of superlibrarian permission by non-superlibrarian user? No, I definitely think that a non-superlib should not be able to remove perms from a superlib. That would be kind of a security hole imo. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28882 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28882 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28882 --- Comment #4 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 124082 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124082&action=edit Bug 28882: Disable select/hide all links We want to prevent non-superlibrarian users to remove superlibrarian flag. It's handled already in the controller, but needs to be done client-side. Here we are disabling the select/hide all links to remove the remaining possibility to remove the superlibrarian flag from non-superlibrarian users. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28882 Hayley Pelham <hayleypelham@catalyst.net.nz> 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=28882 Hayley Pelham <hayleypelham@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #123972|0 |1 is obsolete| | --- Comment #5 from Hayley Pelham <hayleypelham@catalyst.net.nz> --- Created attachment 124443 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124443&action=edit Bug 28882: permission UI allows selection of superlibrarian permission With bug 20100 and 22150 we allow to select all the permissions but the superlibrarian permission, to ease the selection. And we also forbid a non-superlibrarian user to add superlibrarian permission. However there is something wrong in the JS code and it's possible to add the superlibrarian permission. The user is getting an ugly 500 and so the permission change is not done, but the UI checks must be fixed. To recreate: Login with a non-superlibrarian user Edit permission Clear all => You can select the "superlibrarian" permission Test plan: Login with a non-superlibrarian user Try to set the superlibrarian permissions to a user => not possible Try the select all/clear all => still cannot set the superlibrarian permission Work to be done: Login with a non-superlibrarian user Edit permissions for a superlibrarian user => You can remove it, then cannot add it back Should we allow removal of superlibrarian permission by non-superlibrarian user? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28882 Hayley Pelham <hayleypelham@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124082|0 |1 is obsolete| | Attachment #124443|0 |1 is obsolete| | --- Comment #6 from Hayley Pelham <hayleypelham@catalyst.net.nz> --- Created attachment 124444 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124444&action=edit Bug 28882: permission UI allows selection of superlibrarian permission With bug 20100 and 22150 we allow to select all the permissions but the superlibrarian permission, to ease the selection. And we also forbid a non-superlibrarian user to add superlibrarian permission. However there is something wrong in the JS code and it's possible to add the superlibrarian permission. The user is getting an ugly 500 and so the permission change is not done, but the UI checks must be fixed. To recreate: Login with a non-superlibrarian user Edit permission Clear all => You can select the "superlibrarian" permission Test plan: Login with a non-superlibrarian user Try to set the superlibrarian permissions to a user => not possible Try the select all/clear all => still cannot set the superlibrarian permission Work to be done: Login with a non-superlibrarian user Edit permissions for a superlibrarian user => You can remove it, then cannot add it back Should we allow removal of superlibrarian permission by non-superlibrarian user? Signed-off-by: Hayley Pelham <hayleypelham@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28882 --- Comment #7 from Hayley Pelham <hayleypelham@catalyst.net.nz> --- Created attachment 124445 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124445&action=edit Bug 28882: Disable select/hide all links We want to prevent non-superlibrarian users to remove superlibrarian flag. It's handled already in the controller, but needs to be done client-side. Here we are disabling the select/hide all links to remove the remaining possibility to remove the superlibrarian flag from non-superlibrarian users. Signed-off-by: Hayley Pelham <hayleypelham@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28882 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=28882 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124444|0 |1 is obsolete| | --- Comment #8 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 124558 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124558&action=edit Bug 28882: permission UI allows selection of superlibrarian permission With bug 20100 and 22150 we allow to select all the permissions but the superlibrarian permission, to ease the selection. And we also forbid a non-superlibrarian user to add superlibrarian permission. However there is something wrong in the JS code and it's possible to add the superlibrarian permission. The user is getting an ugly 500 and so the permission change is not done, but the UI checks must be fixed. To recreate: Login with a non-superlibrarian user Edit permission Clear all => You can select the "superlibrarian" permission Test plan: Login with a non-superlibrarian user Try to set the superlibrarian permissions to a user => not possible Try the select all/clear all => still cannot set the superlibrarian permission Work to be done: Login with a non-superlibrarian user Edit permissions for a superlibrarian user => You can remove it, then cannot add it back Should we allow removal of superlibrarian permission by non-superlibrarian user? Signed-off-by: Hayley Pelham <hayleypelham@catalyst.net.nz> 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=28882 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124445|0 |1 is obsolete| | --- Comment #9 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 124559 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124559&action=edit Bug 28882: Disable select/hide all links We want to prevent non-superlibrarian users to remove superlibrarian flag. It's handled already in the controller, but needs to be done client-side. Here we are disabling the select/hide all links to remove the remaining possibility to remove the superlibrarian flag from non-superlibrarian users. Signed-off-by: Hayley Pelham <hayleypelham@catalyst.net.nz> 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=28882 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 | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28882 --- Comment #10 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- What remains for me is the question: Should we allow a non-superlibrarian to give him or herself the privilege to change systemprefs? Because you can disable ProtectSuperlib etc.. But this should be solved on another report imo. Should we only protect that pref, or a specific set of prefs, or actually not allow changing sysprefs at all by non superlibs? I tend to choose the latter. Even a few other options on the Admin menu like Koha to MARC mapping (among others) might be candidates too for restricting them to superlibrarians only. What do you think? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28882 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=28882 --- Comment #11 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=28882 --- Comment #12 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Marcel de Rooy from comment #10)
What remains for me is the question: Should we allow a non-superlibrarian to give him or herself the privilege to change systemprefs? Because you can disable ProtectSuperlib etc.. But this should be solved on another report imo.
Should we only protect that pref, or a specific set of prefs, or actually not allow changing sysprefs at all by non superlibs? I tend to choose the latter.
I don't think shutting out non-superlibrarians from the admin tools/system preferences is a feasible solution. That forces you to give out superlibrarian permissions to more people instead of making it a protected circle. We just added granular permissions for administration areas - that would make them practically useless. Maybe the ProtectSuperLib should not be a pref in the first place?
Even a few other options on the Admin menu like Koha to MARC mapping (among others) might be candidates too for restricting them to superlibrarians only. What do you think?
We do already have separate permissions for all those areas, I believe adding dependencies like that, will only make the system more intransparent. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28882 --- Comment #13 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Katrin Fischer from comment #12) Thx for your feedback.
I don't think shutting out non-superlibrarians from the admin tools/system preferences is a feasible solution. That forces you to give out superlibrarian permissions to more people instead of making it a protected circle. We just added granular permissions for administration areas - that would make them practically useless.
It might be helpful if we could distinguish between viewing sysprefs and changing them. And along those lines, a small set of sysprefs could somehow be set apart for changes only by superlibrarian. Perhaps even configurable by sysadmin. The gap in the changing permissions form is that we allow a staff user to set and clear permissions that he does not have himself. Which is a bit awkward, especially for Admininstration. I think that we should give that a bit more thought.
Maybe the ProtectSuperLib should not be a pref in the first place?
Currently, a lot of devs need a preference to come in since we are too afraid to change 'current behavior'. This pref could be marked as privileged. See before. I will open a few reports for these things. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28882 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=28991 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28882 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=28990 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28882 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=28989 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28882 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com Version(s)|21.11.00 |21.11.00,21.05.04 released in| | Status|Pushed to master |Pushed to stable --- Comment #14 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to 21.05.x for 21.05.04 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28882 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #15 from Fridolin Somers <fridolin.somers@biblibre.com> --- Depends on Bug B22150 not in 20.11.x -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org