Skip to content

Commit

Permalink
fix(Contexts): cast incoming node data on controller level
Browse files Browse the repository at this point in the history 
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
  • Loading branch information
@blizzz
blizzz committed May 30, 2024
1 parent 2d6614a commit cadd5cc
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 39 deletions.
42 changes: 39 additions & 3 deletions lib/Controller/ContextController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace OCA\Tables\Controller;

use InvalidArgumentException;
use OCA\Tables\Db\Context;
use OCA\Tables\Errors\BadRequestError;
use OCA\Tables\Errors\InternalError;
Expand Down Expand Up @@ -89,7 +90,7 @@ public function show(int $contextId): DataResponse {
* @param string $name Name of the context
* @param string $iconName Material design icon name of the context
* @param string $description Descriptive text of the context
* @param array{id: int, type: int, permissions: int}|array<empty> $nodes optional nodes to be connected to this context
* @psalm-param list<array{id: int, type: int, permissions?: int}> $nodes optional nodes to be connected to this context
*
* @return DataResponse<Http::STATUS_OK, TablesContext, array{}>|DataResponse<Http::STATUS_INTERNAL_SERVER_ERROR|Http::STATUS_BAD_REQUEST|Http::STATUS_FORBIDDEN, array{message: string}, array{}>
*
Expand All @@ -99,12 +100,19 @@ public function show(int $contextId): DataResponse {
*/
public function create(string $name, string $iconName, string $description = '', array $nodes = []): DataResponse {
try {
return new DataResponse($this->contextService->create($name, $iconName, $description, $nodes, $this->userId, 0)->jsonSerialize());
return new DataResponse($this->contextService->create(
$name,
$iconName,
$description,
$this->sanitizeInputNodes($nodes),
$this->userId,
0,
)->jsonSerialize());
} catch (Exception $e) {
return $this->handleError($e);
} catch (PermissionError $e) {
return $this->handlePermissionError($e);
} catch (\InvalidArgumentException $e) {
} catch (InvalidArgumentException $e) {
return $this->handleBadRequestError(new BadRequestError($e->getMessage(), $e->getCode(), $e));
}
}
Expand All @@ -129,6 +137,7 @@ public function create(string $name, string $iconName, string $description = '',
*/
public function update(int $contextId, ?string $name, ?string $iconName, ?string $description, ?array $nodes): DataResponse {
try {
$nodes = $nodes ? $this->sanitizeInputNodes($nodes) : null;
return new DataResponse($this->contextService->update(
$contextId,
$this->userId,
Expand All @@ -144,6 +153,33 @@ public function update(int $contextId, ?string $name, ?string $iconName, ?string
}
}

/**
* @psalm-param list<array{id: mixed, type: mixed, permissions?: mixed, order?: mixed}> $nodes
* @psalm-return list<array{id: int, type: int, permissions?: int, order?: int}>
*/
protected function sanitizeInputNodes(array $nodes): array {
foreach ($nodes as &$node) {
if (!is_numeric($node['type'])) {
throw new InvalidArgumentException('Unexpected node type');
}
$node['type'] = (int)$node['type'];

if (!is_numeric($node['id'])) {
throw new InvalidArgumentException('Unexpected node id');
}
$node['id'] = (int)$node['id'];

if (isset($node['permissions'])) {
$node['permissions'] = (int)$node['permissions'];
}

if (isset($node['order'])) {
$node['order'] = (int)$node['order'];
}
}
return $nodes;
}

/**
* [api v2] Delete an existing context and return it
*
Expand Down
14 changes: 6 additions & 8 deletions lib/Service/ContextService.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ public function findById(int $id, ?string $userId): Context {
}

/**
* @psalm-param list<array{id: int, type: int, permissions?: int, order?: int}> $nodes
* @throws Exception|PermissionError|InvalidArgumentException
*/
public function create(string $name, string $iconName, string $description, array $nodes, string $ownerId, int $ownerType): Context {
Expand All @@ -131,6 +132,7 @@ public function create(string $name, string $iconName, string $description, arra
}

/**
* @psalm-param list<array{id: int, type: int, permissions?: int, order?: int}> $nodes
* @throws Exception
* @throws DoesNotExistException
* @throws MultipleObjectsReturnedException
Expand Down Expand Up @@ -431,6 +433,7 @@ protected function insertPage(Context $context): void {
}

/**
* @psalm-param list<array{id: int, type: int, permissions?: int, order?: int}> $nodes
* @throws PermissionError|InvalidArgumentException
*/
protected function insertNodesFromArray(Context $context, array $nodes): void {
Expand All @@ -439,25 +442,20 @@ protected function insertNodesFromArray(Context $context, array $nodes): void {
$userId = $context->getOwnerType() === Application::OWNER_TYPE_USER ? $context->getOwnerId() : null;
foreach ($nodes as $node) {
try {
if (!is_numeric($node['type'])) {
throw new InvalidArgumentException('Unexpected node type');
}
$nodeType = (int)($node['type']);
if (!is_numeric($node['id'])) {
throw new InvalidArgumentException('Unexpected node id');
}
$nodeId = (int)($node['id']);

if (!$this->permissionsService->canManageNodeById($nodeType, $nodeId, $userId)) {
throw new PermissionError(sprintf('Owner cannot manage node %d (type %d)', $nodeId, $nodeType));
}
$contextNodeRel = $this->addNodeToContext($context, $nodeId, $nodeType, (int)($node['permissions'] ?? Application::PERMISSION_READ));
$contextNodeRel = $this->addNodeToContext($context, $nodeId, $nodeType, $node['permissions'] ?? Application::PERMISSION_READ);
$addedNodes[] = $contextNodeRel->jsonSerialize();
} catch (Exception $e) {
$this->logger->warning('Could not add node {ntype}/{nid} to context {cid}, skipping.', [
'app' => Application::APP_ID,
'ntype' => $node['type'],
'nid' => $node['id'],
'permissions' => $node['permissions'],
'permissions' => $node['permissions'] ?? '',
'cid' => $context['id'],
'exception' => $e,
]);
Expand Down
48 changes: 21 additions & 27 deletions openapi.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8219,39 +8219,33 @@
}
},
{
"name": "nodes",
"name": "nodes[]",
"in": "query",
"description": "optional nodes to be connected to this context",
"schema": {
"type": "array",
"default": [],
"oneOf": [
{
"type": "object",
"required": [
"id",
"type",
"permissions"
],
"properties": {
"id": {
"type": "integer",
"format": "int64"
},
"type": {
"type": "integer",
"format": "int64"
},
"permissions": {
"type": "integer",
"format": "int64"
}
"items": {
"type": "object",
"required": [
"id",
"type"
],
"properties": {
"id": {
"type": "integer",
"format": "int64"
},
"type": {
"type": "integer",
"format": "int64"
},
"permissions": {
"type": "integer",
"format": "int64"
}
},
{
"type": "array",
"maxLength": 0
}
]
}
}
},
{
Expand Down
1 change: 1 addition & 0 deletions psalm.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
resolveFromConfigFile="true"
findUnusedCode="false"
findUnusedBaselineEntry="true"
cacheDirectory="/dev/null"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="https://getpsalm.org/schema/config"
xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"
Expand Down
2 changes: 1 addition & 1 deletion src/types/openapi/openapi.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3093,7 +3093,7 @@ export type operations = {
/** Format: int64 */
type: number;
/** Format: int64 */
permissions: number;
permissions?: number;
}, unknown[]]>;
};
header: {
Expand Down

0 comments on commit cadd5cc

Please sign in to comment.