[HLAPI] Fail search on invalid filters (#18512)

* fail hlapi search on invalid filters

* fix some schemas missing user fields

src/Glpi/Api/HL/APIException.php

@@ -46,7 +46,9 @@ class APIException extends \Exception
 {
     private string $user_message;
 
-    public function __construct(string $message = '', string $user_message = '', int $code = 0, ?Throwable $previous = null)
+    private string|array|null $details;
+
+    public function __construct(string $message = '', string $user_message = '', string|array|null $details = null, int $code = 0, ?Throwable $previous = null)
     {
         if ($user_message === '') {
             $user_message = __('An error occurred while processing your request.');
@@ -55,11 +57,17 @@ public function __construct(string $message = '', string $user_message = '', int
             $message = $user_message;
         }
         $this->user_message = $user_message;
+        $this->details = $details;
         parent::__construct($message, $code, $previous);
     }
 
     public function getUserMessage(): string
     {
         return $this->user_message;
     }
+
+    public function getDetails(): string|array|null
+    {
+        return $this->details;
+    }
 }

src/Glpi/Api/HL/Controller/AbstractController.php

@@ -226,12 +226,12 @@ protected function getMyUserID(): int
      * @param string $status
      * @phpstan-param self::ERROR_* $status
      * @param string $title
-     * @param ?string $detail
+     * @param string|array|null $detail
      * @param AdditionalErrorMessage[] $additionalMessages
      * @return array
      * @phpstan-return ErrorResponseBody
      */
-    public static function getErrorResponseBody(string $status, string $title, ?string $detail = null, array $additionalMessages = []): array
+    public static function getErrorResponseBody(string $status, string $title, string|array|null $detail = null, array $additionalMessages = []): array
     {
         $body = [
             'status' => $status,

src/Glpi/Api/HL/Controller/AdministrationController.php

@@ -729,8 +729,14 @@ private function getUsedOrManagedItems(int $users_id, bool $is_managed, array $r
     {
         /** @var array $CFG_GLPI */
         global $CFG_GLPI;
+
         // Create a union schema with all relevant item types
-        $schema = Doc\Schema::getUnionSchemaForItemtypes($CFG_GLPI['assignable_types'], $api_version);
+        $schema = Doc\Schema::getUnionSchemaForItemtypes(
+            itemtypes: array_filter($CFG_GLPI['assignable_types'], static function ($t) use ($is_managed) {
+                return (new $t())->isField($is_managed ? 'users_id_tech' : 'users_id');
+            }),
+            api_version: $api_version
+        );
         $rsql_filter = $request_params['filter'] ?? '';
         if (!empty($rsql_filter)) {
             $rsql_filter = "($rsql_filter);";

src/Glpi/Api/HL/Controller/AssetController.php

@@ -564,6 +564,8 @@ class: User::class,
                 'comment' => ['type' => Doc\Schema::TYPE_STRING],
                 'date_creation' => ['type' => Doc\Schema::TYPE_STRING, 'format' => Doc\Schema::FORMAT_STRING_DATE_TIME],
                 'date_mod' => ['type' => Doc\Schema::TYPE_STRING, 'format' => Doc\Schema::FORMAT_STRING_DATE_TIME],
+                'user' => self::getDropdownTypeSchema(class: User::class, full_schema: 'User'),
+                'user_tech' => self::getDropdownTypeSchema(class: User::class, field: 'users_id_tech', full_schema: 'User'),
                 'printer_models' => [
                     'type' => Doc\Schema::TYPE_ARRAY,
                     'description' => 'List of printer models that can use this cartridge',
@@ -671,6 +673,8 @@ class: User::class,
                 'comment' => ['type' => Doc\Schema::TYPE_STRING],
                 'date_creation' => ['type' => Doc\Schema::TYPE_STRING, 'format' => Doc\Schema::FORMAT_STRING_DATE_TIME],
                 'date_mod' => ['type' => Doc\Schema::TYPE_STRING, 'format' => Doc\Schema::FORMAT_STRING_DATE_TIME],
+                'user' => self::getDropdownTypeSchema(class: User::class, full_schema: 'User'),
+                'user_tech' => self::getDropdownTypeSchema(class: User::class, field: 'users_id_tech', full_schema: 'User'),
                 'consumables' => [
                     'type' => Doc\Schema::TYPE_ARRAY,
                     'description' => 'List of consumables',

src/Glpi/Api/HL/Controller/ComponentController.php

@@ -615,6 +615,7 @@ protected static function getRawKnownSchemas(): array
                     'msin' => ['type' => Doc\Schema::TYPE_STRING],
                     'line' => self::getDropdownTypeSchema(class: \Line::class, full_schema: 'Line'),
                     'user' => self::getDropdownTypeSchema(class: \User::class, full_schema: 'User'),
+                    'user_tech' => self::getDropdownTypeSchema(class: \User::class, field: 'users_id_tech', full_schema: 'User'),
                     'group' => self::getDropdownTypeSchema(class: \Group::class, full_schema: 'Group'),
                 ]
             ],

src/Glpi/Api/HL/RSQL/Error.php

@@ -0,0 +1,51 @@
+<?php
+
+/**
+ * ---------------------------------------------------------------------
+ *
+ * GLPI - Gestionnaire Libre de Parc Informatique
+ *
+ * http://glpi-project.org
+ *
+ * @copyright 2015-2024 Teclib' and contributors.
+ * @licence   https://www.gnu.org/licenses/gpl-3.0.html
+ *
+ * ---------------------------------------------------------------------
+ *
+ * LICENSE
+ *
+ * This file is part of GLPI.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ *
+ * ---------------------------------------------------------------------
+ */
+
+namespace Glpi\Api\HL\RSQL;
+
+enum Error: int
+{
+    case UNKNOWN_PROPERTY = 1;
+    case UNKNOWN_OPERATOR = 2;
+    case MAPPED_PROPERTY = 3;
+
+    public function getMessage(): string
+    {
+        return match ($this) {
+            self::UNKNOWN_PROPERTY => 'Unknown property',
+            self::UNKNOWN_OPERATOR => 'Unknown operator',
+            self::MAPPED_PROPERTY => 'Mapped properties cannot be used in RSQL',
+        };
+    }
+}

src/Glpi/Api/HL/RSQL/Parser.php

@@ -213,9 +213,9 @@ public function getOperators(): array
 
     /**
      * @param array $tokens Tokens from the RSQL lexer.
-     * @return QueryExpression SQL criteria
+     * @return Result RSQL query result
      */
-    public function parse(array $tokens): QueryExpression
+    public function parse(array $tokens): Result
     {
         $it = new \DBmysqlIterator($this->db);
         // We are building a SQL string instead of criteria array because it isn't worth the complexity or overhead.
@@ -230,6 +230,7 @@ public function parse(array $tokens): QueryExpression
         $operators = array_combine(array_column($operators, 'operator'), $operators);
 
         $flat_props = $this->search->getFlattenedProperties();
+        $invalid_filters = [];
 
         $buffer = [];
         while ($position < $token_count) {
@@ -238,6 +239,14 @@ public function parse(array $tokens): QueryExpression
                 // If the property isn't in the flattened properties array, the filter should be ignored (not valid)
                 if (!isset($flat_props[$value])) {
                     // Not valid. Just fill the buffer and continue to the next token. This will be handled once the value token is reached.
+                    $invalid_filters[$value] = Error::UNKNOWN_PROPERTY;
+                    $buffer = [
+                        'property' => null,
+                        'field' => null,
+                    ];
+                } else if (isset($flat_props[$value]['x-mapped-from'])) {
+                    // Mapped properties cannot be used in RSQL currently since they are calculated after the query is executed
+                    $invalid_filters[$value] = Error::MAPPED_PROPERTY;
                     $buffer = [
                         'property' => null,
                         'field' => null,
@@ -249,9 +258,16 @@ public function parse(array $tokens): QueryExpression
                     ];
                 }
             } else if ($type === Lexer::T_OPERATOR) {
-                $buffer['operator'] = $operators[$value]['sql_where_callable'];
+                if (!isset($operators[$value])) {
+                    if ($buffer['property'] !== null) {
+                        $invalid_filters[$buffer['property']] = Error::UNKNOWN_OPERATOR;
+                    }
+                    $buffer['operator'] = null;
+                } else {
+                    $buffer['operator'] = $operators[$value]['sql_where_callable'];
+                }
             } else if ($type === Lexer::T_VALUE) {
-                if ($buffer['property'] !== null && $buffer['field'] !== null) {
+                if ($buffer['property'] !== null && $buffer['operator'] !== null && $buffer['field'] !== null) {
                     // Unquote value if it is quoted
                     if (preg_match('/^".*"$/', $value) || preg_match("/^'.*'$/", $value)) {
                         $value = substr($value, 1, -1);
@@ -285,6 +301,6 @@ public function parse(array $tokens): QueryExpression
             $sql_string = '1';
         }
 
-        return new QueryExpression($sql_string);
+        return new Result(new QueryExpression($sql_string), $invalid_filters);
     }
 }

src/Glpi/Api/HL/RSQL/Result.php

@@ -0,0 +1,60 @@
+<?php
+
+/**
+ * ---------------------------------------------------------------------
+ *
+ * GLPI - Gestionnaire Libre de Parc Informatique
+ *
+ * http://glpi-project.org
+ *
+ * @copyright 2015-2024 Teclib' and contributors.
+ * @licence   https://www.gnu.org/licenses/gpl-3.0.html
+ *
+ * ---------------------------------------------------------------------
+ *
+ * LICENSE
+ *
+ * This file is part of GLPI.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ *
+ * ---------------------------------------------------------------------
+ */
+
+namespace Glpi\Api\HL\RSQL;
+
+use Glpi\DBAL\QueryExpression;
+
+class Result
+{
+    /**
+     * @param QueryExpression $sql_criteria
+     * @param array<string, Error> $invalid_filters
+     */
+    public function __construct(
+        private QueryExpression $sql_criteria,
+        private array $invalid_filters = []
+    ) {
+    }
+
+    public function getSQLCriteria(): QueryExpression
+    {
+        return $this->sql_criteria;
+    }
+
+    public function getInvalidFilters(): array
+    {
+        return $this->invalid_filters;
+    }
+}

src/Glpi/Api/HL/Search.php

@@ -375,7 +375,15 @@ private function getSearchCriteria(): array
 
         // Handle RSQL filter
         if (isset($this->request_params['filter']) && !empty($this->request_params['filter'])) {
-            $criteria['WHERE'] = [$this->rsql_parser->parse(Lexer::tokenize($this->request_params['filter']))];
+            $filter_result = $this->rsql_parser->parse(Lexer::tokenize($this->request_params['filter']));
+            // Fail the request if any of the filters are invalid
+            if (!empty($filter_result->getInvalidFilters())) {
+                throw new RSQLException(
+                    message: 'RSQL query has invalid filters',
+                    details: array_map(static fn ($rsql_error) => $rsql_error->getMessage(), $filter_result->getInvalidFilters())
+                );
+            }
+            $criteria['WHERE'] = [$filter_result->getSQLCriteria()];
         }
 
         // Handle entity and other visibility restrictions
@@ -1176,9 +1184,9 @@ public static function searchBySchema(array $schema, array $request_params): Res
         try {
             $results = self::getSearchResultsBySchema($schema, $request_params);
         } catch (RSQLException $e) {
-            return new JSONResponse(AbstractController::getErrorResponseBody(AbstractController::ERROR_INVALID_PARAMETER, $e->getMessage()), 400);
+            return new JSONResponse(AbstractController::getErrorResponseBody(AbstractController::ERROR_INVALID_PARAMETER, $e->getMessage(), $e->getDetails()), 400);
         } catch (APIException $e) {
-            return new JSONResponse(AbstractController::getErrorResponseBody(AbstractController::ERROR_GENERIC, $e->getUserMessage()));
+            return new JSONResponse(AbstractController::getErrorResponseBody(AbstractController::ERROR_GENERIC, $e->getUserMessage(), $e->getDetails()));
         } catch (\Throwable $e) {
             $message = (new APIException())->getUserMessage();
             return new JSONResponse(AbstractController::getErrorResponseBody(AbstractController::ERROR_GENERIC, $message));

tests/functional/Glpi/Api/HL/RSQL/Parser.php

@@ -130,7 +130,7 @@ public function testParse(array $tokens, string $expected)
         $search_class->getProperty('flattened_properties')->setValue($search, Schema::flattenProperties($schema['properties']));
         $search_class->getProperty('joins')->setValue($search, Schema::getJoins($schema['properties']));
         $parser = new \Glpi\Api\HL\RSQL\Parser($search);
-        $this->string((string)$parser->parse($tokens))->isEqualTo($expected);
+        $this->string((string)$parser->parse($tokens)->getSQLCriteria())->isEqualTo($expected);
     }
 
     /**
@@ -143,6 +143,11 @@ public function testIgnoreInvalidProperties()
             'properties' => [
                 'id' => ['type' => 'integer'],
                 'name' => ['type' => 'string'],
+                'mapped' => [
+                    'type' => 'string',
+                    'x-mapped-from' => 'id',
+                    'x-mapper' => static fn () => 'mapped'
+                ]
             ]
         ];
 
@@ -152,8 +157,23 @@ public function testIgnoreInvalidProperties()
         $search_class->getProperty('flattened_properties')->setValue($search, Schema::flattenProperties($schema['properties']));
         $search_class->getProperty('joins')->setValue($search, Schema::getJoins($schema['properties']));
         $parser = new \Glpi\Api\HL\RSQL\Parser($search);
-        $this->string((string)$parser->parse([[5, 'test'], [6, '=='], [7, 'test']]))->isEqualTo('1');
+
+        $result = $parser->parse([[5, 'test'], [6, '=='], [7, 'test']]);
+        $this->string((string)$result->getSQLCriteria())->isEqualTo('1');
+        $this->variable($result->getInvalidFilters()['test'])->isIdenticalTo(\Glpi\Api\HL\RSQL\Error::UNKNOWN_PROPERTY);
         // Test an invalid filter with a valid one
-        $this->string((string)$parser->parse([[5, 'test'], [6, '=='], [7, 'test'], [1, ';'], [5, 'name'], [6, '=='], [7, 'test']]))->isEqualTo("(`_`.`name` = 'test')");
+        $result = $parser->parse([[5, 'test'], [6, '=='], [7, 'test'], [1, ';'], [5, 'name'], [6, '=='], [7, 'test']]);
+        $this->string((string)$result->getSQLCriteria())->isEqualTo("(`_`.`name` = 'test')");
+        $this->variable($result->getInvalidFilters()['test'])->isIdenticalTo(\Glpi\Api\HL\RSQL\Error::UNKNOWN_PROPERTY);
+
+        // Test invalid operator
+        $result = $parser->parse([[5, 'name'], [6, '=f='], [7, 'test']]);
+        $this->string((string)$result->getSQLCriteria())->isEqualTo('1');
+        $this->variable($result->getInvalidFilters()['name'])->isIdenticalTo(\Glpi\Api\HL\RSQL\Error::UNKNOWN_OPERATOR);
+
+        // Mapped properties should be ignored
+        $result = $parser->parse([[5, 'mapped'], [6, '=='], [7, 'test']]);
+        $this->string((string)$result->getSQLCriteria())->isEqualTo('1');
+        $this->variable($result->getInvalidFilters()['mapped'])->isIdenticalTo(\Glpi\Api\HL\RSQL\Error::MAPPED_PROPERTY);
     }
 }