Skip to content

Commit e6d3318

Browse files
committed
refactor(FoldersController): Refactor editFolder
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
1 parent 0525c8e commit e6d3318

2 files changed

Lines changed: 37 additions & 0 deletions

File tree

lib/Controller/FoldersController.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,11 @@ public function editFolder(int $folderId, ?string $title = null, ?int $parent_fo
362362
try {
363363
if ($parent_folder !== null) {
364364
$parent_folder = $this->toInternalFolderId($parent_folder);
365+
if (!Authorizer::hasPermission(Authorizer::PERM_EDIT, $this->authorizer->getPermissionsForFolder($parent_folder, $this->request))) {
366+
$res = new JSONResponse(['status' => 'error', 'data' => ['Not found']], Http::STATUS_NOT_FOUND);
367+
$res->throttle();
368+
return $res;
369+
}
365370
}
366371
$folder = $this->folders->updateSharedFolderOrFolder($this->authorizer->getUserId(), $folderId, $title, $parent_folder);
367372
return new JSONResponse(['status' => 'success', 'item' => $this->_returnFolderAsArray($folder)]);

tests/FolderControllerTest.php

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,38 @@ public function testSharedEdit(bool $canWrite): void {
464464
$this->assertEquals([$this->folder4->getId()], $topLevelFolders);
465465
}
466466

467+
/**
468+
* Regression test: moving a folder into a parent folder the user
469+
* is not allowed to edit must be rejected.
470+
*
471+
* @throws AlreadyExistsError
472+
* @throws UrlParseError
473+
* @throws UserLimitExceededError
474+
* @throws \OCP\AppFramework\Db\DoesNotExistException
475+
* @throws MultipleObjectsReturnedException
476+
*/
477+
public function testEditMoveToUnauthorizedParentFail(): void {
478+
$this->setupBookmarks();
479+
$this->authorizer->setUserId($this->userId);
480+
481+
// The user owns (and may edit) folder1 but has no access to folder4,
482+
// which belongs to otherUser and is not shared.
483+
$output = $this->controller->editFolder($this->folder1->getId(), 'blabla', $this->folder4->getId());
484+
$data = $output->getData();
485+
$this->assertEquals('error', $data['status'], var_export($data, true));
486+
487+
// folder1 must stay where it was (a top-level folder), untouched.
488+
$output = $this->controller->getFolder($this->folder1->getId());
489+
$data = $output->getData();
490+
$this->assertEquals('success', $data['status'], var_export($data, true));
491+
$this->assertEquals('foo', $data['item']['title']); // title unchanged
492+
$this->assertEquals($this->folderMapper->findRootFolder($this->userId)->getId(), $data['item']['parent_folder']);
493+
494+
$output = $this->controller->getFolders();
495+
$topLevelFolders = array_map(fn ($item) => $item['id'], $output->getData()['data']);
496+
$this->assertEquals([$this->folder1->getId(), $this->folder3->getId()], $topLevelFolders);
497+
}
498+
467499
/**
468500
* @throws AlreadyExistsError
469501
* @throws UrlParseError

0 commit comments

Comments
 (0)