From a18e61a1e5cd55f573f2e7b58c91adccfd872d91 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Mon, 12 May 2025 18:40:35 +0200 Subject: [PATCH] feat(files_sharing): add config option for extending link-share permissions This allows the admin to control the behavior whether link shares with READ permissions should be extended to also gain SHARE permissions, allowing users (public share receivers) to add the share to their cloud. Signed-off-by: Ferdinand Thiessen --- .../lib/Controller/ShareAPIController.php | 7 ++- apps/files_sharing/tests/ApiTest.php | 38 ++++++++++++---- .../Controller/ShareAPIControllerTest.php | 12 ++++++ apps/settings/lib/Settings/Admin/Sharing.php | 4 ++ .../components/AdminSettingsSharingForm.vue | 5 +++ .../tests/Settings/Admin/SharingTest.php | 15 ++++++- .../features/bootstrap/Sharing.php | 2 + .../features/bootstrap/SharingContext.php | 1 + core/AppInfo/Application.php | 3 ++ core/AppInfo/ConfigLexicon.php | 43 +++++++++++++++++++ lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + 12 files changed, 120 insertions(+), 12 deletions(-) create mode 100644 core/AppInfo/ConfigLexicon.php diff --git a/apps/files_sharing/lib/Controller/ShareAPIController.php b/apps/files_sharing/lib/Controller/ShareAPIController.php index a19eb50faf7..7591493167f 100644 --- a/apps/files_sharing/lib/Controller/ShareAPIController.php +++ b/apps/files_sharing/lib/Controller/ShareAPIController.php @@ -10,6 +10,7 @@ declare(strict_types=1); 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\Mount\IShareOwnerlessMount; 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 @@ class ShareAPIController extends OCSController { 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 @@ class ShareAPIController extends OCSController { : 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; } diff --git a/apps/files_sharing/tests/ApiTest.php b/apps/files_sharing/tests/ApiTest.php index bb686c1ea8c..676809eebff 100644 --- a/apps/files_sharing/tests/ApiTest.php +++ b/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\AppFramework\OCS\OCSNotFoundException; 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\Server; 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 @@ class ApiTest extends TestCase { $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 @@ class ApiTest extends TestCase { Server::get(IURLGenerator::class), $l, $config, + $this->appConfig, $appManager, $serverContainer, $userStatusManager, @@ -233,8 +237,12 @@ class ApiTest extends TestCase { /** * @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 @@ class ApiTest extends TestCase { | Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE | Constants::PERMISSION_DELETE - | Constants::PERMISSION_SHARE, + | $permissions, $data['permissions'] ); $this->assertEmpty($data['expiration']); @@ -1005,8 +1013,13 @@ class ApiTest extends TestCase { /** * @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,7 +1039,7 @@ class ApiTest extends TestCase { | Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE | Constants::PERMISSION_DELETE - | Constants::PERMISSION_SHARE, + | $permissions, $share1->getPermissions() ); @@ -1034,6 +1047,13 @@ class ApiTest extends TestCase { $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 */ diff --git a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php index 8bb220d684b..abc405fc21c 100644 --- a/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareAPIControllerTest.php @@ -22,6 +22,7 @@ use OCP\Files\Mount\IMountPoint; 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 @@ class ShareAPIControllerTest extends TestCase { 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 @@ class ShareAPIControllerTest extends TestCase { $this->urlGenerator, $this->l, $this->config, + $this->appConfig, $this->appManager, $this->serverContainer, $this->userStatusManager, @@ -155,6 +159,7 @@ class ShareAPIControllerTest extends TestCase { $this->urlGenerator, $this->l, $this->config, + $this->appConfig, $this->appManager, $this->serverContainer, $this->userStatusManager, @@ -838,6 +843,7 @@ class ShareAPIControllerTest extends TestCase { $this->urlGenerator, $this->l, $this->config, + $this->appConfig, $this->appManager, $this->serverContainer, $this->userStatusManager, @@ -1469,6 +1475,7 @@ class ShareAPIControllerTest extends TestCase { $this->urlGenerator, $this->l, $this->config, + $this->appConfig, $this->appManager, $this->serverContainer, $this->userStatusManager, @@ -1856,6 +1863,7 @@ class ShareAPIControllerTest extends TestCase { $this->urlGenerator, $this->l, $this->config, + $this->appConfig, $this->appManager, $this->serverContainer, $this->userStatusManager, @@ -1954,6 +1962,7 @@ class ShareAPIControllerTest extends TestCase { $this->urlGenerator, $this->l, $this->config, + $this->appConfig, $this->appManager, $this->serverContainer, $this->userStatusManager, @@ -2380,6 +2389,7 @@ class ShareAPIControllerTest extends TestCase { $this->urlGenerator, $this->l, $this->config, + $this->appConfig, $this->appManager, $this->serverContainer, $this->userStatusManager, @@ -2451,6 +2461,7 @@ class ShareAPIControllerTest extends TestCase { $this->urlGenerator, $this->l, $this->config, + $this->appConfig, $this->appManager, $this->serverContainer, $this->userStatusManager, @@ -2689,6 +2700,7 @@ class ShareAPIControllerTest extends TestCase { $this->urlGenerator, $this->l, $this->config, + $this->appConfig, $this->appManager, $this->serverContainer, $this->userStatusManager, diff --git a/apps/settings/lib/Settings/Admin/Sharing.php b/apps/settings/lib/Settings/Admin/Sharing.php index 3dede7fbdb4..e038b2a6231 100644 --- a/apps/settings/lib/Settings/Admin/Sharing.php +++ b/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 @@ use OCP\Util; 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 @@ class Sharing implements IDelegatedSettings { '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), diff --git a/apps/settings/src/components/AdminSettingsSharingForm.vue b/apps/settings/src/components/AdminSettingsSharingForm.vue index 1973781edee..c582e9febee 100644 --- a/apps/settings/src/components/AdminSettingsSharingForm.vue +++ b/apps/settings/src/components/AdminSettingsSharingForm.vue @@ -48,6 +48,10 @@ {{ t('settings', 'Allow public uploads') }} + + {{ 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.') }} + {{ t('settings', 'Always ask for a password') }} @@ -241,6 +245,7 @@ interface IShareSettings { allowPublicUpload: boolean allowResharing: boolean allowShareDialogUserEnumeration: boolean + allowFederationOnPublicShares: boolean restrictUserEnumerationToGroup: boolean restrictUserEnumerationToPhone: boolean restrictUserEnumerationFullMatch: boolean diff --git a/apps/settings/tests/Settings/Admin/SharingTest.php b/apps/settings/tests/Settings/Admin/SharingTest.php index 62e3440fa54..12ab5c3cada 100644 --- a/apps/settings/tests/Settings/Admin/SharingTest.php +++ b/apps/settings/tests/Settings/Admin/SharingTest.php @@ -11,6 +11,7 @@ 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; @@ -19,18 +20,22 @@ use PHPUnit\Framework\MockObject\MockObject; 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); @@ -38,6 +43,7 @@ class SharingTest extends TestCase { $this->admin = new Sharing( $this->config, + $this->appConfig, $this->l10n, $this->shareManager, $this->appManager, @@ -48,6 +54,12 @@ class SharingTest extends TestCase { } 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 @@ class SharingTest extends TestCase { 'allowPublicUpload' => true, 'allowResharing' => true, 'allowShareDialogUserEnumeration' => true, + 'allowFederationOnPublicShares' => true, 'restrictUserEnumerationToGroup' => false, 'restrictUserEnumerationToPhone' => false, 'restrictUserEnumerationFullMatch' => true, diff --git a/build/integration/features/bootstrap/Sharing.php b/build/integration/features/bootstrap/Sharing.php index 039ee7d1121..0cc490ff110 100644 --- a/build/integration/features/bootstrap/Sharing.php +++ b/build/integration/features/bootstrap/Sharing.php @@ -337,6 +337,8 @@ trait Sharing { return $this->isExpectedUrl((string)$data->$field, 'index.php/s/'); } elseif ($contentExpected == $data->$field) { return true; + } else { + print($data->$field); } return false; } diff --git a/build/integration/features/bootstrap/SharingContext.php b/build/integration/features/bootstrap/SharingContext.php index fe4d3bb6bf1..a9dd99108a9 100644 --- a/build/integration/features/bootstrap/SharingContext.php +++ b/build/integration/features/bootstrap/SharingContext.php @@ -29,6 +29,7 @@ class SharingContext implements Context, SnippetAcceptingContext { $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'); diff --git a/core/AppInfo/Application.php b/core/AppInfo/Application.php index b94f010b02b..f1fe7d763e3 100644 --- a/core/AppInfo/Application.php +++ b/core/AppInfo/Application.php @@ -78,6 +78,9 @@ class Application extends App implements IBootstrap { // Tags $context->registerEventListener(UserDeletedEvent::class, TagManager::class); + + // config lexicon + $context->registerConfigLexicon(ConfigLexicon::class); } public function boot(IBootContext $context): void { diff --git a/core/AppInfo/ConfigLexicon.php b/core/AppInfo/ConfigLexicon.php new file mode 100644 index 00000000000..cc7fd1a3b09 --- /dev/null +++ b/core/AppInfo/ConfigLexicon.php @@ -0,0 +1,43 @@ + $baseDir . '/lib/private/Contacts/ContactsMenu/Providers/ProfileProvider.php', 'OC\\ContextChat\\ContentManager' => $baseDir . '/lib/private/ContextChat/ContentManager.php', 'OC\\Core\\AppInfo\\Application' => $baseDir . '/core/AppInfo/Application.php', + 'OC\\Core\\AppInfo\\ConfigLexicon' => $baseDir . '/core/AppInfo/ConfigLexicon.php', 'OC\\Core\\BackgroundJobs\\BackgroundCleanupUpdaterBackupsJob' => $baseDir . '/core/BackgroundJobs/BackgroundCleanupUpdaterBackupsJob.php', 'OC\\Core\\BackgroundJobs\\CheckForUserCertificates' => $baseDir . '/core/BackgroundJobs/CheckForUserCertificates.php', 'OC\\Core\\BackgroundJobs\\CleanupLoginFlowV2' => $baseDir . '/core/BackgroundJobs/CleanupLoginFlowV2.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 15ae0a9373f..daa1a1940dd 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1265,6 +1265,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Contacts\\ContactsMenu\\Providers\\ProfileProvider' => __DIR__ . '/../../..' . '/lib/private/Contacts/ContactsMenu/Providers/ProfileProvider.php', 'OC\\ContextChat\\ContentManager' => __DIR__ . '/../../..' . '/lib/private/ContextChat/ContentManager.php', 'OC\\Core\\AppInfo\\Application' => __DIR__ . '/../../..' . '/core/AppInfo/Application.php', + 'OC\\Core\\AppInfo\\ConfigLexicon' => __DIR__ . '/../../..' . '/core/AppInfo/ConfigLexicon.php', 'OC\\Core\\BackgroundJobs\\BackgroundCleanupUpdaterBackupsJob' => __DIR__ . '/../../..' . '/core/BackgroundJobs/BackgroundCleanupUpdaterBackupsJob.php', 'OC\\Core\\BackgroundJobs\\CheckForUserCertificates' => __DIR__ . '/../../..' . '/core/BackgroundJobs/CheckForUserCertificates.php', 'OC\\Core\\BackgroundJobs\\CleanupLoginFlowV2' => __DIR__ . '/../../..' . '/core/BackgroundJobs/CleanupLoginFlowV2.php',