diff --git a/apps/dav/lib/SystemTag/SystemTagPlugin.php b/apps/dav/lib/SystemTag/SystemTagPlugin.php index 9c00ba26d300c..07c422b6546f3 100644 --- a/apps/dav/lib/SystemTag/SystemTagPlugin.php +++ b/apps/dav/lib/SystemTag/SystemTagPlugin.php @@ -416,12 +416,12 @@ public function handleUpdateProperties($path, PropPatch $propPatch) { return false; } - if (isset($props[self::OBJECTIDS_PROPERTYNAME])) { - $user = $this->userSession->getUser(); - if (!$user) { - throw new Forbidden('You don’t have permissions to update tags'); - } + $user = $this->userSession->getUser(); + if (!$user) { + throw new Forbidden('You don’t have permissions to update tags'); + } + if (isset($props[self::OBJECTIDS_PROPERTYNAME])) { $propValue = $props[self::OBJECTIDS_PROPERTYNAME]; if (!$propValue instanceof SystemTagsObjectList || count($propValue->getObjects()) === 0) { throw new BadRequest('Invalid object-ids property'); @@ -440,30 +440,11 @@ public function handleUpdateProperties($path, PropPatch $propPatch) { throw new BadRequest('Invalid object-ids property type. Only files are supported'); } - // Get all current tagged objects - $taggedObjects = $this->tagMapper->getObjectIdsForTags([$node->getSystemTag()->getId()], 'files'); - $toAddObjects = array_map(fn ($value) => (string)$value, array_keys($objects)); - - // Compute the tags to add and remove - $addedObjects = array_values(array_diff($toAddObjects, $taggedObjects)); - $removedObjects = array_values(array_diff($taggedObjects, $toAddObjects)); - - // Check permissions for each object to be freshly tagged or untagged - if (!$this->canUpdateTagForFileIds(array_merge($addedObjects, $removedObjects))) { - throw new Forbidden('You don’t have permissions to update tags'); - } - - $this->tagMapper->setObjectIdsForTag($node->getSystemTag()->getId(), $node->getName(), array_keys($objects)); + $this->setVisibleObjectIdsForTag($user, $node, array_map(fn ($value) => (string)$value, array_keys($objects))); } if ($props[self::OBJECTIDS_PROPERTYNAME] === null) { - // Check the user have permissions to remove the tag from all currently tagged objects - $taggedObjects = $this->tagMapper->getObjectIdsForTags([$node->getSystemTag()->getId()], 'files'); - if (!$this->canUpdateTagForFileIds($taggedObjects)) { - throw new Forbidden('You don’t have permissions to update tags'); - } - - $this->tagMapper->setObjectIdsForTag($node->getSystemTag()->getId(), $node->getName(), []); + $this->setVisibleObjectIdsForTag($user, $node, []); } return true; @@ -544,6 +525,31 @@ public function handleUpdateProperties($path, PropPatch $propPatch) { }); } + /** + * Files the user cannot see keep the tag, as they are never listed to them + * + * @param list $objectIds + * @throws Forbidden + */ + private function setVisibleObjectIdsForTag(IUser $user, SystemTagObjectType $node, array $objectIds): void { + $tagId = $node->getSystemTag()->getId(); + $taggedObjects = $this->tagMapper->getObjectIdsForTags([$tagId], 'files'); + + $userFolder = $this->rootFolder->getUserFolder($user->getUID()); + $hiddenObjects = array_filter( + array_diff($taggedObjects, $objectIds), + fn (string $objectId): bool => $userFolder->getFirstNodeById((int)$objectId) === null, + ); + + $addedObjects = array_diff($objectIds, $taggedObjects); + $removedObjects = array_diff($taggedObjects, $objectIds, $hiddenObjects); + if (!$this->canUpdateTagForFileIds(array_merge($addedObjects, $removedObjects))) { + throw new Forbidden('You don’t have permissions to update tags'); + } + + $this->tagMapper->setObjectIdsForTag($tagId, $node->getName(), array_merge($objectIds, $hiddenObjects)); + } + /** * Check if the user can update the tag for the given file ids * diff --git a/apps/dav/tests/unit/SystemTag/SystemTagPluginTest.php b/apps/dav/tests/unit/SystemTag/SystemTagPluginTest.php index ae15fa6166bcb..c93cb88499b2e 100644 --- a/apps/dav/tests/unit/SystemTag/SystemTagPluginTest.php +++ b/apps/dav/tests/unit/SystemTag/SystemTagPluginTest.php @@ -11,10 +11,14 @@ use OC\SystemTag\SystemTag; use OCA\DAV\SystemTag\SystemTagNode; +use OCA\DAV\SystemTag\SystemTagObjectType; use OCA\DAV\SystemTag\SystemTagPlugin; use OCA\DAV\SystemTag\SystemTagsByIdCollection; +use OCA\DAV\SystemTag\SystemTagsObjectList; use OCA\DAV\SystemTag\SystemTagsObjectMappingCollection; +use OCP\Constants; use OCP\Files\IRootFolder; +use OCP\Files\IUserFolder; use OCP\IGroupManager; use OCP\IUser; use OCP\IUserSession; @@ -23,6 +27,7 @@ use OCP\SystemTag\ISystemTagObjectMapper; use OCP\SystemTag\TagAlreadyExistsException; use PHPUnit\Framework\MockObject\MockObject; +use Sabre\DAV\PropPatch; use Sabre\DAV\Tree; use Sabre\HTTP\RequestInterface; use Sabre\HTTP\ResponseInterface; @@ -331,6 +336,74 @@ public function testUpdatePropertiesForbidden(): void { $propPatch->commit(); } + public static function updateObjectIdsProvider(): array { + return [ + 'add a visible file' => [['2', '3'], ['2', '3', '1']], + 'remove a visible file' => [['3'], ['3', '1']], + 'remove all visible files' => [null, ['1']], + ]; + } + + #[\PHPUnit\Framework\Attributes\DataProvider(methodName: 'updateObjectIdsProvider')] + public function testUpdateObjectIdsKeepsHiddenObjects(?array $requestedIds, array $expectedIds): void { + $this->mockObjectIdsUpdate(Constants::PERMISSION_ALL); + + $this->tagMapper->expects($this->once()) + ->method('setObjectIdsForTag') + ->with('5', 'files', $expectedIds); + + $this->updateObjectIds($requestedIds); + } + + public static function updateObjectIdsForbiddenProvider(): array { + return [ + 'add a hidden file' => [['1', '2', '4'], Constants::PERMISSION_ALL], + 'remove a read-only file' => [null, Constants::PERMISSION_READ], + ]; + } + + #[\PHPUnit\Framework\Attributes\DataProvider(methodName: 'updateObjectIdsForbiddenProvider')] + public function testUpdateObjectIdsForbidden(?array $requestedIds, int $permissions): void { + $this->expectException(\Sabre\DAV\Exception\Forbidden::class); + $this->mockObjectIdsUpdate($permissions); + + $this->tagMapper->expects($this->never()) + ->method('setObjectIdsForTag'); + + $this->updateObjectIds($requestedIds); + } + + /** + * Files 1 and 2 are tagged, only files 2 and 3 are visible to the user + */ + private function mockObjectIdsUpdate(int $permissions): void { + $this->user->method('getUID')->willReturn('user'); + + $node = $this->createMock(SystemTagObjectType::class); + $node->method('getName')->willReturn('files'); + $node->method('getSystemTag')->willReturn(new SystemTag('5', 'Test', true, true)); + $this->tree->method('getNodeForPath')->willReturn($node); + + $this->tagMapper->method('getObjectIdsForTags')->willReturn(['1', '2']); + + $fileNode = $this->createMock(\OCP\Files\Node::class); + $fileNode->method('getPermissions')->willReturn($permissions); + $userFolder = $this->createMock(IUserFolder::class); + $userFolder->method('getFirstNodeById') + ->willReturnCallback(fn (int $id): ?\OCP\Files\Node => in_array($id, [2, 3], true) ? $fileNode : null); + $userFolder->method('getById') + ->willReturnCallback(fn (int $id): array => in_array($id, [2, 3], true) ? [$fileNode] : []); + $this->rootFolder->method('getUserFolder')->willReturn($userFolder); + } + + private function updateObjectIds(?array $objectIds): void { + $propPatch = new PropPatch([ + SystemTagPlugin::OBJECTIDS_PROPERTYNAME => $objectIds === null ? null : new SystemTagsObjectList(array_fill_keys($objectIds, 'files')), + ]); + $this->plugin->handleUpdateProperties('/systemtags/5/files', $propPatch); + $propPatch->commit(); + } + public static function createTagInsufficientPermissionsProvider(): array { return [ [true, false, ''],