From 28e928775d9a58b957666d6fb5f013e26ce034c1 Mon Sep 17 00:00:00 2001 From: Neil Blakey-Milner Date: Wed, 29 Mar 2006 14:30:51 +0000 Subject: [PATCH] Rework the login/session verification code to make it easier to understand, and to make it more dynamic in terms of error conditions later on. --- lib/authentication/authenticationprovider.inc.php | 16 ++++++++++++++++ lib/authentication/authenticationutil.inc.php | 14 ++++++++++++-- lib/dispatcher.inc.php | 14 ++++---------- lib/session/Session.inc | 148 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------------------------------------ lib/session/control.inc | 59 ++++++++++++++++++++++++++++++++--------------------------- login.php | 11 +++++++---- 6 files changed, 141 insertions(+), 121 deletions(-) diff --git a/lib/authentication/authenticationprovider.inc.php b/lib/authentication/authenticationprovider.inc.php index 05be14c..ca14ffd 100644 --- a/lib/authentication/authenticationprovider.inc.php +++ b/lib/authentication/authenticationprovider.inc.php @@ -87,4 +87,20 @@ class KTAuthenticationProvider extends KTStandardDispatcher { function do_performEditSourceProvider() { return $this->errorRedirectTo('viewsource', _kt("Provider does not support editing"), 'source_id=' . $_REQUEST['source_id']); } + + /** + * Perform provider-specific on-logout activities + * + * @param User The user who has just logged in + */ + function login($oUser) { + } + + /** + * Perform provider-specific on-logout activities + * + * @param User The user who is about to be logged out + */ + function logout($oUser) { + } } diff --git a/lib/authentication/authenticationutil.inc.php b/lib/authentication/authenticationutil.inc.php index 1aed545..5b1e667 100644 --- a/lib/authentication/authenticationutil.inc.php +++ b/lib/authentication/authenticationutil.inc.php @@ -42,16 +42,26 @@ class KTAuthenticationUtil { return KTAuthenticationUtil::getAuthenticatorForSource($iSourceId); } + function &getAuthenticationProviderForUser($oUser) { + $iSourceId = $oUser->getAuthenticationSourceId(); + return KTAuthenticationUtil::getAuthenticationProviderForSource($iSourceId); + } + function &getAuthenticatorForSource($oSource) { + $oProvider =& KTAuthenticationUtil::getAuthenticationProviderForSource($oSource); + return $oProvider->getAuthenticator($oSource); + } + + function &getAuthenticationProviderForSource($oSource) { if ($oSource) { $oSource =& KTUtil::getObject('KTAuthenticationSource', $oSource); $sProvider = $oSource->getAuthenticationProvider(); $oRegistry =& KTAuthenticationProviderRegistry::getSingleton(); $oProvider =& $oRegistry->getAuthenticationProvider($sProvider); } else { - $oProvider = new KTBuiltinAuthenticationProvider; + $oProvider =& new KTBuiltinAuthenticationProvider; } - return $oProvider->getAuthenticator($oSource); + return $oProvider; } function synchroniseGroupToSource($oGroup) { diff --git a/lib/dispatcher.inc.php b/lib/dispatcher.inc.php index 51443e9..292c51a 100644 --- a/lib/dispatcher.inc.php +++ b/lib/dispatcher.inc.php @@ -197,19 +197,13 @@ class KTStandardDispatcher extends KTDispatcher { } function loginRequired() { - $url = generateControllerUrl("login"); - $redirect = urlencode($_SERVER['REQUEST_URI']); - if ((strlen($redirect) > 1)) { - $url = $url . "&redirect=" . $redirect; - } - redirect($url); - exit(0); + checkSessionAndRedirect(true); } function dispatch () { - $session = new Session(); - $sessionStatus = $session->verify($bDownload); - if ($sessionStatus === false) { + $this->session = new Session(); + $sessionStatus = $this->session->verify(); + if ($sessionStatus !== true) { $this->loginRequired(); } diff --git a/lib/session/Session.inc b/lib/session/Session.inc index efea108..1017944 100644 --- a/lib/session/Session.inc +++ b/lib/session/Session.inc @@ -10,7 +10,6 @@ * it under the terms of the GNU General Public License as published by * the Free Software Foundation; using version 2 of the License. * - * * This program is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the @@ -21,9 +20,8 @@ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA * * @version $Revision$ - * @author Michael Joseph , Jam Warehouse (Pty) Ltd, South Africa - * @package lib.session */ + class Session { /** @@ -32,13 +30,14 @@ class Session { * @param int the id of the user to create a session for * @return string the generated sessionID */ - function create($iUserID) { + function create($oUser) { + $iUserId = $oUser->getId(); global $default; session_start(); // bind user id to session - $_SESSION["userID"] = $iUserID; + $_SESSION["userID"] = $iUserId; $_SESSION["KTErrorMessage"] = array(); // use the PHP generated session id @@ -47,13 +46,12 @@ class Session { // retrieve client ip $ip = $this->getClientIP(); - $default->log->debug("Session::create() new session for $iUserID, from $ip, sessionID=$sessionID"); + $default->log->debug("Session::create() new session for $iUserId, from $ip, sessionID=$sessionID"); // insert session information into db - $sql = $default->db; $aParams = array( 'session_id' => $sessionID, - 'user_id' => $iUserID, + 'user_id' => $iUserId, 'lastused' => date("Y-m-d H:i:s", time()), 'ip' => $ip, ); @@ -62,6 +60,10 @@ class Session { if (PEAR::isError($result)) { die("Error creating session: " . $result->toString()); } + + $oProvider =& KTAuthenticationUtil::getAuthenticationProviderForUser($oUser); + $oProvider->login($oUser); + return $sessionID; } @@ -106,85 +108,75 @@ class Session { * @param boolean optional parameter set if we're downloading a file * @return int session verification status */ - function verify($bDownload = false) { - global $default, $lang_sesstimeout, $lang_sessinuse, $lang_err_sess_notvalid; + function verify() { + global $default; + // this is a workaround for an SSL download bug with IE. session_cache_limiter('none'); session_start(); header("Cache-Control: must-revalidate"); header("Expires: " . gmdate("D, d M Y H:i:s", time() - 3600) . " GMT"); $sessionID = session_id(); - if (strlen($sessionID) > 0) { - // initialise return status - $sessionStatus = 0; - - // this should be an existing session, so check the db - $sql = $default->db; - $sql->query(array("SELECT * FROM $default->sessions_table WHERE session_id = ?", $sessionID));/*ok*/ - $numrows = $sql->num_rows($sql); - - // FIXME: if there aren't more rows that the max sessions for this user - if ($numrows >= 1) { - $default->log->debug("Session::verify found session in db"); - while($sql->next_record()) { - $iUserID = $sql->f("user_id"); - $ip = $this->getClientIP(); - // check that ip matches - if ($ip == trim($sql->f("ip"))) { - // now check if the timeout has been exceeded - $lastused = $sql->f("lastused"); - $diff = time() - strtotime($lastused); - if($diff <= $default->sessionTimeout) { - // session has been verified, update status - $sessionStatus = 1; - // use userID to refresh user details and set on session - - // ??: will this change during a user session? - // only set the userID if its not in the array already - if (!$_SESSION["userID"]) { - $_SESSION["userID"] = $iUserID; - } - - // update last used timestamp - $aFV = array( - 'lastused' => getCurrentDateTime(), - ); - $aWFV = array( - 'user_id' => $iUserID, - 'session_id' => $sessionID, - ); - $res = DBUtil::whereUpdate($default->sessions_table, $aFV, $aWFV); - // add the array to the session - $_SESSION["sessionStatus"] = $sessionStatus; - } else { - // session timed out status - $sessionStatus = 2; - // destroy this session - $this->destroy(); - $_SESSION["errorMessage"] = $lang_sesstimeout; - } - } else { - // session in use status - $sessionStatus = 3; - $_SESSION["errorMessage"] = $lang_sessinuse; - } - } - } else { - // the session doesn't exist in the db - $default->log->info("Session::verify sessionID=$sessionID, not in db"); - $sessionStatus = false; - } - } else { + + if (empty($sessionID)) { $default->log->info("Session::verify session not in db"); - // there is no session - $sessionStatus = false; + return PEAR::raiseError('You need to login to access this page'); + } + + // this should be an existing session, so check the db + $aRows = DBUtil::getResultArray(array("SELECT * FROM $default->sessions_table WHERE session_id = ?", $sessionID)); + $numrows = count($aRows); + + // FIXME: if there aren't more rows that the max sessions for this user + if ($numrows < 1) { + // the session doesn't exist in the db + $default->log->info("Session::verify sessionID=$sessionID, not in db"); + return PEAR::raiseError('You need to login to access this page'); + return false; } - - // remove old sessions - Session::removeStaleSessions(); - // return the status - return $sessionStatus; + $default->log->debug("Session::verify found session in db"); + $aRow = $aRows[0]; + // foreach ($aRows as $aRow) { + + $iUserID = $aRow["user_id"]; + + // check that ip matches + $ip = $this->getClientIP(); + if ($ip != trim($aRow["ip"])) { + return PEAR::raiseError("You are coming from a different IP address than the session requires"); + return false; + } + + + // now check if the timeout has been exceeded + $lastused = $aRow["lastused"]; + $diff = time() - strtotime($lastused); + if($diff <= $default->sessionTimeout) { + // update last used timestamp + $aFV = array( + 'lastused' => getCurrentDateTime(), + ); + $aWFV = array( + 'user_id' => $iUserID, + 'session_id' => $sessionID, + ); + $res = DBUtil::whereUpdate($default->sessions_table, $aFV, $aWFV); + // add the array to the session + $_SESSION["sessionStatus"] = $sessionStatus; + + Session::removeStaleSessions(); + + return true; + } else { + return PEAR::raiseError('Session timed out'); + } + + // } + + Session::removeStaleSessions(); + + return false; } /** diff --git a/lib/session/control.inc b/lib/session/control.inc index dc95762..72049c1 100644 --- a/lib/session/control.inc +++ b/lib/session/control.inc @@ -126,37 +126,42 @@ function checkSessionAndRedirect($bRedirect, $bDownload = false) { global $default; $session = new Session(); - $sessionStatus = $session->verify($bDownload); + $sessionStatus = $session->verify(); - if ($sessionStatus != 1) { - // verification failed - $default->log->debug("checkSession:: session check failed"); - if ($bRedirect) { - // redirect to login with error message - if ($sessionStatus == 2) { - // session timed out - $url = generateControllerUrl("login", "errorMessage=" . urlencode("Session timed out")); - } else { - $url = generateControllerUrl("login"); - } - $redirect = urlencode(KTUtil::addQueryStringSelf($_SERVER["QUERY_STRING"])); - if ((strlen($redirect) > 1)) { - $default->log->debug("checkSession:: redirect url=$redirect"); - // this session verification failure represents either the first visit to - // the site OR a session timeout etc. (in which case we still want to bounce - // the user to the login page, and then back to whatever page they're on now) - $url = $url . "&redirect=" . $redirect; - } - $default->log->debug("checkSession:: about to redirect to $url"); - redirect($url); - exit; - } else { - return false; - } - } else { + if ($sessionStatus === true) { $default->log->debug("checkSession:: returning true"); return true; } + + // verification failed + $default->log->debug("checkSession:: session check failed"); + if (empty($bRedirect)) { + return false; + } + + $sErrorMessage = ""; + if (PEAR::isError($sessionStatus)) { + $sErrorMessage = $sessionStatus->getMessage(); + } + // redirect to login with error message + if ($sErrorMessage) { + // session timed out + $url = generateControllerUrl("login", "errorMessage=" . urlencode($sErrorMessage)); + } else { + $url = generateControllerUrl("login"); + } + + $redirect = urlencode(KTUtil::addQueryStringSelf($_SERVER["QUERY_STRING"])); + if ((strlen($redirect) > 1)) { + $default->log->debug("checkSession:: redirect url=$redirect"); + // this session verification failure represents either the first visit to + // the site OR a session timeout etc. (in which case we still want to bounce + // the user to the login page, and then back to whatever page they're on now) + $url = $url . "&redirect=" . $redirect; + } + $default->log->debug("checkSession:: about to redirect to $url"); + redirect($url); + exit; } /** diff --git a/login.php b/login.php index 694c232..4d81238 100644 --- a/login.php +++ b/login.php @@ -38,11 +38,11 @@ class LoginPageDispatcher extends KTDispatcher { function check() { // bounce out immediately. - $session = new Session(); - if ($session->verify() == 1) { // erk. neil - DOUBLE CHECK THIS PLEASE. + $this->session = new Session(); + if ($this->session->verify() == 1) { // erk. neil - DOUBLE CHECK THIS PLEASE. exit(redirect(generateControllerLink('dashboard'))); } else { - $session->destroy(); // toast it - its probably a hostile session. + $this->session->destroy(); // toast it - its probably a hostile session. } return true; } @@ -139,7 +139,7 @@ class LoginPageDispatcher extends KTDispatcher { } $session = new Session(); - $sessionID = $session->create($oUser->getId()); + $sessionID = $session->create($oUser); // DEPRECATED initialise page-level authorisation array $_SESSION["pageAccess"] = NULL; @@ -156,6 +156,9 @@ class LoginPageDispatcher extends KTDispatcher { } else { $url = generateControllerUrl("dashboard"); } + $oAuthenticator =& KTAuthenticationUtil::getAuthenticatorForUser($oUser); + $oAuthenticator->login($oUser); + exit(redirect($url)); } } -- libgit2 0.21.4