Bug 14298 - AutoSelfCheckID user should only be able to access SCO
Summary: AutoSelfCheckID user should only be able to access SCO
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Self checkout (show other bugs)
Version: Main
Hardware: All All
: P5 - low critical
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-05-29 12:59 UTC by Magnus Enger
Modified: 2019-06-27 09:24 UTC (History)
6 users (show)

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


Attachments
Bug 14298: Add the new 'self_checkout' permissions (3.89 KB, patch)
2015-09-02 15:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14298: Use the new permission in the sco scripts (2.04 KB, patch)
2015-09-02 15:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14298: Add warning to the about page if the sco user does not have correct permissions (4.02 KB, patch)
2015-09-02 15:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14298: Add the new 'self_checkout' permissions (4.00 KB, patch)
2015-09-02 22:19 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 14298: Use the new permission in the sco scripts (2.22 KB, patch)
2015-09-02 22:20 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 14298: Add warning to the about page if the sco user does not have correct permissions (4.15 KB, patch)
2015-09-02 22:20 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 14298: Add perms for other languages (8.02 KB, patch)
2015-09-14 09:50 UTC, Jonathan Druart
Details | Diff | Splinter Review
[PASSED QA] Bug 14298: Add the new 'self_checkout' permissions (4.07 KB, patch)
2015-09-14 19:27 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 14298: Use the new permission in the sco scripts (2.29 KB, patch)
2015-09-14 19:27 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 14298: Add warning to the about page if the sco user does not have correct permissions (4.21 KB, patch)
2015-09-14 19:27 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 14298: Add perms for other languages (8.09 KB, patch)
2015-09-14 19:27 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 14298: Do not insert the permission if already exists. (1.08 KB, patch)
2015-11-25 13:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED OFF] Bug 14298: Do not insert the permission if already exists. (1.16 KB, patch)
2015-11-25 14:15 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 Magnus Enger 2015-05-29 12:59:58 UTC
This is how the problem was discovered:

Sysprefs:

 WebBasedSelfCheck = Enable
 AutoSelfCheckAllowed = Allow
 AutoSelfCheckID = username of user with circ permission
 AutoSelfCheckPass = password of the same user

- First go to http://koha/cgi-bin/koha/opac-main.pl and verify you are not logged in
- Then visit http://koha/cgi-bin/koha/sco/sco-main.pl (you do not have to do anything here)
- Then go back to http://koha/cgi-bin/koha/opac-main.pl
- You are logged in as the AutoSelfCheckID user

Now this user only has circ rights, so visiting the main door of the intranet gives an error: 
http://koha:8080/

But there is access to the circ parts of the intranet:
http://koha:8080/cgi-bin/koha/circ/circulation-home.pl

If you have any waitingreserves you can see a given book waiting for a given borrower:
http://koha:8080/cgi-bin/koha/circ/waitingreserves.pl

And if someone was a bit careless, this user could be a superlibrarian...

I *think* this is only a problem when the intranet runs on the same domain/different port as the OPAC, because when the two are on different domains I think you have to sign into each one individually?  

