Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enh(API): add OCS API to create rows #1161

Merged
merged 3 commits into from
Jun 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions appinfo/routes.php
Original file line number Diff line number Diff line change
Expand Up @@ -131,12 +131,15 @@

['name' => 'ApiFavorite#create', 'url' => '/api/2/favorites/{nodeType}/{nodeId}', 'verb' => 'POST', 'requirements' => ['nodeType' => '(\d+)', 'nodeId' => '(\d+)']],
['name' => 'ApiFavorite#destroy', 'url' => '/api/2/favorites/{nodeType}/{nodeId}', 'verb' => 'DELETE', 'requirements' => ['nodeType' => '(\d+)', 'nodeId' => '(\d+)']],

['name' => 'Context#index', 'url' => '/api/2/contexts', 'verb' => 'GET'],
['name' => 'Context#show', 'url' => '/api/2/contexts/{contextId}', 'verb' => 'GET'],
['name' => 'Context#create', 'url' => '/api/2/contexts', 'verb' => 'POST'],
['name' => 'Context#update', 'url' => '/api/2/contexts/{contextId}', 'verb' => 'PUT'],
['name' => 'Context#destroy', 'url' => '/api/2/contexts/{contextId}', 'verb' => 'DELETE'],
['name' => 'Context#transfer', 'url' => '/api/2/contexts/{contextId}/transfer', 'verb' => 'PUT'],
['name' => 'Context#updateContentOrder', 'url' => '/api/2/contexts/{contextId}/pages/{pageId}', 'verb' => 'PUT'],

['name' => 'RowOCS#createRow', 'url' => '/api/2/{nodeCollection}/{nodeId}/rows', 'verb' => 'POST', 'requirements' => ['nodeCollection' => '(tables|views)', 'nodeId' => '(\d+)']],
]
];
86 changes: 86 additions & 0 deletions lib/Controller/RowOCSController.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
<?php

namespace OCA\Tables\Controller;

use OCA\Tables\AppInfo\Application;
use OCA\Tables\Errors\BadRequestError;
use OCA\Tables\Errors\InternalError;
use OCA\Tables\Errors\NotFoundError;
use OCA\Tables\Errors\PermissionError;
use OCA\Tables\Helper\ConversionHelper;
use OCA\Tables\Middleware\Attribute\RequirePermission;
use OCA\Tables\Model\RowDataInput;
use OCA\Tables\ResponseDefinitions;
use OCA\Tables\Service\RowService;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
use OCP\AppFramework\Http\DataResponse;
use OCP\IL10N;
use OCP\IRequest;
use Psr\Log\LoggerInterface;

/**
* @psalm-import-type TablesRow from ResponseDefinitions
*/
class RowOCSController extends AOCSController {

public function __construct(
IRequest $request,
LoggerInterface $logger,
IL10N $n,
string $userId,
protected RowService $rowService,
) {
parent::__construct($request, $logger, $n, $userId);
}

/**
* [api v2] Create a new row in a table or a view
*
* @param 'tables'|'views' $nodeCollection Indicates whether to create a row on a table or view
* @param int $nodeId The identifier of the targeted table or view
* @param string|array{columnId: int, value: mixed} $data An array containing the column identifiers and their values
* @return DataResponse<Http::STATUS_OK, TablesRow, array{}>|DataResponse<Http::STATUS_FORBIDDEN|Http::STATUS_BAD_REQUEST|Http::STATUS_NOT_FOUND|Http::STATUS_INTERNAL_SERVER_ERROR, array{message: string}, array{}>
*
* 200: Row returned
* 400: Invalid request parameters
* 403: No permissions
* 404: Not found
* 500: Internal error
*
* @NoAdminRequired (26 compatibility)
*/
#[NoAdminRequired]
#[RequirePermission(permission: Application::PERMISSION_CREATE, typeParam: 'nodeCollection')]
public function createRow(string $nodeCollection, int $nodeId, mixed $data): DataResponse {
if (is_string($data)) {
$data = json_decode($data, true);
}
if (!is_array($data)) {
return $this->handleBadRequestError(new BadRequestError('Cannot create row: data input is invalid.'));
}

$iNodeType = ConversionHelper::stringNodeType2Const($nodeCollection);
$tableId = $viewId = null;
if ($iNodeType === Application::NODE_TYPE_TABLE) {
$tableId = $nodeId;
} elseif ($iNodeType === Application::NODE_TYPE_VIEW) {
$viewId = $nodeId;
}

$newRowData = new RowDataInput();
foreach ($data as $key => $value) {
$newRowData->add((int)$key, $value);
}

try {
return new DataResponse($this->rowService->create($tableId, $viewId, $newRowData)->jsonSerialize());
} catch (NotFoundError $e) {
return $this->handleNotFoundError($e);
} catch (PermissionError $e) {
return $this->handlePermissionError($e);
} catch (InternalError|\Exception $e) {
return $this->handleError($e);
}
}
}
31 changes: 31 additions & 0 deletions lib/Helper/ConversionHelper.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php

