From cadd5ccca0facaca15cc37884d7003815117428b Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 28 May 2024 12:56:00 +0200 Subject: [PATCH] fix(Contexts): cast incoming node data on controller level Signed-off-by: Arthur Schiwon --- lib/Controller/ContextController.php | 42 ++++++++++++++++++++++-- lib/Service/ContextService.php | 14 ++++---- openapi.json | 48 ++++++++++++---------------- psalm.xml | 1 + src/types/openapi/openapi.ts | 2 +- 5 files changed, 68 insertions(+), 39 deletions(-) diff --git a/lib/Controller/ContextController.php b/lib/Controller/ContextController.php index a859c7ab4..e2794686c 100644 --- a/lib/Controller/ContextController.php +++ b/lib/Controller/ContextController.php @@ -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; @@ -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 $nodes optional nodes to be connected to this context + * @psalm-param list $nodes optional nodes to be connected to this context * * @return DataResponse|DataResponse * @@ -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)); } } @@ -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, @@ -144,6 +153,33 @@ public function update(int $contextId, ?string $name, ?string $iconName, ?string } } + /** + * @psalm-param list $nodes + * @psalm-return list + */ + 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 * diff --git a/lib/Service/ContextService.php b/lib/Service/ContextService.php index be9a2d1e0..5646302f3 100644 --- a/lib/Service/ContextService.php +++ b/lib/Service/ContextService.php @@ -106,6 +106,7 @@ public function findById(int $id, ?string $userId): Context { } /** + * @psalm-param list $nodes * @throws Exception|PermissionError|InvalidArgumentException */ public function create(string $name, string $iconName, string $description, array $nodes, string $ownerId, int $ownerType): Context { @@ -131,6 +132,7 @@ public function create(string $name, string $iconName, string $description, arra } /** + * @psalm-param list $nodes * @throws Exception * @throws DoesNotExistException * @throws MultipleObjectsReturnedException @@ -431,6 +433,7 @@ protected function insertPage(Context $context): void { } /** + * @psalm-param list $nodes * @throws PermissionError|InvalidArgumentException */ protected function insertNodesFromArray(Context $context, array $nodes): void { @@ -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, ]); diff --git a/openapi.json b/openapi.json index 659c19227..b2419928f 100644 --- a/openapi.json +++ b/openapi.json @@ -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 } - ] + } } }, { diff --git a/psalm.xml b/psalm.xml index 651516ea3..7842786b3 100644 --- a/psalm.xml +++ b/psalm.xml @@ -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" diff --git a/src/types/openapi/openapi.ts b/src/types/openapi/openapi.ts index ea2c07618..7f6c92aab 100644 --- a/src/types/openapi/openapi.ts +++ b/src/types/openapi/openapi.ts @@ -3093,7 +3093,7 @@ export type operations = { /** Format: int64 */ type: number; /** Format: int64 */ - permissions: number; + permissions?: number; }, unknown[]]>; }; header: {