[WIP] allow every with direct access to a shared file to modify all shares for the file

apps/files/lib/Sharing/Source/NodeShareSourceType.php

@@ -25,6 +25,7 @@
 use OCP\Files\Events\Node\NodeDeletedEvent;
 use OCP\Files\IRootFolder;
 use OCP\Files\Node;
+use OCP\Files\Storage\ISharedStorage;
 use OCP\IDBConnection;
 use OCP\Interaction\InteractionResource;
 use OCP\Interaction\Resources\NodeResource;
@@ -95,4 +96,18 @@ public function handle(Event $event): void {
 			throw $exception;
 		}
 	}
+
+	#[\Override]
+	public function userHasDirectSharingAccessToSource(IUser $user, string $source): bool {
+		// TODO: cache nodes by id?
+		$userFolder = $this->rootFolder->getUserFolder($user->getUID());
+		$nodes = $userFolder->getById((int)$source);
+		foreach ($nodes as $node) {
+			if (!$node->getStorage() instanceof ISharedStorage && $node->isShareable()) {
+				return true;
+			}
+		}
+
+		return false;
+	}
 }

lib/private/Sharing/SharingBackend.php

@@ -588,9 +588,22 @@ private function hideDisabledUserShares(): bool {
 	 * @return list<Share>
 	 */
 	private function list(
-		ShareAccessContext $accessContext, ?string $filterShareID, ?string $filterSourceTypeClass, ?string $filterSourceTypeValue, ?string $lastShareID,
+		ShareAccessContext $accessContext,
+		?string $filterShareID,
+		?string $filterSourceTypeClass,
+		?string $filterSourceTypeValue,
+		?string $lastShareID,
 		?int $limit,
 	): array {
+		if ($filterSourceTypeClass) {
+			$filterSourceType = $this->registry->getSourceTypes()[$filterSourceTypeClass] ?? null;
+			if ($filterSourceType === null) {
+				throw new RuntimeException('The source type is not registered: ' . $filterSourceTypeClass);
+			}
+		} else {
+			$filterSourceType = null;
+		}
+
 		/** @var array<class-string<IShareRecipientType>, list<string>> $recipientTypeValues */
 		$recipientTypeValues = [];
 
@@ -601,7 +614,11 @@ private function list(
 		} else {
 			if ($accessContext->currentUser instanceof IUser) {
 				$qb = $this->connection->getQueryBuilder();
-				$qb->where($qb->expr()->eq('s.owner_user_id', $qb->createNamedParameter($accessContext->currentUser->getUID())));
+				// if we're filtering by source or id, we need to also check for non-owned shares
+				if ($filterSourceTypeValue === null && $filterShareID === null) {
+					$qb->where($qb->expr()->eq('s.owner_user_id', $qb->createNamedParameter($accessContext->currentUser->getUID())));
+				}
+
 				$queries[] = $qb;
 			}
 
@@ -613,6 +630,7 @@ private function list(
 			}
 
 			// Do not add a query if no recipients matched, otherwise all shares will be returned.
+			// If the user has "direct" access, we already get all the shares, so no need to run an extra query for recipients
 			if ($recipientTypeValues !== []) {
 				$qb = $this->connection->getQueryBuilder();
 				$qb->innerJoin(
@@ -672,7 +690,7 @@ private function list(
 				$qb->andWhere($qb->expr()->eq('s.id', $qb->createNamedParameter($filterShareID)));
 			}
 
-			if ($filterSourceTypeClass !== null) {
+			if ($filterSourceType !== null && $filterSourceTypeClass !== null) {
 				$sourceTypeFilters = [
 					$qb->expr()->eq('s.id', 'ss.share_id'),
 					$qb->expr()->eq(
@@ -867,8 +885,10 @@ private function list(
 
 		// Some recipients might have been removed if the initiator was disabled, so check again if this share can be accessed by the current user as a recipient.
 		// This logic is a bit duplicated with the SQL logic that selects shares based on the secret and the recipient type values, but neither can be removed.
+		/** @var array<string, bool> $hasRecipientAccess */
+		$hasRecipientAccess = [];
 		if (!$accessContext->overrideChecks) {
-			foreach ($shares as $id => &$share) {
+			foreach ($shares as &$share) {
 				if ($share['owner']->isCurrentUser($accessContext)) {
 					continue;
 				}
@@ -904,9 +924,7 @@ private function list(
 
 				unset($recipient);
 
-				if (!$isAnyMatchingRecipient) {
-					unset($shares[$id]);
-				}
+				$hasRecipientAccess[$share['id']] = $isAnyMatchingRecipient && $share['state'] === ShareState::Active;
 			}
 
 			unset($share);
@@ -1016,6 +1034,41 @@ private function list(
 			$share['permissions'],
 		), $shares);
 
+		// when listing shares for a source, we also return any non-owned share if the user has "direct" access to the source
+		// but we do need to validate that the user has "direct" access to *all* of the sources in the share, not just one
+		$hasSourceAccess = [];
+		if (!$accessContext->overrideChecks && $accessContext->currentUser instanceof IUser) {
+			foreach ($shares as $share) {
+				if ($share->owner->isCurrentUser($accessContext)) {
+					continue;
+				}
+
+				if ($hasRecipientAccess[$share->id]) {
+					continue;
+				}
+
+				if ($share->sources === []) {
+					$hasSourceAccess[$share->id] = false;
+					continue;
+				}
+
+				$hasSourceAccess[$share->id] = true;
+				foreach ($share->sources as $source) {
+					$sourceType = $this->registry->getSourceTypes()[$source->class];
+					if (!$sourceType->userHasDirectSharingAccessToSource($accessContext->currentUser, $source->value)) {
+						$hasSourceAccess[$share->id] = false;
+					}
+				}
+			}
+		}
+
+		if (!$accessContext->overrideChecks) {
+			$shares = array_filter(
+				$shares,
+				fn (Share $share): bool => $share->owner->isCurrentUser($accessContext) || $hasRecipientAccess[$share->id] || $hasSourceAccess[$share->id]
+			);
+		}
+
 		if (!$accessContext->overrideChecks) {
 			$filterPropertyTypes = array_filter(
 				$registryPropertyTypes, static fn (ISharePropertyType $propertyType): bool => $propertyType instanceof ISharePropertyTypeFilter

lib/private/Sharing/SharingManager.php

@@ -190,7 +190,7 @@ public function updateShareState(ShareAccessContext $accessContext, Share $share
 		$time = $this->getTime();
 		$this->backend->setLastUpdated([$share->id], $time);
 
-		$this->validateShareOwnerOperation($accessContext, $share->owner);
+		$this->validateShareEditPermissions($accessContext, $share);
 
 		if ($state === ShareState::Active) {
 			$this->assertShareCanBeActive($share);
@@ -217,7 +217,8 @@ public function updateShareState(ShareAccessContext $accessContext, Share $share
 	public function addShareSource(ShareAccessContext $accessContext, Share $share, ShareSource $source): Share {
 		$this->assertInTransaction();
 
-		$this->validateShareOwnerOperation($accessContext, $share->owner);
+		// only the owner can add sources, otherwise a user could add sources others don't have access to, which would remove their access
+		$this->validateShareEditPermissions($accessContext, $share, true);
 
 		if (($sourceType = $this->registry->getSourceTypes()[$source->class] ?? null) === null) {
 			throw new RuntimeException('The source type is not registered: ' . $source->class);
@@ -260,7 +261,8 @@ public function addShareSource(ShareAccessContext $accessContext, Share $share,
 	public function removeShareSource(ShareAccessContext $accessContext, Share $share, ShareSource $source): Share {
 		$this->assertInTransaction();
 
-		$this->validateShareOwnerOperation($accessContext, $share->owner);
+		// only the owner can remove sources, to mirror the "add source" permissions
+		$this->validateShareEditPermissions($accessContext, $share, true);
 
 		$time = $this->getTime();
 		$this->backend->setLastUpdated([$share->id], $time);
@@ -321,7 +323,7 @@ public function addShareRecipient(ShareAccessContext $accessContext, Share $shar
 		$this->assertInTransaction();
 
 		try {
-			$this->validateShareOwnerOperation($accessContext, $share->owner);
+			$this->validateShareEditPermissions($accessContext, $share);
 		} catch (ShareOperationForbiddenException) {
 			$this->validatePermission($share, ReshareSharePermissionType::class);
 		}
@@ -398,7 +400,7 @@ public function removeShareRecipient(ShareAccessContext $accessContext, Share $s
 		$this->assertInTransaction();
 
 		try {
-			$this->validateShareOwnerOperation($accessContext, $share->owner);
+			$this->validateShareEditPermissions($accessContext, $share);
 		} catch (ShareOperationForbiddenException) {
 			// This does not allow removing own recipients. A user can only reject a share, but not remove it for the recipient.
 			$this->validateReshareOperation($accessContext, $share, $recipient);
@@ -484,7 +486,7 @@ public function updateShareRecipientSecret(ShareAccessContext $accessContext, Sh
 		$this->assertInTransaction();
 
 		try {
-			$this->validateShareOwnerOperation($accessContext, $share->owner);
+			$this->validateShareEditPermissions($accessContext, $share);
 		} catch (ShareOperationForbiddenException) {
 			$this->validateReshareOperation($accessContext, $share, $recipient);
 		}
@@ -540,7 +542,7 @@ public function updateShareRecipientSecret(ShareAccessContext $accessContext, Sh
 	public function updateShareProperty(ShareAccessContext $accessContext, Share $share, ShareProperty $property): Share {
 		$this->assertInTransaction();
 
-		$this->validateShareOwnerOperation($accessContext, $share->owner);
+		$this->validateShareEditPermissions($accessContext, $share);
 
 		if (($propertyType = $this->registry->getPropertyTypes()[$property->class] ?? null) === null) {
 			throw new RuntimeException('The property is not registered: ' . $property->class);
@@ -577,7 +579,7 @@ public function updateShareProperty(ShareAccessContext $accessContext, Share $sh
 	public function updateSharePermission(ShareAccessContext $accessContext, Share $share, SharePermission $permission): Share {
 		$this->assertInTransaction();
 
-		$this->validateShareOwnerOperation($accessContext, $share->owner);
+		$this->validateShareEditPermissions($accessContext, $share);
 
 		if (!isset($this->registry->getPermissionTypes()[$permission->class])) {
 			throw new RuntimeException('The permission type is not registered: ' . $permission->class);
@@ -614,7 +616,7 @@ public function updateSharePermission(ShareAccessContext $accessContext, Share $
 	public function selectSharePermissionPreset(ShareAccessContext $accessContext, Share $share, string $permissionPresetClass): Share {
 		$this->assertInTransaction();
 
-		$this->validateShareOwnerOperation($accessContext, $share->owner);
+		$this->validateShareEditPermissions($accessContext, $share);
 
 		if (($this->registry->getPermissionPresetCompatiblePermissionTypeClasses()[$permissionPresetClass] ?? null) === null) {
 			throw new RuntimeException('The permission preset is not registered: ' . $permissionPresetClass);
@@ -654,7 +656,7 @@ public function deleteShare(ShareAccessContext $accessContext, Share $share): vo
 
 		// No need to update the last updated timestamp, because the share will be deleted anyway.
 
-		$this->validateShareOwnerOperation($accessContext, $share->owner);
+		$this->validateShareEditPermissions($accessContext, $share);
 
 		$this->backend->deleteShare($share->id);
 
@@ -710,27 +712,43 @@ private function assertInTransaction(): void {
 		}
 	}
 
-	// TODO: Support IShareOwnerlessMount
-
 	/**
 	 * @throws ShareOperationForbiddenException
 	 */
-	private function validateShareOwnerOperation(ShareAccessContext $accessContext, ShareUser $owner): void {
+	private function validateShareEditPermissions(ShareAccessContext $accessContext, Share $share, bool $onlyOwner = false): void {
 		if ($accessContext->overrideChecks) {
 			return;
 		}
 
-		if ($owner->instance !== null || !$accessContext->currentUser instanceof IUser || $owner->userId !== $accessContext->currentUser->getUID()) {
+		if ($share->owner->instance !== null || !$accessContext->currentUser instanceof IUser) {
+			throw new ShareOperationForbiddenException();
+		}
+
+		if ($share->owner->userId === $accessContext->currentUser->getUID()) {
+			return;
+		}
+
+		if ($onlyOwner) {
 			throw new ShareOperationForbiddenException();
 		}
+
+		foreach ($share->sources as $source) {
+			$sourceType = $this->registry->getSourceTypes()[$source->class] ?? null;
+			if (!$sourceType) {
+				throw new ShareOperationForbiddenException();
+			}
+
+			if (!$sourceType->userHasDirectSharingAccessToSource($accessContext->currentUser, $source->value)) {
+				throw new ShareOperationForbiddenException();
+			}
+		}
 	}
 
 	/**
 	 * @param class-string<ISharePermissionType> $permissionTypeClass
 	 * @throws ShareOperationForbiddenException
 	 */
 	private function validatePermission(Share $share, string $permissionTypeClass): void {
-		// TODO: Only fetch permisions
 		if ((($permission = $share->permissions[$permissionTypeClass] ?? null) !== null) && $permission->enabled) {
 			return;
 		}
@@ -744,7 +762,6 @@ private function validatePermission(Share $share, string $permissionTypeClass):
 	private function validateReshareOperation(ShareAccessContext $accessContext, Share $share, ShareRecipient $recipient): void {
 		$this->validatePermission($share, ReshareSharePermissionType::class);
 
-		// TODO: Only fetch recipients
 		foreach ($share->recipients as $shareRecipient) {
 			if (
 				$recipient->class === $shareRecipient->class

lib/unstable/Sharing/Source/IShareSourceType.php

@@ -55,4 +55,14 @@ public function getSourcesMetadata(array $sources): array;
 	 * @experimental 35.0.0
 	 */
 	public function getSourceInteractionResource(IUser $user, string $source): InteractionResource;
+
+	/**
+	 * Check if a user has access to the specified source without taking sharing into account, and has sufficient permissions to create shares.
+	 *
+	 * All users with "direct" access to the source will be able to see and manage shares made by other users for the source.
+	 *
+	 * @experimental 35.0.0
+	 * @param non-empty-string $source
+	 */
+	public function userHasDirectSharingAccessToSource(IUser $user, string $source): bool;
 }