namespace OCA\Tables\Helper;

use InvalidArgumentException;
use OCA\Tables\AppInfo\Application;

class ConversionHelper {

/**
* @throws InvalidArgumentException
*/
public static function constNodeType2String(int $nodeType): string {
return match ($nodeType) {
Application::NODE_TYPE_TABLE => 'table',
Application::NODE_TYPE_VIEW => 'view',
default => throw new InvalidArgumentException('Invalid node type'),
};
}

/**
* @throws InvalidArgumentException
*/
public static function stringNodeType2Const(string $nodeType): int {
return match ($nodeType) {
'table', 'tables' => Application::NODE_TYPE_TABLE,
'view', 'views' => Application::NODE_TYPE_VIEW,
default => throw new InvalidArgumentException('Invalid node type'),
};
}
}
96 changes: 78 additions & 18 deletions lib/Middleware/PermissionMiddleware.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@

namespace OCA\Tables\Middleware;

use InvalidArgumentException;
use OCA\Tables\AppInfo\Application;
use OCA\Tables\Errors\InternalError;
use OCA\Tables\Errors\NotFoundError;
use OCA\Tables\Errors\PermissionError;
use OCA\Tables\Helper\ConversionHelper;
use OCA\Tables\Middleware\Attribute\RequirePermission;
use OCA\Tables\Service\PermissionsService;
use OCP\AppFramework\Http;
Expand Down Expand Up @@ -76,18 +78,19 @@ protected function checkPermission(RequirePermission $attribute): void {
// currently they are only passed as string as well
$isContext = true;
} else {
$nodeType = match ($nodeType) {
'table' => Application::NODE_TYPE_TABLE,
'view' => Application::NODE_TYPE_VIEW,
default => throw new InternalError('Invalid node type'),
};
try {
$nodeType = ConversionHelper::stringNodeType2Const((string)$nodeType);
} catch (InvalidArgumentException) {
throw new InternalError('Invalid node type');
}
}
} elseif (!in_array((int)$nodeType, [Application::NODE_TYPE_TABLE, Application::NODE_TYPE_VIEW], true)) {
throw new InternalError('Invalid node type');
}
$nodeType = (int)$nodeType;

