From b38a1b0209f546d4824a0db81a34c4e30fcdebaf Mon Sep 17 00:00:00 2001 From: ArthurHoaro Date: Tue, 20 Oct 2020 11:47:07 +0200 Subject: Use PSR-3 logger for login attempts Fixes #1122 --- application/security/BanManager.php | 28 +++++++------- application/security/LoginManager.php | 69 +++++++++++++++-------------------- 2 files changed, 43 insertions(+), 54 deletions(-) (limited to 'application/security') 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; } -- cgit v1.2.3