6 files changed, 630 insertions, 184 deletions

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 @@
+<?php
+
+
+namespace Shaarli\Security;
+
+use Shaarli\FileUtils;
+
+/**
+ * Class BanManager
+ *
+ * Failed login attempts will store the associated IP address.
+ * After N failed attempts, the IP will be prevented from log in for duration D.
+ * Both N and D can be set in the configuration file.
+ *
+ * @package Shaarli\Security
+ */
+class BanManager
+{
+    /** @var array List of allowed proxies IP */
+    protected $trustedProxies;
+
+    /** @var int Number of allowed failed attempt before the ban */
+    protected $nbAttempts;
+
+    /** @var  int Ban duration in seconds */
+    protected $banDuration;
+
+    /** @var string Path to the file containing IP bans and failures */
+    protected $banFile;
+
+    /** @var string Path to the log file, used to log bans */
+    protected $logFile;
+
+    /** @var array List of IP with their associated number of failed attempts */
+    protected $failures = [];
+
+    /** @var array List of banned IP with their associated unban timestamp */
+    protected $bans = [];
+
+    /**
+     * BanManager constructor.
+     *
+     * @param array  $trustedProxies List of allowed proxies IP
+     * @param int    $nbAttempts     Number of allowed failed attempt before the ban
+     * @param int    $banDuration    Ban duration in seconds
+     * @param string $banFile        Path to the file containing IP bans and failures
+     * @param string $logFile        Path to the log file, used to log bans
+     */
+    public function __construct($trustedProxies, $nbAttempts, $banDuration, $banFile, $logFile) {
+        $this->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;
         }
@@ -158,65 +162,13 @@ class LoginManager
     }
 
     /**
-     * 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,
-            "<?php\n\$GLOBALS['IPBANS']=" . var_export($this->globals['IPBANS'], true) . ";\n?>"
-        );
-    }
-
-    /**
      * Handle a failed login and ban the IP after too many failed attempts
      *
      * @param array $server The $_SERVER array
      */
     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 @@
+<?php
+
+
+namespace Shaarli\Security;
+
+use PHPUnit\Framework\TestCase;
+use Shaarli\FileUtils;
+
+/**
+ * Test coverage for BanManager
+ */
+class BanManagerTest extends TestCase
+{
+    /** @var BanManager Ban Manager instance */
+    protected $banManager;
+
+    /** @var string Banned IP filename */
+    protected $banFile = 'sandbox/ipbans.php';
+
+    /** @var string Log filename */
+    protected $logFile = 'sandbox/shaarli.log';
+
+    /** @var string Local client IP address */
+    protected $ipAddr = '127.0.0.1';
+
+    /** @var string Trusted proxy IP address */
+    protected $trustedProxy = '10.1.1.100';
+
+    /** @var array Simulates the $_SERVER array */
+    protected $server = [];
+
+    /**
+     * Prepare or reset test resources
+     */
+    public function setUp()
+    {
+        if (file_exists($this->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,
-            "<?php\n\$GLOBALS['IPBANS']=array('FAILURES' => 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]));
     }
 
     /**
@@ -221,33 +158,6 @@ class LoginManagerTest extends TestCase
     }
 
     /**
-     * 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
      */
     public function testGenerateStaySignedInToken()
@@ -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());