From fa40e528abf94fdc42bf2bb699503e23716cdaa0 Mon Sep 17 00:00:00 2001 From: Julien Loizelet Date: Sat, 7 Jan 2023 17:00:47 +0900 Subject: [PATCH 1/2] feat(*): Add cleanConfigs method and update logs --- src/AbstractClient.php | 8 +++-- src/Bouncer.php | 2 +- src/Configuration.php | 73 ++++++++++++++++++++++++++++-------------- 3 files changed, 56 insertions(+), 27 deletions(-) diff --git a/src/AbstractClient.php b/src/AbstractClient.php index 7113414..ffffc52 100644 --- a/src/AbstractClient.php +++ b/src/AbstractClient.php @@ -58,6 +58,10 @@ public function __construct( $logger->pushHandler(new NullHandler()); } $this->logger = $logger; + $this->logger->debug('Instantiate client', [ + 'type' => 'CLIENT_INIT', + 'configs' => array_merge($configs, ['api_key' => '***']) + ]); } /** @@ -97,7 +101,7 @@ protected function request( $method = strtoupper($method); if (!in_array($method, $this->allowedMethods)) { $message = "Method ($method) is not allowed."; - $this->logger->error($message, ['type' => 'LAPI_CLIENT_REQUEST']); + $this->logger->error($message, ['type' => 'CLIENT_REQUEST']); throw new ClientException($message); } @@ -133,7 +137,7 @@ private function formatResponseBody(Response $response): array if (null === $decoded) { $message = 'Body response is not a valid json'; - $this->logger->error($message, ['type' => 'LAPI_CLIENT_FORMAT_RESPONSE']); + $this->logger->error($message, ['type' => 'CLIENT_FORMAT_RESPONSE']); throw new ClientException($message); } } diff --git a/src/Bouncer.php b/src/Bouncer.php index d38d38c..89070dc 100644 --- a/src/Bouncer.php +++ b/src/Bouncer.php @@ -91,7 +91,7 @@ private function configure(array $configs): void { $configuration = new Configuration(); $processor = new Processor(); - $this->configs = $processor->processConfiguration($configuration, [$configs]); + $this->configs = $processor->processConfiguration($configuration, [$configuration->cleanConfigs($configs)]); } /** diff --git a/src/Configuration.php b/src/Configuration.php index cdd9a15..0ecd573 100644 --- a/src/Configuration.php +++ b/src/Configuration.php @@ -10,7 +10,7 @@ use Symfony\Component\Config\Definition\ConfigurationInterface; /** - * The Watcher configuration. + * The LAPI client configuration. * * @author CrowdSec team * @@ -21,6 +21,30 @@ */ class Configuration implements ConfigurationInterface { + /** @var array The list of each configuration tree key */ + protected $keys = [ + 'user_agent_suffix', + 'user_agent_version', + 'api_url', + 'auth_type', + 'api_key', + 'tls_cert_path', + 'tls_key_path', + 'tls_ca_cert_path', + 'tls_verify_peer', + 'api_timeout' + ]; + + /** + * Keep only necessary configs + * @param array $configs + * @return array + */ + public function cleanConfigs(array $configs): array + { + return array_intersect_key($configs, array_flip($this->keys)); + } + /** * @throws \InvalidArgumentException * @throws \RuntimeException @@ -54,8 +78,8 @@ public function getConfigTreeBuilder(): TreeBuilder ->end() ->end() ; - $this->validate($rootNode); $this->addConnectionNodes($rootNode); + $this->validate($rootNode); return $treeBuilder; } @@ -109,35 +133,36 @@ private function addConnectionNodes($rootNode) */ private function validate($rootNode) { - $rootNode->validate() - ->ifTrue(function (array $v) { - if (Constants::AUTH_KEY === $v['auth_type'] && empty($v['api_key'])) { - return true; - } + $rootNode + ->validate() + ->ifTrue(function (array $v) { + if (Constants::AUTH_KEY === $v['auth_type'] && empty($v['api_key'])) { + return true; + } - return false; - }) - ->thenInvalid('Api key is required as auth type is api_key') + return false; + }) + ->thenInvalid('Api key is required as auth type is api_key') ->end() ->validate() - ->ifTrue(function (array $v) { - if (Constants::AUTH_TLS === $v['auth_type']) { - return empty($v['tls_cert_path']) || empty($v['tls_key_path']); - } + ->ifTrue(function (array $v) { + if (Constants::AUTH_TLS === $v['auth_type']) { + return empty($v['tls_cert_path']) || empty($v['tls_key_path']); + } - return false; - }) - ->thenInvalid('Bouncer certificate and key paths are required for tls authentification.') + return false; + }) + ->thenInvalid('Bouncer certificate and key paths are required for tls authentification.') ->end() ->validate() - ->ifTrue(function (array $v) { - if (Constants::AUTH_TLS === $v['auth_type'] && true === $v['tls_verify_peer']) { - return empty($v['tls_ca_cert_path']); - } + ->ifTrue(function (array $v) { + if (Constants::AUTH_TLS === $v['auth_type'] && true === $v['tls_verify_peer']) { + return empty($v['tls_ca_cert_path']); + } - return false; - }) - ->thenInvalid('CA path is required for tls authentification with verify_peer.') + return false; + }) + ->thenInvalid('CA path is required for tls authentification with verify_peer.') ->end(); } } From 77e427c5e9a723b255d71aafca5124e67e50d69d Mon Sep 17 00:00:00 2001 From: Julien Loizelet Date: Tue, 10 Jan 2023 19:34:27 +0900 Subject: [PATCH 2/2] test(unit): Update unit test for cleaned config --- CHANGELOG.md | 11 +++++++++++ src/Constants.php | 2 +- tests/Unit/AbstractClientTest.php | 1 + tests/Unit/BouncerTest.php | 18 ++++++++++++++++++ tests/Unit/CurlTest.php | 1 + tests/Unit/FileGetContentsTest.php | 1 + 6 files changed, 33 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b14605c..0db57a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,17 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/) and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.4.0](https://github.com/crowdsecurity/php-lapi-client/releases/tag/v0.4.0) - 2023-01-12 +[_Compare with previous release_](https://github.com/crowdsecurity/php-lapi-client/compare/v0.3.0...v0.4.0) + +### Changed + +- Unexpected configuration keys are automatically removed by a new `cleanConfigs` method +- Update some logs + +--- + + ## [0.3.0](https://github.com/crowdsecurity/php-lapi-client/releases/tag/v0.3.0) - 2023-01-05 [_Compare with previous release_](https://github.com/crowdsecurity/php-lapi-client/compare/v0.2.0...v0.3.0) diff --git a/src/Constants.php b/src/Constants.php index 4d2fed0..511eeb5 100644 --- a/src/Constants.php +++ b/src/Constants.php @@ -37,5 +37,5 @@ class Constants /** * @var string The current version of this library */ - public const VERSION = 'v0.3.0'; + public const VERSION = 'v0.4.0'; } diff --git a/tests/Unit/AbstractClientTest.php b/tests/Unit/AbstractClientTest.php index 1a35055..8240469 100644 --- a/tests/Unit/AbstractClientTest.php +++ b/tests/Unit/AbstractClientTest.php @@ -29,6 +29,7 @@ * @uses \CrowdSec\LapiClient\Configuration::addConnectionNodes * @uses \CrowdSec\LapiClient\Configuration::validate * @uses \CrowdSec\LapiClient\RequestHandler\AbstractRequestHandler::__construct + * @uses \CrowdSec\LapiClient\Configuration::cleanConfigs * * @covers \CrowdSec\LapiClient\AbstractClient::__construct * @covers \CrowdSec\LapiClient\AbstractClient::getConfig diff --git a/tests/Unit/BouncerTest.php b/tests/Unit/BouncerTest.php index bf75f49..9a501c0 100644 --- a/tests/Unit/BouncerTest.php +++ b/tests/Unit/BouncerTest.php @@ -32,6 +32,9 @@ * @uses \CrowdSec\LapiClient\RequestHandler\Curl::handle * @uses \CrowdSec\LapiClient\RequestHandler\AbstractRequestHandler::__construct * + * + * + * @covers \CrowdSec\LapiClient\Configuration::cleanConfigs * @covers \CrowdSec\LapiClient\Bouncer::__construct * @covers \CrowdSec\LapiClient\Bouncer::configure * @covers \CrowdSec\LapiClient\Bouncer::manageRequest @@ -274,5 +277,20 @@ public function testConfigure() $error, 'CA cert path should be required if verify peer is true' ); + // Unexpected conf + $client = new Bouncer(['api_key' => '1111', 'user_agent_version' => 'v4.56.7', 'unexpected' => true]); + + $this->assertEquals( + 'v4.56.7', + $client->getConfig('user_agent_version'), + 'user_agent_version should be configurable' + ); + + $this->assertEquals( + null, + $client->getConfig('unexpected'), + 'Unexpected config should have been removed with no exception' + ); + } } diff --git a/tests/Unit/CurlTest.php b/tests/Unit/CurlTest.php index 498d8c4..8149760 100644 --- a/tests/Unit/CurlTest.php +++ b/tests/Unit/CurlTest.php @@ -34,6 +34,7 @@ * @uses \CrowdSec\LapiClient\Bouncer::formatUserAgent * @uses \CrowdSec\LapiClient\Configuration::addConnectionNodes * @uses \CrowdSec\LapiClient\Configuration::validate + * @uses \CrowdSec\LapiClient\Configuration::cleanConfigs * * @covers \CrowdSec\LapiClient\RequestHandler\Curl::createOptions * @covers \CrowdSec\LapiClient\RequestHandler\Curl::handle diff --git a/tests/Unit/FileGetContentsTest.php b/tests/Unit/FileGetContentsTest.php index bd6483f..2252734 100644 --- a/tests/Unit/FileGetContentsTest.php +++ b/tests/Unit/FileGetContentsTest.php @@ -38,6 +38,7 @@ * @uses \CrowdSec\LapiClient\Bouncer::manageRequest * @uses \CrowdSec\LapiClient\Configuration::addConnectionNodes * @uses \CrowdSec\LapiClient\Configuration::validate + * @uses \CrowdSec\LapiClient\Configuration::cleanConfigs * * @covers \CrowdSec\LapiClient\RequestHandler\FileGetContents::handle * @covers \CrowdSec\LapiClient\RequestHandler\FileGetContents::createContextConfig