From a7abe2c35a170d60b6860aa929fe9c3c658c4bc8 Mon Sep 17 00:00:00 2001 From: Jan-Philipp Litza Date: Wed, 16 Nov 2022 15:25:10 +0100 Subject: [PATCH] Honor permissions of new link share via OCS API The API currently overrides the supplied permissions with "read only" when a file is shared via link. It allows to update the permissions later, however. This keeps the default to "read only" but honors the permissions supplied by API call if any. Signed-off-by: Jan-Philipp Litza --- .../lib/Controller/ShareAPIController.php | 11 ++++++++--- apps/files_sharing/tests/ApiTest.php | 3 +-- .../tests/Controller/ShareAPIControllerTest.php | 8 ++++---- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 43f43d10731a3..ab318a81fc232 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -478,7 +478,14 @@ public function createShare( $share = $this->shareManager->newShare(); if ($permissions === null) { - $permissions = (int)$this->config->getAppValue('core', 'shareapi_default_permissions', (string)Constants::PERMISSION_ALL); + if ($shareType === IShare::TYPE_LINK + || $shareType === IShare::TYPE_EMAIL) { + + // to keep legacy default behaviour, we ignore the setting below for link shares + $permissions = Constants::PERMISSION_READ; + } else { + $permissions = (int)$this->config->getAppValue('core', 'shareapi_default_permissions', (string)Constants::PERMISSION_ALL); + } } // Verify path @@ -581,8 +588,6 @@ public function createShare( Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE | Constants::PERMISSION_DELETE; - } else { - $permissions = Constants::PERMISSION_READ; } // TODO: It might make sense to have a dedicated setting to allow/deny converting link shares into federated ones diff --git a/apps/files_sharing/tests/ApiTest.php b/apps/files_sharing/tests/ApiTest.php index 9b88297a30959..4d7389be24ed0 100644 --- a/apps/files_sharing/tests/ApiTest.php +++ b/apps/files_sharing/tests/ApiTest.php @@ -211,8 +211,7 @@ public function testCreateShareLink() { $ocs->cleanup(); $data = $result->getData(); - $this->assertEquals(\OCP\Constants::PERMISSION_READ | - \OCP\Constants::PERMISSION_SHARE, + $this->assertEquals(\OCP\Constants::PERMISSION_ALL, $data['permissions']); $this->assertEmpty($data['expiration']); $this->assertTrue(is_string($data['token'])); diff --git a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php index 6f053eef12464..6405181d0dcc3 100644 --- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php @@ -2055,7 +2055,7 @@ public function testCreateShareLinkPassword() { $this->callback(function (\OCP\Share\IShare $share) use ($path) { return $share->getNode() === $path && $share->getShareType() === IShare::TYPE_LINK && - $share->getPermissions() === \OCP\Constants::PERMISSION_READ && + $share->getPermissions() === \OCP\Constants::PERMISSION_ALL && $share->getSharedBy() === 'currentUser' && $share->getPassword() === 'password' && $share->getExpirationDate() === null; @@ -2095,7 +2095,7 @@ public function testCreateShareLinkSendPasswordByTalk() { $this->callback(function (\OCP\Share\IShare $share) use ($path) { return $share->getNode() === $path && $share->getShareType() === IShare::TYPE_LINK && - $share->getPermissions() === \OCP\Constants::PERMISSION_READ && + $share->getPermissions() === \OCP\Constants::PERMISSION_ALL && $share->getSharedBy() === 'currentUser' && $share->getPassword() === 'password' && $share->getSendPasswordByTalk() === true && @@ -2179,7 +2179,7 @@ public function testCreateShareValidExpireDate() { return $share->getNode() === $path && $share->getShareType() === IShare::TYPE_LINK && - $share->getPermissions() === \OCP\Constants::PERMISSION_READ && + $share->getPermissions() === \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_SHARE && $share->getSharedBy() === 'currentUser' && $share->getPassword() === null && $share->getExpirationDate() == $date; @@ -2187,7 +2187,7 @@ public function testCreateShareValidExpireDate() { )->willReturnArgument(0); $expected = new DataResponse([]); - $result = $ocs->createShare('valid-path', \OCP\Constants::PERMISSION_ALL, IShare::TYPE_LINK, null, 'false', '', null, '2000-01-01'); + $result = $ocs->createShare('valid-path', null, IShare::TYPE_LINK, null, 'false', '', null, '2000-01-01'); $this->assertInstanceOf(get_class($expected), $result); $this->assertEquals($expected->getData(), $result->getData());