feat(request): Introduce required headers property

crowdsecurity · Sep 6, 2024 · b5ade8f · b5ade8f
1 parent 6d0afcc
commit b5ade8f
Show file tree

Hide file tree

Showing 8 changed files with 235 additions and 46 deletions.
diff --git a/src/Client/HttpMessage/AppSecRequest.php b/src/Client/HttpMessage/AppSecRequest.php
@@ -20,6 +20,17 @@ class AppSecRequest extends Request
      * @var array
      */
     protected $headers = [];
+    /**
+     * @var string[]
+     */
+    protected $requiredHeaders = [
+        'X-Crowdsec-Appsec-Ip',
+        'X-Crowdsec-Appsec-User-Agent',
+        'X-Crowdsec-Appsec-Verb',
+        'X-Crowdsec-Appsec-Uri',
+        'X-Crowdsec-Appsec-Host',
+        'X-Crowdsec-Appsec-Api-Key',
+    ];
     /**
      * @var string
      */

diff --git a/src/Client/HttpMessage/Request.php b/src/Client/HttpMessage/Request.php
@@ -4,6 +4,8 @@
 
 namespace CrowdSec\Common\Client\HttpMessage;
 
+use CrowdSec\Common\Client\ClientException;
+
 /**
  * Request that will be sent to CrowdSec.
  *
@@ -23,6 +25,12 @@ class Request extends AbstractMessage
         'Accept' => 'application/json',
         'Content-Type' => 'application/json',
     ];
+    /**
+     * @var string[]
+     */
+    protected $requiredHeaders = [
+        'User-Agent',
+    ];
     /**
      * @var string
      */
@@ -62,4 +70,21 @@ public function getUri(): string
     {
         return $this->uri;
     }
+
+    /**
+     * Retrieve validated headers.
+     *
+     * @throws ClientException
+     */
+    public function getValidatedHeaders(): array
+    {
+        $headers = $this->getHeaders();
+        foreach ($this->requiredHeaders as $requiredHeader) {
+            if (!isset($headers[$requiredHeader])) {
+                throw new ClientException("Header \"$requiredHeader\" is required", 400);
+            }
+        }
+
+        return $headers;
+    }
 }
diff --git a/src/Client/RequestHandler/Curl.php b/src/Client/RequestHandler/Curl.php
@@ -92,7 +92,11 @@ private function handleSSL(Request $request): array
     {
         $result = [\CURLOPT_SSL_VERIFYPEER => false];
         if ($request instanceof AppSecRequest) {
-            // AppSec does not require SSL verification
+            /**
+             * AppSec does not currently support TLS authentication.
+             *
+             * @see https://github.com/crowdsecurity/crowdsec/issues/3172
+             */
             return $result;
         }
 
@@ -113,7 +117,7 @@ private function handleSSL(Request $request): array
         return $result;
     }
 
