feat(files_sharing): add config option for extending link-share permissions

apps/files_sharing/lib/Controller/ShareAPIController.php

@@ -10,6 +10,7 @@
 namespace OCA\Files_Sharing\Controller;
 
 use Exception;
+use OC\Core\AppInfo\ConfigLexicon;
 use OC\Files\FileInfo;
 use OC\Files\Storage\Wrapper\Wrapper;
 use OCA\Circles\Api\v1\Circles;
@@ -41,6 +42,7 @@
 use OCP\Files\Node;
 use OCP\Files\NotFoundException;
 use OCP\HintException;
+use OCP\IAppConfig;
 use OCP\IConfig;
 use OCP\IDateTimeZone;
 use OCP\IGroupManager;
@@ -88,6 +90,7 @@ public function __construct(
 		private IURLGenerator $urlGenerator,
 		private IL10N $l,
 		private IConfig $config,
+		private IAppConfig $appConfig,
 		private IAppManager $appManager,
 		private ContainerInterface $serverContainer,
 		private IUserStatusManager $userStatusManager,
@@ -969,9 +972,9 @@ private function getLinkSharePermissions(?int $permissions, ?bool $legacyPublicU
 				: Constants::PERMISSION_READ;
 		}
 
-		// TODO: It might make sense to have a dedicated setting to allow/deny converting link shares into federated ones
 		if ($this->hasPermission($permissions, Constants::PERMISSION_READ)
-			&& $this->shareManager->outgoingServer2ServerSharesAllowed()) {
+			&& $this->shareManager->outgoingServer2ServerSharesAllowed()
+			&& $this->appConfig->getValueBool('core', ConfigLexicon::SHAREAPI_ALLOW_FEDERATION_ON_PUBLIC_SHARES)) {
 			$permissions |= Constants::PERMISSION_SHARE;
 		}
 

apps/files_sharing/tests/ApiTest.php

@@ -7,6 +7,7 @@
  */
 namespace OCA\Files_Sharing\Tests;
 
+use OC\Core\AppInfo\ConfigLexicon;
 use OC\Files\Cache\Scanner;
 use OC\Files\FileInfo;
 use OC\Files\Filesystem;
@@ -21,6 +22,7 @@
 use OCP\Constants;
 use OCP\Files\Folder;
 use OCP\Files\IRootFolder;
+use OCP\IAppConfig;
 use OCP\IConfig;
 use OCP\IDateTimeZone;
 use OCP\IGroupManager;
@@ -35,6 +37,7 @@
 use OCP\Share\IProviderFactory;
 use OCP\Share\IShare;
 use OCP\UserStatus\IManager as IUserStatusManager;
+use PHPUnit\Framework\MockObject\MockObject;
 use Psr\Container\ContainerInterface;
 use Psr\Log\LoggerInterface;
 
@@ -50,11 +53,9 @@ class ApiTest extends TestCase {
 
 	private static $tempStorage;
 
-	/** @var Folder */
-	private $userFolder;
-
-	/** @var string */
-	private $subsubfolder;
+	private Folder $userFolder;
+	private string $subsubfolder;
+	protected IAppConfig&MockObject $appConfig;
 
 	protected function setUp(): void {
 		parent::setUp();
@@ -81,6 +82,8 @@ protected function setUp(): void {
 		$mount->getStorage()->getScanner()->scan('', Scanner::SCAN_RECURSIVE);
 
 		$this->userFolder = \OC::$server->getUserFolder(self::TEST_FILES_SHARING_API_USER1);
+
+		$this->appConfig = $this->createMock(IAppConfig::class);
 	}
 
 	protected function tearDown(): void {
@@ -126,6 +129,7 @@ private function createOCS($userId) {
 			Server::get(IURLGenerator::class),
 			$l,
 			$config,
+			$this->appConfig,
 			$appManager,
 			$serverContainer,
 			$userStatusManager,
@@ -233,8 +237,12 @@ public function testCreateShareLink(): void {
 
 	/**
 	 * @group RoutingWeirdness
+	 * @dataProvider dataAllowFederationOnPublicShares
 	 */
-	public function testCreateShareLinkPublicUpload(): void {
+	public function testCreateShareLinkPublicUpload(array $appConfig, int $permissions): void {
+		$this->appConfig->method('getValueBool')
+			->willReturnMap([$appConfig]);
+
 		$ocs = $this->createOCS(self::TEST_FILES_SHARING_API_USER1);
 		$result = $ocs->createShare($this->folder, Constants::PERMISSION_ALL, IShare::TYPE_LINK, null, 'true');
 		$ocs->cleanup();
@@ -245,7 +253,7 @@ public function testCreateShareLinkPublicUpload(): void {
 			| Constants::PERMISSION_CREATE
 			| Constants::PERMISSION_UPDATE
 			| Constants::PERMISSION_DELETE
-			| Constants::PERMISSION_SHARE,
+			| $permissions,
 			$data['permissions']
 		);
 		$this->assertEmpty($data['expiration']);
@@ -1005,8 +1013,13 @@ public function testUpdateShare(): void {
 
 	/**
 	 * @medium
+	 * @dataProvider dataAllowFederationOnPublicShares
 	 */
-	public function testUpdateShareUpload(): void {
+	public function testUpdateShareUpload(array $appConfig, int $permissions): void {
+		$this->appConfig->method('getValueBool')->willReturnMap([
+			$appConfig,
+		]);
+
 		$node1 = $this->userFolder->get($this->folder);
 		$share1 = $this->shareManager->newShare();
 		$share1->setNode($node1)
@@ -1026,14 +1039,21 @@ public function testUpdateShareUpload(): void {
 			| Constants::PERMISSION_CREATE
 			| Constants::PERMISSION_UPDATE
 			| Constants::PERMISSION_DELETE
-			| Constants::PERMISSION_SHARE,
+			| $permissions,
 			$share1->getPermissions()
 		);
 
 		// cleanup
 		$this->shareManager->deleteShare($share1);
 	}
 
+	public static function dataAllowFederationOnPublicShares(): array {
+		return [
+			[['core', ConfigLexicon::SHAREAPI_ALLOW_FEDERATION_ON_PUBLIC_SHARES, false, false], 0],
+			[['core', ConfigLexicon::SHAREAPI_ALLOW_FEDERATION_ON_PUBLIC_SHARES, false, true], Constants::PERMISSION_SHARE],
+		];
+	}
+
 	/**
 	 * @medium
 	 */

apps/files_sharing/tests/Controller/ShareAPIControllerTest.php

@@ -22,6 +22,7 @@
 use OCP\Files\Mount\IShareOwnerlessMount;
 use OCP\Files\NotFoundException;
 use OCP\Files\Storage\IStorage;
+use OCP\IAppConfig;
 use OCP\IConfig;
 use OCP\IDateTimeZone;
 use OCP\IGroup;
@@ -71,6 +72,7 @@ class ShareAPIControllerTest extends TestCase {
 	private IURLGenerator&MockObject $urlGenerator;
 	private IL10N&MockObject $l;
 	private IConfig&MockObject $config;
+	private IAppConfig&MockObject $appConfig;
 	private IAppManager&MockObject $appManager;
 	private ContainerInterface&MockObject $serverContainer;
 	private IUserStatusManager&MockObject $userStatusManager;
@@ -103,6 +105,7 @@ protected function setUp(): void {
 				return vsprintf($text, $parameters);
 			});
 		$this->config = $this->createMock(IConfig::class);
+		$this->appConfig = $this->createMock(IAppConfig::class);
 		$this->appManager = $this->createMock(IAppManager::class);
 		$this->serverContainer = $this->createMock(ContainerInterface::class);
 		$this->userStatusManager = $this->createMock(IUserStatusManager::class);
@@ -127,6 +130,7 @@ protected function setUp(): void {
 			$this->urlGenerator,
 			$this->l,
 			$this->config,
+			$this->appConfig,
 			$this->appManager,
 			$this->serverContainer,
 			$this->userStatusManager,
@@ -155,6 +159,7 @@ private function mockFormatShare() {
 				$this->urlGenerator,
 				$this->l,
 				$this->config,
+				$this->appConfig,
 				$this->appManager,
 				$this->serverContainer,
 				$this->userStatusManager,
@@ -838,6 +843,7 @@ public function testGetShare(IShare $share, array $result): void {
 				$this->urlGenerator,
 				$this->l,
 				$this->config,
+				$this->appConfig,
 				$this->appManager,
 				$this->serverContainer,
 				$this->userStatusManager,
@@ -1469,6 +1475,7 @@ public function testGetShares(array $getSharesParameters, array $shares, array $
 				$this->urlGenerator,
 				$this->l,
 				$this->config,
+				$this->appConfig,
 				$this->appManager,
 				$this->serverContainer,
 				$this->userStatusManager,
@@ -1856,6 +1863,7 @@ public function testCreateShareUser(): void {
 				$this->urlGenerator,
 				$this->l,
 				$this->config,
+				$this->appConfig,
 				$this->appManager,
 				$this->serverContainer,
 				$this->userStatusManager,
@@ -1954,6 +1962,7 @@ public function testCreateShareGroup(): void {
 				$this->urlGenerator,
 				$this->l,
 				$this->config,
+				$this->appConfig,
 				$this->appManager,
 				$this->serverContainer,
 				$this->userStatusManager,
@@ -2380,6 +2389,7 @@ public function testCreateShareRemote(): void {
 				$this->urlGenerator,
 				$this->l,
 				$this->config,
+				$this->appConfig,
 				$this->appManager,
 				$this->serverContainer,
 				$this->userStatusManager,
@@ -2451,6 +2461,7 @@ public function testCreateShareRemoteGroup(): void {
 				$this->urlGenerator,
 				$this->l,
 				$this->config,
+				$this->appConfig,
 				$this->appManager,
 				$this->serverContainer,
 				$this->userStatusManager,
@@ -2689,6 +2700,7 @@ public function testCreateReshareOfFederatedMountNoDeletePermissions(): void {
 				$this->urlGenerator,
 				$this->l,
 				$this->config,
+				$this->appConfig,
 				$this->appManager,
 				$this->serverContainer,
 				$this->userStatusManager,

apps/settings/lib/Settings/Admin/Sharing.php

@@ -6,10 +6,12 @@
  */
 namespace OCA\Settings\Settings\Admin;
 
+use OC\Core\AppInfo\ConfigLexicon;
 use OCP\App\IAppManager;
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\AppFramework\Services\IInitialState;
 use OCP\Constants;
+use OCP\IAppConfig;
 use OCP\IConfig;
 use OCP\IL10N;
 use OCP\IURLGenerator;
@@ -20,6 +22,7 @@
 class Sharing implements IDelegatedSettings {
 	public function __construct(
 		private IConfig $config,
+		private IAppConfig $appConfig,
 		private IL10N $l,
 		private IManager $shareManager,
 		private IAppManager $appManager,
@@ -47,6 +50,7 @@ public function getForm() {
 			'allowPublicUpload' => $this->getHumanBooleanConfig('core', 'shareapi_allow_public_upload', true),
 			'allowResharing' => $this->getHumanBooleanConfig('core', 'shareapi_allow_resharing', true),
 			'allowShareDialogUserEnumeration' => $this->getHumanBooleanConfig('core', 'shareapi_allow_share_dialog_user_enumeration', true),
+			'allowFederationOnPublicShares' => $this->appConfig->getValueBool('core', ConfigLexicon::SHAREAPI_ALLOW_FEDERATION_ON_PUBLIC_SHARES),
 			'restrictUserEnumerationToGroup' => $this->getHumanBooleanConfig('core', 'shareapi_restrict_user_enumeration_to_group'),
 			'restrictUserEnumerationToPhone' => $this->getHumanBooleanConfig('core', 'shareapi_restrict_user_enumeration_to_phone'),
 			'restrictUserEnumerationFullMatch' => $this->getHumanBooleanConfig('core', 'shareapi_restrict_user_enumeration_full_match', true),

apps/settings/src/components/AdminSettingsSharingForm.vue

@@ -48,6 +48,10 @@
 				<NcCheckboxRadioSwitch :checked.sync="settings.allowPublicUpload">
 					{{ t('settings', 'Allow public uploads') }}
 				</NcCheckboxRadioSwitch>
+				<NcCheckboxRadioSwitch v-model="settings.allowFederationOnPublicShares">
+					{{ t('settings', 'Allow public shares to be added to other clouds by federation.') }}
+					{{ t('settings', 'This will add share permissions to all newly created link shares.') }}
+				</NcCheckboxRadioSwitch>
 				<NcCheckboxRadioSwitch :checked.sync="settings.enableLinkPasswordByDefault">
 					{{ t('settings', 'Always ask for a password') }}
 				</NcCheckboxRadioSwitch>
@@ -241,6 +245,7 @@ interface IShareSettings {
 	allowPublicUpload: boolean
 	allowResharing: boolean
 	allowShareDialogUserEnumeration: boolean
+	allowFederationOnPublicShares: boolean
 	restrictUserEnumerationToGroup: boolean
 	restrictUserEnumerationToPhone: boolean
 	restrictUserEnumerationFullMatch: boolean

apps/settings/tests/Settings/Admin/SharingTest.php

@@ -11,6 +11,7 @@
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\AppFramework\Services\IInitialState;
 use OCP\Constants;
+use OCP\IAppConfig;
 use OCP\IConfig;
 use OCP\IL10N;
 use OCP\IURLGenerator;
@@ -19,25 +20,30 @@
 use Test\TestCase;
 
 class SharingTest extends TestCase {
+	private Sharing $admin;
+
 	private IConfig&MockObject $config;
+	private IAppConfig&MockObject $appConfig;
 	private IL10N&MockObject $l10n;
 	private IManager&MockObject $shareManager;
 	private IAppManager&MockObject $appManager;
 	private IURLGenerator&MockObject $urlGenerator;
 	private IInitialState&MockObject $initialState;
-	private Sharing $admin;
 
 	protected function setUp(): void {
 		parent::setUp();
 		$this->config = $this->createMock(IConfig::class);
+		$this->appConfig = $this->createMock(IAppConfig::class);
 		$this->l10n = $this->createMock(IL10N::class);
+
 		$this->shareManager = $this->createMock(IManager::class);
 		$this->appManager = $this->createMock(IAppManager::class);
 		$this->urlGenerator = $this->createMock(IURLGenerator::class);
 		$this->initialState = $this->createMock(IInitialState::class);
 
 		$this->admin = new Sharing(
 			$this->config,
+			$this->appConfig,
 			$this->l10n,
 			$this->shareManager,
 			$this->appManager,
@@ -48,6 +54,12 @@ protected function setUp(): void {
 	}
 
 	public function testGetFormWithoutExcludedGroups(): void {
+		$this->appConfig
+			->method('getValueBool')
+			->willReturnMap([
+				['core', 'shareapi_allow_federation_on_public_shares', false, false, true],
+			]);
+
 		$this->config
 			->method('getAppValue')
 			->willReturnMap([
@@ -103,6 +115,7 @@ public function testGetFormWithoutExcludedGroups(): void {
 				'allowPublicUpload' => true,
 				'allowResharing' => true,
 				'allowShareDialogUserEnumeration' => true,
+				'allowFederationOnPublicShares' => true,
 				'restrictUserEnumerationToGroup' => false,
 				'restrictUserEnumerationToPhone' => false,
 				'restrictUserEnumerationFullMatch' => true,

build/integration/features/bootstrap/Sharing.php

@@ -337,6 +337,8 @@ public function isFieldInResponse($field, $contentExpected) {
 				return $this->isExpectedUrl((string)$data->$field, 'index.php/s/');
 			} elseif ($contentExpected == $data->$field) {
 				return true;
+			} else {
+				print($data->$field);
 			}
 			return false;
 		}

build/integration/features/bootstrap/SharingContext.php

@@ -29,6 +29,7 @@ protected function resetAppConfigs() {
 		$this->deleteServerConfig('core', 'shareapi_default_expire_date');
 		$this->deleteServerConfig('core', 'shareapi_expire_after_n_days');
 		$this->deleteServerConfig('core', 'link_defaultExpDays');
+		$this->deleteServerConfig('core', 'shareapi_allow_federation_on_public_shares');
 		$this->deleteServerConfig('files_sharing', 'outgoing_server2server_share_enabled');
 		$this->deleteServerConfig('core', 'shareapi_allow_view_without_download');
 

core/AppInfo/Application.php

@@ -78,6 +78,9 @@ public function register(IRegistrationContext $context): void {
 
 		// Tags
 		$context->registerEventListener(UserDeletedEvent::class, TagManager::class);
+
+		// config lexicon
+		$context->registerConfigLexicon(ConfigLexicon::class);
 	}
 
 	public function boot(IBootContext $context): void {