diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 6511135a510..bca92e095ed 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -2095,14 +2095,15 @@ class ShareAPIController extends OCSController { $canDownload = false; $hideDownload = true; + $userExplicitlySetHideDownload = $share->getHideDownload(); // Capture user's explicit choice $userFolder = $this->rootFolder->getUserFolder($share->getSharedBy()); $nodes = $userFolder->getById($share->getNodeId()); foreach ($nodes as $node) { - // Owner always can download it - so allow it and break + // Owner always can download it - so allow it, but respect their explicit choice about hiding downloads if ($node->getOwner()?->getUID() === $share->getSharedBy()) { $canDownload = true; - $hideDownload = false; + $hideDownload = $userExplicitlySetHideDownload; break; } @@ -2120,23 +2121,44 @@ class ShareAPIController extends OCSController { /** @var SharedStorage $storage */ $originalShare = $storage->getShare(); $inheritedAttributes = $originalShare->getAttributes(); - // hide if hidden and also the current share enforces hide (can only be false if one share is false or user is owner) - $hideDownload = $hideDownload && $originalShare->getHideDownload(); - // allow download if already allowed by previous share or when the current share allows downloading - $canDownload = $canDownload || $inheritedAttributes === null || $inheritedAttributes->getAttribute('permissions', 'download') !== false; + + // For federated shares: users can only be MORE restrictive, never LESS restrictive + // If parent has hideDownload=true, child MUST have hideDownload=true + $parentHidesDownload = $originalShare->getHideDownload(); + + // Check if download permission is available from parent + $parentAllowsDownload = $inheritedAttributes === null || $inheritedAttributes->getAttribute('permissions', 'download') !== false; + + // Apply inheritance rules: + // 1. If parent hides download, child must hide download + // 2. If parent allows download, child can choose to hide or allow + // 3. If parent forbids download, child cannot allow download + $hideDownload = $parentHidesDownload || $userExplicitlySetHideDownload; + + $canDownload = $canDownload || $parentAllowsDownload; + } elseif ($node->getStorage()->instanceOfStorage(Storage::class)) { $canDownload = true; // in case of federation storage, we can expect the download to be activated by default + // For external federation storage, respect user's choice if downloads are available + $hideDownload = $userExplicitlySetHideDownload; } } - if ($hideDownload || !$canDownload) { + // Apply the final restrictions: + // 1. If parent doesn't allow downloads at all, force hide and disable download attribute + // 2. If parent allows downloads, respect user's hideDownload choice + if (!$canDownload) { + // Parent completely forbids downloads - must enforce this restriction $share->setHideDownload(true); - - if (!$canDownload) { - $attributes = $share->getAttributes() ?? $share->newAttributes(); - $attributes->setAttribute('permissions', 'download', false); - $share->setAttributes($attributes); - } + $attributes = $share->getAttributes() ?? $share->newAttributes(); + $attributes->setAttribute('permissions', 'download', false); + $share->setAttributes($attributes); + } elseif ($hideDownload) { + // Either parent forces hide, or user chooses to hide - respect this + $share->setHideDownload(true); + } else { + // User explicitly wants to allow downloads and parent permits it + $share->setHideDownload(false); } } diff --git a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php index 4ced55ab7c9..1030285463b 100644 --- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php @@ -6,8 +6,11 @@ */ namespace OCA\Files_Sharing\Tests\Controller; +use OC\Files\Storage\Wrapper\Wrapper; use OCA\Federation\TrustedServers; use OCA\Files_Sharing\Controller\ShareAPIController; +use OCA\Files_Sharing\External\Storage; +use OCA\Files_Sharing\SharedStorage; use OCP\App\IAppManager; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\OCS\OCSBadRequestException; @@ -5299,4 +5302,217 @@ class ShareAPIControllerTest extends TestCase { $this->assertTrue($result['is_trusted_server']); } + + public function testOwnerCanAlwaysDownload(): void { + $ocs = $this->mockFormatShare(); + + $share = $this->createMock(IShare::class); + $node = $this->createMock(File::class); + $userFolder = $this->createMock(Folder::class); + $owner = $this->createMock(IUser::class); + + $share->method('getSharedBy')->willReturn('sharedByUser'); + $share->method('getNodeId')->willReturn(42); + $node->method('getOwner')->willReturn($owner); + $owner->method('getUID')->willReturn('sharedByUser'); + + $userFolder->method('getById')->with(42)->willReturn([$node]); + $this->rootFolder->method('getUserFolder')->with('sharedByUser')->willReturn($userFolder); + + // Expect hideDownload to be set to false since owner can always download + $share->expects($this->once())->method('setHideDownload')->with(false); + + $this->invokePrivate($ocs, 'checkInheritedAttributes', [$share]); + } + + public function testParentHideDownloadEnforcedOnChild(): void { + $ocs = $this->mockFormatShare(); + + $share = $this->createMock(IShare::class); + $node = $this->createMock(File::class); + $userFolder = $this->createMock(Folder::class); + $owner = $this->createMock(IUser::class); + $storage = $this->createMock(SharedStorage::class); + $originalShare = $this->createMock(IShare::class); + + $share->method('getSharedBy')->willReturn('sharedByUser'); + $share->method('getNodeId')->willReturn(42); + $share->method('getHideDownload')->willReturn(false); // User wants to allow downloads + $node->method('getOwner')->willReturn($owner); + $owner->method('getUID')->willReturn('differentOwner'); + $node->method('getStorage')->willReturn($storage); + $storage->method('instanceOfStorage')->with(SharedStorage::class)->willReturn(true); + $storage->method('getInstanceOfStorage')->with(SharedStorage::class)->willReturn($storage); + $storage->method('getShare')->willReturn($originalShare); + $originalShare->method('getHideDownload')->willReturn(true); // Parent hides download + $originalShare->method('getAttributes')->willReturn(null); + + $userFolder->method('getById')->with(42)->willReturn([$node]); + $this->rootFolder->method('getUserFolder')->with('sharedByUser')->willReturn($userFolder); + + // Should be forced to hide download due to parent restriction + $share->expects($this->once())->method('setHideDownload')->with(true); + + $this->invokePrivate($ocs, 'checkInheritedAttributes', [$share]); + } + + public function testUserCanHideWhenParentAllows(): void { + $ocs = $this->mockFormatShare(); + + $share = $this->createMock(IShare::class); + $node = $this->createMock(File::class); + $userFolder = $this->createMock(Folder::class); + $owner = $this->createMock(IUser::class); + $storage = $this->createMock(SharedStorage::class); + $originalShare = $this->createMock(IShare::class); + + $share->method('getSharedBy')->willReturn('sharedByUser'); + $share->method('getNodeId')->willReturn(42); + $share->method('getHideDownload')->willReturn(true); // User chooses to hide downloads + $node->method('getOwner')->willReturn($owner); + $owner->method('getUID')->willReturn('differentOwner'); + $node->method('getStorage')->willReturn($storage); + $storage->method('instanceOfStorage')->with(SharedStorage::class)->willReturn(true); + $storage->method('getInstanceOfStorage')->with(SharedStorage::class)->willReturn($storage); + $storage->method('getShare')->willReturn($originalShare); + $originalShare->method('getHideDownload')->willReturn(false); // Parent allows download + $originalShare->method('getAttributes')->willReturn(null); + + $userFolder->method('getById')->with(42)->willReturn([$node]); + $this->rootFolder->method('getUserFolder')->with('sharedByUser')->willReturn($userFolder); + + // Should respect user's choice to hide downloads + $share->expects($this->once())->method('setHideDownload')->with(true); + + $this->invokePrivate($ocs, 'checkInheritedAttributes', [$share]); + } + + public function testParentDownloadAttributeInherited(): void { + $ocs = $this->mockFormatShare(); + + $share = $this->createMock(IShare::class); + $node = $this->createMock(File::class); + $userFolder = $this->createMock(Folder::class); + $owner = $this->createMock(IUser::class); + $storage = $this->createMock(SharedStorage::class); + $originalShare = $this->createMock(IShare::class); + $attributes = $this->createMock(\OCP\Share\IAttributes::class); + $shareAttributes = $this->createMock(\OCP\Share\IAttributes::class); + + $share->method('getSharedBy')->willReturn('sharedByUser'); + $share->method('getNodeId')->willReturn(42); + $share->method('getHideDownload')->willReturn(false); // User wants to allow downloads + $share->method('getAttributes')->willReturn($shareAttributes); + $share->method('newAttributes')->willReturn($shareAttributes); + $node->method('getOwner')->willReturn($owner); + $owner->method('getUID')->willReturn('differentOwner'); + $node->method('getStorage')->willReturn($storage); + $storage->method('instanceOfStorage')->with(SharedStorage::class)->willReturn(true); + $storage->method('getInstanceOfStorage')->with(SharedStorage::class)->willReturn($storage); + $storage->method('getShare')->willReturn($originalShare); + $originalShare->method('getHideDownload')->willReturn(false); + $originalShare->method('getAttributes')->willReturn($attributes); + $attributes->method('getAttribute')->with('permissions', 'download')->willReturn(false); // Parent forbids download + + $userFolder->method('getById')->with(42)->willReturn([$node]); + $this->rootFolder->method('getUserFolder')->with('sharedByUser')->willReturn($userFolder); + + // Should be forced to hide download and set download attribute to false + $share->expects($this->once())->method('setHideDownload')->with(true); + $shareAttributes->expects($this->once())->method('setAttribute')->with('permissions', 'download', false); + $share->expects($this->once())->method('setAttributes')->with($shareAttributes); + + $this->invokePrivate($ocs, 'checkInheritedAttributes', [$share]); + } + + public function testFederatedStorageRespectsUserChoice(): void { + $ocs = $this->mockFormatShare(); + + $share = $this->createMock(IShare::class); + $node = $this->createMock(File::class); + $userFolder = $this->createMock(Folder::class); + $owner = $this->createMock(IUser::class); + $storage = $this->createMock(\OCA\Files_Sharing\External\Storage::class); + + $share->method('getSharedBy')->willReturn('sharedByUser'); + $share->method('getNodeId')->willReturn(42); + $share->method('getHideDownload')->willReturn(true); // User chooses to hide downloads + $node->method('getOwner')->willReturn($owner); + $owner->method('getUID')->willReturn('differentOwner'); + $node->method('getStorage')->willReturn($storage); + $storage->method('instanceOfStorage')->willReturnMap([ + [SharedStorage::class, false], + [\OCA\Files_Sharing\External\Storage::class, true] + ]); + + $userFolder->method('getById')->with(42)->willReturn([$node]); + $this->rootFolder->method('getUserFolder')->with('sharedByUser')->willReturn($userFolder); + + // For federated storage, should respect user's choice + $share->expects($this->once())->method('setHideDownload')->with(true); + + $this->invokePrivate($ocs, 'checkInheritedAttributes', [$share]); + } + + public function testUserAllowsDownloadWhenParentPermits(): void { + $ocs = $this->mockFormatShare(); + + $share = $this->createMock(IShare::class); + $node = $this->createMock(File::class); + $userFolder = $this->createMock(Folder::class); + $owner = $this->createMock(IUser::class); + $storage = $this->createMock(SharedStorage::class); + $originalShare = $this->createMock(IShare::class); + + $share->method('getSharedBy')->willReturn('sharedByUser'); + $share->method('getNodeId')->willReturn(42); + $share->method('getHideDownload')->willReturn(false); // User wants to allow downloads + $node->method('getOwner')->willReturn($owner); + $owner->method('getUID')->willReturn('differentOwner'); + $node->method('getStorage')->willReturn($storage); + $storage->method('instanceOfStorage')->with(SharedStorage::class)->willReturn(true); + $storage->method('getInstanceOfStorage')->with(SharedStorage::class)->willReturn($storage); + $storage->method('getShare')->willReturn($originalShare); + $originalShare->method('getHideDownload')->willReturn(false); // Parent allows download + $originalShare->method('getAttributes')->willReturn(null); + + $userFolder->method('getById')->with(42)->willReturn([$node]); + $this->rootFolder->method('getUserFolder')->with('sharedByUser')->willReturn($userFolder); + + // Should allow downloads as both user and parent permit it + $share->expects($this->once())->method('setHideDownload')->with(false); + + $this->invokePrivate($ocs, 'checkInheritedAttributes', [$share]); + } + + public function testWrapperStorageUnwrapped(): void { + $ocs = $this->mockFormatShare(); + + $share = $this->createMock(IShare::class); + $node = $this->createMock(File::class); + $userFolder = $this->createMock(Folder::class); + $owner = $this->createMock(IUser::class); + $wrapperStorage = $this->createMock(Wrapper::class); + $innerStorage = $this->createMock(SharedStorage::class); + $originalShare = $this->createMock(IShare::class); + + $share->method('getSharedBy')->willReturn('sharedByUser'); + $share->method('getNodeId')->willReturn(42); + $share->method('getHideDownload')->willReturn(false); + $node->method('getOwner')->willReturn($owner); + $owner->method('getUID')->willReturn('differentOwner'); + $node->method('getStorage')->willReturn($wrapperStorage); + $wrapperStorage->method('instanceOfStorage')->with(SharedStorage::class)->willReturn(true); + $wrapperStorage->method('getInstanceOfStorage')->with(SharedStorage::class)->willReturn($innerStorage); + $innerStorage->method('getShare')->willReturn($originalShare); + $originalShare->method('getHideDownload')->willReturn(false); + $originalShare->method('getAttributes')->willReturn(null); + + $userFolder->method('getById')->with(42)->willReturn([$node]); + $this->rootFolder->method('getUserFolder')->with('sharedByUser')->willReturn($userFolder); + + $share->expects($this->once())->method('setHideDownload')->with(false); + + $this->invokePrivate($ocs, 'checkInheritedAttributes', [$share]); + } }