Skip to content

feat: add permissions for public link shares #2483

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.

Sign up for GitHub

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

Open
benjaminfrueh wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: add permissions for public link shares #2483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions appinfo/info.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ Have a good time and manage whatever you want.
<repair-steps>
<pre-migration>
<step>OCA\Tables\Migration\FixContextsDefaults</step>
<step>OCA\Tables\Migration\ResetPublicSharePermissions</step>
</pre-migration>
<post-migration>
<step>OCA\Tables\Migration\NewDbStructureRepairStep</step>
Expand Down
1 change: 1 addition & 0 deletions appinfo/routes.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@
['name' => 'share#show', 'url' => '/share/{id}', 'verb' => 'GET'],
['name' => 'share#create', 'url' => '/share', 'verb' => 'POST'],
['name' => 'share#updatePermission', 'url' => '/share/{id}/permission', 'verb' => 'PUT'],
['name' => 'share#updatePermissions', 'url' => '/share/{id}/permissions', 'verb' => 'PUT'],
['name' => 'share#updateDisplayMode', 'url' => '/share/{id}/display-mode', 'verb' => 'PUT'],
['name' => 'share#destroy', 'url' => '/share/{id}', 'verb' => 'DELETE'],

Expand Down
2 changes: 1 addition & 1 deletion lib/Controller/Api1Controller.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,7 @@ public function deleteShare(int $shareId): DataResponse {
#[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)]
public function updateSharePermissions(int $shareId, string $permissionType, bool $permissionValue): DataResponse {
try {
return new DataResponse($this->shareService->updatePermission($shareId, $permissionType, $permissionValue)->jsonSerialize());
return new DataResponse($this->shareService->updatePermission($shareId, [$permissionType => $permissionValue])->jsonSerialize());
} catch (PermissionError $e) {
$this->logger->warning('A permission error occurred: ' . $e->getMessage(), ['exception' => $e]);
$message = ['message' => $e->getMessage()];
Expand Down
169 changes: 169 additions & 0 deletions lib/Controller/PublicRowOCSController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@
namespace OCA\Tables\Controller;

use OCA\Tables\AppInfo\Application;
use OCA\Tables\Db\Row2Mapper;
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\AssertShareAccessIsAccessible;
use OCA\Tables\Model\RowDataInput;
use OCA\Tables\ResponseDefinitions;
use OCA\Tables\Service\RowService;
use OCA\Tables\Service\ShareService;
Expand All @@ -37,11 +39,13 @@ class PublicRowOCSController extends AOCSController {
public function __construct(
protected ShareService $shareService,
protected RowService $rowService,
protected Row2Mapper $row2Mapper,
IRequest $request,
LoggerInterface $logger,
IL10N $l,
) {
parent::__construct($request, $logger, $l, '');
$this->rowService->setPublicContext();
}

/**
Expand All @@ -68,6 +72,10 @@ public function getRows(string $token, ?int $limit, ?int $offset): DataResponse
$shareToken = new ShareToken($token);
$share = $this->shareService->findByToken($shareToken);

if (!$share->getPermissionRead()) {
return $this->handlePermissionError(new PermissionError('No read permission on this share'));
}

$limit = $limit !== null ? max(0, min(500, $limit)) : null;
$offset = $offset !== null ? max(0, $offset) : null;

Expand All @@ -90,4 +98,165 @@ public function getRows(string $token, ?int $limit, ?int $offset): DataResponse
return $this->handleBadRequestError($e);
}
}

/**
* [api v2] Create a row in a link share
*
* @param string $token The share token
* @param string|array<string, mixed> $data An array containing the column identifiers and their values
* @return DataResponse<Http::STATUS_OK, TablesPublicRow, array{}>|DataResponse<Http::STATUS_FORBIDDEN|Http::STATUS_BAD_REQUEST|Http::STATUS_NOT_FOUND|Http::STATUS_INTERNAL_SERVER_ERROR, array{message: string}, array{}>
*
* 200: Row created
* 400: Invalid request parameters
* 403: No permissions
* 404: Not found
* 500: Internal error
*/
#[PublicPage]
#[AssertShareAccessIsAccessible]
#[ApiRoute(verb: 'POST', url: '/api/2/public/{token}/rows', requirements: ['token' => '[a-zA-Z0-9]{16}'])]
#[OpenAPI]
#[AnonRateLimit(limit: 20, period: 30)]
public function createRow(string $token, mixed $data): DataResponse {
try {
$shareToken = new ShareToken($token);
$share = $this->shareService->findByToken($shareToken);
$this->row2Mapper->setUserId('public-' . $token);

if (!$share->getPermissionCreate()) {
return $this->handlePermissionError(new PermissionError('No create permission on this share'));
}

if (is_string($data)) {
$data = json_decode($data, true);
}
if (!is_array($data)) {
return $this->handleBadRequestError(new BadRequestError('Invalid data input'));
}

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

$tableId = $share->getNodeType() === 'table' ? $share->getNodeId() : null;
$viewId = $share->getNodeType() === 'view' ? $share->getNodeId() : null;

if ($viewId === null && $tableId === null) {
throw new InternalError('Cannot create row without table or view provided');
}

$row = $this->rowService->create($tableId, $viewId, $newRowData);
return new DataResponse($this->rowService->formatRowsForPublicShare([$row])[0]);
} catch (PermissionError $e) {
return $this->handlePermissionError($e);
} catch (NotFoundError $e) {
return $this->handleNotFoundError($e);
} catch (BadRequestError $e) {
return $this->handleBadRequestError($e);
} catch (InternalError|\Exception $e) {
return $this->handleError($e);
}
}

/**
* [api v2] Update a row in a link share
*
* @param string $token The share token
* @param int $rowId The row identifier
* @param string|array<string, mixed> $data An array containing the column identifiers and their values
* @return DataResponse<Http::STATUS_OK, TablesPublicRow, array{}>|DataResponse<Http::STATUS_FORBIDDEN|Http::STATUS_BAD_REQUEST|Http::STATUS_NOT_FOUND|Http::STATUS_INTERNAL_SERVER_ERROR, array{message: string}, array{}>
*
* 200: Row updated
* 400: Invalid request parameters
* 403: No permissions
* 404: Not found
* 500: Internal error
*/
#[PublicPage]
#[AssertShareAccessIsAccessible]
#[ApiRoute(verb: 'PUT', url: '/api/2/public/{token}/rows/{rowId}', requirements: ['token' => '[a-zA-Z0-9]{16}', 'rowId' => '\d+'])]
#[OpenAPI]
#[AnonRateLimit(limit: 20, period: 30)]
public function updateRow(string $token, int $rowId, mixed $data): DataResponse {
try {
$shareToken = new ShareToken($token);
$share = $this->shareService->findByToken($shareToken);
$this->row2Mapper->setUserId('public-' . $token);

if (!$share->getPermissionUpdate()) {
return $this->handlePermissionError(new PermissionError('No update permission on this share'));
}

if (is_string($data)) {
$data = json_decode($data, true);
}
if (!is_array($data)) {
return $this->handleBadRequestError(new BadRequestError('Invalid data input'));
}

$viewId = $share->getNodeType() === 'view' ? $share->getNodeId() : null;
$tableId = $share->getNodeType() === 'table' ? $share->getNodeId() : null;

if ($viewId === null && $tableId === null) {
throw new InternalError('Cannot update row without table or view provided');
}

$row = $this->rowService->updateSet($rowId, $viewId, $data, '', $tableId);
return new DataResponse($this->rowService->formatRowsForPublicShare([$row])[0]);
} catch (PermissionError $e) {
return $this->handlePermissionError($e);
} catch (NotFoundError $e) {
return $this->handleNotFoundError($e);
} catch (BadRequestError $e) {
return $this->handleBadRequestError($e);
} catch (InternalError|\Exception $e) {
return $this->handleError($e);
}
}

/**
* [api v2] Delete a row in a link share
*
* @param string $token The share token
* @param int $rowId The row identifier
* @return DataResponse<Http::STATUS_OK, TablesPublicRow, array{}>|DataResponse<Http::STATUS_FORBIDDEN|Http::STATUS_NOT_FOUND|Http::STATUS_INTERNAL_SERVER_ERROR, array{message: string}, array{}>
*
* 200: Row deleted
* 403: No permissions
* 404: Not found
* 500: Internal error
*/
#[PublicPage]
#[AssertShareAccessIsAccessible]
#[ApiRoute(verb: 'DELETE', url: '/api/2/public/{token}/rows/{rowId}', requirements: ['token' => '[a-zA-Z0-9]{16}', 'rowId' => '\d+'])]
#[OpenAPI]
#[AnonRateLimit(limit: 20, period: 30)]
public function deleteRow(string $token, int $rowId): DataResponse {
try {
$shareToken = new ShareToken($token);
$share = $this->shareService->findByToken($shareToken);
$this->row2Mapper->setUserId('public-' . $token);

if (!$share->getPermissionDelete()) {
return $this->handlePermissionError(new PermissionError('No delete permission on this share'));
}

$viewId = $share->getNodeType() === 'view' ? $share->getNodeId() : null;
$tableId = $share->getNodeType() === 'table' ? $share->getNodeId() : null;

if ($viewId === null && $tableId === null) {
throw new InternalError('Cannot delete row without table or view provided');
}

$row = $this->rowService->delete($rowId, $viewId, '', $tableId);
return new DataResponse($this->rowService->formatRowsForPublicShare([$row])[0]);
} catch (PermissionError $e) {
return $this->handlePermissionError($e);
} catch (NotFoundError $e) {
return $this->handleNotFoundError($e);
} catch (InternalError|\Exception $e) {
return $this->handleError($e);
}
Comment thread
benjaminfrueh marked this conversation as resolved.
}
}
6 changes: 6 additions & 0 deletions lib/Controller/PublicSharePageController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,12 @@ public function showShare(): TemplateResponse {
$this->initialState->provideInitialState('shareToken', (string)$this->shareToken);
$this->initialState->provideInitialState('nodeType', $this->share->getNodeType());
$this->initialState->provideInitialState('nodeData', $nodeData);
$this->initialState->provideInitialState('sharePermissions', [
'read' => $this->share->getPermissionRead(),
'create' => $this->share->getPermissionCreate(),
'update' => $this->share->getPermissionUpdate(),
'delete' => $this->share->getPermissionDelete(),
]);

if (class_exists(LoadEditor::class)) {
$this->eventDispatcher->dispatchTyped(new LoadEditor());
Expand Down
20 changes: 19 additions & 1 deletion lib/Controller/ShareController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,25 @@ public function create(
#[NoAdminRequired]
public function updatePermission(int $id, string $permission, bool $value): DataResponse {
return $this->handleError(function () use ($id, $permission, $value) {
return $this->service->updatePermission($id, $permission, $value);
return $this->service->updatePermission($id, [$permission => $value]);
});
}

#[NoAdminRequired]
public function updatePermissions(
int $id,
bool $permissionRead = false,
bool $permissionCreate = false,
bool $permissionUpdate = false,
bool $permissionDelete = false,
): DataResponse {
return $this->handleError(function () use ($id, $permissionRead, $permissionCreate, $permissionUpdate, $permissionDelete) {
Comment thread
benjaminfrueh marked this conversation as resolved.
return $this->service->updatePermission($id, [
'read' => $permissionRead,
'create' => $permissionCreate,
'update' => $permissionUpdate && $permissionRead,
'delete' => $permissionDelete && $permissionRead,
]);
});
}

Expand Down
4 changes: 4 additions & 0 deletions lib/Db/Row2Mapper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,10 @@ public function getTableIdForRow(int $rowId): ?int {
return $rowSleeve->getTableId();
}

public function setUserId(string $userId): void {
$this->userId = $userId;
}

/**
* @return int[]
* @throws InternalError
Expand Down
51 changes: 51 additions & 0 deletions lib/Migration/ResetPublicSharePermissions.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\Tables\Migration;

use OCA\Tables\AppInfo\Application;
use OCA\Tables\Constants\ShareReceiverType;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\Migration\IOutput;
use OCP\Migration\IRepairStep;

class ResetPublicSharePermissions implements IRepairStep {

public function __construct(
protected IConfig $config,
protected IDBConnection $dbc,
) {
}

public function getName(): string {
return 'Reset public link share permissions to read-only for versions before 2.0.2';
}

public function run(IOutput $output): void {
$appVersion = $this->config->getAppValue(Application::APP_ID, 'installed_version', '0.0');
if (\version_compare($appVersion, '2.0.2', '>=')) {
$output->info('Not applicable, skipping.');
return;
}

$qb = $this->dbc->getQueryBuilder();
$qb->update('tables_shares')
->set('permission_read', $qb->createNamedParameter(1, IQueryBuilder::PARAM_INT))
->set('permission_create', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))
->set('permission_update', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))
->set('permission_delete', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))
->set('permission_manage', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))
->where($qb->expr()->eq('receiver_type', $qb->createNamedParameter(ShareReceiverType::LINK, IQueryBuilder::PARAM_STR)))
->executeStatement();

$output->info('Reset public link share permissions to read-only.');
}
}
9 changes: 8 additions & 1 deletion lib/Service/PermissionsService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ class PermissionsService {

protected bool $isCli = false;

private bool $isPublicContext = false;

private ContextMapper $contextMapper;

public function __construct(
Expand Down Expand Up @@ -549,6 +551,11 @@ public function getPermissionArrayForNodeFromContexts(int $nodeId, string $nodeT
);
}

public function setPublicContext(): void {
$this->userId = '';
Comment thread
benjaminfrueh marked this conversation as resolved.
$this->isPublicContext = true;
}

private function hasPermission(int $existingPermissions, string $permissionName): bool {
$constantName = 'PERMISSION_' . strtoupper($permissionName);
try {
Expand Down Expand Up @@ -634,7 +641,7 @@ private function basisCheck(Table|View|Context $element, string $nodeType, ?stri
}

if ($userId === '') {
return true;
return $this->isCli || $this->isPublicContext;
Copy link
Copy Markdown
Contributor

@enjeck enjeck Apr 23, 2026

Choose a reason for hiding this comment

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

i wonder if this breaks existing shares 🤔

Copy link
Copy Markdown
Contributor Author

@benjaminfrueh benjaminfrueh Apr 23, 2026

Choose a reason for hiding this comment

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

It should not break existing shares, anything going through PublicRowOCSController, so also the existing getRows() will have isPublicContext set to true. I just wanted to be more explicit and safe here, in what cases a empty userId should be allowed.

Was there any other case than CLI and isPublicContext when a empty userId should bypass the permission check? We can of course also revert this line and always return true here again, just to be sure.

I think we should think about a way to refactor this later and maybe use something else than an empty userId to bypass the permission checks.

}

if ($this->userIsElementOwner($element, $userId, $nodeType)) {
Expand Down
10 changes: 8 additions & 2 deletions lib/Service/RowService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ public function find(int $rowId): Row2 {
* @throws InternalError
*/
public function create(?int $tableId, ?int $viewId, RowDataInput|array $data): Row2 {
if ($this->userId === null || $this->userId === '') {
if ($this->userId === null) {
$e = new \Exception('No user id in context, but needed.');
$this->logger->error($e->getMessage(), ['exception' => $e]);
throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage());
Expand Down Expand Up @@ -685,7 +685,7 @@ public function updateSet(
* @throws PermissionError
* @noinspection DuplicatedCode
*/
public function delete(int $id, ?int $viewId, string $userId): Row2 {
public function delete(int $id, ?int $viewId, string $userId, ?int $tableId = null): Row2 {
try {
$item = $this->getRowById($id);
} catch (InternalError $e) {
Expand Down Expand Up @@ -720,6 +720,12 @@ public function delete(int $id, ?int $viewId, string $userId): Row2 {
throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage());
}
} else {
if ($tableId !== null && $tableId !== $item->getTableId()) {
$e = new \Exception('Row does not belong to table with id ' . $tableId);
$this->logger->error($e->getMessage(), ['exception' => $e]);
throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage());
}

Copy link
Copy Markdown
Member

@blizzz blizzz Apr 24, 2026

Choose a reason for hiding this comment

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

theoretically both table and view can be null. Not from the calling Controller, given it is internally called after validation there is no much pressure… but it leaves a funny feeling in the tummy. #paranoia

Copy link
Copy Markdown
Contributor Author

@benjaminfrueh benjaminfrueh Apr 24, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

As a defensive fix, I now added a simple check to the create, update and delete in the PublicRowOCSController to be sure they can not call the service with both tableId and viewId being null.

// security
if (!$this->permissionsService->canReadRowsByElementId($item->getTableId(), 'table', $userId)) {
$e = new \Exception('Row not found.');
Expand Down
Loading
Loading