-    private function handleMethod(string $method, string $url, array $parameters = [], $rawBody = ''): array
+    private function handleMethod(string $method, string $url, array $parameters = [], string $rawBody = ''): array
     {
         $result = [];
         if ('POST' === strtoupper($method)) {
@@ -145,19 +149,11 @@ private function handleMethod(string $method, string $url, array $parameters = [
      */
     private function createOptions(Request $request): array
     {
-        $isAppSec = $request instanceof AppSecRequest;
-        $headers = $request->getHeaders();
+        $headers = $request->getValidatedHeaders();
         $method = $request->getMethod();
         $url = $request->getUri();
         $parameters = $request->getParams();
-        if (!isset($headers['User-Agent']) && !$isAppSec) {
-            throw new ClientException('User agent is required', 400);
-        }
-        $rawBody = '';
-        if ($isAppSec) {
-            /** @var AppSecRequest $request */
-            $rawBody = $request->getRawBody();
-        }
+        $rawBody = $request instanceof AppSecRequest ? $request->getRawBody() : '';
         $options = [
             \CURLOPT_HEADER => false,
             \CURLOPT_RETURNTRANSFER => true,

diff --git a/src/Client/RequestHandler/FileGetContents.php b/src/Client/RequestHandler/FileGetContents.php
@@ -48,6 +48,19 @@ public function handle(Request $request): Response
         return new Response($responseBody, $status);
     }
 
+    /**
+     * Convert a key-value array of headers to the official HTTP header string.
+     */
+    protected function convertHeadersToString(array $headers): string
+    {
+        $builtHeaderString = '';
+        foreach ($headers as $key => $value) {
+            $builtHeaderString .= "$key: $value\r\n";
+        }
+
+        return $builtHeaderString;
+    }
+
     /**
      * @codeCoverageIgnore
      *
@@ -73,19 +86,6 @@ protected function getResponseHttpCode(array $parts): int
         return $status;
     }
 
-    /**
-     * Convert a key-value array of headers to the official HTTP header string.
-     */
-    protected function convertHeadersToString(array $headers): string
-    {
-        $builtHeaderString = '';
-        foreach ($headers as $key => $value) {
-            $builtHeaderString .= "$key: $value\r\n";
-        }
-
-        return $builtHeaderString;
-    }
-
     /**
      * Retrieve configuration for the stream content.
      *
@@ -95,11 +95,8 @@ protected function convertHeadersToString(array $headers): string
      */
     private function createContextConfig(Request $request): array
     {
-        $headers = $request->getHeaders();
+        $headers = $request->getValidatedHeaders();
         $isAppSec = $request instanceof AppSecRequest;
-        if (!isset($headers['User-Agent']) && !$isAppSec) {
-            throw new ClientException('User agent is required', 400);
-        }
         $rawBody = '';
         if ($isAppSec) {
             /** @var AppSecRequest $request */
@@ -133,7 +130,11 @@ private function handleSSL(Request $request): array
     {
         $result = ['ssl' => ['verify_peer' => false]];
         if ($request instanceof AppSecRequest) {
-            // AppSec does not require SSL verification
+            /**
+             * AppSec does not currently support TLS authentication.
+             *
+             * @see https://github.com/crowdsecurity/crowdsec/issues/3172
+             */
             return $result;
         }
         $authType = $this->getConfig('auth_type');

diff --git a/src/Constants.php b/src/Constants.php
@@ -84,14 +84,6 @@ class Constants
      * @var string The CrowdSec range scope for decisions
      */
     public const SCOPE_RANGE = 'range';
-    /**
-     * @var string The REST API type
-     */
-    public const TYPE_REST = 'rest';
-    /**
-     * @var string The APPSEC API type
-     */
-    public const TYPE_APPSEC = 'app_sec';
     /**
      * @var string The current version of this library
      */

diff --git a/tests/Unit/CurlTest.php b/tests/Unit/CurlTest.php
@@ -59,7 +59,25 @@ public function testHandleError()
         $this->assertEquals(400, $code);
 
         $this->assertEquals(
-            'User agent is required',
+            'Header "User-Agent" is required',
+            $error,
+            'Should failed and throw if no user agent'
+        );
+        // Test 1 bis AppSec header required
+        $request = new AppSecRequest('test-uri', 'POST', ['User-Agent' => 'ok']);
+        $error = '';
+        $code = 0;
+        try {
+            $mockCurlRequest->handle($request);
+        } catch (ClientException $e) {
+            $error = $e->getMessage();
+            $code = $e->getCode();
+        }
+
+        $this->assertEquals(400, $code);
+
+        $this->assertEquals(
+            'Header "X-Crowdsec-Appsec-Ip" is required',
             $error,
             'Should failed and throw if no user agent'
         );
@@ -275,7 +293,15 @@ public function testOptionsForAppSec()
     {
         $url = TestConstants::APPSEC_URL . '/';
         $method = 'POST';
-        $headers = ['X-Crowdsec-Appsec-test' => 'test-value'];
+        $headers = [
+            'X-Crowdsec-Appsec-Ip' => 'test-value',
+            'X-Crowdsec-Appsec-Host' => 'test-value',
+            'X-Crowdsec-Appsec-User-Agent' => 'test-value',
+            'X-Crowdsec-Appsec-Verb' => 'test-value',
+            'X-Crowdsec-Appsec-Method' => 'test-value',
+            'X-Crowdsec-Appsec-Uri' => 'test-value',
+            'X-Crowdsec-Appsec-Api-Key' => 'test-value',
+        ];
         $rawBody = 'this is raw body';
         $configs = $this->tlsConfigs;
 
@@ -291,7 +317,13 @@ public function testOptionsForAppSec()
             \CURLOPT_HEADER => false,
             \CURLOPT_RETURNTRANSFER => true,
             \CURLOPT_HTTPHEADER => [
-                'X-Crowdsec-Appsec-test:test-value',
+                'X-Crowdsec-Appsec-Ip:test-value',
+                'X-Crowdsec-Appsec-Host:test-value',
+                'X-Crowdsec-Appsec-User-Agent:test-value',
+                'X-Crowdsec-Appsec-Verb:test-value',
+                'X-Crowdsec-Appsec-Method:test-value',
+                'X-Crowdsec-Appsec-Uri:test-value',
+                'X-Crowdsec-Appsec-Api-Key:test-value',
             ],
             \CURLOPT_POST => true,
             \CURLOPT_POSTFIELDS => 'this is raw body',
@@ -325,7 +357,13 @@ public function testOptionsForAppSec()
             \CURLOPT_RETURNTRANSFER => true,
             \CURLOPT_USERAGENT => TestConstants::USER_AGENT_SUFFIX,
             \CURLOPT_HTTPHEADER => [
-                'X-Crowdsec-Appsec-test:test-value',
+                'X-Crowdsec-Appsec-Ip:test-value',
+                'X-Crowdsec-Appsec-Host:test-value',
+                'X-Crowdsec-Appsec-User-Agent:test-value',
+                'X-Crowdsec-Appsec-Verb:test-value',
+                'X-Crowdsec-Appsec-Method:test-value',
+                'X-Crowdsec-Appsec-Uri:test-value',
+                'X-Crowdsec-Appsec-Api-Key:test-value',
                 'User-Agent:' . TestConstants::USER_AGENT_SUFFIX,
             ],
             \CURLOPT_POST => false,

diff --git a/tests/Unit/FileGetContentsTest.php b/tests/Unit/FileGetContentsTest.php
@@ -160,7 +160,15 @@ public function testContextConfig()
     public function testContextConfigForAppSec()
     {
         $method = 'POST';
-        $headers = ['X-CrowdSec-AppSec-test' => 'test-value'];
+        $headers = [
+            'X-Crowdsec-Appsec-Ip' => 'test-value',
+            'X-Crowdsec-Appsec-Host' => 'test-value',
+            'X-Crowdsec-Appsec-User-Agent' => 'test-value',
+            'X-Crowdsec-Appsec-Verb' => 'test-value',
+            'X-Crowdsec-Appsec-Method' => 'test-value',
+            'X-Crowdsec-Appsec-Uri' => 'test-value',
+            'X-Crowdsec-Appsec-Api-Key' => 'test-value',
+        ];
         $rawBody = 'This is a raw body';
         $configs = $this->tlsConfigs;
 
@@ -179,7 +187,13 @@ public function testContextConfigForAppSec()
         $expected = [
             'http' => [
                 'method' => $method,
-                'header' => 'X-CrowdSec-AppSec-test: test-value
+                'header' => 'X-Crowdsec-Appsec-Ip: test-value
+X-Crowdsec-Appsec-Host: test-value
+X-Crowdsec-Appsec-User-Agent: test-value
+X-Crowdsec-Appsec-Verb: test-value
+X-Crowdsec-Appsec-Method: test-value
+X-Crowdsec-Appsec-Uri: test-value
+X-Crowdsec-Appsec-Api-Key: test-value
 ',
                 'ignore_errors' => true,
                 'content' => 'This is a raw body',
@@ -211,7 +225,13 @@ public function testContextConfigForAppSec()
         $expected = [
             'http' => [
                 'method' => $method,
-                'header' => 'X-CrowdSec-AppSec-test: test-value
+                'header' => 'X-Crowdsec-Appsec-Ip: test-value
+X-Crowdsec-Appsec-Host: test-value
+X-Crowdsec-Appsec-User-Agent: test-value
+X-Crowdsec-Appsec-Verb: test-value
+X-Crowdsec-Appsec-Method: test-value
+X-Crowdsec-Appsec-Uri: test-value
+X-Crowdsec-Appsec-Api-Key: test-value
 User-Agent: ' . TestConstants::USER_AGENT_SUFFIX . '
 ',
                 'ignore_errors' => true,
@@ -242,7 +262,21 @@ public function testHandleError()
         }
 
         $this->assertEquals(
-            'User agent is required',
+            'Header "User-Agent" is required',
+            $error,
+            'Should failed and throw if no user agent'
+        );
+
+        $request = new AppSecRequest('test-uri', 'POST', ['User-Agent' => 'test']);
+        $error = false;
+        try {
+            $mockFGCRequest->handle($request);
+        } catch (ClientException $e) {
+            $error = $e->getMessage();
+        }
+
+        $this->assertEquals(
+            'Header "X-Crowdsec-Appsec-Ip" is required',
             $error,
             'Should failed and throw if no user agent'
         );