From 334414fe7344fcb20b38c4ed9463a792b47918c3 Mon Sep 17 00:00:00 2001 From: David Cook Date: Thu, 19 Jul 2018 16:57:22 +1000 Subject: [PATCH] Bug 13779: sessionID declared twice in C4::Auth::checkauth() Content-Type: text/plain; charset=utf-8 The bug is that $sessionID is declared twice in C4::Auth::checkauth(). At the moment, this doesn't actually create a problem, because no one seems to be using the $sessionID which is returned by checkauth(), except in the case of opac/external/overdrive/auth.pl which skips the second declartion as it doesn't require auth. This patch removes the redefining of the $sessionID variable. In terms of testing, try logging in with a username and password and see if it works. The only risk this patch would pose is breaking auth I would think, since nothing is actually using the return value from checkauth() for $sessionID. NOTE: It was initially defined near the top of the function (~line 791). I believe the scoping would mean the correct version of $sessionID would be used in the latter lines for the unset'ing. I have skimmed code to see if the sessionID return value is used. I did not test overdrive, as I do not know how. However, this is the only area, I think this could possibly break. This change makes sense to me. QA: Please test overdrive. opac/external/overdrive/auth.pl only checks if the value is set, so this patch might fix/break something there? opac/svc/overdrive same kind of check. Signed-off-by: Mark Tompsett Signed-off-by: Marcel de Rooy This is a (tiny) code improvement. Now we consistently return the session id that is also stored in the returned cookie. (Which can be an 'anon' session.) Fact is that Koha almost everywhere ignores the returned session id and sometimes gets the session from the cookie (obviously). The session id is also passed to the template by get_template_and_user but never used in templates. As mentioned, the two overdrive scripts are the exception. But since both test on both $user && $sessionID, they will not choke on an anynonomous session id without userid. So theoretically fine, but not tested. --- C4/Auth.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/C4/Auth.pm b/C4/Auth.pm index 8fd9728..4854477 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -943,7 +943,7 @@ sub checkauth { $session->param( 'search_history', $anon_search_history ); } - my $sessionID = $session->id; + $sessionID = $session->id; C4::Context->_new_userenv($sessionID); $cookie = $query->cookie( -name => 'CGISESSID', -- 2.1.4