refactor: consolidate relation target resolution

lib/Db/Column.php

@@ -121,6 +121,9 @@ class Column extends EntitySuper implements JsonSerializable {
 	public const RELATION_TYPE = 'relationType';
 	public const RELATION_TARGET_ID = 'targetId';
 	public const RELATION_LABEL_COLUMN = 'labelColumn';
+	public const RELATION_TARGET_SET_BY = 'targetSetBy';
+	public const RELATION_TYPE_TABLE = 'table';
+	public const RELATION_TYPE_VIEW = 'view';
 
 	protected ?string $uuid = null;
 	protected ?string $title = null;

lib/Service/ColumnService.php

@@ -294,6 +294,7 @@ public function create(
 		$item = Column::fromDto($columnDto);
 		$item->setTitle($newTitle);
 		$item->setTableId($table->getId());
+		$item->setCustomSettings($this->authorizeRelationTarget($item->getCustomSettings(), $userId));
 		$this->updateMetadata($item, $userId, true);
 
 		try {
@@ -423,7 +424,7 @@ public function update(
 			$item->setUsergroupSelectTeams($columnDto->getUsergroupSelectTeams());
 			$item->setShowUserStatus($columnDto->getShowUserStatus());
 			$this->validateCustomSettings($columnDto->getCustomSettings());
-			$item->setCustomSettings($columnDto->getCustomSettings());
+			$item->setCustomSettings($this->authorizeRelationTarget($columnDto->getCustomSettings(), $userId, $item->getCustomSettings()));
 
 			$this->updateMetadata($item, $userId);
 			try {
@@ -446,7 +447,7 @@ public function update(
 			} catch (\OCP\DB\Exception $e) {
 				$this->handleColumnPersistDbException($e, static::class . ' - ' . __FUNCTION__);
 			}
-		} catch (BadRequestError $e) {
+		} catch (BadRequestError|PermissionError|NotFoundError $e) {
 			throw $e;
 		} catch (Exception $e) {
 			$this->logger->error($e->getMessage());
@@ -507,6 +508,97 @@ private function validateCustomSettings(?string $customSettings): void {
 		}
 	}
 
+	private function hasRelationTarget(array $settings): bool {
+		return !empty($settings[Column::RELATION_TYPE])
+			&& !empty($settings[Column::RELATION_TARGET_ID])
+			&& !empty($settings[Column::RELATION_LABEL_COLUMN]);
+	}
+
+	/**
+	 * @return array{type: string, targetId: int, labelColumn: int}|null
+	 */
+	private function parseRelationTarget(?string $customSettings): ?array {
+		$settings = $customSettings !== null ? json_decode($customSettings, true) : null;
+		if (!is_array($settings) || !$this->hasRelationTarget($settings)) {
+			return null;
+		}
+
+		return [
+			'type' => (string)$settings[Column::RELATION_TYPE],
+			'targetId' => (int)$settings[Column::RELATION_TARGET_ID],
+			'labelColumn' => (int)$settings[Column::RELATION_LABEL_COLUMN],
+		];
+	}
+
+	private function setRelationTargetAuthor(?string $customSettings, ?string $userId): ?string {
+		if ($customSettings === null) {
+			return null;
+		}
+		$settings = json_decode($customSettings, true);
+		if (!is_array($settings)) {
+			return $customSettings;
+		}
+
+		unset($settings[Column::RELATION_TARGET_SET_BY]);
+		if ((string)$userId !== '' && $this->hasRelationTarget($settings)) {
+			$settings[Column::RELATION_TARGET_SET_BY] = $userId;
+		}
+
+		return json_encode($settings, JSON_THROW_ON_ERROR);
+	}
+
+	/**
+	 * @throws BadRequestError
+	 * @throws NotFoundError
+	 * @throws PermissionError
+	 */
+	private function authorizeRelationTarget(?string $customSettings, ?string $userId, ?string $storedSettings = null): ?string {
+		$target = $this->parseRelationTarget($customSettings);
+		if ($target !== null && $target === $this->parseRelationTarget($storedSettings)) {
+			return $this->setRelationTargetAuthor($customSettings, $this->getRelationTargetAuthor($storedSettings));
+		}
+		if ($target !== null) {
+			$this->assertCanReadRelationTarget($target['type'], $target['targetId'], $target['labelColumn'], $userId);
+		}
+		return $this->setRelationTargetAuthor($customSettings, $userId);
+	}
+
+	private function getRelationTargetAuthor(?string $customSettings): ?string {
+		$settings = $customSettings !== null ? json_decode($customSettings, true) : null;
+		return is_array($settings) && isset($settings[Column::RELATION_TARGET_SET_BY]) ? (string)$settings[Column::RELATION_TARGET_SET_BY] : null;
+	}
+
+	/**
+	 * @throws BadRequestError
+	 * @throws NotFoundError
+	 * @throws PermissionError
+	 */
+	private function assertCanReadRelationTarget(string $relationType, int $targetId, int $labelColumnId, ?string $userId): void {
+		if (!in_array($relationType, [Column::RELATION_TYPE_TABLE, Column::RELATION_TYPE_VIEW], true)) {
+			throw new BadRequestError('Unknown relation type ' . $relationType . '.');
+		}
+		if (!$this->permissionsService->canReadRelationTarget($relationType, $targetId, $userId)) {
+			throw new PermissionError('No permission to use ' . $relationType . ' ' . $targetId . ' as relation target.');
+		}
+
+		if ($relationType === Column::RELATION_TYPE_VIEW) {
+			$view = $this->viewService->find($targetId, true, $userId);
+			if (!in_array($labelColumnId, $view->getColumnIds(), true)) {
+				throw new BadRequestError('Relation label column does not belong to the relation target.');
+			}
+			return;
+		}
+
+		try {
+			$labelColumn = $this->mapper->find($labelColumnId);
+		} catch (DoesNotExistException|MultipleObjectsReturnedException|\OCP\DB\Exception) {
+			throw new BadRequestError('Relation label column does not belong to the relation target.');
+		}
+		if ($labelColumn->getTableId() !== $targetId) {
+			throw new BadRequestError('Relation label column does not belong to the relation target.');
+		}
+	}
+
 	private function normalizeTitle(?string $title, bool $required): ?string {
 		if ($title === null) {
 			if ($required) {
@@ -779,7 +871,7 @@ public function importColumn(Table $table, array $column): int {
 		$item->setUsergroupSelectGroups($column['usergroupSelectGroups']);
 		$item->setUsergroupSelectTeams($column['usergroupSelectTeams']);
 		$item->setShowUserStatus($column['showUserStatus']);
-		$item->setCustomSettings(json_encode($column['customSettings']));
+		$item->setCustomSettings($this->setRelationTargetAuthor(json_encode($column['customSettings']) ?: null, $this->userId));
 
 		try {
 			$newColumn = $this->mapper->insert($item);

lib/Service/PermissionsService.php

@@ -8,6 +8,7 @@
 namespace OCA\Tables\Service;
 
 use OCA\Tables\AppInfo\Application;
+use OCA\Tables\Db\Column;
 use OCA\Tables\Db\Context;
 use OCA\Tables\Db\ContextMapper;
 use OCA\Tables\Db\Share;
@@ -271,6 +272,14 @@ public function canReadColumnsByTableId(int $tableId, ?string $userId = null): b
 		return $canCreateRows || $canReadRows;
 	}
 
+	public function canReadRelationTarget(string $relationType, int $targetId, ?string $userId = null): bool {
+		return match ($relationType) {
+			Column::RELATION_TYPE_TABLE => $this->canReadRowsByElementId($targetId, 'table', $userId),
+			Column::RELATION_TYPE_VIEW => $this->canReadRowsByElementId($targetId, 'view', $userId),
+			default => false,
+		};
+	}
+
 	/**
 	 * @param Table $table
 	 * @param string|null $userId

lib/Service/RelationService.php

@@ -25,6 +25,7 @@ public function __construct(
 		private readonly ViewMapper $viewMapper,
 		private readonly Row2Mapper $row2Mapper,
 		private readonly ColumnService $columnService,
+		private readonly PermissionsService $permissionsService,
 		private readonly ?string $userId,
 	) {
 	}
@@ -103,7 +104,7 @@ private function groupColumnsByTarget(array $columns): array {
 				continue;
 			}
 
-			$target = sprintf('%s_%s_%s', $settings['relationType'], $settings['targetId'], $settings['labelColumn']);
+			$target = sprintf('%s_%s_%s_%s', $settings['relationType'], $settings['targetId'], $settings['labelColumn'], $settings[Column::RELATION_TARGET_SET_BY] ?? '');
 			$groups[$target] ??= [];
 			$groups[$target][] = $column;
 		}
@@ -127,7 +128,7 @@ public function getRelationData(Column $column): array {
 			return [];
 		}
 
-		$target = sprintf('%s_%s_%s', $settings['relationType'], $settings['targetId'], $settings['labelColumn']);
+		$target = sprintf('%s_%s_%s_%s', $settings['relationType'], $settings['targetId'], $settings['labelColumn'], $settings[Column::RELATION_TARGET_SET_BY] ?? '');
 
 		return $this->getRelationDataForTarget($target, $column);
 	}
@@ -153,13 +154,25 @@ private function getRelationDataForTarget(string $target, Column $column): array
 			return [];
 		}
 
-		$isView = $settings[Column::RELATION_TYPE] === 'view';
+		$relationType = (string)$settings[Column::RELATION_TYPE];
+		$isView = $relationType === Column::RELATION_TYPE_VIEW;
 		$targetId = $settings[Column::RELATION_TARGET_ID] ?? null;
 
+		$targetSetBy = $settings[Column::RELATION_TARGET_SET_BY] ?? '';
+		$authorizeAs = $targetSetBy !== '' ? (string)$targetSetBy : $this->userId;
+		if (!$this->permissionsService->canReadRelationTarget($relationType, (int)$targetId, $authorizeAs)) {
+			$this->cacheRelationData[$cacheKey] = [];
+			return [];
+		}
+
 		try {
 			$targetColumn = $this->columnMapper->find($settings[Column::RELATION_LABEL_COLUMN]);
 			if ($isView) {
 				$view = $this->viewMapper->find($targetId);
+				if (!in_array($targetColumn->getId(), $view->getColumnIds(), true)) {
+					$this->cacheRelationData[$cacheKey] = [];
+					return [];
+				}
 				$rows = $this->row2Mapper->findAll(
 					[$targetColumn->getId()],
 					$view->getTableId(),

tests/integration/features/APIv1.feature

@@ -764,6 +764,7 @@ Feature: APIv1
     And user "participant1" sets filter to view "available-fruits"
       | column    | operator | value |
       | available | is-equal | true  |
+    And user "participant1" sets columnSettings "name,available" to view "available-fruits"
     Given table "Smoothies" with emoji "🥤" exists for user "participant1" as "smoothies"
     Then relation column "ingredient" exists on table "smoothies" pointing to view "available-fruits" using label column "name"
     When user "participant1" fetches relations for table "smoothies"
@@ -804,6 +805,51 @@ Feature: APIv1
     When user "participant2" fetches relations for table "secret-orders"
     Then the reported status is "404"
 
+  @api1 @relation @table-sharing
+  Scenario: Unauthorized user cannot create a relation column to a table
+    Given table "Secret Products" with emoji "🔒" exists for user "participant1" as "secret-products"
+    Then column "name" exists with following properties
+      | type      | text |
+      | subtype   | line |
+      | mandatory | 0    |
+    Given table "My Orders" with emoji "🛒" exists for user "participant2" as "my-orders"
+    When user "participant2" attempts to create relation column "product" on table "my-orders" pointing to table "secret-products" using label column "name"
+    Then the reported status is "403"
+
+  @api1 @relation @table-sharing
+  Scenario: Shared relation column shows labels without access to the target table
+    Given table "Products" with emoji "📦" exists for user "participant1" as "products"
+    Then column "name" exists with following properties
+      | type      | text |
+      | subtype   | line |
+      | mandatory | 0    |
+    Then row exists with following values
+      | name | Apple |
+    Given table "Orders" with emoji "🛒" exists for user "participant1" as "orders"
+    Then relation column "product" exists on table "orders" pointing to table "products" using label column "name"
+    Then user "participant1" shares table with user "participant2"
+    When user "participant2" fetches relations for table "orders"
+    Then the reported status is "200"
+    Then the relations response contains 1 entries for column "product"
+    Then the relations response for column "product" has an entry with label "Apple"
+
+  @api1 @relation
+  Scenario: Relation column to a view cannot use a column hidden in the view
+    Given table "Fruits" with emoji "🍎" exists for user "participant1" as "fruits"
+    Then column "name" exists with following properties
+      | type      | text |
+      | subtype   | line |
+      | mandatory | 0    |
+    Then column "price" exists with following properties
+      | type      | text |
+      | subtype   | line |
+      | mandatory | 0    |
+    When user "participant1" create view "Names only" with emoji "🏷️" for "fruits" as "names-only"
+    And user "participant1" sets columnSettings "name" to view "names-only"
+    Given table "Smoothies" with emoji "🥤" exists for user "participant1" as "smoothies"
+    When user "participant1" attempts to create relation column "ingredient" on table "smoothies" pointing to view "names-only" using label column "price"
+    Then the reported status is "400"
+
   @api1 @columns @rows
   Scenario: Column technicalName is returned and dataByAlias maps alias to row values
     Given table "Alias test" with emoji "🔖" exists for user "participant1" as "base1"