diff --git a/apps/files_sharing/lib/MountProvider.php b/apps/files_sharing/lib/MountProvider.php index 7c101314931..0a6c80769b2 100644 --- a/apps/files_sharing/lib/MountProvider.php +++ b/apps/files_sharing/lib/MountProvider.php @@ -7,6 +7,7 @@ */ namespace OCA\Files_Sharing; +use InvalidArgumentException; use OC\Files\View; use OCA\Files_Sharing\Event\ShareMountedEvent; use OCP\Cache\CappedMemoryCache; @@ -18,6 +19,7 @@ use OCP\Files\Storage\IStorageFactory; use OCP\ICacheFactory; use OCP\IConfig; use OCP\IUser; +use OCP\Share\IAttributes; use OCP\Share\IManager; use OCP\Share\IShare; use Psr\Log\LoggerInterface; @@ -175,23 +177,24 @@ class MountProvider implements IMountProvider { } /** - * Build super shares (virtual share) by grouping them by node id and target, - * then for each group compute the super share and return it along with the matching - * grouped shares. The most permissive permissions are used based on the permissions - * of all shares within the group. + * Groups shares by node ID and builds a new share object (super share) + * which represents a summarized version of all the shares in the group. + * + * The permissions and attributes of the super share are accumulated from + * the shares in the group, forming the most permissive combination + * possible. * * @param IShare[] $allShares * @param IUser $user user - * @return array Tuple of [superShare, groupedShares] + * @return list}> Tuple of [superShare, groupedShares] */ private function buildSuperShares(array $allShares, IUser $user) { $result = []; $groupedShares = $this->groupShares($allShares); - /** @var IShare[] $shares */ foreach ($groupedShares as $shares) { - if (count($shares) === 0) { + if (\count($shares) === 0) { continue; } @@ -204,14 +207,7 @@ class MountProvider implements IMountProvider { ->setShareType($shares[0]->getShareType()) ->setTarget($shares[0]->getTarget()); - // Gather notes from all the shares. - // Since these are readly available here, storing them - // enables the DAV FilesPlugin to avoid executing many - // DB queries to retrieve the same information. - $allNotes = implode("\n", array_map(function ($sh) { - return $sh->getNote(); - }, $shares)); - $superShare->setNote($allNotes); + $this->combineNotes($shares, $superShare); // use most permissive permissions // this covers the case where there are multiple shares for the same @@ -220,7 +216,6 @@ class MountProvider implements IMountProvider { $superAttributes = $this->shareManager->newShare()->newAttributes(); $status = IShare::STATUS_PENDING; foreach ($shares as $share) { - $superPermissions |= $share->getPermissions(); $status = max($status, $share->getStatus()); // update permissions $superPermissions |= $share->getPermissions(); @@ -228,38 +223,11 @@ class MountProvider implements IMountProvider { // update share permission attributes $attributes = $share->getAttributes(); if ($attributes !== null) { - foreach ($attributes->toArray() as $attribute) { - if ($superAttributes->getAttribute($attribute['scope'], $attribute['key']) === true) { - // if super share attribute is already enabled, it is most permissive - continue; - } - // update supershare attributes with subshare attribute - $superAttributes->setAttribute($attribute['scope'], $attribute['key'], $attribute['value']); - } + $this->mergeAttributes($attributes, $superAttributes); } - // adjust target, for database consistency if needed - if ($share->getTarget() !== $superShare->getTarget()) { - $share->setTarget($superShare->getTarget()); - try { - $this->shareManager->moveShare($share, $user->getUID()); - } catch (\InvalidArgumentException $e) { - // ignore as it is not important and we don't want to - // block FS setup - - // the subsequent code anyway only uses the target of the - // super share - - // such issue can usually happen when dealing with - // null groups which usually appear with group backend - // caching inconsistencies - $this->logger->debug( - 'Could not adjust share target for share ' . $share->getId() . ' to make it consistent: ' . $e->getMessage(), - ['app' => 'files_sharing'] - ); - } - } - if (!is_null($share->getNodeCacheEntry())) { + $this->adjustTarget($share, $superShare, $user); + if ($share->getNodeCacheEntry() !== null) { $superShare->setNodeCacheEntry($share->getNodeCacheEntry()); } } @@ -273,4 +241,82 @@ class MountProvider implements IMountProvider { return $result; } + + /** + * Combines $attributes into the most permissive set of attributes and + * sets them in $superAttributes. + */ + private function mergeAttributes( + IAttributes $attributes, + IAttributes $superAttributes, + ): void { + foreach ($attributes->toArray() as $attribute) { + if ($superAttributes->getAttribute( + $attribute['scope'], + $attribute['key'] + ) === true) { + // if super share attribute is already enabled, it is most permissive + continue; + } + // update super share attributes with subshare attribute + $superAttributes->setAttribute( + $attribute['scope'], + $attribute['key'], + $attribute['value'] + ); + } + } + + /** + * Gather notes from all the shares. Since these are readily available + * here, storing them enables the DAV FilesPlugin to avoid executing many + * DB queries to retrieve the same information. + * + * @param array $shares + * @param IShare $superShare + * @return void + */ + private function combineNotes( + array &$shares, + IShare $superShare, + ): void { + $allNotes = implode( + "\n", + array_map(static fn ($sh) => $sh->getNote(), $shares) + ); + $superShare->setNote($allNotes); + } + + /** + * Adjusts the target in $share for DB consistency, if needed. + */ + private function adjustTarget( + IShare $share, + IShare $superShare, + IUser $user, + ): void { + if ($share->getTarget() === $superShare->getTarget()) { + return; + } + + $share->setTarget($superShare->getTarget()); + try { + $this->shareManager->moveShare($share, $user->getUID()); + } catch (InvalidArgumentException $e) { + // ignore as it is not important and we don't want to + // block FS setup + + // the subsequent code anyway only uses the target of the + // super share + + // such issue can usually happen when dealing with + // null groups which usually appear with group backend + // caching inconsistencies + $this->logger->debug( + 'Could not adjust share target for share ' . $share->getId( + ) . ' to make it consistent: ' . $e->getMessage(), + ['app' => 'files_sharing'] + ); + } + } }