Bug 10799 - Able to become self-checkout staff user in OPAC
Summary: Able to become self-checkout staff user in OPAC
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Self checkout (show other bugs)
Version: 3.20
Hardware: All All
: P5 - low critical (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-08-28 17:10 UTC by Mark Tompsett
Modified: 2019-06-27 09:24 UTC (History)
10 users (show)

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


Attachments
Bug 10799: Limit the SCO user to the SCO module (1.13 KB, patch)
2015-09-02 16:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10799: Remove "Welcome Sco user" (1.82 KB, patch)
2015-09-02 16:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10799: Limit the SCO user to the SCO module (1.34 KB, patch)
2015-09-02 21:55 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 10799: Remove "Welcome Sco user" (1.87 KB, patch)
2015-09-02 21:55 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 10799: Limit the SCO user to the SCO module (1.77 KB, patch)
2015-09-14 11:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10799: Limit the SCO user to the SCO module (1.91 KB, patch)
2015-10-27 08:46 UTC, Marc Véron
Details | Diff | Splinter Review
[PASSED QA] Bug 10799: Limit the SCO user to the SCO module (1.99 KB, patch)
2015-10-30 17:57 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Tompsett 2013-08-28 17:10:34 UTC
While testing for oleonard at responsive.mykoha.co.nz, I found these steps duplicated a weird glitch.

1) go to responsive.mykoha.co.nz
2) log in
The user is member, member
3) click the self checkout link
4) log in
5) click the browser back button twice
6) refresh page
The user is now staff

Similarly,
1) go to responsive.mykoha.co.nz
2) click the self checkout link
3) log in
4) click the finish button
5) change the address back to the http://responsive.mykoha.co.nz/
6) refresh page
The user is now staff

The log in used was the member, member one, and not staff.
Comment 1 Mark Tompsett 2013-08-28 17:32:05 UTC
To trigger the problem in firefox for method one:

1) go to responsive.mykoha.co.nz
2) log in
The user is member, member
3) copy the self checkout link
4) search for a book
5) view the details
6) paste the self checkout link
7) log in
8) click the browser back button twice
9) refresh page.

Without actually being on a page past the log in, Firefox would resend the data and avoid this bug.
Comment 2 Owen Leonard 2013-08-28 18:12:33 UTC
Note that this problem isn't a problem just with the demo system linked above, but with any system running master (or before, probably). In order to reproduce the problem you have to have the AutoSelfCheckAllowed preference set to "allow the web-based self checkout system to automatically login with this staff login..." and have staff login credentials saved.

So it's not exactly the case that one is able to change the logged in user--the staff user must be logged in for the self checkout to work. It's more a question of whether there should be a built-in way to prevent the self checkout user from accessing a regular OPAC session.
Comment 3 Mark Tompsett 2013-08-28 18:36:03 UTC
Unless you lock your browser down, someone is likely to click back or paste things into the address bar. The self-checkout isn't really self-checkout, it is use this staff user for checkout. Sort of like a seteuid system implemented as a setuid instead of actually implementing a seteuid system.
Comment 4 Jonathan Druart 2015-07-13 13:47:52 UTC

*** This bug has been marked as a duplicate of bug 14298 ***
Comment 5 Jonathan Druart 2015-09-02 16:17:36 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2015-09-02 16:17:40 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2015-09-02 16:19:37 UTC
I am not familiar enough with this module to be confident with these patches, please be involved!
Comment 8 Mark Tompsett 2015-09-02 17:31:33 UTC
Comment on attachment 42232 [details] [review]
Bug 10799: Limit the SCO user to the SCO module

Review of attachment 42232 [details] [review]:
-----------------------------------------------------------------

