upstream/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2026-02-20 00:12:30 -05:00
Code Issues Projects Releases Packages Wiki Activity Actions 38

fix(systemtags): Forbid tagging of readonly files

Browse source 
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
This commit is contained in:
Côme Chilliet 2024-02-27 10:31:59 +01:00
parent fa33b00be7
commit edf5925f71
No known key found for this signature in database
GPG key ID: A3E2F658B28C760A
7 changed files with 126 additions and 233 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

64
apps/dav/lib/SystemTag/SystemTagMappingNode.php
View file

@ -37,62 +37,15 @@ use Sabre\DAV\Exception\NotFound;
* 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 @@ class SystemTagMappingNode implements \Sabre\DAV\INode {
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

61
apps/dav/lib/SystemTag/SystemTagsObjectMappingCollection.php
View file

@ -40,56 +40,14 @@ use Sabre\DAV\ICollection;
* 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 @@ class SystemTagsObjectMappingCollection implements ICollection {
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 @@ class SystemTagsObjectMappingCollection implements ICollection {
$this->objectType,
$this->user,
$this->tagManager,
$this->tagMapper
$this->tagMapper,
$this->childWriteAccessFunction,
);
}
}

63
apps/dav/lib/SystemTag/SystemTagsObjectTypeCollection.php
View file

@ -38,61 +38,15 @@ use Sabre\DAV\ICollection;
* 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 @@ class SystemTagsObjectTypeCollection implements ICollection {
$this->objectType,
$this->userSession->getUser(),
$this->tagManager,
$this->tagMapper
$this->tagMapper,
$this->childWriteAccessFunction,
);
}

17
apps/dav/lib/SystemTag/SystemTagsRelationsCollection.php
View file

@ -36,7 +36,6 @@ use Sabre\DAV\SimpleCollection;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
class SystemTagsRelationsCollection extends SimpleCollection {
/**
* SystemTagsRelationsCollection constructor.
*
@ -60,10 +59,19 @@ class SystemTagsRelationsCollection extends SimpleCollection {
$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 @@ class SystemTagsRelationsCollection extends SimpleCollection {
$tagMapper,
$userSession,
$groupManager,
$entityExistsFunction
$entityExistsFunction,
fn ($name) => true,
);
}

51
apps/dav/tests/unit/SystemTag/SystemTagMappingNodeTest.php
View file

@ -33,21 +33,9 @@ use OCP\SystemTag\ISystemTagObjectMapper;
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 @@ class SystemTagMappingNodeTest extends \Test\TestCase {
->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 @@ class SystemTagMappingNodeTest extends \Test\TestCase {
'files',
$this->user,
$this->tagManager,
$this->tagMapper
$this->tagMapper,
fn ($id): bool => in_array($id, $writableNodeIds),
);
}
@ -83,8 +72,8 @@ class SystemTagMappingNodeTest extends \Test\TestCase {
$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 @@ class SystemTagMappingNodeTest extends \Test\TestCase {
$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 @@ class SystemTagMappingNodeTest extends \Test\TestCase {
$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 @@ class SystemTagMappingNodeTest extends \Test\TestCase {
->with(123, 'files', 1)
->will($this->throwException(new TagNotFoundException()));
$this->getMappingNode($tag)->delete();
$this->getMappingNode($tag, [123])->delete();
}
}

72
apps/dav/tests/unit/SystemTag/SystemTagsObjectMappingCollectionTest.php
View file

@ -32,21 +32,9 @@ use OCP\SystemTag\ISystemTagObjectMapper;
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 @@ class SystemTagsObjectMappingCollectionTest extends \Test\TestCase {
->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 @@ class SystemTagsObjectMappingCollectionTest extends \Test\TestCase {
->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 @@ class SystemTagsObjectMappingCollectionTest extends \Test\TestCase {
$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 @@ class SystemTagsObjectMappingCollectionTest extends \Test\TestCase {
$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 @@ class SystemTagsObjectMappingCollectionTest extends \Test\TestCase {
$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 @@ class SystemTagsObjectMappingCollectionTest extends \Test\TestCase {
$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 @@ class SystemTagsObjectMappingCollectionTest extends \Test\TestCase {
$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 @@ class SystemTagsObjectMappingCollectionTest extends \Test\TestCase {
$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 @@ class SystemTagsObjectMappingCollectionTest extends \Test\TestCase {
$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 @@ class SystemTagsObjectMappingCollectionTest extends \Test\TestCase {
$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');

31
apps/dav/tests/unit/SystemTag/SystemTagsObjectTypeCollectionTest.php
View file

@ -33,7 +33,6 @@ use OCP\SystemTag\ISystemTagManager;
use OCP\SystemTag\ISystemTagObjectMapper;
class SystemTagsObjectTypeCollectionTest extends \Test\TestCase {
/**
* @var \OCA\DAV\SystemTag\SystemTagsObjectTypeCollection
*/
@ -87,6 +86,15 @@ class SystemTagsObjectTypeCollectionTest extends \Test\TestCase {
$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 @@ class SystemTagsObjectTypeCollectionTest extends \Test\TestCase {
$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 @@ class SystemTagsObjectTypeCollectionTest extends \Test\TestCase {
$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 @@ class SystemTagsObjectTypeCollectionTest extends \Test\TestCase {
$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 @@ class SystemTagsObjectTypeCollectionTest extends \Test\TestCase {
$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');