From ca5e98da4867f720dc863dac55cd1fa2360068e7 Mon Sep 17 00:00:00 2001 From: ArthurHoaro Date: Tue, 20 Oct 2020 10:39:58 +0200 Subject: [PATCH 1/2] Composer: explicitly import katzgrau/klogger (already included in netscape-bookmark-parser) --- composer.json | 1 + composer.lock | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index c0855e47..64f0025e 100644 --- a/composer.json +++ b/composer.json @@ -23,6 +23,7 @@ "erusev/parsedown": "^1.6", "erusev/parsedown-extra": "^0.8.1", "gettext/gettext": "^4.4", + "katzgrau/klogger": "^1.2", "malkusch/lock": "^2.1", "pubsubhubbub/publisher": "dev-master", "shaarli/netscape-bookmark-parser": "^2.1", diff --git a/composer.lock b/composer.lock index c379d8e7..3c89036f 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "932b191006135ff8be495aa0b4ba7e09", + "content-hash": "61360efbb2e1ba4c4fe00ce1f7a78ec5", "packages": [ { "name": "arthurhoaro/web-thumbnailer", From b38a1b0209f546d4824a0db81a34c4e30fcdebaf Mon Sep 17 00:00:00 2001 From: ArthurHoaro Date: Tue, 20 Oct 2020 11:47:07 +0200 Subject: [PATCH 2/2] Use PSR-3 logger for login attempts Fixes #1122 --- application/Utils.php | 24 ++++--- application/container/ContainerBuilder.php | 10 ++- application/container/ShaarliContainer.php | 2 + .../controller/visitor/LoginController.php | 1 - application/render/PageBuilder.php | 29 +++++--- application/security/BanManager.php | 28 ++++---- application/security/LoginManager.php | 69 ++++++++----------- index.php | 19 ++++- tests/UtilsTest.php | 36 +++------- tests/container/ContainerBuilderTest.php | 5 +- .../visitor/LoginControllerTest.php | 2 +- tests/security/BanManagerTest.php | 3 +- tests/security/LoginManagerTest.php | 51 ++++++++++---- tests/security/SessionManagerTest.php | 5 +- tests/utils/FakeConfigManager.php | 10 ++- 15 files changed, 170 insertions(+), 124 deletions(-) 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 @@ 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 @@ public function __construct( 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 @@ public function build(): ShaarliContainer $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 @@ public function build(): ShaarliContainer 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 @@ namespace Shaarli\Container; +use Psr\Log\LoggerInterface; use Shaarli\Bookmark\BookmarkServiceInterface; use Shaarli\Config\ConfigManager; use Shaarli\Feed\FeedBuilder; @@ -36,6 +37,7 @@ * @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 @@ public function login(Request $request, Response $response): Response } 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 @@ private function initialize() $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 @@ public function handleFailedAttempt($server) 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 @@ public function isBanned($server) 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 @@ public function isLoggedIn(): bool /** * 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 __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 @@ }); } +$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 @@ $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 @@ protected function getLastLogEntry() } /** - * 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 @@ namespace Shaarli\Container; +use Psr\Log\LoggerInterface; use Shaarli\Bookmark\BookmarkServiceInterface; use Shaarli\Config\ConfigManager; use Shaarli\Feed\FeedBuilder; @@ -55,7 +56,8 @@ public function setUp(): void $this->conf, $this->sessionManager, $this->cookieManager, - $this->loginManager + $this->loginManager, + $this->createMock(LoggerInterface::class) ); } @@ -73,6 +75,7 @@ public function testBuildContainer(): void 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 @@ public function testProcessLoginWithValidParameters(): void $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 @@ protected function getNewBanManagerInstance() 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 @@ */ 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 @@ protected function setUp(): void $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 @@ protected function setUp(): void 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 @@ public function testHandleFailedLoginBehindTrustedProxy() '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 @@ public function testGenerateStaySignedInTokenSessionProtectionDisabled() */ 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 @@ public function testCheckLoginStateClientIpChanged() public function testCheckCredentialsWrongLogin() { $this->assertFalse( - $this->loginManager->checkCredentials('', '', 'b4dl0g1n', $this->password) + $this->loginManager->checkCredentials('', 'b4dl0g1n', $this->password) ); } @@ -280,7 +307,7 @@ public function testCheckCredentialsWrongLogin() public function testCheckCredentialsWrongPassword() { $this->assertFalse( - $this->loginManager->checkCredentials('', '', $this->login, 'b4dp455wd') + $this->loginManager->checkCredentials('', $this->login, 'b4dp455wd') ); } @@ -290,7 +317,7 @@ public function testCheckCredentialsWrongPassword() public function testCheckCredentialsWrongLoginAndPassword() { $this->assertFalse( - $this->loginManager->checkCredentials('', '', 'b4dl0g1n', 'b4dp455wd') + $this->loginManager->checkCredentials('', 'b4dl0g1n', 'b4dp455wd') ); } @@ -300,7 +327,7 @@ public function testCheckCredentialsWrongLoginAndPassword() public function testCheckCredentialsGoodLoginAndPassword() { $this->assertTrue( - $this->loginManager->checkCredentials('', '', $this->login, $this->password) + $this->loginManager->checkCredentials('', $this->login, $this->password) ); } @@ -311,7 +338,7 @@ public function testCheckCredentialsFromUnreachableLdap() { $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 @@ public static function setUpBeforeClass(): void */ 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 @@ values[$key] = $value; } @@ -35,7 +39,7 @@ public function set($key, $value) * * @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];