From 1d54db7c096f3e34f8876c6751c70e5ce7e0f859 Mon Sep 17 00:00:00 2001 From: Julien Loizelet Date: Mon, 28 Apr 2025 14:43:39 +0900 Subject: [PATCH 1/4] feat(metrics): Add a method to reset metrics and check Blaas URI --- docs/USER_GUIDE.md | 2 +- src/AbstractBouncer.php | 30 ++++++++++++++++++++++++++--- src/Constants.php | 2 ++ src/Helper.php | 6 +++--- tests/Integration/WatcherClient.php | 4 ++-- 5 files changed, 35 insertions(+), 9 deletions(-) diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index af2cf70..419e4b4 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -299,7 +299,7 @@ Below is the list of available settings: - `captcha_cache_duration`: Set the duration we keep in cache the captcha flow variables for an IP. In seconds. - Defaults to 86400.. In seconds. Defaults to 20. + Defaults to 86400. ### Geolocation diff --git a/src/AbstractBouncer.php b/src/AbstractBouncer.php index ec48f93..6e02787 100644 --- a/src/AbstractBouncer.php +++ b/src/AbstractBouncer.php @@ -48,7 +48,7 @@ abstract class AbstractBouncer public function __construct( array $configs, LapiRemediation $remediationEngine, - ?LoggerInterface $logger = null + ?LoggerInterface $logger = null, ) { // @codeCoverageIgnoreStart if (!$logger) { @@ -225,6 +225,16 @@ abstract public function getRequestUri(): string; */ abstract public function getRequestUserAgent(): string; + /** + * Check if the bouncer is connected to a "Blocklist as a service" Lapi. + */ + public function hasBlassUri(): bool + { + $url = $this->getConfig('api_url'); + + return 0 === strpos($url, Constants::BLAAS_URL); + } + /** * This method prune the cache: it removes all the expired cache items. * @@ -248,7 +258,7 @@ public function pruneCache(): bool public function pushUsageMetrics( string $bouncerName, string $bouncerVersion, - string $bouncerType = LapiConstants::METRICS_TYPE + string $bouncerType = LapiConstants::METRICS_TYPE, ): array { try { return $this->remediationEngine->pushUsageMetrics($bouncerName, $bouncerVersion, $bouncerType); @@ -276,6 +286,20 @@ public function refreshBlocklistCache(): array } } + /** + * @throws InvalidArgumentException + */ + public function resetUsageMetrics(): void + { + // Retrieve metrics cache item + $metricsItem = $this->getRemediationEngine()->getCacheStorage()->getItem(AbstractCache::ORIGINS_COUNT); + if ($metricsItem->isHit()) { + // Reset the metrics + $metricsItem->set([]); + $this->getRemediationEngine()->getCacheStorage()->getAdapter()->save($metricsItem); + } + } + /** * Handle a bounce for current IP. * @@ -400,7 +424,7 @@ protected function getBanHtml(): string protected function getCaptchaHtml( bool $error, string $captchaImageSrc, - string $captchaResolutionFormUrl + string $captchaResolutionFormUrl, ): string { $template = new Template('captcha.html.twig'); diff --git a/src/Constants.php b/src/Constants.php index 501aa2e..f987999 100644 --- a/src/Constants.php +++ b/src/Constants.php @@ -18,6 +18,8 @@ */ class Constants extends RemConstants { + /** @var string The URL prefix for Blocklist as a service LAPI */ + public const BLAAS_URL = 'https://admin.api.crowdsec.net'; /** @var int The duration we keep a captcha flow in cache */ public const CACHE_EXPIRATION_FOR_CAPTCHA = 86400; /** @var string The "MEMCACHED" cache system */ diff --git a/src/Helper.php b/src/Helper.php index dcc6a75..600b5a7 100644 --- a/src/Helper.php +++ b/src/Helper.php @@ -34,7 +34,7 @@ private function buildRawBodyFromSuperglobals( $stream, array $serverData = [], // $_SERVER array $postData = [], // $_POST - array $filesData = [] // $_FILES + array $filesData = [], // $_FILES ): string { $contentType = $serverData['CONTENT_TYPE'] ?? ''; // The threshold is the maximum body size converted in bytes + 1 @@ -53,7 +53,7 @@ private function appendFileData( string $fileKey, string $boundary, int $threshold, - int &$currentSize + int &$currentSize, ): string { $fileName = is_array($fileArray['name']) ? $fileArray['name'][$index] : $fileArray['name']; $fileTmpName = is_array($fileArray['tmp_name']) ? $fileArray['tmp_name'][$index] : $fileArray['tmp_name']; @@ -119,7 +119,7 @@ private function getMultipartRawBody( string $contentType, int $threshold, array $postData, - array $filesData + array $filesData, ): string { try { $boundary = $this->extractBoundary($contentType); diff --git a/tests/Integration/WatcherClient.php b/tests/Integration/WatcherClient.php index 0a7cca7..9f2db18 100644 --- a/tests/Integration/WatcherClient.php +++ b/tests/Integration/WatcherClient.php @@ -50,7 +50,7 @@ public function __construct(array $configs) private function manageRequest( string $method, string $endpoint, - array $parameters = [] + array $parameters = [], ): array { $this->logger->debug('', [ 'type' => 'WATCHER_CLIENT_REQUEST', @@ -158,7 +158,7 @@ public function addDecision( string $dateTimeDurationString, string $value, string $type, - string $scope = Constants::SCOPE_IP + string $scope = Constants::SCOPE_IP, ) { $stopAt = (clone $now)->modify($dateTimeDurationString)->format('Y-m-d\TH:i:s.000\Z'); $startAt = $now->format('Y-m-d\TH:i:s.000\Z'); From 527c5459e9f75ad8c3c3ac1c1d462e6a7d5fd9d8 Mon Sep 17 00:00:00 2001 From: Julien Loizelet Date: Mon, 28 Apr 2025 15:12:38 +0900 Subject: [PATCH 2/4] test(reset metrics): Add code coverage --- src/AbstractBouncer.php | 4 +- tests/Integration/AbstractBouncerTest.php | 85 +++++++++++++++++++++++ tests/Unit/AbstractBouncerTest.php | 25 +++++++ 3 files changed, 112 insertions(+), 2 deletions(-) diff --git a/src/AbstractBouncer.php b/src/AbstractBouncer.php index 6e02787..1916e80 100644 --- a/src/AbstractBouncer.php +++ b/src/AbstractBouncer.php @@ -228,9 +228,9 @@ abstract public function getRequestUserAgent(): string; /** * Check if the bouncer is connected to a "Blocklist as a service" Lapi. */ - public function hasBlassUri(): bool + public function hasBlaasUri(): bool { - $url = $this->getConfig('api_url'); + $url = $this->getRemediationEngine()->getClient()->getConfig('api_url'); return 0 === strpos($url, Constants::BLAAS_URL); } diff --git a/tests/Integration/AbstractBouncerTest.php b/tests/Integration/AbstractBouncerTest.php index dbec499..8494af6 100644 --- a/tests/Integration/AbstractBouncerTest.php +++ b/tests/Integration/AbstractBouncerTest.php @@ -81,6 +81,7 @@ * @covers \CrowdSecBouncer\AbstractBouncer::shouldBounceCurrentIp * @covers \CrowdSecBouncer\AbstractBouncer::handleBounceExclusion * @covers \CrowdSecBouncer\AbstractBouncer::pushUsageMetrics + * @covers \CrowdSecBouncer\AbstractBouncer::resetUsageMetrics */ final class AbstractBouncerTest extends TestCase { @@ -1053,6 +1054,90 @@ public function testRun() ); } + public function testResetMetrics() + { + $client = new BouncerClient($this->configs, null, $this->logger); + $cache = new PhpFiles($this->configs, $this->logger); + $originCountItem = $cache->getItem(AbstractCache::ORIGINS_COUNT)->get(); + $this->assertEquals( + null, + $originCountItem, + 'The origin count for clean should be empty' + ); + + // bouncing URI + $client = new BouncerClient($this->configs, null, $this->logger); + $cache = new PhpFiles($this->configs, $this->logger); + $lapiRemediation = new LapiRemediation($this->configs, $client, $cache, $this->logger); + // Mock sendResponse and redirectResponse to avoid PHP UNIT header already sent or exit error + $bouncer = $this->getMockForAbstractClass(AbstractBouncer::class, [$this->configs, $lapiRemediation, $this->logger], + '', true, + true, true, [ + 'sendResponse', + 'redirectResponse', + 'getHttpMethod', + 'getPostedVariable', + 'getHttpRequestHeader', + 'getRemoteIp', + 'getRequestUri', + ]); + + $bouncer->method('getRequestUri')->willReturnOnConsecutiveCalls('/home'); + $bouncer->method('getRemoteIp')->willReturnOnConsecutiveCalls('127.0.0.3'); + $this->assertEquals(true, $bouncer->run(), 'Should bounce uri'); + $originCountItem = $cache->getItem(AbstractCache::ORIGINS_COUNT)->get(); + $this->assertEquals( + ['clean' => ['bypass' => 1]], + $originCountItem, + 'The origin count for clean should be 1' + ); + + // Test no-forward + $bouncerConfigs = array_merge( + $this->configs, + [ + 'forced_test_forwarded_ip' => Constants::X_FORWARDED_DISABLED, + ] + ); + $client = new BouncerClient($bouncerConfigs, null, $this->logger); + $cache = new PhpFiles($bouncerConfigs, $this->logger); + $lapiRemediation = new LapiRemediation($bouncerConfigs, $client, $cache, $this->logger); + // Mock sendResponse and redirectResponse to avoid PHP UNIT header already sent or exit error + $bouncer = $this->getMockForAbstractClass(AbstractBouncer::class, [$bouncerConfigs, $lapiRemediation, $this->logger], + '', true, + true, true, [ + 'sendResponse', + 'redirectResponse', + 'getHttpMethod', + 'getPostedVariable', + 'getHttpRequestHeader', + 'getRemoteIp', + 'getRequestUri', + ]); + + $bouncer->method('getRequestUri')->willReturnOnConsecutiveCalls('/home'); + $bouncer->method('getRemoteIp')->willReturnOnConsecutiveCalls('127.0.0.7'); + + $bouncer->run(); + + $originCountItem = $cache->getItem(AbstractCache::ORIGINS_COUNT)->get(); + $this->assertEquals( + ['clean' => ['bypass' => 2]], + $originCountItem, + 'The origin count for clean should be 2' + ); + + + // Test: reset metrics + $result = $bouncer->resetUsageMetrics(); + $originCountItem = $cache->getItem(AbstractCache::ORIGINS_COUNT)->get(); + $this->assertEquals( + [], + $originCountItem, + 'The origin count item should be reset' + ); + } + public function testPrivateAndProtectedMethods() { // handleCache diff --git a/tests/Unit/AbstractBouncerTest.php b/tests/Unit/AbstractBouncerTest.php index 0027add..7571ad2 100644 --- a/tests/Unit/AbstractBouncerTest.php +++ b/tests/Unit/AbstractBouncerTest.php @@ -77,6 +77,7 @@ * @uses \CrowdSecBouncer\AbstractBouncer::handleBounceExclusion * * @covers \CrowdSecBouncer\AbstractBouncer::pushUsageMetrics + * @covers \CrowdSecBouncer\AbstractBouncer::hasBlaasUri */ final class AbstractBouncerTest extends TestCase { @@ -774,6 +775,30 @@ public function testPushUsageMetricsException() $this->assertEquals('Error in unit test', $errorMessage); } + public function testHasBlockAsAServiceUri() + { + $configs = $this->configs; + $client = new BouncerClient($configs); + $cache = new PhpFiles($configs); + $lapiRemediation = new LapiRemediation($configs, $client, $cache); + $bouncer = $this->getMockForAbstractClass(AbstractBouncer::class, [$configs, $lapiRemediation]); + + $result = $bouncer->hasBlaasUri(); + $this->assertEquals(false, $result); + + $configs = array_merge($this->configs, [ + 'api_url' => 'https://admin.api.crowdsec.net', + ]); + + $client = new BouncerClient($configs); + $cache = new PhpFiles($configs); + $lapiRemediation = new LapiRemediation($configs, $client, $cache); + $bouncer = $this->getMockForAbstractClass(AbstractBouncer::class, [$configs, $lapiRemediation]); + + $result = $bouncer->hasBlaasUri(); + $this->assertEquals(true, $result); + } + public function testShouldBounceCurrentIp() { $configs = $this->configs; From 5f4358cdb9a836116af1ad0ea8573477c87d0fa6 Mon Sep 17 00:00:00 2001 From: Julien Loizelet Date: Mon, 28 Apr 2025 15:16:42 +0900 Subject: [PATCH 3/4] docs(changelog): Prepare 4.3.0 release --- CHANGELOG.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a6c78b4..94a6367 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,18 @@ As far as possible, we try to adhere to [Symfony guidelines](https://symfony.com --- +## [4.3.0](https://github.com/crowdsecurity/php-cs-bouncer/releases/tag/v4.3.0) - 2025-05-?? +[_Compare with previous release_](https://github.com/crowdsecurity/php-cs-bouncer/compare/v4.2.0...HEAD) + +__This release is not published yet__ + +### Added + +- Add `hasBlaasUri` to detect if the bouncer is connected to a Block As A Service Lapi +- Add `resetUsageMetrics` to reset the usage metrics cache item + +--- + ## [4.2.0](https://github.com/crowdsecurity/php-cs-bouncer/releases/tag/v4.2.0) - 2025-01-31 [_Compare with previous release_](https://github.com/crowdsecurity/php-cs-bouncer/compare/v4.1.0...v4.2.0) From 8d09859e0cf2a8b74eff8c11d70557d71a4097b8 Mon Sep 17 00:00:00 2001 From: Julien Loizelet Date: Mon, 28 Apr 2025 15:23:03 +0900 Subject: [PATCH 4/4] style(*): Remove trailing comma for 7.2 --- src/AbstractBouncer.php | 6 +++--- src/Helper.php | 6 +++--- tests/Integration/AbstractBouncerTest.php | 1 - tests/Integration/WatcherClient.php | 4 ++-- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/AbstractBouncer.php b/src/AbstractBouncer.php index 1916e80..167742a 100644 --- a/src/AbstractBouncer.php +++ b/src/AbstractBouncer.php @@ -48,7 +48,7 @@ abstract class AbstractBouncer public function __construct( array $configs, LapiRemediation $remediationEngine, - ?LoggerInterface $logger = null, + ?LoggerInterface $logger = null ) { // @codeCoverageIgnoreStart if (!$logger) { @@ -258,7 +258,7 @@ public function pruneCache(): bool public function pushUsageMetrics( string $bouncerName, string $bouncerVersion, - string $bouncerType = LapiConstants::METRICS_TYPE, + string $bouncerType = LapiConstants::METRICS_TYPE ): array { try { return $this->remediationEngine->pushUsageMetrics($bouncerName, $bouncerVersion, $bouncerType); @@ -424,7 +424,7 @@ protected function getBanHtml(): string protected function getCaptchaHtml( bool $error, string $captchaImageSrc, - string $captchaResolutionFormUrl, + string $captchaResolutionFormUrl ): string { $template = new Template('captcha.html.twig'); diff --git a/src/Helper.php b/src/Helper.php index 600b5a7..dcc6a75 100644 --- a/src/Helper.php +++ b/src/Helper.php @@ -34,7 +34,7 @@ private function buildRawBodyFromSuperglobals( $stream, array $serverData = [], // $_SERVER array $postData = [], // $_POST - array $filesData = [], // $_FILES + array $filesData = [] // $_FILES ): string { $contentType = $serverData['CONTENT_TYPE'] ?? ''; // The threshold is the maximum body size converted in bytes + 1 @@ -53,7 +53,7 @@ private function appendFileData( string $fileKey, string $boundary, int $threshold, - int &$currentSize, + int &$currentSize ): string { $fileName = is_array($fileArray['name']) ? $fileArray['name'][$index] : $fileArray['name']; $fileTmpName = is_array($fileArray['tmp_name']) ? $fileArray['tmp_name'][$index] : $fileArray['tmp_name']; @@ -119,7 +119,7 @@ private function getMultipartRawBody( string $contentType, int $threshold, array $postData, - array $filesData, + array $filesData ): string { try { $boundary = $this->extractBoundary($contentType); diff --git a/tests/Integration/AbstractBouncerTest.php b/tests/Integration/AbstractBouncerTest.php index 8494af6..a1d7344 100644 --- a/tests/Integration/AbstractBouncerTest.php +++ b/tests/Integration/AbstractBouncerTest.php @@ -1127,7 +1127,6 @@ public function testResetMetrics() 'The origin count for clean should be 2' ); - // Test: reset metrics $result = $bouncer->resetUsageMetrics(); $originCountItem = $cache->getItem(AbstractCache::ORIGINS_COUNT)->get(); diff --git a/tests/Integration/WatcherClient.php b/tests/Integration/WatcherClient.php index 9f2db18..0a7cca7 100644 --- a/tests/Integration/WatcherClient.php +++ b/tests/Integration/WatcherClient.php @@ -50,7 +50,7 @@ public function __construct(array $configs) private function manageRequest( string $method, string $endpoint, - array $parameters = [], + array $parameters = [] ): array { $this->logger->debug('', [ 'type' => 'WATCHER_CLIENT_REQUEST', @@ -158,7 +158,7 @@ public function addDecision( string $dateTimeDurationString, string $value, string $type, - string $scope = Constants::SCOPE_IP, + string $scope = Constants::SCOPE_IP ) { $stopAt = (clone $now)->modify($dateTimeDurationString)->format('Y-m-d\TH:i:s.000\Z'); $startAt = $now->format('Y-m-d\TH:i:s.000\Z');