From b365a3cd6b2128a4bc5c3c937e1a503b13331854 Mon Sep 17 00:00:00 2001 From: "Enjeck C." Date: Fri, 26 Jun 2026 06:02:57 +0100 Subject: [PATCH] feat: enhance relation permissions and accessibility checks Signed-off-by: Enjeck C. --- lib/AppInfo/Application.php | 4 + lib/Service/ColumnService.php | 42 ++++++++++ lib/Service/RelationService.php | 76 ++++++++++++++++++- lib/Service/ShareService.php | 62 ++++++++++----- .../columnTypePartials/forms/RelationForm.vue | 43 ++++++++--- src/store/data.js | 30 +++++++- 6 files changed, 226 insertions(+), 31 deletions(-) diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 4c00c491df..e31e959998 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -49,6 +49,10 @@ class Application extends App implements IBootstrap { public const NODE_TYPE_TABLE = 0; public const NODE_TYPE_VIEW = 1; + public const NODE_TYPE_NAME_TABLE = 'table'; + public const NODE_TYPE_NAME_VIEW = 'view'; + public const NODE_TYPE_NAME_CONTEXT = 'context'; + public const OWNER_TYPE_USER = 0; public const NAV_ENTRY_MODE_HIDDEN = 0; diff --git a/lib/Service/ColumnService.php b/lib/Service/ColumnService.php index 1f7322e8f1..300ae44f2d 100644 --- a/lib/Service/ColumnService.php +++ b/lib/Service/ColumnService.php @@ -9,6 +9,7 @@ use DateTime; use Exception; +use OCA\Tables\AppInfo\Application; use OCA\Tables\Constants\ColumnType; use OCA\Tables\Db\Column; use OCA\Tables\Db\ColumnMapper; @@ -304,6 +305,9 @@ public function create( } $this->validateCustomSettings($columnDto->getCustomSettings()); + if ($columnDto->getType() === Column::TYPE_RELATION) { + $this->validateRelationTargetAccess($columnDto->getCustomSettings(), $userId); + } $item = Column::fromDto($columnDto); $item->setTitle($newTitle); @@ -408,6 +412,9 @@ public function update( $item->setUsergroupSelectTeams($columnDto->getUsergroupSelectTeams()); $item->setShowUserStatus($columnDto->getShowUserStatus()); $this->validateCustomSettings($columnDto->getCustomSettings()); + if ($columnDto->getType() === Column::TYPE_RELATION || $item->getType() === Column::TYPE_RELATION) { + $this->validateRelationTargetAccess($columnDto->getCustomSettings(), $userId); + } $item->setCustomSettings($columnDto->getCustomSettings()); $this->updateMetadata($item, $userId); @@ -450,6 +457,41 @@ private function validateCustomSettings(?string $customSettings): void { } } + /** + * Ensure the user configuring a relation column can actually read the target + * + * @param string|null $customSettings + * @throws BadRequestError + */ + private function validateRelationTargetAccess(?string $customSettings, ?string $userId): void { + if ($customSettings === null) { + return; + } + $settings = json_decode($customSettings, true); + if (!is_array($settings)) { + return; + } + + $relationType = $settings[Column::RELATION_TYPE] ?? null; + $targetId = isset($settings[Column::RELATION_TARGET_ID]) ? (int)$settings[Column::RELATION_TARGET_ID] : null; + if (empty($relationType) || empty($targetId)) { + return; + } + + if ($relationType === Application::NODE_TYPE_NAME_VIEW) { + $canRead = $this->permissionsService->canReadColumnsByViewId($targetId, $userId); + } elseif ($relationType === Application::NODE_TYPE_NAME_TABLE) { + $canRead = $this->permissionsService->canReadColumnsByTableId($targetId, $userId); + } else { + $canRead = false; + } + + if (!$canRead) { + $translatedMessage = $this->l->t('You can only link to a table or view that you have access to.'); + throw new BadRequestError($translatedMessage, 0, null, $translatedMessage); + } + } + private function normalizeTitle(?string $title, bool $required): ?string { if ($title === null) { if ($required) { diff --git a/lib/Service/RelationService.php b/lib/Service/RelationService.php index 7c53b3b875..e38c09d8cf 100644 --- a/lib/Service/RelationService.php +++ b/lib/Service/RelationService.php @@ -7,9 +7,11 @@ namespace OCA\Tables\Service; +use OCA\Tables\AppInfo\Application; use OCA\Tables\Db\Column; use OCA\Tables\Db\ColumnMapper; use OCA\Tables\Db\Row2Mapper; +use OCA\Tables\Db\TableMapper; use OCA\Tables\Db\ViewMapper; use OCA\Tables\Errors\InternalError; use OCA\Tables\Errors\NotFoundError; @@ -20,11 +22,17 @@ class RelationService { /** @var array Cache for relation data */ private array $cacheRelationData = []; + /** @var array Cache for manager accessibility decisions, keyed by host table + target */ + private array $cacheManagerAccess = []; + public function __construct( private ColumnMapper $columnMapper, private ViewMapper $viewMapper, + private TableMapper $tableMapper, private Row2Mapper $row2Mapper, private ColumnService $columnService, + private PermissionsService $permissionsService, + private ShareService $shareService, private ?string $userId, ) { } @@ -83,9 +91,10 @@ private function getRelationsForColumns(array $relationColumns): array { foreach ($groupedColumns as $target => $columns) { $relationData = $this->getRelationDataForTarget($target, $columns[0]); - // Assign the same data to all columns with this target + // Assign the same data to all columns with this target, but only when + // the relation is still backed by a manager of the hosting table. foreach ($columns as $column) { - $result[$column->getId()] = $relationData; + $result[$column->getId()] = $this->isTargetAccessibleByManager($column) ? $relationData : []; } } @@ -133,11 +142,72 @@ public function getRelationData(Column $column): array { return []; } + if (!$this->isTargetAccessibleByManager($column)) { + return []; + } + $target = sprintf('%s_%s_%s', $settings['relationType'], $settings['targetId'], $settings['labelColumn']); return $this->getRelationDataForTarget($target, $column); } + public function isTargetAccessibleByManager(Column $relationColumn): bool { + $settings = $relationColumn->getCustomSettingsArray(); + $relationType = $settings[Column::RELATION_TYPE] ?? null; + $targetId = isset($settings[Column::RELATION_TARGET_ID]) ? (int)$settings[Column::RELATION_TARGET_ID] : null; + if (empty($relationType) || empty($targetId)) { + return false; + } + + $hostTableId = $relationColumn->getTableId(); + $cacheKey = sprintf('%s_%s_%s', $hostTableId, $relationType, $targetId); + if (isset($this->cacheManagerAccess[$cacheKey])) { + return $this->cacheManagerAccess[$cacheKey]; + } + + $candidateUserIds = []; + try { + $hostTable = $this->tableMapper->find($hostTableId); + if ($hostTable->getOwnership() !== null && $hostTable->getOwnership() !== '') { + $candidateUserIds[] = $hostTable->getOwnership(); + } + } catch (DoesNotExistException|\OCP\AppFramework\Db\MultipleObjectsReturnedException|\OCP\DB\Exception $e) { + // host table gone, so nothing to expose + $this->cacheManagerAccess[$cacheKey] = false; + return false; + } + + try { + $candidateUserIds = array_unique(array_merge( + $candidateUserIds, + $this->shareService->findManagerUserIds($hostTableId, Application::NODE_TYPE_NAME_TABLE), + )); + } catch (InternalError $e) { + // fall back to the owner only + } + + $accessible = false; + foreach ($candidateUserIds as $candidateUserId) { + if ($candidateUserId === null || $candidateUserId === '') { + continue; + } + if ($relationType === Application::NODE_TYPE_NAME_VIEW) { + $canRead = $this->permissionsService->canReadColumnsByViewId($targetId, $candidateUserId); + } elseif ($relationType === Application::NODE_TYPE_NAME_TABLE) { + $canRead = $this->permissionsService->canReadColumnsByTableId($targetId, $candidateUserId); + } else { + $canRead = false; + } + if ($canRead) { + $accessible = true; + break; + } + } + + $this->cacheManagerAccess[$cacheKey] = $accessible; + return $accessible; + } + /** * Get relation data for a specific target * @@ -159,7 +229,7 @@ private function getRelationDataForTarget(string $target, Column $column): array return []; } - $isView = $settings[Column::RELATION_TYPE] === 'view'; + $isView = $settings[Column::RELATION_TYPE] === Application::NODE_TYPE_NAME_VIEW; $targetId = $settings[Column::RELATION_TARGET_ID] ?? null; try { diff --git a/lib/Service/ShareService.php b/lib/Service/ShareService.php index 72f8a09524..8d737480e2 100644 --- a/lib/Service/ShareService.php +++ b/lib/Service/ShareService.php @@ -794,29 +794,57 @@ public function transferSharesForContext(int $contextId, string $newOwnerId, str public function findSharedWithUserIds(int $elementId, string $elementType): array { try { $shares = $this->mapper->findAllSharesForNode($elementType, $elementId, ''); - $sharedWithUserIds = []; - - foreach ($shares as $share) { - if ($share->getReceiverType() === ShareReceiverType::USER) { - $sharedWithUserIds[$share->getReceiver()] = 1; - } - if ($share->getReceiverType() === ShareReceiverType::CIRCLE && $this->circleHelper->isCirclesEnabled()) { - $userIds = $this->circleHelper->getUserIdsInCircle($share->getReceiver()); - $sharedWithUserIds += array_fill_keys($userIds, 1); - } - if ($share->getReceiverType() === ShareReceiverType::GROUP) { - $userIds = $this->groupHelper->getUserIdsInGroup($share->getReceiver()); - $sharedWithUserIds += array_fill_keys($userIds, 1); - } - } - - return array_keys($sharedWithUserIds); + return $this->resolveShareReceiversToUserIds($shares); } catch (Exception $e) { $this->logger->error('Could not find shared with users: ' . $e->getMessage(), ['exception' => $e]); throw new InternalError('Could not find shared with users'); } } + /** + * Returns the IDs of all users that hold the manage permission on the given node + * + * @param int $elementId + * @param string $elementType + * @return string[] + * @throws InternalError + */ + public function findManagerUserIds(int $elementId, string $elementType): array { + try { + $shares = $this->mapper->findAllSharesForNode($elementType, $elementId, ''); + return $this->resolveShareReceiversToUserIds($shares, true); + } catch (Exception $e) { + $this->logger->error('Could not find managing users: ' . $e->getMessage(), ['exception' => $e]); + throw new InternalError('Could not find managing users'); + } + } + + /** + * Resolves a list of shares into the set of user IDs they grant access to + * + * @param Share[] $shares + * @param bool $requireManage when true, only shares carrying the manage permission are considered + * @return string[] + */ + private function resolveShareReceiversToUserIds(array $shares, bool $requireManage = false): array { + $userIds = []; + + foreach ($shares as $share) { + if ($requireManage && !$share->getPermissionManage()) { + continue; + } + if ($share->getReceiverType() === ShareReceiverType::USER) { + $userIds[$share->getReceiver()] = 1; + } elseif ($share->getReceiverType() === ShareReceiverType::CIRCLE && $this->circleHelper->isCirclesEnabled()) { + $userIds += array_fill_keys($this->circleHelper->getUserIdsInCircle($share->getReceiver()), 1); + } elseif ($share->getReceiverType() === ShareReceiverType::GROUP) { + $userIds += array_fill_keys($this->groupHelper->getUserIdsInGroup($share->getReceiver()), 1); + } + } + + return array_keys($userIds); + } + /** * @param int $nodeId * @param array $share diff --git a/src/shared/components/ncTable/partials/columnTypePartials/forms/RelationForm.vue b/src/shared/components/ncTable/partials/columnTypePartials/forms/RelationForm.vue index aaf9d967f9..ad1aa1ee8e 100644 --- a/src/shared/components/ncTable/partials/columnTypePartials/forms/RelationForm.vue +++ b/src/shared/components/ncTable/partials/columnTypePartials/forms/RelationForm.vue @@ -55,17 +55,22 @@ {{ t('tables', 'Only text and number columns can be used as label') }} + +