I have discussed this problem with tcohen.
Comment 1 Chris Cormack 2015-06-01 19:29:07 UTC
Yep cookies are tied to domains not ports. This will be a tricky one to fix it's essentially the same as an user. IE any user can do this if they have an opac login and the wrong permissions. The only difference is the sco user will definitely have circ permissions
Comment 2 Magnus Enger 2015-06-03 12:38:49 UTC
Could it be possible to not use the general login functionality of Koha for  AutoSelfCheckID? I guess that means not to set the same cookie as the regular login, for this user? That way it should not be possible to carry the login over to other parts of Koha.
Comment 3 Jonathan Druart 2015-07-10 11:38:45 UTC
Why not adding a new permission?
It could solve easily the problem I think.
Comment 4 Jonathan Druart 2015-07-10 11:40:52 UTC
We could even add a check on the permission page not to allow this new permission with others (and/or warn on the about page).
I suppose the user used to access SCO is not used as a real user.
Comment 5 Jonathan Druart 2015-07-13 13:47:52 UTC
*** Bug 10799 has been marked as a duplicate of this bug. ***
Comment 6 Jonathan Druart 2015-08-19 14:20:36 UTC
ping?
Comment 7 Magnus Enger 2015-08-24 06:55:04 UTC
(In reply to Jonathan Druart from comment #3)
> Why not adding a new permission?
> It could solve easily the problem I think.

Yeah, that sounds like it might be a nice way to solve it!
Comment 8 Jonathan Druart 2015-09-02 15:54:20 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2015-09-02 15:54:23 UTC Comment hidden (obsolete)
Comment 10 Jonathan Druart 2015-09-02 15:54:26 UTC Comment hidden (obsolete)
Comment 11 Jonathan Druart 2015-09-02 16:19:49 UTC
I am not familiar enough with this module to be confident with these patches, please be involved!
Comment 12 Chris Cormack 2015-09-02 22:01:04 UTC
Bug 10799 fixes the OPAC side of this.

I am testing the new permission fixes the staff side.
Comment 13 Chris Cormack 2015-09-02 22:19:57 UTC Comment hidden (obsolete)
Comment 14 Chris Cormack 2015-09-02 22:20:11 UTC Comment hidden (obsolete)
Comment 15 Chris Cormack 2015-09-02 22:20:28 UTC Comment hidden (obsolete)
Comment 16 Magnus Enger 2015-09-03 06:29:02 UTC
Thanks for the fix and the signoff!
Comment 17 Katrin Fischer 2015-09-06 20:57:57 UTC
This works alright, but please add the permission to the translated installers before this is being pushed so we don't create inconsistencies.
Comment 18 Jonathan Druart 2015-09-14 09:50:25 UTC Comment hidden (obsolete)
Comment 19 Jonathan Druart 2015-09-14 09:51:39 UTC
(In reply to Katrin Fischer from comment #17)
> This works alright, but please add the permission to the translated
> installers before this is being pushed so we don't create inconsistencies.

I didn't forget that, it was completely expected.
I always wait for someone to confirm the wording.
Comment 20 Katrin Fischer 2015-09-14 19:27:13 UTC
Created attachment 42536 [details] [review]
[PASSED QA] Bug 14298: Add the new 'self_checkout' permissions

There is a security issue in the self checkout module.
The user used to check items out must have the circulate =>
circulate_remaining_permissions permissions.
So even if a user does not have a login/password or a barcode he cans
access to the circulation module of the intranet.
Imagine if the sco patron used is a superlibrarian...

This patch set will change the behavior and adds a new permission to
access to the sco module (circulate => self_checkout).
This permission should be the only one defined for this patron.

IMPORTANT NOTE: Hopefully, this only works if both interfaces use the
same domains (but different ports).

Test plan:
0/ Does not apply this patch set
1/ Create a patron with the circulate => circulate_remaining_permissions
and some others. Note his userid/pwd (later 'sco/sco').
Turn on WebBasedSelfCheck and AutoSelfCheckAllowed
Fill the AutoSelfCheckID and AutoSelfCheckPass wich 'sco' and 'sco'
2/ Log you out from the OPAC and the intranet
3/ Go on the sco page
4/ Note that your are automatically logged in
5/ Go on the circulation module on the intranet side
6/ Oops
7/ Apply this patch
8/ Execute the updatedatabase
9/ Note that the sco user only has the new permission circulate =>
self_checkout, others have been removed
10/ Try to reproduce the issue, it should not access anything on the
intranet side
11/ Confirm that there is no regression in the sco module

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Works well no regressions, changes the permissions appropriately.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 21 Katrin Fischer 2015-09-14 19:27:16 UTC
Created attachment 42537 [details] [review]
[PASSED QA] Bug 14298: Use the new permission in the sco scripts

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Note without this patch, you won't be able to access the SCO side.
So don't panic if you cant use SCO with just the first patch

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 22 Katrin Fischer 2015-09-14 19:27:19 UTC
Created attachment 42538 [details] [review]
[PASSED QA] Bug 14298: Add warning to the about page if the sco user does not have correct permissions

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Note: Made a little change, changed He should have, to They should have

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 23 Katrin Fischer 2015-09-14 19:27:22 UTC
Created attachment 42539 [details] [review]
[PASSED QA] Bug 14298: Add perms for other languages

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 24 Chris Cormack 2015-09-22 18:08:13 UTC
Pushed to 3.20.x will be in 3.20.4
Comment 25 Tomás Cohen Arazi (tcohen) 2015-09-25 14:56:51 UTC
Patches pushed to master.

Thanks Jonathan!
Comment 26 Jonathan Druart 2015-11-25 13:33:05 UTC Comment hidden (obsolete)
Comment 27 Katrin Fischer 2015-11-25 14:15:58 UTC
Created attachment 45139 [details] [review]
[SIGNED OFF] Bug 14298: Do not insert the permission if already exists.

To avoid a warning on updating from 3.20.x to 3.22, use "insert ignore
into".

This patch is not useful for 3.20.x.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 28 Tomás Cohen Arazi (tcohen) 2015-11-25 14:38:49 UTC
Followup pushed to master.

Thanks Jonathan!
Comment 29 Katrin Fischer 2015-11-25 14:45:57 UTC
Comment on attachment 42536 [details] [review]
[PASSED QA] Bug 14298: Add the new 'self_checkout' permissions

Bringing the old patches back