Merge pull request #1086 from virtualtam/refactor/login

Refactor user login and session management

5 files changed, 516 insertions, 153 deletions

diff --git a/tests/HttpUtils/ClientIpIdTest.php b/tests/HttpUtils/ClientIpIdTest.php
new file mode 100644
index 00000000..c15ac5cc
--- /dev/null
+++ b/tests/HttpUtils/ClientIpIdTest.php
@@ -0,0 +1,52 @@
+<?php
+/**
+ * HttpUtils' tests
+ */
+
+require_once 'application/HttpUtils.php';
+
+/**
+ * Unitary tests for client_ip_id()
+ */
+class ClientIpIdTest extends PHPUnit_Framework_TestCase
+{
+    /**
+     * Get a remote client ID based on its IP
+     */
+    public function testClientIpIdRemote()
+    {
+        $this->assertEquals(
+            '10.1.167.42',
+            client_ip_id(['REMOTE_ADDR' => '10.1.167.42'])
+        );
+    }
+
+    /**
+     * Get a remote client ID based on its IP and proxy information (1)
+     */
+    public function testClientIpIdRemoteForwarded()
+    {
+        $this->assertEquals(
+            '10.1.167.42_127.0.1.47',
+            client_ip_id([
+                'REMOTE_ADDR' => '10.1.167.42',
+                'HTTP_X_FORWARDED_FOR' => '127.0.1.47'
+            ])
+        );
+    }
+
+    /**
+     * Get a remote client ID based on its IP and proxy information (2)
+     */
+    public function testClientIpIdRemoteForwardedClient()
+    {
+        $this->assertEquals(
+            '10.1.167.42_10.1.167.56_127.0.1.47',
+            client_ip_id([
+                'REMOTE_ADDR' => '10.1.167.42',
+                'HTTP_X_FORWARDED_FOR' => '10.1.167.56',
+                'HTTP_CLIENT_IP' => '127.0.1.47'
+            ])
+        );
+    }
+}
diff --git a/tests/SessionManagerTest.php b/tests/SessionManagerTest.php
deleted file mode 100644
index aa75962a..00000000
--- a/tests/SessionManagerTest.php
+++ /dev/null
@@ -1,149 +0,0 @@
-<?php
-require_once 'tests/utils/FakeConfigManager.php';
-
-// Initialize reference data _before_ PHPUnit starts a session
-require_once 'tests/utils/ReferenceSessionIdHashes.php';
-ReferenceSessionIdHashes::genAllHashes();
-
-use \Shaarli\SessionManager;
-use \PHPUnit\Framework\TestCase;
-
-
-/**
- * Test coverage for SessionManager
- */
-class SessionManagerTest extends TestCase
-{
-    // Session ID hashes
-    protected static $sidHashes = null;
-
-    // Fake ConfigManager
-    protected static $conf = null;
-
-    /**
-     * Assign reference data
-     */
-    public static function setUpBeforeClass()
-    {
-        self::$sidHashes = ReferenceSessionIdHashes::getHashes();
-        self::$conf = new FakeConfigManager();
-    }
-
-    /**
-     * Generate a session token
-     */
-    public function testGenerateToken()
-    {
-        $session = [];
-        $sessionManager = new SessionManager($session, self::$conf);
-
-        $token = $sessionManager->generateToken();
-
-        $this->assertEquals(1, $session['tokens'][$token]);
-        $this->assertEquals(40, strlen($token));
-    }
-
-    /**
-     * Check a session token
-     */
-    public function testCheckToken()
-    {
-        $token = '4dccc3a45ad9d03e5542b90c37d8db6d10f2b38b';
-        $session = [
-            'tokens' => [
-                $token => 1,
-            ],
-        ];
-        $sessionManager = new SessionManager($session, self::$conf);
-
-        // check and destroy the token
-        $this->assertTrue($sessionManager->checkToken($token));
-        $this->assertFalse(isset($session['tokens'][$token]));
-
-        // ensure the token has been destroyed
-        $this->assertFalse($sessionManager->checkToken($token));
-    }
-
-    /**
-     * Generate and check a session token
-     */
-    public function testGenerateAndCheckToken()
-    {
-        $session = [];
-        $sessionManager = new SessionManager($session, self::$conf);
-
-        $token = $sessionManager->generateToken();
-
-        // ensure a token has been generated
-        $this->assertEquals(1, $session['tokens'][$token]);
-        $this->assertEquals(40, strlen($token));
-
-        // check and destroy the token
-        $this->assertTrue($sessionManager->checkToken($token));
-        $this->assertFalse(isset($session['tokens'][$token]));
-
-        // ensure the token has been destroyed
-        $this->assertFalse($sessionManager->checkToken($token));
-    }
-
-    /**
-     * Check an invalid session token
-     */
-    public function testCheckInvalidToken()
-    {
-        $session = [];
-        $sessionManager = new SessionManager($session, self::$conf);
-
-        $this->assertFalse($sessionManager->checkToken('4dccc3a45ad9d03e5542b90c37d8db6d10f2b38b'));
-    }
-
-    /**
-     * Test SessionManager::checkId with a valid ID - TEST ALL THE HASHES!
-     *
-     * This tests extensively covers all hash algorithms / bit representations
-     */
-    public function testIsAnyHashSessionIdValid()
-    {
-        foreach (self::$sidHashes as $algo => $bpcs) {
-            foreach ($bpcs as $bpc => $hash) {
-                $this->assertTrue(SessionManager::checkId($hash));
-            }
-        }
-    }
-
-    /**
-     * Test checkId with a valid ID - SHA-1 hashes
-     */
-    public function testIsSha1SessionIdValid()
-    {
-        $this->assertTrue(SessionManager::checkId(sha1('shaarli')));
-    }
-
-    /**
-     * Test checkId with a valid ID - SHA-256 hashes
-     */
-    public function testIsSha256SessionIdValid()
-    {
-        $this->assertTrue(SessionManager::checkId(hash('sha256', 'shaarli')));
-    }
-
-    /**
-     * Test checkId with a valid ID - SHA-512 hashes
-     */
-    public function testIsSha512SessionIdValid()
-    {
-        $this->assertTrue(SessionManager::checkId(hash('sha512', 'shaarli')));
-    }
-
-    /**
-     * Test checkId with invalid IDs.
-     */
-    public function testIsSessionIdInvalid()
-    {
-        $this->assertFalse(SessionManager::checkId(''));
-        $this->assertFalse(SessionManager::checkId([]));
-        $this->assertFalse(
-            SessionManager::checkId('c0ZqcWF3VFE2NmJBdm1HMVQ0ZHJ3UmZPbTFsNGhkNHI=')
-        );
-    }
-}
diff --git a/tests/LoginManagerTest.php b/tests/security/LoginManagerTest.php
index 4159038e..f26cd1eb 100644
--- a/tests/LoginManagerTest.php
+++ b/tests/security/LoginManagerTest.php
@@ -1,5 +1,5 @@
 <?php
