From b38a1b0209f546d4824a0db81a34c4e30fcdebaf Mon Sep 17 00:00:00 2001 From: ArthurHoaro Date: Tue, 20 Oct 2020 11:47:07 +0200 Subject: Use PSR-3 logger for login attempts Fixes #1122 --- tests/UtilsTest.php | 36 +++++---------- tests/container/ContainerBuilderTest.php | 5 ++- .../controller/visitor/LoginControllerTest.php | 2 +- tests/security/BanManagerTest.php | 3 +- tests/security/LoginManagerTest.php | 51 +++++++++++++++++----- tests/security/SessionManagerTest.php | 5 ++- tests/utils/FakeConfigManager.php | 10 +++-- 7 files changed, 66 insertions(+), 46 deletions(-) (limited to 'tests') 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 } /** - * Log a message to a file - IPv4 client address + * Format a log a message - IPv4 client address */ - public function testLogmIp4() + public function testFormatLogIp4() { - $logMessage = 'IPv4 client connected'; - logm(self::$testLogFile, '127.0.0.1', $logMessage); - list($date, $ip, $message) = $this->getLastLogEntry(); + $message = 'IPv4 client connected'; + $log = format_log($message, '127.0.0.1'); - $this->assertInstanceOf( - 'DateTime', - DateTime::createFromFormat(self::$dateFormat, $date) - ); - $this->assertTrue( - filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4) !== false - ); - $this->assertEquals($logMessage, $message); + static::assertSame('- 127.0.0.1 - IPv4 client connected', $log); } /** - * Log a message to a file - IPv6 client address + * Format a log a message - IPv6 client address */ - public function testLogmIp6() + public function testFormatLogIp6() { - $logMessage = 'IPv6 client connected'; - logm(self::$testLogFile, '2001:db8::ff00:42:8329', $logMessage); - list($date, $ip, $message) = $this->getLastLogEntry(); + $message = 'IPv6 client connected'; + $log = format_log($message, '2001:db8::ff00:42:8329'); - $this->assertInstanceOf( - 'DateTime', - DateTime::createFromFormat(self::$dateFormat, $date) - ); - $this->assertTrue( - filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6) !== false - ); - $this->assertEquals($logMessage, $message); + static::assertSame('- 2001:db8::ff00:42:8329 - IPv6 client connected', $log); } /** 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); namespace Shaarli\Container; +use Psr\Log\LoggerInterface; use Shaarli\Bookmark\BookmarkServiceInterface; use Shaarli\Config\ConfigManager; use Shaarli\Feed\FeedBuilder; @@ -55,7 +56,8 @@ class ContainerBuilderTest extends TestCase $this->conf, $this->sessionManager, $this->cookieManager, - $this->loginManager + $this->loginManager, + $this->createMock(LoggerInterface::class) ); } @@ -73,6 +75,7 @@ class ContainerBuilderTest extends TestCase static::assertInstanceOf(History::class, $container->history); static::assertInstanceOf(HttpAccess::class, $container->httpAccess); static::assertInstanceOf(LoginManager::class, $container->loginManager); + static::assertInstanceOf(LoggerInterface::class, $container->logger); static::assertInstanceOf(MetadataRetriever::class, $container->metadataRetriever); static::assertInstanceOf(NetscapeBookmarkUtils::class, $container->netscapeBookmarkUtils); 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 $this->container->loginManager ->expects(static::once()) ->method('checkCredentials') - ->with('1.2.3.4', '1.2.3.4', 'bob', 'pass') + ->with('1.2.3.4', 'bob', 'pass') ->willReturn(true) ; $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 @@ namespace Shaarli\Security; +use Psr\Log\LoggerInterface; use Shaarli\FileUtils; use Shaarli\TestCase; @@ -387,7 +388,7 @@ class BanManagerTest extends TestCase 3, 1800, $this->banFile, - $this->logFile + $this->createMock(LoggerInterface::class) ); } } 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 @@ namespace Shaarli\Security; +use Psr\Log\LoggerInterface; +use Shaarli\FakeConfigManager; use Shaarli\TestCase; /** @@ -9,7 +11,7 @@ use Shaarli\TestCase; */ class LoginManagerTest extends TestCase { - /** @var \FakeConfigManager Configuration Manager instance */ + /** @var FakeConfigManager Configuration Manager instance */ protected $configManager = null; /** @var LoginManager Login Manager instance */ @@ -60,6 +62,9 @@ class LoginManagerTest extends TestCase /** @var CookieManager */ protected $cookieManager; + /** @var BanManager */ + protected $banManager; + /** * Prepare or reset test resources */ @@ -71,7 +76,7 @@ class LoginManagerTest extends TestCase $this->passwordHash = sha1($this->password . $this->login . $this->salt); - $this->configManager = new \FakeConfigManager([ + $this->configManager = new FakeConfigManager([ 'credentials.login' => $this->login, 'credentials.hash' => $this->passwordHash, 'credentials.salt' => $this->salt, @@ -91,18 +96,29 @@ class LoginManagerTest extends TestCase return $this->cookie[$key] ?? null; }); $this->sessionManager = new SessionManager($this->session, $this->configManager, 'session_path'); - $this->loginManager = new LoginManager($this->configManager, $this->sessionManager, $this->cookieManager); + $this->banManager = $this->createMock(BanManager::class); + $this->loginManager = new LoginManager( + $this->configManager, + $this->sessionManager, + $this->cookieManager, + $this->banManager, + $this->createMock(LoggerInterface::class) + ); $this->server['REMOTE_ADDR'] = $this->ipAddr; } /** * Record a failed login attempt */ - public function testHandleFailedLogin() + public function testHandleFailedLogin(): void { + $this->banManager->expects(static::exactly(2))->method('handleFailedAttempt'); + $this->banManager->method('isBanned')->willReturn(true); + $this->loginManager->handleFailedLogin($this->server); $this->loginManager->handleFailedLogin($this->server); - $this->assertFalse($this->loginManager->canLogin($this->server)); + + static::assertFalse($this->loginManager->canLogin($this->server)); } /** @@ -114,8 +130,13 @@ class LoginManagerTest extends TestCase 'REMOTE_ADDR' => $this->trustedProxy, 'HTTP_X_FORWARDED_FOR' => $this->ipAddr, ]; + + $this->banManager->expects(static::exactly(2))->method('handleFailedAttempt'); + $this->banManager->method('isBanned')->willReturn(true); + $this->loginManager->handleFailedLogin($server); $this->loginManager->handleFailedLogin($server); + $this->assertFalse($this->loginManager->canLogin($server)); } @@ -196,10 +217,16 @@ class LoginManagerTest extends TestCase */ public function testCheckLoginStateNotConfigured() { - $configManager = new \FakeConfigManager([ + $configManager = new FakeConfigManager([ 'resource.ban_file' => $this->banFile, ]); - $loginManager = new LoginManager($configManager, null, $this->cookieManager); + $loginManager = new LoginManager( + $configManager, + $this->sessionManager, + $this->cookieManager, + $this->banManager, + $this->createMock(LoggerInterface::class) + ); $loginManager->checkLoginState(''); $this->assertFalse($loginManager->isLoggedIn()); @@ -270,7 +297,7 @@ class LoginManagerTest extends TestCase public function testCheckCredentialsWrongLogin() { $this->assertFalse( - $this->loginManager->checkCredentials('', '', 'b4dl0g1n', $this->password) + $this->loginManager->checkCredentials('', 'b4dl0g1n', $this->password) ); } @@ -280,7 +307,7 @@ class LoginManagerTest extends TestCase public function testCheckCredentialsWrongPassword() { $this->assertFalse( - $this->loginManager->checkCredentials('', '', $this->login, 'b4dp455wd') + $this->loginManager->checkCredentials('', $this->login, 'b4dp455wd') ); } @@ -290,7 +317,7 @@ class LoginManagerTest extends TestCase public function testCheckCredentialsWrongLoginAndPassword() { $this->assertFalse( - $this->loginManager->checkCredentials('', '', 'b4dl0g1n', 'b4dp455wd') + $this->loginManager->checkCredentials('', 'b4dl0g1n', 'b4dp455wd') ); } @@ -300,7 +327,7 @@ class LoginManagerTest extends TestCase public function testCheckCredentialsGoodLoginAndPassword() { $this->assertTrue( - $this->loginManager->checkCredentials('', '', $this->login, $this->password) + $this->loginManager->checkCredentials('', $this->login, $this->password) ); } @@ -311,7 +338,7 @@ class LoginManagerTest extends TestCase { $this->configManager->set('ldap.host', 'dummy'); $this->assertFalse( - $this->loginManager->checkCredentials('', '', $this->login, $this->password) + $this->loginManager->checkCredentials('', $this->login, $this->password) ); } 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 @@ namespace Shaarli\Security; +use Shaarli\FakeConfigManager; use Shaarli\TestCase; /** @@ -12,7 +13,7 @@ class SessionManagerTest extends TestCase /** @var array Session ID hashes */ protected static $sidHashes = null; - /** @var \FakeConfigManager ConfigManager substitute for testing */ + /** @var FakeConfigManager ConfigManager substitute for testing */ protected $conf = null; /** @var array $_SESSION array for testing */ @@ -34,7 +35,7 @@ class SessionManagerTest extends TestCase */ protected function setUp(): void { - $this->conf = new \FakeConfigManager([ + $this->conf = new FakeConfigManager([ 'credentials.login' => 'johndoe', 'credentials.salt' => 'salt', '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 @@ values[$key] = $value; } @@ -35,7 +39,7 @@ class FakeConfigManager * * @return mixed The value if set, else the name of the key */ - public function get($key) + public function get($key, $default = '') { if (isset($this->values[$key])) { return $this->values[$key]; -- cgit v1.2.3