Knah-Tsaeb/MyShaarli
Knah-Tsaeb/MyShaarli
1
0
Fork 0
Code Issues Pull requests Releases Wiki Activity

Rewrite IP ban management

Browse source 
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
This commit is contained in:
ArthurHoaro 2019-02-09 16:44:48 +01:00
parent 905f8675a7
commit b49a04f796
6 changed files with 630 additions and 184 deletions

393
tests/security/BanManagerTest.php Normal file
View file

@ -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
);
}
}

104
tests/security/LoginManagerTest.php
View file

@ -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]));
}
/**
@ -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());