-namespace Shaarli;
+namespace Shaarli\Security;
 
 require_once 'tests/utils/FakeConfigManager.php';
 use \PHPUnit\Framework\TestCase;
@@ -9,15 +9,54 @@ use \PHPUnit\Framework\TestCase;
  */
 class LoginManagerTest extends TestCase
 {
+    /** @var \FakeConfigManager Configuration Manager instance */
     protected $configManager = null;
+
+    /** @var LoginManager Login Manager instance */
     protected $loginManager = null;
+
+    /** @var SessionManager Session Manager instance */
+    protected $sessionManager = null;
+
+    /** @var string Banned IP filename */
     protected $banFile = 'sandbox/ipbans.php';
+
+    /** @var string Log filename */
     protected $logFile = 'sandbox/shaarli.log';
+
+    /** @var array Simulates the $_COOKIE array */
+    protected $cookie = [];
+
+    /** @var array Simulates the $GLOBALS array */
     protected $globals = [];
-    protected $ipAddr = '127.0.0.1';
+
+    /** @var array Simulates the $_SERVER array */
     protected $server = [];
+
+    /** @var array Simulates the $_SESSION array */
+    protected $session = [];
+
+    /** @var string Advertised client IP address */
+    protected $clientIpAddress = '10.1.47.179';
+
+    /** @var string Local client IP address */
+    protected $ipAddr = '127.0.0.1';
+
+    /** @var string Trusted proxy IP address */
     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 +66,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,
@@ -35,10 +79,15 @@ class LoginManagerTest extends TestCase
             'security.trusted_proxies' => [$this->trustedProxy],
         ]);
 
+        $this->cookie = [];
+
         $this->globals = &$GLOBALS;
         unset($this->globals['IPBANS']);
 
-        $this->loginManager = new LoginManager($this->globals, $this->configManager);
+        $this->session = [];
+
+        $this->sessionManager = new SessionManager($this->session, $this->configManager);
+        $this->loginManager = new LoginManager($this->globals, $this->configManager, $this->sessionManager);
         $this->server['REMOTE_ADDR'] = $this->ipAddr;
     }
 