// pre-test: if the node is not accessible in first place, we do not have to reveal it
// this is also an assertion for READ permissions.
if ($isContext) {
if (!$this->permissionsService->canAccessContextById($nodeId, $this->userId)) {
throw new NotFoundError();
Expand All @@ -98,21 +101,78 @@ protected function checkPermission(RequirePermission $attribute): void {
}
}

if ($attribute->getPermission() === Application::PERMISSION_MANAGE) {
if (!$isContext) {
if (!$this->permissionsService->canManageNodeById($nodeType, $nodeId, $this->userId)) {
throw new PermissionError(sprintf('User %s cannot manage node %d (type %d)',
$this->userId, $nodeId, $nodeType
));
}
} else {
if (!$this->permissionsService->canManageContextById($nodeId, $this->userId)) {
throw new PermissionError(sprintf('User %s cannot manage context %d',
$this->userId, $nodeId
));
}
match ($attribute->getPermission()) {
Application::PERMISSION_MANAGE => $this->assertManagePermission($isContext, $nodeType, $nodeId),
Application::PERMISSION_CREATE => $this->assertCreatePermissions($nodeType, $nodeId),
Application::PERMISSION_UPDATE => $this->assertUpdatePermissions($nodeType, $nodeId),
Application::PERMISSION_DELETE => $this->assertDeletePermissions($nodeType, $nodeId),
Application::PERMISSION_ALL => $this->assertManagePermission($isContext, $nodeType, $nodeId)
&& $this->assertCreatePermissions($nodeType, $nodeId)
&& $this->assertUpdatePermissions($nodeType, $nodeId)
&& $this->assertDeletePermissions($nodeType, $nodeId),
};
}

/**
* @throws PermissionError
*/
private function assertCreatePermissions(int $nodeType, int $nodeId): bool {
if (!$this->permissionsService->canCreateRowsById($nodeType, $nodeId, $this->userId)) {
throw new PermissionError(sprintf('User %s cannot create rows on node %d (type %d)',
$this->userId, $nodeId, $nodeType
));
}
return true;
}

/**
* @throws PermissionError
*/
private function assertUpdatePermissions(int $nodeType, int $nodeId): bool {
if (
($nodeType === Application::NODE_TYPE_TABLE && !$this->permissionsService->canUpdateRowsByTableId($nodeId, $this->userId))
|| ($nodeType === Application::NODE_TYPE_VIEW && !$this->permissionsService->canUpdateRowsByViewId($nodeId, $this->userId))
) {
throw new PermissionError(sprintf('User %s cannot update rows on node %d (type %d)',
$this->userId, $nodeId, $nodeType
));
}
return true;
}

/**
* @throws PermissionError
*/
private function assertDeletePermissions(int $nodeType, int $nodeId): bool {
if (
($nodeType === Application::NODE_TYPE_TABLE && !$this->permissionsService->canDeleteRowsByTableId($nodeId, $this->userId))
|| ($nodeType === Application::NODE_TYPE_VIEW && !$this->permissionsService->canDeleteRowsByViewId($nodeId, $this->userId))
) {
throw new PermissionError(sprintf('User %s cannot delete rows on node %d (type %d)',
$this->userId, $nodeId, $nodeType
));
}
return true;
}

/**
* @throws PermissionError
*/
private function assertManagePermission(bool $isContext, int $nodeType, int $nodeId): bool {
if (!$isContext) {
if (!$this->permissionsService->canManageNodeById($nodeType, $nodeId, $this->userId)) {
throw new PermissionError(sprintf('User %s cannot manage node %d (type %d)',
$this->userId, $nodeId, $nodeType
));
}
} else {
if (!$this->permissionsService->canManageContextById($nodeId, $this->userId)) {
throw new PermissionError(sprintf('User %s cannot manage context %d',
$this->userId, $nodeId
));
}
}
return true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a use case for returning true in the assert-Methods if we throw anways in case of no permission?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise I could not chain them in the match block: https://github.com/nextcloud/tables/pull/1161/files#diff-58a7c8ba568eef76405b8471729c4b735def0967160958d5ab0105ddb7afb34eR109-R112

could turn it into switch construction instead of course. I find the match more compact and readable.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok, that makes sense then 👍

}

/**
Expand Down
15 changes: 15 additions & 0 deletions lib/Model/Permissions.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?php

namespace OCA\Tables\Model;

class Permissions {
public function __construct(
public bool $read = false,
public bool $create = false,
public bool $update = false,
public bool $delete = false,
public bool $manage = false,
public bool $manageTable = false,
) {
}
}
43 changes: 43 additions & 0 deletions lib/Model/RowDataInput.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<?php

namespace OCA\Tables\Model;

use ArrayAccess;

/**
* @template-implements ArrayAccess<mixed, array{'columnId': int, 'value': mixed}>
*/
class RowDataInput implements ArrayAccess {
protected const DATA_KEY = 'columnId';
protected const DATA_VAL = 'value';
/** @psalm-var array<array{'columnId': int, 'value': mixed}> */
protected array $data = [];

public function add(int $columnId, string $value): self {
$this->data[] = [self::DATA_KEY => $columnId, self::DATA_VAL => $value];
return $this;
}

public function offsetExists(mixed $offset): bool {
foreach ($this->data as $data) {
if ($data[self::DATA_KEY] === $offset[self::DATA_KEY]) {
return true;
}
}
return false;
}

public function offsetGet(mixed $offset): mixed {
return $this->data[$offset];
}

public function offsetSet(mixed $offset, mixed $value): void {
$this->data[$offset] = $value;
}

public function offsetUnset(mixed $offset): void {
if (isset($this->data[$offset])) {
unset($this->data[$offset]);
}
}
}
12 changes: 7 additions & 5 deletions lib/Service/PermissionsService.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use OCA\Tables\Db\ViewMapper;
use OCA\Tables\Errors\InternalError;
use OCA\Tables\Errors\NotFoundError;
use OCA\Tables\Helper\ConversionHelper;
use OCA\Tables\Helper\UserHelper;
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\AppFramework\Db\MultipleObjectsReturnedException;
Expand Down Expand Up @@ -315,6 +316,11 @@ public function canCreateRows($element, string $nodeType = 'view', ?string $user
return $this->checkPermission($element, 'view', 'create', $userId);
}

public function canCreateRowsById(int $nodeType, int $nodeId, ?string $userId = null): bool {
$sNodeType = ConversionHelper::constNodeType2String($nodeType);
return $this->checkPermissionById($nodeId, $sNodeType, 'create', $userId);
}

/**
* @param int $viewId
* @param string|null $userId
Expand All @@ -333,7 +339,6 @@ public function canUpdateRowsByTableId(int $tableId, ?string $userId = null): bo
return $this->checkPermissionById($tableId, 'table', 'update', $userId);
}


/**
* @param int $viewId
* @param string|null $userId
Expand Down Expand Up @@ -470,10 +475,7 @@ public function getSharedPermissionsIfSharedWithMe(int $elementId, string $eleme
public function getPermissionIfAvailableThroughContext(int $nodeId, string $nodeType, string $userId): int {
$permissions = 0;
$found = false;
$iNodeType = match ($nodeType) {
'table' => Application::NODE_TYPE_TABLE,
'view' => Application::NODE_TYPE_VIEW,
};
$iNodeType = ConversionHelper::stringNodeType2Const($nodeType);
$contexts = $this->contextMapper->findAllContainingNode($iNodeType, $nodeId, $userId);
foreach ($contexts as $context) {
$found = true;
Expand Down
Loading
Loading