Skip to content

Commit ebc5ae8

Browse files
susnuxsusnux
authored
Merge pull request #52777 from nextcloud/feat/add-config-for-share-perm
feat(files_sharing): add config option for extending link-share permissions
2 parents 2cbfdcc + 1eb6f0b commit ebc5ae8

File tree

16 files changed

+130
-18
lines changed

16 files changed

+130
-18
lines changed

‎apps/files_sharing/lib/Controller/ShareAPIController.php‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
namespace OCA\Files_Sharing\Controller;
1111

1212
use Exception;
13+
use OC\Core\AppInfo\ConfigLexicon;
1314
use OC\Files\FileInfo;
1415
use OC\Files\Storage\Wrapper\Wrapper;
1516
use OCA\Circles\Api\v1\Circles;
@@ -41,6 +42,7 @@
4142
use OCP\Files\Node;
4243
use OCP\Files\NotFoundException;
4344
use OCP\HintException;
45+
use OCP\IAppConfig;
4446
use OCP\IConfig;
4547
use OCP\IDateTimeZone;
4648
use OCP\IGroupManager;
@@ -88,6 +90,7 @@ public function __construct(
8890
private IURLGenerator $urlGenerator,
8991
private IL10N $l,
9092
private IConfig $config,
93+
private IAppConfig $appConfig,
9194
private IAppManager $appManager,
9295
private ContainerInterface $serverContainer,
9396
private IUserStatusManager $userStatusManager,
@@ -969,9 +972,9 @@ private function getLinkSharePermissions(?int $permissions, ?bool $legacyPublicU
969972
: Constants::PERMISSION_READ;
970973
}
971974

972-
// TODO: It might make sense to have a dedicated setting to allow/deny converting link shares into federated ones
973975
if ($this->hasPermission($permissions, Constants::PERMISSION_READ)
974-
&& $this->shareManager->outgoingServer2ServerSharesAllowed()) {
976+
&& $this->shareManager->outgoingServer2ServerSharesAllowed()
977+
&& $this->appConfig->getValueBool('core', ConfigLexicon::SHAREAPI_ALLOW_FEDERATION_ON_PUBLIC_SHARES)) {
975978
$permissions |= Constants::PERMISSION_SHARE;
976979
}
977980

‎apps/files_sharing/tests/ApiTest.php‎

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
*/
88
namespace OCA\Files_Sharing\Tests;
99

10+
use OC\Core\AppInfo\ConfigLexicon;
1011
use OC\Files\Cache\Scanner;
1112
use OC\Files\FileInfo;
1213
use OC\Files\Filesystem;
@@ -21,6 +22,7 @@
2122
use OCP\Constants;
2223
use OCP\Files\Folder;
2324
use OCP\Files\IRootFolder;
25+
use OCP\IAppConfig;
2426
use OCP\IConfig;
2527
use OCP\IDateTimeZone;
2628
use OCP\IGroupManager;
@@ -35,6 +37,7 @@
3537
use OCP\Share\IProviderFactory;
3638
use OCP\Share\IShare;
3739
use OCP\UserStatus\IManager as IUserStatusManager;
40+
use PHPUnit\Framework\MockObject\MockObject;
3841
use Psr\Container\ContainerInterface;
3942
use Psr\Log\LoggerInterface;
4043

@@ -50,11 +53,9 @@ class ApiTest extends TestCase {
5053

5154
private static $tempStorage;
5255

53-
/** @var Folder */
54-
private $userFolder;
55-
56-
/** @var string */
57-
private $subsubfolder;
56+
private Folder $userFolder;
57+
private string $subsubfolder;
58+
protected IAppConfig&MockObject $appConfig;
5859

5960
protected function setUp(): void {
6061
parent::setUp();
@@ -81,6 +82,8 @@ protected function setUp(): void {
8182
$mount->getStorage()->getScanner()->scan('', Scanner::SCAN_RECURSIVE);
8283

8384
$this->userFolder = \OC::$server->getUserFolder(self::TEST_FILES_SHARING_API_USER1);
85+
86+
$this->appConfig = $this->createMock(IAppConfig::class);
8487
}
8588

8689
protected function tearDown(): void {
@@ -126,6 +129,7 @@ private function createOCS($userId) {
126129
Server::get(IURLGenerator::class),
127130
$l,
128131
$config,
132+
$this->appConfig,
129133
$appManager,
130134
$serverContainer,
131135
$userStatusManager,
@@ -233,8 +237,12 @@ public function testCreateShareLink(): void {
233237

234238
/**
235239
* @group RoutingWeirdness
240+
* @dataProvider dataAllowFederationOnPublicShares
236241
*/
237-
public function testCreateShareLinkPublicUpload(): void {
242+
public function testCreateShareLinkPublicUpload(array $appConfig, int $permissions): void {
243+
$this->appConfig->method('getValueBool')
244+
->willReturnMap([$appConfig]);
245+
238246
$ocs = $this->createOCS(self::TEST_FILES_SHARING_API_USER1);
239247
$result = $ocs->createShare($this->folder, Constants::PERMISSION_ALL, IShare::TYPE_LINK, null, 'true');
240248
$ocs->cleanup();
@@ -245,7 +253,7 @@ public function testCreateShareLinkPublicUpload(): void {
245253
| Constants::PERMISSION_CREATE
246254
| Constants::PERMISSION_UPDATE
247255
| Constants::PERMISSION_DELETE
248-
| Constants::PERMISSION_SHARE,
256+
| $permissions,
249257
$data['permissions']
250258
);
251259
$this->assertEmpty($data['expiration']);
@@ -1005,8 +1013,13 @@ public function testUpdateShare(): void {
10051013

10061014
/**
10071015
* @medium
1016+
* @dataProvider dataAllowFederationOnPublicShares
10081017
*/
1009-
public function testUpdateShareUpload(): void {
1018+
public function testUpdateShareUpload(array $appConfig, int $permissions): void {
1019+
$this->appConfig->method('getValueBool')->willReturnMap([
1020+
$appConfig,
1021+
]);
1022+
10101023
$node1 = $this->userFolder->get($this->folder);
10111024
$share1 = $this->shareManager->newShare();
10121025
$share1->setNode($node1)
@@ -1026,14 +1039,21 @@ public function testUpdateShareUpload(): void {
10261039
| Constants::PERMISSION_CREATE
10271040
| Constants::PERMISSION_UPDATE
10281041
| Constants::PERMISSION_DELETE
1029-
| Constants::PERMISSION_SHARE,
1042+
| $permissions,
10301043
$share1->getPermissions()
10311044
);
10321045

10331046
// cleanup
10341047
$this->shareManager->deleteShare($share1);
10351048
}
10361049

1050+
public static function dataAllowFederationOnPublicShares(): array {
1051+
return [
1052+
[['core', ConfigLexicon::SHAREAPI_ALLOW_FEDERATION_ON_PUBLIC_SHARES, false, false], 0],
1053+
[['core', ConfigLexicon::SHAREAPI_ALLOW_FEDERATION_ON_PUBLIC_SHARES, false, true], Constants::PERMISSION_SHARE],
1054+
];
1055+
}
1056+
10371057
/**
10381058
* @medium
10391059
*/

‎apps/files_sharing/tests/Controller/ShareAPIControllerTest.php‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
use OCP\Files\Mount\IShareOwnerlessMount;
2323
use OCP\Files\NotFoundException;
2424
use OCP\Files\Storage\IStorage;
25+
use OCP\IAppConfig;
2526
use OCP\IConfig;
2627
use OCP\IDateTimeZone;
2728
use OCP\IGroup;
@@ -71,6 +72,7 @@ class ShareAPIControllerTest extends TestCase {
7172
private IURLGenerator&MockObject $urlGenerator;
7273
private IL10N&MockObject $l;
7374
private IConfig&MockObject $config;
75+
private IAppConfig&MockObject $appConfig;
7476
private IAppManager&MockObject $appManager;
7577
private ContainerInterface&MockObject $serverContainer;
7678
private IUserStatusManager&MockObject $userStatusManager;
@@ -103,6 +105,7 @@ protected function setUp(): void {
103105
return vsprintf($text, $parameters);
104106
});
105107
$this->config = $this->createMock(IConfig::class);
108+
$this->appConfig = $this->createMock(IAppConfig::class);
106109
$this->appManager = $this->createMock(IAppManager::class);
107110
$this->serverContainer = $this->createMock(ContainerInterface::class);
108111
$this->userStatusManager = $this->createMock(IUserStatusManager::class);
@@ -127,6 +130,7 @@ protected function setUp(): void {
127130
$this->urlGenerator,
128131
$this->l,
129132
$this->config,
133+
$this->appConfig,
130134
$this->appManager,
131135
$this->serverContainer,
132136
$this->userStatusManager,
@@ -155,6 +159,7 @@ private function mockFormatShare() {
155159
$this->urlGenerator,
156160
$this->l,
157161
$this->config,
162+
$this->appConfig,
158163
$this->appManager,
159164
$this->serverContainer,
160165
$this->userStatusManager,
@@ -838,6 +843,7 @@ public function testGetShare(IShare $share, array $result): void {
838843
$this->urlGenerator,
839844
$this->l,
840845
$this->config,
846+
$this->appConfig,
841847
$this->appManager,
842848
$this->serverContainer,
843849
$this->userStatusManager,
@@ -1469,6 +1475,7 @@ public function testGetShares(array $getSharesParameters, array $shares, array $
14691475
$this->urlGenerator,
14701476
$this->l,
14711477
$this->config,
1478+
$this->appConfig,
14721479
$this->appManager,
14731480
$this->serverContainer,
14741481
$this->userStatusManager,
@@ -1856,6 +1863,7 @@ public function testCreateShareUser(): void {
18561863
$this->urlGenerator,
18571864
$this->l,
18581865
$this->config,
1866+
$this->appConfig,
18591867
$this->appManager,
18601868
$this->serverContainer,
18611869
$this->userStatusManager,
@@ -1954,6 +1962,7 @@ public function testCreateShareGroup(): void {
19541962
$this->urlGenerator,
19551963
$this->l,
19561964
$this->config,
1965+
$this->appConfig,
19571966
$this->appManager,
19581967
$this->serverContainer,
19591968
$this->userStatusManager,
@@ -2380,6 +2389,7 @@ public function testCreateShareRemote(): void {
23802389
$this->urlGenerator,
23812390
$this->l,
23822391
$this->config,
2392+
$this->appConfig,
23832393
$this->appManager,
23842394
$this->serverContainer,
23852395
$this->userStatusManager,
@@ -2451,6 +2461,7 @@ public function testCreateShareRemoteGroup(): void {
24512461
$this->urlGenerator,
24522462
$this->l,
24532463
$this->config,
2464+
$this->appConfig,
24542465
$this->appManager,
24552466
$this->serverContainer,
24562467
$this->userStatusManager,
@@ -2689,6 +2700,7 @@ public function testCreateReshareOfFederatedMountNoDeletePermissions(): void {
26892700
$this->urlGenerator,
26902701
$this->l,
26912702
$this->config,
2703+
$this->appConfig,
26922704
$this->appManager,
26932705
$this->serverContainer,
26942706
$this->userStatusManager,

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,12 @@
66
*/
77
namespace OCA\Settings\Settings\Admin;
88

9+
use OC\Core\AppInfo\ConfigLexicon;
910
use OCP\App\IAppManager;
1011
use OCP\AppFramework\Http\TemplateResponse;
1112
use OCP\AppFramework\Services\IInitialState;
1213
use OCP\Constants;
14+
use OCP\IAppConfig;
1315
use OCP\IConfig;
1416
use OCP\IL10N;
1517
use OCP\IURLGenerator;
@@ -20,6 +22,7 @@
2022
class Sharing implements IDelegatedSettings {
2123
public function __construct(
2224
private IConfig $config,
25+
private IAppConfig $appConfig,
2326
private IL10N $l,
2427
private IManager $shareManager,
2528
private IAppManager $appManager,
@@ -47,6 +50,7 @@ public function getForm() {
4750
'allowPublicUpload' => $this->getHumanBooleanConfig('core', 'shareapi_allow_public_upload', true),
4851
'allowResharing' => $this->getHumanBooleanConfig('core', 'shareapi_allow_resharing', true),
4952
'allowShareDialogUserEnumeration' => $this->getHumanBooleanConfig('core', 'shareapi_allow_share_dialog_user_enumeration', true),
53+
'allowFederationOnPublicShares' => $this->appConfig->getValueBool('core', ConfigLexicon::SHAREAPI_ALLOW_FEDERATION_ON_PUBLIC_SHARES),
5054
'restrictUserEnumerationToGroup' => $this->getHumanBooleanConfig('core', 'shareapi_restrict_user_enumeration_to_group'),
5155
'restrictUserEnumerationToPhone' => $this->getHumanBooleanConfig('core', 'shareapi_restrict_user_enumeration_to_phone'),
5256
'restrictUserEnumerationFullMatch' => $this->getHumanBooleanConfig('core', 'shareapi_restrict_user_enumeration_full_match', true),

‎apps/settings/src/components/AdminSettingsSharingForm.vue‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,10 @@
4848
<NcCheckboxRadioSwitch :checked.sync="settings.allowPublicUpload">
4949
{{ t('settings', 'Allow public uploads') }}
5050
</NcCheckboxRadioSwitch>
51+
<NcCheckboxRadioSwitch v-model="settings.allowFederationOnPublicShares">
52+
{{ t('settings', 'Allow public shares to be added to other clouds by federation.') }}
53+
{{ t('settings', 'This will add share permissions to all newly created link shares.') }}
54+
</NcCheckboxRadioSwitch>
5155
<NcCheckboxRadioSwitch :checked.sync="settings.enableLinkPasswordByDefault">
5256
{{ t('settings', 'Always ask for a password') }}
5357
</NcCheckboxRadioSwitch>
@@ -241,6 +245,7 @@ interface IShareSettings {
241245
allowPublicUpload: boolean
242246
allowResharing: boolean
243247
allowShareDialogUserEnumeration: boolean
248+
allowFederationOnPublicShares: boolean
244249
restrictUserEnumerationToGroup: boolean
245250
restrictUserEnumerationToPhone: boolean
246251
restrictUserEnumerationFullMatch: boolean

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

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use OCP\AppFramework\Http\TemplateResponse;
1212
use OCP\AppFramework\Services\IInitialState;
1313
use OCP\Constants;
14+
use OCP\IAppConfig;
1415
use OCP\IConfig;
1516
use OCP\IL10N;
1617
use OCP\IURLGenerator;
@@ -19,25 +20,30 @@
1920
use Test\TestCase;
2021

2122
class SharingTest extends TestCase {
23+
private Sharing $admin;
24+
2225
private IConfig&MockObject $config;
26+
private IAppConfig&MockObject $appConfig;
2327
private IL10N&MockObject $l10n;
2428
private IManager&MockObject $shareManager;
2529
private IAppManager&MockObject $appManager;
2630
private IURLGenerator&MockObject $urlGenerator;
2731
private IInitialState&MockObject $initialState;
28-
private Sharing $admin;
2932

3033
protected function setUp(): void {
3134
parent::setUp();
3235
$this->config = $this->createMock(IConfig::class);
36+
$this->appConfig = $this->createMock(IAppConfig::class);
3337
$this->l10n = $this->createMock(IL10N::class);
38+
3439
$this->shareManager = $this->createMock(IManager::class);
3540
$this->appManager = $this->createMock(IAppManager::class);
3641
$this->urlGenerator = $this->createMock(IURLGenerator::class);
3742
$this->initialState = $this->createMock(IInitialState::class);
3843

3944
$this->admin = new Sharing(
4045
$this->config,
46+
$this->appConfig,
4147
$this->l10n,
4248
$this->shareManager,
4349
$this->appManager,
@@ -48,6 +54,12 @@ protected function setUp(): void {
4854
}
4955

5056
public function testGetFormWithoutExcludedGroups(): void {
57+
$this->appConfig
58+
->method('getValueBool')
59+
->willReturnMap([
60+
['core', 'shareapi_allow_federation_on_public_shares', false, false, true],
61+
]);
62+
5163
$this->config
5264
->method('getAppValue')
5365
->willReturnMap([
@@ -103,6 +115,7 @@ public function testGetFormWithoutExcludedGroups(): void {
103115
'allowPublicUpload' => true,
104116
'allowResharing' => true,
105117
'allowShareDialogUserEnumeration' => true,
118+
'allowFederationOnPublicShares' => true,
106119
'restrictUserEnumerationToGroup' => false,
107120
'restrictUserEnumerationToPhone' => false,
108121
'restrictUserEnumerationFullMatch' => true,

‎build/integration/features/bootstrap/Sharing.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,8 @@ public function isFieldInResponse($field, $contentExpected) {
337337
return $this->isExpectedUrl((string)$data->$field, 'index.php/s/');
338338
} elseif ($contentExpected == $data->$field) {
339339
return true;
340+
} else {
341+
print($data->$field);
340342
}
341343
return false;
342344
}

‎build/integration/features/bootstrap/SharingContext.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ protected function resetAppConfigs() {
2929
$this->deleteServerConfig('core', 'shareapi_default_expire_date');
3030
$this->deleteServerConfig('core', 'shareapi_expire_after_n_days');
3131
$this->deleteServerConfig('core', 'link_defaultExpDays');
32+
$this->deleteServerConfig('core', 'shareapi_allow_federation_on_public_shares');
3233
$this->deleteServerConfig('files_sharing', 'outgoing_server2server_share_enabled');
3334
$this->deleteServerConfig('core', 'shareapi_allow_view_without_download');
3435

‎core/AppInfo/Application.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,9 @@ public function register(IRegistrationContext $context): void {
7878

7979
// Tags
8080
$context->registerEventListener(UserDeletedEvent::class, TagManager::class);
81+
82+
// config lexicon
83+
$context->registerConfigLexicon(ConfigLexicon::class);
8184
}
8285

8386
public function boot(IBootContext $context): void {

0 commit comments

Comments
 (0)