Bug 35231 - Cannot logout from OPAC and not login afterwards
Summary: Cannot logout from OPAC and not login afterwards
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Authentication (show other bugs)
Version: Main
Hardware: All All
: P5 - low blocker (vote)
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Keywords:
Depends on: 15504
Blocks:
  Show dependency treegraph
 
Reported: 2023-11-02 13:13 UTC by Marcel de Rooy
Modified: 2023-11-10 02:14 UTC (History)
5 users (show)

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


Attachments
Bug 35231: Resolve crash on unexisting patron in Auth (1.04 KB, patch)
2023-11-02 13:18 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 35231: Resolve crash on unexisting patron in Auth (1.22 KB, patch)
2023-11-02 13:50 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 35231: Resolve crash on unexisting patron in Auth (1.23 KB, patch)
2023-11-02 13:52 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 35231: Resolve crash on unexisting patron in Auth (1.26 KB, patch)
2023-11-02 13:59 UTC, Caroline Cyr La Rose
Details | Diff | Splinter Review
Bug 35231: Resolve crash on unexisting patron in Auth (1.33 KB, patch)
2023-11-02 16:42 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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 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 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 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