From 60bbe224f77fcd057d17b46f95b1bd9d6b2b09e3 Mon Sep 17 00:00:00 2001 From: Mark Tompsett Date: Fri, 1 Nov 2013 09:51:28 -0400 Subject: [PATCH] Bug 11077 - Correct silent warnings in C4/Auth.pm This patch corrects a noisy ne condition. $userid = $retuserid if ( $retuserid ne ''); became $userid = $retuserid if ( $retuserid ); It also integrates Srdjan Jankovic's patch with Petter Goksoyrsen's patch, while correcting the problems found. This includes: my $q_userid = $query->param('userid') // ''; along with: my $s_userid = ''; and: my $s_userid = $session->param('id') // ''; Indentation does not reflect actual scoping. And the 'None' is changed to '', since that behaves properly in the undef case: my $pki_field = C4::Context->preference('AllowPKIAuth'); if (!defined($pki_field)) { print STDERR "Error: Missing AllowPKIAuth System Preference!\n"; $pki_field = ''; } Because if it was set to 'None' in the undefined case, then 'None' ne 'None' is false, and the if case would not run. However, undef ne 'None' is true and runs with an error log entry. By putting the define check as a separate if clause, a meaningful error message can be given rather than a ubiquitous undef compare check failure message. --- C4/Auth.pm | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/C4/Auth.pm b/C4/Auth.pm index 9d0b623..790f56c 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -650,7 +650,7 @@ sub checkauth { # This parameter is the name of the CAS server we want to authenticate against, # when using authentication against multiple CAS servers, as configured in Auth_cas_servers.yaml my $casparam = $query->param('cas'); - my $q_userid = $query->param('userid'); + my $q_userid = $query->param('userid') // ''; if ( $userid = $ENV{'REMOTE_USER'} ) { # Using Basic Authentication, no cookies required @@ -670,9 +670,9 @@ sub checkauth { my $session = get_session($sessionID); C4::Context->_new_userenv($sessionID); my ($ip, $lasttime, $sessiontype); - my $s_userid; + my $s_userid = ''; if ($session){ - $s_userid = $session->param('id'); + $s_userid = $session->param('id') // ''; C4::Context::set_userenv( $session->param('number'), $s_userid, $session->param('cardnumber'), $session->param('firstname'), @@ -690,7 +690,7 @@ sub checkauth { $userid = $s_userid; $sessiontype = $session->param('sessiontype') || ''; } - if ( ( $query->param('koha_login_context') && ($q_userid && $s_userid && $q_userid ne $s_userid) ) + if ( ( $query->param('koha_login_context') && ($q_userid ne $s_userid) ) || ( $cas && $query->param('ticket') ) ) { #if a user enters an id ne to the id in the current session, we need to log them in... #first we need to clear the anonymous session... @@ -714,7 +714,7 @@ sub checkauth { logout_cas($query); } } - elsif ( $lasttime && ($lasttime < time() - $timeout) ) { + elsif ( !$lasttime || ($lasttime < time() - $timeout) ) { # timed logout $info{'timed_out'} = 1; $session->delete() if $session; @@ -763,7 +763,7 @@ sub checkauth { -HttpOnly => 1 ); $userid = $q_userid; - my $pki_field = C4::Context->preference('AllowPKIAuth') // 'None'; + my $pki_field = C4::Context->preference('AllowPKIAuth') // ''; if ( ( $cas && $query->param('ticket') ) || $userid || $pki_field ne 'None' @@ -838,7 +838,7 @@ sub checkauth { my $retuserid; ( $return, $cardnumber, $retuserid ) = checkpw( $dbh, $userid, $password, $query ); - $userid = $retuserid if ( $retuserid ne '' ); + $userid = $retuserid if ( $retuserid ); } if ($return) { #_session_log(sprintf "%20s from %16s logged in at %30s.\n", $userid,$ENV{'REMOTE_ADDR'},(strftime '%c', localtime)); -- 1.7.9.5