fix(Controller): respond with 400 on InvalidArgumentExceptions

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
This commit is contained in:
Arthur Schiwon
2024-08-21 16:21:09 +02:00
parent 3eeb1101b9
commit 94da27afcb
5 changed files with 71 additions and 16 deletions

View File

@ -10,6 +10,7 @@
namespace OCA\Tables\Controller;
use Exception;
use InvalidArgumentException;
use OCA\Tables\Api\V1Api;
use OCA\Tables\AppInfo\Application;
use OCA\Tables\Db\ViewMapper;
@ -376,9 +377,10 @@ class Api1Controller extends ApiController {
*
* @param int $viewId View ID
* @param array{key: 'title'|'emoji'|'description', value: string}|array{key: 'columns', value: int[]}|array{key: 'sort', value: array{columnId: int, mode: 'ASC'|'DESC'}}|array{key: 'filter', value: array{columnId: int, operator: 'begins-with'|'ends-with'|'contains'|'is-equal'|'is-greater-than'|'is-greater-than-or-equal'|'is-lower-than'|'is-lower-than-or-equal'|'is-empty', value: string|int|float}} $data key-value pairs
* @return DataResponse<Http::STATUS_OK, TablesView, array{}>|DataResponse<Http::STATUS_FORBIDDEN|Http::STATUS_INTERNAL_SERVER_ERROR, array{message: string}, array{}>
* @return DataResponse<Http::STATUS_OK, TablesView, array{}>|DataResponse<Http::STATUS_FORBIDDEN|Http::STATUS_BAD_REQUEST|Http::STATUS_INTERNAL_SERVER_ERROR, array{message: string}, array{}>
*
* 200: View updated
* 400: Invalid data
* 403: No permissions
* 404: Not found
*/
@ -389,6 +391,10 @@ class Api1Controller extends ApiController {
$this->logger->warning('A permission error occurred: ' . $e->getMessage(), ['exception' => $e]);
$message = ['message' => $e->getMessage()];
return new DataResponse($message, Http::STATUS_FORBIDDEN);
} catch (InvalidArgumentException $e) {
$this->logger->warning('An invalid request occurred: ' . $e->getMessage(), ['exception' => $e]);
$message = ['message' => $e->getMessage()];
return new DataResponse($message, Http::STATUS_BAD_REQUEST);
} catch (InternalError|Exception $e) {
$this->logger->error('An internal error or exception occurred: '.$e->getMessage(), ['exception' => $e]);
$message = ['message' => $e->getMessage()];

View File

@ -8,6 +8,7 @@
namespace OCA\Tables\Controller;
use Closure;
use InvalidArgumentException;
use OCA\Tables\Errors\InternalError;
use OCA\Tables\Errors\NotFoundError;
use OCA\Tables\Errors\PermissionError;
@ -20,13 +21,17 @@ trait Errors {
try {
return new DataResponse($callback());
} catch (PermissionError $e) {
$this->logger->warning('A permission error accured: '.$e->getMessage(), ['exception' => $e]);
$this->logger->warning('A permission error occurred: '.$e->getMessage(), ['exception' => $e]);
$message = ['message' => $e->getMessage()];
return new DataResponse($message, Http::STATUS_FORBIDDEN);
} catch (NotFoundError $e) {
$this->logger->warning('A not found error accured: '.$e->getMessage(), ['exception' => $e]);
$this->logger->warning('A not found error occurred: ' . $e->getMessage(), ['exception' => $e]);
$message = ['message' => $e->getMessage()];
return new DataResponse($message, Http::STATUS_NOT_FOUND);
} catch (InvalidArgumentException $e) {
$this->logger->warning('An invalid request occurred: ' . $e->getMessage(), ['exception' => $e]);
$message = ['message' => $e->getMessage()];
return new DataResponse($message, Http::STATUS_BAD_REQUEST);
} catch (InternalError|\Exception $e) {
$this->logger->error('An internal error or exception occurred: '.$e->getMessage(), ['exception' => $e]);
$message = ['message' => $e->getMessage()];

View File

@ -11,13 +11,12 @@ namespace OCA\Tables\Service;
use DateTime;
use Exception;
use InvalidArgumentException;
use OCA\Tables\AppInfo\Application;
use OCA\Tables\Db\Column;
use OCA\Tables\Db\Table;
use OCA\Tables\Db\View;
use OCA\Tables\Db\ViewMapper;
use OCA\Tables\Errors\InternalError;
use OCA\Tables\Errors\NotFoundError;
use OCA\Tables\Errors\PermissionError;
@ -74,7 +73,6 @@ class ViewService extends SuperService {
$this->contextService = $contextService;
}
/**
* @param Table $table
* @param string|null $userId
@ -246,6 +244,7 @@ class ViewService extends SuperService {
* @return View
* @throws InternalError
* @throws PermissionError
* @throws InvalidArgumentException
*/
public function update(int $id, array $data, ?string $userId = null, bool $skipTableEnhancement = false): View {
$userId = $this->permissionsService->preCheckUserId($userId);
@ -264,6 +263,20 @@ class ViewService extends SuperService {
if (!in_array($key, $updatableParameter)) {
throw new InternalError('View parameter ' . $key . ' can not be updated.');
}
if ($key === 'columns') {
// we have to fetch the service here as ColumnService already depends on the ViewService, i.e. no DI
$columnService = \OCP\Server::get(ColumnService::class);
$columnIds = \json_decode($value, true);
$availableColumns = $columnService->findAllByTable($view->getTableId(), $view->getId(), $this->userId);
$availableColumns = array_map(static fn (Column $column) => $column->getId(), $availableColumns);
foreach ($columnIds as $columnId) {
if (!in_array($columnId, $availableColumns, true)) {
throw new InvalidArgumentException('Invalid column ID provided');
}
}
}
$setterMethod = 'set' . ucfirst($key);
$view->$setterMethod($value);
}
@ -275,6 +288,8 @@ class ViewService extends SuperService {
$this->enhanceView($view, $userId);
}
return $view;
} catch (InvalidArgumentException $e) {
throw $e;
} catch (Exception $e) {
$this->logger->error($e->getMessage(), ['exception' => $e]);
throw new InternalError($e->getMessage());

View File

@ -1769,6 +1769,24 @@
}
}
},
"400": {
"description": "Invalid data",
"content": {
"application/json": {
"schema": {
"type": "object",
"required": [
"message"
],
"properties": {
"message": {
"type": "string"
}
}
}
}
}
},
"500": {
"description": "",
"content": {

View File

@ -1476,6 +1476,17 @@ export interface operations {
readonly "application/json": components["schemas"]["View"];
};
};
/** @description Invalid data */
readonly 400: {
headers: {
readonly [name: string]: unknown;
};
content: {
readonly "application/json": {
readonly message: string;
};
};
};
/** @description No permissions */
readonly 403: {
headers: {
@ -2796,7 +2807,7 @@ export interface operations {
readonly "application/json": {
/** @description Data as key - value store */
readonly data: string | {
readonly [key: string]: Record<string, never>;
readonly [key: string]: Record<string, never> | undefined;
};
};
};
@ -2909,7 +2920,7 @@ export interface operations {
readonly "application/json": {
/** @description Data as key - value store */
readonly data: string | {
readonly [key: string]: Record<string, never>;
readonly [key: string]: Record<string, never> | undefined;
};
};
};
@ -3022,7 +3033,7 @@ export interface operations {
readonly viewId?: number | null;
/** @description Data as key - value store */
readonly data: string | {
readonly [key: string]: Record<string, never>;
readonly [key: string]: Record<string, never> | undefined;
};
};
};
@ -5421,7 +5432,7 @@ export interface operations {
readonly "application/json": {
/** @description An array containing the column identifiers and their values */
readonly data: string | {
readonly [key: string]: Record<string, never>;
readonly [key: string]: Record<string, never> | undefined;
};
};
};