Bug 35231

Summary: Cannot logout from OPAC and not login afterwards
Product: Koha Reporter: Marcel de Rooy <m.de.rooy>
Component: AuthenticationAssignee: Marcel de Rooy <m.de.rooy>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: blocker    
Priority: P5 - low CC: caroline.cyr-la-rose, dpavlin, fridolin.somers, martin.renvoize, tomascohen
Version: Main   
Hardware: All   
OS: All   
GIT URL: Change sponsored?: ---
Patch complexity: Trivial patch Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
23.11.00
Circulation function:
Bug Depends on: 15504    
Bug Blocks:    
Attachments: Bug 35231: Resolve crash on unexisting patron in Auth
Bug 35231: Resolve crash on unexisting patron in Auth
Bug 35231: Resolve crash on unexisting patron in Auth
Bug 35231: Resolve crash on unexisting patron in Auth
Bug 35231: Resolve crash on unexisting patron in Auth

Description Marcel de Rooy 2023-11-02 13:13:45 UTC
Can't call method "update_lastseen" on an undefined value at /usr/share/koha/C4/Auth.pm line 1322

Please tell me that I am wrong :)

Logout in OPAC.
Crash
Try to login again.
Crash
Comment 1 Marcel de Rooy 2023-11-02 13:18:10 UTC Comment hidden (obsolete)
Comment 2 Marcel de Rooy 2023-11-02 13:20:25 UTC Comment hidden (obsolete)
Comment 3 Marcel de Rooy 2023-11-02 13:20:55 UTC Comment hidden (obsolete)
Comment 4 Caroline Cyr La Rose 2023-11-02 13:34:12 UTC
With the patch, my opac is "in maintenance"
Comment 5 Marcel de Rooy 2023-11-02 13:35:35 UTC
(In reply to Caroline Cyr La Rose from comment #4)
> With the patch, my opac is "in maintenance"

Yes, this patch is not the answer. Will obsolete it. It undoes the whole feature. Did you check if you need to upgrade your version btw?
Comment 6 Marcel de Rooy 2023-11-02 13:37:50 UTC
Before 15504 we had:

-    return if !$userid || !$activity || !C4::Context->preference('TrackLastPatronActivity');

The unauthorized opac access get stuck here since we dont check empty userid anymore?
Comment 7 Marcel de Rooy 2023-11-02 13:50:02 UTC Comment hidden (obsolete)
Comment 8 Marcel de Rooy 2023-11-02 13:52:19 UTC
Created attachment 158260 [details] [review]
Bug 35231: Resolve crash on unexisting patron in Auth

Test plan:
Without this patch:
Logout from OPAC. Crash.
Try to login. Crash.

With this patch:
Enable login in tracking triggers. Clear lastseen.
Flush memcache.
Login. Check lastseen.
Logout.
Login.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 9 Caroline Cyr La Rose 2023-11-02 13:58:56 UTC
(In reply to Marcel de Rooy from comment #5)
> (In reply to Caroline Cyr La Rose from comment #4)
> > With the patch, my opac is "in maintenance"
> 
> feature. Did you check if you need to upgrade your version btw?

Ah yes, my mistake... I git pulled, but forgot to updatedb...

Trying again
Comment 10 Caroline Cyr La Rose 2023-11-02 13:59:31 UTC
Created attachment 158261 [details] [review]
Bug 35231: Resolve crash on unexisting patron in Auth

Test plan:
Without this patch:
Logout from OPAC. Crash.
Try to login. Crash.

With this patch:
Enable login in tracking triggers. Clear lastseen.
Flush memcache.
Login. Check lastseen.
Logout.
Login.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Comment 11 Caroline Cyr La Rose 2023-11-02 14:00:34 UTC
It works! I tried with the pref enabled (all activities checked) and disabled (no activities checked)
Comment 12 Marcel de Rooy 2023-11-02 14:14:56 UTC
(In reply to Caroline Cyr La Rose from comment #11)
> It works! I tried with the pref enabled (all activities checked) and
> disabled (no activities checked)

Great!
Comment 13 Martin Renvoize (ashimema) 2023-11-02 16:42:54 UTC
Created attachment 158272 [details] [review]
Bug 35231: Resolve crash on unexisting patron in Auth

Test plan:
Without this patch:
Logout from OPAC. Crash.
Try to login. Crash.

With this patch:
Enable login in tracking triggers. Clear lastseen.
Flush memcache.
Login. Check lastseen.
Logout.
Login.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 14 Martin Renvoize (ashimema) 2023-11-02 16:44:24 UTC
Man, the code in Auth.pm is so complicated.

I wondered if $userid may ever be invalid at the point we're using it, but I'm pretty confident it's been validated long prior to this point in the code so we can assume Koha::Patron->find will actually find something.

Passing QA
Comment 15 Tomás Cohen Arazi (tcohen) 2023-11-03 15:06:28 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 16 Fridolin Somers 2023-11-10 02:14:17 UTC
Depends on Bug 15504 not in 23.05.x