From 752bbc1531a99369080806710540231b6799b756 Mon Sep 17 00:00:00 2001 From: Tony Murray Date: Fri, 18 Nov 2022 16:27:56 -0600 Subject: [PATCH] Port search API search more than one fields (#14646) * Fix port search columns * Port search API search more than one fields Fixup port APIs Change validate_column_list api helper to throw a renderable exception on error and return the valid columns DeviceCache::get() can handle a bigger range of input * whitespace * Refactor exceptions a bit * change throws type to be more generic * Lint fixes --- LibreNMS/Exceptions/ApiClientException.php | 47 +++++++ LibreNMS/Exceptions/ApiException.php | 36 ++++-- .../InvalidTableColumnException.php | 35 +++++ app/ApiClients/RipeApi.php | 12 +- .../Controllers/Ajax/RipeNccApiController.php | 4 +- doc/API/Ports.md | 5 +- includes/html/api_functions.inc.php | 121 +++++++++--------- phpstan-baseline.neon | 20 --- 8 files changed, 177 insertions(+), 103 deletions(-) create mode 100644 LibreNMS/Exceptions/ApiClientException.php create mode 100644 LibreNMS/Exceptions/InvalidTableColumnException.php diff --git a/LibreNMS/Exceptions/ApiClientException.php b/LibreNMS/Exceptions/ApiClientException.php new file mode 100644 index 0000000000..d69e8411cd --- /dev/null +++ b/LibreNMS/Exceptions/ApiClientException.php @@ -0,0 +1,47 @@ +. + * + * @link https://www.librenms.org + * + * @copyright 2019 Tony Murray + * @author Tony Murray + */ + +namespace LibreNMS\Exceptions; + +class ApiClientException extends \Exception +{ + /** @var array */ + private $output; + + /** + * @param string $message + * @param array $output + */ + public function __construct($message = '', $output = []) + { + parent::__construct($message, 0, null); + $this->output = $output; + } + + public function getOutput(): array + { + return $this->output; + } +} diff --git a/LibreNMS/Exceptions/ApiException.php b/LibreNMS/Exceptions/ApiException.php index 88a84ea040..73b393eb15 100644 --- a/LibreNMS/Exceptions/ApiException.php +++ b/LibreNMS/Exceptions/ApiException.php @@ -1,5 +1,5 @@ . + * along with this program. If not, see . * - * @link https://www.librenms.org - * - * @copyright 2019 Tony Murray + * @package LibreNMS + * @link http://librenms.org + * @copyright 2022 Tony Murray * @author Tony Murray */ namespace LibreNMS\Exceptions; +use Illuminate\Http\JsonResponse; + class ApiException extends \Exception { - private $output; - - public function __construct($message = '', $output = []) + /** + * @param string $message + * @param int $code + * @param \Throwable|null $previous + */ + public function __construct($message = '', $code = 400, $previous = null) { - parent::__construct($message, 0, null); - $this->output = $output; + parent::__construct($message, $code, $previous); } - public function getOutput() + /** + * Render the exception into an HTTP response. + * + * @param \Illuminate\Http\Request $request + */ + public function render($request): JsonResponse { - return $this->output; + return response()->json([ + 'status' => 'error', + 'message' => $this->getMessage(), + ], $this->getCode(), [], JSON_PRETTY_PRINT); } } diff --git a/LibreNMS/Exceptions/InvalidTableColumnException.php b/LibreNMS/Exceptions/InvalidTableColumnException.php new file mode 100644 index 0000000000..115845c500 --- /dev/null +++ b/LibreNMS/Exceptions/InvalidTableColumnException.php @@ -0,0 +1,35 @@ +. + * + * @package LibreNMS + * @link http://librenms.org + * @copyright 2022 Tony Murray + * @author Tony Murray + */ + +namespace LibreNMS\Exceptions; + +class InvalidTableColumnException extends ApiException +{ + public function __construct( + public readonly array $columns + ) { + parent::__construct('Invalid columns: ' . join(',', $this->columns)); + } +} diff --git a/app/ApiClients/RipeApi.php b/app/ApiClients/RipeApi.php index ee9fdc7c84..95a1eac6bd 100644 --- a/app/ApiClients/RipeApi.php +++ b/app/ApiClients/RipeApi.php @@ -26,7 +26,7 @@ namespace App\ApiClients; use GuzzleHttp\Exception\RequestException; -use LibreNMS\Exceptions\ApiException; +use LibreNMS\Exceptions\ApiClientException; class RipeApi extends BaseApi { @@ -38,7 +38,7 @@ class RipeApi extends BaseApi /** * Get whois info * - * @throws ApiException + * @throws ApiClientException */ public function getWhois(string $resource): array { @@ -52,7 +52,7 @@ class RipeApi extends BaseApi /** * Get Abuse contact * - * @throws ApiException + * @throws ApiClientException */ public function getAbuseContact(string $resource): mixed { @@ -64,7 +64,7 @@ class RipeApi extends BaseApi } /** - * @throws ApiException + * @throws ApiClientException */ private function makeApiCall(string $uri, array $options): mixed { @@ -73,13 +73,13 @@ class RipeApi extends BaseApi if (isset($response_data['status']) && $response_data['status'] == 'ok') { return $response_data; } else { - throw new ApiException('RIPE API call failed', $response_data); + throw new ApiClientException('RIPE API call failed', $response_data); } } catch (RequestException $e) { $message = 'RIPE API call to ' . $e->getRequest()->getUri() . ' failed: '; $message .= $e->getResponse()->getReasonPhrase() . ' ' . $e->getResponse()->getStatusCode(); - throw new ApiException( + throw new ApiClientException( $message, json_decode($e->getResponse()->getBody(), true) ); diff --git a/app/Http/Controllers/Ajax/RipeNccApiController.php b/app/Http/Controllers/Ajax/RipeNccApiController.php index f17c80bb01..2275b9244e 100644 --- a/app/Http/Controllers/Ajax/RipeNccApiController.php +++ b/app/Http/Controllers/Ajax/RipeNccApiController.php @@ -28,7 +28,7 @@ namespace App\Http\Controllers\Ajax; use App\ApiClients\RipeApi; use App\Http\Controllers\Controller; use Illuminate\Http\Request; -use LibreNMS\Exceptions\ApiException; +use LibreNMS\Exceptions\ApiClientException; class RipeNccApiController extends Controller { @@ -50,7 +50,7 @@ class RipeNccApiController extends Controller 'message' => 'Queried', 'output' => $output, ]); - } catch (ApiException $e) { + } catch (ApiClientException $e) { $response = $e->getOutput(); $message = $e->getMessage(); diff --git a/doc/API/Ports.md b/doc/API/Ports.md index a3ce88abf0..2fd624c4f2 100644 --- a/doc/API/Ports.md +++ b/doc/API/Ports.md @@ -87,13 +87,14 @@ Output: } ``` -### `search_ports in specific column` +### `search_ports in specific field(s)` Specific search for ports matching the query. Route: `/api/v0/ports/search/:field/:search` -- search string to search in field specified by field +- field: comma separated list of field(s) to search +- search: string to search in fields Input: diff --git a/includes/html/api_functions.inc.php b/includes/html/api_functions.inc.php index efae6e5e4b..7b49f2a4ef 100644 --- a/includes/html/api_functions.inc.php +++ b/includes/html/api_functions.inc.php @@ -27,6 +27,7 @@ use App\Models\Sensor; use App\Models\ServiceTemplate; use App\Models\UserPref; use Illuminate\Database\Eloquent\Builder; +use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; use Illuminate\Routing\Router; use Illuminate\Support\Arr; @@ -35,13 +36,14 @@ use Illuminate\Support\Str; use LibreNMS\Alerting\QueryBuilderParser; use LibreNMS\Config; use LibreNMS\Exceptions\InvalidIpException; +use LibreNMS\Exceptions\InvalidTableColumnException; use LibreNMS\Util\Graph; use LibreNMS\Util\IP; use LibreNMS\Util\IPv4; use LibreNMS\Util\Number; use LibreNMS\Util\Rewrite; -function api_success($result, $result_name, $message = null, $code = 200, $count = null, $extra = null) +function api_success($result, $result_name, $message = null, $code = 200, $count = null, $extra = null): JsonResponse { if (isset($result) && ! isset($result_name)) { return api_error(500, 'Result name not specified'); @@ -68,12 +70,12 @@ function api_success($result, $result_name, $message = null, $code = 200, $count return response()->json($output, $code, [], JSON_PRETTY_PRINT); } // end api_success() -function api_success_noresult($code, $message = null) +function api_success_noresult($code, $message = null): JsonResponse { return api_success(null, null, $message, $code); } // end api_success_noresult -function api_error($statusCode, $message) +function api_error($statusCode, $message): JsonResponse { return response()->json([ 'status' => 'error', @@ -81,7 +83,7 @@ function api_error($statusCode, $message) ], $statusCode, [], JSON_PRETTY_PRINT); } // end api_error() -function api_not_found() +function api_not_found(): JsonResponse { return api_error(404, "This API route doesn't exist."); } @@ -970,25 +972,16 @@ function list_available_wireless_graphs(Illuminate\Http\Request $request) }); } -function get_port_graphs(Illuminate\Http\Request $request) +/** + * @throws \LibreNMS\Exceptions\ApiException + */ +function get_port_graphs(Illuminate\Http\Request $request): JsonResponse { - $hostname = $request->route('hostname'); - $columns = $request->get('columns', 'ifName'); + $device = DeviceCache::get($request->route('hostname')); + $columns = validate_column_list($request->get('columns'), 'ports', ['ifName']); - if (($validate = validate_column_list($columns, 'ports')) !== true) { - return $validate; - } - - // use hostname as device_id if it's all digits - $device_id = ctype_digit($hostname) ? $hostname : getidbyname($hostname); - $sql = ''; - $params = [$device_id]; - if (! device_permitted($device_id)) { - $sql = 'AND `port_id` IN (select `port_id` from `ports_perms` where `user_id` = ?)'; - array_push($params, Auth::id()); - } - - $ports = dbFetchRows("SELECT $columns FROM `ports` WHERE `device_id` = ? AND `deleted` = '0' $sql ORDER BY `ifIndex`", $params); + $ports = $device->ports()->isNotDeleted()->hasAccess(Auth::user()) + ->select($columns)->orderBy('ifIndex')->get(); return api_success($ports, 'ports'); } @@ -1052,29 +1045,31 @@ function get_port_info(Illuminate\Http\Request $request) }); } -function search_ports(Illuminate\Http\Request $request) +/** + * @throws \LibreNMS\Exceptions\ApiException + */ +function search_ports(Illuminate\Http\Request $request): JsonResponse { + $columns = validate_column_list($request->get('columns'), 'ports', ['device_id', 'port_id', 'ifIndex', 'ifName']); $field = $request->route('field'); $search = $request->route('search'); - $columns = $request->get('columns'); - if (($validate = validate_column_list($columns, 'ports')) !== true) { - return $validate; + + // if only field is set, swap values + if (empty($search)) { + [$field, $search] = [$search, $field]; } + $fields = validate_column_list($field, 'ports', ['ifAlias', 'ifDescr', 'ifName']); - $query = Port::hasAccess(Auth::user()) - ->select(['device_id', 'port_id', 'ifIndex', 'ifName', $columns]); - - if (isset($search)) { - $query->where($field, 'like', "%$search%"); - } else { - $value = "%$field%"; - $query->where('ifAlias', 'like', $value) - ->orWhere('ifDescr', 'like', $value) - ->orWhere('ifName', 'like', $value); - } - - $ports = $query->orderBy('ifName') - ->get(); + $ports = Port::hasAccess(Auth::user()) + ->isNotDeleted() + ->where(function ($query) use ($fields, $search) { + foreach ($fields as $field) { + $query->orWhere($field, 'like', "%$search%"); + } + }) + ->select($columns) + ->orderBy('ifName') + ->get(); if ($ports->isEmpty()) { return api_error(404, 'No ports found'); @@ -1083,21 +1078,17 @@ function search_ports(Illuminate\Http\Request $request) return api_success($ports, 'ports'); } -function get_all_ports(Illuminate\Http\Request $request) +/** + * @throws \LibreNMS\Exceptions\ApiException + */ +function get_all_ports(Illuminate\Http\Request $request): JsonResponse { - $columns = $request->get('columns', 'port_id, ifName'); - if (($validate = validate_column_list($columns, 'ports')) !== true) { - return $validate; - } + $columns = validate_column_list($request->get('columns'), 'ports', ['port_id', 'ifName']); - $params = []; - $sql = ''; - if (! Auth::user()->hasGlobalRead()) { - $sql = ' AND (device_id IN (SELECT device_id FROM devices_perms WHERE user_id = ?) OR port_id IN (SELECT port_id FROM ports_perms WHERE user_id = ?))'; - array_push($params, Auth::id()); - array_push($params, Auth::id()); - } - $ports = dbFetchRows("SELECT $columns FROM `ports` WHERE `deleted` = 0 $sql", $params); + $ports = Port::hasAccess(Auth::user()) + ->select($columns) + ->isNotDeleted() + ->get(); return api_success($ports, 'ports'); } @@ -1119,7 +1110,7 @@ function get_port_stack(Illuminate\Http\Request $request) }); } -function update_device_port_notes(Illuminate\Http\Request $request): \Illuminate\Http\JsonResponse +function update_device_port_notes(Illuminate\Http\Request $request): JsonResponse { $portid = $request->route('portid'); @@ -1153,7 +1144,10 @@ function list_alert_rules(Illuminate\Http\Request $request) return api_success($rules->toArray($request), 'rules'); } -function list_alerts(Illuminate\Http\Request $request) +/** + * @throws \LibreNMS\Exceptions\ApiException + */ +function list_alerts(Illuminate\Http\Request $request): JsonResponse { $id = $request->route('id'); @@ -1191,9 +1185,7 @@ function list_alerts(Illuminate\Http\Request $request) if ($request->has('order')) { [$sort_column, $sort_order] = explode(' ', $request->get('order'), 2); - if (($res = validate_column_list($sort_column, 'alerts')) !== true) { - return $res; - } + validate_column_list($sort_column, 'alerts'); if (in_array($sort_order, ['asc', 'desc'])) { $order = $request->get('order'); } @@ -2618,22 +2610,29 @@ function list_logs(Illuminate\Http\Request $request, Router $router) return api_success($logs, 'logs', null, 200, null, ['total' => $count]); } -function validate_column_list($columns, $tableName) +/** + * @throws \LibreNMS\Exceptions\ApiException + */ +function validate_column_list(?string $columns, string $table, array $default = []): array { + if ($columns == '') { // no user input, return default + return $default; + } + static $schema; if (is_null($schema)) { $schema = new \LibreNMS\DB\Schema(); } $column_names = is_array($columns) ? $columns : explode(',', $columns); - $valid_columns = $schema->getColumns($tableName); + $valid_columns = $schema->getColumns($table); $invalid_columns = array_diff(array_map('trim', $column_names), $valid_columns); if (count($invalid_columns) > 0) { - return api_error(400, 'Invalid columns: ' . join(',', $invalid_columns)); + throw new InvalidTableColumnException($invalid_columns); } - return true; + return $column_names; } function missing_fields($required_fields, $data) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 21d3b51fcf..6e10490846 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -2430,26 +2430,6 @@ parameters: count: 1 path: LibreNMS/Device/YamlDiscovery.php - - - message: "#^Method LibreNMS\\\\Exceptions\\\\ApiException\\:\\:__construct\\(\\) has parameter \\$message with no type specified\\.$#" - count: 1 - path: LibreNMS/Exceptions/ApiException.php - - - - message: "#^Method LibreNMS\\\\Exceptions\\\\ApiException\\:\\:__construct\\(\\) has parameter \\$output with no type specified\\.$#" - count: 1 - path: LibreNMS/Exceptions/ApiException.php - - - - message: "#^Method LibreNMS\\\\Exceptions\\\\ApiException\\:\\:getOutput\\(\\) has no return type specified\\.$#" - count: 1 - path: LibreNMS/Exceptions/ApiException.php - - - - message: "#^Property LibreNMS\\\\Exceptions\\\\ApiException\\:\\:\\$output has no type specified\\.$#" - count: 1 - path: LibreNMS/Exceptions/ApiException.php - - message: "#^Method LibreNMS\\\\Exceptions\\\\AuthenticationException\\:\\:__construct\\(\\) has parameter \\$code with no type specified\\.$#" count: 1