diff options
-rw-r--r-- | application/Utils.php | 24 | ||||
-rw-r--r-- | application/container/ContainerBuilder.php | 10 | ||||
-rw-r--r-- | application/container/ShaarliContainer.php | 2 | ||||
-rw-r--r-- | application/front/controller/visitor/LoginController.php | 1 | ||||
-rw-r--r-- | application/render/PageBuilder.php | 29 | ||||
-rw-r--r-- | application/security/BanManager.php | 28 | ||||
-rw-r--r-- | application/security/LoginManager.php | 69 | ||||
-rw-r--r-- | index.php | 19 | ||||
-rw-r--r-- | tests/UtilsTest.php | 36 | ||||
-rw-r--r-- | tests/container/ContainerBuilderTest.php | 5 | ||||
-rw-r--r-- | tests/front/controller/visitor/LoginControllerTest.php | 2 | ||||
-rw-r--r-- | tests/security/BanManagerTest.php | 3 | ||||
-rw-r--r-- | tests/security/LoginManagerTest.php | 51 | ||||
-rw-r--r-- | tests/security/SessionManagerTest.php | 5 | ||||
-rw-r--r-- | 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 @@ | |||
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 | */ |
15 | function logm($logFile, $clientIp, $message) | 14 | function 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); | |||
5 | namespace Shaarli\Container; | 5 | namespace Shaarli\Container; |
6 | 6 | ||
7 | use malkusch\lock\mutex\FlockMutex; | 7 | use malkusch\lock\mutex\FlockMutex; |
8 | use Psr\Log\LoggerInterface; | ||
8 | use Shaarli\Bookmark\BookmarkFileService; | 9 | use Shaarli\Bookmark\BookmarkFileService; |
9 | use Shaarli\Bookmark\BookmarkServiceInterface; | 10 | use Shaarli\Bookmark\BookmarkServiceInterface; |
10 | use Shaarli\Config\ConfigManager; | 11 | use 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 | ||
5 | namespace Shaarli\Container; | 5 | namespace Shaarli\Container; |
6 | 6 | ||
7 | use Psr\Log\LoggerInterface; | ||
7 | use Shaarli\Bookmark\BookmarkServiceInterface; | 8 | use Shaarli\Bookmark\BookmarkServiceInterface; |
8 | use Shaarli\Config\ConfigManager; | 9 | use Shaarli\Config\ConfigManager; |
9 | use Shaarli\Feed\FeedBuilder; | 10 | use 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 @@ | |||
3 | namespace Shaarli\Render; | 3 | namespace Shaarli\Render; |
4 | 4 | ||
5 | use Exception; | 5 | use Exception; |
6 | use exceptions\MissingBasePathException; | 6 | use Psr\Log\LoggerInterface; |
7 | use RainTPL; | 7 | use RainTPL; |
8 | use Shaarli\ApplicationUtils; | 8 | use Shaarli\ApplicationUtils; |
9 | use Shaarli\Bookmark\BookmarkServiceInterface; | 9 | use 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 | ||
4 | namespace Shaarli\Security; | 4 | namespace Shaarli\Security; |
5 | 5 | ||
6 | use Psr\Log\LoggerInterface; | ||
6 | use Shaarli\FileUtils; | 7 | use 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 @@ | |||
2 | namespace Shaarli\Security; | 2 | namespace Shaarli\Security; |
3 | 3 | ||
4 | use Exception; | 4 | use Exception; |
5 | use Psr\Log\LoggerInterface; | ||
5 | use Shaarli\Config\ConfigManager; | 6 | use 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 | ||
@@ -25,9 +25,12 @@ require_once 'application/Utils.php'; | |||
25 | 25 | ||
26 | require_once __DIR__ . '/init.php'; | 26 | require_once __DIR__ . '/init.php'; |
27 | 27 | ||
28 | use Katzgrau\KLogger\Logger; | ||
29 | use Psr\Log\LogLevel; | ||
28 | use Shaarli\Config\ConfigManager; | 30 | use Shaarli\Config\ConfigManager; |
29 | use Shaarli\Container\ContainerBuilder; | 31 | use Shaarli\Container\ContainerBuilder; |
30 | use Shaarli\Languages; | 32 | use Shaarli\Languages; |
33 | use Shaarli\Security\BanManager; | ||
31 | use Shaarli\Security\CookieManager; | 34 | use Shaarli\Security\CookieManager; |
32 | use Shaarli\Security\LoginManager; | 35 | use Shaarli\Security\LoginManager; |
33 | use Shaarli\Security\SessionManager; | 36 | use 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 | ||
5 | namespace Shaarli\Container; | 5 | namespace Shaarli\Container; |
6 | 6 | ||
7 | use Psr\Log\LoggerInterface; | ||
7 | use Shaarli\Bookmark\BookmarkServiceInterface; | 8 | use Shaarli\Bookmark\BookmarkServiceInterface; |
8 | use Shaarli\Config\ConfigManager; | 9 | use Shaarli\Config\ConfigManager; |
9 | use Shaarli\Feed\FeedBuilder; | 10 | use 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 | ||
4 | namespace Shaarli\Security; | 4 | namespace Shaarli\Security; |
5 | 5 | ||
6 | use Psr\Log\LoggerInterface; | ||
6 | use Shaarli\FileUtils; | 7 | use Shaarli\FileUtils; |
7 | use Shaarli\TestCase; | 8 | use 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 | ||
3 | namespace Shaarli\Security; | 3 | namespace Shaarli\Security; |
4 | 4 | ||
5 | use Psr\Log\LoggerInterface; | ||
6 | use Shaarli\FakeConfigManager; | ||
5 | use Shaarli\TestCase; | 7 | use Shaarli\TestCase; |
6 | 8 | ||
7 | /** | 9 | /** |
@@ -9,7 +11,7 @@ use Shaarli\TestCase; | |||
9 | */ | 11 | */ |
10 | class LoginManagerTest extends TestCase | 12 | class 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 | ||
3 | namespace Shaarli\Security; | 3 | namespace Shaarli\Security; |
4 | 4 | ||
5 | use Shaarli\FakeConfigManager; | ||
5 | use Shaarli\TestCase; | 6 | use 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 | ||
3 | namespace Shaarli; | ||
4 | |||
5 | use Shaarli\Config\ConfigManager; | ||
6 | |||
3 | /** | 7 | /** |
4 | * Fake ConfigManager | 8 | * Fake ConfigManager |
5 | */ | 9 | */ |
6 | class FakeConfigManager | 10 | class 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]; |