Support CRUD share permissions

Signed-off-by: Louis Chemineau <louis@chmn.me>

apps/files_sharing/lib/Controller/ShareAPIController.php

@@ -1004,10 +1004,9 @@ public function getInheritedShares(string $path): DataResponse {
 	}
 
 	/**
-	 * Check wether a permissions set contains some permissions.
-	 * @return bool
+	 * Check whether a set of permissions contains the permissions to check.
 	 */
-	private function hasPermission(int $permissionsSet, int $permissionsToCheck) {
+	private function hasPermission(int $permissionsSet, int $permissionsToCheck): bool {
 		return ($permissionsSet & $permissionsToCheck) === $permissionsToCheck;
 	}
 

apps/files_sharing/src/components/SharePermissionsEditor.vue

@@ -77,12 +77,12 @@
 				<ActionCheckbox :checked="shareHasPermissions(atomicPermissions.CREATE)"
 					:disabled="saving || !canToggleSharePermissions(atomicPermissions.CREATE)"
 					@update:checked="toggleSharePermissions(atomicPermissions.CREATE)">
-					{{ t('files_sharing', 'Create') }}
+					{{ t('files_sharing', 'Upload') }}
 				</ActionCheckbox>
 				<ActionCheckbox :checked="shareHasPermissions(atomicPermissions.UPDATE)"
 					:disabled="saving || !canToggleSharePermissions(atomicPermissions.UPDATE)"
 					@update:checked="toggleSharePermissions(atomicPermissions.UPDATE)">
-					{{ t('files_sharing', 'Update') }}
+					{{ t('files_sharing', 'Edit') }}
 				</ActionCheckbox>
 				<ActionCheckbox :checked="shareHasPermissions(atomicPermissions.DELETE)"
 					:disabled="saving || !canToggleSharePermissions(atomicPermissions.DELETE)"
@@ -94,7 +94,7 @@
 					<template #icon>
 						<ChevronLeft />
 					</template>
-					{{ t('files_sharing', 'Bundle permissions') }}
+					{{ t('files_sharing', 'Bundled permissions') }}
 				</ActionButton>
 			</span>
 		</template>
@@ -116,8 +116,8 @@ import {
 	canTogglePermissions,
 } from '../lib/SharePermissionsToolBox'
 
-import Tune from 'vue-material-design-icons/Tune.vue'
-import ChevronLeft from 'vue-material-design-icons/ChevronLeft.vue'
+import Tune from 'vue-material-design-icons/Tune'
+import ChevronLeft from 'vue-material-design-icons/ChevronLeft'
 
 export default {
 	name: 'SharePermissionsEditor',
@@ -134,7 +134,7 @@ export default {
 
 	data() {
 		return {
-			randomFormName: Math.random().toString(27).substr(2),
+			randomFormName: Math.random().toString(27).substring(2),
 
 			showCustomPermissionsForm: false,
 
@@ -155,11 +155,11 @@ export default {
 				.map(permission => {
 					switch (permission) {
 					case this.atomicPermissions.CREATE:
-						return this.t('files_sharing', 'Create')
+						return this.t('files_sharing', 'Upload')
 					case this.atomicPermissions.READ:
 						return this.t('files_sharing', 'Read')
 					case this.atomicPermissions.UPDATE:
-						return this.t('files_sharing', 'Update')
+						return this.t('files_sharing', 'Edit')
 					case this.atomicPermissions.DELETE:
 						return this.t('files_sharing', 'Delete')
 					default:
@@ -170,7 +170,7 @@ export default {
 		},
 
 		/**
-		 * Return wether the share's permission is a bundle.
+		 * Return whether the share's permission is a bundle.
 		 *
 		 * @return {boolean}
 		 */
@@ -182,7 +182,7 @@ export default {
 		},
 
 		/**
-		 * Return wether the share's permission is valid.
+		 * Return whether the share's permission is valid.
 		 *
 		 * @return {boolean}
 		 */
@@ -218,7 +218,7 @@ export default {
 
 	methods: {
 		/**
-		 * Return wether the share has the exact given permissions.
+		 * Return whether the share has the exact given permissions.
 		 *
 		 * @param {number} permissions - the permissions to check.
 		 *
@@ -230,7 +230,7 @@ export default {
 		},
 
 		/**
-		 * Return wether the share has the given permissions.
+		 * Return whether the share has the given permissions.
 		 *
 		 * @param {number} permissions - the permissions to check.
 		 *
@@ -248,12 +248,12 @@ export default {
 		 * @return {void}
 		 */
 		setSharePermissions(permissions) {
-			this.share.permissions = parseInt(permissions, 10)
+			this.share.permissions = permissions
 			this.queueUpdate('permissions')
 		},
 
 		/**
-		 * Return wether some given permissions can be toggled.
+		 * Return whether some given permissions can be toggled.
 		 *
 		 * @param {number} permissionsToToggle - the permissions to toggle.
 		 *

apps/files_sharing/src/lib/SharePermissionsToolBox.js

@@ -37,7 +37,7 @@ export const BUNDLED_PERMISSIONS = {
 }
 
 /**
- * Return wether a given permissions set contains some permissions.
+ * Return whether a given permissions set contains some permissions.
  *
  * @param {number} initialPermissionSet - the permissions set.
  * @param {number} permissionsToCheck - the permissions to check.
@@ -48,7 +48,7 @@ export function hasPermissions(initialPermissionSet, permissionsToCheck) {
 }
 
 /**
- * Return wether a given permissions set is valid.
+ * Return whether a given permissions set is valid.
  *
  * @param {number} permissionsSet - the permissions set.
  *
@@ -111,7 +111,7 @@ export function togglePermissions(initialPermissionSet, permissionsToToggle) {
 }
 
 /**
- * Return wether some given permissions can be toggled from a permission set.
+ * Return whether some given permissions can be toggled from a permission set.
  *
  * @param {number} permissionSet - the initial permissions set.
  * @param {number} permissionsToToggle - the permissions to toggle.

apps/files_sharing/tests/Controller/ShareAPIControllerTest.php

@@ -2793,6 +2793,99 @@ public function testUpdateLinkShareEnablePublicUpload($permissions, $publicUploa
 	}
 
 
+	public function publicLinkValidPermissionsProvider() {
+		return [
+			[\OCP\Constants::PERMISSION_CREATE],
+			[\OCP\Constants::PERMISSION_READ],
+			[\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_UPDATE],
+			[\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_DELETE],
+			[\OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE],
+		];
+	}
+
+	/**
+	 * @dataProvider publicLinkValidPermissionsProvider
+	 */
+	public function testUpdateLinkShareSetCRUDPermissions($permissions) {
+		$ocs = $this->mockFormatShare();
+
+		$folder = $this->getMockBuilder(Folder::class)->getMock();
+		$folder->method('getId')
+			->willReturn(42);
+
+		$share = \OC::$server->getShareManager()->newShare();
+		$share->setPermissions(\OCP\Constants::PERMISSION_ALL)
+			->setSharedBy($this->currentUser)
+			->setShareType(IShare::TYPE_LINK)
+			->setPassword('password')
+			->setNode($folder);
+
+		$this->shareManager->method('getShareById')->with('ocinternal:42')->willReturn($share);
+		$this->shareManager->method('shareApiLinkAllowPublicUpload')->willReturn(true);
+		$this->shareManager->method('getSharedWith')->willReturn([]);
+
+		$this->shareManager
+			->expects($this->any())
+			->method('updateShare')
+			->willReturnArgument(0);
+
+		$userFolder = $this->createMock(Folder::class);
+		$this->rootFolder->method('getUserFolder')
+			->with($this->currentUser)
+			->willReturn($userFolder);
+
+		$userFolder->method('getById')
+			->with(42)
+			->willReturn([$folder]);
+
+		$mountPoint = $this->createMock(IMountPoint::class);
+		$folder->method('getMountPoint')
+			->willReturn($mountPoint);
+		$mountPoint->method('getStorageRootId')
+			->willReturn(42);
+
+		$expected = new DataResponse([]);
+		$result = $ocs->updateShare(42, $permissions, 'password', null, 'true', null);
+
+		$this->assertInstanceOf(get_class($expected), $result);
+		$this->assertEquals($expected->getData(), $result->getData());
+	}
+
+	public function publicLinkInvalidPermissionsProvider1() {
+		return [
+			[\OCP\Constants::PERMISSION_DELETE],
+			[\OCP\Constants::PERMISSION_UPDATE],
+			[\OCP\Constants::PERMISSION_SHARE],
+		];
+	}
+
+	/**
+	 * @dataProvider publicLinkInvalidPermissionsProvider1
+	 */
+	public function testUpdateLinkShareSetInvalidCRUDPermissions1($permissions) {
+		$this->expectException(\OCP\AppFramework\OCS\OCSBadRequestException::class);
+		$this->expectExceptionMessage('Share must at least have READ or CREATE permissions');
+
+		$this->testUpdateLinkShareSetCRUDPermissions($permissions);
+	}
+
+	public function publicLinkInvalidPermissionsProvider2() {
+		return [
+			[\OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_DELETE],
+			[\OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE],
+		];
+	}
+
+	/**
+	 * @dataProvider publicLinkInvalidPermissionsProvider2
+	 */
+	public function testUpdateLinkShareSetInvalidCRUDPermissions2($permissions) {
+		$this->expectException(\OCP\AppFramework\OCS\OCSBadRequestException::class);
+		$this->expectExceptionMessage('Share must have READ permission if UPDATE or DELETE permission is set.');
+
+		$this->testUpdateLinkShareSetCRUDPermissions($permissions);
+	}
+
 	public function testUpdateLinkShareInvalidDate() {
 		$this->expectException(\OCP\AppFramework\OCS\OCSBadRequestException::class);
 		$this->expectExceptionMessage('Invalid date. Format must be YYYY-MM-DD');