From 8727b520143a2115c135ffd3d3bfd8b7ae269482 Mon Sep 17 00:00:00 2001 From: Julien Loizelet Date: Fri, 30 Dec 2022 11:28:40 +0900 Subject: [PATCH] feat(*): Add logs and avoid generic Exception for createSignal method --- CHANGELOG.md | 13 +++++ src/AbstractClient.php | 12 +++- src/Constants.php | 4 +- src/Watcher.php | 109 ++++++++++++++++++++++++++++++------- tests/Unit/WatcherTest.php | 7 +-- 5 files changed, 118 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0bf0a75..197e914 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,19 @@ 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.8.0](https://github.com/crowdsecurity/php-capi-client/releases/tag/v0.8.0) - 2022-12-30 +[_Compare with previous release_](https://github.com/crowdsecurity/php-capi-client/compare/v0.7.0...v0.8.0) + +### Added + +- Add some relevant debug and error logs + +### Changed + +- `createSignal` throws now a `ClientException` instead of a generic `Exception` during date manipulation + +--- + ## [0.7.0](https://github.com/crowdsecurity/php-capi-client/releases/tag/v0.7.0) - 2022-12-29 [_Compare with previous release_](https://github.com/crowdsecurity/php-capi-client/compare/v0.6.2...v0.7.0) diff --git a/src/AbstractClient.php b/src/AbstractClient.php index 11f9824..4289588 100644 --- a/src/AbstractClient.php +++ b/src/AbstractClient.php @@ -96,7 +96,12 @@ protected function request( ): array { $method = strtoupper($method); if (!in_array($method, $this->allowedMethods)) { - throw new ClientException("Method ($method) is not allowed."); + $message = "Method ($method) is not allowed."; + $this->logger->error('', [ + 'type' => 'WATCHER_CLIENT_REQUEST', + 'message' => $message, + ]); + throw new ClientException($message); } $response = $this->sendRequest( @@ -108,6 +113,7 @@ protected function request( /** * @codeCoverageIgnore + * * @throws ClientException */ private function sendRequest(Request $request): Response @@ -136,6 +142,10 @@ private function formatResponseBody(Response $response): array if ($statusCode < 200 || $statusCode >= 300) { $message = "Unexpected response status code: $statusCode. Body was: " . str_replace("\n", '', $body); + $this->logger->error('', [ + 'type' => 'WATCHER_CLIENT_FORMAT_RESPONSE', + 'message' => $message, + ]); throw new ClientException($message, $statusCode); } diff --git a/src/Constants.php b/src/Constants.php index 24a848b..d4e2d43 100644 --- a/src/Constants.php +++ b/src/Constants.php @@ -17,7 +17,7 @@ class Constants { /** - * @var int The default timeout (in seconds) when calling CAPI. + * @var int the default timeout (in seconds) when calling CAPI */ public const API_TIMEOUT = 120; /** @@ -63,5 +63,5 @@ class Constants /** * @var string The current version of this library */ - public const VERSION = 'v0.7.0'; + public const VERSION = 'v0.8.0'; } diff --git a/src/Watcher.php b/src/Watcher.php index 599a062..d114954 100644 --- a/src/Watcher.php +++ b/src/Watcher.php @@ -8,7 +8,6 @@ use CrowdSec\CapiClient\RequestHandler\RequestHandlerInterface; use CrowdSec\CapiClient\Storage\StorageInterface; use DateTime; -use DateTimeInterface; use DateTimeZone; use Psr\Log\LoggerInterface; use Symfony\Component\Config\Definition\Processor; @@ -116,6 +115,7 @@ public function __construct( * Process an enroll call to CAPI. * * @see https://crowdsecurity.github.io/api_doc/index.html?urls.primaryName=CAPI#/watchers/post_watchers_enroll + * * @throws ClientException */ public function enroll(string $name, bool $overwrite, string $enrollKey, array $tags = []): array @@ -136,6 +136,7 @@ public function enroll(string $name, bool $overwrite, string $enrollKey, array $ * Process a decisions stream call to CAPI. * * @see https://crowdsecurity.github.io/api_doc/index.html?urls.primaryName=CAPI#/watchers/get_decisions_stream + * * @throws ClientException */ public function getStreamDecisions(): array @@ -147,6 +148,7 @@ public function getStreamDecisions(): array * Process a signals call to CAPI. * * @see https://crowdsecurity.github.io/api_doc/index.html?urls.primaryName=CAPI#/watchers/post_signals + * * @throws ClientException */ public function pushSignals(array $signals): array @@ -164,22 +166,30 @@ private function convertSecondsToDuration(int $seconds): string /** * Helper to create well formatted signal array. - * @throws \Exception + * + * @throws ClientException */ public function createSignal( string $scenario, string $sourceValue, - ?DateTimeInterface $startAt, - ?DateTimeInterface $stopAt, + ?\DateTimeInterface $startAt, + ?\DateTimeInterface $stopAt, string $message = '', string $sourceScope = Constants::SCOPE_IP, int $decisionDuration = Constants::DURATION, string $decisionType = Constants::REMEDIATION_BAN ): array { - $currentTime = new DateTime('now', new DateTimeZone('UTC')); - $createdAt = $currentTime->format(Constants::DATE_FORMAT); - $startAt = $startAt ? $startAt->format(Constants::DATE_FORMAT) : $createdAt; - $stopAt = $stopAt ? $stopAt->format(Constants::DATE_FORMAT) : $createdAt; + try { + $currentTime = new DateTime('now', new DateTimeZone('UTC')); + $createdAt = $currentTime->format(Constants::DATE_FORMAT); + $startAt = $startAt ? $startAt->format(Constants::DATE_FORMAT) : $createdAt; + $stopAt = $stopAt ? $stopAt->format(Constants::DATE_FORMAT) : $createdAt; + // @codeCoverageIgnoreStart + } catch (\Exception $e) { + throw new ClientException('Something went wrong with date during signal creation'); + // @codeCoverageIgnoreEnd + } + $machineId = $this->storage->retrieveMachineId(); if (!$machineId) { $this->ensureRegister(); @@ -239,6 +249,7 @@ private function configure(array $configs): void /** * Ensure that machine is registered and that we have a token. + * * @throws ClientException */ private function ensureAuth(): void @@ -252,6 +263,7 @@ private function ensureAuth(): void /** * Ensure that machine credentials are ready tu use. + * * @throws ClientException */ private function ensureRegister(): void @@ -278,6 +290,7 @@ private function formatUserAgent(array $configs = []): string /** * Generate a random machine_id. + * * @throws ClientException */ private function generateMachineId(array $configs = []): string @@ -292,6 +305,7 @@ private function generateMachineId(array $configs = []): string /** * Generate a random password. + * * @throws ClientException */ private function generatePassword(): string @@ -333,7 +347,13 @@ private function handleLogin(): void $this->token = $loginResponse['token'] ?? null; if (!$this->token) { - throw new ClientException('Login response does not contain required token.', 401); + $message = 'Login response does not contain required token.'; + $this->logger->error('', [ + 'type' => 'WATCHER_CLIENT_HANDLE_LOGIN', + 'message' => $message, + 'response' => $loginResponse, + ]); + throw new ClientException($message, 401); } $this->storage->storeToken($this->token); $configScenarios = $this->getConfig('scenarios'); @@ -348,6 +368,11 @@ private function handleLogin(): void private function handleTokenHeader(): array { if (!$this->token) { + $message = 'Token is required.'; + $this->logger->error('', [ + 'type' => 'WATCHER_CLIENT_HANDLE_TOKEN', + 'message' => $message, + ]); throw new ClientException('Token is required.', 401); } @@ -358,6 +383,7 @@ private function handleTokenHeader(): array * Process a login call to CAPI. * * @see https://crowdsecurity.github.io/api_doc/index.html?urls.primaryName=CAPI#/watchers/post_watchers_login + * * @throws ClientException */ private function login(): array @@ -384,7 +410,7 @@ private function manageRequest( array $parameters = [] ): array { $this->logger->debug('', [ - 'type' => 'WATCHER_CLIENT_REQUEST', + 'type' => 'WATCHER_REQUEST', 'method' => $method, 'endpoint' => $endpoint, 'parameters' => $parameters, @@ -403,21 +429,37 @@ private function manageRequest( $headers = array_merge($this->headers, $this->handleTokenHeader()); $response = $this->request($method, $endpoint, $parameters, $headers); } catch (ClientException $e) { + $message = $e->getMessage(); + $code = $e->getCode(); /** * If there is an issue with credentials or token, CAPI returns a 401 error. * In this case only, we try to log in again. */ - if (401 !== $e->getCode()) { - throw new ClientException($e->getMessage(), $e->getCode()); + if (401 !== $code) { + $this->logger->error('', [ + 'type' => 'WATCHER_REQUEST_ERROR', + 'message' => $message, + 'code' => $code, + ]); + throw new ClientException($message, $code); } + $this->logger->warning('', [ + 'type' => 'WATCHER_REQUEST_ERROR_401', + 'message' => $message, + 'code' => $code, + ]); ++$loginRetry; $retry = true; $lastMessage = $e->getMessage(); } } while ($retry && ($loginRetry <= self::LOGIN_RETRY)); if ($loginRetry > self::LOGIN_RETRY) { - $message = "Could not login after $loginRetry attempts. Last error was: "; - throw new ClientException($message . $lastMessage); + $message = "Could not login after $loginRetry attempts. Last error was: $lastMessage"; + $this->logger->error('', [ + 'type' => 'WATCHER_REQUEST_TOO_MANY_ATTEMPTS', + 'message' => $message + ]); + throw new ClientException($message); } return $response; @@ -432,10 +474,20 @@ private function normalizeTags(array $tags): array { foreach ($tags as $tag) { if (!is_string($tag)) { - throw new ClientException('Tag must be a string: ' . gettype($tag) . ' given.', 500); + $message = 'Tag must be a string: ' . gettype($tag) . ' given.'; + $this->logger->error('', [ + 'type' => 'WATCHER_NORMALIZE_TAGS', + 'message' => $message, + ]); + throw new ClientException($message, 500); } if (empty($tag)) { - throw new ClientException('Tag must not be empty', 500); + $message = 'Tag must not be empty'; + $this->logger->error('', [ + 'type' => 'WATCHER_NORMALIZE_TAGS', + 'message' => $message, + ]); + throw new ClientException($message, 500); } } @@ -444,6 +496,7 @@ private function normalizeTags(array $tags): array /** * Generate and store new machine_id/password pair. + * * @throws ClientException */ private function refreshCredentials(): void @@ -481,21 +534,37 @@ private function register(): void $this->headers ); } catch (ClientException $e) { + $message = $e->getMessage(); + $code = $e->getCode(); /** * If the machine_id is already registered, CAPI returns a 500 error. * In this case only, we try to register again with new credentials. */ - if (500 !== $e->getCode()) { - throw new ClientException($e->getMessage(), $e->getCode()); + if (500 !== $code) { + $this->logger->error('', [ + 'type' => 'WATCHER_REGISTER_ERROR', + 'message' => $message, + 'code' => $code, + ]); + throw new ClientException($message, $code); } + $this->logger->warning('', [ + 'type' => 'WATCHER_REGISTER_ERROR_500', + 'message' => $message, + 'code' => $code, + ]); ++$registerRetry; $retry = true; $lastMessage = $e->getMessage(); } } while ($retry && ($registerRetry <= self::REGISTER_RETRY)); if ($registerRetry > self::REGISTER_RETRY) { - $message = "Could not register after $registerRetry attempts. Last error was: "; - throw new ClientException($message . $lastMessage); + $message = "Could not register after $registerRetry attempts. Last error was: $lastMessage"; + $this->logger->error('', [ + 'type' => 'WATCHER_REGISTER_TOO_MANY_ATTEMPTS', + 'message' => $message, + ]); + throw new ClientException($message); } } diff --git a/tests/Unit/WatcherTest.php b/tests/Unit/WatcherTest.php index 848e644..589a590 100644 --- a/tests/Unit/WatcherTest.php +++ b/tests/Unit/WatcherTest.php @@ -465,7 +465,6 @@ public function testConfigure() 'Each scenario respect some regex' ); - $error = ''; try { new Watcher(['machine_id_prefix' => 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaa'], new FileStorage()); @@ -503,7 +502,7 @@ public function testConfigure() ); $this->assertTrue( - (int) $client->getConfig('api_timeout') === Constants::API_TIMEOUT, + Constants::API_TIMEOUT === (int) $client->getConfig('api_timeout'), 'api timeout should be default' ); @@ -557,14 +556,14 @@ public function testConfigure() 'env should be dev or prod' ); - $client = new Watcher(['scenarios' => TestConstants::SCENARIOS, 'api_timeout' => 0], new FileStorage()); + $client = new Watcher(['scenarios' => TestConstants::SCENARIOS, 'api_timeout' => 0], new FileStorage()); $this->assertEquals( 0, $client->getConfig('api_timeout'), 'api timeout can be 0' ); - $client = new Watcher(['scenarios' => TestConstants::SCENARIOS, 'api_timeout' => -1], new FileStorage()); + $client = new Watcher(['scenarios' => TestConstants::SCENARIOS, 'api_timeout' => -1], new FileStorage()); $this->assertEquals( -1, $client->getConfig('api_timeout'),