::: C4/Auth.pm
@@ +179,5 @@
>      }
>  
> +    # If the user logged in is the SCO user and he tries to go out the SCO module, the log user out.
> +    if ( $in->{type} eq 'opac' and $in->{template_name} !~ m|sco/| ) {
> +        if ( $user eq C4::Context->preference('AutoSelfCheckID') ) {

I know things have changed lately, but the preference() did in the past return undef as well as defined values. We don't want floody log noise.

if ( C4::Context->preference('AutoSelfCheckID') and ($user eq C4::Context->preference('AutoSelfCheckID') ) ) {
Comment 9 Chris Cormack 2015-09-02 21:55:07 UTC Comment hidden (obsolete)
Comment 10 Chris Cormack 2015-09-02 21:55:27 UTC Comment hidden (obsolete)
Comment 11 Jonathan Druart 2015-09-03 07:27:53 UTC
(In reply to M. Tompsett from comment #8)
> Comment on attachment 42232 [details] [review] [review]
> Bug 10799: Limit the SCO user to the SCO module
> 
> Review of attachment 42232 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: C4/Auth.pm
> @@ +179,5 @@
> >      }
> >  
> > +    # If the user logged in is the SCO user and he tries to go out the SCO module, the log user out.
> > +    if ( $in->{type} eq 'opac' and $in->{template_name} !~ m|sco/| ) {
> > +        if ( $user eq C4::Context->preference('AutoSelfCheckID') ) {
> 
> I know things have changed lately, but the preference() did in the past
> return undef as well as defined values. We don't want floody log noise.
> 
> if ( C4::Context->preference('AutoSelfCheckID') and ($user eq
> C4::Context->preference('AutoSelfCheckID') ) ) {

Default is "", shouldn't change anything.
Comment 12 Katrin Fischer 2015-09-06 19:18:23 UTC
The patch works, but there is a small glitch we need to fix:

http://localhost/cgi-bin/koha/opac-user.pl
Software error:

Can't use an undefined value as an ARRAY reference at /usr/lib/perl5/DBI.pm line 2064.

For help, please send mail to the webmaster (webmaster@bumblebee), giving this error message and the time and date of the error. 

When you try to login, an error message displays.
Comment 13 Jonathan Druart 2015-09-14 11:43:08 UTC Comment hidden (obsolete)
Comment 14 Jonathan Druart 2015-09-14 11:43:45 UTC
I have rewritten the patch to fix the comment 12's issue
Comment 15 Marc Véron 2015-10-26 12:33:17 UTC
The first patch does not apply, I think it is already covered by:
Bug 14776 - SCO/shelfcheck 'log out' option redirects to OPAC, not back to SCO
(Pushed to master)
Comment 16 Marc Véron 2015-10-26 13:07:38 UTC
Regarding the 2nd patch: Limit the SCO user to the SCO module 

I have sysprefs:

AutoSelfCheckAllowed: [Allow} the web-based self checkout system to automatically login with this staff login 

...and AutoSelfCheckID / AutoSelfCheckPass are set.

In that case, I would not expect a login form but a redirect to the SCO page (with the SCO user logged in).
Comment 17 Jonathan Druart 2015-10-27 08:33:22 UTC
I'd consider that as an improvement, this one is a security fix.
Comment 18 Marc Véron 2015-10-27 08:46:41 UTC Comment hidden (obsolete)
Comment 19 Kyle M Hall 2015-10-30 17:57:02 UTC
Created attachment 44243 [details] [review]
[PASSED QA] Bug 10799: Limit the SCO user to the SCO module

The SCO user should only be allowed to access to the SCO module.

This patch make the session ends if the user tries to access another
page after the SCO module.

Test plan:
0/ Configure the SCO module correctly
1/ Go on the sco main page (sco/sco-main.pl)
2/ Try to go somewhere else: you should not be logged in

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 20 Tomás Cohen Arazi 2015-11-02 14:38:05 UTC
Patch pushed to master.

Thanks Jonathan!
Comment 21 Frédéric Demians 2015-11-29 20:14:05 UTC
This patch has been pushed to 3.20.x, will be in 3.20.6.
Comment 22 Chris Cormack 2016-03-14 19:32:49 UTC
Released in stable branches, moving out of security