feat: enhance relation permissions and accessibility checks

Signed-off-by: Enjeck C. <patrathewhiz@gmail.com>

Author: enjeck

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;

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) {

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<string, array> Cache for relation data */
 	private array $cacheRelationData = [];
 
+	/** @var array<string, bool> 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 {

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

src/shared/components/ncTable/partials/columnTypePartials/forms/RelationForm.vue

@@ -55,25 +55,32 @@
 			<IconInformation :size="16" class="info-icon" />
 			<span>{{ t('tables', 'Only text and number columns can be used as label') }}</span>
 		</div>
+
+		<NcNoteCard v-if="targetInaccessible"
+			type="warning"
+			:text="t('tables', 'The linked table or view is no longer accessible to you, so this relation is disabled. Ask its owner to share it with you, or pick a different target.')" />
 	</div>
 </template>
 
 <script>
-import { NcSelect } from '@nextcloud/vue'
+import { NcSelect, NcNoteCard } from '@nextcloud/vue'
 import { translate as t } from '@nextcloud/l10n'
 import { mapState } from 'pinia'
 import { useTablesStore } from '../../../../../../store/store.js'
 import { useDataStore } from '../../../../../../store/data.js'
 import NumberColumn from '../../../mixins/columnsTypes/number.js'
 import TextLineColumn from '../../../mixins/columnsTypes/textLine.js'
+import permissionsMixin from '../../../mixins/permissionsMixin.js'
 import IconInformation from 'vue-material-design-icons/InformationOutline.vue'
 
 export default {
 	name: 'RelationForm',
 	components: {
 		IconInformation,
 		NcSelect,
+		NcNoteCard,
 	},
+	mixins: [permissionsMixin],
 	props: {
 		column: {
 			type: Object,
@@ -88,6 +95,7 @@ export default {
 				relationType: this.column.customSettings.relationType ?? 'table',
 			},
 			loadingColumns: false,
+			targetInaccessible: false,
 			relationTypeOptions: [
 				{ id: 'table', label: t('tables', 'Table') },
 				{ id: 'view', label: t('tables', 'View') },
@@ -96,20 +104,29 @@ export default {
 		}
 	},
 	computed: {
-		...mapState(useTablesStore, ['tables', 'views']),
+		...mapState(useTablesStore, ['tables', 'views', 'activeTable', 'activeView']),
+		hostTableId() {
+			return this.activeView ? this.activeView.tableId : (this.activeTable?.id ?? null)
+		},
 		availableTargets() {
 			if (this.customSettings.relationType === 'table') {
-				return this.tables.map(table => ({
-					id: table.id,
-					label: `${table.emoji} ${table.title}`,
-				}))
+				return this.tables
+					// exclude the host table (no self-reference) and anything the
+					// user cannot read
+					.filter(table => table.id !== this.hostTableId && this.canReadData(table))
+					.map(table => ({
+						id: table.id,
+						label: `${table.emoji} ${table.title}`,
+					}))
 			}
 
 			if (this.customSettings.relationType === 'view') {
-				return this.views.map(view => ({
-					id: view.id,
-					label: `${view.emoji} ${view.title}`,
-				}))
+				return this.views
+					.filter(view => view.tableId !== this.hostTableId && this.canReadData(view))
+					.map(view => ({
+						id: view.id,
+						label: `${view.emoji} ${view.title}`,
+					}))
 			}
 
 			return []
@@ -129,17 +146,23 @@ export default {
 			}
 
 			this.loadingColumns = true
+			this.targetInaccessible = false
 			try {
 				const dataStore = useDataStore()
 				const columns = await dataStore.getColumnsFromBE({
 					tableId: this.customSettings.relationType === 'table' ? this.customSettings.targetId : null,
 					viewId: this.customSettings.relationType === 'view' ? this.customSettings.targetId : null,
+					showError: false,
 				})
 				this.availableLabelColumns = columns
 					.filter(column =>
 						column instanceof NumberColumn || column instanceof TextLineColumn,
 					)
 					.map(column => ({ id: column.id, label: column.title }))
+			} catch (e) {
+				// The target is no longer readable (e.g. it was unshared)
+				this.availableLabelColumns = []
+				this.targetInaccessible = true
 			} finally {
 				this.loadingColumns = false
 			}