Bug 36586

Summary: Self-checkouts will get CSRF errors if left inactive for 8 hours
Product: Koha Reporter: David Cook <dcook>
Component: Self checkoutAssignee: Bugs List <koha-bugs>
Status: NEW --- QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: ddaghita, jonathan.druart, nick
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38327
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Attachments: Bug 36586: WIP
Bug 36586: WIP
Bug 36586: Define self-checkin timer using Javascript class

Description David Cook 2024-04-12 04:37:29 UTC
Currently, the CSRF token is only good for 8 hours. This means that a self-checkout terminal left inactive for 8 hours (e.g. overnight, over a holiday closure, etc) will create a wrong CSRF token error on the first activity attempt in the morning.
Comment 1 David Cook 2024-04-12 04:44:52 UTC
I think the most obvious (but not necessarily the best) solution is to have a timer that refreshes the main SCO page at certain intervals. 

--

(Note that this same problem will probably arise with Vue.js driven pages.)
Comment 2 Jonathan Druart 2024-11-06 14:58:07 UTC
SCI has a SelfCheckInTimeout pref that refreshes the screen.
Comment 3 David Cook 2024-11-06 22:10:57 UTC
(In reply to Jonathan Druart from comment #2)
> SCI has a SelfCheckInTimeout pref that refreshes the screen.

That should do the trick. We could move that out to a .js file and use it in both interfaces.
Comment 4 Nick Clemens (kidclamp) 2024-12-04 21:14:08 UTC
Created attachment 175210 [details] [review]
Bug 36586: WIP

This patch moves the refresh code into a js include and sets a few variables in the page.
It also moves the fetch of the pref into the template.

We need to copy this over for the sco - and probably add a new pref.

Lastly, the code here only refreshes during an active session, probably we could have a single pref
SelfCheckRefresh
that would trigger on the main page, default to 3 or 4 hours?
Comment 5 David Cook 2024-12-04 23:12:38 UTC
(In reply to Nick Clemens (kidclamp) from comment #4)
> This patch moves the refresh code into a js include and sets a few variables
> in the page.

Did you write the JS file? It's not appearing in the patch.

> Lastly, the code here only refreshes during an active session, probably we
> could have a single pref
> SelfCheckRefresh
> that would trigger on the main page, default to 3 or 4 hours?

As in, the current timeout is just an activity timeout to log them out? 

Yeah, I think a timer on the main page that defaults to even 1 time per hour would be good.
Comment 6 Nick Clemens (kidclamp) 2024-12-05 12:34:11 UTC
Created attachment 175239 [details] [review]
Bug 36586: WIP

This patch moves the refresh code into a js include and sets a few variables in the page.
It also moves the fetch of the pref into the template.

We need to copy this over for the sco - and probably add a new pref.

Lastly, the code here only refreshes during an active session, probably we could have a single pref
SelfCheckRefresh
that would trigger on the main page, default to 3 or 4 hours?
Comment 7 Nick Clemens (kidclamp) 2024-12-05 12:35:24 UTC
(In reply to David Cook from comment #5)
> (In reply to Nick Clemens (kidclamp) from comment #4) 
> Did you write the JS file? It's not appearing in the patch.

heh, "git commit -a", but didn't add the new file ;-)
Comment 8 David Cook 2024-12-06 00:44:35 UTC
Comment on attachment 175239 [details] [review]
Bug 36586: WIP

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

::: koha-tmpl/opac-tmpl/bootstrap/en/modules/sci/sci-main.tt
@@ +274,4 @@
>  });
>  
>  
> +        let idleTimeout = [% Koha.Preference('SelfCheckInTimeOut') || 120 %];

There's a filter missing here. I noticed with the eyeballs and QA tools says it as well.
Comment 9 David Cook 2024-12-06 00:54:49 UTC
(In reply to Nick Clemens (kidclamp) from comment #7)
> heh, "git commit -a", but didn't add the new file ;-)

I've been writing a lot of new code on a different project, and I feel this pain haha
Comment 10 David Cook 2024-12-06 01:07:08 UTC
This WIP looks like it works for the existing functionality. 

But since we want timers for the borrower session and the SCO/SCI session, I'm thinking we should encapsulate the timers in objects. We'd create 2 objects, and then feed them their respective timer thresholds.
Comment 11 Nick Clemens (kidclamp) 2024-12-23 17:28:44 UTC
(In reply to David Cook from comment #10)
> This WIP looks like it works for the existing functionality. 
> 
> But since we want timers for the borrower session and the SCO/SCI session,
> I'm thinking we should encapsulate the timers in objects. We'd create 2
> objects, and then feed them their respective timer thresholds.

I don't think I am quite sure what you mean here, JS objects? Koha Objects? Can you help with a patch or some pseudo code?
Comment 12 David Cook 2024-12-23 22:55:00 UTC
(In reply to Nick Clemens (kidclamp) from comment #11)
> (In reply to David Cook from comment #10)
> > This WIP looks like it works for the existing functionality. 
> > 
> > But since we want timers for the borrower session and the SCO/SCI session,
> > I'm thinking we should encapsulate the timers in objects. We'd create 2
> > objects, and then feed them their respective timer thresholds.
> 
> I don't think I am quite sure what you mean here, JS objects? Koha Objects?
> Can you help with a patch or some pseudo code?

Ah my bad. Yeah, JS objects. 

Sure, I'll look at doing a quick patch.
Comment 13 David Cook 2024-12-24 00:07:37 UTC
Created attachment 175919 [details] [review]
Bug 36586: Define self-checkin timer using Javascript class

This patch rewrites the self-checkin timer code to be defined in a
Javascript class which is then instantiated in once in sci-main.pl
depending on certain conditions (ie a barcode has been entered, or
an item has been checked in by barcode).
Comment 14 David Cook 2024-12-24 00:12:06 UTC
So the Javascript class here allows us to create multiple objects with the same code, so we can have the "window.sci_login_timer" which just governs the login timeout.

But then we could easily create another timer for however many hours we want. (For that scenario, perhaps we'd actually want to set a higher interval than 1 second, and instead maybe check every minute or X minutes. 

Anyway, that's what I had in mind. 

I wrote it without jQuery, and I tweaked the conditions a bit, so that we're only invoking the timer where we need it in the first place rather than running it and checking for the button visibility on each increment.

--

Also added in the missing TT filter and wrapper the idleTimeout in double quotes to avoid XSS.