From fafcac9b94a29352f7b8982b827da1f7d51a0455 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 27 Feb 2024 10:31:59 +0100 Subject: [PATCH 1/6] fix(systemtags): Forbid tagging of readonly files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../lib/SystemTag/SystemTagMappingNode.php | 64 +++-------------- .../SystemTagsObjectMappingCollection.php | 61 +++------------- .../SystemTagsObjectTypeCollection.php | 63 +++------------- .../SystemTagsRelationsCollection.php | 17 +++-- .../SystemTag/SystemTagMappingNodeTest.php | 51 +++++++------ .../SystemTagsObjectMappingCollectionTest.php | 72 ++++++++++--------- .../SystemTagsObjectTypeCollectionTest.php | 31 ++++---- 7 files changed, 126 insertions(+), 233 deletions(-) diff --git a/apps/dav/lib/SystemTag/SystemTagMappingNode.php b/apps/dav/lib/SystemTag/SystemTagMappingNode.php index 344ff1dbc70a9..ba6f9d23ad4e9 100644 --- a/apps/dav/lib/SystemTag/SystemTagMappingNode.php +++ b/apps/dav/lib/SystemTag/SystemTagMappingNode.php @@ -37,62 +37,15 @@ * Mapping node for system tag to object id */ class SystemTagMappingNode implements \Sabre\DAV\INode { - /** - * @var ISystemTag - */ - protected $tag; - - /** - * @var string - */ - private $objectId; - - /** - * @var string - */ - private $objectType; - - /** - * User - * - * @var IUser - */ - protected $user; - - /** - * @var ISystemTagManager - */ - protected $tagManager; - - /** - * @var ISystemTagObjectMapper - */ - private $tagMapper; - - /** - * Sets up the node, expects a full path name - * - * @param ISystemTag $tag system tag - * @param string $objectId - * @param string $objectType - * @param IUser $user user - * @param ISystemTagManager $tagManager - * @param ISystemTagObjectMapper $tagMapper - */ public function __construct( - ISystemTag $tag, - $objectId, - $objectType, - IUser $user, - ISystemTagManager $tagManager, - ISystemTagObjectMapper $tagMapper + private ISystemTag $tag, + private string $objectId, + private string $objectType, + private IUser $user, + private ISystemTagManager $tagManager, + private ISystemTagObjectMapper $tagMapper, + private \Closure $childWriteAccessFunction, ) { - $this->tag = $tag; - $this->objectId = $objectId; - $this->objectType = $objectType; - $this->user = $user; - $this->tagManager = $tagManager; - $this->tagMapper = $tagMapper; } /** @@ -161,6 +114,9 @@ public function delete() { if (!$this->tagManager->canUserAssignTag($this->tag, $this->user)) { throw new Forbidden('No permission to unassign tag ' . $this->tag->getId()); } + if (!($this->childWriteAccessFunction)($this->objectId)) { + throw new Forbidden('No permission to unassign tag to ' . $this->objectId); + } $this->tagMapper->unassignTags($this->objectId, $this->objectType, $this->tag->getId()); } catch (TagNotFoundException $e) { // can happen if concurrent deletion occurred diff --git a/apps/dav/lib/SystemTag/SystemTagsObjectMappingCollection.php b/apps/dav/lib/SystemTag/SystemTagsObjectMappingCollection.php index 8bb34182b0c2b..4f46c580c51b5 100644 --- a/apps/dav/lib/SystemTag/SystemTagsObjectMappingCollection.php +++ b/apps/dav/lib/SystemTag/SystemTagsObjectMappingCollection.php @@ -40,56 +40,14 @@ * Collection containing tags by object id */ class SystemTagsObjectMappingCollection implements ICollection { - - /** - * @var string - */ - private $objectId; - - /** - * @var string - */ - private $objectType; - - /** - * @var ISystemTagManager - */ - private $tagManager; - - /** - * @var ISystemTagObjectMapper - */ - private $tagMapper; - - /** - * User - * - * @var IUser - */ - private $user; - - - /** - * Constructor - * - * @param string $objectId object id - * @param string $objectType object type - * @param IUser $user user - * @param ISystemTagManager $tagManager tag manager - * @param ISystemTagObjectMapper $tagMapper tag mapper - */ public function __construct( - $objectId, - $objectType, - IUser $user, - ISystemTagManager $tagManager, - ISystemTagObjectMapper $tagMapper + private string $objectId, + private string $objectType, + private IUser $user, + private ISystemTagManager $tagManager, + private ISystemTagObjectMapper $tagMapper, + protected \Closure $childWriteAccessFunction, ) { - $this->tagManager = $tagManager; - $this->tagMapper = $tagMapper; - $this->objectId = $objectId; - $this->objectType = $objectType; - $this->user = $user; } public function createFile($name, $data = null) { @@ -103,7 +61,9 @@ public function createFile($name, $data = null) { if (!$this->tagManager->canUserAssignTag($tag, $this->user)) { throw new Forbidden('No permission to assign tag ' . $tagId); } - + if (!($this->childWriteAccessFunction)($this->objectId)) { + throw new Forbidden('No permission to assign tag to ' . $this->objectId); + } $this->tagMapper->assignTags($this->objectId, $this->objectType, $tagId); } catch (TagNotFoundException $e) { throw new PreconditionFailed('Tag with id ' . $tagId . ' does not exist, cannot assign'); @@ -204,7 +164,8 @@ private function makeNode(ISystemTag $tag) { $this->objectType, $this->user, $this->tagManager, - $this->tagMapper + $this->tagMapper, + $this->childWriteAccessFunction, ); } } diff --git a/apps/dav/lib/SystemTag/SystemTagsObjectTypeCollection.php b/apps/dav/lib/SystemTag/SystemTagsObjectTypeCollection.php index 1ca45c32ce46b..945183519e7a8 100644 --- a/apps/dav/lib/SystemTag/SystemTagsObjectTypeCollection.php +++ b/apps/dav/lib/SystemTag/SystemTagsObjectTypeCollection.php @@ -38,61 +38,15 @@ * Collection containing object ids by object type */ class SystemTagsObjectTypeCollection implements ICollection { - - /** - * @var string - */ - private $objectType; - - /** - * @var ISystemTagManager - */ - private $tagManager; - - /** - * @var ISystemTagObjectMapper - */ - private $tagMapper; - - /** - * @var IGroupManager - */ - private $groupManager; - - /** - * @var IUserSession - */ - private $userSession; - - /** - * @var \Closure - **/ - protected $childExistsFunction; - - /** - * Constructor - * - * @param string $objectType object type - * @param ISystemTagManager $tagManager - * @param ISystemTagObjectMapper $tagMapper - * @param IUserSession $userSession - * @param IGroupManager $groupManager - * @param \Closure $childExistsFunction - */ public function __construct( - $objectType, - ISystemTagManager $tagManager, - ISystemTagObjectMapper $tagMapper, - IUserSession $userSession, - IGroupManager $groupManager, - \Closure $childExistsFunction + private string $objectType, + private ISystemTagManager $tagManager, + private ISystemTagObjectMapper $tagMapper, + private IUserSession $userSession, + private IGroupManager $groupManager, + protected \Closure $childExistsFunction, + protected \Closure $childWriteAccessFunction, ) { - $this->tagManager = $tagManager; - $this->tagMapper = $tagMapper; - $this->objectType = $objectType; - $this->userSession = $userSession; - $this->groupManager = $groupManager; - $this->childExistsFunction = $childExistsFunction; } /** @@ -129,7 +83,8 @@ public function getChild($objectName) { $this->objectType, $this->userSession->getUser(), $this->tagManager, - $this->tagMapper + $this->tagMapper, + $this->childWriteAccessFunction, ); } diff --git a/apps/dav/lib/SystemTag/SystemTagsRelationsCollection.php b/apps/dav/lib/SystemTag/SystemTagsRelationsCollection.php index 4c179d5f0f661..cb0f3f1f5997c 100644 --- a/apps/dav/lib/SystemTag/SystemTagsRelationsCollection.php +++ b/apps/dav/lib/SystemTag/SystemTagsRelationsCollection.php @@ -36,7 +36,6 @@ use Symfony\Component\EventDispatcher\EventDispatcherInterface; class SystemTagsRelationsCollection extends SimpleCollection { - /** * SystemTagsRelationsCollection constructor. * @@ -60,10 +59,19 @@ public function __construct( $tagMapper, $userSession, $groupManager, - function ($name) { + function ($name): bool { $nodes = \OC::$server->getUserFolder()->getById((int)$name); return !empty($nodes); - } + }, + function ($name): bool { + $nodes = \OC::$server->getUserFolder()->getById((int)$name); + foreach ($nodes as $node) { + if (($node->getPermissions() & Constants::PERMISSION_UPDATE) === Constants::PERMISSION_UPDATE) { + return true; + } + } + return false; + }, ), ]; @@ -77,7 +85,8 @@ function ($name) { $tagMapper, $userSession, $groupManager, - $entityExistsFunction + $entityExistsFunction, + fn ($name) => true, ); } diff --git a/apps/dav/tests/unit/SystemTag/SystemTagMappingNodeTest.php b/apps/dav/tests/unit/SystemTag/SystemTagMappingNodeTest.php index e39cb0a04d3dd..287e7c568b86c 100644 --- a/apps/dav/tests/unit/SystemTag/SystemTagMappingNodeTest.php +++ b/apps/dav/tests/unit/SystemTag/SystemTagMappingNodeTest.php @@ -33,21 +33,9 @@ use OCP\SystemTag\TagNotFoundException; class SystemTagMappingNodeTest extends \Test\TestCase { - - /** - * @var \OCP\SystemTag\ISystemTagManager - */ - private $tagManager; - - /** - * @var \OCP\SystemTag\ISystemTagObjectMapper - */ - private $tagMapper; - - /** - * @var \OCP\IUser - */ - private $user; + private ISystemTagManager $tagManager; + private ISystemTagObjectMapper $tagMapper; + private IUser $user; protected function setUp(): void { parent::setUp(); @@ -60,7 +48,7 @@ protected function setUp(): void { ->getMock(); } - public function getMappingNode($tag = null) { + public function getMappingNode($tag = null, array $writableNodeIds = []) { if ($tag === null) { $tag = new SystemTag(1, 'Test', true, true); } @@ -70,7 +58,8 @@ public function getMappingNode($tag = null) { 'files', $this->user, $this->tagManager, - $this->tagMapper + $this->tagMapper, + fn ($id): bool => in_array($id, $writableNodeIds), ); } @@ -83,8 +72,8 @@ public function testGetters() { $this->assertEquals('files', $node->getObjectType()); } - public function testDeleteTag() { - $node = $this->getMappingNode(); + public function testDeleteTag(): void { + $node = $this->getMappingNode(null, [123]); $this->tagManager->expects($this->once()) ->method('canUserSeeTag') ->with($node->getSystemTag()) @@ -102,6 +91,25 @@ public function testDeleteTag() { $node->delete(); } + public function testDeleteTagForbidden(): void { + $node = $this->getMappingNode(); + $this->tagManager->expects($this->once()) + ->method('canUserSeeTag') + ->with($node->getSystemTag()) + ->willReturn(true); + $this->tagManager->expects($this->once()) + ->method('canUserAssignTag') + ->with($node->getSystemTag()) + ->willReturn(true); + $this->tagManager->expects($this->never()) + ->method('deleteTags'); + $this->tagMapper->expects($this->never()) + ->method('unassignTags'); + + $this->expectException(\Sabre\DAV\Exception\Forbidden::class); + $node->delete(); + } + public function tagNodeDeleteProviderPermissionException() { return [ [ @@ -144,8 +152,7 @@ public function testDeleteTagExpectedException(ISystemTag $tag, $expectedExcepti $this->assertInstanceOf($expectedException, $thrown); } - - public function testDeleteTagNotFound() { + public function testDeleteTagNotFound(): void { $this->expectException(\Sabre\DAV\Exception\NotFound::class); // assuming the tag existed at the time the node was created, @@ -164,6 +171,6 @@ public function testDeleteTagNotFound() { ->with(123, 'files', 1) ->will($this->throwException(new TagNotFoundException())); - $this->getMappingNode($tag)->delete(); + $this->getMappingNode($tag, [123])->delete(); } } diff --git a/apps/dav/tests/unit/SystemTag/SystemTagsObjectMappingCollectionTest.php b/apps/dav/tests/unit/SystemTag/SystemTagsObjectMappingCollectionTest.php index bb71de8ea8e85..8e0b7199d82f1 100644 --- a/apps/dav/tests/unit/SystemTag/SystemTagsObjectMappingCollectionTest.php +++ b/apps/dav/tests/unit/SystemTag/SystemTagsObjectMappingCollectionTest.php @@ -32,21 +32,9 @@ use OCP\SystemTag\TagNotFoundException; class SystemTagsObjectMappingCollectionTest extends \Test\TestCase { - - /** - * @var \OCP\SystemTag\ISystemTagManager - */ - private $tagManager; - - /** - * @var \OCP\SystemTag\ISystemTagObjectMapper - */ - private $tagMapper; - - /** - * @var \OCP\IUser - */ - private $user; + private ISystemTagManager $tagManager; + private ISystemTagObjectMapper $tagMapper; + private IUser $user; protected function setUp(): void { parent::setUp(); @@ -60,13 +48,14 @@ protected function setUp(): void { ->getMock(); } - public function getNode() { + public function getNode(array $writableNodeIds = []) { return new \OCA\DAV\SystemTag\SystemTagsObjectMappingCollection( 111, 'files', $this->user, $this->tagManager, - $this->tagMapper + $this->tagMapper, + fn ($id): bool => in_array($id, $writableNodeIds), ); } @@ -89,6 +78,28 @@ public function testAssignTag() { ->method('assignTags') ->with(111, 'files', '555'); + $this->getNode([111])->createFile('555'); + } + + public function testAssignTagForbidden(): void { + $tag = new SystemTag('1', 'Test', true, true); + $this->tagManager->expects($this->once()) + ->method('canUserSeeTag') + ->with($tag) + ->willReturn(true); + $this->tagManager->expects($this->once()) + ->method('canUserAssignTag') + ->with($tag) + ->willReturn(true); + + $this->tagManager->expects($this->once()) + ->method('getTagsByIds') + ->with(['555']) + ->willReturn([$tag]); + $this->tagMapper->expects($this->never()) + ->method('assignTags'); + + $this->expectException(\Sabre\DAV\Exception\Forbidden::class); $this->getNode()->createFile('555'); } @@ -132,8 +143,7 @@ public function testAssignTagNoPermission($userVisible, $userAssignable, $expect $this->assertInstanceOf($expectedException, $thrown); } - - public function testAssignTagNotFound() { + public function testAssignTagNotFound(): void { $this->expectException(\Sabre\DAV\Exception\PreconditionFailed::class); $this->tagManager->expects($this->once()) @@ -144,8 +154,7 @@ public function testAssignTagNotFound() { $this->getNode()->createFile('555'); } - - public function testForbiddenCreateDirectory() { + public function testForbiddenCreateDirectory(): void { $this->expectException(\Sabre\DAV\Exception\Forbidden::class); $this->getNode()->createDirectory('789'); @@ -174,8 +183,7 @@ public function testGetChild() { $this->assertEquals('555', $childNode->getName()); } - - public function testGetChildNonVisible() { + public function testGetChildNonVisible(): void { $this->expectException(\Sabre\DAV\Exception\NotFound::class); $tag = new SystemTag(555, 'TheTag', false, false); @@ -197,8 +205,7 @@ public function testGetChildNonVisible() { $this->getNode()->getChild('555'); } - - public function testGetChildRelationNotFound() { + public function testGetChildRelationNotFound(): void { $this->expectException(\Sabre\DAV\Exception\NotFound::class); $this->tagMapper->expects($this->once()) @@ -209,8 +216,7 @@ public function testGetChildRelationNotFound() { $this->getNode()->getChild('777'); } - - public function testGetChildInvalidId() { + public function testGetChildInvalidId(): void { $this->expectException(\Sabre\DAV\Exception\BadRequest::class); $this->tagMapper->expects($this->once()) @@ -221,8 +227,7 @@ public function testGetChildInvalidId() { $this->getNode()->getChild('badid'); } - - public function testGetChildTagDoesNotExist() { + public function testGetChildTagDoesNotExist(): void { $this->expectException(\Sabre\DAV\Exception\NotFound::class); $this->tagMapper->expects($this->once()) @@ -325,8 +330,7 @@ public function testChildExistsTagNotFound() { $this->assertFalse($this->getNode()->childExists('555')); } - - public function testChildExistsInvalidId() { + public function testChildExistsInvalidId(): void { $this->expectException(\Sabre\DAV\Exception\BadRequest::class); $this->tagMapper->expects($this->once()) @@ -337,15 +341,13 @@ public function testChildExistsInvalidId() { $this->getNode()->childExists('555'); } - - public function testDelete() { + public function testDelete(): void { $this->expectException(\Sabre\DAV\Exception\Forbidden::class); $this->getNode()->delete(); } - - public function testSetName() { + public function testSetName(): void { $this->expectException(\Sabre\DAV\Exception\Forbidden::class); $this->getNode()->setName('somethingelse'); diff --git a/apps/dav/tests/unit/SystemTag/SystemTagsObjectTypeCollectionTest.php b/apps/dav/tests/unit/SystemTag/SystemTagsObjectTypeCollectionTest.php index 11c9fc5977c8f..27140292642c7 100644 --- a/apps/dav/tests/unit/SystemTag/SystemTagsObjectTypeCollectionTest.php +++ b/apps/dav/tests/unit/SystemTag/SystemTagsObjectTypeCollectionTest.php @@ -33,7 +33,6 @@ use OCP\SystemTag\ISystemTagObjectMapper; class SystemTagsObjectTypeCollectionTest extends \Test\TestCase { - /** * @var \OCA\DAV\SystemTag\SystemTagsObjectTypeCollection */ @@ -87,6 +86,15 @@ protected function setUp(): void { $nodes = $userFolder->getById(intval($name)); return !empty($nodes); }; + $writeAccessClosure = function ($name) use ($userFolder) { + $nodes = $userFolder->getById((int)$name); + foreach ($nodes as $node) { + if (($node->getPermissions() & Constants::PERMISSION_UPDATE) === Constants::PERMISSION_UPDATE) { + return true; + } + } + return false; + }; $this->node = new \OCA\DAV\SystemTag\SystemTagsObjectTypeCollection( 'files', @@ -94,19 +102,18 @@ protected function setUp(): void { $this->tagMapper, $userSession, $groupManager, - $closure + $closure, + $writeAccessClosure, ); } - - public function testForbiddenCreateFile() { + public function testForbiddenCreateFile(): void { $this->expectException(\Sabre\DAV\Exception\Forbidden::class); $this->node->createFile('555'); } - - public function testForbiddenCreateDirectory() { + public function testForbiddenCreateDirectory(): void { $this->expectException(\Sabre\DAV\Exception\Forbidden::class); $this->node->createDirectory('789'); @@ -123,8 +130,7 @@ public function testGetChild() { $this->assertEquals('555', $childNode->getName()); } - - public function testGetChildWithoutAccess() { + public function testGetChildWithoutAccess(): void { $this->expectException(\Sabre\DAV\Exception\NotFound::class); $this->userFolder->expects($this->once()) @@ -134,8 +140,7 @@ public function testGetChildWithoutAccess() { $this->node->getChild('555'); } - - public function testGetChildren() { + public function testGetChildren(): void { $this->expectException(\Sabre\DAV\Exception\MethodNotAllowed::class); $this->node->getChildren(); @@ -157,15 +162,13 @@ public function testChildExistsWithoutAccess() { $this->assertFalse($this->node->childExists('555')); } - - public function testDelete() { + public function testDelete(): void { $this->expectException(\Sabre\DAV\Exception\Forbidden::class); $this->node->delete(); } - - public function testSetName() { + public function testSetName(): void { $this->expectException(\Sabre\DAV\Exception\Forbidden::class); $this->node->setName('somethingelse'); From 1013d8b42f25419fa38caac48583e4eab70932cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 4 Mar 2024 12:01:20 +0100 Subject: [PATCH 2/6] chore: Break closure call on two lines to make it readable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- apps/dav/lib/SystemTag/SystemTagMappingNode.php | 3 ++- apps/dav/lib/SystemTag/SystemTagsObjectMappingCollection.php | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/apps/dav/lib/SystemTag/SystemTagMappingNode.php b/apps/dav/lib/SystemTag/SystemTagMappingNode.php index ba6f9d23ad4e9..96180d0e0d6f5 100644 --- a/apps/dav/lib/SystemTag/SystemTagMappingNode.php +++ b/apps/dav/lib/SystemTag/SystemTagMappingNode.php @@ -114,7 +114,8 @@ public function delete() { if (!$this->tagManager->canUserAssignTag($this->tag, $this->user)) { throw new Forbidden('No permission to unassign tag ' . $this->tag->getId()); } - if (!($this->childWriteAccessFunction)($this->objectId)) { + $writeAccessFunction = $this->childWriteAccessFunction; + if (!$writeAccessFunction($this->objectId)) { throw new Forbidden('No permission to unassign tag to ' . $this->objectId); } $this->tagMapper->unassignTags($this->objectId, $this->objectType, $this->tag->getId()); diff --git a/apps/dav/lib/SystemTag/SystemTagsObjectMappingCollection.php b/apps/dav/lib/SystemTag/SystemTagsObjectMappingCollection.php index 4f46c580c51b5..38b2c3b2f824d 100644 --- a/apps/dav/lib/SystemTag/SystemTagsObjectMappingCollection.php +++ b/apps/dav/lib/SystemTag/SystemTagsObjectMappingCollection.php @@ -61,7 +61,8 @@ public function createFile($name, $data = null) { if (!$this->tagManager->canUserAssignTag($tag, $this->user)) { throw new Forbidden('No permission to assign tag ' . $tagId); } - if (!($this->childWriteAccessFunction)($this->objectId)) { + $writeAccessFunction = $this->childWriteAccessFunction; + if (!$writeAccessFunction($this->objectId)) { throw new Forbidden('No permission to assign tag to ' . $this->objectId); } $this->tagMapper->assignTags($this->objectId, $this->objectType, $tagId); From b57486182f02c49f3a5d68797538ca43c63f375e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 19 Mar 2024 11:20:03 +0100 Subject: [PATCH 3/6] fix(dav): Add missing use for OCP\Constants MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- apps/dav/lib/SystemTag/SystemTagsRelationsCollection.php | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/dav/lib/SystemTag/SystemTagsRelationsCollection.php b/apps/dav/lib/SystemTag/SystemTagsRelationsCollection.php index cb0f3f1f5997c..7a3b5edd9543a 100644 --- a/apps/dav/lib/SystemTag/SystemTagsRelationsCollection.php +++ b/apps/dav/lib/SystemTag/SystemTagsRelationsCollection.php @@ -26,6 +26,7 @@ */ namespace OCA\DAV\SystemTag; +use OCP\Constants; use OCP\IGroupManager; use OCP\IUserSession; use OCP\SystemTag\ISystemTagManager; From a5ac6d9db378c846c78dea70d402929740483f88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 19 Mar 2024 14:28:54 +0100 Subject: [PATCH 4/6] chore: Remove PHP8 constructor property promotions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 25 supports 7.4 Signed-off-by: Côme Chilliet --- .../lib/SystemTag/SystemTagMappingNode.php | 29 ++++++++++++++----- .../SystemTagsObjectMappingCollection.php | 25 ++++++++++++---- .../SystemTagsObjectTypeCollection.php | 29 ++++++++++++++----- 3 files changed, 63 insertions(+), 20 deletions(-) diff --git a/apps/dav/lib/SystemTag/SystemTagMappingNode.php b/apps/dav/lib/SystemTag/SystemTagMappingNode.php index 96180d0e0d6f5..68e52ea9514fd 100644 --- a/apps/dav/lib/SystemTag/SystemTagMappingNode.php +++ b/apps/dav/lib/SystemTag/SystemTagMappingNode.php @@ -37,15 +37,30 @@ * Mapping node for system tag to object id */ class SystemTagMappingNode implements \Sabre\DAV\INode { + private ISystemTag $tag; + private string $objectId; + private string $objectType; + private IUser $user; + private ISystemTagManager $tagManager; + private ISystemTagObjectMapper $tagMapper; + private \Closure $childWriteAccessFunction; + public function __construct( - private ISystemTag $tag, - private string $objectId, - private string $objectType, - private IUser $user, - private ISystemTagManager $tagManager, - private ISystemTagObjectMapper $tagMapper, - private \Closure $childWriteAccessFunction, + ISystemTag $tag, + string $objectId, + string $objectType, + IUser $user, + ISystemTagManager $tagManager, + ISystemTagObjectMapper $tagMapper, + \Closure $childWriteAccessFunction ) { + $this->tag = $tag; + $this->objectId = $objectId; + $this->objectType = $objectType; + $this->user = $user; + $this->tagManager = $tagManager; + $this->tagMapper = $tagMapper; + $this->childWriteAccessFunction = $childWriteAccessFunction; } /** diff --git a/apps/dav/lib/SystemTag/SystemTagsObjectMappingCollection.php b/apps/dav/lib/SystemTag/SystemTagsObjectMappingCollection.php index 38b2c3b2f824d..fba4ac64fb617 100644 --- a/apps/dav/lib/SystemTag/SystemTagsObjectMappingCollection.php +++ b/apps/dav/lib/SystemTag/SystemTagsObjectMappingCollection.php @@ -40,14 +40,27 @@ * Collection containing tags by object id */ class SystemTagsObjectMappingCollection implements ICollection { + private string $objectId; + private string $objectType; + private IUser $user; + private ISystemTagManager $tagManager; + private ISystemTagObjectMapper $tagMapper; + protected \Closure $childWriteAccessFunction; + public function __construct( - private string $objectId, - private string $objectType, - private IUser $user, - private ISystemTagManager $tagManager, - private ISystemTagObjectMapper $tagMapper, - protected \Closure $childWriteAccessFunction, + string $objectId, + string $objectType, + IUser $user, + ISystemTagManager $tagManager, + ISystemTagObjectMapper $tagMapper, + \Closure $childWriteAccessFunction ) { + $this->objectId = $objectId; + $this->objectType = $objectType; + $this->user = $user; + $this->tagManager = $tagManager; + $this->tagMapper = $tagMapper; + $this->childWriteAccessFunction = $childWriteAccessFunction; } public function createFile($name, $data = null) { diff --git a/apps/dav/lib/SystemTag/SystemTagsObjectTypeCollection.php b/apps/dav/lib/SystemTag/SystemTagsObjectTypeCollection.php index 945183519e7a8..d989bdb96834e 100644 --- a/apps/dav/lib/SystemTag/SystemTagsObjectTypeCollection.php +++ b/apps/dav/lib/SystemTag/SystemTagsObjectTypeCollection.php @@ -38,15 +38,30 @@ * Collection containing object ids by object type */ class SystemTagsObjectTypeCollection implements ICollection { + private string $objectType; + private ISystemTagManager $tagManager; + private ISystemTagObjectMapper $tagMapper; + private IUserSession $userSession; + private IGroupManager $groupManager; + protected \Closure $childExistsFunction; + protected \Closure $childWriteAccessFunction; + public function __construct( - private string $objectType, - private ISystemTagManager $tagManager, - private ISystemTagObjectMapper $tagMapper, - private IUserSession $userSession, - private IGroupManager $groupManager, - protected \Closure $childExistsFunction, - protected \Closure $childWriteAccessFunction, + string $objectType, + ISystemTagManager $tagManager, + ISystemTagObjectMapper $tagMapper, + IUserSession $userSession, + IGroupManager $groupManager, + \Closure $childExistsFunction, + \Closure $childWriteAccessFunction ) { + $this->objectType = $objectType; + $this->tagManager = $tagManager; + $this->tagMapper = $tagMapper; + $this->userSession = $userSession; + $this->groupManager = $groupManager; + $this->childExistsFunction = $childExistsFunction; + $this->childWriteAccessFunction = $childWriteAccessFunction; } /** From b4a166e1cc365d58ed63bc6e490fdb8ae8c7f566 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 19 Mar 2024 16:11:30 +0100 Subject: [PATCH 5/6] chore: Remove typed properties to support 7.3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 23 still supports PHP 7.3 Signed-off-by: Côme Chilliet --- .../lib/SystemTag/SystemTagMappingNode.php | 21 ++++++++++++------- .../SystemTagsObjectMappingCollection.php | 18 ++++++++++------ .../SystemTagsObjectTypeCollection.php | 21 ++++++++++++------- .../SystemTag/SystemTagMappingNodeTest.php | 9 +++++--- .../SystemTagsObjectMappingCollectionTest.php | 9 +++++--- 5 files changed, 52 insertions(+), 26 deletions(-) diff --git a/apps/dav/lib/SystemTag/SystemTagMappingNode.php b/apps/dav/lib/SystemTag/SystemTagMappingNode.php index 68e52ea9514fd..a0dc855778b07 100644 --- a/apps/dav/lib/SystemTag/SystemTagMappingNode.php +++ b/apps/dav/lib/SystemTag/SystemTagMappingNode.php @@ -37,13 +37,20 @@ * Mapping node for system tag to object id */ class SystemTagMappingNode implements \Sabre\DAV\INode { - private ISystemTag $tag; - private string $objectId; - private string $objectType; - private IUser $user; - private ISystemTagManager $tagManager; - private ISystemTagObjectMapper $tagMapper; - private \Closure $childWriteAccessFunction; + /** @var ISystemTag */ + private $tag; + /** @var string */ + private $objectId; + /** @var string */ + private $objectType; + /** @var IUser */ + private $user; + /** @var ISystemTagManager */ + private $tagManager; + /** @var ISystemTagObjectMapper */ + private $tagMapper; + /** @var \Closure */ + private $childWriteAccessFunction; public function __construct( ISystemTag $tag, diff --git a/apps/dav/lib/SystemTag/SystemTagsObjectMappingCollection.php b/apps/dav/lib/SystemTag/SystemTagsObjectMappingCollection.php index fba4ac64fb617..7b6d5279d5d99 100644 --- a/apps/dav/lib/SystemTag/SystemTagsObjectMappingCollection.php +++ b/apps/dav/lib/SystemTag/SystemTagsObjectMappingCollection.php @@ -40,12 +40,18 @@ * Collection containing tags by object id */ class SystemTagsObjectMappingCollection implements ICollection { - private string $objectId; - private string $objectType; - private IUser $user; - private ISystemTagManager $tagManager; - private ISystemTagObjectMapper $tagMapper; - protected \Closure $childWriteAccessFunction; + /** @var string */ + private $objectId; + /** @var string */ + private $objectType; + /** @var IUser */ + private $user; + /** @var ISystemTagManager */ + private $tagManager; + /** @var ISystemTagObjectMapper */ + private $tagMapper; + /** @var \Closure */ + protected $childWriteAccessFunction; public function __construct( string $objectId, diff --git a/apps/dav/lib/SystemTag/SystemTagsObjectTypeCollection.php b/apps/dav/lib/SystemTag/SystemTagsObjectTypeCollection.php index d989bdb96834e..fe9d0acff238c 100644 --- a/apps/dav/lib/SystemTag/SystemTagsObjectTypeCollection.php +++ b/apps/dav/lib/SystemTag/SystemTagsObjectTypeCollection.php @@ -38,13 +38,20 @@ * Collection containing object ids by object type */ class SystemTagsObjectTypeCollection implements ICollection { - private string $objectType; - private ISystemTagManager $tagManager; - private ISystemTagObjectMapper $tagMapper; - private IUserSession $userSession; - private IGroupManager $groupManager; - protected \Closure $childExistsFunction; - protected \Closure $childWriteAccessFunction; + /** @var string */ + private $objectType; + /** @var ISystemTagManager */ + private $tagManager; + /** @var ISystemTagObjectMapper */ + private $tagMapper; + /** @var IUserSession */ + private $userSession; + /** @var IGroupManager */ + private $groupManager; + /** @var \Closure */ + protected $childExistsFunction; + /** @var \Closure */ + protected $childWriteAccessFunction; public function __construct( string $objectType, diff --git a/apps/dav/tests/unit/SystemTag/SystemTagMappingNodeTest.php b/apps/dav/tests/unit/SystemTag/SystemTagMappingNodeTest.php index 287e7c568b86c..f1af249186ed8 100644 --- a/apps/dav/tests/unit/SystemTag/SystemTagMappingNodeTest.php +++ b/apps/dav/tests/unit/SystemTag/SystemTagMappingNodeTest.php @@ -33,9 +33,12 @@ use OCP\SystemTag\TagNotFoundException; class SystemTagMappingNodeTest extends \Test\TestCase { - private ISystemTagManager $tagManager; - private ISystemTagObjectMapper $tagMapper; - private IUser $user; + /** @var ISystemTagManager */ + private $tagManager; + /** @var ISystemTagObjectMapper */ + private $tagMapper; + /** @var IUser */ + private $user; protected function setUp(): void { parent::setUp(); diff --git a/apps/dav/tests/unit/SystemTag/SystemTagsObjectMappingCollectionTest.php b/apps/dav/tests/unit/SystemTag/SystemTagsObjectMappingCollectionTest.php index 8e0b7199d82f1..3f3aa4e4695e1 100644 --- a/apps/dav/tests/unit/SystemTag/SystemTagsObjectMappingCollectionTest.php +++ b/apps/dav/tests/unit/SystemTag/SystemTagsObjectMappingCollectionTest.php @@ -32,9 +32,12 @@ use OCP\SystemTag\TagNotFoundException; class SystemTagsObjectMappingCollectionTest extends \Test\TestCase { - private ISystemTagManager $tagManager; - private ISystemTagObjectMapper $tagMapper; - private IUser $user; + /** @var ISystemTagManager */ + private $tagManager; + /** @var ISystemTagObjectMapper */ + private $tagMapper; + /** @var IUser */ + private $user; protected function setUp(): void { parent::setUp(); From a4473ce8c0c53bd5d6042980c8962dac511c92ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 21 Mar 2024 11:50:03 +0100 Subject: [PATCH 6/6] chore(dav): Remove short function closure syntax not supported in PHP 7.3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- apps/dav/lib/SystemTag/SystemTagsRelationsCollection.php | 4 +++- apps/dav/tests/unit/SystemTag/SystemTagMappingNodeTest.php | 4 +++- .../unit/SystemTag/SystemTagsObjectMappingCollectionTest.php | 4 +++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/apps/dav/lib/SystemTag/SystemTagsRelationsCollection.php b/apps/dav/lib/SystemTag/SystemTagsRelationsCollection.php index 7a3b5edd9543a..a6be9671bfbf8 100644 --- a/apps/dav/lib/SystemTag/SystemTagsRelationsCollection.php +++ b/apps/dav/lib/SystemTag/SystemTagsRelationsCollection.php @@ -87,7 +87,9 @@ function ($name): bool { $userSession, $groupManager, $entityExistsFunction, - fn ($name) => true, + function ($name) { + return true; + }, ); } diff --git a/apps/dav/tests/unit/SystemTag/SystemTagMappingNodeTest.php b/apps/dav/tests/unit/SystemTag/SystemTagMappingNodeTest.php index f1af249186ed8..b6078e69d6302 100644 --- a/apps/dav/tests/unit/SystemTag/SystemTagMappingNodeTest.php +++ b/apps/dav/tests/unit/SystemTag/SystemTagMappingNodeTest.php @@ -62,7 +62,9 @@ public function getMappingNode($tag = null, array $writableNodeIds = []) { $this->user, $this->tagManager, $this->tagMapper, - fn ($id): bool => in_array($id, $writableNodeIds), + function ($id) use ($writableNodeIds): bool { + return in_array($id, $writableNodeIds); + }, ); } diff --git a/apps/dav/tests/unit/SystemTag/SystemTagsObjectMappingCollectionTest.php b/apps/dav/tests/unit/SystemTag/SystemTagsObjectMappingCollectionTest.php index 3f3aa4e4695e1..d99ab2512cfcf 100644 --- a/apps/dav/tests/unit/SystemTag/SystemTagsObjectMappingCollectionTest.php +++ b/apps/dav/tests/unit/SystemTag/SystemTagsObjectMappingCollectionTest.php @@ -58,7 +58,9 @@ public function getNode(array $writableNodeIds = []) { $this->user, $this->tagManager, $this->tagMapper, - fn ($id): bool => in_array($id, $writableNodeIds), + function ($id) use ($writableNodeIds): bool { + return in_array($id, $writableNodeIds); + }, ); }