When IndependentBranches is turned on it should prevent all non-superlibrarians from changing their logged in branch. There are a few places where it incorrectly will allow non-superlibrarians with CAN_user_editcatalogue_edit_catalogue permissions to change their library. -circ-nav.inc -header.inc -set-library.tt With IndependentBranches turned on only super librarians should be allowed to change libraries.
Created attachment 132352 [details] [review] Bug 30387: Only allow superlibrarians to set library with IndependentBranches enabled To test: 1. Turn on IndependentBranches. 2. Give a staff member editcatalogue permissions but not superlibrarian permissions. 3. Once logged in as that staff member click on the name in the top right corner, notice that the 'Set library' button is there. 4. Turn on the CircSidebar system preference and go to any page that includes the CircSidebar. ( returns.tt, circulation,tt ). Notcie the Set library button is visibile. 5. Go to '/cgi-bin/koha/circ/set-library.pl', notice you can set your library to something else. 6. Apply patch 7. Again with the staff member who has editcatalogue permissions but not superlibrarian permissions try steps 3-5. This time you should not see the 'Set library' links. On '/cgi-bin/koha/circ/set-library.pl' you should not be able to change your library. 8. With a superlibrarian try steps 3-5, you should see the 'Set library' links and on '/cgi-bin/koha/circ/set-library.pl' you should be able to set your library 9. Turn on 'UseCirculationDesks'. 10. When repeating 3-5 make sure you can see only the 'Set desk' links while logged in as the non-superlibrarian. On '/cgi-bin/koha/circ/set-library.pl' you should only be able to 'Set desk'. 11. Now with a staff member who has neither superlibrarian permissions OR editcatalogue permissions do steps 3-5. You should NOT see the 'Set library links' but you should see the 'Set desk' links.
Test plan failed on step 10. As a non-superlibrarian, I get the Set Desks page, but no content in the Desks dropdown.
Nevermind, user error. Back to NSO.
Created attachment 133990 [details] [review] Bug 30387: Only allow superlibrarians to set library with IndependentBranches enabled To test: 1. Turn on IndependentBranches. 2. Give a staff member editcatalogue permissions but not superlibrarian permissions. 3. Once logged in as that staff member click on the name in the top right corner, notice that the 'Set library' button is there. 4. Turn on the CircSidebar system preference and go to any page that includes the CircSidebar. ( returns.tt, circulation,tt ). Notcie the Set library button is visibile. 5. Go to '/cgi-bin/koha/circ/set-library.pl', notice you can set your library to something else. 6. Apply patch 7. Again with the staff member who has editcatalogue permissions but not superlibrarian permissions try steps 3-5. This time you should not see the 'Set library' links. On '/cgi-bin/koha/circ/set-library.pl' you should not be able to change your library. 8. With a superlibrarian try steps 3-5, you should see the 'Set library' links and on '/cgi-bin/koha/circ/set-library.pl' you should be able to set your library 9. Turn on 'UseCirculationDesks'. 10. When repeating 3-5 make sure you can see only the 'Set desk' links while logged in as the non-superlibrarian. On '/cgi-bin/koha/circ/set-library.pl' you should only be able to 'Set desk'. 11. Now with a staff member who has neither superlibrarian permissions OR editcatalogue permissions do steps 3-5. You should NOT see the 'Set library links' but you should see the 'Set desk' links. Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Created attachment 134002 [details] [review] Bug 30387: Only allow superlibrarians to set library with IndependentBranches enabled To test: 1. Turn on IndependentBranches. 2. Give a staff member editcatalogue permissions but not superlibrarian permissions. 3. Once logged in as that staff member click on the name in the top right corner, notice that the 'Set library' button is there. 4. Turn on the CircSidebar system preference and go to any page that includes the CircSidebar. ( returns.tt, circulation,tt ). Notcie the Set library button is visibile. 5. Go to '/cgi-bin/koha/circ/set-library.pl', notice you can set your library to something else. 6. Apply patch 7. Again with the staff member who has editcatalogue permissions but not superlibrarian permissions try steps 3-5. This time you should not see the 'Set library' links. On '/cgi-bin/koha/circ/set-library.pl' you should not be able to change your library. 8. With a superlibrarian try steps 3-5, you should see the 'Set library' links and on '/cgi-bin/koha/circ/set-library.pl' you should be able to set your library 9. Turn on 'UseCirculationDesks'. 10. When repeating 3-5 make sure you can see only the 'Set desk' links while logged in as the non-superlibrarian. On '/cgi-bin/koha/circ/set-library.pl' you should only be able to 'Set desk'. 11. Now with a staff member who has neither superlibrarian permissions OR editcatalogue permissions do steps 3-5. You should NOT see the 'Set library links' but you should see the 'Set desk' links. Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org>
Hmm, wasn't there actually a reason cataloguers needed to be able to change library? I have a feeling this was a feature rather than a bug as I've asked this question before.. ccing people to help dredge the hive mind.
(In reply to Martin Renvoize from comment #6) > Hmm, wasn't there actually a reason cataloguers needed to be able to change > library? I have a feeling this was a feature rather than a bug as I've > asked this question before.. ccing people to help dredge the hive mind. It's the purpose of bug 29887 if I understand correctly. This patch should provide a change at the controller level (and not only hide the link in the template).
I think Joubu is right about that the change needs to be not only in the templates, but secured server side as well. I think we'd definitely need to push this together with 29887 to make sure we don't break someones use case by closing this loop-holes. Would it make sense to test both patches together? As this is now SO I have moved bug 29887 back into the QA queue.
So I dug through the history.. it was indeed a deliberate feature introduced in bug 6841.. though the bug is sparse of details.. the commit is a little clearer: https://git.koha-community.org/Koha-community/Koha/commit/c2015c710355a0ab81306515a70af29e6aef3504 Honestly, I'm not close enough IndependantBranches to really wrap my head around the objectives as a whole.. but it sounds like a reasonable change to me generally.
Freeing this one from 'In Discussion' and moving it to Failed QA for comment#7: We need these changes not only in templates, but also server side.
Since we need to move this logic from the system preference IndependentBranchesLoggedInLibrary into a permission, I am resolving this bug and will work on adding the permission in Bug 30624.