Bug 33881

Summary:	SCO/SCI user leaving the module doesn't clear session (ie JWT)
Product:	Koha	Reporter:	Nick Clemens (kidclamp) <nick>
Component:	Self checkout	Assignee:	Bugs List <koha-bugs>
Status:	RESOLVED FIXED	QA Contact:	Testopia <testopia>
Severity:	normal
Priority:	P5 - low	CC:	1joynelson, danyonsewell, dcook, didier.gautheron, fridolin.somers, jonathan.druart, laurent.ducos, lucas, m.de.rooy, martin.renvoize, nick, pedro.amorim, tomascohen
Version:	unspecified
Hardware:	All
OS:	All
See Also:	https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38640
Change sponsored?:	---	Patch complexity:	---
Documentation contact:		Documentation submission:
Text to go in the release notes:		Version(s) released in:	23.11.00,23.05.02,22.11.09
Circulation function:
Bug Depends on:	29543
Bug Blocks:
Attachments:	Bug 33881: Clear self-check JWT during auth kick out Bug 33881: Clear self-check JWT during auth kick out Bug 33881: Clear self-check JWT during auth kick out Bug 33881: [21.11] Clear self-check JWT during auth kick out

Description Nick Clemens (kidclamp) 2023-06-01 19:58:09 UTC

There is code in C4/Auth to kickout the self check users if they leave the module.

This works, however, the session is not cleared. This means that if a patron were using the SCO, then navigated to the OPAC they would have to sign in to view their account, and they might assume their info was cleared, however, returning to the SCO will re-enter the user session

To test:
1 - Setup self_check user
2 - Open sco module
3 - Sign in a patron
4 - Checkout an item
5 - Browse to the opac
6 - You are not logged in
7 - Return to SCO
8 - User account is still signed in

Comment 1 Nick Clemens (kidclamp) 2023-06-01 19:58:56 UTC

This was discovered in testing bug 23105 - previously the user session would be ended and require a login. Now this no longer happens

Comment 2 Nick Clemens (kidclamp) 2023-06-01 19:59:08 UTC

bug 23102 that is

Comment 3 David Cook 2023-06-01 23:39:27 UTC

When you say "setup self_check user", do you mean a "AutoSelfCheck" user? That's the user that has the session for the SCO initially.

When a human user then provides a cardnumber or login, they're not really logged in. They're just authenticating that they're that person.

When they browse to the OPAC, the CGISESSID cookie for the "AutoSelfCheck" session is cleared, so that the human user isn't hijacking the "AutoSelfCheck" session.

--

What do you mean by "Return to SCO"? Is that using the "Back" button, or is it by clicking a link, or something else?

Looking at the kick_out code, we probably should be deleting the SCO session and not just sending back an empty cookie. There might be scenarios where the browser is hanging on to the cookie that contains the session ID and that's what you're seeing. 

But need a bit more information to confirm I reckon.

Comment 4 Nick Clemens (kidclamp) 2023-06-05 11:12:54 UTC

