From 88110550b89617dcda16441212599b8a40faa20c Mon Sep 17 00:00:00 2001
From: VirtualTam <virtualtam@flibidi.net>
Date: Fri, 16 Feb 2018 21:51:44 +0100
Subject: Refactor client session hijacking protection

Signed-off-by: VirtualTam <virtualtam@flibidi.net>
---
 application/HttpUtils.php          | 33 +++++++++++++++++++++++-
 index.php                          | 14 ++--------
 tests/HttpUtils/ClientIpIdTest.php | 52 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 86 insertions(+), 13 deletions(-)
 create mode 100644 tests/HttpUtils/ClientIpIdTest.php

diff --git a/application/HttpUtils.php b/application/HttpUtils.php
index 83a4c5e2..e9282506 100644
--- a/application/HttpUtils.php
+++ b/application/HttpUtils.php
@@ -1,7 +1,7 @@
 <?php
 /**
  * GET an HTTP URL to retrieve its content
- * Uses the cURL library or a fallback method 
+ * Uses the cURL library or a fallback method
  *
  * @param string          $url               URL to get (http://...)
  * @param int             $timeout           network timeout (in seconds)
@@ -415,6 +415,37 @@ function getIpAddressFromProxy($server, $trustedIps)
     return array_pop($ips);
 }
 
+
+/**
+ * Return an identifier based on the advertised client IP address(es)
+ *
+ * This aims at preventing session hijacking from users behind the same proxy
+ * by relying on HTTP headers.
+ *
+ * See:
+ * - https://secure.php.net/manual/en/reserved.variables.server.php
+ * - https://stackoverflow.com/questions/3003145/how-to-get-the-client-ip-address-in-php
+ * - https://stackoverflow.com/questions/12233406/preventing-session-hijacking
+ * - https://stackoverflow.com/questions/21354859/trusting-x-forwarded-for-to-identify-a-visitor
+ *
+ * @param array $server The $_SERVER array
+ *
+ * @return string An identifier based on client IP address information
+ */
+function client_ip_id($server)
+{
+    $ip = $server['REMOTE_ADDR'];
+
+    if (isset($server['HTTP_X_FORWARDED_FOR'])) {
+        $ip = $ip . '_' . $server['HTTP_X_FORWARDED_FOR'];
+    }
+    if (isset($server['HTTP_CLIENT_IP'])) {
+        $ip = $ip . '_' . $server['HTTP_CLIENT_IP'];
+    }
+    return $ip;
+}
+
+
 /**
  * Returns true if Shaarli's currently browsed in HTTPS.
  * Supports reverse proxies (if the headers are correctly set).
diff --git a/index.php b/index.php
index 2fe3f821..08a69327 100644
--- a/index.php
+++ b/index.php
@@ -207,7 +207,7 @@ function setup_login_state($conf)
     }
     // If session does not exist on server side, or IP address has changed, or session has expired, logout.
     if (empty($_SESSION['uid'])
-        || ($conf->get('security.session_protection_disabled') === false && $_SESSION['ip'] != allIPs())
+        || ($conf->get('security.session_protection_disabled') === false && $_SESSION['ip'] != client_ip_id($_SERVER))
         || time() >= $_SESSION['expires_on'])
     {
         logout();
@@ -231,16 +231,6 @@ $userIsLoggedIn = setup_login_state($conf);
 // ------------------------------------------------------------------------------------------
 // Session management
 
-// Returns the IP address of the client (Used to prevent session cookie hijacking.)
-function allIPs()
-{
-    $ip = $_SERVER['REMOTE_ADDR'];
-    // Then we use more HTTP headers to prevent session hijacking from users behind the same proxy.
-    if (isset($_SERVER['HTTP_X_FORWARDED_FOR'])) { $ip=$ip.'_'.$_SERVER['HTTP_X_FORWARDED_FOR']; }
-    if (isset($_SERVER['HTTP_CLIENT_IP'])) { $ip=$ip.'_'.$_SERVER['HTTP_CLIENT_IP']; }
-    return $ip;
-}
-
 /**
  * Load user session.
  *
@@ -249,7 +239,7 @@ function allIPs()
 function fillSessionInfo($conf)
 {
     $_SESSION['uid'] = sha1(uniqid('',true).'_'.mt_rand()); // Generate unique random number (different than phpsessionid)
-    $_SESSION['ip']=allIPs();                // We store IP address(es) of the client to make sure session is not hijacked.
+    $_SESSION['ip'] = client_ip_id($_SERVER);
     $_SESSION['username']= $conf->get('credentials.login');
     $_SESSION['expires_on']=time()+INACTIVITY_TIMEOUT;  // Set session expiration.
 }
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'
+            ])
+        );
+    }
+}
-- 
cgit v1.2.3


From db45a36a53dbd722e5e891827e49d9e7651f2a5e Mon Sep 17 00:00:00 2001
From: VirtualTam <virtualtam@flibidi.net>
Date: Fri, 16 Feb 2018 22:21:59 +0100
Subject: Refactor SessionManager::$INACTIVITY_TIMEOUT

Changed:
- move INACTIVITY_TIMEOUT to SessionManager
- inject a dependency to a SessionManager instance in:
  - fillSessionInfo()
  - setup_login_state()
  - check_auth()
- cleanup related code and comments

Signed-off-by: VirtualTam <virtualtam@flibidi.net>
---
 application/SessionManager.php |  4 ++++
 index.php                      | 48 +++++++++++++++++++++---------------------
 2 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/application/SessionManager.php b/application/SessionManager.php
index 71f0b38d..704f8504 100644
--- a/application/SessionManager.php
+++ b/application/SessionManager.php
@@ -6,6 +6,10 @@ namespace Shaarli;
  */
 class SessionManager
 {
+    /** Session expiration timeout, in seconds */
+    public static $INACTIVITY_TIMEOUT = 3600;
+
+    /** Local reference to the global $_SESSION array */
     protected $session = [];
 
     /**
diff --git a/index.php b/index.php
index 08a69327..9cbc9241 100644
--- a/index.php
+++ b/index.php
@@ -101,8 +101,6 @@ if (dirname($_SERVER['SCRIPT_NAME']) != '/') {
 // Set default cookie expiration and path.
 session_set_cookie_params($cookie['lifetime'], $cookiedir, $_SERVER['SERVER_NAME']);
 // Set session parameters on server side.
-// If the user does not access any page within this time, his/her session is considered expired.
-define('INACTIVITY_TIMEOUT', 3600); // in seconds.
 // Use cookies to store session.
 ini_set('session.use_cookies', 1);
 // Force cookies for session (phpsessionID forbidden in URL).
@@ -183,11 +181,12 @@ define('STAY_SIGNED_IN_TOKEN', sha1($conf->get('credentials.hash') . $_SERVER['R
 /**
  * Checking session state (i.e. is the user still logged in)
  *
- * @param ConfigManager $conf The configuration manager.
+ * @param ConfigManager  $conf           Configuration Manager instance.
+ * @param SessionManager $sessionManager SessionManager instance
  *
- * @return bool: true if the user is logged in, false otherwise.
+ * @return bool true if the user is logged in, false otherwise.
  */
-function setup_login_state($conf)
+function setup_login_state($conf, $sessionManager)
 {
     if ($conf->get('security.open_shaarli')) {
         return true;
@@ -202,7 +201,7 @@ function setup_login_state($conf)
         $_COOKIE['shaarli_staySignedIn']===STAY_SIGNED_IN_TOKEN &&
         !$loginFailure)
     {
-        fillSessionInfo($conf);
+        fillSessionInfo($conf, $sessionManager);
         $userIsLoggedIn = true;
     }
     // If session does not exist on server side, or IP address has changed, or session has expired, logout.
@@ -216,9 +215,8 @@ function setup_login_state($conf)
     }
     if (!empty($_SESSION['longlastingsession'])) {
         $_SESSION['expires_on']=time()+$_SESSION['longlastingsession']; // In case of "Stay signed in" checked.
-    }
-    else {
-        $_SESSION['expires_on']=time()+INACTIVITY_TIMEOUT; // Standard session expiration date.
+    } else {
+        $_SESSION['expires_on'] = time() + $sessionManager::$INACTIVITY_TIMEOUT;
     }
     if (!$loginFailure) {
         $userIsLoggedIn = true;
@@ -226,39 +224,42 @@ function setup_login_state($conf)
 
     return $userIsLoggedIn;
 }
-$userIsLoggedIn = setup_login_state($conf);
+
+$userIsLoggedIn = setup_login_state($conf, $sessionManager);
 
 // ------------------------------------------------------------------------------------------
 // Session management
 
 /**
- * Load user session.
+ * Load user session
  *
- * @param ConfigManager $conf Configuration Manager instance.
+ * @param ConfigManager  $conf           Configuration Manager instance.
+ * @param SessionManager $sessionManager SessionManager instance
  */
-function fillSessionInfo($conf)
+function fillSessionInfo($conf, $sessionManager)
 {
     $_SESSION['uid'] = sha1(uniqid('',true).'_'.mt_rand()); // Generate unique random number (different than phpsessionid)
     $_SESSION['ip'] = client_ip_id($_SERVER);
     $_SESSION['username']= $conf->get('credentials.login');
-    $_SESSION['expires_on']=time()+INACTIVITY_TIMEOUT;  // Set session expiration.
+    $_SESSION['expires_on'] = time() + $sessionManager::$INACTIVITY_TIMEOUT;
 }
 
 /**
  * Check that user/password is correct.
  *
- * @param string        $login    Username
- * @param string        $password User password
- * @param ConfigManager $conf     Configuration Manager instance.
+ * @param string         $login          Username
+ * @param string         $password       User password
+ * @param ConfigManager  $conf           Configuration Manager instance.
+ * @param SessionManager $sessionManager SessionManager instance
  *
  * @return bool: authentication successful or not.
  */
-function check_auth($login, $password, $conf)
+function check_auth($login, $password, $conf, $sessionManager)
 {
     $hash = sha1($password . $login . $conf->get('credentials.salt'));
-    if ($login == $conf->get('credentials.login') && $hash == $conf->get('credentials.hash'))
-    {   // Login/password is correct.
-        fillSessionInfo($conf);
+    if ($login == $conf->get('credentials.login') && $hash == $conf->get('credentials.hash')) {
+        // Login/password is correct.
+        fillSessionInfo($conf, $sessionManager);
         logm($conf->get('resource.log'), $_SERVER['REMOTE_ADDR'], 'Login successful');
         return true;
     }
@@ -287,14 +288,13 @@ function logout() {
 
 // ------------------------------------------------------------------------------------------
 // Process login form: Check if login/password is correct.
-if (isset($_POST['login']))
-{
+if (isset($_POST['login'])) {
     if (! $loginManager->canLogin($_SERVER)) {
         die(t('I said: NO. You are banned for the moment. Go away.'));
     }
     if (isset($_POST['password'])
         && $sessionManager->checkToken($_POST['token'])
-        && (check_auth($_POST['login'], $_POST['password'], $conf))
+        && (check_auth($_POST['login'], $_POST['password'], $conf, $sessionManager))
     ) {
         // Login/password is OK.
         $loginManager->handleSuccessfulLogin($_SERVER);
-- 
cgit v1.2.3


From 49f183231662c642ca9df6ceabf43fe128a5ffc1 Mon Sep 17 00:00:00 2001
From: VirtualTam <virtualtam@flibidi.net>
Date: Sat, 17 Feb 2018 01:14:58 +0100
Subject: Refactor PHP session handling during login/logout

Changed:
- move $_SESSION handling to SessionManager
- code cleanup

Signed-off-by: VirtualTam <virtualtam@flibidi.net>
---
 application/SessionManager.php | 40 ++++++++++++++++++++++++++++++++++
 index.php                      | 49 +++++++++++-------------------------------
 2 files changed, 53 insertions(+), 36 deletions(-)

diff --git a/application/SessionManager.php b/application/SessionManager.php
index 704f8504..7bfd2220 100644
--- a/application/SessionManager.php
+++ b/application/SessionManager.php
@@ -9,9 +9,15 @@ class SessionManager
     /** Session expiration timeout, in seconds */
     public static $INACTIVITY_TIMEOUT = 3600;
 
+    /** Name of the cookie set after logging in **/
+    public static $LOGGED_IN_COOKIE = 'shaarli_staySignedIn';
+
     /** Local reference to the global $_SESSION array */
     protected $session = [];
 
+    /** ConfigManager instance **/
+    protected $conf = null;
+
     /**
      * Constructor
      *
@@ -84,4 +90,38 @@ class SessionManager
 
         return true;
     }
+
+    /**
+     * Store user login information after a successful login
+     *
+     * @param array $server The global $_SERVER array
+     */
+    public function storeLoginInfo($server)
+    {
+        // Generate unique random number (different than phpsessionid)
+        $this->session['uid'] = sha1(uniqid('', true) . '_' . mt_rand());
+        $this->session['ip'] = client_ip_id($server);
+        $this->session['username'] = $this->conf->get('credentials.login');
+        $this->session['expires_on'] = time() + self::$INACTIVITY_TIMEOUT;
+    }
+
+    /**
+     * Logout a user by unsetting all login information
+     *
+     * See:
+     * - https://secure.php.net/manual/en/function.setcookie.php
+     *
+     * @param string $webPath path on the server in which the cookie will be available on
+     */
+    public function logout($webPath)
+    {
+        if (isset($this->session)) {
+            unset($this->session['uid']);
+            unset($this->session['ip']);
+            unset($this->session['username']);
+            unset($this->session['visibility']);
+            unset($this->session['untaggedonly']);
+        }
+        setcookie(self::$LOGGED_IN_COOKIE, 'false', 0, $webPath);
+    }
 }
diff --git a/index.php b/index.php
index 9cbc9241..34785209 100644
--- a/index.php
+++ b/index.php
@@ -197,11 +197,11 @@ function setup_login_state($conf, $sessionManager)
         $userIsLoggedIn = false;  // Shaarli is not configured yet.
         $loginFailure = true;
     }
-    if (isset($_COOKIE['shaarli_staySignedIn']) &&
-        $_COOKIE['shaarli_staySignedIn']===STAY_SIGNED_IN_TOKEN &&
-        !$loginFailure)
-    {
-        fillSessionInfo($conf, $sessionManager);
+    if (isset($_COOKIE[SessionManager::$LOGGED_IN_COOKIE])
+        && $_COOKIE[SessionManager::$LOGGED_IN_COOKIE] === STAY_SIGNED_IN_TOKEN
+        && !$loginFailure
+    ) {
+        $sessionManager->storeLoginInfo($_SERVER);
         $userIsLoggedIn = true;
     }
     // If session does not exist on server side, or IP address has changed, or session has expired, logout.
@@ -209,7 +209,7 @@ function setup_login_state($conf, $sessionManager)
         || ($conf->get('security.session_protection_disabled') === false && $_SESSION['ip'] != client_ip_id($_SERVER))
         || time() >= $_SESSION['expires_on'])
     {
-        logout();
+        $sessionManager->logout(WEB_PATH);
         $userIsLoggedIn = false;
         $loginFailure = true;
     }
@@ -230,20 +230,6 @@ $userIsLoggedIn = setup_login_state($conf, $sessionManager);
 // ------------------------------------------------------------------------------------------
 // Session management
 
-/**
- * Load user session
- *
- * @param ConfigManager  $conf           Configuration Manager instance.
- * @param SessionManager $sessionManager SessionManager instance
- */
-function fillSessionInfo($conf, $sessionManager)
-{
-    $_SESSION['uid'] = sha1(uniqid('',true).'_'.mt_rand()); // Generate unique random number (different than phpsessionid)
-    $_SESSION['ip'] = client_ip_id($_SERVER);
-    $_SESSION['username']= $conf->get('credentials.login');
-    $_SESSION['expires_on'] = time() + $sessionManager::$INACTIVITY_TIMEOUT;
-}
-
 /**
  * Check that user/password is correct.
  *
@@ -259,7 +245,7 @@ function check_auth($login, $password, $conf, $sessionManager)
     $hash = sha1($password . $login . $conf->get('credentials.salt'));
     if ($login == $conf->get('credentials.login') && $hash == $conf->get('credentials.hash')) {
         // Login/password is correct.
-        fillSessionInfo($conf, $sessionManager);
+        $sessionManager->storeLoginInfo($_SERVER);
         logm($conf->get('resource.log'), $_SERVER['REMOTE_ADDR'], 'Login successful');
         return true;
     }
@@ -274,18 +260,6 @@ function isLoggedIn()
     return $userIsLoggedIn;
 }
 
-// Force logout.
-function logout() {
-    if (isset($_SESSION)) {
-        unset($_SESSION['uid']);
-        unset($_SESSION['ip']);
-        unset($_SESSION['username']);
-        unset($_SESSION['visibility']);
-        unset($_SESSION['untaggedonly']);
-    }
-    setcookie('shaarli_staySignedIn', FALSE, 0, WEB_PATH);
-}
-
 // ------------------------------------------------------------------------------------------
 // Process login form: Check if login/password is correct.
 if (isset($_POST['login'])) {
@@ -303,10 +277,13 @@ if (isset($_POST['login'])) {
         if (!empty($_POST['longlastingsession'])) {
             $_SESSION['longlastingsession'] = 31536000; // (31536000 seconds = 1 year)
             $expiration = time() + $_SESSION['longlastingsession']; // calculate relative cookie expiration (1 year from now)
-            setcookie('shaarli_staySignedIn', STAY_SIGNED_IN_TOKEN, $expiration, WEB_PATH);
+            setcookie($sessionManager::$LOGGED_IN_COOKIE, STAY_SIGNED_IN_TOKEN, $expiration, WEB_PATH);
             $_SESSION['expires_on'] = $expiration;  // Set session expiration on server-side.
 
-            $cookiedir = ''; if(dirname($_SERVER['SCRIPT_NAME'])!='/') $cookiedir=dirname($_SERVER["SCRIPT_NAME"]).'/';
+            $cookiedir = '';
+            if (dirname($_SERVER['SCRIPT_NAME']) != '/') {
+                $cookiedir = dirname($_SERVER["SCRIPT_NAME"]) . '/';
+            }
             session_set_cookie_params($_SESSION['longlastingsession'],$cookiedir,$_SERVER['SERVER_NAME']); // Set session cookie expiration on client side
             // Note: Never forget the trailing slash on the cookie path!
             session_regenerate_id(true);  // Send cookie with new expiration date to browser.
@@ -676,7 +653,7 @@ function renderPage($conf, $pluginManager, $LINKSDB, $history, $sessionManager,
     if (isset($_SERVER['QUERY_STRING']) && startsWith($_SERVER['QUERY_STRING'], 'do=logout'))
     {
         invalidateCaches($conf->get('resource.page_cache'));
-        logout();
+        $sessionManager->logout(WEB_PATH);
         header('Location: ?');
         exit;
     }
-- 
cgit v1.2.3


From 63ea23c2a67d2a1cf6cda79fa2fe49a143571cde Mon Sep 17 00:00:00 2001
From: VirtualTam <virtualtam@flibidi.net>
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 <virtualtam@flibidi.net>
---
 application/LoginManager.php | 109 +++++++++++++++++++++++++++++++-
 index.php                    | 144 ++++++++++++-------------------------------
 tests/LoginManagerTest.php   |   4 +-
 3 files changed, 146 insertions(+), 111 deletions(-)

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;
     }
 
     /**
diff --git a/index.php b/index.php
index 34785209..5e15b9c2 100644
--- a/index.php
+++ b/index.php
@@ -121,8 +121,8 @@ if (isset($_COOKIE['shaarli']) && !SessionManager::checkId($_COOKIE['shaarli']))
 }
 
 $conf = new ConfigManager();
-$loginManager = new LoginManager($GLOBALS, $conf);
 $sessionManager = new SessionManager($_SESSION, $conf);
+$loginManager = new LoginManager($GLOBALS, $conf, $sessionManager);
 
 // LC_MESSAGES isn't defined without php-intl, in this case use LC_COLLATE locale instead.
 if (! defined('LC_MESSAGES')) {
@@ -178,88 +178,20 @@ if (! is_file($conf->getConfigFileExt())) {
 // 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')));
 
-/**
- * Checking session state (i.e. is the user still logged in)
- *
- * @param ConfigManager  $conf           Configuration Manager instance.
- * @param SessionManager $sessionManager SessionManager instance
- *
- * @return bool true if the user is logged in, false otherwise.
- */
-function setup_login_state($conf, $sessionManager)
-{
-    if ($conf->get('security.open_shaarli')) {
-        return true;
-    }
-    $userIsLoggedIn = false; // By default, we do not consider the user as logged in;
-    $loginFailure = false; // If set to true, every attempt to authenticate the user will fail. This indicates that an important condition isn't met.
-    if (! $conf->exists('credentials.login')) {
-        $userIsLoggedIn = false;  // Shaarli is not configured yet.
-        $loginFailure = true;
-    }
-    if (isset($_COOKIE[SessionManager::$LOGGED_IN_COOKIE])
-        && $_COOKIE[SessionManager::$LOGGED_IN_COOKIE] === STAY_SIGNED_IN_TOKEN
-        && !$loginFailure
-    ) {
-        $sessionManager->storeLoginInfo($_SERVER);
-        $userIsLoggedIn = true;
-    }
-    // If session does not exist on server side, or IP address has changed, or session has expired, logout.
-    if (empty($_SESSION['uid'])
-        || ($conf->get('security.session_protection_disabled') === false && $_SESSION['ip'] != client_ip_id($_SERVER))
-        || time() >= $_SESSION['expires_on'])
-    {
-        $sessionManager->logout(WEB_PATH);
-        $userIsLoggedIn = false;
-        $loginFailure = true;
-    }
-    if (!empty($_SESSION['longlastingsession'])) {
-        $_SESSION['expires_on']=time()+$_SESSION['longlastingsession']; // In case of "Stay signed in" checked.
-    } else {
-        $_SESSION['expires_on'] = time() + $sessionManager::$INACTIVITY_TIMEOUT;
-    }
-    if (!$loginFailure) {
-        $userIsLoggedIn = true;
-    }
-
-    return $userIsLoggedIn;
-}
-
-$userIsLoggedIn = setup_login_state($conf, $sessionManager);
-
-// ------------------------------------------------------------------------------------------
-// Session management
+$loginManager->checkLoginState($_SERVER, $_SESSION, $_COOKIE, WEB_PATH, STAY_SIGNED_IN_TOKEN);
 
 /**
- * Check that user/password is correct.
- *
- * @param string         $login          Username
- * @param string         $password       User password
- * @param ConfigManager  $conf           Configuration Manager instance.
- * @param SessionManager $sessionManager SessionManager instance
+ * Adapter function for PageBuilder
  *
- * @return bool: authentication successful or not.
+ * TODO: update PageBuilder and tests
  */
-function check_auth($login, $password, $conf, $sessionManager)
-{
-    $hash = sha1($password . $login . $conf->get('credentials.salt'));
-    if ($login == $conf->get('credentials.login') && $hash == $conf->get('credentials.hash')) {
-        // Login/password is correct.
-        $sessionManager->storeLoginInfo($_SERVER);
-        logm($conf->get('resource.log'), $_SERVER['REMOTE_ADDR'], 'Login successful');
-        return true;
-    }
-    logm($conf->get('resource.log'), $_SERVER['REMOTE_ADDR'], 'Login failed for user '.$login);
-    return false;
-}
-
-// Returns true if the user is logged in.
 function isLoggedIn()
 {
-    global $userIsLoggedIn;
-    return $userIsLoggedIn;
+    global $loginManager;
+    return $loginManager->isLoggedIn();
 }
 
+
 // ------------------------------------------------------------------------------------------
 // Process login form: Check if login/password is correct.
 if (isset($_POST['login'])) {
@@ -268,7 +200,7 @@ if (isset($_POST['login'])) {
     }
     if (isset($_POST['password'])
         && $sessionManager->checkToken($_POST['token'])
-        && (check_auth($_POST['login'], $_POST['password'], $conf, $sessionManager))
+        && $loginManager->checkCredentials($_SERVER, $_POST['login'], $_POST['password'])
     ) {
         // Login/password is OK.
         $loginManager->handleSuccessfulLogin($_SERVER);
@@ -347,15 +279,16 @@ if (!isset($_SESSION['tokens'])) $_SESSION['tokens']=array();  // Token are atta
  * Gives the last 7 days (which have links).
  * This RSS feed cannot be filtered.
  *
- * @param ConfigManager $conf Configuration Manager instance.
+ * @param ConfigManager $conf         Configuration Manager instance
+ * @param LoginManager  $loginManager LoginManager instance
  */
-function showDailyRSS($conf) {
+function showDailyRSS($conf, $loginManager) {
     // Cache system
     $query = $_SERVER['QUERY_STRING'];
     $cache = new CachedPage(
         $conf->get('config.PAGE_CACHE'),
         page_url($_SERVER),
-        startsWith($query,'do=dailyrss') && !isLoggedIn()
+        startsWith($query,'do=dailyrss') && !$loginManager->isLoggedIn()
     );
     $cached = $cache->cachedVersion();
     if (!empty($cached)) {
@@ -367,7 +300,7 @@ function showDailyRSS($conf) {
     // Read links from database (and filter private links if used it not logged in).
     $LINKSDB = new LinkDB(
         $conf->get('resource.datastore'),
-        isLoggedIn(),
+        $loginManager->isLoggedIn(),
         $conf->get('privacy.hide_public_links'),
         $conf->get('redirector.url'),
         $conf->get('redirector.encode_url')
@@ -509,7 +442,7 @@ function showDaily($pageBuilder, $LINKSDB, $conf, $pluginManager)
 
     /* Hook is called before column construction so that plugins don't have
        to deal with columns. */
-    $pluginManager->executeHooks('render_daily', $data, array('loggedin' => isLoggedIn()));
+    $pluginManager->executeHooks('render_daily', $data, array('loggedin' => $loginManager->isLoggedIn()));
 
     /* We need to spread the articles on 3 columns.
        I did not want to use a JavaScript lib like http://masonry.desandro.com/
@@ -553,8 +486,8 @@ function showDaily($pageBuilder, $LINKSDB, $conf, $pluginManager)
  * @param ConfigManager $conf    Configuration Manager instance.
  * @param PluginManager $pluginManager Plugin Manager instance.
  */
-function showLinkList($PAGE, $LINKSDB, $conf, $pluginManager) {
-    buildLinkList($PAGE,$LINKSDB, $conf, $pluginManager); // Compute list of links to display
+function showLinkList($PAGE, $LINKSDB, $conf, $pluginManager, $loginManager) {
+    buildLinkList($PAGE,$LINKSDB, $conf, $pluginManager, $loginManager);
     $PAGE->renderPage('linklist');
 }
 
@@ -574,7 +507,7 @@ function renderPage($conf, $pluginManager, $LINKSDB, $history, $sessionManager,
         read_updates_file($conf->get('resource.updates')),
         $LINKSDB,
         $conf,
-        isLoggedIn()
+        $loginManager->isLoggedIn()
     );
     try {
         $newUpdates = $updater->update();
@@ -596,11 +529,11 @@ function renderPage($conf, $pluginManager, $LINKSDB, $history, $sessionManager,
 
     // Determine which page will be rendered.
     $query = (isset($_SERVER['QUERY_STRING'])) ? $_SERVER['QUERY_STRING'] : '';
-    $targetPage = Router::findPage($query, $_GET, isLoggedIn());
+    $targetPage = Router::findPage($query, $_GET, $loginManager->isLoggedIn());
 
     if (
         // if the user isn't logged in
-        !isLoggedIn() &&
+        !$loginManager->isLoggedIn() &&
         // and Shaarli doesn't have public content...
         $conf->get('privacy.hide_public_links') &&
         // and is configured to enforce the login
@@ -628,7 +561,7 @@ function renderPage($conf, $pluginManager, $LINKSDB, $history, $sessionManager,
         $pluginManager->executeHooks('render_' . $name, $plugin_data,
             array(
                 'target' => $targetPage,
-                'loggedin' => isLoggedIn()
+                'loggedin' => $loginManager->isLoggedIn()
             )
         );
         $PAGE->assign('plugins_' . $name, $plugin_data);
@@ -680,7 +613,7 @@ function renderPage($conf, $pluginManager, $LINKSDB, $history, $sessionManager,
         $data = array(
             'linksToDisplay' => $linksToDisplay,
         );
-        $pluginManager->executeHooks('render_picwall', $data, array('loggedin' => isLoggedIn()));
+        $pluginManager->executeHooks('render_picwall', $data, array('loggedin' => $loginManager->isLoggedIn()));
 
         foreach ($data as $key => $value) {
             $PAGE->assign($key, $value);
@@ -727,7 +660,7 @@ function renderPage($conf, $pluginManager, $LINKSDB, $history, $sessionManager,
             'search_tags' => $searchTags,
             'tags' => $tagList,
         );
-        $pluginManager->executeHooks('render_tagcloud', $data, array('loggedin' => isLoggedIn()));
+        $pluginManager->executeHooks('render_tagcloud', $data, array('loggedin' => $loginManager->isLoggedIn()));
 
         foreach ($data as $key => $value) {
             $PAGE->assign($key, $value);
@@ -760,7 +693,7 @@ function renderPage($conf, $pluginManager, $LINKSDB, $history, $sessionManager,
             'search_tags' => $searchTags,
             'tags' => $tags,
         ];
-        $pluginManager->executeHooks('render_taglist', $data, ['loggedin' => isLoggedIn()]);
+        $pluginManager->executeHooks('render_taglist', $data, ['loggedin' => $loginManager->isLoggedIn()]);
 
         foreach ($data as $key => $value) {
             $PAGE->assign($key, $value);
@@ -787,7 +720,7 @@ function renderPage($conf, $pluginManager, $LINKSDB, $history, $sessionManager,
         $cache = new CachedPage(
             $conf->get('resource.page_cache'),
             page_url($_SERVER),
-            startsWith($query,'do='. $targetPage) && !isLoggedIn()
+            startsWith($query,'do='. $targetPage) && !$loginManager->isLoggedIn()
         );
         $cached = $cache->cachedVersion();
         if (!empty($cached)) {
@@ -796,15 +729,15 @@ function renderPage($conf, $pluginManager, $LINKSDB, $history, $sessionManager,
         }
 
         // Generate data.
-        $feedGenerator = new FeedBuilder($LINKSDB, $feedType, $_SERVER, $_GET, isLoggedIn());
+        $feedGenerator = new FeedBuilder($LINKSDB, $feedType, $_SERVER, $_GET, $loginManager->isLoggedIn());
         $feedGenerator->setLocale(strtolower(setlocale(LC_COLLATE, 0)));
-        $feedGenerator->setHideDates($conf->get('privacy.hide_timestamps') && !isLoggedIn());
+        $feedGenerator->setHideDates($conf->get('privacy.hide_timestamps') && !$loginManager->isLoggedIn());
         $feedGenerator->setUsePermalinks(isset($_GET['permalinks']) || !$conf->get('feed.rss_permalinks'));
         $data = $feedGenerator->buildData();
 
         // Process plugin hook.
         $pluginManager->executeHooks('render_feed', $data, array(
-            'loggedin' => isLoggedIn(),
+            'loggedin' => $loginManager->isLoggedIn(),
             'target' => $targetPage,
         ));
 
@@ -952,7 +885,7 @@ function renderPage($conf, $pluginManager, $LINKSDB, $history, $sessionManager,
     }
 
     // -------- Handle other actions allowed for non-logged in users:
-    if (!isLoggedIn())
+    if (!$loginManager->isLoggedIn())
     {
         // User tries to post new link but is not logged in:
         // Show login screen, then redirect to ?post=...
@@ -968,7 +901,7 @@ function renderPage($conf, $pluginManager, $LINKSDB, $history, $sessionManager,
             exit;
         }
 
-        showLinkList($PAGE, $LINKSDB, $conf, $pluginManager);
+        showLinkList($PAGE, $LINKSDB, $conf, $pluginManager, $loginManager);
         if (isset($_GET['edit_link'])) {
             header('Location: ?do=login&edit_link='. escape($_GET['edit_link']));
             exit;
@@ -1019,7 +952,7 @@ function renderPage($conf, $pluginManager, $LINKSDB, $history, $sessionManager,
             $conf->set('credentials.salt', sha1(uniqid('', true) .'_'. mt_rand()));
             $conf->set('credentials.hash', sha1($_POST['setpassword'] . $conf->get('credentials.login') . $conf->get('credentials.salt')));
             try {
-                $conf->write(isLoggedIn());
+                $conf->write($loginManager->isLoggedIn());
             }
             catch(Exception $e) {
                 error_log(
@@ -1070,7 +1003,7 @@ function renderPage($conf, $pluginManager, $LINKSDB, $history, $sessionManager,
             $conf->set('translation.language', escape($_POST['language']));
 
             try {
-                $conf->write(isLoggedIn());
+                $conf->write($loginManager->isLoggedIn());
                 $history->updateSettings();
                 invalidateCaches($conf->get('resource.page_cache'));
             }
@@ -1522,7 +1455,7 @@ function renderPage($conf, $pluginManager, $LINKSDB, $history, $sessionManager,
             else {
                 $conf->set('general.enabled_plugins', save_plugin_config($_POST));
             }
-            $conf->write(isLoggedIn());
+            $conf->write($loginManager->isLoggedIn());
             $history->updateSettings();
         }
         catch (Exception $e) {
@@ -1547,7 +1480,7 @@ function renderPage($conf, $pluginManager, $LINKSDB, $history, $sessionManager,
     }
 
     // -------- Otherwise, simply display search form and links:
-    showLinkList($PAGE, $LINKSDB, $conf, $pluginManager);
+    showLinkList($PAGE, $LINKSDB, $conf, $pluginManager, $loginManager);
     exit;
 }
 
@@ -1559,8 +1492,9 @@ function renderPage($conf, $pluginManager, $LINKSDB, $history, $sessionManager,
  * @param LinkDB        $LINKSDB       LinkDB instance.
  * @param ConfigManager $conf          Configuration Manager instance.
  * @param PluginManager $pluginManager Plugin Manager instance.
+ * @param LoginManager  $loginManager  LoginManager instance
  */
-function buildLinkList($PAGE,$LINKSDB, $conf, $pluginManager)
+function buildLinkList($PAGE, $LINKSDB, $conf, $pluginManager, $loginManager)
 {
     // Used in templates
     if (isset($_GET['searchtags'])) {
@@ -1599,8 +1533,6 @@ function buildLinkList($PAGE,$LINKSDB, $conf, $pluginManager)
         $keys[] = $key;
     }
 
-
-
     // Select articles according to paging.
     $pagecount = ceil(count($keys) / $_SESSION['LINKS_PER_PAGE']);
     $pagecount = $pagecount == 0 ? 1 : $pagecount;
@@ -1681,7 +1613,7 @@ function buildLinkList($PAGE,$LINKSDB, $conf, $pluginManager)
         $data['pagetitle'] .= '- '. $conf->get('general.title');
     }
 
-    $pluginManager->executeHooks('render_linklist', $data, array('loggedin' => isLoggedIn()));
+    $pluginManager->executeHooks('render_linklist', $data, array('loggedin' => $loginManager->isLoggedIn()));
 
     foreach ($data as $key => $value) {
         $PAGE->assign($key, $value);
@@ -1952,7 +1884,7 @@ function install($conf, $sessionManager) {
         );
         try {
             // Everything is ok, let's create config file.
-            $conf->write(isLoggedIn());
+            $conf->write($loginManager->isLoggedIn());
         }
         catch(Exception $e) {
             error_log(
@@ -2216,7 +2148,7 @@ try {
 
 $linkDb = new LinkDB(
     $conf->get('resource.datastore'),
-    isLoggedIn(),
+    $loginManager->isLoggedIn(),
     $conf->get('privacy.hide_public_links'),
     $conf->get('redirector.url'),
     $conf->get('redirector.encode_url')
diff --git a/tests/LoginManagerTest.php b/tests/LoginManagerTest.php
index 4159038e..27ca0db5 100644
--- a/tests/LoginManagerTest.php
+++ b/tests/LoginManagerTest.php
@@ -38,7 +38,7 @@ class LoginManagerTest extends TestCase
         $this->globals = &$GLOBALS;
         unset($this->globals['IPBANS']);
 
-        $this->loginManager = new LoginManager($this->globals, $this->configManager);
+        $this->loginManager = new LoginManager($this->globals, $this->configManager, null);
         $this->server['REMOTE_ADDR'] = $this->ipAddr;
     }
 
@@ -59,7 +59,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']);
     }
 
-- 
cgit v1.2.3


From 1b28c66cc77b59f716aa47e6207142a7f86c2c2d Mon Sep 17 00:00:00 2001
From: VirtualTam <virtualtam@flibidi.net>
Date: Wed, 4 Apr 2018 00:43:48 +0200
Subject: Document LoginManager properties

Signed-off-by: VirtualTam <virtualtam@flibidi.net>
---
 application/LoginManager.php | 11 +++++++++++
 1 file changed, 11 insertions(+)

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 <virtualtam@flibidi.net>
Date: Wed, 4 Apr 2018 00:54:59 +0200
Subject: Delegate session operations to SessionManager

Signed-off-by: VirtualTam <virtualtam@flibidi.net>
---
 application/LoginManager.php   | 27 +++++++----------
 application/SessionManager.php | 66 +++++++++++++++++++++++++++++++++++++-----
 2 files changed, 69 insertions(+), 24 deletions(-)

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 @@
 <?php
 namespace Shaarli;
 
+use Shaarli\Config\ConfigManager;
+
 /**
  * User login management
  */
@@ -62,34 +64,24 @@ class LoginManager
             return;
         }
 
+        $clientIpId = client_ip_id($server);
+
         if (isset($cookie[SessionManager::$LOGGED_IN_COOKIE])
             && $cookie[SessionManager::$LOGGED_IN_COOKIE] === $token
         ) {
-            $this->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'],
diff --git a/application/SessionManager.php b/application/SessionManager.php
index 7bfd2220..63eeb8aa 100644
--- a/application/SessionManager.php
+++ b/application/SessionManager.php
@@ -1,21 +1,23 @@
 <?php
 namespace Shaarli;
 
+use Shaarli\Config\ConfigManager;
+
 /**
  * Manages the server-side session
  */
 class SessionManager
 {
-    /** Session expiration timeout, in seconds */
+    /** @var int Session expiration timeout, in seconds */
     public static $INACTIVITY_TIMEOUT = 3600;
 
-    /** Name of the cookie set after logging in **/
+    /** @var string Name of the cookie set after logging in **/
     public static $LOGGED_IN_COOKIE = 'shaarli_staySignedIn';
 
-    /** Local reference to the global $_SESSION array */
+    /** @var array Local reference to the global $_SESSION array */
     protected $session = [];
 
-    /** ConfigManager instance **/
+    /** @var ConfigManager Configuration Manager instance **/
     protected $conf = null;
 
     /**
@@ -94,17 +96,30 @@ class SessionManager
     /**
      * Store user login information after a successful login
      *
-     * @param array $server The global $_SERVER array
+     * @param string $clientIpId Client IP address identifier
      */
-    public function storeLoginInfo($server)
+    public function storeLoginInfo($clientIpId)
     {
         // Generate unique random number (different than phpsessionid)
         $this->session['uid'] = sha1(uniqid('', true) . '_' . mt_rand());
-        $this->session['ip'] = client_ip_id($server);
+        $this->session['ip'] = $clientIpId;
         $this->session['username'] = $this->conf->get('credentials.login');
         $this->session['expires_on'] = time() + self::$INACTIVITY_TIMEOUT;
     }
 
+    /**
+     * Extend session validity
+     */
+    public function extendSession()
+    {
+        if (! empty($this->session['longlastingsession'])) {
+            // "Stay signed in" is enabled
+            $this->session['expires_on'] = time() + $this->session['longlastingsession'];
+            return;
+        }
+        $this->session['expires_on'] = time() + self::$INACTIVITY_TIMEOUT;
+    }
+
     /**
      * Logout a user by unsetting all login information
      *
@@ -124,4 +139,41 @@ class SessionManager
         }
         setcookie(self::$LOGGED_IN_COOKIE, 'false', 0, $webPath);
     }
+
+    /**
+     * Check whether the session has expired
+     *
+     * @param string $clientIpId Client IP address identifier
+     *
+     * @return bool true if the session has expired, false otherwise
+     */
+    public function hasSessionExpired()
+    {
+        if (empty($this->session['uid'])) {
+            return true;
+        }
+        if (time() >= $this->session['expires_on']) {
+            return true;
+        }
+        return false;
+    }
+
+    /**
+     * Check whether the client IP address has changed
+     *
+     * @param string $clientIpId Client IP address identifier
+     *
+     * @return bool true if the IP has changed, false if it has not, or
+     *              if session protection has been disabled
+     */
+    public function hasClientIpChanged($clientIpId)
+    {
+        if ($this->conf->get('security.session_protection_disabled') === true) {
+            return false;
+        }
+        if ($this->session['ip'] == $clientIpId) {
+            return false;
+        }
+        return true;
+    }
 }
-- 
cgit v1.2.3


From 847420847455c1339f3302b1b67568ee0f382a11 Mon Sep 17 00:00:00 2001
From: VirtualTam <virtualtam@flibidi.net>
Date: Wed, 18 Apr 2018 23:09:45 +0200
Subject: Pass the client IP ID to LoginManager

Signed-off-by: VirtualTam <virtualtam@flibidi.net>
---
 application/LoginManager.php | 28 +++++++++++++---------------
 index.php                    |  5 +++--
 2 files changed, 16 insertions(+), 17 deletions(-)

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;
diff --git a/index.php b/index.php
index 5e15b9c2..04b0e4ba 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);
+$clientIpId = client_ip_id($_SERVER);
 
 // LC_MESSAGES isn't defined without php-intl, in this case use LC_COLLATE locale instead.
 if (! defined('LC_MESSAGES')) {
@@ -178,7 +179,7 @@ if (! is_file($conf->getConfigFileExt())) {
 // 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($_SERVER, $_SESSION, $_COOKIE, WEB_PATH, STAY_SIGNED_IN_TOKEN);
+$loginManager->checkLoginState($_SESSION, $_COOKIE, WEB_PATH, $clientIpId, STAY_SIGNED_IN_TOKEN);
 
 /**
  * Adapter function for PageBuilder
@@ -200,7 +201,7 @@ if (isset($_POST['login'])) {
     }
     if (isset($_POST['password'])
         && $sessionManager->checkToken($_POST['token'])
-        && $loginManager->checkCredentials($_SERVER, $_POST['login'], $_POST['password'])
+        && $loginManager->checkCredentials($_SERVER['REMOTE_ADDR'], $clientIpId, $_POST['login'], $_POST['password'])
     ) {
         // Login/password is OK.
         $loginManager->handleSuccessfulLogin($_SERVER);
-- 
cgit v1.2.3


From 89ccc83ba497fa91eaaf2a2a52d8549aea5e2aee Mon Sep 17 00:00:00 2001
From: VirtualTam <virtualtam@flibidi.net>
Date: Wed, 18 Apr 2018 23:45:05 +0200
Subject: Login: update PageBuilder and default/vintage templates

Signed-off-by: VirtualTam <virtualtam@flibidi.net>
---
 application/PageBuilder.php      |  9 +++++++--
 index.php                        | 15 +++++++++------
 tpl/default/linklist.html        |  8 ++++----
 tpl/default/linklist.paging.html |  4 ++--
 tpl/default/page.footer.html     |  2 +-
 tpl/default/page.header.html     | 10 +++++-----
 tpl/default/tag.list.html        |  6 +++---
 tpl/vintage/daily.html           |  2 +-
 tpl/vintage/linklist.html        |  4 ++--
 tpl/vintage/linklist.paging.html |  2 +-
 tpl/vintage/page.footer.html     |  2 +-
 tpl/vintage/page.header.html     |  4 ++--
 12 files changed, 38 insertions(+), 30 deletions(-)

diff --git a/application/PageBuilder.php b/application/PageBuilder.php
index 3233d6b6..a4483870 100644
--- a/application/PageBuilder.php
+++ b/application/PageBuilder.php
@@ -25,6 +25,9 @@ class PageBuilder
      * @var LinkDB $linkDB instance.
      */
     protected $linkDB;
+    
+    /** @var bool $isLoggedIn Whether the user is logged in **/
+    protected $isLoggedIn = false;
 
     /**
      * PageBuilder constructor.
@@ -34,12 +37,13 @@ class PageBuilder
      * @param LinkDB        $linkDB instance.
      * @param string        $token  Session token
      */
-    public function __construct(&$conf, $linkDB = null, $token = null)
+    public function __construct(&$conf, $linkDB = null, $token = null, $isLoggedIn = false)
     {
         $this->tpl = false;
         $this->conf = $conf;
         $this->linkDB = $linkDB;
         $this->token = $token;
+        $this->isLoggedIn = $isLoggedIn;
     }
 
     /**
@@ -55,7 +59,7 @@ class PageBuilder
                 $this->conf->get('resource.update_check'),
                 $this->conf->get('updates.check_updates_interval'),
                 $this->conf->get('updates.check_updates'),
-                isLoggedIn(),
+                $this->isLoggedIn,
                 $this->conf->get('updates.check_updates_branch')
             );
             $this->tpl->assign('newVersion', escape($version));
@@ -67,6 +71,7 @@ class PageBuilder
             $this->tpl->assign('versionError', escape($exc->getMessage()));
         }
 
+        $this->tpl->assign('is_logged_in', $this->isLoggedIn);
         $this->tpl->assign('feedurl', escape(index_url($_SERVER)));
         $searchcrits = ''; // Search criteria
         if (!empty($_GET['searchtags'])) {
diff --git a/index.php b/index.php
index 04b0e4ba..c87ebf65 100644
--- a/index.php
+++ b/index.php
@@ -182,9 +182,11 @@ define('STAY_SIGNED_IN_TOKEN', sha1($conf->get('credentials.hash') . $_SERVER['R
 $loginManager->checkLoginState($_SESSION, $_COOKIE, WEB_PATH, $clientIpId, STAY_SIGNED_IN_TOKEN);
 
 /**
- * Adapter function for PageBuilder
+ * Adapter function to ensure compatibility with third-party templates
  *
- * TODO: update PageBuilder and tests
+ * @see https://github.com/shaarli/Shaarli/pull/1086
+ *
+ * @return bool true when the user is logged in, false otherwise
  */
 function isLoggedIn()
 {
@@ -383,9 +385,10 @@ function showDailyRSS($conf, $loginManager) {
  * @param PageBuilder   $pageBuilder   Template engine wrapper.
  * @param LinkDB        $LINKSDB       LinkDB instance.
  * @param ConfigManager $conf          Configuration Manager instance.
- * @param PluginManager $pluginManager Plugin Manager instane.
+ * @param PluginManager $pluginManager Plugin Manager instance.
+ * @param LoginManager  $loginManager  Login Manager instance
  */
-function showDaily($pageBuilder, $LINKSDB, $conf, $pluginManager)
+function showDaily($pageBuilder, $LINKSDB, $conf, $pluginManager, $loginManager)
 {
     $day = date('Ymd', strtotime('-1 day')); // Yesterday, in format YYYYMMDD.
     if (isset($_GET['day'])) {
@@ -523,7 +526,7 @@ function renderPage($conf, $pluginManager, $LINKSDB, $history, $sessionManager,
         die($e->getMessage());
     }
 
-    $PAGE = new PageBuilder($conf, $LINKSDB, $sessionManager->generateToken());
+    $PAGE = new PageBuilder($conf, $LINKSDB, $sessionManager->generateToken(), $loginManager->isLoggedIn());
     $PAGE->assign('linkcount', count($LINKSDB));
     $PAGE->assign('privateLinkcount', count_private($LINKSDB));
     $PAGE->assign('plugin_errors', $pluginManager->getErrors());
@@ -708,7 +711,7 @@ function renderPage($conf, $pluginManager, $LINKSDB, $history, $sessionManager,
 
     // Daily page.
     if ($targetPage == Router::$PAGE_DAILY) {
-        showDaily($PAGE, $LINKSDB, $conf, $pluginManager);
+        showDaily($PAGE, $LINKSDB, $conf, $pluginManager, $loginManager);
     }
 
     // ATOM and RSS feed.
diff --git a/tpl/default/linklist.html b/tpl/default/linklist.html
index d546be0a..322cddd5 100644
--- a/tpl/default/linklist.html
+++ b/tpl/default/linklist.html
@@ -136,7 +136,7 @@
               <div class="linklist-item-thumbnail">{$thumb}</div>
             {/if}
 
-            {if="isLoggedIn()"}
+            {if="$is_logged_in"}
               <div class="linklist-item-editbuttons">
                 {if="$value.private"}
                   <span class="label label-private">{$strPrivate}</span>
@@ -179,7 +179,7 @@
 
             <div class="linklist-item-infos-date-url-block pure-g">
               <div class="linklist-item-infos-dateblock pure-u-lg-7-12 pure-u-1">
-                {if="isLoggedIn()"}
+                {if="$is_logged_in"}
                   <div class="linklist-item-infos-controls-group pure-u-0 pure-u-lg-visible">
                     <span class="linklist-item-infos-controls-item ctrl-checkbox">
                       <input type="checkbox" class="delete-checkbox" value="{$value.id}">
@@ -196,7 +196,7 @@
                   </div>
                 {/if}
                 <a href="?{$value.shorturl}" title="{$strPermalink}">
-                  {if="!$hide_timestamps || isLoggedIn()"}
+                  {if="!$hide_timestamps || $is_logged_in"}
                     {$updated=$value.updated_timestamp ? $strEdited. format_date($value.updated) : $strPermalink}
                     <span class="linkdate" title="{$updated}">
                       <i class="fa fa-clock-o"></i>
@@ -236,7 +236,7 @@
                     {if="$link_plugin_counter - 1 != $counter"}&middot;{/if}
                   {/loop}
                 {/if}
-                {if="isLoggedIn()"}
+                {if="$is_logged_in"}
                   &middot;
                   <a href="?delete_link&amp;lf_linkdate={$value.id}&amp;token={$token}"
                      title="{$strDelete}" class="delete-link confirm-delete">
diff --git a/tpl/default/linklist.paging.html b/tpl/default/linklist.paging.html
index 72bdd931..5309e348 100644
--- a/tpl/default/linklist.paging.html
+++ b/tpl/default/linklist.paging.html
@@ -1,11 +1,11 @@
 <div class="linklist-paging">
   <div class="paging pure-g">
     <div class="linklist-filters pure-u-1-3">
-      {if="isLoggedIn() or !empty($action_plugin)"}
+      {if="$is_logged_in or !empty($action_plugin)"}
         <span class="linklist-filters-text pure-u-0 pure-u-lg-visible">
           {'Filters'|t}
         </span>
-        {if="isLoggedIn()"}
+        {if="$is_logged_in"}
         <a href="?visibility=private" title="{'Only display private links'|t}"
            class="{if="$visibility==='private'"}filter-on{else}filter-off{/if}"
         ><i class="fa fa-user-secret"></i></a>
diff --git a/tpl/default/page.footer.html b/tpl/default/page.footer.html
index 34193743..5af39be7 100644
--- a/tpl/default/page.footer.html
+++ b/tpl/default/page.footer.html
@@ -4,7 +4,7 @@
   <div class="pure-u-2-24"></div>
   <div id="footer" class="pure-u-20-24 footer-container">
     <strong><a href="https://github.com/shaarli/Shaarli">Shaarli</a></strong>
-    {if="isLoggedIn()===true"}
+    {if="$is_logged_in===true"}
       {$version}
     {/if}
     &middot;
diff --git a/tpl/default/page.header.html b/tpl/default/page.header.html
index 18aa77c8..82568d63 100644
--- a/tpl/default/page.header.html
+++ b/tpl/default/page.header.html
@@ -17,7 +17,7 @@
             {$shaarlititle}
           </a>
         </li>
-        {if="isLoggedIn() || $openshaarli"}
+        {if="$is_logged_in || $openshaarli"}
           <li class="pure-menu-item">
             <a href="?do=addlink" class="pure-menu-link" id="shaarli-menu-shaare">
               <i class="fa fa-plus" ></i> {'Shaare'|t}
@@ -50,7 +50,7 @@
         <li class="pure-menu-item pure-u-lg-0 shaarli-menu-mobile" id="shaarli-menu-mobile-rss">
             <a href="?do={$feed_type}{$searchcrits}" class="pure-menu-link">{'RSS Feed'|t}</a>
         </li>
-        {if="isLoggedIn()"}
+        {if="$is_logged_in"}
           <li class="pure-menu-item pure-u-lg-0 shaarli-menu-mobile" id="shaarli-menu-mobile-logout">
             <a href="?do=logout" class="pure-menu-link">{'Logout'|t}</a>
           </li>
@@ -74,7 +74,7 @@
               <i class="fa fa-rss"></i>
             </a>
           </li>
-          {if="!isLoggedIn()"}
+          {if="!$is_logged_in"}
             <li class="pure-menu-item" id="shaarli-menu-desktop-login">
               <a href="?do=login" class="pure-menu-link"
                  data-open-id="header-login-form"
@@ -120,7 +120,7 @@
       </div>
     </div>
   </div>
-  {if="!isLoggedIn()"}
+  {if="!$is_logged_in"}
     <form method="post" name="loginform">
       <div class="subheader-form header-login-form" id="header-login-form">
         <input type="text" name="login" placeholder="{'Username'|t}" tabindex="3">
@@ -155,7 +155,7 @@
   </div>
 {/if}
 
-{if="!empty($plugin_errors) && isLoggedIn()"}
+{if="!empty($plugin_errors) && $is_logged_in"}
   <div class="pure-g new-version-message pure-alert pure-alert-error pure-alert-closable" id="shaarli-errors-alert">
     <div class="pure-u-2-24"></div>
     <div class="pure-u-20-24">
diff --git a/tpl/default/tag.list.html b/tpl/default/tag.list.html
index 772d6ad3..bcddcd56 100644
--- a/tpl/default/tag.list.html
+++ b/tpl/default/tag.list.html
@@ -49,7 +49,7 @@
       {loop="tags"}
         <div class="tag-list-item pure-g" data-tag="{$key}">
           <div class="pure-u-1">
-            {if="isLoggedIn()===true"}
+            {if="$is_logged_in===true"}
               <a href="#" class="delete-tag"><i class="fa fa-trash"></i></a>&nbsp;&nbsp;
               <a href="?do=changetag&fromtag={$key|urlencode}" class="rename-tag">
                 <i class="fa fa-pencil-square-o {$key}"></i>
@@ -63,7 +63,7 @@
               {$value}
             {/loop}
           </div>
-          {if="isLoggedIn()===true"}
+          {if="$is_logged_in===true"}
             <div class="rename-tag-form pure-u-1">
               <input type="text" name="{$key}" value="{$key}" class="rename-tag-input" />
               <a href="#" class="validate-rename-tag"><i class="fa fa-check"></i></a>
@@ -81,7 +81,7 @@
   </div>
 </div>
 
-{if="isLoggedIn()===true"}
+{if="$is_logged_in===true"}
   <input type="hidden" name="taglist" value="{loop="$tags"}{$key} {/loop}"
 {/if}
 
diff --git a/tpl/vintage/daily.html b/tpl/vintage/daily.html
index 42db16a7..ede35910 100644
--- a/tpl/vintage/daily.html
+++ b/tpl/vintage/daily.html
@@ -53,7 +53,7 @@
                                 <img src="img/squiggle.png" width="25" height="26" title="permalink" alt="permalink">
                             </a>
                         </div>
-                        {if="!$hide_timestamps || isLoggedIn()"}
+                        {if="!$hide_timestamps || $is_logged_in"}
                             <div class="dailyEntryLinkdate">
                                 <a href="?{$value.shorturl}">{function="strftime('%c', $link.timestamp)"}</a>
                             </div>
diff --git a/tpl/vintage/linklist.html b/tpl/vintage/linklist.html
index e7137246..1ca51be3 100644
--- a/tpl/vintage/linklist.html
+++ b/tpl/vintage/linklist.html
@@ -82,7 +82,7 @@
             <a id="{$value.shorturl}"></a>
             <div class="thumbnail">{$value.url|thumbnail}</div>
             <div class="linkcontainer">
-                {if="isLoggedIn()"}
+                {if="$is_logged_in"}
                     <div class="linkeditbuttons">
                         <form method="GET" class="buttoneditform">
                             <input type="hidden" name="edit_link" value="{$value.id}">
@@ -102,7 +102,7 @@
                 </span>
                 <br>
                 {if="$value.description"}<div class="linkdescription">{$value.description}</div>{/if}
-                {if="!$hide_timestamps || isLoggedIn()"}
+                {if="!$hide_timestamps || $is_logged_in"}
                     {$updated=$value.updated_timestamp ? 'Edited: '. format_date($value.updated) : 'Permalink'}
                     <span class="linkdate" title="Permalink">
                         <a href="?{$value.shorturl}">
diff --git a/tpl/vintage/linklist.paging.html b/tpl/vintage/linklist.paging.html
index e3b88ee6..35149a6b 100644
--- a/tpl/vintage/linklist.paging.html
+++ b/tpl/vintage/linklist.paging.html
@@ -1,5 +1,5 @@
 <div class="paging">
-{if="isLoggedIn()"}
+{if="$is_logged_in"}
     <div class="paging_privatelinks">
       <a href="?visibility=private">
 		{if="$visibility=='private'"}
diff --git a/tpl/vintage/page.footer.html b/tpl/vintage/page.footer.html
index 1485e1ce..f409721e 100644
--- a/tpl/vintage/page.footer.html
+++ b/tpl/vintage/page.footer.html
@@ -25,7 +25,7 @@
 
 <script src="js/shaarli.min.js"></script>
 
-{if="isLoggedIn()"}
+{if="$is_logged_in"}
 <script>function confirmDeleteLink() { var agree=confirm("Are you sure you want to delete this link ?"); if (agree) return true ; else return false ; }</script>
 {/if}
 
diff --git a/tpl/vintage/page.header.html b/tpl/vintage/page.header.html
index 8a58844e..40c53e5b 100644
--- a/tpl/vintage/page.header.html
+++ b/tpl/vintage/page.header.html
@@ -17,7 +17,7 @@
     {ignore} When called as a popup from bookmarklet, do not display menu. {/ignore}
 {else}
 <li><a href="{$titleLink}" class="nomobile">Home</a></li>
-    {if="isLoggedIn()"}
+    {if="$is_logged_in"}
     <li><a href="?do=logout">Logout</a></li>
     <li><a href="?do=tools">Tools</a></li>
     <li><a href="?do=addlink">Add link</a></li>
@@ -46,7 +46,7 @@
   </ul>
 </div>
 
-{if="!empty($plugin_errors) && isLoggedIn()"}
+{if="!empty($plugin_errors) && $is_logged_in"}
     <ul class="errors">
         {loop="$plugin_errors"}
             <li>{$value}</li>
-- 
cgit v1.2.3


From 68dcaccfa46649addc66674b627a83064798bbc0 Mon Sep 17 00:00:00 2001
From: VirtualTam <virtualtam@flibidi.net>
Date: Fri, 27 Apr 2018 22:00:35 +0200
Subject: LoginManager: remove unused parameter

Signed-off-by: VirtualTam <virtualtam@flibidi.net>
---
 application/LoginManager.php | 3 +--
 index.php                    | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

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
diff --git a/index.php b/index.php
index c87ebf65..30bfc170 100644
--- a/index.php
+++ b/index.php
@@ -179,7 +179,7 @@ if (! is_file($conf->getConfigFileExt())) {
 // 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($_SESSION, $_COOKIE, WEB_PATH, $clientIpId, STAY_SIGNED_IN_TOKEN);
+$loginManager->checkLoginState($_COOKIE, WEB_PATH, $clientIpId, STAY_SIGNED_IN_TOKEN);
 
 /**
  * Adapter function to ensure compatibility with third-party templates
-- 
cgit v1.2.3


From fab87c2696b9d6a26310f1bfc024b018ca5184fe Mon Sep 17 00:00:00 2001
From: VirtualTam <virtualtam@flibidi.net>
Date: Fri, 27 Apr 2018 22:12:22 +0200
Subject: Move LoginManager and SessionManager to the Security namespace

Signed-off-by: VirtualTam <virtualtam@flibidi.net>
---
 application/LoginManager.php            | 238 --------------------------------
 application/SessionManager.php          | 179 ------------------------
 application/security/LoginManager.php   | 238 ++++++++++++++++++++++++++++++++
 application/security/SessionManager.php | 179 ++++++++++++++++++++++++
 composer.json                           |   3 +-
 index.php                               |   4 +-
 tests/LoginManagerTest.php              | 199 --------------------------
 tests/SessionManagerTest.php            | 149 --------------------
 tests/security/LoginManagerTest.php     | 199 ++++++++++++++++++++++++++
 tests/security/SessionManagerTest.php   | 149 ++++++++++++++++++++
 10 files changed, 769 insertions(+), 768 deletions(-)
 delete mode 100644 application/LoginManager.php
 delete mode 100644 application/SessionManager.php
 create mode 100644 application/security/LoginManager.php
 create mode 100644 application/security/SessionManager.php
 delete mode 100644 tests/LoginManagerTest.php
 delete mode 100644 tests/SessionManagerTest.php
 create mode 100644 tests/security/LoginManagerTest.php
 create mode 100644 tests/security/SessionManagerTest.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 @@
-<?php
-namespace Shaarli;
-
-use Shaarli\Config\ConfigManager;
-
-/**
- * User login management
- */
-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;
-
-    /**
-     * Constructor
-     *
-     * @param array          $globals        The $GLOBALS array (reference)
-     * @param ConfigManager  $configManager  Configuration Manager instance
-     * @param SessionManager $sessionManager SessionManager instance
-     */
-    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  $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,
-            "<?php\n\$GLOBALS['IPBANS']=" . var_export($this->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;
-    }
-}
diff --git a/application/SessionManager.php b/application/SessionManager.php
deleted file mode 100644
index 63eeb8aa..00000000
--- a/application/SessionManager.php
+++ /dev/null
@@ -1,179 +0,0 @@
-<?php
-namespace Shaarli;
-
-use Shaarli\Config\ConfigManager;
-
-/**
- * Manages the server-side session
- */
-class SessionManager
-{
-    /** @var int Session expiration timeout, in seconds */
-    public static $INACTIVITY_TIMEOUT = 3600;
-
-    /** @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 = [];
-
-    /** @var ConfigManager Configuration Manager instance **/
-    protected $conf = null;
-
-    /**
-     * Constructor
-     *
-     * @param array         $session The $_SESSION array (reference)
-     * @param ConfigManager $conf    ConfigManager instance
-     */
-    public function __construct(& $session, $conf)
-    {
-        $this->session = &$session;
-        $this->conf = $conf;
-    }
-
-    /**
-     * Generates a session token
-     *
-     * @return string token
-     */
-    public function generateToken()
-    {
-        $token = sha1(uniqid('', true) .'_'. mt_rand() . $this->conf->get('credentials.salt'));
-        $this->session['tokens'][$token] = 1;
-        return $token;
-    }
-
-    /**
-     * Checks the validity of a session token, and destroys it afterwards
-     *
-     * @param string $token The token to check
-     *
-     * @return bool true if the token is valid, else false
-     */
-    public function checkToken($token)
-    {
-        if (! isset($this->session['tokens'][$token])) {
-            // the token is wrong, or has already been used
-            return false;
-        }
-
-        // destroy the token to prevent future use
-        unset($this->session['tokens'][$token]);
-        return true;
-    }
-
-    /**
-     * Validate session ID to prevent Full Path Disclosure.
-     *
-     * See #298.
-     * The session ID's format depends on the hash algorithm set in PHP settings
-     *
-     * @param string $sessionId Session ID
-     *
-     * @return true if valid, false otherwise.
-     *
-     * @see http://php.net/manual/en/function.hash-algos.php
-     * @see http://php.net/manual/en/session.configuration.php
-     */
-    public static function checkId($sessionId)
-    {
-        if (empty($sessionId)) {
-            return false;
-        }
-
-        if (!$sessionId) {
-            return false;
-        }
-
-        if (!preg_match('/^[a-zA-Z0-9,-]{2,128}$/', $sessionId)) {
-            return false;
-        }
-
-        return true;
-    }
-
-    /**
-     * Store user login information after a successful login
-     *
-     * @param string $clientIpId Client IP address identifier
-     */
-    public function storeLoginInfo($clientIpId)
-    {
-        // Generate unique random number (different than phpsessionid)
-        $this->session['uid'] = sha1(uniqid('', true) . '_' . mt_rand());
-        $this->session['ip'] = $clientIpId;
-        $this->session['username'] = $this->conf->get('credentials.login');
-        $this->session['expires_on'] = time() + self::$INACTIVITY_TIMEOUT;
-    }
-
-    /**
-     * Extend session validity
-     */
-    public function extendSession()
-    {
-        if (! empty($this->session['longlastingsession'])) {
-            // "Stay signed in" is enabled
-            $this->session['expires_on'] = time() + $this->session['longlastingsession'];
-            return;
-        }
-        $this->session['expires_on'] = time() + self::$INACTIVITY_TIMEOUT;
-    }
-
-    /**
-     * Logout a user by unsetting all login information
-     *
-     * See:
-     * - https://secure.php.net/manual/en/function.setcookie.php
-     *
-     * @param string $webPath path on the server in which the cookie will be available on
-     */
-    public function logout($webPath)
-    {
-        if (isset($this->session)) {
-            unset($this->session['uid']);
-            unset($this->session['ip']);
-            unset($this->session['username']);
-            unset($this->session['visibility']);
-            unset($this->session['untaggedonly']);
-        }
-        setcookie(self::$LOGGED_IN_COOKIE, 'false', 0, $webPath);
-    }
-
-    /**
-     * Check whether the session has expired
-     *
-     * @param string $clientIpId Client IP address identifier
-     *
-     * @return bool true if the session has expired, false otherwise
-     */
-    public function hasSessionExpired()
-    {
-        if (empty($this->session['uid'])) {
-            return true;
-        }
-        if (time() >= $this->session['expires_on']) {
-            return true;
-        }
-        return false;
-    }
-
-    /**
-     * Check whether the client IP address has changed
-     *
-     * @param string $clientIpId Client IP address identifier
-     *
-     * @return bool true if the IP has changed, false if it has not, or
-     *              if session protection has been disabled
-     */
-    public function hasClientIpChanged($clientIpId)
-    {
-        if ($this->conf->get('security.session_protection_disabled') === true) {
-            return false;
-        }
-        if ($this->session['ip'] == $clientIpId) {
-            return false;
-        }
-        return true;
-    }
-}
diff --git a/application/security/LoginManager.php b/application/security/LoginManager.php
new file mode 100644
index 00000000..e7b9b21e
--- /dev/null
+++ b/application/security/LoginManager.php
@@ -0,0 +1,238 @@
+<?php
+namespace Shaarli\Security;
+
+use Shaarli\Config\ConfigManager;
+
+/**
+ * User login management
+ */
+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;
+
+    /**
+     * Constructor
+     *
+     * @param array          $globals        The $GLOBALS array (reference)
+     * @param ConfigManager  $configManager  Configuration Manager instance
+     * @param SessionManager $sessionManager SessionManager instance
+     */
+    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  $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,
+            "<?php\n\$GLOBALS['IPBANS']=" . var_export($this->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;
+    }
+}
diff --git a/application/security/SessionManager.php b/application/security/SessionManager.php
new file mode 100644
index 00000000..6f004b24
--- /dev/null
+++ b/application/security/SessionManager.php
@@ -0,0 +1,179 @@
+<?php
+namespace Shaarli\Security;
+
+use Shaarli\Config\ConfigManager;
+
+/**
+ * Manages the server-side session
+ */
+class SessionManager
+{
+    /** @var int Session expiration timeout, in seconds */
+    public static $INACTIVITY_TIMEOUT = 3600;
+
+    /** @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 = [];
+
+    /** @var ConfigManager Configuration Manager instance **/
+    protected $conf = null;
+
+    /**
+     * Constructor
+     *
+     * @param array         $session The $_SESSION array (reference)
+     * @param ConfigManager $conf    ConfigManager instance
+     */
+    public function __construct(& $session, $conf)
+    {
+        $this->session = &$session;
+        $this->conf = $conf;
+    }
+
+    /**
+     * Generates a session token
+     *
+     * @return string token
+     */
+    public function generateToken()
+    {
+        $token = sha1(uniqid('', true) .'_'. mt_rand() . $this->conf->get('credentials.salt'));
+        $this->session['tokens'][$token] = 1;
+        return $token;
+    }
+
+    /**
+     * Checks the validity of a session token, and destroys it afterwards
+     *
+     * @param string $token The token to check
+     *
+     * @return bool true if the token is valid, else false
+     */
+    public function checkToken($token)
+    {
+        if (! isset($this->session['tokens'][$token])) {
+            // the token is wrong, or has already been used
+            return false;
+        }
+
+        // destroy the token to prevent future use
+        unset($this->session['tokens'][$token]);
+        return true;
+    }
+
+    /**
+     * Validate session ID to prevent Full Path Disclosure.
+     *
+     * See #298.
+     * The session ID's format depends on the hash algorithm set in PHP settings
+     *
+     * @param string $sessionId Session ID
+     *
+     * @return true if valid, false otherwise.
+     *
+     * @see http://php.net/manual/en/function.hash-algos.php
+     * @see http://php.net/manual/en/session.configuration.php
+     */
+    public static function checkId($sessionId)
+    {
+        if (empty($sessionId)) {
+            return false;
+        }
+
+        if (!$sessionId) {
+            return false;
+        }
+
+        if (!preg_match('/^[a-zA-Z0-9,-]{2,128}$/', $sessionId)) {
+            return false;
+        }
+
+        return true;
+    }
+
+    /**
+     * Store user login information after a successful login
+     *
+     * @param string $clientIpId Client IP address identifier
+     */
+    public function storeLoginInfo($clientIpId)
+    {
+        // Generate unique random number (different than phpsessionid)
+        $this->session['uid'] = sha1(uniqid('', true) . '_' . mt_rand());
+        $this->session['ip'] = $clientIpId;
+        $this->session['username'] = $this->conf->get('credentials.login');
+        $this->session['expires_on'] = time() + self::$INACTIVITY_TIMEOUT;
+    }
+
+    /**
+     * Extend session validity
+     */
+    public function extendSession()
+    {
+        if (! empty($this->session['longlastingsession'])) {
+            // "Stay signed in" is enabled
+            $this->session['expires_on'] = time() + $this->session['longlastingsession'];
+            return;
+        }
+        $this->session['expires_on'] = time() + self::$INACTIVITY_TIMEOUT;
+    }
+
+    /**
+     * Logout a user by unsetting all login information
+     *
+     * See:
+     * - https://secure.php.net/manual/en/function.setcookie.php
+     *
+     * @param string $webPath path on the server in which the cookie will be available on
+     */
+    public function logout($webPath)
+    {
+        if (isset($this->session)) {
+            unset($this->session['uid']);
+            unset($this->session['ip']);
+            unset($this->session['username']);
+            unset($this->session['visibility']);
+            unset($this->session['untaggedonly']);
+        }
+        setcookie(self::$LOGGED_IN_COOKIE, 'false', 0, $webPath);
+    }
+
+    /**
+     * Check whether the session has expired
+     *
+     * @param string $clientIpId Client IP address identifier
+     *
+     * @return bool true if the session has expired, false otherwise
+     */
+    public function hasSessionExpired()
+    {
+        if (empty($this->session['uid'])) {
+            return true;
+        }
+        if (time() >= $this->session['expires_on']) {
+            return true;
+        }
+        return false;
+    }
+
+    /**
+     * Check whether the client IP address has changed
+     *
+     * @param string $clientIpId Client IP address identifier
+     *
+     * @return bool true if the IP has changed, false if it has not, or
+     *              if session protection has been disabled
+     */
+    public function hasClientIpChanged($clientIpId)
+    {
+        if ($this->conf->get('security.session_protection_disabled') === true) {
+            return false;
+        }
+        if ($this->session['ip'] == $clientIpId) {
+            return false;
+        }
+        return true;
+    }
+}
diff --git a/composer.json b/composer.json
index 15e082f8..0d4c623c 100644
--- a/composer.json
+++ b/composer.json
@@ -36,7 +36,8 @@
             "Shaarli\\Api\\Controllers\\": "application/api/controllers",
             "Shaarli\\Api\\Exceptions\\": "application/api/exceptions",
             "Shaarli\\Config\\": "application/config/",
-            "Shaarli\\Config\\Exception\\": "application/config/exception"
+            "Shaarli\\Config\\Exception\\": "application/config/exception",
+            "Shaarli\\Security\\": "application/security"
         }
     }
 }
diff --git a/index.php b/index.php
index 30bfc170..139812d7 100644
--- a/index.php
+++ b/index.php
@@ -78,8 +78,8 @@ require_once 'application/Updater.php';
 use \Shaarli\Languages;
 use \Shaarli\ThemeUtils;
 use \Shaarli\Config\ConfigManager;
-use \Shaarli\LoginManager;
-use \Shaarli\SessionManager;
+use \Shaarli\Security\LoginManager;
+use \Shaarli\Security\SessionManager;
 
 // Ensure the PHP version is supported
 try {
diff --git a/tests/LoginManagerTest.php b/tests/LoginManagerTest.php
deleted file mode 100644
index 27ca0db5..00000000
--- a/tests/LoginManagerTest.php
+++ /dev/null
@@ -1,199 +0,0 @@
-<?php
-namespace Shaarli;
-
-require_once 'tests/utils/FakeConfigManager.php';
-use \PHPUnit\Framework\TestCase;
-
-/**
- * Test coverage for LoginManager
- */
-class LoginManagerTest extends TestCase
-{
-    protected $configManager = null;
-    protected $loginManager = null;
-    protected $banFile = 'sandbox/ipbans.php';
-    protected $logFile = 'sandbox/shaarli.log';
-    protected $globals = [];
-    protected $ipAddr = '127.0.0.1';
-    protected $server = [];
-    protected $trustedProxy = '10.1.1.100';
-
-    /**
-     * Prepare or reset test resources
-     */
-    public function setUp()
-    {
-        if (file_exists($this->banFile)) {
-            unlink($this->banFile);
-        }
-
-        $this->configManager = new \FakeConfigManager([
-            'resource.ban_file' => $this->banFile,
-            'resource.log' => $this->logFile,
-            'security.ban_after' => 4,
-            'security.ban_duration' => 3600,
-            'security.trusted_proxies' => [$this->trustedProxy],
-        ]);
-
-        $this->globals = &$GLOBALS;
-        unset($this->globals['IPBANS']);
-
-        $this->loginManager = new LoginManager($this->globals, $this->configManager, null);
-        $this->server['REMOTE_ADDR'] = $this->ipAddr;
-    }
-
-    /**
-     * Wipe test resources
-     */
-    public function tearDown()
-    {
-        unset($this->globals['IPBANS']);
-    }
-
-    /**
-     * Instantiate a LoginManager and load ban records
-     */
-    public function testReadBanFile()
-    {
-        file_put_contents(
-            $this->banFile,
-            "<?php\n\$GLOBALS['IPBANS']=array('FAILURES' => array('127.0.0.1' => 99));\n?>"
-        );
-        new LoginManager($this->globals, $this->configManager, null);
-        $this->assertEquals(99, $this->globals['IPBANS']['FAILURES']['127.0.0.1']);
-    }
-
-    /**
-     * Record a failed login attempt
-     */
-    public function testHandleFailedLogin()
-    {
-        $this->loginManager->handleFailedLogin($this->server);
-        $this->assertEquals(1, $this->globals['IPBANS']['FAILURES'][$this->ipAddr]);
-
-        $this->loginManager->handleFailedLogin($this->server);
-        $this->assertEquals(2, $this->globals['IPBANS']['FAILURES'][$this->ipAddr]);
-    }
-
-    /**
-     * Record a failed login attempt - IP behind a trusted proxy
-     */
-    public function testHandleFailedLoginBehindTrustedProxy()
-    {
-        $server = [
-            'REMOTE_ADDR' => $this->trustedProxy,
-            'HTTP_X_FORWARDED_FOR' => $this->ipAddr,
-        ];
-        $this->loginManager->handleFailedLogin($server);
-        $this->assertEquals(1, $this->globals['IPBANS']['FAILURES'][$this->ipAddr]);
-
-        $this->loginManager->handleFailedLogin($server);
-        $this->assertEquals(2, $this->globals['IPBANS']['FAILURES'][$this->ipAddr]);
-    }
-
-    /**
-     * Record a failed login attempt - IP behind a trusted proxy but not forwarded
-     */
-    public function testHandleFailedLoginBehindTrustedProxyNoIp()
-    {
-        $server = [
-            'REMOTE_ADDR' => $this->trustedProxy,
-        ];
-        $this->loginManager->handleFailedLogin($server);
-        $this->assertFalse(isset($this->globals['IPBANS']['FAILURES'][$this->ipAddr]));
-
-        $this->loginManager->handleFailedLogin($server);
-        $this->assertFalse(isset($this->globals['IPBANS']['FAILURES'][$this->ipAddr]));
-    }
-
-    /**
-     * Record a failed login attempt and ban the IP after too many failures
-     */
-    public function testHandleFailedLoginBanIp()
-    {
-        $this->loginManager->handleFailedLogin($this->server);
-        $this->assertEquals(1, $this->globals['IPBANS']['FAILURES'][$this->ipAddr]);
-        $this->assertTrue($this->loginManager->canLogin($this->server));
-
-        $this->loginManager->handleFailedLogin($this->server);
-        $this->assertEquals(2, $this->globals['IPBANS']['FAILURES'][$this->ipAddr]);
-        $this->assertTrue($this->loginManager->canLogin($this->server));
-
-        $this->loginManager->handleFailedLogin($this->server);
-        $this->assertEquals(3, $this->globals['IPBANS']['FAILURES'][$this->ipAddr]);
-        $this->assertTrue($this->loginManager->canLogin($this->server));
-
-        $this->loginManager->handleFailedLogin($this->server);
-        $this->assertEquals(4, $this->globals['IPBANS']['FAILURES'][$this->ipAddr]);
-        $this->assertFalse($this->loginManager->canLogin($this->server));
-
-        // handleFailedLogin is not supposed to be called at this point:
-        // - no login form should be displayed once an IP has been banned
-        // - yet this could happen when using custom templates / scripts
-        $this->loginManager->handleFailedLogin($this->server);
-        $this->assertEquals(5, $this->globals['IPBANS']['FAILURES'][$this->ipAddr]);
-        $this->assertFalse($this->loginManager->canLogin($this->server));
-    }
-
-    /**
-     * Nothing to do
-     */
-    public function testHandleSuccessfulLogin()
-    {
-        $this->assertTrue($this->loginManager->canLogin($this->server));
-
-        $this->loginManager->handleSuccessfulLogin($this->server);
-        $this->assertTrue($this->loginManager->canLogin($this->server));
-    }
-
-    /**
-     * Erase failure records after successfully logging in from this IP
-     */
-    public function testHandleSuccessfulLoginAfterFailure()
-    {
-        $this->loginManager->handleFailedLogin($this->server);
-        $this->loginManager->handleFailedLogin($this->server);
-        $this->assertEquals(2, $this->globals['IPBANS']['FAILURES'][$this->ipAddr]);
-        $this->assertTrue($this->loginManager->canLogin($this->server));
-
-        $this->loginManager->handleSuccessfulLogin($this->server);
-        $this->assertTrue($this->loginManager->canLogin($this->server));
-        $this->assertFalse(isset($this->globals['IPBANS']['FAILURES'][$this->ipAddr]));
-        $this->assertFalse(isset($this->globals['IPBANS']['BANS'][$this->ipAddr]));
-    }
-
-    /**
-     * The IP is not banned
-     */
-    public function testCanLoginIpNotBanned()
-    {
-        $this->assertTrue($this->loginManager->canLogin($this->server));
-    }
-
-    /**
-     * The IP is banned
-     */
-    public function testCanLoginIpBanned()
-    {
-        // ban the IP for an hour
-        $this->globals['IPBANS']['FAILURES'][$this->ipAddr] = 10;
-        $this->globals['IPBANS']['BANS'][$this->ipAddr] = time() + 3600;
-
-        $this->assertFalse($this->loginManager->canLogin($this->server));
-    }
-
-    /**
-     * The IP is banned, and the ban duration is over
-     */
-    public function testCanLoginIpBanExpired()
-    {
-        // ban the IP for an hour
-        $this->globals['IPBANS']['FAILURES'][$this->ipAddr] = 10;
-        $this->globals['IPBANS']['BANS'][$this->ipAddr] = time() + 3600;
-        $this->assertFalse($this->loginManager->canLogin($this->server));
-
-        // lift the ban
-        $this->globals['IPBANS']['BANS'][$this->ipAddr] = time() - 3600;
-        $this->assertTrue($this->loginManager->canLogin($this->server));
-    }
-}
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/security/LoginManagerTest.php b/tests/security/LoginManagerTest.php
new file mode 100644
index 00000000..b957abe3
--- /dev/null
+++ b/tests/security/LoginManagerTest.php
@@ -0,0 +1,199 @@
+<?php
+namespace Shaarli\Security;
+
+require_once 'tests/utils/FakeConfigManager.php';
+use \PHPUnit\Framework\TestCase;
+
+/**
+ * Test coverage for LoginManager
+ */
+class LoginManagerTest extends TestCase
+{
+    protected $configManager = null;
+    protected $loginManager = null;
+    protected $banFile = 'sandbox/ipbans.php';
+    protected $logFile = 'sandbox/shaarli.log';
+    protected $globals = [];
+    protected $ipAddr = '127.0.0.1';
+    protected $server = [];
+    protected $trustedProxy = '10.1.1.100';
+
+    /**
+     * Prepare or reset test resources
+     */
+    public function setUp()
+    {
+        if (file_exists($this->banFile)) {
+            unlink($this->banFile);
+        }
+
+        $this->configManager = new \FakeConfigManager([
+            'resource.ban_file' => $this->banFile,
+            'resource.log' => $this->logFile,
+            'security.ban_after' => 4,
+            'security.ban_duration' => 3600,
+            'security.trusted_proxies' => [$this->trustedProxy],
+        ]);
+
+        $this->globals = &$GLOBALS;
+        unset($this->globals['IPBANS']);
+
+        $this->loginManager = new LoginManager($this->globals, $this->configManager, null);
+        $this->server['REMOTE_ADDR'] = $this->ipAddr;
+    }
+
+    /**
+     * Wipe test resources
+     */
+    public function tearDown()
+    {
+        unset($this->globals['IPBANS']);
+    }
+
+    /**
+     * Instantiate a LoginManager and load ban records
+     */
+    public function testReadBanFile()
+    {
+        file_put_contents(
+            $this->banFile,
+            "<?php\n\$GLOBALS['IPBANS']=array('FAILURES' => array('127.0.0.1' => 99));\n?>"
+        );
+        new LoginManager($this->globals, $this->configManager, null);
+        $this->assertEquals(99, $this->globals['IPBANS']['FAILURES']['127.0.0.1']);
+    }
+
+    /**
+     * Record a failed login attempt
+     */
+    public function testHandleFailedLogin()
+    {
+        $this->loginManager->handleFailedLogin($this->server);
+        $this->assertEquals(1, $this->globals['IPBANS']['FAILURES'][$this->ipAddr]);
+
+        $this->loginManager->handleFailedLogin($this->server);
+        $this->assertEquals(2, $this->globals['IPBANS']['FAILURES'][$this->ipAddr]);
+    }
+
+    /**
+     * Record a failed login attempt - IP behind a trusted proxy
+     */
+    public function testHandleFailedLoginBehindTrustedProxy()
+    {
+        $server = [
+            'REMOTE_ADDR' => $this->trustedProxy,
+            'HTTP_X_FORWARDED_FOR' => $this->ipAddr,
+        ];
+        $this->loginManager->handleFailedLogin($server);
+        $this->assertEquals(1, $this->globals['IPBANS']['FAILURES'][$this->ipAddr]);
+
+        $this->loginManager->handleFailedLogin($server);
+        $this->assertEquals(2, $this->globals['IPBANS']['FAILURES'][$this->ipAddr]);
+    }
+
+    /**
+     * Record a failed login attempt - IP behind a trusted proxy but not forwarded
+     */
+    public function testHandleFailedLoginBehindTrustedProxyNoIp()
+    {
+        $server = [
+            'REMOTE_ADDR' => $this->trustedProxy,
+        ];
+        $this->loginManager->handleFailedLogin($server);
+        $this->assertFalse(isset($this->globals['IPBANS']['FAILURES'][$this->ipAddr]));
+
+        $this->loginManager->handleFailedLogin($server);
+        $this->assertFalse(isset($this->globals['IPBANS']['FAILURES'][$this->ipAddr]));
+    }
+
+    /**
+     * Record a failed login attempt and ban the IP after too many failures
+     */
+    public function testHandleFailedLoginBanIp()
+    {
+        $this->loginManager->handleFailedLogin($this->server);
+        $this->assertEquals(1, $this->globals['IPBANS']['FAILURES'][$this->ipAddr]);
+        $this->assertTrue($this->loginManager->canLogin($this->server));
+
+        $this->loginManager->handleFailedLogin($this->server);
+        $this->assertEquals(2, $this->globals['IPBANS']['FAILURES'][$this->ipAddr]);
+        $this->assertTrue($this->loginManager->canLogin($this->server));
+
+        $this->loginManager->handleFailedLogin($this->server);
+        $this->assertEquals(3, $this->globals['IPBANS']['FAILURES'][$this->ipAddr]);
+        $this->assertTrue($this->loginManager->canLogin($this->server));
+
+        $this->loginManager->handleFailedLogin($this->server);
+        $this->assertEquals(4, $this->globals['IPBANS']['FAILURES'][$this->ipAddr]);
+        $this->assertFalse($this->loginManager->canLogin($this->server));
+
+        // handleFailedLogin is not supposed to be called at this point:
+        // - no login form should be displayed once an IP has been banned
+        // - yet this could happen when using custom templates / scripts
+        $this->loginManager->handleFailedLogin($this->server);
+        $this->assertEquals(5, $this->globals['IPBANS']['FAILURES'][$this->ipAddr]);
+        $this->assertFalse($this->loginManager->canLogin($this->server));
+    }
+
+    /**
+     * Nothing to do
+     */
+    public function testHandleSuccessfulLogin()
+    {
+        $this->assertTrue($this->loginManager->canLogin($this->server));
+
+        $this->loginManager->handleSuccessfulLogin($this->server);
+        $this->assertTrue($this->loginManager->canLogin($this->server));
+    }
+
+    /**
+     * Erase failure records after successfully logging in from this IP
+     */
+    public function testHandleSuccessfulLoginAfterFailure()
+    {
+        $this->loginManager->handleFailedLogin($this->server);
+        $this->loginManager->handleFailedLogin($this->server);
+        $this->assertEquals(2, $this->globals['IPBANS']['FAILURES'][$this->ipAddr]);
+        $this->assertTrue($this->loginManager->canLogin($this->server));
+
+        $this->loginManager->handleSuccessfulLogin($this->server);
+        $this->assertTrue($this->loginManager->canLogin($this->server));
+        $this->assertFalse(isset($this->globals['IPBANS']['FAILURES'][$this->ipAddr]));
+        $this->assertFalse(isset($this->globals['IPBANS']['BANS'][$this->ipAddr]));
+    }
+
+    /**
+     * The IP is not banned
+     */
+    public function testCanLoginIpNotBanned()
+    {
+        $this->assertTrue($this->loginManager->canLogin($this->server));
+    }
+
+    /**
+     * The IP is banned
+     */
+    public function testCanLoginIpBanned()
+    {
+        // ban the IP for an hour
+        $this->globals['IPBANS']['FAILURES'][$this->ipAddr] = 10;
+        $this->globals['IPBANS']['BANS'][$this->ipAddr] = time() + 3600;
+
+        $this->assertFalse($this->loginManager->canLogin($this->server));
+    }
+
+    /**
+     * The IP is banned, and the ban duration is over
+     */
+    public function testCanLoginIpBanExpired()
+    {
+        // ban the IP for an hour
+        $this->globals['IPBANS']['FAILURES'][$this->ipAddr] = 10;
+        $this->globals['IPBANS']['BANS'][$this->ipAddr] = time() + 3600;
+        $this->assertFalse($this->loginManager->canLogin($this->server));
+
+        // lift the ban
+        $this->globals['IPBANS']['BANS'][$this->ipAddr] = time() - 3600;
+        $this->assertTrue($this->loginManager->canLogin($this->server));
+    }
+}
diff --git a/tests/security/SessionManagerTest.php b/tests/security/SessionManagerTest.php
new file mode 100644
index 00000000..e4e1cfbc
--- /dev/null
+++ b/tests/security/SessionManagerTest.php
@@ -0,0 +1,149 @@
+<?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
+{
+    // 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=')
+        );
+    }
+}
-- 
cgit v1.2.3


From 51f0128cdba52099c40693379e72f094b42a6f80 Mon Sep 17 00:00:00 2001
From: VirtualTam <virtualtam@flibidi.net>
Date: Fri, 27 Apr 2018 23:17:38 +0200
Subject: Refactor session and cookie timeout control

Signed-off-by: VirtualTam <virtualtam@flibidi.net>
---
 application/security/LoginManager.php   |   5 +-
 application/security/SessionManager.php |  48 +++++++--
 index.php                               |  47 +++++----
 tests/security/SessionManagerTest.php   | 181 ++++++++++++++++++++++++++++----
 4 files changed, 224 insertions(+), 57 deletions(-)

diff --git a/application/security/LoginManager.php b/application/security/LoginManager.php
index e7b9b21e..27247f3f 100644
--- a/application/security/LoginManager.php
+++ b/application/security/LoginManager.php
@@ -49,13 +49,12 @@ class LoginManager
      * 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)
+    public function checkLoginState($cookie, $clientIpId, $token)
     {
         if (! $this->configManager->exists('credentials.login')) {
             // Shaarli is not configured yet
@@ -73,7 +72,7 @@ class LoginManager
         if ($this->sessionManager->hasSessionExpired()
             || $this->sessionManager->hasClientIpChanged($clientIpId)
         ) {
-            $this->sessionManager->logout($webPath);
+            $this->sessionManager->logout();
             $this->isLoggedIn = false;
             return;
         }
diff --git a/application/security/SessionManager.php b/application/security/SessionManager.php
index 6f004b24..0dcd7f90 100644
--- a/application/security/SessionManager.php
+++ b/application/security/SessionManager.php
@@ -9,7 +9,10 @@ use Shaarli\Config\ConfigManager;
 class SessionManager
 {
     /** @var int Session expiration timeout, in seconds */
-    public static $INACTIVITY_TIMEOUT = 3600;
+    public static $SHORT_TIMEOUT = 3600;    // 1 hour
+
+    /** @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';
@@ -20,6 +23,9 @@ class SessionManager
     /** @var ConfigManager Configuration Manager instance **/
     protected $conf = null;
 
+    /** @var bool Whether the user should stay signed in (LONG_TIMEOUT) */
+    protected $staySignedIn = false;
+
     /**
      * Constructor
      *
@@ -32,6 +38,16 @@ class SessionManager
         $this->conf = $conf;
     }
 
+    /**
+     * Define whether the user should stay signed in across browser sessions
+     *
+     * @param bool $staySignedIn Keep the user signed in
+     */
+    public function setStaySignedIn($staySignedIn)
+    {
+        $this->staySignedIn = $staySignedIn;
+    }
+
     /**
      * Generates a session token
      *
@@ -104,7 +120,7 @@ class SessionManager
         $this->session['uid'] = sha1(uniqid('', true) . '_' . mt_rand());
         $this->session['ip'] = $clientIpId;
         $this->session['username'] = $this->conf->get('credentials.login');
-        $this->session['expires_on'] = time() + self::$INACTIVITY_TIMEOUT;
+        $this->extendTimeValidityBy(self::$SHORT_TIMEOUT);
     }
 
     /**
@@ -112,12 +128,24 @@ class SessionManager
      */
     public function extendSession()
     {
-        if (! empty($this->session['longlastingsession'])) {
-            // "Stay signed in" is enabled
-            $this->session['expires_on'] = time() + $this->session['longlastingsession'];
-            return;
+        if ($this->staySignedIn) {
+            return $this->extendTimeValidityBy(self::$LONG_TIMEOUT);
         }
-        $this->session['expires_on'] = time() + self::$INACTIVITY_TIMEOUT;
+        return $this->extendTimeValidityBy(self::$SHORT_TIMEOUT);
+    }
+
+    /**
+     * Extend expiration time
+     *
+     * @param int $duration Expiration time extension (seconds)
+     *
+     * @return int New session expiration time
+     */
+    protected function extendTimeValidityBy($duration)
+    {
+        $expirationTime = time() + $duration;
+        $this->session['expires_on'] = $expirationTime;
+        return $expirationTime;
     }
 
     /**
@@ -125,19 +153,17 @@ class SessionManager
      *
      * See:
      * - https://secure.php.net/manual/en/function.setcookie.php
-     *
-     * @param string $webPath path on the server in which the cookie will be available on
      */
-    public function logout($webPath)
+    public function logout()
     {
         if (isset($this->session)) {
             unset($this->session['uid']);
             unset($this->session['ip']);
+            unset($this->session['expires_on']);
             unset($this->session['username']);
             unset($this->session['visibility']);
             unset($this->session['untaggedonly']);
         }
-        setcookie(self::$LOGGED_IN_COOKIE, 'false', 0, $webPath);
     }
 
     /**
diff --git a/index.php b/index.php
index 139812d7..8e3bade0 100644
--- a/index.php
+++ b/index.php
@@ -179,7 +179,7 @@ if (! is_file($conf->getConfigFileExt())) {
 // 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, WEB_PATH, $clientIpId, STAY_SIGNED_IN_TOKEN);
+$loginManager->checkLoginState($_COOKIE, $clientIpId, STAY_SIGNED_IN_TOKEN);
 
 /**
  * Adapter function to ensure compatibility with third-party templates
@@ -205,31 +205,35 @@ if (isset($_POST['login'])) {
         && $sessionManager->checkToken($_POST['token'])
         && $loginManager->checkCredentials($_SERVER['REMOTE_ADDR'], $clientIpId, $_POST['login'], $_POST['password'])
     ) {
-        // Login/password is OK.
         $loginManager->handleSuccessfulLogin($_SERVER);
 
-        // If user wants to keep the session cookie even after the browser closes:
-        if (!empty($_POST['longlastingsession'])) {
-            $_SESSION['longlastingsession'] = 31536000; // (31536000 seconds = 1 year)
-            $expiration = time() + $_SESSION['longlastingsession']; // calculate relative cookie expiration (1 year from now)
-            setcookie($sessionManager::$LOGGED_IN_COOKIE, STAY_SIGNED_IN_TOKEN, $expiration, WEB_PATH);
-            $_SESSION['expires_on'] = $expiration;  // Set session expiration on server-side.
-
-            $cookiedir = '';
-            if (dirname($_SERVER['SCRIPT_NAME']) != '/') {
-                $cookiedir = dirname($_SERVER["SCRIPT_NAME"]) . '/';
-            }
-            session_set_cookie_params($_SESSION['longlastingsession'],$cookiedir,$_SERVER['SERVER_NAME']); // Set session cookie expiration on client side
+        $cookiedir = '';
+        if (dirname($_SERVER['SCRIPT_NAME']) != '/') {
             // Note: Never forget the trailing slash on the cookie path!
-            session_regenerate_id(true);  // Send cookie with new expiration date to browser.
+            $cookiedir = dirname($_SERVER["SCRIPT_NAME"]) . '/';
         }
-        else // Standard session expiration (=when browser closes)
-        {
-            $cookiedir = ''; if(dirname($_SERVER['SCRIPT_NAME'])!='/') $cookiedir=dirname($_SERVER["SCRIPT_NAME"]).'/';
-            session_set_cookie_params(0,$cookiedir,$_SERVER['SERVER_NAME']); // 0 means "When browser closes"
-            session_regenerate_id(true);
+
+        if (!empty($_POST['longlastingsession'])) {
+            // Keep the session cookie even after the browser closes
+            $sessionManager->setStaySignedIn(true);
+            $expirationTime = $sessionManager->extendSession();
+
+            setcookie(
+                $sessionManager::$LOGGED_IN_COOKIE,
+                STAY_SIGNED_IN_TOKEN,
+                $expirationTime,
+                WEB_PATH
+            );
+
+        } else {
+            // Standard session expiration (=when browser closes)
+            $expirationTime = 0;
         }
 
+        // Send cookie with the new expiration date to the browser
+        session_set_cookie_params($expirationTime, $cookiedir, $_SERVER['SERVER_NAME']);
+        session_regenerate_id(true);
+
         // Optional redirect after login:
         if (isset($_GET['post'])) {
             $uri = '?post='. urlencode($_GET['post']);
@@ -590,7 +594,8 @@ function renderPage($conf, $pluginManager, $LINKSDB, $history, $sessionManager,
     if (isset($_SERVER['QUERY_STRING']) && startsWith($_SERVER['QUERY_STRING'], 'do=logout'))
     {
         invalidateCaches($conf->get('resource.page_cache'));
-        $sessionManager->logout(WEB_PATH);
+        $sessionManager->logout();
+        setcookie(SessionManager::$LOGGED_IN_COOKIE, 'false', 0, WEB_PATH);
         header('Location: ?');
         exit;
     }
diff --git a/tests/security/SessionManagerTest.php b/tests/security/SessionManagerTest.php
index e4e1cfbc..e1c72707 100644
--- a/tests/security/SessionManagerTest.php
+++ b/tests/security/SessionManagerTest.php
@@ -14,11 +14,17 @@ use \PHPUnit\Framework\TestCase;
  */
 class SessionManagerTest extends TestCase
 {
-    // Session ID hashes
+    /** @var array Session ID hashes */
     protected static $sidHashes = null;
 
-    // Fake ConfigManager
-    protected static $conf = 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
@@ -26,7 +32,20 @@ class SessionManagerTest extends TestCase
     public static function setUpBeforeClass()
     {
         self::$sidHashes = ReferenceSessionIdHashes::getHashes();
-        self::$conf = new FakeConfigManager();
+    }
+
+    /**
+     * 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);
     }
 
     /**
@@ -34,12 +53,9 @@ class SessionManagerTest extends TestCase
      */
     public function testGenerateToken()
     {
-        $session = [];
-        $sessionManager = new SessionManager($session, self::$conf);
-
-        $token = $sessionManager->generateToken();
+        $token = $this->sessionManager->generateToken();
 
-        $this->assertEquals(1, $session['tokens'][$token]);
+        $this->assertEquals(1, $this->session['tokens'][$token]);
         $this->assertEquals(40, strlen($token));
     }
 
@@ -54,7 +70,7 @@ class SessionManagerTest extends TestCase
                 $token => 1,
             ],
         ];
-        $sessionManager = new SessionManager($session, self::$conf);
+        $sessionManager = new SessionManager($session, $this->conf);
 
         // check and destroy the token
         $this->assertTrue($sessionManager->checkToken($token));
@@ -69,21 +85,18 @@ class SessionManagerTest extends TestCase
      */
     public function testGenerateAndCheckToken()
     {
-        $session = [];
-        $sessionManager = new SessionManager($session, self::$conf);
-
-        $token = $sessionManager->generateToken();
+        $token = $this->sessionManager->generateToken();
 
         // ensure a token has been generated
-        $this->assertEquals(1, $session['tokens'][$token]);
+        $this->assertEquals(1, $this->session['tokens'][$token]);
         $this->assertEquals(40, strlen($token));
 
         // check and destroy the token
-        $this->assertTrue($sessionManager->checkToken($token));
-        $this->assertFalse(isset($session['tokens'][$token]));
+        $this->assertTrue($this->sessionManager->checkToken($token));
+        $this->assertFalse(isset($this->session['tokens'][$token]));
 
         // ensure the token has been destroyed
-        $this->assertFalse($sessionManager->checkToken($token));
+        $this->assertFalse($this->sessionManager->checkToken($token));
     }
 
     /**
@@ -91,10 +104,7 @@ class SessionManagerTest extends TestCase
      */
     public function testCheckInvalidToken()
     {
-        $session = [];
-        $sessionManager = new SessionManager($session, self::$conf);
-
-        $this->assertFalse($sessionManager->checkToken('4dccc3a45ad9d03e5542b90c37d8db6d10f2b38b'));
+        $this->assertFalse($this->sessionManager->checkToken('4dccc3a45ad9d03e5542b90c37d8db6d10f2b38b'));
     }
 
     /**
@@ -146,4 +156,131 @@ class SessionManagerTest extends TestCase
             SessionManager::checkId('c0ZqcWF3VFE2NmJBdm1HMVQ0ZHJ3UmZPbTFsNGhkNHI=')
         );
     }
+
+    /**
+     * Store login information after a successful login
+     */
+    public function testStoreLoginInfo()
+    {
+        $this->sessionManager->storeLoginInfo('ip_id');
+
+        $this->assertTrue(isset($this->session['uid']));
+        $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 = [
+            'uid' => 'some-uid',
+            'ip' => 'ip_id',
+            'expires_on' => time() + 1000,
+            'username' => 'johndoe',
+            'visibility' => 'public',
+            'untaggedonly' => false,
+        ];
+        $this->sessionManager->logout();
+
+        $this->assertFalse(isset($this->session['uid']));
+        $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 considered as expired because the UID is missing
+     */
+    public function testHasExpiredNoUid()
+    {
+        $this->assertTrue($this->sessionManager->hasSessionExpired());
+    }
+
+    /**
+     * The session is active and expiration time has been reached
+     */
+    public function testHasExpiredTimeElapsed()
+    {
+        $this->session['uid'] = 'some-uid';
+        $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['uid'] = 'some-uid';
+        $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'));
+    }
 }
-- 
cgit v1.2.3


From c689e108639a4f6aa9e15928422e14db7cbe30ca Mon Sep 17 00:00:00 2001
From: VirtualTam <virtualtam@flibidi.net>
Date: Sun, 6 May 2018 17:06:36 +0200
Subject: Refactor LoginManager stay-signed-in token management

Signed-off-by: VirtualTam <virtualtam@flibidi.net>
---
 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()
+        );
+    }
 }
-- 
cgit v1.2.3


From ebf615173824a46de82fa97a165bcfd883db15ce Mon Sep 17 00:00:00 2001
From: VirtualTam <virtualtam@flibidi.net>
Date: Thu, 10 May 2018 13:07:51 +0200
Subject: SessionManager: remove unused UID token
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

There already are dedicated tokens for:
- CSRF protection
- user stay-signed-in feature, via cookie

This token was most likely intended as a randomly generated,
server-side, secret key to be used when generating hashes.

See http://sebsauvage.net/wiki/doku.php?id=php:session [FR]

Relevant section:

  Une clé secrète unique aléatoire est générée côté serveur (et jamais
  envoyée). Elle peut servir pour signer les formulaires (HMAC) ou
  générer des token de formulaires (protection contre XSRF).
  Voir $_SESSION['uid'].

Translation:

  A unique, server-side secret key is randomly generated (and never
  transmitted). It can be used to sign forms (HMAC) or generate form
  tokens (protection against XSRF).
  See $_SESSION['uid']

Signed-off-by: VirtualTam <virtualtam@flibidi.net>
---
 application/security/SessionManager.php |  6 ------
 tests/security/SessionManagerTest.php   | 13 -------------
 2 files changed, 19 deletions(-)

diff --git a/application/security/SessionManager.php b/application/security/SessionManager.php
index 58973130..24e25528 100644
--- a/application/security/SessionManager.php
+++ b/application/security/SessionManager.php
@@ -113,8 +113,6 @@ class SessionManager
      */
     public function storeLoginInfo($clientIpId)
     {
-        // Generate unique random number (different than phpsessionid)
-        $this->session['uid'] = sha1(uniqid('', true) . '_' . mt_rand());
         $this->session['ip'] = $clientIpId;
         $this->session['username'] = $this->conf->get('credentials.login');
         $this->extendTimeValidityBy(self::$SHORT_TIMEOUT);
@@ -154,7 +152,6 @@ class SessionManager
     public function logout()
     {
         if (isset($this->session)) {
-            unset($this->session['uid']);
             unset($this->session['ip']);
             unset($this->session['expires_on']);
             unset($this->session['username']);
@@ -172,9 +169,6 @@ class SessionManager
      */
     public function hasSessionExpired()
     {
-        if (empty($this->session['uid'])) {
-            return true;
-        }
         if (time() >= $this->session['expires_on']) {
             return true;
         }
diff --git a/tests/security/SessionManagerTest.php b/tests/security/SessionManagerTest.php
index e1c72707..ae10ffa6 100644
--- a/tests/security/SessionManagerTest.php
+++ b/tests/security/SessionManagerTest.php
@@ -164,7 +164,6 @@ class SessionManagerTest extends TestCase
     {
         $this->sessionManager->storeLoginInfo('ip_id');
 
-        $this->assertTrue(isset($this->session['uid']));
         $this->assertGreaterThan(time(), $this->session['expires_on']);
         $this->assertEquals('ip_id', $this->session['ip']);
         $this->assertEquals('johndoe', $this->session['username']);
@@ -209,7 +208,6 @@ class SessionManagerTest extends TestCase
     public function testLogout()
     {
         $this->session = [
-            'uid' => 'some-uid',
             'ip' => 'ip_id',
             'expires_on' => time() + 1000,
             'username' => 'johndoe',
@@ -218,7 +216,6 @@ class SessionManagerTest extends TestCase
         ];
         $this->sessionManager->logout();
 
-        $this->assertFalse(isset($this->session['uid']));
         $this->assertFalse(isset($this->session['ip']));
         $this->assertFalse(isset($this->session['expires_on']));
         $this->assertFalse(isset($this->session['username']));
@@ -226,20 +223,11 @@ class SessionManagerTest extends TestCase
         $this->assertFalse(isset($this->session['untaggedonly']));
     }
 
-    /**
-     * The session is considered as expired because the UID is missing
-     */
-    public function testHasExpiredNoUid()
-    {
-        $this->assertTrue($this->sessionManager->hasSessionExpired());
-    }
-
     /**
      * The session is active and expiration time has been reached
      */
     public function testHasExpiredTimeElapsed()
     {
-        $this->session['uid'] = 'some-uid';
         $this->session['expires_on'] = time() - 10;
 
         $this->assertTrue($this->sessionManager->hasSessionExpired());
@@ -250,7 +238,6 @@ class SessionManagerTest extends TestCase
      */
     public function testHasNotExpired()
     {
-        $this->session['uid'] = 'some-uid';
         $this->session['expires_on'] = time() + 1000;
 
         $this->assertFalse($this->sessionManager->hasSessionExpired());
-- 
cgit v1.2.3


From 704637bfebc73ada4b800b35c457e9fe56ad3567 Mon Sep 17 00:00:00 2001
From: VirtualTam <virtualtam@flibidi.net>
Date: Sun, 6 May 2018 17:12:48 +0200
Subject: Add test coverage for LoginManager methods

Signed-off-by: VirtualTam <virtualtam@flibidi.net>
---
 application/security/LoginManager.php |   9 +-
 tests/security/LoginManagerTest.php   | 149 ++++++++++++++++++++++++++++++++--
 tests/security/SessionManagerTest.php |   2 +-
 tests/utils/FakeConfigManager.php     |  12 +++
 4 files changed, 161 insertions(+), 11 deletions(-)

diff --git a/application/security/LoginManager.php b/application/security/LoginManager.php
index 41fa9a20..4946850b 100644
--- a/application/security/LoginManager.php
+++ b/application/security/LoginManager.php
@@ -46,7 +46,7 @@ class LoginManager
         $this->sessionManager = $sessionManager;
         $this->banFile = $this->configManager->get('resource.ban_file', 'data/ipbans.php');
         $this->readBanFile();
-        if ($this->configManager->get('security.open_shaarli')) {
+        if ($this->configManager->get('security.open_shaarli') === true) {
             $this->openShaarli = true;
         }
     }
@@ -80,8 +80,6 @@ class LoginManager
      *
      * @param array  $cookie     The $_COOKIE array
      * @param string $clientIpId Client IP address identifier
-     *
-     * @return bool true if the user session is valid, false otherwise
      */
     public function checkLoginState($cookie, $clientIpId)
     {
@@ -94,11 +92,12 @@ class LoginManager
         if (isset($cookie[self::$STAY_SIGNED_IN_COOKIE])
             && $cookie[self::$STAY_SIGNED_IN_COOKIE] === $this->staySignedInToken
         ) {
+            // The user client has a valid stay-signed-in cookie
+            // Session information is updated with the current client information
             $this->sessionManager->storeLoginInfo($clientIpId);
             $this->isLoggedIn = true;
-        }
 
-        if ($this->sessionManager->hasSessionExpired()
+        } elseif ($this->sessionManager->hasSessionExpired()
             || $this->sessionManager->hasClientIpChanged($clientIpId)
         ) {
             $this->sessionManager->logout();
diff --git a/tests/security/LoginManagerTest.php b/tests/security/LoginManagerTest.php
index 633f1bb9..fad09992 100644
--- a/tests/security/LoginManagerTest.php
+++ b/tests/security/LoginManagerTest.php
@@ -9,13 +9,40 @@ 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 */
@@ -52,10 +79,18 @@ 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, null);
+        $this->session = [
+            'expires_on' => time() + 100,
+            'ip' => $this->clientIpAddress,
+        ];
+
+        $this->sessionManager = new SessionManager($this->session, $this->configManager);
+        $this->loginManager = new LoginManager($this->globals, $this->configManager, $this->sessionManager);
         $this->server['REMOTE_ADDR'] = $this->ipAddr;
     }
 
@@ -219,12 +254,116 @@ class LoginManagerTest extends TestCase
      */
     public function testGenerateStaySignedInToken()
     {
-        $ipAddress = '10.1.47.179';
-        $this->loginManager->generateStaySignedInToken($ipAddress);
+        $this->loginManager->generateStaySignedInToken($this->clientIpAddress);
 
         $this->assertEquals(
-            sha1($this->passwordHash . $ipAddress . $this->salt),
+            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()
+    {
+        $this->loginManager->generateStaySignedInToken($this->clientIpAddress);
+        $this->cookie[LoginManager::$STAY_SIGNED_IN_COOKIE] = 'nope';
+
+        $this->loginManager->checkLoginState($this->cookie, $this->clientIpAddress);
+
+        $this->assertFalse($this->loginManager->isLoggedIn());
+    }
+
+    /**
+     * 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());
+    }
+
+    /**
+     * 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
index ae10ffa6..9bd868f8 100644
--- a/tests/security/SessionManagerTest.php
+++ b/tests/security/SessionManagerTest.php
@@ -17,7 +17,7 @@ class SessionManagerTest extends TestCase
     /** @var array Session ID hashes */
     protected static $sidHashes = null;
 
-    /** @var FakeConfigManager ConfigManager substitute for testing */
+    /** @var \FakeConfigManager ConfigManager substitute for testing */
     protected $conf = null;
 
     /** @var array $_SESSION array for testing */
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);
+    }
 }
-- 
cgit v1.2.3


From 8edd7f15886620b07064aa889aea05c5acbc0e58 Mon Sep 17 00:00:00 2001
From: VirtualTam <virtualtam@flibidi.net>
Date: Wed, 30 May 2018 02:09:09 +0200
Subject: SessionManager+LoginManager: fix checkLoginState logic

Signed-off-by: VirtualTam <virtualtam@flibidi.net>
---
 application/security/LoginManager.php   |  2 +-
 application/security/SessionManager.php |  5 ++++-
 tests/security/LoginManagerTest.php     | 15 ++++++++++-----
 3 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/application/security/LoginManager.php b/application/security/LoginManager.php
index 4946850b..d6784d6d 100644
--- a/application/security/LoginManager.php
+++ b/application/security/LoginManager.php
@@ -95,7 +95,6 @@ class LoginManager
             // The user client has a valid stay-signed-in cookie
             // Session information is updated with the current client information
             $this->sessionManager->storeLoginInfo($clientIpId);
-            $this->isLoggedIn = true;
 
         } elseif ($this->sessionManager->hasSessionExpired()
             || $this->sessionManager->hasClientIpChanged($clientIpId)
@@ -105,6 +104,7 @@ class LoginManager
             return;
         }
 
+        $this->isLoggedIn = true;
         $this->sessionManager->extendSession();
     }
 
diff --git a/application/security/SessionManager.php b/application/security/SessionManager.php
index 24e25528..b8b8ab8d 100644
--- a/application/security/SessionManager.php
+++ b/application/security/SessionManager.php
@@ -169,6 +169,9 @@ class SessionManager
      */
     public function hasSessionExpired()
     {
+        if (empty($this->session['expires_on'])) {
+            return true;
+        }
         if (time() >= $this->session['expires_on']) {
             return true;
         }
@@ -188,7 +191,7 @@ class SessionManager
         if ($this->conf->get('security.session_protection_disabled') === true) {
             return false;
         }
-        if ($this->session['ip'] == $clientIpId) {
+        if (isset($this->session['ip']) && $this->session['ip'] === $clientIpId) {
             return false;
         }
         return true;
diff --git a/tests/security/LoginManagerTest.php b/tests/security/LoginManagerTest.php
index fad09992..f26cd1eb 100644
--- a/tests/security/LoginManagerTest.php
+++ b/tests/security/LoginManagerTest.php
@@ -84,10 +84,7 @@ class LoginManagerTest extends TestCase
         $this->globals = &$GLOBALS;
         unset($this->globals['IPBANS']);
 
-        $this->session = [
-            'expires_on' => time() + 100,
-            'ip' => $this->clientIpAddress,
-        ];
+        $this->session = [];
 
         $this->sessionManager = new SessionManager($this->session, $this->configManager);
         $this->loginManager = new LoginManager($this->globals, $this->configManager, $this->sessionManager);
@@ -281,12 +278,18 @@ class LoginManagerTest extends TestCase
      */
     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->assertFalse($this->loginManager->isLoggedIn());
+        $this->assertTrue($this->loginManager->isLoggedIn());
+        $this->assertTrue(empty($this->session['username']));
     }
 
     /**
@@ -300,6 +303,8 @@ class LoginManagerTest extends TestCase
         $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']);
     }
 
     /**
-- 
cgit v1.2.3