Bug 30387 - IndependentBranches allows users to change libraries when they should not be able to
Summary: IndependentBranches allows users to change libraries when they should not be ...
Status: CLOSED INVALID
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Lucas Gass
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 29887
  Show dependency treegraph
 
Reported: 2022-03-28 16:28 UTC by Lucas Gass
Modified: 2023-06-08 22:26 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 30387: Only allow superlibrarians to set library with IndependentBranches enabled (4.22 KB, patch)
2022-03-28 16:59 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30387: Only allow superlibrarians to set library with IndependentBranches enabled (4.29 KB, patch)
2022-04-26 19:35 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 30387: Only allow superlibrarians to set library with IndependentBranches enabled (4.34 KB, patch)
2022-04-26 20:11 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lucas Gass 2022-03-28 16:28:08 UTC
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.
Comment 1 Lucas Gass 2022-03-28 16:59:57 UTC
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.
Comment 2 Andrew Fuerste-Henry 2022-04-26 19:09:40 UTC
Test plan failed on step 10. As a non-superlibrarian, I get the Set Desks page, but no content in the Desks dropdown.
Comment 3 Andrew Fuerste-Henry 2022-04-26 19:21:10 UTC
Nevermind, user error. Back to NSO.
Comment 4 Andrew Fuerste-Henry 2022-04-26 19:35:12 UTC
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>
Comment 5 Andrew Fuerste-Henry 2022-04-26 20:11:35 UTC
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>
Comment 6 Martin Renvoize 2022-04-27 17:43:40 UTC
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.
Comment 7 Jonathan Druart 2022-04-28 07:53:45 UTC
(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).
Comment 8 Katrin Fischer 2022-04-28 16:52:30 UTC
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.
Comment 9 Martin Renvoize 2022-04-29 13:04:22 UTC
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.
Comment 10 Katrin Fischer 2022-06-12 11:40:28 UTC
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.
Comment 11 Lucas Gass 2022-06-13 22:18:51 UTC
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.