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 c059526fd..17eb86bef 100644 --- a/lib/Service/ContextService.php +++ b/lib/Service/ContextService.php @@ -109,6 +109,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 { @@ -134,6 +135,7 @@ public function create(string $name, string $iconName, string $description, arra } /** + * @psalm-param list $nodes * @throws Exception * @throws DoesNotExistException * @throws MultipleObjectsReturnedException @@ -445,6 +447,7 @@ protected function insertPage(Context $context): void { } /** + * @psalm-param list $nodes * @throws PermissionError|InvalidArgumentException */ protected function insertNodesFromArray(Context $context, array $nodes): void { @@ -453,25 +456,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/src/types/openapi/openapi.ts b/src/types/openapi/openapi.ts index ea2c07618..2cac211b6 100644 --- a/src/types/openapi/openapi.ts +++ b/src/types/openapi/openapi.ts @@ -3087,14 +3087,14 @@ export type operations = { /** @description Descriptive text of the context */ description?: string; /** @description optional nodes to be connected to this context */ - nodes?: OneOf<[{ - /** Format: int64 */ - id: number; - /** Format: int64 */ - type: number; - /** Format: int64 */ - permissions: number; - }, unknown[]]>; + "nodes[]"?: { + /** Format: int64 */ + id: number; + /** Format: int64 */ + type: number; + /** Format: int64 */ + permissions?: number; + }[]; }; header: { /** @description Required to be true for the API request to pass */ diff --git a/tests/integration/features/APIv2.feature b/tests/integration/features/APIv2.feature index f57e5df77..4181c879e 100644 --- a/tests/integration/features/APIv2.feature +++ b/tests/integration/features/APIv2.feature @@ -335,3 +335,61 @@ Feature: APIv2 And the fetched Context "c1" does not contain following data: | field | value | | node | table:t2:read | + + @api2 @contexts @contexts-update + Scenario: Add an inaccessible table to an inaccessible context + Given table "Table 1 via api v2" with emoji "👋" exists for user "participant1-v2" as "t1" via v2 + And table "Table 2 via api v2" with emoji "📸" exists for user "participant2-v2" as "t2" via v2 + And user "participant1-v2" creates the Context "c1" with name "Enchanting Guitar" with icon "tennis" and description "Lorem ipsum dolor etc pp" and nodes: + | alias | type | permissions | + | t1 | table | read,created,update | + When user "participant2-v2" updates the nodes of the Context "c1" to + | alias | type | permissions | + | t1 | table | read,create,update | + | t2 | table | read | + Then the reported status is "404" + When user "participant1-v2" fetches Context "c1" + Then the fetched Context "c1" has following data: + | field | value | + | node | table:t1:read,create,update | + And the fetched Context "c1" does not contain following data: + | field | value | + | node | table:t2:read | + + @api2 @contexts @contexts-update + Scenario: Remove a table from an owned context + Given table "Table 1 via api v2" with emoji "👋" exists for user "participant1-v2" as "t1" via v2 + And table "Table 2 via api v2" with emoji "📸" exists for user "participant1-v2" as "t2" via v2 + And user "participant1-v2" creates the Context "c1" with name "Enchanting Guitar" with icon "tennis" and description "Lorem ipsum dolor etc pp" and nodes: + | alias | type | permissions | + | t1 | table | read,create,update | + | t2 | table | read | + When user "participant1-v2" updates the nodes of the Context "c1" to + | alias | type | permissions | + | t1 | table | read,create,update | + Then the reported status is "200" + When user "participant1-v2" fetches Context "c1" + Then the fetched Context "c1" has following data: + | field | value | + | node | table:t1:read,create,update | + And the fetched Context "c1" does not contain following data: + | field | value | + | node | table:t2:read | + + @api2 @contexts @contexts-update + Scenario: Remove a non-existing table from an inaccessible context + Given table "Table 1 via api v2" with emoji "👋" exists for user "participant1-v2" as "t1" via v2 + And table "Table 2 via api v2" with emoji "📸" exists for user "participant1-v2" as "t2" via v2 + And user "participant1-v2" creates the Context "c1" with name "Enchanting Guitar" with icon "tennis" and description "Lorem ipsum dolor etc pp" and nodes: + | alias | type | permissions | + | t1 | table | read,create,update | + | t2 | table | read | + When user "participant2-v2" updates the nodes of the Context "c1" to + | alias | type | permissions | + | t1 | table | read,create,update | + Then the reported status is "404" + When user "participant1-v2" fetches Context "c1" + Then the fetched Context "c1" has following data: + | field | value | + | node | table:t1:read,create,update | + | node | table:t2:read |