From: ArthurHoaro <arthur@hoa.ro>
Date: Tue, 20 Oct 2020 09:47:07 +0000 (+0200)
Subject: Use PSR-3 logger for login attempts
X-Git-Tag: v0.12.1^2~28^2
X-Git-Url: https://git.immae.eu/?p=github%2Fshaarli%2FShaarli.git;a=commitdiff_plain;h=b38a1b0209f546d4824a0db81a34c4e30fcdebaf

Use PSR-3 logger for login attempts

Fixes #1122
---

diff --git a/application/Utils.php b/application/Utils.php
index bcfda65c..7a9d2645 100644
--- a/application/Utils.php
+++ b/application/Utils.php
@@ -4,21 +4,23 @@
  */
 
 /**
- * Logs a message to a text file
+ * Format log using provided data.
  *
- * The log format is compatible with fail2ban.
+ * @param string      $message  the message to log
+ * @param string|null $clientIp the client's remote IPv4/IPv6 address
  *
- * @param string $logFile  where to write the logs
- * @param string $clientIp the client's remote IPv4/IPv6 address
- * @param string $message  the message to log
+ * @return string Formatted message to log
  */
-function logm($logFile, $clientIp, $message)
+function format_log(string $message, string $clientIp = null): string
 {
-    file_put_contents(
-        $logFile,
-        date('Y/m/d H:i:s').' - '.$clientIp.' - '.strval($message).PHP_EOL,
-        FILE_APPEND
-    );
+    $out = $message;
+
+    if (!empty($clientIp)) {
+        // Note: we keep the first dash to avoid breaking fail2ban configs
+        $out = '- ' . $clientIp . ' - ' . $out;
+    }
+
+    return $out;
 }
 
 /**
diff --git a/application/container/ContainerBuilder.php b/application/container/ContainerBuilder.php
index fd94a1c3..d84418ad 100644
--- a/application/container/ContainerBuilder.php
+++ b/application/container/ContainerBuilder.php
@@ -5,6 +5,7 @@ declare(strict_types=1);
 namespace Shaarli\Container;
 
 use malkusch\lock\mutex\FlockMutex;
+use Psr\Log\LoggerInterface;
 use Shaarli\Bookmark\BookmarkFileService;
 use Shaarli\Bookmark\BookmarkServiceInterface;
 use Shaarli\Config\ConfigManager;
@@ -49,6 +50,9 @@ class ContainerBuilder
     /** @var LoginManager */
     protected $login;
 
+    /** @var LoggerInterface */
+    protected $logger;
+
     /** @var string|null */
     protected $basePath = null;
 
@@ -56,12 +60,14 @@ class ContainerBuilder
         ConfigManager $conf,
         SessionManager $session,
         CookieManager $cookieManager,
-        LoginManager $login
+        LoginManager $login,
+        LoggerInterface $logger
     ) {
         $this->conf = $conf;
         $this->session = $session;
         $this->login = $login;
         $this->cookieManager = $cookieManager;
+        $this->logger = $logger;
     }
 
     public function build(): ShaarliContainer
@@ -72,6 +78,7 @@ class ContainerBuilder
         $container['sessionManager'] = $this->session;
         $container['cookieManager'] = $this->cookieManager;
         $container['loginManager'] = $this->login;
