Bug 20707 - Permissions for circ/ysearch.pl override specific page level permissions and delete sessions improperly
Summary: Permissions for circ/ysearch.pl override specific page level permissions and ...
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-05-03 20:08 UTC by Nick Clemens
Modified: 2018-10-24 09:13 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 20707: Replace circ/search.pl circulate permission requirement with catalogue (1.51 KB, patch)
2018-05-04 13:59 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 20707: Replace circ/search.pl circulate permission requirement with catalogue (1.58 KB, patch)
2018-05-10 14:06 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 20707: Replace circ/search.pl circulate permission requirement with catalogue (1.63 KB, patch)
2018-05-10 19:02 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2018-05-03 20:08:54 UTC
To recreate:
1 - Have a user with permissions:
catalogue
reserveforothers
and more..
but not:
borrowers
permissions

2 - Attempt to place a hold on the staff client as that user
3 - Note that it goes poorly

In 17.11 we observed the user being kicked out when searching for a patron
In master I observed the user being kicked out when placing the hold

Oddly there isn't a 'You don't have access warning' just a sign in page
Comment 1 Nick Clemens 2018-05-04 13:52:47 UTC
With Kyle's help we tracked this down:

when placing a hold if you trigger the autocomplete i.e. ysearch.pl you are logged out as not having permissions
circ/ysearch requires circulate => '*'
whereas
request.pl requires reserveforothers => 'place_holds'

this is also true for course reserves - searching for an instructor will log the user out unless they have circulate permissions.

tags-review uses it as well

I think the most straightforward route is to remove the circulate permission check from ysearch and require simply catalogue.
Comment 2 Nick Clemens 2018-05-04 13:59:30 UTC Comment hidden (obsolete)
Comment 3 Nick Clemens 2018-05-04 14:21:52 UTC
Note: Workaround is to disable CircAutoCompl
Comment 4 Jonathan Druart 2018-05-07 14:07:35 UTC
So it's not critical finally?
Comment 5 Owen Leonard 2018-05-10 14:06:05 UTC
Created attachment 75220 [details] [review]
Bug 20707: Replace circ/search.pl circulate permission requirement with catalogue

To test:
 1 - Setup a staff patron with permissions:
   - catalogue
   - reserveforothers
   - course_reserves
 2 - Log in to staff client as that patron
 3 - Find a record and click to place a hold
 4 - Type more than three letters into the search bar but don't submit
 5 - Note that you will not receive autocomplete results
 6 - Either submit, or try to visit any page in staff client
 7 - Your session has been expired, you must log in again
 8 - Log in, go to course reserves
 9 - As before, trigger the autocomplete search in the instrcutor field
10 - Again your session has been terminated
11 - Apply patch
12 - Repeat above actions, this time you will not be kicked out

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 6 Katrin Fischer 2018-05-10 19:02:21 UTC
Created attachment 75242 [details] [review]
Bug 20707: Replace circ/search.pl circulate permission requirement with catalogue

To test:
 1 - Setup a staff patron with permissions:
   - catalogue
   - reserveforothers
   - course_reserves
 2 - Log in to staff client as that patron
 3 - Find a record and click to place a hold
 4 - Type more than three letters into the search bar but don't submit
 5 - Note that you will not receive autocomplete results
 6 - Either submit, or try to visit any page in staff client
 7 - Your session has been expired, you must log in again
 8 - Log in, go to course reserves
 9 - As before, trigger the autocomplete search in the instrcutor field
10 - Again your session has been terminated
11 - Apply patch
12 - Repeat above actions, this time you will not be kicked out

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 7 Jonathan Druart 2018-05-11 17:12:48 UTC
Pushed to master for 18.05, thanks to everybody involved!