Merge pull request #21109 from nextcloud/backport/19793/stable18

[stable18] Fix resharing of federated shares that were created out of links

Author: rullzer

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);

apps/files_sharing/js/dist/files_sharing_tab.js

@@ -490,16 +490,21 @@ 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 (($permissions & Constants::PERMISSION_READ) && $this->shareManager->outgoingServer2ServerSharesAllowed()) {
+				$permissions |= Constants::PERMISSION_SHARE;
 			}
 
+			$share->setPermissions($permissions);
+
 			// Set password
 			if ($password !== '') {
 				$share->setPassword($password);
@@ -1030,6 +1035,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;
 			}

apps/files_sharing/js/dist/files_sharing_tab.js.map

@@ -126,21 +126,21 @@
 				<template v-if="share.canEdit">
 					<!-- folder -->
 					<template v-if="isFolder && fileHasCreatePermission && config.isPublicUploadEnabled">
-						<ActionRadio :checked="share.permissions === publicUploadRValue"
+						<ActionRadio :checked="sharePermissions === publicUploadRValue"
 							:value="publicUploadRValue"
 							:name="randomId"
 							:disabled="saving"
 							@change="togglePermissions">
 							{{ t('files_sharing', 'Read only') }}
 						</ActionRadio>
-						<ActionRadio :checked="share.permissions === publicUploadRWValue"
+						<ActionRadio :checked="sharePermissions === publicUploadRWValue"
 							:value="publicUploadRWValue"
 							:disabled="saving"
 							:name="randomId"
 							@change="togglePermissions">
 							{{ t('files_sharing', 'Allow upload and editing') }}
 						</ActionRadio>
-						<ActionRadio :checked="share.permissions === publicUploadWValue"
+						<ActionRadio :checked="sharePermissions === publicUploadWValue"
 							:value="publicUploadWValue"
 							:disabled="saving"
 							:name="randomId"
@@ -357,6 +357,15 @@ export default {
 	},
 
 	computed: {
+		/**
+		 * Return the current share permissions
+		 * We always ignore the SHARE permission as this is used for the
+		 * federated sharing.
+		 * @returns {number}
+		 */
+		sharePermissions() {
+			return this.share.permissions & ~OC.PERMISSION_SHARE
+		},
 		/**
 		 * Generate a unique random id for this SharingEntryLink only
 		 * This allows ActionRadios to have the same name prop

apps/files_sharing/lib/Controller/ShareAPIController.php

@@ -205,7 +205,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']));
 
@@ -230,7 +232,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']);
@@ -1002,7 +1005,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()
 		);
 

apps/files_sharing/src/components/SharingEntryLink.vue

@@ -91,7 +91,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 |
@@ -130,7 +130,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 |
@@ -163,7 +163,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 |
@@ -195,7 +195,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 |
@@ -259,7 +259,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 |

apps/files_sharing/tests/ApiTest.php

@@ -626,11 +626,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))) {

build/integration/sharing_features/sharing-v1.feature

@@ -1375,24 +1375,6 @@ public function testLinkCreateChecksNoLinkSharesAllowed() {
 	}
 
 
-	public function testLinkCreateChecksSharePermissions() {
-		$this->expectException(\Exception::class);
-		$this->expectExceptionMessage('Link shares can’t have reshare permissions');
-
-		$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]);
-	}
-
-
 	public function testLinkCreateChecksNoPublicUpload() {
 		$this->expectException(\Exception::class);
 		$this->expectExceptionMessage('Public upload is not allowed');