(In reply to David Cook from comment #3)
> When you say "setup self_check user", do you mean a "AutoSelfCheck" user?
> That's the user that has the session for the SCO initially.

Yes, the AutoSelfCheck

> 
> When a human user then provides a cardnumber or login, they're not really
> logged in. They're just authenticating that they're that person.
> 
> When they browse to the OPAC, the CGISESSID cookie for the "AutoSelfCheck"
> session is cleared, so that the human user isn't hijacking the
> "AutoSelfCheck" session.

Correct - the user is not logged in as themself or the AutoSelfCheck user when they leave the SCO

> What do you mean by "Return to SCO"? Is that using the "Back" button, or is
> it by clicking a link, or something else?

I tested by entering the URL manually:

I left by entering search url manually:
http://localhost:8080/cgi-bin/koha/opac-search.pl?idx=&q=a&weight_search=1

And returned entering the SCO url manually:
http://localhost:8080/cgi-bin/koha/sco/sco-main.pl


> 
> Looking at the kick_out code, we probably should be deleting the SCO session
> and not just sending back an empty cookie. There might be scenarios where
> the browser is hanging on to the cookie that contains the session ID and
> that's what you're seeing. 

Agreed

> 
> But need a bit more information to confirm I reckon.

Let me know if this is more clear now

Comment 5 David Cook 2023-06-06 02:24:30 UTC

(In reply to Nick Clemens from comment #4)
> Let me know if this is more clear now

I think so.

I have a couple of things I need to do, but I'll try to get back to this today.

Comment 6 David Cook 2023-06-06 03:28:58 UTC

Ok so the issue isn't the cookie or the session. It's the JWT header. That is being persisted and it contains {"id": "<cardnumber>"}.

That's why it's going back in.

Comment 7 David Cook 2023-06-06 03:30:21 UTC

(In reply to David Cook from comment #6)
> Ok so the issue isn't the cookie or the session. It's the JWT header. That
> is being persisted and it contains {"id": "<cardnumber>"}.
> 
> That's why it's going back in.

Sorry my bad. Not JWT header. JWT cookie (as distinct from the CGISESSID cookie which our authentication cookie for the self check user).

Comment 8 David Cook 2023-06-06 03:31:20 UTC

(In reply to David Cook from comment #7)
> Sorry my bad. Not JWT header. JWT cookie (as distinct from the CGISESSID
> cookie which our authentication cookie for the self check user).

Actually, I haven't checked the CGISESSID in this context. But we don't need to worry about that right now. 

Basically we need to make sure the JWT gets cleared on kick out and not just log out.

Comment 9 David Cook 2023-06-06 03:43:48 UTC

(In reply to David Cook from comment #8)
> Basically we need to make sure the JWT gets cleared on kick out and not just
> log out.

This will be ugly since the generic cookie name of "JWT" was used instead of something like "SC_JWT", but... it should be safe enough...

Comment 10 David Cook 2023-06-06 03:52:56 UTC

Created attachment 152029 [details] [review]
Bug 33881: Clear self-check JWT during auth kick out

This patch clears the JWT cookie during auth kick out (ie
when a web user navigates from the self-check out/in to
the rest of Koha).

Test plan:
0. Apply patch and koha-plack --reload kohadev
1. Go to http://localhost:8080/cgi-bin/koha/sco/sco-main.pl
2. Log in as the "koha" user
3. In another tab, go to http://localhost:8080/cgi-bin/koha/sco/sco-main.pl
4. Go to http://localhost:8080/cgi-bin/koha/opac-search.pl?idx=&q=a&weight_search=1
5. Note that you are prompted to "Log in to your account" via the normal Koha prompt
6. Go to http://localhost:8080/cgi-bin/koha/sco/sco-main.pl
7. Note that you are prompted to "Log in to your account" within the "Self checkout system",
and note that your self-checkout session for the "koha" user has *not* persisted like
it did before the patch was applied

Comment 11 Andrew Fuerste-Henry 2023-07-08 16:28:31 UTC

Created attachment 153209 [details] [review]
Bug 33881: Clear self-check JWT during auth kick out

This patch clears the JWT cookie during auth kick out (ie
when a web user navigates from the self-check out/in to
the rest of Koha).

Test plan:
0. Apply patch and koha-plack --reload kohadev
1. Go to http://localhost:8080/cgi-bin/koha/sco/sco-main.pl
2. Log in as the "koha" user
3. In another tab, go to http://localhost:8080/cgi-bin/koha/sco/sco-main.pl
4. Go to http://localhost:8080/cgi-bin/koha/opac-search.pl?idx=&q=a&weight_search=1
5. Note that you are prompted to "Log in to your account" via the normal Koha prompt
6. Go to http://localhost:8080/cgi-bin/koha/sco/sco-main.pl
7. Note that you are prompted to "Log in to your account" within the "Self checkout system",
and note that your self-checkout session for the "koha" user has *not* persisted like
it did before the patch was applied

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>

Comment 12 Nick Clemens (kidclamp) 2023-07-11 11:30:05 UTC

Created attachment 153312 [details] [review]
Bug 33881: Clear self-check JWT during auth kick out

This patch clears the JWT cookie during auth kick out (ie
when a web user navigates from the self-check out/in to
the rest of Koha).

Test plan:
0. Apply patch and koha-plack --reload kohadev
1. Go to http://localhost:8080/cgi-bin/koha/sco/sco-main.pl
2. Log in as the "koha" user
3. In another tab, go to http://localhost:8080/cgi-bin/koha/sco/sco-main.pl
4. Go to http://localhost:8080/cgi-bin/koha/opac-search.pl?idx=&q=a&weight_search=1
5. Note that you are prompted to "Log in to your account" via the normal Koha prompt
6. Go to http://localhost:8080/cgi-bin/koha/sco/sco-main.pl
7. Note that you are prompted to "Log in to your account" within the "Self checkout system",
and note that your self-checkout session for the "koha" user has *not* persisted like
it did before the patch was applied

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Comment 13 Danyon Sewell 2023-07-21 01:59:45 UTC

This patch doesn't apply cleanly for 21.11.x could somebody please rebase it.

Comment 14 David Cook 2023-07-21 03:06:40 UTC

(In reply to Danyon Sewell from comment #13)
> This patch doesn't apply cleanly for 21.11.x could somebody please rebase it.

This one hasn't been pushed to master yet. Are you updating the correct bug report?

Comment 15 Danyon Sewell 2023-07-21 03:15:09 UTC

(In reply to David Cook from comment #14)
> (In reply to Danyon Sewell from comment #13)
> > This patch doesn't apply cleanly for 21.11.x could somebody please rebase it.
> 
> This one hasn't been pushed to master yet. Are you updating the correct bug
> report?

Hi David.

I'm the release maintainer for 21.11.x and am in the process of backporting all of the required security patches in time for the release.

This is one of those patches.

Comment 16 David Cook 2023-07-21 03:49:45 UTC

(In reply to Danyon Sewell from comment #15)
> Hi David.
> 
> I'm the release maintainer for 21.11.x and am in the process of backporting
> all of the required security patches in time for the release.
> 
> This is one of those patches.

I'm guessing the RM/RMaints must be discussing outside Bugzilla. Looking at other Koha security patches, it looks like sometimes the RM will say pushed to master and sometimes not...

bug 33595 and bug 32208

I'll put it back to "Patch doesn't apply". 

Resetting assignee as someone else can take it over. I probably won't get a chance to work on this for a while.

Comment 17 Jonathan Druart 2023-07-21 06:06:31 UTC

Created attachment 153750 [details] [review]
Bug 33881: [21.11] Clear self-check JWT during auth kick out

This patch clears the JWT cookie during auth kick out (ie
when a web user navigates from the self-check out/in to
the rest of Koha).

Test plan:
0. Apply patch and koha-plack --reload kohadev
1. Go to http://localhost:8080/cgi-bin/koha/sco/sco-main.pl
2. Log in as the "koha" user
3. In another tab, go to http://localhost:8080/cgi-bin/koha/sco/sco-main.pl
4. Go to http://localhost:8080/cgi-bin/koha/opac-search.pl?idx=&q=a&weight_search=1
5. Note that you are prompted to "Log in to your account" via the normal Koha prompt
6. Go to http://localhost:8080/cgi-bin/koha/sco/sco-main.pl
7. Note that you are prompted to "Log in to your account" within the "Self checkout system",
and note that your self-checkout session for the "koha" user has *not* persisted like
it did before the patch was applied

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Comment 18 Jonathan Druart 2023-07-21 06:06:51 UTC

Here is a rebase version for 21.11. I have not tested it though.

Comment 19 Jonathan Druart 2023-07-21 06:08:25 UTC

(In reply to David Cook from comment #16)
> Resetting assignee as someone else can take it over. I probably won't get a
> chance to work on this for a while.

It took me 30sec.

Comment 20 David Cook 2023-07-21 07:04:50 UTC

(In reply to Jonathan Druart from comment #19)
> (In reply to David Cook from comment #16)
> > Resetting assignee as someone else can take it over. I probably won't get a
> > chance to work on this for a while.
> 
> It took me 30sec.

I'll buy you a beer in Montreal next year. 

(In reply to Jonathan Druart from comment #18)
> Here is a rebase version for 21.11. I have not tested it though.

The testing part seemed daunting, especially as my mind is on other things for other versions at the moment. 

I think perhaps I do not understand the security release procedure well enough as well.

Comment 21 Katrin Fischer 2023-07-26 17:25:22 UTC

Should this still be patch doesn't apply?

Comment 22 David Cook 2023-07-27 01:08:12 UTC

(In reply to Katrin Fischer from comment #21)
> Should this still be patch doesn't apply?

I don't think so. I think it would be OK to put back to "Passed QA"?

Comment 23 Tomás Cohen Arazi (tcohen) 2023-07-28 13:56:53 UTC

Pushed to master for 23.11.

Nice work everyone, thanks!

Comment 24 Fridolin Somers 2023-08-08 06:24:40 UTC

Pushed to 23.05.x for 23.05.02

Comment 25 Pedro Amorim 2023-08-28 13:44:49 UTC

I think this was not added in time for the v22.11.08 tag release but it's included in 22.11.09.

Comment 26 Lucas Gass (lukeg) 2023-09-13 20:50:05 UTC

This needs a 22.05.x rebase, if possible.

Comment 27 David Cook 2024-12-06 00:49:06 UTC

Looks like this one is misnamed. The fix is only for SCO and not SCI...