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 <opensource@fthiessen.de>
This commit is contained in:
Ferdinand Thiessen 2025-05-12 18:40:35 +02:00
parent 2cbfdcc493
commit a18e61a1e5
No known key found for this signature in database
GPG key ID: 45FAE7268762B400
12 changed files with 120 additions and 12 deletions

View file

@ -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;
}

View file

@ -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
*/

View file

@ -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,

View file

@ -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),

View file

@ -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

View file

@ -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,

View file

@ -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;
}

View file

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

View file

@ -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 {

View file

@ -0,0 +1,43 @@
<?php
declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace OC\Core\AppInfo;
use NCU\Config\Lexicon\ConfigLexiconEntry;
use NCU\Config\Lexicon\ConfigLexiconStrictness;
use NCU\Config\Lexicon\IConfigLexicon;
use NCU\Config\ValueType;
/**
* Config Lexicon for core.
*
* Please Add & Manage your Config Keys in that file and keep the Lexicon up to date!
*/
class ConfigLexicon implements IConfigLexicon {
public const SHAREAPI_ALLOW_FEDERATION_ON_PUBLIC_SHARES = 'shareapi_allow_federation_on_public_shares';
public function getStrictness(): ConfigLexiconStrictness {
return ConfigLexiconStrictness::IGNORE;
}
public function getAppConfigs(): array {
return [
new ConfigLexiconEntry(
key: self::SHAREAPI_ALLOW_FEDERATION_ON_PUBLIC_SHARES,
type: ValueType::BOOL,
lazy: true,
defaultRaw: true,
definition: 'adds share permission to public shares to allow adding them to your Nextcloud (federation)',
),
];
}
public function getUserConfigs(): array {
return [];
}
}

View file

@ -1224,6 +1224,7 @@ return array(
'OC\\Contacts\\ContactsMenu\\Providers\\ProfileProvider' => $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',

View file

@ -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',