diff --git a/apps/files/lib/Sharing/Source/NodeShareSourceType.php b/apps/files/lib/Sharing/Source/NodeShareSourceType.php index b5aabf1dc6f2f..d3385d6f94705 100644 --- a/apps/files/lib/Sharing/Source/NodeShareSourceType.php +++ b/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; + } } diff --git a/lib/private/Sharing/SharingBackend.php b/lib/private/Sharing/SharingBackend.php index e8cd285e50389..aaa9d4f852b37 100644 --- a/lib/private/Sharing/SharingBackend.php +++ b/lib/private/Sharing/SharingBackend.php @@ -588,9 +588,22 @@ private function hideDisabledUserShares(): bool { * @return list */ 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, list> $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 $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 diff --git a/lib/private/Sharing/SharingManager.php b/lib/private/Sharing/SharingManager.php index 468be32cb0dee..c04308a9a7e15 100644 --- a/lib/private/Sharing/SharingManager.php +++ b/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,19 +712,36 @@ 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(); + } + } } /** @@ -730,7 +749,6 @@ private function validateShareOwnerOperation(ShareAccessContext $accessContext, * @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 diff --git a/lib/unstable/Sharing/Source/IShareSourceType.php b/lib/unstable/Sharing/Source/IShareSourceType.php index 8bda3d2131d9d..f4cf17ae7f697 100644 --- a/lib/unstable/Sharing/Source/IShareSourceType.php +++ b/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; } diff --git a/tests/lib/Sharing/AbstractSharingManagerTests.php b/tests/lib/Sharing/AbstractSharingManagerTests.php index d1f79eb076427..74d1bed522ecb 100644 --- a/tests/lib/Sharing/AbstractSharingManagerTests.php +++ b/tests/lib/Sharing/AbstractSharingManagerTests.php @@ -9,6 +9,8 @@ namespace Test\Sharing; +use NCU\Sharing\Exception\ShareNotFoundException; +use NCU\Sharing\Exception\ShareOperationForbiddenException; use NCU\Sharing\ISharingManager; use NCU\Sharing\ISharingRegistry; use NCU\Sharing\Permission\SharePermission; @@ -41,7 +43,9 @@ * @psalm-suppress PossiblyUndefinedArrayOffset */ abstract class AbstractSharingManagerTests extends TestCase { - abstract protected function searchRecipients(ShareAccessContext $accessContext, ?array $filterRecipientTypeClasses, string $query, int $limit, int $offset, ?Share $forShare = null): array; + abstract protected function searchRecipients( + ShareAccessContext $accessContext, ?array $filterRecipientTypeClasses, string $query, int $limit, int $offset, ?Share $forShare = null, + ): array; /** * @return SharingShare @@ -103,7 +107,9 @@ abstract protected function getShare(ShareAccessContext $accessContext, string $ /** * @return SharingShare[] */ - abstract protected function getShares(ShareAccessContext $accessContext, ?string $filterSourceTypeClass, ?string $filterSourceTypeValue, ?string $lastShareID, ?int $limit): array; + abstract protected function getShares( + ShareAccessContext $accessContext, ?string $filterSourceTypeClass, ?string $filterSourceTypeValue, ?string $lastShareID, ?int $limit, + ): array; protected IDBConnection $dbConnection; @@ -117,9 +123,14 @@ abstract protected function getShares(ShareAccessContext $accessContext, ?string protected IUser $user2; + protected TestShareSourceType1 $shareSourceType1; + + protected TestShareSourceType2 $shareSourceType2; + protected IFactory $l10nFactory; - private function parseTime(string $timestampMs): \DateTimeImmutable { + private function parseTime(mixed $timestampMs): \DateTimeImmutable { + $timestampMs = (int)$timestampMs; $time = \DateTimeImmutable::createFromFormat('U.u', number_format((float)$timestampMs / 1000.0, 3, '.', '')); if ($time === false) { throw new \RuntimeException('invalid timestamp: ' . $timestampMs); @@ -162,30 +173,37 @@ public function setUp(): void { $this->registry = Server::get(ISharingRegistry::class); $this->registry->clear(); - $this->registry->registerSourceType(new TestShareSourceType1(['source1' => 'Source 1'])); - $this->registry->registerSourceType(new TestShareSourceType2(['source2' => 'Source 2'])); - $this->registry->registerRecipientType(new TestShareRecipientType1( - [ - 'recipient1' => 'Recipient 1', - ], - [ - $this->user1->getUID() => ['recipient1'], - ], - [ - new ShareRecipient(TestShareRecipientType1::class, 'recipient1', null), - ], - )); - $this->registry->registerRecipientType(new TestShareRecipientType2( - [ - 'recipient2' => 'Recipient 2', - ], - [ - $this->user2->getUID() => ['recipient2'], - ], - [ - new ShareRecipient(TestShareRecipientType2::class, 'recipient2', null), - ], - )); + + $this->shareSourceType1 = new TestShareSourceType1(['source1' => 'Source 1']); + $this->registry->registerSourceType($this->shareSourceType1); + $this->shareSourceType2 = new TestShareSourceType2(['source2' => 'Source 2']); + $this->registry->registerSourceType($this->shareSourceType2); + $this->registry->registerRecipientType( + new TestShareRecipientType1( + [ + 'recipient1' => 'Recipient 1', + ], + [ + $this->user1->getUID() => ['recipient1'], + ], + [ + new ShareRecipient(TestShareRecipientType1::class, 'recipient1', null), + ], + ) + ); + $this->registry->registerRecipientType( + new TestShareRecipientType2( + [ + 'recipient2' => 'Recipient 2', + ], + [ + $this->user2->getUID() => ['recipient2'], + ], + [ + new ShareRecipient(TestShareRecipientType2::class, 'recipient2', null), + ], + ) + ); $this->registry->registerPropertyType(new TestSharePropertyType1(['valid1'])); $this->registry->markPropertyTypeCompatibleWithSourceType(TestSharePropertyType1::class, TestShareSourceType1::class); $this->registry->markPropertyTypeCompatibleWithRecipientType(TestSharePropertyType1::class, TestShareRecipientType1::class); @@ -251,32 +269,36 @@ private function reloadShare(ShareAccessContext $accessContext, Share $share): S public function testSearchRecipients(): void { $this->registry->clear(); - $this->registry->registerRecipientType(new TestShareRecipientType1( - [ - 'recipient1a' => 'Recipient 1A', - 'recipient1b' => 'Recipient 1B', - 'recipient1c' => 'Recipient 1C', - ], - [], - [ - new ShareRecipient(TestShareRecipientType1::class, 'recipient1a', null), - new ShareRecipient(TestShareRecipientType1::class, 'recipient1b', null), - new ShareRecipient(TestShareRecipientType1::class, 'recipient1c', null), - ], - )); - $this->registry->registerRecipientType(new TestShareRecipientType2( - [ - 'recipient2a' => 'Recipient 2A', - 'recipient2b' => 'Recipient 2B', - 'recipient2c' => 'Recipient 2C', - ], - [], - [ - new ShareRecipient(TestShareRecipientType2::class, 'recipient2a', null), - new ShareRecipient(TestShareRecipientType2::class, 'recipient2b', null), - new ShareRecipient(TestShareRecipientType2::class, 'recipient2c', null), - ], - )); + $this->registry->registerRecipientType( + new TestShareRecipientType1( + [ + 'recipient1a' => 'Recipient 1A', + 'recipient1b' => 'Recipient 1B', + 'recipient1c' => 'Recipient 1C', + ], + [], + [ + new ShareRecipient(TestShareRecipientType1::class, 'recipient1a', null), + new ShareRecipient(TestShareRecipientType1::class, 'recipient1b', null), + new ShareRecipient(TestShareRecipientType1::class, 'recipient1c', null), + ], + ) + ); + $this->registry->registerRecipientType( + new TestShareRecipientType2( + [ + 'recipient2a' => 'Recipient 2A', + 'recipient2b' => 'Recipient 2B', + 'recipient2c' => 'Recipient 2C', + ], + [], + [ + new ShareRecipient(TestShareRecipientType2::class, 'recipient2a', null), + new ShareRecipient(TestShareRecipientType2::class, 'recipient2b', null), + new ShareRecipient(TestShareRecipientType2::class, 'recipient2c', null), + ], + ) + ); $accessContext = new ShareAccessContext($this->owner); @@ -451,26 +473,30 @@ public function testSearchRecipients(): void { public function testSearchRecipientsUniqueDisplayNames(): void { $this->registry->clear(); - $this->registry->registerRecipientType(new TestShareRecipientType1( - [ - 'recipient1' => 'Recipient', - ], - [], - [ - new ShareRecipient(TestShareRecipientType1::class, 'recipient1', null), - ], - )); - $this->registry->registerRecipientType(new TestShareRecipientType2( - [ - 'recipient2' => 'Recipient', - 'recipient3' => 'Other', - ], - [], - [ - new ShareRecipient(TestShareRecipientType2::class, 'recipient2', null), - new ShareRecipient(TestShareRecipientType2::class, 'recipient3', null), - ], - )); + $this->registry->registerRecipientType( + new TestShareRecipientType1( + [ + 'recipient1' => 'Recipient', + ], + [], + [ + new ShareRecipient(TestShareRecipientType1::class, 'recipient1', null), + ], + ) + ); + $this->registry->registerRecipientType( + new TestShareRecipientType2( + [ + 'recipient2' => 'Recipient', + 'recipient3' => 'Other', + ], + [], + [ + new ShareRecipient(TestShareRecipientType2::class, 'recipient2', null), + new ShareRecipient(TestShareRecipientType2::class, 'recipient3', null), + ], + ) + ); $accessContext = new ShareAccessContext($this->owner); @@ -519,17 +545,19 @@ public function testSearchRecipientsUniqueDisplayNames(): void { public function testSearchRecipientsIcons(): void { $this->registry->clear(); - $this->registry->registerRecipientType(new TestShareRecipientType1( - [ - 'svg' => 'SVG', - 'url' => 'URL', - ], - [], - [ - new ShareRecipient(TestShareRecipientType1::class, 'svg', null), - new ShareRecipient(TestShareRecipientType1::class, 'url', null), - ], - )); + $this->registry->registerRecipientType( + new TestShareRecipientType1( + [ + 'svg' => 'SVG', + 'url' => 'URL', + ], + [], + [ + new ShareRecipient(TestShareRecipientType1::class, 'svg', null), + new ShareRecipient(TestShareRecipientType1::class, 'url', null), + ], + ) + ); $accessContext = new ShareAccessContext($this->owner); @@ -945,7 +973,9 @@ public function testAddChildShareRecipientWithResharePermission(): void { $this->dbConnection->commit(); $before = $this->manager->getTime(); - $formatted = $this->addShareRecipient(new ShareAccessContext($this->user1), $share, new ShareRecipient(TestShareRecipientType2::class, 'recipient2', null)); + $formatted = $this->addShareRecipient( + new ShareAccessContext($this->user1), $share, new ShareRecipient(TestShareRecipientType2::class, 'recipient2', null) + ); $after = $this->manager->getTime(); $this->assertDateBetween($before, $after, $this->parseTime($formatted['last_updated'])); $this->assertEquals([ @@ -1103,7 +1133,9 @@ public function testRemoveChildShareRecipientWithoutResharePermission(): void { $share = $this->manager->updateSharePermission($accessContext, $share, new SharePermission(TestSharePermissionType1::class, true)); $share = $this->manager->updateSharePermission($accessContext, $share, new SharePermission(ReshareSharePermissionType::class, true)); $share = $this->manager->updateShareState($accessContext, $share, ShareState::Active); - $share = $this->manager->addShareRecipient(new ShareAccessContext($this->user1), $share, new ShareRecipient(TestShareRecipientType2::class, 'recipient2', null)); + $share = $this->manager->addShareRecipient( + new ShareAccessContext($this->user1), $share, new ShareRecipient(TestShareRecipientType2::class, 'recipient2', null) + ); $share = $this->manager->updateSharePermission($accessContext, $share, new SharePermission(ReshareSharePermissionType::class, false)); $this->dbConnection->commit(); @@ -1127,12 +1159,16 @@ public function testRemoveChildShareRecipientWithResharePermission(): void { $share = $this->manager->getShare($accessContext, $share->id); $share = $this->manager->updateSharePermission($accessContext, $share, new SharePermission(ReshareSharePermissionType::class, true)); $share = $this->manager->updateShareState($accessContext, $share, ShareState::Active); - $share = $this->manager->addShareRecipient(new ShareAccessContext($this->user1), $share, new ShareRecipient(TestShareRecipientType2::class, 'recipient2', null)); + $share = $this->manager->addShareRecipient( + new ShareAccessContext($this->user1), $share, new ShareRecipient(TestShareRecipientType2::class, 'recipient2', null) + ); $this->dbConnection->commit(); $before = $this->manager->getTime(); - $formatted = $this->removeShareRecipient(new ShareAccessContext($this->user1), $share, new ShareRecipient(TestShareRecipientType2::class, 'recipient2', null)); + $formatted = $this->removeShareRecipient( + new ShareAccessContext($this->user1), $share, new ShareRecipient(TestShareRecipientType2::class, 'recipient2', null) + ); $after = $this->manager->getTime(); $this->assertDateBetween($before, $after, $this->parseTime($formatted['last_updated'])); $this->assertEquals([ @@ -1216,7 +1252,9 @@ public function testRemoveParentShareRecipientWithoutResharePermission(): void { $share = $this->manager->updateSharePermission($accessContext, $share, new SharePermission(TestSharePermissionType1::class, true)); $share = $this->manager->updateSharePermission($accessContext, $share, new SharePermission(ReshareSharePermissionType::class, true)); $share = $this->manager->updateShareState($accessContext, $share, ShareState::Active); - $share = $this->manager->addShareRecipient(new ShareAccessContext($this->user1), $share, new ShareRecipient(TestShareRecipientType2::class, 'recipient2', null)); + $share = $this->manager->addShareRecipient( + new ShareAccessContext($this->user1), $share, new ShareRecipient(TestShareRecipientType2::class, 'recipient2', null) + ); $share = $this->manager->updateSharePermission($accessContext, $share, new SharePermission(ReshareSharePermissionType::class, false)); $this->dbConnection->commit(); @@ -1239,7 +1277,9 @@ public function testRemoveParentShareRecipientWithResharePermission(): void { $share = $this->manager->getShare($accessContext, $share->id); $share = $this->manager->updateSharePermission($accessContext, $share, new SharePermission(ReshareSharePermissionType::class, true)); $share = $this->manager->updateShareState($accessContext, $share, ShareState::Active); - $share = $this->manager->addShareRecipient(new ShareAccessContext($this->user1), $share, new ShareRecipient(TestShareRecipientType2::class, 'recipient2', null)); + $share = $this->manager->addShareRecipient( + new ShareAccessContext($this->user1), $share, new ShareRecipient(TestShareRecipientType2::class, 'recipient2', null) + ); $this->dbConnection->commit(); @@ -1263,14 +1303,16 @@ public static function dataUpdateShareRecipientSecret(): array { #[DataProvider('dataUpdateShareRecipientSecret')] public function testUpdateShareRecipientSecret(bool $isSecretUpdatable): void { - $this->registry->registerRecipientType(new TestShareRecipientTypePublicSecret( - [ - 'recipient1' => 'Recipient 1', - ], - [], - true, - $isSecretUpdatable, - )); + $this->registry->registerRecipientType( + new TestShareRecipientTypePublicSecret( + [ + 'recipient1' => 'Recipient 1', + ], + [], + true, + $isSecretUpdatable, + ) + ); $accessContext = new ShareAccessContext($this->owner); @@ -1482,7 +1524,9 @@ public function testUpdateSharePropertyRequired(): void { } public function testUpdateSharePropertyModifyProperties(): void { - $this->registry->registerPropertyType(new TestSharePropertyTypeModifyValue(['old-value', 'modify-on-save-old-value', 'modify-on-save', 'modify-on-load'])); + $this->registry->registerPropertyType( + new TestSharePropertyTypeModifyValue(['old-value', 'modify-on-save-old-value', 'modify-on-save', 'modify-on-load']) + ); $this->registry->markPropertyTypeCompatibleWithSourceType(TestSharePropertyTypeModifyValue::class, TestShareSourceType1::class); $this->registry->markPropertyTypeCompatibleWithRecipientType(TestSharePropertyTypeModifyValue::class, TestShareRecipientType1::class); @@ -1741,7 +1785,9 @@ public function testUpdateSharePermission(): void { public function testUpdateSharePermissionInteractionRestricted(): void { $listener = function (RestrictInteractionEvent $event): void { - if ($event->action instanceof ShareAction && $event->action->unifiedSharingPermissions !== null && in_array(TestSharePermissionType1::class, $event->action->unifiedSharingPermissions, true)) { + if ($event->action instanceof ShareAction && $event->action->unifiedSharingPermissions !== null && in_array( + TestSharePermissionType1::class, $event->action->unifiedSharingPermissions, true + )) { throw new InteractionRestrictedException('Permission not allowed.', 'You are not allowed to enable this permission.'); } }; @@ -2344,7 +2390,8 @@ public function testGetShareAsRecipientWithArguments(): void { $this->dbConnection->commit(); $after = $this->manager->getTime(); - $formatted = $this->getShare(new ShareAccessContext(currentUser: $this->user1, arguments: [TestShareRecipientTypeArguments::class => 'secret']), $share->id); + $formatted = $this->getShare(new ShareAccessContext(currentUser: $this->user1, arguments: [TestShareRecipientTypeArguments::class => 'secret']), + $share->id); $this->assertDateBetween($before, $after, $this->parseTime($formatted['last_updated'])); unset($formatted['last_updated']); $this->assertEquals([ @@ -2910,7 +2957,8 @@ public function testGetShareAsRecipientFilteredArguments(): void { 'permission_preset' => TestSharePermissionPreset1::class, ], $formatted); - $formatted = $this->getShare(new ShareAccessContext(currentUser: $this->owner, arguments: [TestSharePropertyTypeFilter::class => 'filtered']), $share->id); + $formatted = $this->getShare(new ShareAccessContext(currentUser: $this->owner, arguments: [TestSharePropertyTypeFilter::class => 'filtered']), + $share->id); $this->assertDateBetween($before, $after, $this->parseTime($formatted['last_updated'])); unset($formatted['last_updated']); $this->assertEquals([ @@ -3026,21 +3074,25 @@ public static function dataGetShareWithPublicSecret(): array { #[DataProvider('dataGetShareWithPublicSecret')] public function testGetShareWithPublicSecret(bool $isSecretPublic): void { $this->registry->clear(); - $this->registry->registerRecipientType(new TestShareRecipientType1( - [ - 'recipient1' => 'Recipient 1', - ], - [], - [], - )); - $this->registry->registerRecipientType(new TestShareRecipientTypePublicSecret( - [ - 'recipient2' => 'Recipient 2', - ], - [], - $isSecretPublic, - false, - )); + $this->registry->registerRecipientType( + new TestShareRecipientType1( + [ + 'recipient1' => 'Recipient 1', + ], + [], + [], + ) + ); + $this->registry->registerRecipientType( + new TestShareRecipientTypePublicSecret( + [ + 'recipient2' => 'Recipient 2', + ], + [], + $isSecretPublic, + false, + ) + ); $accessContext = new ShareAccessContext($this->owner); @@ -3104,20 +3156,22 @@ public function testGetShareWithPublicSecret(bool $isSecretPublic): void { public function testGetShareWithSecret(): void { $this->registry->clear(); $this->registry->registerSourceType(new TestShareSourceType1(['source1' => 'Source'])); - $this->registry->registerRecipientType(new TestShareRecipientTypePublicSecret( - [ - 'recipient1' => 'Recipient 1', - 'recipient2' => 'Recipient 2', - 'recipient3' => 'Recipient 3', - 'recipient4' => 'Recipient 4', - ], - [ - $this->user1->getUID() => ['recipient1'], - $this->user2->getUID() => ['recipient2'], - ], - true, - false, - )); + $this->registry->registerRecipientType( + new TestShareRecipientTypePublicSecret( + [ + 'recipient1' => 'Recipient 1', + 'recipient2' => 'Recipient 2', + 'recipient3' => 'Recipient 3', + 'recipient4' => 'Recipient 4', + ], + [ + $this->user1->getUID() => ['recipient1'], + $this->user2->getUID() => ['recipient2'], + ], + true, + false, + ) + ); $this->registry->registerPermissionType(null, Server::get(ReshareSharePermissionType::class)); $accessContext = new ShareAccessContext($this->owner); @@ -3130,9 +3184,15 @@ public function testGetShareWithSecret(): void { $share = $this->manager->updateSharePermission($accessContext, $share, new SharePermission(ReshareSharePermissionType::class, true)); $share = $this->manager->updateShareState($accessContext, $share, ShareState::Active); - $share = $this->manager->addShareRecipient(new ShareAccessContext($this->user1), $share, new ShareRecipient(TestShareRecipientTypePublicSecret::class, 'recipient2', null)); - $share = $this->manager->addShareRecipient(new ShareAccessContext($this->user1), $share, new ShareRecipient(TestShareRecipientTypePublicSecret::class, 'recipient3', null)); - $share = $this->manager->addShareRecipient(new ShareAccessContext($this->user2), $share, new ShareRecipient(TestShareRecipientTypePublicSecret::class, 'recipient4', null)); + $share = $this->manager->addShareRecipient( + new ShareAccessContext($this->user1), $share, new ShareRecipient(TestShareRecipientTypePublicSecret::class, 'recipient2', null) + ); + $share = $this->manager->addShareRecipient( + new ShareAccessContext($this->user1), $share, new ShareRecipient(TestShareRecipientTypePublicSecret::class, 'recipient3', null) + ); + $share = $this->manager->addShareRecipient( + new ShareAccessContext($this->user2), $share, new ShareRecipient(TestShareRecipientTypePublicSecret::class, 'recipient4', null) + ); $this->dbConnection->commit(); $after = $this->manager->getTime(); @@ -3344,7 +3404,9 @@ public function testGetShareDisabledInitiator(): void { $share = $this->manager->addShareRecipient($accessContext, $share, new ShareRecipient(TestShareRecipientType1::class, 'recipient1', null)); $share = $this->manager->updateSharePermission($accessContext, $share, new SharePermission(ReshareSharePermissionType::class, true)); $share = $this->manager->updateShareState($accessContext, $share, ShareState::Active); - $share = $this->manager->addShareRecipient(new ShareAccessContext(currentUser: $this->user1), $share, new ShareRecipient(TestShareRecipientType2::class, 'recipient2', null)); + $share = $this->manager->addShareRecipient( + new ShareAccessContext(currentUser: $this->user1), $share, new ShareRecipient(TestShareRecipientType2::class, 'recipient2', null) + ); $this->dbConnection->commit(); $after = $this->manager->getTime(); @@ -4010,7 +4072,9 @@ public function testInitiatorDeleted(): void { $share = $this->manager->addShareRecipient($accessContext, $share, new ShareRecipient(TestShareRecipientType1::class, 'recipient1', null)); $share = $this->manager->updateSharePermission($accessContext, $share, new SharePermission(ReshareSharePermissionType::class, true)); $share = $this->manager->updateShareState($accessContext, $share, ShareState::Active); - $share = $this->manager->addShareRecipient(new ShareAccessContext(currentUser: $this->user1), $share, new ShareRecipient(TestShareRecipientType2::class, 'recipient2', null)); + $share = $this->manager->addShareRecipient( + new ShareAccessContext(currentUser: $this->user1), $share, new ShareRecipient(TestShareRecipientType2::class, 'recipient2', null) + ); $before = $this->manager->getTime(); $this->user1->delete(); @@ -4064,4 +4128,107 @@ public function testInitiatorDeleted(): void { ], ], $formatted['recipients']); } + + public function testGetWithDirectAccess(): void { + $accessContext = new ShareAccessContext($this->owner); + $accessContext2 = new ShareAccessContext(currentUser: $this->user2); + + $before = $this->manager->getTime(); + $this->dbConnection->beginTransaction(); + $share = $this->manager->createShare($accessContext); + $share = $this->manager->addShareSource($accessContext, $share, new ShareSource(TestShareSourceType1::class, 'source1')); + $share = $this->manager->addShareRecipient($accessContext, $share, new ShareRecipient(TestShareRecipientType1::class, 'recipient1', null)); + + $share = $this->manager->updateSharePermission($accessContext, $share, new SharePermission(TestSharePermissionType1::class, true)); + $share = $this->manager->updateShareState($accessContext, $share, ShareState::Active); + + $this->shareSourceType1->userAccess[$this->owner->getUID()] = ['source1']; + + $this->dbConnection->commit(); + + $after = $this->manager->getTime(); + + // user2 has no direct access, no shares + $formattedShares = $this->getShares($accessContext2, TestShareSourceType1::class, 'source1', null, null); + $this->assertCount(0, $formattedShares); + + try { + $this->getShare($accessContext2, $share->id); + $this->fail('user has invalid share access'); + } catch (ShareNotFoundException) { + + } + + // give user2 direct access, can see shares + $this->shareSourceType1->userAccess[$this->user2->getUID()] = ['source1']; + $formattedShares = $this->getShares($accessContext2, TestShareSourceType1::class, 'source1', null, null); + + $this->assertCount(1, $formattedShares); + $formatted = $formattedShares[0]; + + $this->assertDateBetween($before, $after, $this->parseTime($formatted['last_updated'])); + $this->assertEquals([ + 'user_id' => 'owner', + 'instance' => null, + 'display_name' => 'Owner', + 'icon' => [ + 'light' => 'http://localhost/index.php/avatar/owner/64', + 'dark' => 'http://localhost/index.php/avatar/owner/64/dark', + ], + ], $formatted['owner']); + + $singleFormatted = $this->getShare($accessContext2, $share->id); + $this->assertEquals($singleFormatted, $formatted); + + // add a source that user2 doesn't have access to, can't see share anymore + $this->dbConnection->beginTransaction(); + $this->manager->addShareSource($accessContext, $share, new ShareSource(TestShareSourceType2::class, 'source2')); + $this->dbConnection->commit(); + $formattedShares = $this->getShares(new ShareAccessContext(currentUser: $this->user2), TestShareSourceType1::class, 'source1', null, null); + $this->assertCount(0, $formattedShares); + } + + public function testEditWithDirectAccess(): void { + $accessContext = new ShareAccessContext($this->owner); + $accessContext2 = new ShareAccessContext(currentUser: $this->user2); + + $this->dbConnection->beginTransaction(); + $share = $this->manager->createShare($accessContext); + $share = $this->manager->addShareSource($accessContext, $share, new ShareSource(TestShareSourceType1::class, 'source1')); + $share = $this->manager->addShareRecipient($accessContext, $share, new ShareRecipient(TestShareRecipientType1::class, 'recipient1', null)); + + $share = $this->manager->updateSharePermission($accessContext, $share, new SharePermission(TestSharePermissionType1::class, true)); + $share = $this->manager->updateShareState($accessContext, $share, ShareState::Active); + + $this->shareSourceType1->userAccess[$this->owner->getUID()] = ['source1']; + + $this->dbConnection->commit(); + + try { + $this->updateShareProperty($accessContext2, $share, new ShareProperty(TestSharePropertyType1::class, 'valid1')); + $this->fail('update allowed'); + } catch (ShareOperationForbiddenException) { + + } + + // give user2 direct access + $this->shareSourceType1->userAccess[$this->user2->getUID()] = ['source1']; + $user2Share = $this->reloadShare($accessContext2, $share); + $this->assertNull($user2Share->recipients[0]->secret); + $formatted = $this->updateShareProperty($accessContext2, $user2Share, new ShareProperty(TestSharePropertyType1::class, 'valid1')); + + $this->assertEquals([ + [ + 'display_name' => 'TestSharePropertyType1', + 'hint' => 'hint TestSharePropertyType1', + 'priority' => 1, + 'required' => false, + 'advanced' => false, + 'value' => 'valid1', + 'class' => \Test\Sharing\TestSharePropertyType1::class, + 'type' => 'enum', + 'valid_values' => ['valid1'], + ] + ], $formatted['properties']); + } } diff --git a/tests/lib/Sharing/TestShareSourceType1.php b/tests/lib/Sharing/TestShareSourceType1.php index 10678bc298d0d..a189335f32913 100644 --- a/tests/lib/Sharing/TestShareSourceType1.php +++ b/tests/lib/Sharing/TestShareSourceType1.php @@ -21,6 +21,8 @@ class TestShareSourceType1 implements IShareSourceType { public function __construct( /** @var array $validSources */ private readonly array $validSources, + /** @var array $validSources */ + public array $userAccess = [], ) { } @@ -59,4 +61,10 @@ public function getSourcesMetadata(array $sources): array { public function getSourceInteractionResource(IUser $user, string $source): InteractionResource { return new TestInteractionResource($source); } + + #[\Override] + public function userHasDirectSharingAccessToSource(IUser $user, string $source): bool { + $userSources = $this->userAccess[$user->getUID()] ?? []; + return in_array($source, $userSources); + } }