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
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.
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))){
Created attachment 23874 [details] [review] 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>
Created attachment 23887 [details] [review] 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>
Created attachment 23907 [details] [review] [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.
Pushed to master.
Thanks Galen :)
Will be for 3.16 because depends on enhancement Bug 10277
Pushed to 3.14.x, will be in 3.14.10 I decided in the end to integrate Bug 10277 so also this one.