From c689e108639a4f6aa9e15928422e14db7cbe30ca Mon Sep 17 00:00:00 2001 From: VirtualTam Date: Sun, 6 May 2018 17:06:36 +0200 Subject: [PATCH] Refactor LoginManager stay-signed-in token management Signed-off-by: VirtualTam --- application/security/LoginManager.php | 37 ++++++++++++++++++++++--- application/security/SessionManager.php | 3 -- index.php | 12 ++++---- tests/security/LoginManagerTest.php | 31 +++++++++++++++++++++ 4 files changed, 69 insertions(+), 14 deletions(-) diff --git a/application/security/LoginManager.php b/application/security/LoginManager.php index 27247f3f..41fa9a20 100644 --- a/application/security/LoginManager.php +++ b/application/security/LoginManager.php @@ -8,6 +8,9 @@ use Shaarli\Config\ConfigManager; */ class LoginManager { + /** @var string Name of the cookie set after logging in **/ + public static $STAY_SIGNED_IN_COOKIE = 'shaarli_staySignedIn'; + /** @var array A reference to the $_GLOBALS array */ protected $globals = []; @@ -26,6 +29,9 @@ class LoginManager /** @var bool Whether the Shaarli instance is open to public edition **/ protected $openShaarli = false; + /** @var string User sign-in token depending on remote IP and credentials */ + protected $staySignedInToken = ''; + /** * Constructor * @@ -45,16 +51,39 @@ class LoginManager } } + /** + * Generate a token depending on deployment salt, user password and client IP + * + * @param string $clientIpAddress The remote client IP address + */ + public function generateStaySignedInToken($clientIpAddress) + { + $this->staySignedInToken = sha1( + $this->configManager->get('credentials.hash') + . $clientIpAddress + . $this->configManager->get('credentials.salt') + ); + } + + /** + * Return the user's client stay-signed-in token + * + * @return string User's client stay-signed-in token + */ + public function getStaySignedInToken() + { + return $this->staySignedInToken; + } + /** * Check user session state and validity (expiration) * * @param array $cookie The $_COOKIE array * @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, $clientIpId, $token) + public function checkLoginState($cookie, $clientIpId) { if (! $this->configManager->exists('credentials.login')) { // Shaarli is not configured yet @@ -62,8 +91,8 @@ class LoginManager return; } - if (isset($cookie[SessionManager::$LOGGED_IN_COOKIE]) - && $cookie[SessionManager::$LOGGED_IN_COOKIE] === $token + if (isset($cookie[self::$STAY_SIGNED_IN_COOKIE]) + && $cookie[self::$STAY_SIGNED_IN_COOKIE] === $this->staySignedInToken ) { $this->sessionManager->storeLoginInfo($clientIpId); $this->isLoggedIn = true; diff --git a/application/security/SessionManager.php b/application/security/SessionManager.php index 0dcd7f90..58973130 100644 --- a/application/security/SessionManager.php +++ b/application/security/SessionManager.php @@ -14,9 +14,6 @@ class SessionManager /** @var int Session expiration timeout, in seconds */ public static $LONG_TIMEOUT = 31536000; // 1 year - /** @var string Name of the cookie set after logging in **/ - public static $LOGGED_IN_COOKIE = 'shaarli_staySignedIn'; - /** @var array Local reference to the global $_SESSION array */ protected $session = []; diff --git a/index.php b/index.php index 8e3bade0..c34434dd 100644 --- a/index.php +++ b/index.php @@ -123,6 +123,7 @@ if (isset($_COOKIE['shaarli']) && !SessionManager::checkId($_COOKIE['shaarli'])) $conf = new ConfigManager(); $sessionManager = new SessionManager($_SESSION, $conf); $loginManager = new LoginManager($GLOBALS, $conf, $sessionManager); +$loginManager->generateStaySignedInToken($_SERVER['REMOTE_ADDR']); $clientIpId = client_ip_id($_SERVER); // LC_MESSAGES isn't defined without php-intl, in this case use LC_COLLATE locale instead. @@ -176,10 +177,7 @@ if (! is_file($conf->getConfigFileExt())) { install($conf, $sessionManager); } -// a token depending of deployment salt, user password, and the current ip -define('STAY_SIGNED_IN_TOKEN', sha1($conf->get('credentials.hash') . $_SERVER['REMOTE_ADDR'] . $conf->get('credentials.salt'))); - -$loginManager->checkLoginState($_COOKIE, $clientIpId, STAY_SIGNED_IN_TOKEN); +$loginManager->checkLoginState($_COOKIE, $clientIpId); /** * Adapter function to ensure compatibility with third-party templates @@ -219,8 +217,8 @@ if (isset($_POST['login'])) { $expirationTime = $sessionManager->extendSession(); setcookie( - $sessionManager::$LOGGED_IN_COOKIE, - STAY_SIGNED_IN_TOKEN, + $loginManager::$STAY_SIGNED_IN_COOKIE, + $loginManager->getStaySignedInToken(), $expirationTime, WEB_PATH ); @@ -595,7 +593,7 @@ function renderPage($conf, $pluginManager, $LINKSDB, $history, $sessionManager, { invalidateCaches($conf->get('resource.page_cache')); $sessionManager->logout(); - setcookie(SessionManager::$LOGGED_IN_COOKIE, 'false', 0, WEB_PATH); + setcookie(LoginManager::$STAY_SIGNED_IN_COOKIE, 'false', 0, WEB_PATH); header('Location: ?'); exit; } diff --git a/tests/security/LoginManagerTest.php b/tests/security/LoginManagerTest.php index b957abe3..633f1bb9 100644 --- a/tests/security/LoginManagerTest.php +++ b/tests/security/LoginManagerTest.php @@ -18,6 +18,18 @@ class LoginManagerTest extends TestCase protected $server = []; protected $trustedProxy = '10.1.1.100'; + /** @var string User login */ + protected $login = 'johndoe'; + + /** @var string User password */ + protected $password = 'IC4nHazL0g1n?'; + + /** @var string Hash of the salted user password */ + protected $passwordHash = ''; + + /** @var string Salt used by hash functions */ + protected $salt = '669e24fa9c5a59a613f98e8e38327384504a4af2'; + /** * Prepare or reset test resources */ @@ -27,7 +39,12 @@ class LoginManagerTest extends TestCase unlink($this->banFile); } + $this->passwordHash = sha1($this->password . $this->login . $this->salt); + $this->configManager = new \FakeConfigManager([ + 'credentials.login' => $this->login, + 'credentials.hash' => $this->passwordHash, + 'credentials.salt' => $this->salt, 'resource.ban_file' => $this->banFile, 'resource.log' => $this->logFile, 'security.ban_after' => 4, @@ -196,4 +213,18 @@ class LoginManagerTest extends TestCase $this->globals['IPBANS']['BANS'][$this->ipAddr] = time() - 3600; $this->assertTrue($this->loginManager->canLogin($this->server)); } + + /** + * Generate a token depending on the user credentials and client IP + */ + public function testGenerateStaySignedInToken() + { + $ipAddress = '10.1.47.179'; + $this->loginManager->generateStaySignedInToken($ipAddress); + + $this->assertEquals( + sha1($this->passwordHash . $ipAddress . $this->salt), + $this->loginManager->getStaySignedInToken() + ); + } } -- 2.41.0