From da435b1e67930e85fc30fd1b94c6214caa086f4f Mon Sep 17 00:00:00 2001 From: Louis Chemineau Date: Wed, 2 Feb 2022 16:10:52 +0100 Subject: Support CRUD share permissions Signed-off-by: Louis Chemineau --- .../lib/Controller/ShareAPIController.php | 27 +- .../src/components/SharePermissionsEditor.vue | 291 +++++++++++++++++++++ .../src/components/SharingEntryLink.vue | 119 +-------- .../src/lib/SharePermissionsToolBox.js | 123 +++++++++ .../src/lib/SharePermissionsToolBox.spec.js | 96 +++++++ apps/files_sharing/src/mixins/ShareRequests.js | 5 +- .../tests/Controller/ShareAPIControllerTest.php | 93 +++++++ 7 files changed, 633 insertions(+), 121 deletions(-) create mode 100644 apps/files_sharing/src/components/SharePermissionsEditor.vue create mode 100644 apps/files_sharing/src/lib/SharePermissionsToolBox.js create mode 100644 apps/files_sharing/src/lib/SharePermissionsToolBox.spec.js (limited to 'apps') diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index ff134f61e17..fef71a868d5 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -1003,6 +1003,13 @@ class ShareAPIController extends OCSController { return new DataResponse(array_values($shares)); } + /** + * Check whether a set of permissions contains the permissions to check. + */ + private function hasPermission(int $permissionsSet, int $permissionsToCheck): bool { + return ($permissionsSet & $permissionsToCheck) === $permissionsToCheck; + } + /** * @NoAdminRequired @@ -1104,16 +1111,16 @@ class ShareAPIController extends OCSController { $newPermissions = $newPermissions & ~Constants::PERMISSION_SHARE; } - if ($newPermissions !== null && - !in_array($newPermissions, [ - Constants::PERMISSION_READ, - Constants::PERMISSION_READ | Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE, // legacy - Constants::PERMISSION_READ | Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE | Constants::PERMISSION_DELETE, // correct - Constants::PERMISSION_CREATE, // hidden file list - Constants::PERMISSION_READ | Constants::PERMISSION_UPDATE, // allow to edit single files - ], true) - ) { - throw new OCSBadRequestException($this->l->t('Cannot change permissions for public share links')); + if ($newPermissions !== null) { + if (!$this->hasPermission($newPermissions, Constants::PERMISSION_READ) && !$this->hasPermission($newPermissions, Constants::PERMISSION_CREATE)) { + throw new OCSBadRequestException($this->l->t('Share must at least have READ or CREATE permissions')); + } + + if (!$this->hasPermission($newPermissions, Constants::PERMISSION_READ) && ( + $this->hasPermission($newPermissions, Constants::PERMISSION_UPDATE) || $this->hasPermission($newPermissions, Constants::PERMISSION_DELETE) + )) { + throw new OCSBadRequestException($this->l->t('Share must have READ permission if UPDATE or DELETE permission is set.')); + } } if ( diff --git a/apps/files_sharing/src/components/SharePermissionsEditor.vue b/apps/files_sharing/src/components/SharePermissionsEditor.vue new file mode 100644 index 00000000000..41c8c0b5cce --- /dev/null +++ b/apps/files_sharing/src/components/SharePermissionsEditor.vue @@ -0,0 +1,291 @@ + + + + + + diff --git a/apps/files_sharing/src/components/SharingEntryLink.vue b/apps/files_sharing/src/components/SharingEntryLink.vue index 9fefa9b6f90..52215d37ec8 100644 --- a/apps/files_sharing/src/components/SharingEntryLink.vue +++ b/apps/files_sharing/src/components/SharingEntryLink.vue @@ -150,39 +150,12 @@ @submit="onLabelSubmit"> {{ t('files_sharing', 'Share label') }} - - - - - - {{ t('files_sharing', 'Allow editing') }} - + + + + + + + * + * @author Louis Chmn + * + * @license AGPL-3.0-or-later + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +export const ATOMIC_PERMISSIONS = { + NONE: 0, + READ: 1, + UPDATE: 2, + CREATE: 4, + DELETE: 8, + SHARE: 16, +} + +export const BUNDLED_PERMISSIONS = { + READ_ONLY: ATOMIC_PERMISSIONS.READ, + UPLOAD_AND_UPDATE: ATOMIC_PERMISSIONS.READ | ATOMIC_PERMISSIONS.UPDATE | ATOMIC_PERMISSIONS.CREATE | ATOMIC_PERMISSIONS.DELETE, + FILE_DROP: ATOMIC_PERMISSIONS.CREATE, + ALL: ATOMIC_PERMISSIONS.UPDATE | ATOMIC_PERMISSIONS.CREATE | ATOMIC_PERMISSIONS.READ | ATOMIC_PERMISSIONS.DELETE | ATOMIC_PERMISSIONS.SHARE, +} + +/** + * Return whether a given permissions set contains some permissions. + * + * @param {number} initialPermissionSet - the permissions set. + * @param {number} permissionsToCheck - the permissions to check. + * @return {boolean} + */ +export function hasPermissions(initialPermissionSet, permissionsToCheck) { + return initialPermissionSet !== ATOMIC_PERMISSIONS.NONE && (initialPermissionSet & permissionsToCheck) === permissionsToCheck +} + +/** + * Return whether a given permissions set is valid. + * + * @param {number} permissionsSet - the permissions set. + * + * @return {boolean} + */ +export function permissionsSetIsValid(permissionsSet) { + // Must have at least READ or CREATE permission. + if (!hasPermissions(permissionsSet, ATOMIC_PERMISSIONS.READ) && !hasPermissions(permissionsSet, ATOMIC_PERMISSIONS.CREATE)) { + return false + } + + // Must have READ permission if have UPDATE or DELETE. + if (!hasPermissions(permissionsSet, ATOMIC_PERMISSIONS.READ) && ( + hasPermissions(permissionsSet, ATOMIC_PERMISSIONS.UPDATE) || hasPermissions(permissionsSet, ATOMIC_PERMISSIONS.DELETE) + )) { + return false + } + + return true +} + +/** + * Add some permissions to an initial set of permissions. + * + * @param {number} initialPermissionSet - the initial permissions. + * @param {number} permissionsToAdd - the permissions to add. + * + * @return {number} + */ +export function addPermissions(initialPermissionSet, permissionsToAdd) { + return initialPermissionSet | permissionsToAdd +} + +/** + * Remove some permissions from an initial set of permissions. + * + * @param {number} initialPermissionSet - the initial permissions. + * @param {number} permissionsToSubtract - the permissions to remove. + * + * @return {number} + */ +export function subtractPermissions(initialPermissionSet, permissionsToSubtract) { + return initialPermissionSet & ~permissionsToSubtract +} + +/** + * Toggle some permissions from an initial set of permissions. + * + * @param {number} initialPermissionSet - the permissions set. + * @param {number} permissionsToToggle - the permissions to toggle. + * + * @return {number} + */ +export function togglePermissions(initialPermissionSet, permissionsToToggle) { + if (hasPermissions(initialPermissionSet, permissionsToToggle)) { + return subtractPermissions(initialPermissionSet, permissionsToToggle) + } else { + return addPermissions(initialPermissionSet, permissionsToToggle) + } +} + +/** + * 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. + * + * @return {boolean} + */ +export function canTogglePermissions(permissionSet, permissionsToToggle) { + return permissionsSetIsValid(togglePermissions(permissionSet, permissionsToToggle)) +} diff --git a/apps/files_sharing/src/lib/SharePermissionsToolBox.spec.js b/apps/files_sharing/src/lib/SharePermissionsToolBox.spec.js new file mode 100644 index 00000000000..7ae29c7134a --- /dev/null +++ b/apps/files_sharing/src/lib/SharePermissionsToolBox.spec.js @@ -0,0 +1,96 @@ +/** + * @copyright 2022 Louis Chmn + * + * @author Louis Chmn + * + * @license AGPL-3.0-or-later + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +import { + ATOMIC_PERMISSIONS, + BUNDLED_PERMISSIONS, + addPermissions, + subtractPermissions, + hasPermissions, + permissionsSetIsValid, + togglePermissions, + canTogglePermissions, +} from '../lib/SharePermissionsToolBox' + +describe('SharePermissionsToolBox', () => { + test('Adding permissions', () => { + expect(addPermissions(ATOMIC_PERMISSIONS.NONE, ATOMIC_PERMISSIONS.NONE)).toBe(ATOMIC_PERMISSIONS.NONE) + expect(addPermissions(ATOMIC_PERMISSIONS.NONE, ATOMIC_PERMISSIONS.READ)).toBe(ATOMIC_PERMISSIONS.READ) + expect(addPermissions(ATOMIC_PERMISSIONS.READ, ATOMIC_PERMISSIONS.READ)).toBe(ATOMIC_PERMISSIONS.READ) + expect(addPermissions(ATOMIC_PERMISSIONS.READ, ATOMIC_PERMISSIONS.UPDATE)).toBe(ATOMIC_PERMISSIONS.READ | ATOMIC_PERMISSIONS.UPDATE) + expect(addPermissions(ATOMIC_PERMISSIONS.READ | ATOMIC_PERMISSIONS.UPDATE, ATOMIC_PERMISSIONS.CREATE | ATOMIC_PERMISSIONS.DELETE | ATOMIC_PERMISSIONS.SHARE)).toBe(BUNDLED_PERMISSIONS.ALL) + expect(addPermissions(BUNDLED_PERMISSIONS.ALL, ATOMIC_PERMISSIONS.READ)).toBe(BUNDLED_PERMISSIONS.ALL) + expect(addPermissions(BUNDLED_PERMISSIONS.ALL, ATOMIC_PERMISSIONS.NONE)).toBe(BUNDLED_PERMISSIONS.ALL) + }) + + test('Subtract permissions', () => { + expect(subtractPermissions(ATOMIC_PERMISSIONS.READ, ATOMIC_PERMISSIONS.NONE)).toBe(ATOMIC_PERMISSIONS.READ) + expect(subtractPermissions(ATOMIC_PERMISSIONS.READ, ATOMIC_PERMISSIONS.READ)).toBe(ATOMIC_PERMISSIONS.NONE) + expect(subtractPermissions(ATOMIC_PERMISSIONS.READ, ATOMIC_PERMISSIONS.UPDATE)).toBe(ATOMIC_PERMISSIONS.READ) + expect(subtractPermissions(ATOMIC_PERMISSIONS.READ | ATOMIC_PERMISSIONS.UPDATE, ATOMIC_PERMISSIONS.UPDATE)).toBe(ATOMIC_PERMISSIONS.READ) + expect(subtractPermissions(ATOMIC_PERMISSIONS.READ | ATOMIC_PERMISSIONS.UPDATE, ATOMIC_PERMISSIONS.CREATE | ATOMIC_PERMISSIONS.DELETE)).toBe(ATOMIC_PERMISSIONS.READ | ATOMIC_PERMISSIONS.UPDATE) + expect(subtractPermissions(ATOMIC_PERMISSIONS.READ | ATOMIC_PERMISSIONS.UPDATE, ATOMIC_PERMISSIONS.UPDATE | ATOMIC_PERMISSIONS.DELETE)).toBe(ATOMIC_PERMISSIONS.READ) + expect(subtractPermissions(BUNDLED_PERMISSIONS.ALL, ATOMIC_PERMISSIONS.READ)).toBe(ATOMIC_PERMISSIONS.UPDATE | ATOMIC_PERMISSIONS.CREATE | ATOMIC_PERMISSIONS.DELETE | ATOMIC_PERMISSIONS.SHARE) + }) + + test('Has permissions', () => { + expect(hasPermissions(ATOMIC_PERMISSIONS.NONE, ATOMIC_PERMISSIONS.READ)).toBe(false) + expect(hasPermissions(ATOMIC_PERMISSIONS.READ, ATOMIC_PERMISSIONS.NONE)).toBe(true) + expect(hasPermissions(BUNDLED_PERMISSIONS.READ_ONLY, ATOMIC_PERMISSIONS.READ)).toBe(true) + expect(hasPermissions(BUNDLED_PERMISSIONS.READ_ONLY, ATOMIC_PERMISSIONS.UPDATE)).toBe(false) + expect(hasPermissions(BUNDLED_PERMISSIONS.READ_ONLY, ATOMIC_PERMISSIONS.DELETE)).toBe(false) + expect(hasPermissions(BUNDLED_PERMISSIONS.ALL, ATOMIC_PERMISSIONS.DELETE)).toBe(true) + }) + + test('Toggle permissions', () => { + expect(togglePermissions(BUNDLED_PERMISSIONS.ALL, BUNDLED_PERMISSIONS.UPLOAD_AND_UPDATE)).toBe(ATOMIC_PERMISSIONS.SHARE) + expect(togglePermissions(BUNDLED_PERMISSIONS.ALL, BUNDLED_PERMISSIONS.FILE_DROP)).toBe(ATOMIC_PERMISSIONS.READ | ATOMIC_PERMISSIONS.UPDATE | ATOMIC_PERMISSIONS.DELETE | ATOMIC_PERMISSIONS.SHARE) + expect(togglePermissions(BUNDLED_PERMISSIONS.ALL, ATOMIC_PERMISSIONS.NONE)).toBe(BUNDLED_PERMISSIONS.ALL) + expect(togglePermissions(ATOMIC_PERMISSIONS.NONE, BUNDLED_PERMISSIONS.ALL)).toBe(BUNDLED_PERMISSIONS.ALL) + expect(togglePermissions(ATOMIC_PERMISSIONS.READ, BUNDLED_PERMISSIONS.ALL)).toBe(BUNDLED_PERMISSIONS.ALL) + }) + + test('Permissions set is valid', () => { + expect(permissionsSetIsValid(ATOMIC_PERMISSIONS.NONE)).toBe(false) + expect(permissionsSetIsValid(ATOMIC_PERMISSIONS.READ)).toBe(true) + expect(permissionsSetIsValid(ATOMIC_PERMISSIONS.CREATE)).toBe(true) + expect(permissionsSetIsValid(ATOMIC_PERMISSIONS.UPDATE)).toBe(false) + expect(permissionsSetIsValid(ATOMIC_PERMISSIONS.DELETE)).toBe(false) + expect(permissionsSetIsValid(ATOMIC_PERMISSIONS.READ | ATOMIC_PERMISSIONS.UPDATE)).toBe(true) + expect(permissionsSetIsValid(ATOMIC_PERMISSIONS.READ | ATOMIC_PERMISSIONS.DELETE)).toBe(true) + expect(permissionsSetIsValid(ATOMIC_PERMISSIONS.CREATE | ATOMIC_PERMISSIONS.UPDATE)).toBe(false) + expect(permissionsSetIsValid(ATOMIC_PERMISSIONS.CREATE | ATOMIC_PERMISSIONS.DELETE)).toBe(false) + expect(permissionsSetIsValid(ATOMIC_PERMISSIONS.READ | ATOMIC_PERMISSIONS.CREATE | ATOMIC_PERMISSIONS.UPDATE)).toBe(true) + expect(permissionsSetIsValid(ATOMIC_PERMISSIONS.READ | ATOMIC_PERMISSIONS.CREATE | ATOMIC_PERMISSIONS.DELETE)).toBe(true) + }) + + test('Toggle permissions', () => { + expect(canTogglePermissions(ATOMIC_PERMISSIONS.READ, ATOMIC_PERMISSIONS.READ)).toBe(false) + expect(canTogglePermissions(ATOMIC_PERMISSIONS.CREATE, ATOMIC_PERMISSIONS.READ)).toBe(true) + expect(canTogglePermissions(ATOMIC_PERMISSIONS.READ | ATOMIC_PERMISSIONS.UPDATE, ATOMIC_PERMISSIONS.READ)).toBe(false) + expect(canTogglePermissions(ATOMIC_PERMISSIONS.READ | ATOMIC_PERMISSIONS.DELETE, ATOMIC_PERMISSIONS.READ)).toBe(false) + expect(canTogglePermissions(ATOMIC_PERMISSIONS.READ | ATOMIC_PERMISSIONS.CREATE | ATOMIC_PERMISSIONS.UPDATE, ATOMIC_PERMISSIONS.READ)).toBe(false) + expect(canTogglePermissions(ATOMIC_PERMISSIONS.READ | ATOMIC_PERMISSIONS.CREATE | ATOMIC_PERMISSIONS.DELETE, ATOMIC_PERMISSIONS.READ)).toBe(false) + expect(canTogglePermissions(ATOMIC_PERMISSIONS.READ | ATOMIC_PERMISSIONS.CREATE | ATOMIC_PERMISSIONS.UPDATE, ATOMIC_PERMISSIONS.CREATE)).toBe(true) + expect(canTogglePermissions(ATOMIC_PERMISSIONS.READ | ATOMIC_PERMISSIONS.CREATE | ATOMIC_PERMISSIONS.DELETE, ATOMIC_PERMISSIONS.CREATE)).toBe(true) + }) +}) diff --git a/apps/files_sharing/src/mixins/ShareRequests.js b/apps/files_sharing/src/mixins/ShareRequests.js index 15585ec49cf..bc6e3bf1644 100644 --- a/apps/files_sharing/src/mixins/ShareRequests.js +++ b/apps/files_sharing/src/mixins/ShareRequests.js @@ -31,9 +31,6 @@ import axios from '@nextcloud/axios' import Share from '../models/Share' const shareUrl = generateOcsUrl('apps/files_sharing/api/v1/shares') -const headers = { - 'Content-Type': 'application/x-www-form-urlencoded;charset=UTF-8', -} export default { methods: { @@ -103,7 +100,7 @@ export default { */ async updateShare(id, properties) { try { - const request = await axios.put(shareUrl + `/${id}`, properties, headers) + const request = await axios.put(shareUrl + `/${id}`, properties) if (!request?.data?.ocs) { throw request } diff --git a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php index 0a837400725..8bea67dff05 100644 --- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php @@ -2793,6 +2793,99 @@ class ShareAPIControllerTest extends TestCase { } + 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'); -- cgit v1.2.3