Merge pull request #20 from julienloizelet/feat/create-signal-exception

feat(*): Add logs and avoid generic Exception for createSignal method
crowdsecurity · Dec 30, 2022 · bc3354e · bc3354e
2 parents dfa0917 + 8727b52
commit bc3354e
Show file tree

Hide file tree

Showing 5 changed files with 118 additions and 27 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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'),