+        $container['logger'] = $this->logger;
         $container['basePath'] = $this->basePath;
 
         $container['plugins'] = function (ShaarliContainer $container): PluginManager {
@@ -99,6 +106,7 @@ class ContainerBuilder
             return new PageBuilder(
                 $container->conf,
                 $container->sessionManager->getSession(),
+                $container->logger,
                 $container->bookmarkService,
                 $container->sessionManager->generateToken(),
                 $container->loginManager->isLoggedIn()
diff --git a/application/container/ShaarliContainer.php b/application/container/ShaarliContainer.php
index 3a7c238f..3e5bd252 100644
--- a/application/container/ShaarliContainer.php
+++ b/application/container/ShaarliContainer.php
@@ -4,6 +4,7 @@ declare(strict_types=1);
 
 namespace Shaarli\Container;
 
+use Psr\Log\LoggerInterface;
 use Shaarli\Bookmark\BookmarkServiceInterface;
 use Shaarli\Config\ConfigManager;
 use Shaarli\Feed\FeedBuilder;
@@ -36,6 +37,7 @@ use Slim\Container;
  * @property History                  $history
  * @property HttpAccess               $httpAccess
  * @property LoginManager             $loginManager
+ * @property LoggerInterface          $logger
  * @property MetadataRetriever        $metadataRetriever
  * @property NetscapeBookmarkUtils    $netscapeBookmarkUtils
  * @property callable                 $notFoundHandler       Overrides default Slim exception display
diff --git a/application/front/controller/visitor/LoginController.php b/application/front/controller/visitor/LoginController.php
index 121ba40b..f5038fe3 100644
--- a/application/front/controller/visitor/LoginController.php
+++ b/application/front/controller/visitor/LoginController.php
@@ -65,7 +65,6 @@ class LoginController extends ShaarliVisitorController
         }
 
         if (!$this->container->loginManager->checkCredentials(
-                $this->container->environment['REMOTE_ADDR'],
                 client_ip_id($this->container->environment),
                 $request->getParam('login'),
                 $request->getParam('password')
diff --git a/application/render/PageBuilder.php b/application/render/PageBuilder.php
index 2d6d2dbe..512bb79e 100644
--- a/application/render/PageBuilder.php
+++ b/application/render/PageBuilder.php
@@ -3,7 +3,7 @@
 namespace Shaarli\Render;
 
 use Exception;
-use exceptions\MissingBasePathException;
+use Psr\Log\LoggerInterface;
 use RainTPL;
 use Shaarli\ApplicationUtils;
 use Shaarli\Bookmark\BookmarkServiceInterface;
@@ -35,6 +35,9 @@ class PageBuilder
      */
     protected $session;
 
+    /** @var LoggerInterface */
+    protected $logger;
+
     /**
      * @var BookmarkServiceInterface $bookmarkService instance.
      */
@@ -54,17 +57,25 @@ class PageBuilder
      * PageBuilder constructor.
      * $tpl is initialized at false for lazy loading.
      *
-     * @param ConfigManager            $conf    Configuration Manager instance (reference).
-     * @param array                    $session $_SESSION array
-     * @param BookmarkServiceInterface $linkDB  instance.
-     * @param string                   $token   Session token
-     * @param bool                     $isLoggedIn
+     * @param ConfigManager $conf Configuration Manager instance (reference).
+     * @param array $session $_SESSION array
+     * @param LoggerInterface $logger
+     * @param null $linkDB instance.
+     * @param null $token Session token
+     * @param bool $isLoggedIn
      */
-    public function __construct(&$conf, $session, $linkDB = null, $token = null, $isLoggedIn = false)
-    {
+    public function __construct(
+        ConfigManager &$conf,
+        array $session,
+        LoggerInterface $logger,
+        $linkDB = null,
+        $token = null,
+        $isLoggedIn = false
+    ) {
         $this->tpl = false;
         $this->conf = $conf;
         $this->session = $session;
+        $this->logger = $logger;
         $this->bookmarkService = $linkDB;
         $this->token = $token;
         $this->isLoggedIn = $isLoggedIn;
@@ -98,7 +109,7 @@ class PageBuilder
             $this->tpl->assign('newVersion', escape($version));
             $this->tpl->assign('versionError', '');
         } catch (Exception $exc) {
-            logm($this->conf->get('resource.log'), $_SERVER['REMOTE_ADDR'], $exc->getMessage());
+            $this->logger->error(format_log('Error: ' . $exc->getMessage(), client_ip_id($_SERVER)));
             $this->tpl->assign('newVersion', '');
             $this->tpl->assign('versionError', escape($exc->getMessage()));
         }
diff --git a/application/security/BanManager.php b/application/security/BanManager.php
index 68190c54..f72c8b7b 100644
--- a/application/security/BanManager.php
+++ b/application/security/BanManager.php
@@ -3,6 +3,7 @@
 
 namespace Shaarli\Security;
 
+use Psr\Log\LoggerInterface;
 use Shaarli\FileUtils;
 
 /**
@@ -28,8 +29,8 @@ class BanManager
     /** @var string Path to the file containing IP bans and failures */
     protected $banFile;
 
-    /** @var string Path to the log file, used to log bans */
-    protected $logFile;
+    /** @var LoggerInterface Path to the log file, used to log bans */
+    protected $logger;
 
     /** @var array List of IP with their associated number of failed attempts */
     protected $failures = [];
@@ -40,18 +41,19 @@ class BanManager
     /**
      * BanManager constructor.
      *
-     * @param array  $trustedProxies List of allowed proxies IP
-     * @param int    $nbAttempts     Number of allowed failed attempt before the ban
-     * @param int    $banDuration    Ban duration in seconds
-     * @param string $banFile        Path to the file containing IP bans and failures
-     * @param string $logFile        Path to the log file, used to log bans
+     * @param array           $trustedProxies List of allowed proxies IP
+     * @param int             $nbAttempts     Number of allowed failed attempt before the ban
+     * @param int             $banDuration    Ban duration in seconds
+     * @param string          $banFile        Path to the file containing IP bans and failures
+     * @param LoggerInterface $logger         PSR-3 logger to save login attempts in log directory
      */
-    public function __construct($trustedProxies, $nbAttempts, $banDuration, $banFile, $logFile) {
+    public function __construct($trustedProxies, $nbAttempts, $banDuration, $banFile, LoggerInterface $logger) {
         $this->trustedProxies = $trustedProxies;
         $this->nbAttempts = $nbAttempts;
         $this->banDuration = $banDuration;
         $this->banFile = $banFile;
-        $this->logFile = $logFile;
+        $this->logger = $logger;
+
         $this->readBanFile();
     }
 
@@ -78,11 +80,7 @@ class BanManager
 
         if ($this->failures[$ip] >= $this->nbAttempts) {
             $this->bans[$ip] = time() + $this->banDuration;
-            logm(
-                $this->logFile,
-                $server['REMOTE_ADDR'],
-                'IP address banned from login: '. $ip
-            );
+            $this->logger->info(format_log('IP address banned from login: '. $ip, $ip));
         }
         $this->writeBanFile();
     }
@@ -138,7 +136,7 @@ class BanManager
             unset($this->failures[$ip]);
         }
         unset($this->bans[$ip]);
-        logm($this->logFile, $server['REMOTE_ADDR'], 'Ban lifted for: '. $ip);
+        $this->logger->info(format_log('Ban lifted for: '. $ip, $ip));
 
         $this->writeBanFile();
         return false;
diff --git a/application/security/LoginManager.php b/application/security/LoginManager.php
index 65048f10..426e785e 100644
--- a/application/security/LoginManager.php
+++ b/application/security/LoginManager.php
@@ -2,6 +2,7 @@
 namespace Shaarli\Security;
 
 use Exception;
+use Psr\Log\LoggerInterface;
 use Shaarli\Config\ConfigManager;
 
 /**
@@ -31,26 +32,30 @@ class LoginManager
     protected $staySignedInToken = '';
     /** @var CookieManager */
     protected $cookieManager;
+    /** @var LoggerInterface */
+    protected $logger;
 
     /**
      * Constructor
      *
-     * @param ConfigManager  $configManager  Configuration Manager instance
+     * @param ConfigManager $configManager Configuration Manager instance
      * @param SessionManager $sessionManager SessionManager instance
-     * @param CookieManager  $cookieManager  CookieManager instance
+     * @param CookieManager $cookieManager CookieManager instance
+     * @param BanManager $banManager
+     * @param LoggerInterface $logger Used to log login attempts
      */
-    public function __construct($configManager, $sessionManager, $cookieManager)
-    {
+    public function __construct(
+        ConfigManager $configManager,
+        SessionManager $sessionManager,
+        CookieManager $cookieManager,
+        BanManager $banManager,
+        LoggerInterface $logger
+    ) {
         $this->configManager = $configManager;
         $this->sessionManager = $sessionManager;
         $this->cookieManager = $cookieManager;
-        $this->banManager = new BanManager(
-            $this->configManager->get('security.trusted_proxies', []),
-            $this->configManager->get('security.ban_after'),
-            $this->configManager->get('security.ban_duration'),
-            $this->configManager->get('resource.ban_file', 'data/ipbans.php'),
-            $this->configManager->get('resource.log')
-        );
+        $this->banManager = $banManager;
+        $this->logger = $logger;
 
         if ($this->configManager->get('security.open_shaarli') === true) {
             $this->openShaarli = true;
@@ -129,48 +134,34 @@ class LoginManager
     /**
      * 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)
+    public function checkCredentials($clientIpId, $login, $password)
     {
-        // Check login matches config
-        if ($login !== $this->configManager->get('credentials.login')) {
-            return false;
-        }
-
         // Check credentials
         try {
             $useLdapLogin = !empty($this->configManager->get('ldap.host'));
-            if ((false === $useLdapLogin && $this->checkCredentialsFromLocalConfig($login, $password))
-                || (true === $useLdapLogin && $this->checkCredentialsFromLdap($login, $password))
+            if ($login === $this->configManager->get('credentials.login')
+                && (
+                    (false === $useLdapLogin && $this->checkCredentialsFromLocalConfig($login, $password))
+                    || (true === $useLdapLogin && $this->checkCredentialsFromLdap($login, $password))
+                )
             ) {
-                    $this->sessionManager->storeLoginInfo($clientIpId);
-                    logm(
-                        $this->configManager->get('resource.log'),
-                        $remoteIp,
-                        'Login successful'
-                    );
-                    return true;
+                $this->sessionManager->storeLoginInfo($clientIpId);
+                $this->logger->info(format_log('Login successful', $clientIpId));
+
+                return true;
             }
-        }
-        catch(Exception $exception) {
-            logm(
-                $this->configManager->get('resource.log'),
-                $remoteIp,
-                'Exception while checking credentials: ' . $exception
-            );
+        } catch(Exception $exception) {
+            $this->logger->info(format_log('Exception while checking credentials: ' . $exception, $clientIpId));
         }
 
-        logm(
-            $this->configManager->get('resource.log'),
-            $remoteIp,
-            'Login failed for user ' . $login
-        );
+        $this->logger->info(format_log('Login failed for user ' . $login, $clientIpId));
+
         return false;
     }
 
diff --git a/index.php b/index.php
index 220847f5..ea6e8501 100644
--- a/index.php
+++ b/index.php
@@ -25,9 +25,12 @@ require_once 'application/Utils.php';
 
 require_once __DIR__ . '/init.php';
 
+use Katzgrau\KLogger\Logger;
+use Psr\Log\LogLevel;
 use Shaarli\Config\ConfigManager;
 use Shaarli\Container\ContainerBuilder;
 use Shaarli\Languages;
+use Shaarli\Security\BanManager;
 use Shaarli\Security\CookieManager;
 use Shaarli\Security\LoginManager;
 use Shaarli\Security\SessionManager;
@@ -48,10 +51,22 @@ if ($conf->get('dev.debug', false)) {
     });
 }
 
+$logger = new Logger(
+    dirname($conf->get('resource.log')),
+    !$conf->get('dev.debug') ? LogLevel::INFO : LogLevel::DEBUG,
+    ['filename' => basename($conf->get('resource.log'))]
+);
 $sessionManager = new SessionManager($_SESSION, $conf, session_save_path());
 $sessionManager->initialize();
 $cookieManager = new CookieManager($_COOKIE);
-$loginManager = new LoginManager($conf, $sessionManager, $cookieManager);
+$banManager = new BanManager(
+    $conf->get('security.trusted_proxies', []),
+    $conf->get('security.ban_after'),
+    $conf->get('security.ban_duration'),
+    $conf->get('resource.ban_file', 'data/ipbans.php'),
+    $logger
+);
+$loginManager = new LoginManager($conf, $sessionManager, $cookieManager, $banManager, $logger);
 $loginManager->generateStaySignedInToken($_SERVER['REMOTE_ADDR']);
 
 // Sniff browser language and set date format accordingly.
@@ -71,7 +86,7 @@ date_default_timezone_set($conf->get('general.timezone', 'UTC'));
 
 $loginManager->checkLoginState(client_ip_id($_SERVER));
 
-$containerBuilder = new ContainerBuilder($conf, $sessionManager, $cookieManager, $loginManager);
+$containerBuilder = new ContainerBuilder($conf, $sessionManager, $cookieManager, $loginManager, $logger);
 $container = $containerBuilder->build();
 $app = new App($container);
 
diff --git a/tests/UtilsTest.php b/tests/UtilsTest.php
index 6e787d7f..59dca75f 100644
--- a/tests/UtilsTest.php
+++ b/tests/UtilsTest.php
@@ -63,41 +63,25 @@ class UtilsTest extends \Shaarli\TestCase
     }
 
     /**
-     * Log a message to a file - IPv4 client address
+     * Format a log a message - IPv4 client address
      */
-    public function testLogmIp4()
+    public function testFormatLogIp4()
     {
-        $logMessage = 'IPv4 client connected';
-        logm(self::$testLogFile, '127.0.0.1', $logMessage);
-        list($date, $ip, $message) = $this->getLastLogEntry();
+        $message = 'IPv4 client connected';
+        $log = format_log($message, '127.0.0.1');
 
-        $this->assertInstanceOf(
-            'DateTime',
-            DateTime::createFromFormat(self::$dateFormat, $date)
-        );
-        $this->assertTrue(
-            filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4) !== false
-        );
-        $this->assertEquals($logMessage, $message);
+        static::assertSame('- 127.0.0.1 - IPv4 client connected', $log);
     }
 
     /**
-     * Log a message to a file - IPv6 client address
+     * Format a log a message - IPv6 client address
      */
-    public function testLogmIp6()
+    public function testFormatLogIp6()
     {
-        $logMessage = 'IPv6 client connected';
-        logm(self::$testLogFile, '2001:db8::ff00:42:8329', $logMessage);
-        list($date, $ip, $message) = $this->getLastLogEntry();
+        $message = 'IPv6 client connected';
+        $log = format_log($message, '2001:db8::ff00:42:8329');
 
-        $this->assertInstanceOf(
-            'DateTime',
-            DateTime::createFromFormat(self::$dateFormat, $date)
-        );
-        $this->assertTrue(
-            filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6) !== false
-        );
-        $this->assertEquals($logMessage, $message);
+        static::assertSame('- 2001:db8::ff00:42:8329 - IPv6 client connected', $log);
     }
 
     /**
diff --git a/tests/container/ContainerBuilderTest.php b/tests/container/ContainerBuilderTest.php
index 3dadc0b9..3d43c344 100644
--- a/tests/container/ContainerBuilderTest.php
+++ b/tests/container/ContainerBuilderTest.php
@@ -4,6 +4,7 @@ declare(strict_types=1);
 
 namespace Shaarli\Container;
 
+use Psr\Log\LoggerInterface;
 use Shaarli\Bookmark\BookmarkServiceInterface;
 use Shaarli\Config\ConfigManager;
 use Shaarli\Feed\FeedBuilder;
@@ -55,7 +56,8 @@ class ContainerBuilderTest extends TestCase
             $this->conf,
             $this->sessionManager,
             $this->cookieManager,
-            $this->loginManager
+            $this->loginManager,
+            $this->createMock(LoggerInterface::class)
         );
     }
 
@@ -73,6 +75,7 @@ class ContainerBuilderTest extends TestCase
         static::assertInstanceOf(History::class, $container->history);
         static::assertInstanceOf(HttpAccess::class, $container->httpAccess);
         static::assertInstanceOf(LoginManager::class, $container->loginManager);
+        static::assertInstanceOf(LoggerInterface::class, $container->logger);
         static::assertInstanceOf(MetadataRetriever::class, $container->metadataRetriever);
         static::assertInstanceOf(NetscapeBookmarkUtils::class, $container->netscapeBookmarkUtils);
         static::assertInstanceOf(PageBuilder::class, $container->pageBuilder);
diff --git a/tests/front/controller/visitor/LoginControllerTest.php b/tests/front/controller/visitor/LoginControllerTest.php
index 1312ccb7..00d9eab3 100644
--- a/tests/front/controller/visitor/LoginControllerTest.php
+++ b/tests/front/controller/visitor/LoginControllerTest.php
@@ -195,7 +195,7 @@ class LoginControllerTest extends TestCase
         $this->container->loginManager
             ->expects(static::once())
             ->method('checkCredentials')
-            ->with('1.2.3.4', '1.2.3.4', 'bob', 'pass')
+            ->with('1.2.3.4', 'bob', 'pass')
             ->willReturn(true)
         ;
         $this->container->loginManager->method('getStaySignedInToken')->willReturn(bin2hex(random_bytes(8)));
diff --git a/tests/security/BanManagerTest.php b/tests/security/BanManagerTest.php
index 698d3d10..22aa8666 100644
--- a/tests/security/BanManagerTest.php
+++ b/tests/security/BanManagerTest.php
@@ -3,6 +3,7 @@
 
 namespace Shaarli\Security;
 
+use Psr\Log\LoggerInterface;
 use Shaarli\FileUtils;
 use Shaarli\TestCase;
 
@@ -387,7 +388,7 @@ class BanManagerTest extends TestCase
             3,
             1800,
             $this->banFile,
-            $this->logFile
+            $this->createMock(LoggerInterface::class)
         );
     }
 }
diff --git a/tests/security/LoginManagerTest.php b/tests/security/LoginManagerTest.php
index d302983d..f7609fc6 100644
--- a/tests/security/LoginManagerTest.php
+++ b/tests/security/LoginManagerTest.php
@@ -2,6 +2,8 @@
 
 namespace Shaarli\Security;
 
+use Psr\Log\LoggerInterface;
+use Shaarli\FakeConfigManager;
 use Shaarli\TestCase;
 
 /**
@@ -9,7 +11,7 @@ use Shaarli\TestCase;
  */
 class LoginManagerTest extends TestCase
 {
-    /** @var \FakeConfigManager Configuration Manager instance */
+    /** @var FakeConfigManager Configuration Manager instance */
     protected $configManager = null;
 
     /** @var LoginManager Login Manager instance */
@@ -60,6 +62,9 @@ class LoginManagerTest extends TestCase
     /** @var CookieManager */
     protected $cookieManager;
 
+    /** @var BanManager */
+    protected $banManager;
+
     /**
      * Prepare or reset test resources
      */
@@ -71,7 +76,7 @@ class LoginManagerTest extends TestCase
 
         $this->passwordHash = sha1($this->password . $this->login . $this->salt);
 
-        $this->configManager = new \FakeConfigManager([
+        $this->configManager = new FakeConfigManager([
             'credentials.login' => $this->login,
             'credentials.hash' => $this->passwordHash,
             'credentials.salt' => $this->salt,
@@ -91,18 +96,29 @@ class LoginManagerTest extends TestCase
             return $this->cookie[$key] ?? null;
         });
         $this->sessionManager = new SessionManager($this->session, $this->configManager, 'session_path');
-        $this->loginManager = new LoginManager($this->configManager, $this->sessionManager, $this->cookieManager);
+        $this->banManager = $this->createMock(BanManager::class);
+        $this->loginManager = new LoginManager(
+            $this->configManager,
+            $this->sessionManager,
+            $this->cookieManager,
+            $this->banManager,
+            $this->createMock(LoggerInterface::class)
+        );
         $this->server['REMOTE_ADDR'] = $this->ipAddr;
     }
 
     /**
      * Record a failed login attempt
      */
-    public function testHandleFailedLogin()
+    public function testHandleFailedLogin(): void
     {
+        $this->banManager->expects(static::exactly(2))->method('handleFailedAttempt');
+        $this->banManager->method('isBanned')->willReturn(true);
+
         $this->loginManager->handleFailedLogin($this->server);
         $this->loginManager->handleFailedLogin($this->server);
-        $this->assertFalse($this->loginManager->canLogin($this->server));
+
+        static::assertFalse($this->loginManager->canLogin($this->server));
     }
 
     /**
@@ -114,8 +130,13 @@ class LoginManagerTest extends TestCase
             'REMOTE_ADDR' => $this->trustedProxy,
             'HTTP_X_FORWARDED_FOR' => $this->ipAddr,
         ];
+
+        $this->banManager->expects(static::exactly(2))->method('handleFailedAttempt');
+        $this->banManager->method('isBanned')->willReturn(true);
+
         $this->loginManager->handleFailedLogin($server);
         $this->loginManager->handleFailedLogin($server);
+
         $this->assertFalse($this->loginManager->canLogin($server));
     }
 
@@ -196,10 +217,16 @@ class LoginManagerTest extends TestCase
      */
     public function testCheckLoginStateNotConfigured()
     {
-        $configManager = new \FakeConfigManager([
+        $configManager = new FakeConfigManager([
             'resource.ban_file' => $this->banFile,
         ]);
-        $loginManager = new LoginManager($configManager, null, $this->cookieManager);
+        $loginManager = new LoginManager(
+            $configManager,
+            $this->sessionManager,
+            $this->cookieManager,
+            $this->banManager,
+            $this->createMock(LoggerInterface::class)
+        );
         $loginManager->checkLoginState('');
 
         $this->assertFalse($loginManager->isLoggedIn());
@@ -270,7 +297,7 @@ class LoginManagerTest extends TestCase
     public function testCheckCredentialsWrongLogin()
     {
         $this->assertFalse(
-            $this->loginManager->checkCredentials('', '', 'b4dl0g1n', $this->password)
+            $this->loginManager->checkCredentials('', 'b4dl0g1n', $this->password)
         );
     }
 
@@ -280,7 +307,7 @@ class LoginManagerTest extends TestCase
     public function testCheckCredentialsWrongPassword()
     {
         $this->assertFalse(
-            $this->loginManager->checkCredentials('', '', $this->login, 'b4dp455wd')
+            $this->loginManager->checkCredentials('', $this->login, 'b4dp455wd')
         );
     }
 
@@ -290,7 +317,7 @@ class LoginManagerTest extends TestCase
     public function testCheckCredentialsWrongLoginAndPassword()
     {
         $this->assertFalse(
-            $this->loginManager->checkCredentials('', '', 'b4dl0g1n', 'b4dp455wd')
+            $this->loginManager->checkCredentials('', 'b4dl0g1n', 'b4dp455wd')
         );
     }
 
@@ -300,7 +327,7 @@ class LoginManagerTest extends TestCase
     public function testCheckCredentialsGoodLoginAndPassword()
     {
         $this->assertTrue(
-            $this->loginManager->checkCredentials('', '', $this->login, $this->password)
+            $this->loginManager->checkCredentials('', $this->login, $this->password)
         );
     }
 
@@ -311,7 +338,7 @@ class LoginManagerTest extends TestCase
     {
         $this->configManager->set('ldap.host', 'dummy');
         $this->assertFalse(
-            $this->loginManager->checkCredentials('', '', $this->login, $this->password)
+            $this->loginManager->checkCredentials('', $this->login, $this->password)
         );
     }
 
diff --git a/tests/security/SessionManagerTest.php b/tests/security/SessionManagerTest.php
index 3f9c3ef5..6830d714 100644
--- a/tests/security/SessionManagerTest.php
+++ b/tests/security/SessionManagerTest.php
@@ -2,6 +2,7 @@
 
 namespace Shaarli\Security;
 
+use Shaarli\FakeConfigManager;
 use Shaarli\TestCase;
 
 /**
@@ -12,7 +13,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 */
@@ -34,7 +35,7 @@ class SessionManagerTest extends TestCase
      */
     protected function setUp(): void
     {
-        $this->conf = new \FakeConfigManager([
+        $this->conf = new FakeConfigManager([
             'credentials.login' => 'johndoe',
             'credentials.salt' => 'salt',
             'security.session_protection_disabled' => false,
diff --git a/tests/utils/FakeConfigManager.php b/tests/utils/FakeConfigManager.php
index 360b34a9..014c2af0 100644
--- a/tests/utils/FakeConfigManager.php
+++ b/tests/utils/FakeConfigManager.php
@@ -1,9 +1,13 @@
 <?php
 
+namespace Shaarli;
+
+use Shaarli\Config\ConfigManager;
+
 /**
  * Fake ConfigManager
  */
-class FakeConfigManager
+class FakeConfigManager extends ConfigManager
 {
     protected $values = [];
 
@@ -23,7 +27,7 @@ class FakeConfigManager
      * @param string $key   Key of the value to set
      * @param mixed  $value Value to set
      */
-    public function set($key, $value)
+    public function set($key, $value, $write = false, $isLoggedIn = false)
     {
         $this->values[$key] = $value;
     }
@@ -35,7 +39,7 @@ class FakeConfigManager
      *
      * @return mixed The value if set, else the name of the key
      */
-    public function get($key)
+    public function get($key, $default = '')
     {
         if (isset($this->values[$key])) {
             return $this->values[$key];