From 63ea23c2a67d2a1cf6cda79fa2fe49a143571cde Mon Sep 17 00:00:00 2001 From: VirtualTam Date: Sat, 17 Feb 2018 01:46:27 +0100 Subject: Refactor user credential validation at login time Changed: - move login/password verification to LoginManager - code cleanup Signed-off-by: VirtualTam --- application/LoginManager.php | 109 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 106 insertions(+), 3 deletions(-) (limited to 'application/LoginManager.php') diff --git a/application/LoginManager.php b/application/LoginManager.php index 397bc6e3..8f6bf0da 100644 --- a/application/LoginManager.php +++ b/application/LoginManager.php @@ -8,20 +8,123 @@ class LoginManager { protected $globals = []; protected $configManager = null; + protected $sessionManager = null; protected $banFile = ''; + protected $isLoggedIn = false; + protected $openShaarli = false; /** * Constructor * - * @param array $globals The $GLOBALS array (reference) - * @param ConfigManager $configManager Configuration Manager instance. + * @param array $globals The $GLOBALS array (reference) + * @param ConfigManager $configManager Configuration Manager instance + * @param SessionManager $sessionManager SessionManager instance */ - public function __construct(& $globals, $configManager) + public function __construct(& $globals, $configManager, $sessionManager) { $this->globals = &$globals; $this->configManager = $configManager; + $this->sessionManager = $sessionManager; $this->banFile = $this->configManager->get('resource.ban_file', 'data/ipbans.php'); $this->readBanFile(); + if ($this->configManager->get('security.open_shaarli')) { + $this->openShaarli = true; + } + } + + /** + * Check user session state and validity (expiration) + * + * @param array $server The $_SERVER array + * @param array $session The $_SESSION array (reference) + * @param array $cookie The $_COOKIE array + * @param string $webPath Path on the server in which the cookie will be available on + * @param string $token Session token + * + * @return bool true if the user session is valid, false otherwise + */ + public function checkLoginState($server, & $session, $cookie, $webPath, $token) + { + if (! $this->configManager->exists('credentials.login')) { + // Shaarli is not configured yet + $this->isLoggedIn = false; + return; + } + + if (isset($cookie[SessionManager::$LOGGED_IN_COOKIE]) + && $cookie[SessionManager::$LOGGED_IN_COOKIE] === $token + ) { + $this->sessionManager->storeLoginInfo($server); + $this->isLoggedIn = true; + } + + // Logout when: + // - the session does not exist on the server side + // - the session has expired + // - the client IP address has changed + if (empty($session['uid']) + || ($this->configManager->get('security.session_protection_disabled') === false + && $session['ip'] != client_ip_id($server)) + || time() >= $session['expires_on'] + ) { + $this->sessionManager->logout($webPath); + $this->isLoggedIn = false; + return; + } + + // Extend session validity + if (! empty($session['longlastingsession'])) { + // "Stay signed in" is enabled + $session['expires_on'] = time() + $session['longlastingsession']; + } else { + $session['expires_on'] = time() + SessionManager::$INACTIVITY_TIMEOUT; + } + } + + /** + * Return whether the user is currently logged in + * + * @return true when the user is logged in, false otherwise + */ + public function isLoggedIn() + { + if ($this->openShaarli) { + return true; + } + return $this->isLoggedIn; + } + + /** + * Check user credentials are valid + * + * @param array $server The $_SERVER array + * @param string $login Username + * @param string $password Password + * + * @return bool true if the provided credentials are valid, false otherwise + */ + public function checkCredentials($server, $login, $password) + { + $hash = sha1($password . $login . $this->configManager->get('credentials.salt')); + + if ($login != $this->configManager->get('credentials.login') + || $hash != $this->configManager->get('credentials.hash') + ) { + logm( + $this->configManager->get('resource.log'), + $server['REMOTE_ADDR'], + 'Login failed for user ' . $login + ); + return false; + } + + $this->sessionManager->storeLoginInfo($server); + logm( + $this->configManager->get('resource.log'), + $server['REMOTE_ADDR'], + 'Login successful' + ); + return true; } /** -- cgit v1.2.3 From 1b28c66cc77b59f716aa47e6207142a7f86c2c2d Mon Sep 17 00:00:00 2001 From: VirtualTam Date: Wed, 4 Apr 2018 00:43:48 +0200 Subject: Document LoginManager properties Signed-off-by: VirtualTam --- application/LoginManager.php | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'application/LoginManager.php') diff --git a/application/LoginManager.php b/application/LoginManager.php index 8f6bf0da..d81c6c05 100644 --- a/application/LoginManager.php +++ b/application/LoginManager.php @@ -6,11 +6,22 @@ namespace Shaarli; */ class LoginManager { + /** @var array A reference to the $_GLOBALS array */ protected $globals = []; + + /** @var ConfigManager Configuration Manager instance **/ protected $configManager = null; + + /** @var SessionManager Session Manager instance **/ protected $sessionManager = null; + + /** @var string Path to the file containing IP bans */ protected $banFile = ''; + + /** @var bool Whether the user is logged in **/ protected $isLoggedIn = false; + + /** @var bool Whether the Shaarli instance is open to public edition **/ protected $openShaarli = false; /** -- cgit v1.2.3 From c7721487b2459e6760cae9d6292b7d39c306d3d6 Mon Sep 17 00:00:00 2001 From: VirtualTam Date: Wed, 4 Apr 2018 00:54:59 +0200 Subject: Delegate session operations to SessionManager Signed-off-by: VirtualTam --- application/LoginManager.php | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) (limited to 'application/LoginManager.php') diff --git a/application/LoginManager.php b/application/LoginManager.php index d81c6c05..347fb3b9 100644 --- a/application/LoginManager.php +++ b/application/LoginManager.php @@ -1,6 +1,8 @@ sessionManager->storeLoginInfo($server); + $this->sessionManager->storeLoginInfo($clientIpId); $this->isLoggedIn = true; } - // Logout when: - // - the session does not exist on the server side - // - the session has expired - // - the client IP address has changed - if (empty($session['uid']) - || ($this->configManager->get('security.session_protection_disabled') === false - && $session['ip'] != client_ip_id($server)) - || time() >= $session['expires_on'] + if ($this->sessionManager->hasSessionExpired() + || $this->sessionManager->hasClientIpChanged($clientIpId) ) { $this->sessionManager->logout($webPath); $this->isLoggedIn = false; return; } - // Extend session validity - if (! empty($session['longlastingsession'])) { - // "Stay signed in" is enabled - $session['expires_on'] = time() + $session['longlastingsession']; - } else { - $session['expires_on'] = time() + SessionManager::$INACTIVITY_TIMEOUT; - } + $this->sessionManager->extendSession(); } /** @@ -129,7 +121,8 @@ class LoginManager return false; } - $this->sessionManager->storeLoginInfo($server); + $clientIpId = client_ip_id($server); + $this->sessionManager->storeLoginInfo($clientIpId); logm( $this->configManager->get('resource.log'), $server['REMOTE_ADDR'], -- cgit v1.2.3 From 847420847455c1339f3302b1b67568ee0f382a11 Mon Sep 17 00:00:00 2001 From: VirtualTam Date: Wed, 18 Apr 2018 23:09:45 +0200 Subject: Pass the client IP ID to LoginManager Signed-off-by: VirtualTam --- application/LoginManager.php | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) (limited to 'application/LoginManager.php') diff --git a/application/LoginManager.php b/application/LoginManager.php index 347fb3b9..5ce836fa 100644 --- a/application/LoginManager.php +++ b/application/LoginManager.php @@ -48,15 +48,15 @@ class LoginManager /** * Check user session state and validity (expiration) * - * @param array $server The $_SERVER array - * @param array $session The $_SESSION array (reference) - * @param array $cookie The $_COOKIE array - * @param string $webPath Path on the server in which the cookie will be available on - * @param string $token Session token + * @param array $session The $_SESSION array (reference) + * @param array $cookie The $_COOKIE array + * @param string $webPath Path on the server in which the cookie will be available on + * @param string $clientIpId Client IP address identifier + * @param string $token Session token * * @return bool true if the user session is valid, false otherwise */ - public function checkLoginState($server, & $session, $cookie, $webPath, $token) + public function checkLoginState(& $session, $cookie, $webPath, $clientIpId, $token) { if (! $this->configManager->exists('credentials.login')) { // Shaarli is not configured yet @@ -64,8 +64,6 @@ class LoginManager return; } - $clientIpId = client_ip_id($server); - if (isset($cookie[SessionManager::$LOGGED_IN_COOKIE]) && $cookie[SessionManager::$LOGGED_IN_COOKIE] === $token ) { @@ -100,13 +98,14 @@ class LoginManager /** * Check user credentials are valid * - * @param array $server The $_SERVER array - * @param string $login Username - * @param string $password Password + * @param string $remoteIp Remote client IP address + * @param string $clientIpId Client IP address identifier + * @param string $login Username + * @param string $password Password * * @return bool true if the provided credentials are valid, false otherwise */ - public function checkCredentials($server, $login, $password) + public function checkCredentials($remoteIp, $clientIpId, $login, $password) { $hash = sha1($password . $login . $this->configManager->get('credentials.salt')); @@ -115,17 +114,16 @@ class LoginManager ) { logm( $this->configManager->get('resource.log'), - $server['REMOTE_ADDR'], + $remoteIp, 'Login failed for user ' . $login ); return false; } - $clientIpId = client_ip_id($server); $this->sessionManager->storeLoginInfo($clientIpId); logm( $this->configManager->get('resource.log'), - $server['REMOTE_ADDR'], + $remoteIp, 'Login successful' ); return true; -- cgit v1.2.3 From 68dcaccfa46649addc66674b627a83064798bbc0 Mon Sep 17 00:00:00 2001 From: VirtualTam Date: Fri, 27 Apr 2018 22:00:35 +0200 Subject: LoginManager: remove unused parameter Signed-off-by: VirtualTam --- application/LoginManager.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'application/LoginManager.php') diff --git a/application/LoginManager.php b/application/LoginManager.php index 5ce836fa..27d06705 100644 --- a/application/LoginManager.php +++ b/application/LoginManager.php @@ -48,7 +48,6 @@ class LoginManager /** * Check user session state and validity (expiration) * - * @param array $session The $_SESSION array (reference) * @param array $cookie The $_COOKIE array * @param string $webPath Path on the server in which the cookie will be available on * @param string $clientIpId Client IP address identifier @@ -56,7 +55,7 @@ class LoginManager * * @return bool true if the user session is valid, false otherwise */ - public function checkLoginState(& $session, $cookie, $webPath, $clientIpId, $token) + public function checkLoginState($cookie, $webPath, $clientIpId, $token) { if (! $this->configManager->exists('credentials.login')) { // Shaarli is not configured yet -- cgit v1.2.3 From fab87c2696b9d6a26310f1bfc024b018ca5184fe Mon Sep 17 00:00:00 2001 From: VirtualTam Date: Fri, 27 Apr 2018 22:12:22 +0200 Subject: Move LoginManager and SessionManager to the Security namespace Signed-off-by: VirtualTam --- application/LoginManager.php | 238 ------------------------------------------- 1 file changed, 238 deletions(-) delete mode 100644 application/LoginManager.php (limited to 'application/LoginManager.php') diff --git a/application/LoginManager.php b/application/LoginManager.php deleted file mode 100644 index 27d06705..00000000 --- a/application/LoginManager.php +++ /dev/null @@ -1,238 +0,0 @@ -globals = &$globals; - $this->configManager = $configManager; - $this->sessionManager = $sessionManager; - $this->banFile = $this->configManager->get('resource.ban_file', 'data/ipbans.php'); - $this->readBanFile(); - if ($this->configManager->get('security.open_shaarli')) { - $this->openShaarli = true; - } - } - - /** - * Check user session state and validity (expiration) - * - * @param array $cookie The $_COOKIE array - * @param string $webPath Path on the server in which the cookie will be available on - * @param string $clientIpId Client IP address identifier - * @param string $token Session token - * - * @return bool true if the user session is valid, false otherwise - */ - public function checkLoginState($cookie, $webPath, $clientIpId, $token) - { - if (! $this->configManager->exists('credentials.login')) { - // Shaarli is not configured yet - $this->isLoggedIn = false; - return; - } - - if (isset($cookie[SessionManager::$LOGGED_IN_COOKIE]) - && $cookie[SessionManager::$LOGGED_IN_COOKIE] === $token - ) { - $this->sessionManager->storeLoginInfo($clientIpId); - $this->isLoggedIn = true; - } - - if ($this->sessionManager->hasSessionExpired() - || $this->sessionManager->hasClientIpChanged($clientIpId) - ) { - $this->sessionManager->logout($webPath); - $this->isLoggedIn = false; - return; - } - - $this->sessionManager->extendSession(); - } - - /** - * Return whether the user is currently logged in - * - * @return true when the user is logged in, false otherwise - */ - public function isLoggedIn() - { - if ($this->openShaarli) { - return true; - } - return $this->isLoggedIn; - } - - /** - * Check user credentials are valid - * - * @param string $remoteIp Remote client IP address - * @param string $clientIpId Client IP address identifier - * @param string $login Username - * @param string $password Password - * - * @return bool true if the provided credentials are valid, false otherwise - */ - public function checkCredentials($remoteIp, $clientIpId, $login, $password) - { - $hash = sha1($password . $login . $this->configManager->get('credentials.salt')); - - if ($login != $this->configManager->get('credentials.login') - || $hash != $this->configManager->get('credentials.hash') - ) { - logm( - $this->configManager->get('resource.log'), - $remoteIp, - 'Login failed for user ' . $login - ); - return false; - } - - $this->sessionManager->storeLoginInfo($clientIpId); - logm( - $this->configManager->get('resource.log'), - $remoteIp, - 'Login successful' - ); - return true; - } - - /** - * Read a file containing banned IPs - */ - protected function readBanFile() - { - if (! file_exists($this->banFile)) { - return; - } - include $this->banFile; - } - - /** - * Write the banned IPs to a file - */ - protected function writeBanFile() - { - if (! array_key_exists('IPBANS', $this->globals)) { - return; - } - file_put_contents( - $this->banFile, - "globals['IPBANS'], true) . ";\n?>" - ); - } - - /** - * Handle a failed login and ban the IP after too many failed attempts - * - * @param array $server The $_SERVER array - */ - public function handleFailedLogin($server) - { - $ip = $server['REMOTE_ADDR']; - $trusted = $this->configManager->get('security.trusted_proxies', []); - - if (in_array($ip, $trusted)) { - $ip = getIpAddressFromProxy($server, $trusted); - if (! $ip) { - // the IP is behind a trusted forward proxy, but is not forwarded - // in the HTTP headers, so we do nothing - return; - } - } - - // increment the fail count for this IP - if (isset($this->globals['IPBANS']['FAILURES'][$ip])) { - $this->globals['IPBANS']['FAILURES'][$ip]++; - } else { - $this->globals['IPBANS']['FAILURES'][$ip] = 1; - } - - if ($this->globals['IPBANS']['FAILURES'][$ip] >= $this->configManager->get('security.ban_after')) { - $this->globals['IPBANS']['BANS'][$ip] = time() + $this->configManager->get('security.ban_duration', 1800); - logm( - $this->configManager->get('resource.log'), - $server['REMOTE_ADDR'], - 'IP address banned from login' - ); - } - $this->writeBanFile(); - } - - /** - * Handle a successful login - * - * @param array $server The $_SERVER array - */ - public function handleSuccessfulLogin($server) - { - $ip = $server['REMOTE_ADDR']; - // FIXME unban when behind a trusted proxy? - - unset($this->globals['IPBANS']['FAILURES'][$ip]); - unset($this->globals['IPBANS']['BANS'][$ip]); - - $this->writeBanFile(); - } - - /** - * Check if the user can login from this IP - * - * @param array $server The $_SERVER array - * - * @return bool true if the user is allowed to login - */ - public function canLogin($server) - { - $ip = $server['REMOTE_ADDR']; - - if (! isset($this->globals['IPBANS']['BANS'][$ip])) { - // the user is not banned - return true; - } - - if ($this->globals['IPBANS']['BANS'][$ip] > time()) { - // the user is still banned - return false; - } - - // the ban has expired, the user can attempt to log in again - logm($this->configManager->get('resource.log'), $server['REMOTE_ADDR'], 'Ban lifted.'); - unset($this->globals['IPBANS']['FAILURES'][$ip]); - unset($this->globals['IPBANS']['BANS'][$ip]); - - $this->writeBanFile(); - return true; - } -} -- cgit v1.2.3