Refactor LoginManager stay-signed-in token management

Signed-off-by: VirtualTam <virtualtam@flibidi.net>

diff --git a/application/security/LoginManager.php b/application/security/LoginManager.php
index 27247f3f1fd9ac97581e220be54091400dfe91fb..41fa9a20ee9c4d025e6ae72f59897e9c7c3c2f17 100644 (file)
--- 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 0dcd7f905f890a37a5e103fa65deb96cd9aaa4f1..5897313043f0796fe8d2ad380b59206f212aebb8 100644 (file)
--- 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 8e3bade03b993e686712c3701663f3a800ecb752..c34434ddcb0016b896330d63d19c7cdfc16fdd78 100644 (file)
--- 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 b957abe3fcc7f1c25ae436b5e36c47e3a7aaf8a5..633f1bb9ea7e853c2e961f2c11ccc2f2ae81bb24 100644 (file)
--- 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()
+        );
+    }
 }