From 703fca8ec6290a9b8404b6105ba2b4c525d3ee5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 5 Mar 2020 14:56:04 +0100 Subject: [PATCH 1/7] Set proper share type when converting link shares to federated shares MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- .../lib/Controller/MountPublicLinkController.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apps/federatedfilesharing/lib/Controller/MountPublicLinkController.php b/apps/federatedfilesharing/lib/Controller/MountPublicLinkController.php index d54f24d97fe8c..63f5350754db4 100644 --- a/apps/federatedfilesharing/lib/Controller/MountPublicLinkController.php +++ b/apps/federatedfilesharing/lib/Controller/MountPublicLinkController.php @@ -48,6 +48,7 @@ use OCP\IUserSession; use OCP\Share\IManager; use OCP\Util; +use OCP\Share\IShare; /** * Class MountPublicLinkController @@ -161,6 +162,7 @@ public function createFederatedShare($shareWith, $token, $password = '') { } $share->setSharedWith($shareWith); + $share->setShareType(IShare::TYPE_REMOTE); try { $this->federatedShareProvider->create($share); From f50bf10bec8fc1ccd834c16379ddb502972a2174 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 5 Mar 2020 14:56:31 +0100 Subject: [PATCH 2/7] Link shares have reshare permission if outgoing federated shares are enabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- .../lib/Controller/ShareAPIController.php | 13 ++++++++----- apps/files_sharing/tests/ApiTest.php | 7 +++++-- lib/private/Share20/Manager.php | 5 ----- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 09489861e1c1b..299744d6dff19 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -470,15 +470,18 @@ public function createShare( throw new OCSNotFoundException($this->l->t('Public upload is only possible for publicly shared folders')); } - $share->setPermissions( - Constants::PERMISSION_READ | + $permissions = Constants::PERMISSION_READ | Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE | - Constants::PERMISSION_DELETE - ); + Constants::PERMISSION_DELETE; } else { - $share->setPermissions(Constants::PERMISSION_READ); + $permissions = Constants::PERMISSION_READ; + } + // TODO: It might make sense to have a dedicated setting to allow/deny converting link shares into federated ones + if ($this->shareManager->outgoingServer2ServerSharesAllowed()) { + $permissions |= Constants::PERMISSION_SHARE; } + $share->setPermissions($permissions); // Set password if ($password !== '') { diff --git a/apps/files_sharing/tests/ApiTest.php b/apps/files_sharing/tests/ApiTest.php index 0616daed62db6..22cd32c487644 100644 --- a/apps/files_sharing/tests/ApiTest.php +++ b/apps/files_sharing/tests/ApiTest.php @@ -203,7 +203,9 @@ public function testCreateShareLink() { $ocs->cleanup(); $data = $result->getData(); - $this->assertEquals(1, $data['permissions']); + $this->assertEquals(\OCP\Constants::PERMISSION_READ | + \OCP\Constants::PERMISSION_SHARE, + $data['permissions']); $this->assertEmpty($data['expiration']); $this->assertTrue(is_string($data['token'])); @@ -228,7 +230,8 @@ public function testCreateShareLinkPublicUpload() { \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE | - \OCP\Constants::PERMISSION_DELETE, + \OCP\Constants::PERMISSION_DELETE | + \OCP\Constants::PERMISSION_SHARE, $data['permissions'] ); $this->assertEmpty($data['expiration']); diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index f13878d71b478..6a6b52f2c802d 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -537,11 +537,6 @@ protected function linkCreateChecks(\OCP\Share\IShare $share) { throw new \Exception('Link sharing is not allowed'); } - // Link shares by definition can't have share permissions - if ($share->getPermissions() & \OCP\Constants::PERMISSION_SHARE) { - throw new \InvalidArgumentException('Link shares can’t have reshare permissions'); - } - // Check if public upload is allowed if (!$this->shareApiLinkAllowPublicUpload() && ($share->getPermissions() & (\OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE))) { From 1808cf93a26b1f2f37fce5b6b6f6c48984579461 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 9 Apr 2020 11:36:49 +0200 Subject: [PATCH 3/7] Remove unneeded test since links have resharing permissions by default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- tests/lib/Share20/ManagerTest.php | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index cf22ff53bab0b..da5890f8372f3 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -1361,24 +1361,6 @@ public function testLinkCreateChecksNoLinkSharesAllowed() { self::invokePrivate($this->manager, 'linkCreateChecks', [$share]); } - /** - * @expectedException Exception - * @expectedExceptionMessage Link shares can’t have reshare permissions - */ - public function testLinkCreateChecksSharePermissions() { - $share = $this->manager->newShare(); - - $share->setPermissions(\OCP\Constants::PERMISSION_SHARE); - - $this->config - ->method('getAppValue') - ->will($this->returnValueMap([ - ['core', 'shareapi_allow_links', 'yes', 'yes'], - ])); - - self::invokePrivate($this->manager, 'linkCreateChecks', [$share]); - } - /** * @expectedException Exception * @expectedExceptionMessage Public upload is not allowed From 6941bbebbfa1a6f6c8aa4a8a113659312daf0ee2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 23 Apr 2020 16:22:48 +0200 Subject: [PATCH 4/7] Adjust integration tests to new permissions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- build/integration/features/sharing-v1.feature | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/build/integration/features/sharing-v1.feature b/build/integration/features/sharing-v1.feature index dd5cc9fff4f8b..09ad4794edb96 100644 --- a/build/integration/features/sharing-v1.feature +++ b/build/integration/features/sharing-v1.feature @@ -90,7 +90,7 @@ Feature: sharing And the HTTP status code should be "200" And Share fields of last share match with | id | A_NUMBER | - | permissions | 15 | + | permissions | 31 | | expiration | +3 days | | url | AN_URL | | token | A_TOKEN | @@ -128,7 +128,7 @@ Feature: sharing | share_type | 3 | | file_source | A_NUMBER | | file_target | /FOLDER | - | permissions | 1 | + | permissions | 17 | | stime | A_NUMBER | | expiration | +3 days | | token | A_TOKEN | @@ -160,7 +160,7 @@ Feature: sharing | share_type | 3 | | file_source | A_NUMBER | | file_target | /FOLDER | - | permissions | 1 | + | permissions | 17 | | stime | A_NUMBER | | token | A_TOKEN | | storage | A_NUMBER | From b48df49b041951fc27d29325719ed058c3a04dd4 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 6 May 2020 21:39:49 +0200 Subject: [PATCH 5/7] Have share permissions on link shares if it is enabled Signed-off-by: Roeland Jago Douma --- apps/files_sharing/lib/Controller/ShareAPIController.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index 299744d6dff19..f8847125a2998 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -477,10 +477,12 @@ public function createShare( } else { $permissions = Constants::PERMISSION_READ; } + // TODO: It might make sense to have a dedicated setting to allow/deny converting link shares into federated ones - if ($this->shareManager->outgoingServer2ServerSharesAllowed()) { + if (($permissions & Constants::PERMISSION_READ) && $this->shareManager->outgoingServer2ServerSharesAllowed()) { $permissions |= Constants::PERMISSION_SHARE; } + $share->setPermissions($permissions); // Set password @@ -906,6 +908,11 @@ public function updateShare( } if ($newPermissions !== null) { + // TODO: It might make sense to have a dedicated setting to allow/deny converting link shares into federated ones + if (($newPermissions & Constants::PERMISSION_READ) && $this->shareManager->outgoingServer2ServerSharesAllowed()) { + $newPermissions |= Constants::PERMISSION_SHARE; + } + $share->setPermissions($newPermissions); $permissions = $newPermissions; } From cccd0d82de599e6d512d95e75362674304cd916b Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 6 May 2020 21:49:26 +0200 Subject: [PATCH 6/7] Update public link share permission code Signed-off-by: Roeland Jago Douma --- build/integration/features/sharing-v1.feature | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build/integration/features/sharing-v1.feature b/build/integration/features/sharing-v1.feature index 09ad4794edb96..c271ac158b4f6 100644 --- a/build/integration/features/sharing-v1.feature +++ b/build/integration/features/sharing-v1.feature @@ -191,7 +191,7 @@ Feature: sharing | share_type | 3 | | file_source | A_NUMBER | | file_target | /FOLDER | - | permissions | 15 | + | permissions | 31 | | stime | A_NUMBER | | token | A_TOKEN | | storage | A_NUMBER | @@ -253,7 +253,7 @@ Feature: sharing | share_type | 3 | | file_source | A_NUMBER | | file_target | /FOLDER | - | permissions | 15 | + | permissions | 31 | | stime | A_NUMBER | | token | A_TOKEN | | storage | A_NUMBER | From 3584797c16903478335fc07b5e6c7b901ca95e33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Fri, 15 May 2020 08:19:31 +0200 Subject: [PATCH 7/7] Fix share update test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- apps/files_sharing/tests/ApiTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/files_sharing/tests/ApiTest.php b/apps/files_sharing/tests/ApiTest.php index 22cd32c487644..ef5a4330d3fac 100644 --- a/apps/files_sharing/tests/ApiTest.php +++ b/apps/files_sharing/tests/ApiTest.php @@ -977,7 +977,8 @@ function testUpdateShareUpload() { \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE | - \OCP\Constants::PERMISSION_DELETE, + \OCP\Constants::PERMISSION_DELETE | + \OCP\Constants::PERMISSION_SHARE, $share1->getPermissions() );