@@ -59,7 +108,7 @@ class LoginManagerTest extends TestCase
             $this->banFile,
             "<?php\n\$GLOBALS['IPBANS']=array('FAILURES' => array('127.0.0.1' => 99));\n?>"
         );
-        new LoginManager($this->globals, $this->configManager);
+        new LoginManager($this->globals, $this->configManager, null);
         $this->assertEquals(99, $this->globals['IPBANS']['FAILURES']['127.0.0.1']);
     }
 
@@ -196,4 +245,130 @@ 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()
+    {
+        $this->loginManager->generateStaySignedInToken($this->clientIpAddress);
+
+        $this->assertEquals(
+            sha1($this->passwordHash . $this->clientIpAddress . $this->salt),
+            $this->loginManager->getStaySignedInToken()
+        );
+    }
+
+    /**
+     * Check user login - Shaarli has not yet been configured
+     */
+    public function testCheckLoginStateNotConfigured()
+    {
+        $configManager = new \FakeConfigManager([
+            'resource.ban_file' => $this->banFile,
+        ]);
+        $loginManager = new LoginManager($this->globals, $configManager, null);
+        $loginManager->checkLoginState([], '');
+
+        $this->assertFalse($loginManager->isLoggedIn());
+    }
+
+    /**
+     * Check user login - the client cookie does not match the server token
+     */
+    public function testCheckLoginStateStaySignedInWithInvalidToken()
+    {
+        // simulate a previous login
+        $this->session = [
+            'ip' => $this->clientIpAddress,
+            'expires_on' => time() + 100,
+        ];
+        $this->loginManager->generateStaySignedInToken($this->clientIpAddress);
+        $this->cookie[LoginManager::$STAY_SIGNED_IN_COOKIE] = 'nope';
+
+        $this->loginManager->checkLoginState($this->cookie, $this->clientIpAddress);
+
+        $this->assertTrue($this->loginManager->isLoggedIn());
+        $this->assertTrue(empty($this->session['username']));
+    }
+
+    /**
+     * Check user login - the client cookie matches the server token
+     */
+    public function testCheckLoginStateStaySignedInWithValidToken()
+    {
+        $this->loginManager->generateStaySignedInToken($this->clientIpAddress);
+        $this->cookie[LoginManager::$STAY_SIGNED_IN_COOKIE] = $this->loginManager->getStaySignedInToken();
+
+        $this->loginManager->checkLoginState($this->cookie, $this->clientIpAddress);
+
+        $this->assertTrue($this->loginManager->isLoggedIn());
+        $this->assertEquals($this->login, $this->session['username']);
+        $this->assertEquals($this->clientIpAddress, $this->session['ip']);
+    }
+
+    /**
+     * Check user login - the session has expired
+     */
+    public function testCheckLoginStateSessionExpired()
+    {
+        $this->loginManager->generateStaySignedInToken($this->clientIpAddress);
+        $this->session['expires_on'] = time() - 100;
+
+        $this->loginManager->checkLoginState($this->cookie, $this->clientIpAddress);
+
+        $this->assertFalse($this->loginManager->isLoggedIn());
+    }
+
+    /**
+     * Check user login - the remote client IP has changed
+     */
+    public function testCheckLoginStateClientIpChanged()
+    {
+        $this->loginManager->generateStaySignedInToken($this->clientIpAddress);
+
+        $this->loginManager->checkLoginState($this->cookie, '10.7.157.98');
+
+        $this->assertFalse($this->loginManager->isLoggedIn());
+    }
+
+    /**
+     * Check user credentials - wrong login supplied
+     */
+    public function testCheckCredentialsWrongLogin()
+    {
+        $this->assertFalse(
+            $this->loginManager->checkCredentials('', '', 'b4dl0g1n', $this->password)
+        );
+    }
+
+    /**
+     * Check user credentials - wrong password supplied
+     */
+    public function testCheckCredentialsWrongPassword()
+    {
+        $this->assertFalse(
+            $this->loginManager->checkCredentials('', '', $this->login, 'b4dp455wd')
+        );
+    }
+
+    /**
+     * Check user credentials - wrong login and password supplied
+     */
+    public function testCheckCredentialsWrongLoginAndPassword()
+    {
+        $this->assertFalse(
+            $this->loginManager->checkCredentials('', '', 'b4dl0g1n', 'b4dp455wd')
+        );
+    }
+
+    /**
+     * Check user credentials - correct login and password supplied
+     */
+    public function testCheckCredentialsGoodLoginAndPassword()
+    {
+        $this->assertTrue(
+            $this->loginManager->checkCredentials('', '', $this->login, $this->password)
+        );
+    }
 }
