[Bug 7002] New: Inaccurate checks for borrower flags == 1
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7002 Bug #: 7002 Summary: Inaccurate checks for borrower flags == 1 Classification: Unclassified Change sponsored?: --- Product: Koha Version: master Platform: All OS/Version: All Status: NEW Severity: normal Priority: P5 Component: Authentication AssignedTo: gmcharlt@gmail.com ReportedBy: oleonard@myacpl.org QAContact: koha-bugs@lists.koha-community.org CC: dpavlin@rot13.org There are a few instances in the code which I can find which use this to check whether someone has superlibrarian privileges in the context of IndependandBranches: unless (($userenv->{'flags'} == 1) or ($userenv->{'branch'} eq $item->{'homebranch'})) { This check fails if the logged in user has any other privilege checked in addition to superlibrarian. In other words, the user must *only* have superlibrarian checked in order to qualify to perform these operations. catalogue\moredetail.pl line 136 cataloguing\additem.pl line 548 tools\batchMod.pl line 460 -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7002 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |6390 -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7002 Ian Walls <ian.walls@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |ian.walls@bywatersolutions. | |com --- Comment #1 from Ian Walls <ian.walls@bywatersolutions.com> 2011-10-11 15:33:55 UTC --- To check for superlibrarian (or any top-level permission), you need to use bitwise AND. ($user->{'flags'} & 1) should return true of the user has superlibrarian checked, and false otherwise. C4/Serials.pm does similar erroneous checks for ($user->{'flags'} != 1), which will also need modification. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7002 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=10277 -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7002 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #2 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hm, I did a quick search and found some occurences of the problematic syntax in the current code: catalogue/moredetail.pl 178 unless (($userenv->{'flags'} == 1) or ($userenv->{'branch'} eq $item->{'homebranch'})) { cataloguing/additem.pl 695 unless (($userenv->{'flags'} == 1) or (($userenv->{'branch'} eq $subfieldvalue))){ tools/batchMod.pl 498 unless (($userenv->{'flags'} == 1) or (($userenv->{'branch'} eq $itembranchcode))){ -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7002 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |10277 -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7002 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Small patch -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7002 --- Comment #3 from Galen Charlton <gmcharlt@gmail.com> --- Created attachment 23874 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23874&action=edit Bug 7002: fix some invalid superlibrarian permission checks This patch fixes a problem where if a staff user has superlibrarian permissions, but also has module-specific permissions, they are prevent from editing item records that they should be allowed to. To test: [1] Turn on IndependentBranches. [2] Register a superlibrarian staff user at branch A. [3] Give that new account at least one other module-level permission. This cannot be done through the user interface, however, but can be done via SQL: UPDATE borrowers SET flags = 3 WHERE userid = 'XXX'; [4] Log in as that new superlibrarian. [5] Bring up the item details (catalogue/moredetail.pl) page for an item at branch B. Note that there is no 'Edit Item' link. [6] Similarly, try editing that item (cataloging/additem.pl). Note that the edit form forbids you from touching the item. [7] Finally, try editing that item using the Tools | Batch item modification utility. Note that it doesn't allow you to do so. [8] Apply the patch. [9] Repeat steps 5 through 7. This time, the item actions should be allowed. Signed-off-by: Galen Charlton <gmc@esilibrary.com> -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7002 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7002 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #23874|0 |1 is obsolete| | --- Comment #4 from Chris Cormack <chris@bigballofwax.co.nz> --- Created attachment 23887 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23887&action=edit Bug 7002: fix some invalid superlibrarian permission checks This patch fixes a problem where if a staff user has superlibrarian permissions, but also has module-specific permissions, they are prevent from editing item records that they should be allowed to. To test: [1] Turn on IndependentBranches. [2] Register a superlibrarian staff user at branch A. [3] Give that new account at least one other module-level permission. This cannot be done through the user interface, however, but can be done via SQL: UPDATE borrowers SET flags = 3 WHERE userid = 'XXX'; [4] Log in as that new superlibrarian. [5] Bring up the item details (catalogue/moredetail.pl) page for an item at branch B. Note that there is no 'Edit Item' link. [6] Similarly, try editing that item (cataloging/additem.pl). Note that the edit form forbids you from touching the item. [7] Finally, try editing that item using the Tools | Batch item modification utility. Note that it doesn't allow you to do so. [8] Apply the patch. [9] Repeat steps 5 through 7. This time, the item actions should be allowed. Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7002 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7002 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #23887|0 |1 is obsolete| | --- Comment #5 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 23907 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=23907&action=edit [PASSED QA] Bug 7002: fix some invalid superlibrarian permission checks This patch fixes a problem where if a staff user has superlibrarian permissions, but also has module-specific permissions, they are prevent from editing item records that they should be allowed to. To test: [1] Turn on IndependentBranches. [2] Register a superlibrarian staff user at branch A. [3] Give that new account at least one other module-level permission. This cannot be done through the user interface, however, but can be done via SQL: UPDATE borrowers SET flags = 3 WHERE userid = 'XXX'; [4] Log in as that new superlibrarian. [5] Bring up the item details (catalogue/moredetail.pl) page for an item at branch B. Note that there is no 'Edit Item' link. [6] Similarly, try editing that item (cataloging/additem.pl). Note that the edit form forbids you from touching the item. [7] Finally, try editing that item using the Tools | Batch item modification utility. Note that it doesn't allow you to do so. [8] Apply the patch. [9] Repeat steps 5 through 7. This time, the item actions should be allowed. Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Works as described, passes QA script and test suite. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7002 Galen Charlton <gmcharlt@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #6 from Galen Charlton <gmcharlt@gmail.com> --- Pushed to master. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7002 --- Comment #7 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Thanks Galen :) -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7002 Fridolin SOMERS <fridolyn.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolyn.somers@biblibre.co | |m --- Comment #8 from Fridolin SOMERS <fridolyn.somers@biblibre.com> --- Will be for 3.16 because depends on enhancement Bug 10277 -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7002 Fridolin SOMERS <fridolyn.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable --- Comment #9 from Fridolin SOMERS <fridolyn.somers@biblibre.com> --- Pushed to 3.14.x, will be in 3.14.10 I decided in the end to integrate Bug 10277 so also this one. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org