From b49a04f796b9ad8533c53fee541132a4c2214909 Mon Sep 17 00:00:00 2001 From: ArthurHoaro Date: Sat, 9 Feb 2019 16:44:48 +0100 Subject: Rewrite IP ban management This adds a dedicated manager class to handle all ban interactions, which is instantiated and handled by LoginManager. IPs are now stored in the same format as the datastore, through FileUtils. Fixes #1032 #587 --- application/security/BanManager.php | 213 ++++++++++++++++++ application/security/LoginManager.php | 100 ++------- doc/md/Server-configuration.md | 2 + index.php | 2 +- tests/security/BanManagerTest.php | 393 ++++++++++++++++++++++++++++++++++ tests/security/LoginManagerTest.php | 104 +-------- 6 files changed, 630 insertions(+), 184 deletions(-) create mode 100644 application/security/BanManager.php create mode 100644 tests/security/BanManagerTest.php diff --git a/application/security/BanManager.php b/application/security/BanManager.php new file mode 100644 index 00000000..68190c54 --- /dev/null +++ b/application/security/BanManager.php @@ -0,0 +1,213 @@ +trustedProxies = $trustedProxies; + $this->nbAttempts = $nbAttempts; + $this->banDuration = $banDuration; + $this->banFile = $banFile; + $this->logFile = $logFile; + $this->readBanFile(); + } + + /** + * Handle a failed login and ban the IP after too many failed attempts + * + * @param array $server The $_SERVER array + */ + public function handleFailedAttempt($server) + { + $ip = $this->getIp($server); + // the IP is behind a trusted forward proxy, but is not forwarded + // in the HTTP headers, so we do nothing + if (empty($ip)) { + return; + } + + // increment the fail count for this IP + if (isset($this->failures[$ip])) { + $this->failures[$ip]++; + } else { + $this->failures[$ip] = 1; + } + + 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->writeBanFile(); + } + + /** + * Remove failed attempts for the provided client. + * + * @param array $server $_SERVER + */ + public function clearFailures($server) + { + $ip = $this->getIp($server); + // the IP is behind a trusted forward proxy, but is not forwarded + // in the HTTP headers, so we do nothing + if (empty($ip)) { + return; + } + + if (isset($this->failures[$ip])) { + unset($this->failures[$ip]); + } + $this->writeBanFile(); + } + + /** + * Check whether the client IP is banned or not. + * + * @param array $server $_SERVER + * + * @return bool True if the IP is banned, false otherwise + */ + public function isBanned($server) + { + $ip = $this->getIp($server); + // the IP is behind a trusted forward proxy, but is not forwarded + // in the HTTP headers, so we allow the authentication attempt. + if (empty($ip)) { + return false; + } + + // the user is not banned + if (! isset($this->bans[$ip])) { + return false; + } + + // the user is still banned + if ($this->bans[$ip] > time()) { + return true; + } + + // the ban has expired, the user can attempt to log in again + if (isset($this->failures[$ip])) { + unset($this->failures[$ip]); + } + unset($this->bans[$ip]); + logm($this->logFile, $server['REMOTE_ADDR'], 'Ban lifted for: '. $ip); + + $this->writeBanFile(); + return false; + } + + /** + * Retrieve the IP from $_SERVER. + * If the actual IP is behind an allowed reverse proxy, + * we try to extract the forwarded IP from HTTP headers. + * + * @param array $server $_SERVER + * + * @return string|bool The IP or false if none could be extracted + */ + protected function getIp($server) + { + $ip = $server['REMOTE_ADDR']; + if (! in_array($ip, $this->trustedProxies)) { + return $ip; + } + return getIpAddressFromProxy($server, $this->trustedProxies); + } + + /** + * Read a file containing banned IPs + */ + protected function readBanFile() + { + $data = FileUtils::readFlatDB($this->banFile); + if (isset($data['failures']) && is_array($data['failures'])) { + $this->failures = $data['failures']; + } + + if (isset($data['bans']) && is_array($data['bans'])) { + $this->bans = $data['bans']; + } + } + + /** + * Write the banned IPs to a file + */ + protected function writeBanFile() + { + return FileUtils::writeFlatDB( + $this->banFile, + [ + 'failures' => $this->failures, + 'bans' => $this->bans, + ] + ); + } + + /** + * Get the Failures (for UT purpose). + * + * @return array + */ + public function getFailures() + { + return $this->failures; + } + + /** + * Get the Bans (for UT purpose). + * + * @return array + */ + public function getBans() + { + return $this->bans; + } +} diff --git a/application/security/LoginManager.php b/application/security/LoginManager.php index 1ff3d0be..0b0ce0b1 100644 --- a/application/security/LoginManager.php +++ b/application/security/LoginManager.php @@ -20,8 +20,8 @@ class LoginManager /** @var SessionManager Session Manager instance **/ protected $sessionManager = null; - /** @var string Path to the file containing IP bans */ - protected $banFile = ''; + /** @var BanManager Ban Manager instance **/ + protected $banManager; /** @var bool Whether the user is logged in **/ protected $isLoggedIn = false; @@ -35,17 +35,21 @@ class LoginManager /** * Constructor * - * @param array $globals The $GLOBALS array (reference) * @param ConfigManager $configManager Configuration Manager instance * @param SessionManager $sessionManager SessionManager instance */ - public function __construct(& $globals, $configManager, $sessionManager) + public function __construct($configManager, $sessionManager) { - $this->globals = &$globals; $this->configManager = $configManager; $this->sessionManager = $sessionManager; - $this->banFile = $this->configManager->get('resource.ban_file', 'data/ipbans.php'); - $this->readBanFile(); + $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') + ); + if ($this->configManager->get('security.open_shaarli') === true) { $this->openShaarli = true; } @@ -157,31 +161,6 @@ class LoginManager return true; } - /** - * Read a file containing banned IPs - */ - protected function readBanFile() - { - if (! file_exists($this->banFile)) { - return; - } - include $this->banFile; - } - - /** - * Write the banned IPs to a file - */ - protected function writeBanFile() - { - if (! array_key_exists('IPBANS', $this->globals)) { - return; - } - file_put_contents( - $this->banFile, - "globals['IPBANS'], true) . ";\n?>" - ); - } - /** * Handle a failed login and ban the IP after too many failed attempts * @@ -189,34 +168,7 @@ class LoginManager */ public function handleFailedLogin($server) { - $ip = $server['REMOTE_ADDR']; - $trusted = $this->configManager->get('security.trusted_proxies', []); - - if (in_array($ip, $trusted)) { - $ip = getIpAddressFromProxy($server, $trusted); - if (! $ip) { - // the IP is behind a trusted forward proxy, but is not forwarded - // in the HTTP headers, so we do nothing - return; - } - } - - // increment the fail count for this IP - if (isset($this->globals['IPBANS']['FAILURES'][$ip])) { - $this->globals['IPBANS']['FAILURES'][$ip]++; - } else { - $this->globals['IPBANS']['FAILURES'][$ip] = 1; - } - - if ($this->globals['IPBANS']['FAILURES'][$ip] >= $this->configManager->get('security.ban_after')) { - $this->globals['IPBANS']['BANS'][$ip] = time() + $this->configManager->get('security.ban_duration', 1800); - logm( - $this->configManager->get('resource.log'), - $server['REMOTE_ADDR'], - 'IP address banned from login' - ); - } - $this->writeBanFile(); + $this->banManager->handleFailedAttempt($server); } /** @@ -226,13 +178,7 @@ class LoginManager */ public function handleSuccessfulLogin($server) { - $ip = $server['REMOTE_ADDR']; - // FIXME unban when behind a trusted proxy? - - unset($this->globals['IPBANS']['FAILURES'][$ip]); - unset($this->globals['IPBANS']['BANS'][$ip]); - - $this->writeBanFile(); + $this->banManager->clearFailures($server); } /** @@ -244,24 +190,6 @@ class LoginManager */ public function canLogin($server) { - $ip = $server['REMOTE_ADDR']; - - if (! isset($this->globals['IPBANS']['BANS'][$ip])) { - // the user is not banned - return true; - } - - if ($this->globals['IPBANS']['BANS'][$ip] > time()) { - // the user is still banned - return false; - } - - // the ban has expired, the user can attempt to log in again - logm($this->configManager->get('resource.log'), $server['REMOTE_ADDR'], 'Ban lifted.'); - unset($this->globals['IPBANS']['FAILURES'][$ip]); - unset($this->globals['IPBANS']['BANS'][$ip]); - - $this->writeBanFile(); - return true; + return ! $this->banManager->isBanned($server); } } diff --git a/doc/md/Server-configuration.md b/doc/md/Server-configuration.md index 78083a46..0930a489 100644 --- a/doc/md/Server-configuration.md +++ b/doc/md/Server-configuration.md @@ -404,6 +404,8 @@ If Shaarli is served behind a proxy (i.e. there is a proxy server between client - `X-Forwarded-Host` - `X-Forwarded-For` +In you [Shaarli configuration](Shaarli-configuration) `data/config.json.php`, add the public IP of your proxy under `security.trusted_proxies`. + See also [proxy-related](https://github.com/shaarli/Shaarli/issues?utf8=%E2%9C%93&q=label%3Aproxy+) issues. ## Robots and crawlers diff --git a/index.php b/index.php index 633ab89e..a27681dc 100644 --- a/index.php +++ b/index.php @@ -125,7 +125,7 @@ if (isset($_COOKIE['shaarli']) && !SessionManager::checkId($_COOKIE['shaarli'])) $conf = new ConfigManager(); $sessionManager = new SessionManager($_SESSION, $conf); -$loginManager = new LoginManager($GLOBALS, $conf, $sessionManager); +$loginManager = new LoginManager($conf, $sessionManager); $loginManager->generateStaySignedInToken($_SERVER['REMOTE_ADDR']); $clientIpId = client_ip_id($_SERVER); diff --git a/tests/security/BanManagerTest.php b/tests/security/BanManagerTest.php new file mode 100644 index 00000000..bba7c8ad --- /dev/null +++ b/tests/security/BanManagerTest.php @@ -0,0 +1,393 @@ +banFile)) { + unlink($this->banFile); + } + + $this->banManager = $this->getNewBanManagerInstance(); + $this->server['REMOTE_ADDR'] = $this->ipAddr; + } + + /** + * Test constructor with initial file. + */ + public function testInstantiateFromFile() + { + $time = time() + 10; + FileUtils::writeFlatDB( + $this->banFile, + [ + 'failures' => [ + $this->ipAddr => 2, + $ip = '1.2.3.4' => 1, + ], + 'bans' => [ + $ip2 = '8.8.8.8' => $time, + $ip3 = '1.1.1.1' => $time + 1, + ], + ] + ); + $this->banManager = $this->getNewBanManagerInstance(); + + $this->assertCount(2, $this->banManager->getFailures()); + $this->assertEquals(2, $this->banManager->getFailures()[$this->ipAddr]); + $this->assertEquals(1, $this->banManager->getFailures()[$ip]); + $this->assertCount(2, $this->banManager->getBans()); + $this->assertEquals($time, $this->banManager->getBans()[$ip2]); + $this->assertEquals($time + 1, $this->banManager->getBans()[$ip3]); + } + + /** + * Test constructor with initial file with invalid values + */ + public function testInstantiateFromCrappyFile() + { + FileUtils::writeFlatDB($this->banFile, 'plop'); + $this->banManager = $this->getNewBanManagerInstance(); + + $this->assertEquals([], $this->banManager->getFailures()); + $this->assertEquals([], $this->banManager->getBans()); + } + + /** + * Test failed attempt with a direct IP. + */ + public function testHandleFailedAttempt() + { + $this->assertCount(0, $this->banManager->getFailures()); + + $this->banManager->handleFailedAttempt($this->server); + $this->assertCount(1, $this->banManager->getFailures()); + $this->assertEquals(1, $this->banManager->getFailures()[$this->ipAddr]); + + $this->banManager->handleFailedAttempt($this->server); + $this->assertCount(1, $this->banManager->getFailures()); + $this->assertEquals(2, $this->banManager->getFailures()[$this->ipAddr]); + } + + /** + * Test failed attempt behind a trusted proxy IP (with proper IP forwarding). + */ + public function testHandleFailedAttemptBehingProxy() + { + $server = [ + 'REMOTE_ADDR' => $this->trustedProxy, + 'HTTP_X_FORWARDED_FOR' => $this->ipAddr, + ]; + $this->assertCount(0, $this->banManager->getFailures()); + + $this->banManager->handleFailedAttempt($server); + $this->assertCount(1, $this->banManager->getFailures()); + $this->assertEquals(1, $this->banManager->getFailures()[$this->ipAddr]); + + $this->banManager->handleFailedAttempt($server); + $this->assertCount(1, $this->banManager->getFailures()); + $this->assertEquals(2, $this->banManager->getFailures()[$this->ipAddr]); + } + + /** + * Test failed attempt behind a trusted proxy IP but without IP forwarding. + */ + public function testHandleFailedAttemptBehindNotConfiguredProxy() + { + $server = [ + 'REMOTE_ADDR' => $this->trustedProxy, + ]; + $this->assertCount(0, $this->banManager->getFailures()); + + $this->banManager->handleFailedAttempt($server); + $this->assertCount(0, $this->banManager->getFailures()); + + $this->banManager->handleFailedAttempt($server); + $this->assertCount(0, $this->banManager->getFailures()); + } + + /** + * Test failed attempts with multiple direct IP. + */ + public function testHandleFailedAttemptMultipleIp() + { + $this->assertCount(0, $this->banManager->getFailures()); + $this->banManager->handleFailedAttempt($this->server); + $this->server['REMOTE_ADDR'] = '1.2.3.4'; + $this->banManager->handleFailedAttempt($this->server); + $this->banManager->handleFailedAttempt($this->server); + $this->assertCount(2, $this->banManager->getFailures()); + $this->assertEquals(1, $this->banManager->getFailures()[$this->ipAddr]); + $this->assertEquals(2, $this->banManager->getFailures()[$this->server['REMOTE_ADDR']]); + } + + /** + * Test clear failure for provided IP without any additional data. + */ + public function testClearFailuresEmpty() + { + $this->assertCount(0, $this->banManager->getFailures()); + $this->banManager->clearFailures($this->server); + $this->assertCount(0, $this->banManager->getFailures()); + } + + /** + * Test clear failure for provided IP with failed attempts. + */ + public function testClearFailuresFromFile() + { + FileUtils::writeFlatDB( + $this->banFile, + [ + 'failures' => [ + $this->ipAddr => 2, + $ip = '1.2.3.4' => 1, + ] + ] + ); + $this->banManager = $this->getNewBanManagerInstance(); + + $this->assertCount(2, $this->banManager->getFailures()); + $this->banManager->clearFailures($this->server); + $this->assertCount(1, $this->banManager->getFailures()); + $this->assertEquals(1, $this->banManager->getFailures()[$ip]); + } + + /** + * Test clear failure for provided IP with failed attempts, behind a reverse proxy. + */ + public function testClearFailuresFromFileBehindProxy() + { + $server = [ + 'REMOTE_ADDR' => $this->trustedProxy, + 'HTTP_X_FORWARDED_FOR' => $this->ipAddr, + ]; + + FileUtils::writeFlatDB( + $this->banFile, + [ + 'failures' => [ + $this->ipAddr => 2, + $ip = '1.2.3.4' => 1, + ] + ] + ); + $this->banManager = $this->getNewBanManagerInstance(); + + $this->assertCount(2, $this->banManager->getFailures()); + $this->banManager->clearFailures($server); + $this->assertCount(1, $this->banManager->getFailures()); + $this->assertEquals(1, $this->banManager->getFailures()[$ip]); + } + + /** + * Test clear failure for provided IP with failed attempts, + * behind a reverse proxy without forwarding. + */ + public function testClearFailuresFromFileBehindNotConfiguredProxy() + { + $server = [ + 'REMOTE_ADDR' => $this->trustedProxy, + ]; + + FileUtils::writeFlatDB( + $this->banFile, + [ + 'failures' => [ + $this->ipAddr => 2, + $ip = '1.2.3.4' => 1, + ] + ] + ); + $this->banManager = $this->getNewBanManagerInstance(); + + $this->assertCount(2, $this->banManager->getFailures()); + $this->banManager->clearFailures($server); + $this->assertCount(2, $this->banManager->getFailures()); + } + + /** + * Test isBanned without any data + */ + public function testIsBannedEmpty() + { + $this->assertFalse($this->banManager->isBanned($this->server)); + } + + /** + * Test isBanned with banned IP from file data + */ + public function testBannedFromFile() + { + FileUtils::writeFlatDB( + $this->banFile, + [ + 'bans' => [ + $this->ipAddr => time() + 10, + ] + ] + ); + $this->banManager = $this->getNewBanManagerInstance(); + + $this->assertCount(1, $this->banManager->getBans()); + $this->assertTrue($this->banManager->isBanned($this->server)); + } + + /** + * Test isBanned with banned IP from file data behind a reverse proxy + */ + public function testBannedFromFileBehindProxy() + { + $server = [ + 'REMOTE_ADDR' => $this->trustedProxy, + 'HTTP_X_FORWARDED_FOR' => $this->ipAddr, + ]; + FileUtils::writeFlatDB( + $this->banFile, + [ + 'bans' => [ + $this->ipAddr => time() + 10, + ] + ] + ); + $this->banManager = $this->getNewBanManagerInstance(); + + $this->assertCount(1, $this->banManager->getBans()); + $this->assertTrue($this->banManager->isBanned($server)); + } + + /** + * Test isBanned with banned IP from file data behind a reverse proxy, + * without IP forwarding + */ + public function testBannedFromFileBehindNotConfiguredProxy() + { + $server = [ + 'REMOTE_ADDR' => $this->trustedProxy, + ]; + FileUtils::writeFlatDB( + $this->banFile, + [ + 'bans' => [ + $this->ipAddr => time() + 10, + ] + ] + ); + $this->banManager = $this->getNewBanManagerInstance(); + + $this->assertCount(1, $this->banManager->getBans()); + $this->assertFalse($this->banManager->isBanned($server)); + } + + /** + * Test isBanned with an expired ban + */ + public function testLiftBan() + { + FileUtils::writeFlatDB( + $this->banFile, + [ + 'bans' => [ + $this->ipAddr => time() - 10, + ] + ] + ); + $this->banManager = $this->getNewBanManagerInstance(); + + $this->assertCount(1, $this->banManager->getBans()); + $this->assertFalse($this->banManager->isBanned($this->server)); + } + + /** + * Test isBanned with an expired ban behind a reverse proxy + */ + public function testLiftBanBehindProxy() + { + $server = [ + 'REMOTE_ADDR' => $this->trustedProxy, + 'HTTP_X_FORWARDED_FOR' => $this->ipAddr, + ]; + + FileUtils::writeFlatDB( + $this->banFile, + [ + 'bans' => [ + $this->ipAddr => time() - 10, + ] + ] + ); + $this->banManager = $this->getNewBanManagerInstance(); + + $this->assertCount(1, $this->banManager->getBans()); + $this->assertFalse($this->banManager->isBanned($server)); + } + + /** + * Test isBanned with an expired ban behind a reverse proxy + */ + public function testLiftBanBehindNotConfiguredProxy() + { + $server = [ + 'REMOTE_ADDR' => $this->trustedProxy, + ]; + + FileUtils::writeFlatDB( + $this->banFile, + [ + 'bans' => [ + $this->ipAddr => time() - 10, + ] + ] + ); + $this->banManager = $this->getNewBanManagerInstance(); + + $this->assertCount(1, $this->banManager->getBans()); + $this->assertFalse($this->banManager->isBanned($server)); + } + + /** + * Build a new instance of BanManager, which will reread the ban file. + * + * @return BanManager instance + */ + protected function getNewBanManagerInstance() + { + return new BanManager( + [$this->trustedProxy], + 3, + 1800, + $this->banFile, + $this->logFile + ); + } +} diff --git a/tests/security/LoginManagerTest.php b/tests/security/LoginManagerTest.php index 7b0262b3..eef0f22a 100644 --- a/tests/security/LoginManagerTest.php +++ b/tests/security/LoginManagerTest.php @@ -75,54 +75,27 @@ class LoginManagerTest extends TestCase 'credentials.salt' => $this->salt, 'resource.ban_file' => $this->banFile, 'resource.log' => $this->logFile, - 'security.ban_after' => 4, + 'security.ban_after' => 2, 'security.ban_duration' => 3600, 'security.trusted_proxies' => [$this->trustedProxy], ]); $this->cookie = []; - - $this->globals = &$GLOBALS; - unset($this->globals['IPBANS']); - $this->session = []; $this->sessionManager = new SessionManager($this->session, $this->configManager); - $this->loginManager = new LoginManager($this->globals, $this->configManager, $this->sessionManager); + $this->loginManager = new LoginManager($this->configManager, $this->sessionManager); $this->server['REMOTE_ADDR'] = $this->ipAddr; } - /** - * Wipe test resources - */ - public function tearDown() - { - unset($this->globals['IPBANS']); - } - - /** - * Instantiate a LoginManager and load ban records - */ - public function testReadBanFile() - { - file_put_contents( - $this->banFile, - " array('127.0.0.1' => 99));\n?>" - ); - new LoginManager($this->globals, $this->configManager, null); - $this->assertEquals(99, $this->globals['IPBANS']['FAILURES']['127.0.0.1']); - } - /** * Record a failed login attempt */ public function testHandleFailedLogin() { $this->loginManager->handleFailedLogin($this->server); - $this->assertEquals(1, $this->globals['IPBANS']['FAILURES'][$this->ipAddr]); - $this->loginManager->handleFailedLogin($this->server); - $this->assertEquals(2, $this->globals['IPBANS']['FAILURES'][$this->ipAddr]); + $this->assertFalse($this->loginManager->canLogin($this->server)); } /** @@ -135,10 +108,8 @@ class LoginManagerTest extends TestCase 'HTTP_X_FORWARDED_FOR' => $this->ipAddr, ]; $this->loginManager->handleFailedLogin($server); - $this->assertEquals(1, $this->globals['IPBANS']['FAILURES'][$this->ipAddr]); - $this->loginManager->handleFailedLogin($server); - $this->assertEquals(2, $this->globals['IPBANS']['FAILURES'][$this->ipAddr]); + $this->assertFalse($this->loginManager->canLogin($server)); } /** @@ -150,39 +121,8 @@ class LoginManagerTest extends TestCase 'REMOTE_ADDR' => $this->trustedProxy, ]; $this->loginManager->handleFailedLogin($server); - $this->assertFalse(isset($this->globals['IPBANS']['FAILURES'][$this->ipAddr])); - $this->loginManager->handleFailedLogin($server); - $this->assertFalse(isset($this->globals['IPBANS']['FAILURES'][$this->ipAddr])); - } - - /** - * Record a failed login attempt and ban the IP after too many failures - */ - public function testHandleFailedLoginBanIp() - { - $this->loginManager->handleFailedLogin($this->server); - $this->assertEquals(1, $this->globals['IPBANS']['FAILURES'][$this->ipAddr]); - $this->assertTrue($this->loginManager->canLogin($this->server)); - - $this->loginManager->handleFailedLogin($this->server); - $this->assertEquals(2, $this->globals['IPBANS']['FAILURES'][$this->ipAddr]); - $this->assertTrue($this->loginManager->canLogin($this->server)); - - $this->loginManager->handleFailedLogin($this->server); - $this->assertEquals(3, $this->globals['IPBANS']['FAILURES'][$this->ipAddr]); - $this->assertTrue($this->loginManager->canLogin($this->server)); - - $this->loginManager->handleFailedLogin($this->server); - $this->assertEquals(4, $this->globals['IPBANS']['FAILURES'][$this->ipAddr]); - $this->assertFalse($this->loginManager->canLogin($this->server)); - - // handleFailedLogin is not supposed to be called at this point: - // - no login form should be displayed once an IP has been banned - // - yet this could happen when using custom templates / scripts - $this->loginManager->handleFailedLogin($this->server); - $this->assertEquals(5, $this->globals['IPBANS']['FAILURES'][$this->ipAddr]); - $this->assertFalse($this->loginManager->canLogin($this->server)); + $this->assertTrue($this->loginManager->canLogin($server)); } /** @@ -202,14 +142,11 @@ class LoginManagerTest extends TestCase public function testHandleSuccessfulLoginAfterFailure() { $this->loginManager->handleFailedLogin($this->server); - $this->loginManager->handleFailedLogin($this->server); - $this->assertEquals(2, $this->globals['IPBANS']['FAILURES'][$this->ipAddr]); $this->assertTrue($this->loginManager->canLogin($this->server)); $this->loginManager->handleSuccessfulLogin($this->server); + $this->loginManager->handleFailedLogin($this->server); $this->assertTrue($this->loginManager->canLogin($this->server)); - $this->assertFalse(isset($this->globals['IPBANS']['FAILURES'][$this->ipAddr])); - $this->assertFalse(isset($this->globals['IPBANS']['BANS'][$this->ipAddr])); } /** @@ -220,33 +157,6 @@ class LoginManagerTest extends TestCase $this->assertTrue($this->loginManager->canLogin($this->server)); } - /** - * The IP is banned - */ - public function testCanLoginIpBanned() - { - // ban the IP for an hour - $this->globals['IPBANS']['FAILURES'][$this->ipAddr] = 10; - $this->globals['IPBANS']['BANS'][$this->ipAddr] = time() + 3600; - - $this->assertFalse($this->loginManager->canLogin($this->server)); - } - - /** - * The IP is banned, and the ban duration is over - */ - public function testCanLoginIpBanExpired() - { - // ban the IP for an hour - $this->globals['IPBANS']['FAILURES'][$this->ipAddr] = 10; - $this->globals['IPBANS']['BANS'][$this->ipAddr] = time() + 3600; - $this->assertFalse($this->loginManager->canLogin($this->server)); - - // lift the ban - $this->globals['IPBANS']['BANS'][$this->ipAddr] = time() - 3600; - $this->assertTrue($this->loginManager->canLogin($this->server)); - } - /** * Generate a token depending on the user credentials and client IP */ @@ -282,7 +192,7 @@ class LoginManagerTest extends TestCase $configManager = new \FakeConfigManager([ 'resource.ban_file' => $this->banFile, ]); - $loginManager = new LoginManager($this->globals, $configManager, null); + $loginManager = new LoginManager($configManager, null); $loginManager->checkLoginState([], ''); $this->assertFalse($loginManager->isLoggedIn()); -- cgit v1.2.3