Merge branch 'master' into stable

CHANGELOG.md

@@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [13.1.3] - 2023-12-18
+
+### Fixes
+
+* fix(Authorizer)
+
 ## [13.1.2] - 2023-12-14
 
 ### Fixed

Makefile

@@ -7,7 +7,7 @@ source_dir=$(build_dir)/source
 sign_dir=$(build_dir)/sign
 package_name=$(app_name)
 cert_dir=$(HOME)/.nextcloud/certificates
-version+=13.1.2
+version+=13.1.3
 
 all: dev-setup build-js-production composer-no-dev
 

appinfo/info.xml

@@ -22,7 +22,7 @@ Requirements:
    - mbstring: *
  - when using MySQL, use at least v8.0
 ]]></description>
-	<version>13.1.2</version>
+	<version>13.1.3</version>
 	<licence>agpl</licence>
 	<author mail="[email protected]">Marcel Klehr</author>
 	<author mail="[email protected]" homepage="https://www.arthur-schiwon.de">Arthur Schiwon</author>

lib/Controller/FoldersController.php

@@ -247,8 +247,8 @@ public function addToFolder($folderId, $bookmarkId): JSONResponse {
 	 * @PublicPage
 	 */
 	public function removeFromFolder($folderId, $bookmarkId): JSONResponse {
-		if (!Authorizer::hasPermission(Authorizer::PERM_WRITE, $this->authorizer->getPermissionsForFolder($folderId, $this->request)) &&
-			!Authorizer::hasPermission(Authorizer::PERM_EDIT, $this->authorizer->getPermissionsForFolder($bookmarkId, $this->request))) {
+		if (!Authorizer::hasPermission(Authorizer::PERM_WRITE, $this->authorizer->getPermissionsForFolder($folderId, $this->request)) ||
+			!Authorizer::hasPermission(Authorizer::PERM_EDIT, $this->authorizer->getPermissionsForBookmark($bookmarkId, $this->request))) {
 			return new JSONResponse(['status' => 'error', 'data' => 'Unauthorized'], Http::STATUS_FORBIDDEN);
 		}
 		try {

lib/Service/Authorizer.php

@@ -263,7 +263,7 @@ private function findPermissionsByUserAndItem(string $userId, string $type, int
 			if ($share->getFolderId() === $itemId && $type === TreeMapper::TYPE_FOLDER) {
 				// If the sought folder is the root folder of the share, we give EDIT permissions + optionally RESHARE
 				// because the user can edit the shared folder
-				$perms = $this->getMaskFromFlags(true, $share->getCanShare()) | self::PERM_EDIT;
+				$perms = $this->getMaskFromFlags($share->getCanWrite(), $share->getCanShare()) | self::PERM_EDIT;
 			} elseif ($this->treeMapper->hasDescendant($share->getFolderId(), $type, $itemId)) {
 				$perms = $this->getMaskFromFlags($share->getCanWrite(), $share->getCanShare());
 			} else {

package.json

@@ -1,6 +1,6 @@
 {
   "name": "bookmarks",
-  "version": "13.1.2",
+  "version": "13.1.3",
   "main": "js/index.js",
   "scripts": {
     "build": "webpack --node-env production --progress --config webpack.js",

src/components/Controls.vue

@@ -100,8 +100,7 @@
 					{{ !$store.state.public? t('bookmarks', 'The RSS feed requires authentication with your Nextcloud credentials') : '' }}
 				</NcActionButton>
 			</NcActions>
-			<NcTextField
-				:value.sync="search"
+			<NcTextField :value.sync="search"
 				:label="t('bookmarks','Search')"
 				:placeholder="t('bookmarks','Search')"
 				class="inline-search"

tests/FolderControllerTest.php

@@ -234,9 +234,9 @@ public function setupPublicFolder(): void {
 	 * @throws \OCA\Bookmarks\Exception\UnsupportedOperation
 	 * @throws \OCP\AppFramework\Db\DoesNotExistException
 	 */
-	public function setupSharedFolder() {
+	public function setupSharedFolder($canWrite = true, $canShare = false) {
 		$this->authorizer->setUserId($this->userId);
-		$this->share = $this->folders->createShare($this->folder1->getId(), $this->otherUser, \OCP\Share\IShare::TYPE_USER, true, false);
+		$this->share = $this->folders->createShare($this->folder1->getId(), $this->otherUser, \OCP\Share\IShare::TYPE_USER, $canWrite, $canShare);
 	}
 
 	/**
@@ -247,7 +247,7 @@ public function setupSharedFolder() {
 	 * @throws MultipleObjectsReturnedException
 	 */
 	public function testRead(): void {
-		
+
 		$this->setupBookmarks();
 		$this->authorizer->setUserId($this->userId);
 		$output = $this->controller->getFolder($this->folder1->getId());
@@ -322,7 +322,7 @@ public function testHash(bool $useCache): void {
 	 * @throws MultipleObjectsReturnedException
 	 */
 	public function testCreate(): void {
-		
+
 		$this->setupBookmarks();
 		$this->authorizer->setUserId($this->userId);
 		$output = $this->controller->addFolder('foo', $this->folder1->getId());
@@ -424,7 +424,7 @@ public function testSharedEdit(bool $canWrite): void {
 	 * @throws MultipleObjectsReturnedException
 	 */
 	public function testDelete(): void {
-		
+
 		$this->setupBookmarks();
 		$this->authorizer->setUserId($this->userId);
 		$output = $this->controller->deleteFolder($this->folder1->getId());
@@ -442,7 +442,7 @@ public function testDelete(): void {
 	 * @throws MultipleObjectsReturnedException
 	 */
 	public function testGetFullHierarchy(): void {
-		
+
 		$this->setupBookmarks();
 		$this->authorizer->setUserId($this->userId);
 		// Using -1 here because this is the controller
@@ -466,7 +466,7 @@ public function testGetFullHierarchy(): void {
 	 * @throws MultipleObjectsReturnedException
 	 */
 	public function testSetFullHierarchy(): void {
-		
+
 		$this->setupBookmarks();
 		$this->authorizer->setUserId($this->userId);
 		$output = $this->controller->setFolderChildrenOrder($this->folder1->getId(), [
@@ -494,7 +494,7 @@ public function testSetFullHierarchy(): void {
 	 * @throws MultipleObjectsReturnedException
 	 */
 	public function testGetFolderHierarchy(): void {
-		
+
 		$this->setupBookmarks();
 		$this->authorizer->setUserId($this->userId);
 		$output = $this->controller->getFolders(-1, -1);
@@ -517,7 +517,7 @@ public function testGetFolderHierarchy(): void {
 	 * @throws MultipleObjectsReturnedException
 	 */
 	public function testReadNoauthFail(): void {
-		
+
 		$this->setupBookmarks();
 		$this->setupPublicFolder();
 		$this->authorizer->setUserId(null);
@@ -533,7 +533,7 @@ public function testReadNoauthFail(): void {
 	 * @throws MultipleObjectsReturnedException
 	 */
 	public function testCreateNoauthFail(): void {
-		
+
 		$this->setupBookmarks();
 		$this->setupPublicFolder();
 		$this->authorizer->setUserId(null);
@@ -550,7 +550,7 @@ public function testCreateNoauthFail(): void {
 	 * @throws MultipleObjectsReturnedException
 	 */
 	public function testEditNoauthFail(): void {
-		
+
 		$this->setupBookmarks();
 		$this->setupPublicFolder();
 		$this->authorizer->setUserId(null);
@@ -567,7 +567,7 @@ public function testEditNoauthFail(): void {
 	 * @throws MultipleObjectsReturnedException
 	 */
 	public function testDeleteNoauthFail(): void {
-		
+
 		$this->setupBookmarks();
 		$this->setupPublicFolder();
 		$this->authorizer->setUserId(null);
@@ -583,7 +583,7 @@ public function testDeleteNoauthFail(): void {
 	 * @throws MultipleObjectsReturnedException
 	 */
 	public function testGetFullHierarchyNoauthFail(): void {
-		
+
 		$this->setupBookmarks();
 		$this->authorizer->setUserId(null);
 		$this->authorizer->setToken(null);
@@ -598,7 +598,7 @@ public function testGetFullHierarchyNoauthFail(): void {
 	 * @throws MultipleObjectsReturnedException
 	 */
 	public function testSetFullHierarchyNoauthFail(): void {
-		
+
 		$this->setupBookmarks();
 		$this->authorizer->setUserId(null);
 		$this->authorizer->setToken(null);
@@ -616,7 +616,7 @@ public function testSetFullHierarchyNoauthFail(): void {
 	 * @throws MultipleObjectsReturnedException
 	 */
 	public function testGetFolderHierarchyNoauth(): void {
-		
+
 		$this->setupBookmarks();
 		$this->authorizer->setUserId(null);
 		$this->authorizer->setToken(null);
@@ -632,7 +632,7 @@ public function testGetFolderHierarchyNoauth(): void {
 	 * @throws MultipleObjectsReturnedException
 	 */
 	public function testReadPublic(): void {
-		
+
 		$this->setupBookmarks();
 		$this->setupPublicFolder();
 		$this->authorizer->setUserId(null);
@@ -681,7 +681,7 @@ public function testCreatePublicFail(): void {
 	 * @throws MultipleObjectsReturnedException
 	 */
 	public function testEditPublicFail(): void {
-		
+
 		$this->setupBookmarks();
 		$this->setupPublicFolder();
 		$this->authorizer->setUserId(null);
@@ -702,7 +702,7 @@ public function testEditPublicFail(): void {
 	 * @throws MultipleObjectsReturnedException
 	 */
 	public function testDeletePublicFail(): void {
-		
+
 		$this->setupBookmarks();
 		$this->setupPublicFolder();
 		$this->authorizer->setUserId(null);
@@ -721,7 +721,7 @@ public function testDeletePublicFail(): void {
 	 * @throws MultipleObjectsReturnedException
 	 */
 	public function testGetFullHierarchyPublic(): void {
-		
+
 		$this->setupBookmarks();
 		$this->setupPublicFolder();
 		$this->authorizer->setUserId(null);
@@ -770,7 +770,7 @@ public function testSetFullHierarchyPublicFail(): void {
 	 * @throws MultipleObjectsReturnedException
 	 */
 	public function testGetFolderHierarchyPublic(): void {
-		
+
 		$this->setupBookmarks();
 		$this->setupPublicFolder();
 		$this->authorizer->setUserId(null);
@@ -789,7 +789,7 @@ public function testGetFolderHierarchyPublic(): void {
 	 * @throws MultipleObjectsReturnedException
 	 */
 	public function testReadShared(): void {
-		
+
 		$this->setupBookmarks();
 		$this->setupSharedFolder();
 		$this->authorizer->setUserId($this->otherUserId);
@@ -807,7 +807,7 @@ public function testReadShared(): void {
 	 * @throws MultipleObjectsReturnedException
 	 */
 	public function testReadSharedFail(): void {
-		
+
 		$this->setupBookmarks();
 		$this->authorizer->setUserId($this->otherUserId);
 		$output = $this->otherController->getFolder($this->folder1->getId());
@@ -823,7 +823,7 @@ public function testReadSharedFail(): void {
 	 * @throws MultipleObjectsReturnedException
 	 */
 	public function testCreateShared(): void {
-		
+
 		$this->setupBookmarks();
 		$this->setupSharedFolder();
 		$this->authorizer->setUserId($this->otherUserId);
@@ -875,7 +875,7 @@ public function testEditShared(bool $canWrite): void {
 	 * @throws MultipleObjectsReturnedException
 	 */
 	public function testDeleteShared(): void {
-		
+
 		$this->setupBookmarks();
 		$this->setupSharedFolder();
 		$this->authorizer->setUserId($this->otherUserId);
@@ -887,14 +887,35 @@ public function testDeleteShared(): void {
 		$this->assertEquals('error', $data['status'], var_export($data, true));
 	}
 
+	/**
+	 * @throws AlreadyExistsError
+	 * @throws UrlParseError
+	 * @throws UserLimitExceededError
+	 * @throws \OCP\AppFramework\Db\DoesNotExistException
+	 * @throws MultipleObjectsReturnedException
+	 * @dataProvider shareCanWriteDataProvider
+	 */
+	public function testDeleteFromSharedFolder(bool $canWrite): void {
+		$this->setupBookmarks();
+		$this->setupSharedFolder($canWrite);
+		$this->authorizer->setUserId($this->otherUserId);
+		$output = $this->otherController->removeFromFolder($this->folder1->getId(), $this->bookmark1Id);
+		$data = $output->getData();
+		if ($canWrite) {
+			$this->assertEquals('success', $data['status'], var_export($data, true));
+		} else {
+			$this->assertEquals('error', $data['status'], var_export($data, true));
+		}
+	}
+
 	/**
 	 * @throws AlreadyExistsError
 	 * @throws UrlParseError
 	 * @throws UserLimitExceededError
 	 * @throws MultipleObjectsReturnedException
 	 */
 	public function testGetFullHierarchyShared(): void {
-		
+
 		$this->setupBookmarks();
 		$this->setupSharedFolder();
 		$this->authorizer->setUserId($this->otherUserId);
@@ -915,7 +936,7 @@ public function testGetFullHierarchyShared(): void {
 	 * @throws MultipleObjectsReturnedException
 	 */
 	public function testSetFullHierarchyShared(): void {
-		
+
 		$this->setupBookmarks();
 		$this->setupSharedFolder();
 
@@ -947,7 +968,7 @@ public function testSetFullHierarchyShared(): void {
 	 * @throws MultipleObjectsReturnedException
 	 */
 	public function testGetFolderHierarchyShared(): void {
-		
+
 		$this->setupBookmarks();
 		$this->setupSharedFolder();
 		$this->authorizer->setUserId($this->otherUserId);
@@ -971,7 +992,7 @@ public function testGetFolderHierarchyShared(): void {
 	 * @dataProvider shareDataProvider
 	 */
 	public function testCreateShare($participant, $type, $canWrite, $canShare): void {
-		
+
 		$this->setupBookmarks();
 		$this->authorizer->setUserid($this->userId);
 		$res = $this->controller->createShare($this->folder1->getId(), $participant, $type, $canWrite, $canShare);
@@ -992,7 +1013,7 @@ public function testCreateShare($participant, $type, $canWrite, $canShare): void
 	 * @depends      testCreateShare
 	 */
 	public function testGetShare($participant, $type, $canWrite, $canShare): void {
-		
+
 		$this->setupBookmarks();
 		$this->authorizer->setUserId($this->userId);
 		$res = $this->controller->createShare($this->folder1->getId(), $participant, $type, $canWrite, $canShare);
@@ -1020,7 +1041,7 @@ public function testGetShare($participant, $type, $canWrite, $canShare): void {
 	 * @depends      testCreateShare
 	 */
 	public function testEditShare($participant, $type, $canWrite, $canShare): void {
-		
+
 		$this->setupBookmarks();
 		$this->authorizer->setUserId($this->userId);
 		$res = $this->controller->createShare($this->folder1->getId(), $participant, $type, $canWrite, $canShare);
@@ -1056,7 +1077,7 @@ public function testEditShare($participant, $type, $canWrite, $canShare): void {
 	 * @depends      testCreateShare
 	 */
 	public function testDeleteShareOwner($participant, $type, $canWrite, $canShare): void {
-		
+
 		$this->setupBookmarks();
 		$this->authorizer->setUserId($this->userId);
 		$res = $this->controller->createShare($this->folder1->getId(), $participant, $type, $canWrite, $canShare);
@@ -1083,7 +1104,7 @@ public function testDeleteShareOwner($participant, $type, $canWrite, $canShare):
 	 * @depends      testCreateShare
 	 */
 	public function testDeleteShareSharee($participant, $type, $canWrite, $canShare): void {
-		
+
 		$this->setupBookmarks();
 		$this->authorizer->setUserId($this->userId);
 		$res = $this->controller->createShare($this->folder1->getId(), $participant, $type, $canWrite, $canShare);