diff --git a/tests/security/SessionManagerTest.php b/tests/security/SessionManagerTest.php
new file mode 100644
index 00000000..9bd868f8
--- /dev/null
+++ b/tests/security/SessionManagerTest.php
@@ -0,0 +1,273 @@
+<?php
+require_once 'tests/utils/FakeConfigManager.php';
+
+// Initialize reference data _before_ PHPUnit starts a session
+require_once 'tests/utils/ReferenceSessionIdHashes.php';
+ReferenceSessionIdHashes::genAllHashes();
+
+use \Shaarli\Security\SessionManager;
+use \PHPUnit\Framework\TestCase;
+
+
+/**
+ * Test coverage for SessionManager
+ */
+class SessionManagerTest extends TestCase
+{
+    /** @var array Session ID hashes */
+    protected static $sidHashes = null;
+
+    /** @var \FakeConfigManager ConfigManager substitute for testing */
+    protected $conf = null;
+
+    /** @var array $_SESSION array for testing */
+    protected $session = [];
+
+    /** @var SessionManager Server-side session management abstraction */
+    protected $sessionManager = null;
+
+    /**
+     * Assign reference data
+     */
+    public static function setUpBeforeClass()
+    {
+        self::$sidHashes = ReferenceSessionIdHashes::getHashes();
+    }
+
+    /**
+     * Initialize or reset test resources
+     */
+    public function setUp()
+    {
+        $this->conf = new FakeConfigManager([
+            'credentials.login' => 'johndoe',
+            'credentials.salt' => 'salt',
+            'security.session_protection_disabled' => false,
+        ]);
+        $this->session = [];
+        $this->sessionManager = new SessionManager($this->session, $this->conf);
+    }
+
+    /**
+     * Generate a session token
+     */
+    public function testGenerateToken()
+    {
+        $token = $this->sessionManager->generateToken();
+
+        $this->assertEquals(1, $this->session['tokens'][$token]);
+        $this->assertEquals(40, strlen($token));
+    }
+
+    /**
+     * Check a session token
+     */
+    public function testCheckToken()
+    {
+        $token = '4dccc3a45ad9d03e5542b90c37d8db6d10f2b38b';
+        $session = [
+            'tokens' => [
+                $token => 1,
+            ],
+        ];
+        $sessionManager = new SessionManager($session, $this->conf);
+
+        // check and destroy the token
+        $this->assertTrue($sessionManager->checkToken($token));
+        $this->assertFalse(isset($session['tokens'][$token]));
+
+        // ensure the token has been destroyed
+        $this->assertFalse($sessionManager->checkToken($token));
+    }
+
+    /**
+     * Generate and check a session token
+     */
+    public function testGenerateAndCheckToken()
+    {
+        $token = $this->sessionManager->generateToken();
+
+        // ensure a token has been generated
+        $this->assertEquals(1, $this->session['tokens'][$token]);
+        $this->assertEquals(40, strlen($token));
+
+        // check and destroy the token
+        $this->assertTrue($this->sessionManager->checkToken($token));
+        $this->assertFalse(isset($this->session['tokens'][$token]));
+
+        // ensure the token has been destroyed
+        $this->assertFalse($this->sessionManager->checkToken($token));
+    }
+
+    /**
+     * Check an invalid session token
+     */
+    public function testCheckInvalidToken()
+    {
+        $this->assertFalse($this->sessionManager->checkToken('4dccc3a45ad9d03e5542b90c37d8db6d10f2b38b'));
+    }
+
+    /**
+     * Test SessionManager::checkId with a valid ID - TEST ALL THE HASHES!
+     *
+     * This tests extensively covers all hash algorithms / bit representations
+     */
+    public function testIsAnyHashSessionIdValid()
+    {
+        foreach (self::$sidHashes as $algo => $bpcs) {
+            foreach ($bpcs as $bpc => $hash) {
+                $this->assertTrue(SessionManager::checkId($hash));
+            }
+        }
+    }
+
+    /**
+     * Test checkId with a valid ID - SHA-1 hashes
+     */
+    public function testIsSha1SessionIdValid()
+    {
+        $this->assertTrue(SessionManager::checkId(sha1('shaarli')));
+    }
+
+    /**
+     * Test checkId with a valid ID - SHA-256 hashes
+     */
+    public function testIsSha256SessionIdValid()
+    {
+        $this->assertTrue(SessionManager::checkId(hash('sha256', 'shaarli')));
+    }
+
+    /**
+     * Test checkId with a valid ID - SHA-512 hashes
+     */
+    public function testIsSha512SessionIdValid()
+    {
+        $this->assertTrue(SessionManager::checkId(hash('sha512', 'shaarli')));
+    }
+
+    /**
+     * Test checkId with invalid IDs.
+     */
+    public function testIsSessionIdInvalid()
+    {
+        $this->assertFalse(SessionManager::checkId(''));
+        $this->assertFalse(SessionManager::checkId([]));
+        $this->assertFalse(
+            SessionManager::checkId('c0ZqcWF3VFE2NmJBdm1HMVQ0ZHJ3UmZPbTFsNGhkNHI=')
+        );
+    }
+
+    /**
+     * Store login information after a successful login
+     */
+    public function testStoreLoginInfo()
+    {
+        $this->sessionManager->storeLoginInfo('ip_id');
+
+        $this->assertGreaterThan(time(), $this->session['expires_on']);
+        $this->assertEquals('ip_id', $this->session['ip']);
+        $this->assertEquals('johndoe', $this->session['username']);
+    }
+
+    /**
+     * Extend a server-side session by SessionManager::$SHORT_TIMEOUT
+     */
+    public function testExtendSession()
+    {
+        $this->sessionManager->extendSession();
+
+        $this->assertGreaterThan(time(), $this->session['expires_on']);
+        $this->assertLessThanOrEqual(
+            time() + SessionManager::$SHORT_TIMEOUT,
+            $this->session['expires_on']
+        );
+    }
+
+    /**
+     * Extend a server-side session by SessionManager::$LONG_TIMEOUT
+     */
+    public function testExtendSessionStaySignedIn()
+    {
+        $this->sessionManager->setStaySignedIn(true);
+        $this->sessionManager->extendSession();
+
+        $this->assertGreaterThan(time(), $this->session['expires_on']);
+        $this->assertGreaterThan(
+            time() + SessionManager::$LONG_TIMEOUT - 10,
+            $this->session['expires_on']
+        );
+        $this->assertLessThanOrEqual(
+            time() + SessionManager::$LONG_TIMEOUT,
+            $this->session['expires_on']
+        );
+    }
+
+    /**
+     * Unset session variables after logging out
+     */
+    public function testLogout()
+    {
+        $this->session = [
+            'ip' => 'ip_id',
+            'expires_on' => time() + 1000,
+            'username' => 'johndoe',
+            'visibility' => 'public',
+            'untaggedonly' => false,
+        ];
+        $this->sessionManager->logout();
+
+        $this->assertFalse(isset($this->session['ip']));
+        $this->assertFalse(isset($this->session['expires_on']));
+        $this->assertFalse(isset($this->session['username']));
+        $this->assertFalse(isset($this->session['visibility']));
+        $this->assertFalse(isset($this->session['untaggedonly']));
+    }
+
+    /**
+     * The session is active and expiration time has been reached
+     */
+    public function testHasExpiredTimeElapsed()
+    {
+        $this->session['expires_on'] = time() - 10;
+
+        $this->assertTrue($this->sessionManager->hasSessionExpired());
+    }
+
+    /**
+     * The session is active and expiration time has not been reached
+     */
+    public function testHasNotExpired()
+    {
+        $this->session['expires_on'] = time() + 1000;
+
+        $this->assertFalse($this->sessionManager->hasSessionExpired());
+    }
+
+    /**
+     * Session hijacking protection is disabled, we assume the IP has not changed
+     */
+    public function testHasClientIpChangedNoSessionProtection()
+    {
+        $this->conf->set('security.session_protection_disabled', true);
+
+        $this->assertFalse($this->sessionManager->hasClientIpChanged(''));
+    }
+
+    /**
+     * The client IP identifier has not changed
+     */
+    public function testHasClientIpChangedNope()
+    {
+        $this->session['ip'] = 'ip_id';
+        $this->assertFalse($this->sessionManager->hasClientIpChanged('ip_id'));
+    }
+
+    /**
+     * The client IP identifier has changed
+     */
+    public function testHasClientIpChanged()
+    {
+        $this->session['ip'] = 'ip_id_one';
+        $this->assertTrue($this->sessionManager->hasClientIpChanged('ip_id_two'));
+    }
+}
diff --git a/tests/utils/FakeConfigManager.php b/tests/utils/FakeConfigManager.php
index 85434de7..360b34a9 100644
--- a/tests/utils/FakeConfigManager.php
+++ b/tests/utils/FakeConfigManager.php
@@ -42,4 +42,16 @@ class FakeConfigManager
         }
         return $key;
     }
+
+    /**
+     * Check if a setting exists
+     *
+     * @param string $setting Asked setting, keys separated with dots
+     *
+     * @return bool true if the setting exists, false otherwise
+     */
+    public function exists($setting)
+    {
+        return array_key_exists($setting, $this->values);
+    }
 }