aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorArthurHoaro <arthur@hoa.ro>2020-10-20 11:47:07 +0200
committerArthurHoaro <arthur@hoa.ro>2020-10-20 11:47:07 +0200
commitb38a1b0209f546d4824a0db81a34c4e30fcdebaf (patch)
tree0f812cd69bfc0ba654ce0d4b832850b41cc658fa
parentca5e98da4867f720dc863dac55cd1fa2360068e7 (diff)
downloadShaarli-b38a1b0209f546d4824a0db81a34c4e30fcdebaf.tar.gz
Shaarli-b38a1b0209f546d4824a0db81a34c4e30fcdebaf.tar.zst
Shaarli-b38a1b0209f546d4824a0db81a34c4e30fcdebaf.zip
Use PSR-3 logger for login attempts
Fixes #1122
-rw-r--r--application/Utils.php24
-rw-r--r--application/container/ContainerBuilder.php10
-rw-r--r--application/container/ShaarliContainer.php2
-rw-r--r--application/front/controller/visitor/LoginController.php1
-rw-r--r--application/render/PageBuilder.php29
-rw-r--r--application/security/BanManager.php28
-rw-r--r--application/security/LoginManager.php69
-rw-r--r--index.php19
-rw-r--r--tests/UtilsTest.php36
-rw-r--r--tests/container/ContainerBuilderTest.php5
-rw-r--r--tests/front/controller/visitor/LoginControllerTest.php2
-rw-r--r--tests/security/BanManagerTest.php3
-rw-r--r--tests/security/LoginManagerTest.php51
-rw-r--r--tests/security/SessionManagerTest.php5
-rw-r--r--tests/utils/FakeConfigManager.php10
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 @@
4 */ 4 */
5 5
6/** 6/**
7 * Logs a message to a text file 7 * Format log using provided data.
8 * 8 *
9 * The log format is compatible with fail2ban. 9 * @param string $message the message to log
10 * @param string|null $clientIp the client's remote IPv4/IPv6 address
10 * 11 *
11 * @param string $logFile where to write the logs 12 * @return string Formatted message to log
12 * @param string $clientIp the client's remote IPv4/IPv6 address
13 * @param string $message the message to log
14 */ 13 */
15function logm($logFile, $clientIp, $message) 14function format_log(string $message, string $clientIp = null): string
16{ 15{
17 file_put_contents( 16 $out = $message;
18 $logFile, 17
19 date('Y/m/d H:i:s').' - '.$clientIp.' - '.strval($message).PHP_EOL, 18 if (!empty($clientIp)) {
20 FILE_APPEND 19 // Note: we keep the first dash to avoid breaking fail2ban configs
21 ); 20 $out = '- ' . $clientIp . ' - ' . $out;
21 }
22
23 return $out;
22} 24}
23 25
24/** 26/**
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);
5namespace Shaarli\Container; 5namespace Shaarli\Container;
6 6
7use malkusch\lock\mutex\FlockMutex; 7use malkusch\lock\mutex\FlockMutex;
8use Psr\Log\LoggerInterface;
8use Shaarli\Bookmark\BookmarkFileService; 9use Shaarli\Bookmark\BookmarkFileService;
9use Shaarli\Bookmark\BookmarkServiceInterface; 10use Shaarli\Bookmark\BookmarkServiceInterface;
10use Shaarli\Config\ConfigManager; 11use Shaarli\Config\ConfigManager;
@@ -49,6 +50,9 @@ class ContainerBuilder
49 /** @var LoginManager */ 50 /** @var LoginManager */
50 protected $login; 51 protected $login;
51 52
53 /** @var LoggerInterface */
54 protected $logger;
55
52 /** @var string|null */ 56 /** @var string|null */
53 protected $basePath = null; 57 protected $basePath = null;
54 58
@@ -56,12 +60,14 @@ class ContainerBuilder
56 ConfigManager $conf, 60 ConfigManager $conf,
57 SessionManager $session, 61 SessionManager $session,
58 CookieManager $cookieManager, 62 CookieManager $cookieManager,
59 LoginManager $login 63 LoginManager $login,
64 LoggerInterface $logger
60 ) { 65 ) {
61 $this->conf = $conf; 66 $this->conf = $conf;
62 $this->session = $session; 67 $this->session = $session;
63 $this->login = $login; 68 $this->login = $login;
64 $this->cookieManager = $cookieManager; 69 $this->cookieManager = $cookieManager;
70 $this->logger = $logger;
65 } 71 }
66 72
67 public function build(): ShaarliContainer 73 public function build(): ShaarliContainer
@@ -72,6 +78,7 @@ class ContainerBuilder
72 $container['sessionManager'] = $this->session; 78 $container['sessionManager'] = $this->session;
73 $container['cookieManager'] = $this->cookieManager; 79 $container['cookieManager'] = $this->cookieManager;
74 $container['loginManager'] = $this->login; 80 $container['loginManager'] = $this->login;
81 $container['logger'] = $this->logger;
75 $container['basePath'] = $this->basePath; 82 $container['basePath'] = $this->basePath;
76 83
77 $container['plugins'] = function (ShaarliContainer $container): PluginManager { 84 $container['plugins'] = function (ShaarliContainer $container): PluginManager {
@@ -99,6 +106,7 @@ class ContainerBuilder
99 return new PageBuilder( 106 return new PageBuilder(
100 $container->conf, 107 $container->conf,
101 $container->sessionManager->getSession(), 108 $container->sessionManager->getSession(),
109 $container->logger,
102 $container->bookmarkService, 110 $container->bookmarkService,
103 $container->sessionManager->generateToken(), 111 $container->sessionManager->generateToken(),
104 $container->loginManager->isLoggedIn() 112 $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);
4 4
5namespace Shaarli\Container; 5namespace Shaarli\Container;
6 6
7use Psr\Log\LoggerInterface;
7use Shaarli\Bookmark\BookmarkServiceInterface; 8use Shaarli\Bookmark\BookmarkServiceInterface;
8use Shaarli\Config\ConfigManager; 9use Shaarli\Config\ConfigManager;
9use Shaarli\Feed\FeedBuilder; 10use Shaarli\Feed\FeedBuilder;
@@ -36,6 +37,7 @@ use Slim\Container;
36 * @property History $history 37 * @property History $history
37 * @property HttpAccess $httpAccess 38 * @property HttpAccess $httpAccess
38 * @property LoginManager $loginManager 39 * @property LoginManager $loginManager
40 * @property LoggerInterface $logger
39 * @property MetadataRetriever $metadataRetriever 41 * @property MetadataRetriever $metadataRetriever
40 * @property NetscapeBookmarkUtils $netscapeBookmarkUtils 42 * @property NetscapeBookmarkUtils $netscapeBookmarkUtils
41 * @property callable $notFoundHandler Overrides default Slim exception display 43 * @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
65 } 65 }
66 66
67 if (!$this->container->loginManager->checkCredentials( 67 if (!$this->container->loginManager->checkCredentials(
68 $this->container->environment['REMOTE_ADDR'],
69 client_ip_id($this->container->environment), 68 client_ip_id($this->container->environment),
70 $request->getParam('login'), 69 $request->getParam('login'),
71 $request->getParam('password') 70 $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 @@
3namespace Shaarli\Render; 3namespace Shaarli\Render;
4 4
5use Exception; 5use Exception;
6use exceptions\MissingBasePathException; 6use Psr\Log\LoggerInterface;
7use RainTPL; 7use RainTPL;
8use Shaarli\ApplicationUtils; 8use Shaarli\ApplicationUtils;
9use Shaarli\Bookmark\BookmarkServiceInterface; 9use Shaarli\Bookmark\BookmarkServiceInterface;
@@ -35,6 +35,9 @@ class PageBuilder
35 */ 35 */
36 protected $session; 36 protected $session;
37 37
38 /** @var LoggerInterface */
39 protected $logger;
40
38 /** 41 /**
39 * @var BookmarkServiceInterface $bookmarkService instance. 42 * @var BookmarkServiceInterface $bookmarkService instance.
40 */ 43 */
@@ -54,17 +57,25 @@ class PageBuilder
54 * PageBuilder constructor. 57 * PageBuilder constructor.
55 * $tpl is initialized at false for lazy loading. 58 * $tpl is initialized at false for lazy loading.
56 * 59 *
57 * @param ConfigManager $conf Configuration Manager instance (reference). 60 * @param ConfigManager $conf Configuration Manager instance (reference).
58 * @param array $session $_SESSION array 61 * @param array $session $_SESSION array
59 * @param BookmarkServiceInterface $linkDB instance. 62 * @param LoggerInterface $logger
60 * @param string $token Session token 63 * @param null $linkDB instance.
61 * @param bool $isLoggedIn 64 * @param null $token Session token
65 * @param bool $isLoggedIn
62 */ 66 */
63 public function __construct(&$conf, $session, $linkDB = null, $token = null, $isLoggedIn = false) 67 public function __construct(
64 { 68 ConfigManager &$conf,
69 array $session,
70 LoggerInterface $logger,
71 $linkDB = null,
72 $token = null,
73 $isLoggedIn = false
74 ) {
65 $this->tpl = false; 75 $this->tpl = false;
66 $this->conf = $conf; 76 $this->conf = $conf;
67 $this->session = $session; 77 $this->session = $session;
78 $this->logger = $logger;
68 $this->bookmarkService = $linkDB; 79 $this->bookmarkService = $linkDB;
69 $this->token = $token; 80 $this->token = $token;
70 $this->isLoggedIn = $isLoggedIn; 81 $this->isLoggedIn = $isLoggedIn;
@@ -98,7 +109,7 @@ class PageBuilder
98 $this->tpl->assign('newVersion', escape($version)); 109 $this->tpl->assign('newVersion', escape($version));
99 $this->tpl->assign('versionError', ''); 110 $this->tpl->assign('versionError', '');
100 } catch (Exception $exc) { 111 } catch (Exception $exc) {
101 logm($this->conf->get('resource.log'), $_SERVER['REMOTE_ADDR'], $exc->getMessage()); 112 $this->logger->error(format_log('Error: ' . $exc->getMessage(), client_ip_id($_SERVER)));
102 $this->tpl->assign('newVersion', ''); 113 $this->tpl->assign('newVersion', '');
103 $this->tpl->assign('versionError', escape($exc->getMessage())); 114 $this->tpl->assign('versionError', escape($exc->getMessage()));
104 } 115 }
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 @@
3 3
4namespace Shaarli\Security; 4namespace Shaarli\Security;
5 5
6use Psr\Log\LoggerInterface;
6use Shaarli\FileUtils; 7use Shaarli\FileUtils;
7 8
8/** 9/**
@@ -28,8 +29,8 @@ class BanManager
28 /** @var string Path to the file containing IP bans and failures */ 29 /** @var string Path to the file containing IP bans and failures */
29 protected $banFile; 30 protected $banFile;
30 31
31 /** @var string Path to the log file, used to log bans */ 32 /** @var LoggerInterface Path to the log file, used to log bans */
32 protected $logFile; 33 protected $logger;
33 34
34 /** @var array List of IP with their associated number of failed attempts */ 35 /** @var array List of IP with their associated number of failed attempts */
35 protected $failures = []; 36 protected $failures = [];
@@ -40,18 +41,19 @@ class BanManager
40 /** 41 /**
41 * BanManager constructor. 42 * BanManager constructor.
42 * 43 *
43 * @param array $trustedProxies List of allowed proxies IP 44 * @param array $trustedProxies List of allowed proxies IP
44 * @param int $nbAttempts Number of allowed failed attempt before the ban 45 * @param int $nbAttempts Number of allowed failed attempt before the ban
45 * @param int $banDuration Ban duration in seconds 46 * @param int $banDuration Ban duration in seconds
46 * @param string $banFile Path to the file containing IP bans and failures 47 * @param string $banFile Path to the file containing IP bans and failures
47 * @param string $logFile Path to the log file, used to log bans 48 * @param LoggerInterface $logger PSR-3 logger to save login attempts in log directory
48 */ 49 */
49 public function __construct($trustedProxies, $nbAttempts, $banDuration, $banFile, $logFile) { 50 public function __construct($trustedProxies, $nbAttempts, $banDuration, $banFile, LoggerInterface $logger) {
50 $this->trustedProxies = $trustedProxies; 51 $this->trustedProxies = $trustedProxies;
51 $this->nbAttempts = $nbAttempts; 52 $this->nbAttempts = $nbAttempts;
52 $this->banDuration = $banDuration; 53 $this->banDuration = $banDuration;
53 $this->banFile = $banFile; 54 $this->banFile = $banFile;
54 $this->logFile = $logFile; 55 $this->logger = $logger;
56
55 $this->readBanFile(); 57 $this->readBanFile();
56 } 58 }
57 59
@@ -78,11 +80,7 @@ class BanManager
78 80
79 if ($this->failures[$ip] >= $this->nbAttempts) { 81 if ($this->failures[$ip] >= $this->nbAttempts) {
80 $this->bans[$ip] = time() + $this->banDuration; 82 $this->bans[$ip] = time() + $this->banDuration;
81 logm( 83 $this->logger->info(format_log('IP address banned from login: '. $ip, $ip));
82 $this->logFile,
83 $server['REMOTE_ADDR'],
84 'IP address banned from login: '. $ip
85 );
86 } 84 }
87 $this->writeBanFile(); 85 $this->writeBanFile();
88 } 86 }
@@ -138,7 +136,7 @@ class BanManager
138 unset($this->failures[$ip]); 136 unset($this->failures[$ip]);
139 } 137 }
140 unset($this->bans[$ip]); 138 unset($this->bans[$ip]);
141 logm($this->logFile, $server['REMOTE_ADDR'], 'Ban lifted for: '. $ip); 139 $this->logger->info(format_log('Ban lifted for: '. $ip, $ip));
142 140
143 $this->writeBanFile(); 141 $this->writeBanFile();
144 return false; 142 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 @@
2namespace Shaarli\Security; 2namespace Shaarli\Security;
3 3
4use Exception; 4use Exception;
5use Psr\Log\LoggerInterface;
5use Shaarli\Config\ConfigManager; 6use Shaarli\Config\ConfigManager;
6 7
7/** 8/**
@@ -31,26 +32,30 @@ class LoginManager
31 protected $staySignedInToken = ''; 32 protected $staySignedInToken = '';
32 /** @var CookieManager */ 33 /** @var CookieManager */
33 protected $cookieManager; 34 protected $cookieManager;
35 /** @var LoggerInterface */
36 protected $logger;
34 37
35 /** 38 /**
36 * Constructor 39 * Constructor
37 * 40 *
38 * @param ConfigManager $configManager Configuration Manager instance 41 * @param ConfigManager $configManager Configuration Manager instance
39 * @param SessionManager $sessionManager SessionManager instance 42 * @param SessionManager $sessionManager SessionManager instance
40 * @param CookieManager $cookieManager CookieManager instance 43 * @param CookieManager $cookieManager CookieManager instance
44 * @param BanManager $banManager
45 * @param LoggerInterface $logger Used to log login attempts
41 */ 46 */
42 public function __construct($configManager, $sessionManager, $cookieManager) 47 public function __construct(
43 { 48 ConfigManager $configManager,
49 SessionManager $sessionManager,
50 CookieManager $cookieManager,
51 BanManager $banManager,
52 LoggerInterface $logger
53 ) {
44 $this->configManager = $configManager; 54 $this->configManager = $configManager;
45 $this->sessionManager = $sessionManager; 55 $this->sessionManager = $sessionManager;
46 $this->cookieManager = $cookieManager; 56 $this->cookieManager = $cookieManager;
47 $this->banManager = new BanManager( 57 $this->banManager = $banManager;
48 $this->configManager->get('security.trusted_proxies', []), 58 $this->logger = $logger;
49 $this->configManager->get('security.ban_after'),
50 $this->configManager->get('security.ban_duration'),
51 $this->configManager->get('resource.ban_file', 'data/ipbans.php'),
52 $this->configManager->get('resource.log')
53 );
54 59
55 if ($this->configManager->get('security.open_shaarli') === true) { 60 if ($this->configManager->get('security.open_shaarli') === true) {
56 $this->openShaarli = true; 61 $this->openShaarli = true;
@@ -129,48 +134,34 @@ class LoginManager
129 /** 134 /**
130 * Check user credentials are valid 135 * Check user credentials are valid
131 * 136 *
132 * @param string $remoteIp Remote client IP address
133 * @param string $clientIpId Client IP address identifier 137 * @param string $clientIpId Client IP address identifier
134 * @param string $login Username 138 * @param string $login Username
135 * @param string $password Password 139 * @param string $password Password
136 * 140 *
137 * @return bool true if the provided credentials are valid, false otherwise 141 * @return bool true if the provided credentials are valid, false otherwise
138 */ 142 */
139 public function checkCredentials($remoteIp, $clientIpId, $login, $password) 143 public function checkCredentials($clientIpId, $login, $password)
140 { 144 {
141 // Check login matches config
142 if ($login !== $this->configManager->get('credentials.login')) {
143 return false;
144 }
145
146 // Check credentials 145 // Check credentials
147 try { 146 try {
148 $useLdapLogin = !empty($this->configManager->get('ldap.host')); 147 $useLdapLogin = !empty($this->configManager->get('ldap.host'));
149 if ((false === $useLdapLogin && $this->checkCredentialsFromLocalConfig($login, $password)) 148 if ($login === $this->configManager->get('credentials.login')
150 || (true === $useLdapLogin && $this->checkCredentialsFromLdap($login, $password)) 149 && (
150 (false === $useLdapLogin && $this->checkCredentialsFromLocalConfig($login, $password))
151 || (true === $useLdapLogin && $this->checkCredentialsFromLdap($login, $password))
152 )
151 ) { 153 ) {
152 $this->sessionManager->storeLoginInfo($clientIpId); 154 $this->sessionManager->storeLoginInfo($clientIpId);
153 logm( 155 $this->logger->info(format_log('Login successful', $clientIpId));
154 $this->configManager->get('resource.log'), 156
155 $remoteIp, 157 return true;
156 'Login successful'
157 );
158 return true;
159 } 158 }
160 } 159 } catch(Exception $exception) {
161 catch(Exception $exception) { 160 $this->logger->info(format_log('Exception while checking credentials: ' . $exception, $clientIpId));
162 logm(
163 $this->configManager->get('resource.log'),
164 $remoteIp,
165 'Exception while checking credentials: ' . $exception
166 );
167 } 161 }
168 162
169 logm( 163 $this->logger->info(format_log('Login failed for user ' . $login, $clientIpId));
170 $this->configManager->get('resource.log'), 164
171 $remoteIp,
172 'Login failed for user ' . $login
173 );
174 return false; 165 return false;
175 } 166 }
176 167
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';
25 25
26require_once __DIR__ . '/init.php'; 26require_once __DIR__ . '/init.php';
27 27
28use Katzgrau\KLogger\Logger;
29use Psr\Log\LogLevel;
28use Shaarli\Config\ConfigManager; 30use Shaarli\Config\ConfigManager;
29use Shaarli\Container\ContainerBuilder; 31use Shaarli\Container\ContainerBuilder;
30use Shaarli\Languages; 32use Shaarli\Languages;
33use Shaarli\Security\BanManager;
31use Shaarli\Security\CookieManager; 34use Shaarli\Security\CookieManager;
32use Shaarli\Security\LoginManager; 35use Shaarli\Security\LoginManager;
33use Shaarli\Security\SessionManager; 36use Shaarli\Security\SessionManager;
@@ -48,10 +51,22 @@ if ($conf->get('dev.debug', false)) {
48 }); 51 });
49} 52}
50 53
54$logger = new Logger(
55 dirname($conf->get('resource.log')),
56 !$conf->get('dev.debug') ? LogLevel::INFO : LogLevel::DEBUG,
57 ['filename' => basename($conf->get('resource.log'))]
58);
51$sessionManager = new SessionManager($_SESSION, $conf, session_save_path()); 59$sessionManager = new SessionManager($_SESSION, $conf, session_save_path());
52$sessionManager->initialize(); 60$sessionManager->initialize();
53$cookieManager = new CookieManager($_COOKIE); 61$cookieManager = new CookieManager($_COOKIE);
54$loginManager = new LoginManager($conf, $sessionManager, $cookieManager); 62$banManager = new BanManager(
63 $conf->get('security.trusted_proxies', []),
64 $conf->get('security.ban_after'),
65 $conf->get('security.ban_duration'),
66 $conf->get('resource.ban_file', 'data/ipbans.php'),
67 $logger
68);
69$loginManager = new LoginManager($conf, $sessionManager, $cookieManager, $banManager, $logger);
55$loginManager->generateStaySignedInToken($_SERVER['REMOTE_ADDR']); 70$loginManager->generateStaySignedInToken($_SERVER['REMOTE_ADDR']);
56 71
57// Sniff browser language and set date format accordingly. 72// Sniff browser language and set date format accordingly.
@@ -71,7 +86,7 @@ date_default_timezone_set($conf->get('general.timezone', 'UTC'));
71 86
72$loginManager->checkLoginState(client_ip_id($_SERVER)); 87$loginManager->checkLoginState(client_ip_id($_SERVER));
73 88
74$containerBuilder = new ContainerBuilder($conf, $sessionManager, $cookieManager, $loginManager); 89$containerBuilder = new ContainerBuilder($conf, $sessionManager, $cookieManager, $loginManager, $logger);
75$container = $containerBuilder->build(); 90$container = $containerBuilder->build();
76$app = new App($container); 91$app = new App($container);
77 92
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
63 } 63 }
64 64
65 /** 65 /**
66 * Log a message to a file - IPv4 client address 66 * Format a log a message - IPv4 client address
67 */ 67 */
68 public function testLogmIp4() 68 public function testFormatLogIp4()
69 { 69 {
70 $logMessage = 'IPv4 client connected'; 70 $message = 'IPv4 client connected';
71 logm(self::$testLogFile, '127.0.0.1', $logMessage); 71 $log = format_log($message, '127.0.0.1');
72 list($date, $ip, $message) = $this->getLastLogEntry();
73 72
74 $this->assertInstanceOf( 73 static::assertSame('- 127.0.0.1 - IPv4 client connected', $log);
75 'DateTime',
76 DateTime::createFromFormat(self::$dateFormat, $date)
77 );
78 $this->assertTrue(
79 filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4) !== false
80 );
81 $this->assertEquals($logMessage, $message);
82 } 74 }
83 75
84 /** 76 /**
85 * Log a message to a file - IPv6 client address 77 * Format a log a message - IPv6 client address
86 */ 78 */
87 public function testLogmIp6() 79 public function testFormatLogIp6()
88 { 80 {
89 $logMessage = 'IPv6 client connected'; 81 $message = 'IPv6 client connected';
90 logm(self::$testLogFile, '2001:db8::ff00:42:8329', $logMessage); 82 $log = format_log($message, '2001:db8::ff00:42:8329');
91 list($date, $ip, $message) = $this->getLastLogEntry();
92 83
93 $this->assertInstanceOf( 84 static::assertSame('- 2001:db8::ff00:42:8329 - IPv6 client connected', $log);
94 'DateTime',
95 DateTime::createFromFormat(self::$dateFormat, $date)
96 );
97 $this->assertTrue(
98 filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6) !== false
99 );
100 $this->assertEquals($logMessage, $message);
101 } 85 }
102 86
103 /** 87 /**
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);
4 4
5namespace Shaarli\Container; 5namespace Shaarli\Container;
6 6
7use Psr\Log\LoggerInterface;
7use Shaarli\Bookmark\BookmarkServiceInterface; 8use Shaarli\Bookmark\BookmarkServiceInterface;
8use Shaarli\Config\ConfigManager; 9use Shaarli\Config\ConfigManager;
9use Shaarli\Feed\FeedBuilder; 10use Shaarli\Feed\FeedBuilder;
@@ -55,7 +56,8 @@ class ContainerBuilderTest extends TestCase
55 $this->conf, 56 $this->conf,
56 $this->sessionManager, 57 $this->sessionManager,
57 $this->cookieManager, 58 $this->cookieManager,
58 $this->loginManager 59 $this->loginManager,
60 $this->createMock(LoggerInterface::class)
59 ); 61 );
60 } 62 }
61 63
@@ -73,6 +75,7 @@ class ContainerBuilderTest extends TestCase
73 static::assertInstanceOf(History::class, $container->history); 75 static::assertInstanceOf(History::class, $container->history);
74 static::assertInstanceOf(HttpAccess::class, $container->httpAccess); 76 static::assertInstanceOf(HttpAccess::class, $container->httpAccess);
75 static::assertInstanceOf(LoginManager::class, $container->loginManager); 77 static::assertInstanceOf(LoginManager::class, $container->loginManager);
78 static::assertInstanceOf(LoggerInterface::class, $container->logger);
76 static::assertInstanceOf(MetadataRetriever::class, $container->metadataRetriever); 79 static::assertInstanceOf(MetadataRetriever::class, $container->metadataRetriever);
77 static::assertInstanceOf(NetscapeBookmarkUtils::class, $container->netscapeBookmarkUtils); 80 static::assertInstanceOf(NetscapeBookmarkUtils::class, $container->netscapeBookmarkUtils);
78 static::assertInstanceOf(PageBuilder::class, $container->pageBuilder); 81 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
195 $this->container->loginManager 195 $this->container->loginManager
196 ->expects(static::once()) 196 ->expects(static::once())
197 ->method('checkCredentials') 197 ->method('checkCredentials')
198 ->with('1.2.3.4', '1.2.3.4', 'bob', 'pass') 198 ->with('1.2.3.4', 'bob', 'pass')
199 ->willReturn(true) 199 ->willReturn(true)
200 ; 200 ;
201 $this->container->loginManager->method('getStaySignedInToken')->willReturn(bin2hex(random_bytes(8))); 201 $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 @@
3 3
4namespace Shaarli\Security; 4namespace Shaarli\Security;
5 5
6use Psr\Log\LoggerInterface;
6use Shaarli\FileUtils; 7use Shaarli\FileUtils;
7use Shaarli\TestCase; 8use Shaarli\TestCase;
8 9
@@ -387,7 +388,7 @@ class BanManagerTest extends TestCase
387 3, 388 3,
388 1800, 389 1800,
389 $this->banFile, 390 $this->banFile,
390 $this->logFile 391 $this->createMock(LoggerInterface::class)
391 ); 392 );
392 } 393 }
393} 394}
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 @@
2 2
3namespace Shaarli\Security; 3namespace Shaarli\Security;
4 4
5use Psr\Log\LoggerInterface;
6use Shaarli\FakeConfigManager;
5use Shaarli\TestCase; 7use Shaarli\TestCase;
6 8
7/** 9/**
@@ -9,7 +11,7 @@ use Shaarli\TestCase;
9 */ 11 */
10class LoginManagerTest extends TestCase 12class LoginManagerTest extends TestCase
11{ 13{
12 /** @var \FakeConfigManager Configuration Manager instance */ 14 /** @var FakeConfigManager Configuration Manager instance */
13 protected $configManager = null; 15 protected $configManager = null;
14 16
15 /** @var LoginManager Login Manager instance */ 17 /** @var LoginManager Login Manager instance */
@@ -60,6 +62,9 @@ class LoginManagerTest extends TestCase
60 /** @var CookieManager */ 62 /** @var CookieManager */
61 protected $cookieManager; 63 protected $cookieManager;
62 64
65 /** @var BanManager */
66 protected $banManager;
67
63 /** 68 /**
64 * Prepare or reset test resources 69 * Prepare or reset test resources
65 */ 70 */
@@ -71,7 +76,7 @@ class LoginManagerTest extends TestCase
71 76
72 $this->passwordHash = sha1($this->password . $this->login . $this->salt); 77 $this->passwordHash = sha1($this->password . $this->login . $this->salt);
73 78
74 $this->configManager = new \FakeConfigManager([ 79 $this->configManager = new FakeConfigManager([
75 'credentials.login' => $this->login, 80 'credentials.login' => $this->login,
76 'credentials.hash' => $this->passwordHash, 81 'credentials.hash' => $this->passwordHash,
77 'credentials.salt' => $this->salt, 82 'credentials.salt' => $this->salt,
@@ -91,18 +96,29 @@ class LoginManagerTest extends TestCase
91 return $this->cookie[$key] ?? null; 96 return $this->cookie[$key] ?? null;
92 }); 97 });
93 $this->sessionManager = new SessionManager($this->session, $this->configManager, 'session_path'); 98 $this->sessionManager = new SessionManager($this->session, $this->configManager, 'session_path');
94 $this->loginManager = new LoginManager($this->configManager, $this->sessionManager, $this->cookieManager); 99 $this->banManager = $this->createMock(BanManager::class);
100 $this->loginManager = new LoginManager(
101 $this->configManager,
102 $this->sessionManager,
103 $this->cookieManager,
104 $this->banManager,
105 $this->createMock(LoggerInterface::class)
106 );
95 $this->server['REMOTE_ADDR'] = $this->ipAddr; 107 $this->server['REMOTE_ADDR'] = $this->ipAddr;
96 } 108 }
97 109
98 /** 110 /**
99 * Record a failed login attempt 111 * Record a failed login attempt
100 */ 112 */
101 public function testHandleFailedLogin() 113 public function testHandleFailedLogin(): void
102 { 114 {
115 $this->banManager->expects(static::exactly(2))->method('handleFailedAttempt');
116 $this->banManager->method('isBanned')->willReturn(true);
117
103 $this->loginManager->handleFailedLogin($this->server); 118 $this->loginManager->handleFailedLogin($this->server);
104 $this->loginManager->handleFailedLogin($this->server); 119 $this->loginManager->handleFailedLogin($this->server);
105 $this->assertFalse($this->loginManager->canLogin($this->server)); 120
121 static::assertFalse($this->loginManager->canLogin($this->server));
106 } 122 }
107 123
108 /** 124 /**
@@ -114,8 +130,13 @@ class LoginManagerTest extends TestCase
114 'REMOTE_ADDR' => $this->trustedProxy, 130 'REMOTE_ADDR' => $this->trustedProxy,
115 'HTTP_X_FORWARDED_FOR' => $this->ipAddr, 131 'HTTP_X_FORWARDED_FOR' => $this->ipAddr,
116 ]; 132 ];
133
134 $this->banManager->expects(static::exactly(2))->method('handleFailedAttempt');
135 $this->banManager->method('isBanned')->willReturn(true);
136
117 $this->loginManager->handleFailedLogin($server); 137 $this->loginManager->handleFailedLogin($server);
118 $this->loginManager->handleFailedLogin($server); 138 $this->loginManager->handleFailedLogin($server);
139
119 $this->assertFalse($this->loginManager->canLogin($server)); 140 $this->assertFalse($this->loginManager->canLogin($server));
120 } 141 }
121 142
@@ -196,10 +217,16 @@ class LoginManagerTest extends TestCase
196 */ 217 */
197 public function testCheckLoginStateNotConfigured() 218 public function testCheckLoginStateNotConfigured()
198 { 219 {
199 $configManager = new \FakeConfigManager([ 220 $configManager = new FakeConfigManager([
200 'resource.ban_file' => $this->banFile, 221 'resource.ban_file' => $this->banFile,
201 ]); 222 ]);
202 $loginManager = new LoginManager($configManager, null, $this->cookieManager); 223 $loginManager = new LoginManager(
224 $configManager,
225 $this->sessionManager,
226 $this->cookieManager,
227 $this->banManager,
228 $this->createMock(LoggerInterface::class)
229 );
203 $loginManager->checkLoginState(''); 230 $loginManager->checkLoginState('');
204 231
205 $this->assertFalse($loginManager->isLoggedIn()); 232 $this->assertFalse($loginManager->isLoggedIn());
@@ -270,7 +297,7 @@ class LoginManagerTest extends TestCase
270 public function testCheckCredentialsWrongLogin() 297 public function testCheckCredentialsWrongLogin()
271 { 298 {
272 $this->assertFalse( 299 $this->assertFalse(
273 $this->loginManager->checkCredentials('', '', 'b4dl0g1n', $this->password) 300 $this->loginManager->checkCredentials('', 'b4dl0g1n', $this->password)
274 ); 301 );
275 } 302 }
276 303
@@ -280,7 +307,7 @@ class LoginManagerTest extends TestCase
280 public function testCheckCredentialsWrongPassword() 307 public function testCheckCredentialsWrongPassword()
281 { 308 {
282 $this->assertFalse( 309 $this->assertFalse(
283 $this->loginManager->checkCredentials('', '', $this->login, 'b4dp455wd') 310 $this->loginManager->checkCredentials('', $this->login, 'b4dp455wd')
284 ); 311 );
285 } 312 }
286 313
@@ -290,7 +317,7 @@ class LoginManagerTest extends TestCase
290 public function testCheckCredentialsWrongLoginAndPassword() 317 public function testCheckCredentialsWrongLoginAndPassword()
291 { 318 {
292 $this->assertFalse( 319 $this->assertFalse(
293 $this->loginManager->checkCredentials('', '', 'b4dl0g1n', 'b4dp455wd') 320 $this->loginManager->checkCredentials('', 'b4dl0g1n', 'b4dp455wd')
294 ); 321 );
295 } 322 }
296 323
@@ -300,7 +327,7 @@ class LoginManagerTest extends TestCase
300 public function testCheckCredentialsGoodLoginAndPassword() 327 public function testCheckCredentialsGoodLoginAndPassword()
301 { 328 {
302 $this->assertTrue( 329 $this->assertTrue(
303 $this->loginManager->checkCredentials('', '', $this->login, $this->password) 330 $this->loginManager->checkCredentials('', $this->login, $this->password)
304 ); 331 );
305 } 332 }
306 333
@@ -311,7 +338,7 @@ class LoginManagerTest extends TestCase
311 { 338 {
312 $this->configManager->set('ldap.host', 'dummy'); 339 $this->configManager->set('ldap.host', 'dummy');
313 $this->assertFalse( 340 $this->assertFalse(
314 $this->loginManager->checkCredentials('', '', $this->login, $this->password) 341 $this->loginManager->checkCredentials('', $this->login, $this->password)
315 ); 342 );
316 } 343 }
317 344
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 @@
2 2
3namespace Shaarli\Security; 3namespace Shaarli\Security;
4 4
5use Shaarli\FakeConfigManager;
5use Shaarli\TestCase; 6use Shaarli\TestCase;
6 7
7/** 8/**
@@ -12,7 +13,7 @@ class SessionManagerTest extends TestCase
12 /** @var array Session ID hashes */ 13 /** @var array Session ID hashes */
13 protected static $sidHashes = null; 14 protected static $sidHashes = null;
14 15
15 /** @var \FakeConfigManager ConfigManager substitute for testing */ 16 /** @var FakeConfigManager ConfigManager substitute for testing */
16 protected $conf = null; 17 protected $conf = null;
17 18
18 /** @var array $_SESSION array for testing */ 19 /** @var array $_SESSION array for testing */
@@ -34,7 +35,7 @@ class SessionManagerTest extends TestCase
34 */ 35 */
35 protected function setUp(): void 36 protected function setUp(): void
36 { 37 {
37 $this->conf = new \FakeConfigManager([ 38 $this->conf = new FakeConfigManager([
38 'credentials.login' => 'johndoe', 39 'credentials.login' => 'johndoe',
39 'credentials.salt' => 'salt', 40 'credentials.salt' => 'salt',
40 'security.session_protection_disabled' => false, 41 '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 @@
1<?php 1<?php
2 2
3namespace Shaarli;
4
5use Shaarli\Config\ConfigManager;
6
3/** 7/**
4 * Fake ConfigManager 8 * Fake ConfigManager
5 */ 9 */
6class FakeConfigManager 10class FakeConfigManager extends ConfigManager
7{ 11{
8 protected $values = []; 12 protected $values = [];
9 13
@@ -23,7 +27,7 @@ class FakeConfigManager
23 * @param string $key Key of the value to set 27 * @param string $key Key of the value to set
24 * @param mixed $value Value to set 28 * @param mixed $value Value to set
25 */ 29 */
26 public function set($key, $value) 30 public function set($key, $value, $write = false, $isLoggedIn = false)
27 { 31 {
28 $this->values[$key] = $value; 32 $this->values[$key] = $value;
29 } 33 }
@@ -35,7 +39,7 @@ class FakeConfigManager
35 * 39 *
36 * @return mixed The value if set, else the name of the key 40 * @return mixed The value if set, else the name of the key
37 */ 41 */
38 public function get($key) 42 public function get($key, $default = '')
39 { 43 {
40 if (isset($this->values[$key])) { 44 if (isset($this->values[$key])) {
41 return $this->values[$key]; 45 return $this->values[$key];