Skip to content

Commit 2469e80

Browse files
ChristophWurstChristophWurst
authored
Merge pull request #61379 from nextcloud/backport/61293/stable32
[stable32] fix(caldav): respect federation settings
2 parents 7aff6be + 09117f6 commit 2469e80

6 files changed

Lines changed: 210 additions & 5 deletions

File tree

‎apps/dav/lib/CalDAV/Federation/CalendarFederationConfig.php‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,36 @@
99

1010
namespace OCA\DAV\CalDAV\Federation;
1111

12-
use OCP\AppFramework\Services\IAppConfig;
12+
use OCP\IAppConfig;
1313

1414
class CalendarFederationConfig {
1515
public function __construct(
1616
private readonly IAppConfig $appConfig,
17+
private \OCP\GlobalScale\IConfig $gsConfig,
1718
) {
1819
}
1920

2021
public function isFederationEnabled(): bool {
21-
return $this->appConfig->getAppValueBool('enableCalendarFederation', true);
22+
return $this->appConfig->getValueBool('dav', 'enableCalendarFederation', true);
23+
}
24+
25+
/**
26+
* Check if users are allowed to create federated shares
27+
*/
28+
public function isOutgoingServer2serverShareEnabled(): bool {
29+
if ($this->gsConfig->onlyInternalFederation()) {
30+
return false;
31+
}
32+
return $this->appConfig->getValueBool('files_sharing', 'outgoing_server2server_share_enabled', true);
33+
}
34+
35+
/**
36+
* Check if users are allowed to receive federated shares
37+
*/
38+
public function isIncomingServer2serverShareEnabled(): bool {
39+
if ($this->gsConfig->onlyInternalFederation()) {
40+
return false;
41+
}
42+
return $this->appConfig->getValueBool('files_sharing', 'incoming_server2server_share_enabled', true);
2243
}
2344
}

‎apps/dav/lib/CalDAV/Federation/CalendarFederationProvider.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,15 @@ public function shareReceived(ICloudFederationShare $share): string {
5252
);
5353
}
5454

55+
if (!$this->calendarFederationConfig->isIncomingServer2serverShareEnabled()) {
56+
$this->logger->debug('Received a federated calendar share which is not allowed on this instance');
57+
throw new ProviderCouldNotAddShareException(
58+
'Instance does not support receiving federated calendar shares',
59+
'',
60+
Http::STATUS_SERVICE_UNAVAILABLE,
61+
);
62+
}
63+
5564
if (!in_array($share->getShareType(), $this->getSupportedShareTypes(), true)) {
5665
$this->logger->debug('Received a federation invite for invalid share type');
5766
throw new ProviderCouldNotAddShareException(

‎apps/dav/lib/CalDAV/Federation/FederationSharingService.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ public function __construct(
3434
private readonly LoggerInterface $logger,
3535
private readonly ISecureRandom $random,
3636
private readonly SharingMapper $sharingMapper,
37+
private readonly CalendarFederationConfig $config,
3738
) {
3839
}
3940

@@ -70,6 +71,14 @@ private function decodeRemoteUserPrincipal(string $principal): ?string {
7071
public function shareWith(IShareable $shareable, string $principal, int $access): void {
7172
$baseError = 'Failed to create federated calendar share: ';
7273

74+
if (!$this->config->isOutgoingServer2serverShareEnabled()) {
75+
$this->logger->error('cannot share with remote user because federated sharing is disabled on this instance', [
76+
'shareable' => $shareable->getName(),
77+
'encodedShareWith' => $principal,
78+
]);
79+
return;
80+
}
81+
7382
// 1. Validate share data
7483
$shareWith = $this->decodeRemoteUserPrincipal($principal);
7584
if ($shareWith === null) {

‎apps/dav/tests/unit/CalDAV/Federation/CalendarFederationConfigTest.php‎

Lines changed: 71 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@
1010
namespace OCA\DAV\Tests\unit\CalDAV\Federation;
1111

1212
use OCA\DAV\CalDAV\Federation\CalendarFederationConfig;
13-
use OCP\AppFramework\Services\IAppConfig;
13+
use OCP\GlobalScale\IConfig;
14+
use OCP\IAppConfig;
1415
use PHPUnit\Framework\Attributes\DataProvider;
1516
use PHPUnit\Framework\MockObject\MockObject;
1617
use Test\TestCase;
@@ -19,14 +20,17 @@ class CalendarFederationConfigTest extends TestCase {
1920
private CalendarFederationConfig $config;
2021

2122
private IAppConfig&MockObject $appConfig;
23+
private IConfig&MockObject $gsConfig;
2224

2325
protected function setUp(): void {
2426
parent::setUp();
2527

2628
$this->appConfig = $this->createMock(IAppConfig::class);
29+
$this->gsConfig = $this->createMock(IConfig::class);
2730

2831
$this->config = new CalendarFederationConfig(
2932
$this->appConfig,
33+
$this->gsConfig,
3034
);
3135
}
3236

@@ -40,10 +44,74 @@ public static function provideIsFederationEnabledData(): array {
4044
#[DataProvider('provideIsFederationEnabledData')]
4145
public function testIsFederationEnabled(bool $configValue): void {
4246
$this->appConfig->expects(self::once())
43-
->method('getAppValueBool')
44-
->with('enableCalendarFederation', true)
47+
->method('getValueBool')
48+
->with('dav', 'enableCalendarFederation', true)
4549
->willReturn($configValue);
4650

4751
$this->assertEquals($configValue, $this->config->isFederationEnabled());
4852
}
53+
54+
public static function provideIsOutgoingServer2serverShareEnabledData(): array {
55+
return [
56+
[false, false, false],
57+
[false, true, true],
58+
[true, false, false],
59+
[true, false, false],
60+
];
61+
}
62+
63+
#[DataProvider(methodName: 'provideIsOutgoingServer2serverShareEnabledData')]
64+
public function testIsOutgoingServer2serverShareEnabled(
65+
bool $globalScaleEnabled,
66+
bool $expected,
67+
bool $configValue,
68+
): void {
69+
$this->gsConfig->expects(self::once())
70+
->method('onlyInternalFederation')
71+
->willReturn($globalScaleEnabled);
72+
73+
if (!$globalScaleEnabled) {
74+
$this->appConfig->expects(self::once())
75+
->method('getValueBool')
76+
->with('files_sharing', 'outgoing_server2server_share_enabled', true)
77+
->willReturn($configValue);
78+
} else {
79+
$this->appConfig->expects(self::never())
80+
->method('getValueBool');
81+
}
82+
83+
$this->assertEquals($expected, $this->config->isOutgoingServer2serverShareEnabled());
84+
}
85+
86+
public static function provideIsIncomingServer2serverShareEnabledData(): array {
87+
return [
88+
[false, false, false],
89+
[false, true, true],
90+
[true, false, false],
91+
[true, false, true],
92+
];
93+
}
94+
95+
#[DataProvider(methodName: 'provideIsIncomingServer2serverShareEnabledData')]
96+
public function testIsIncomingServer2serverShareEnabled(
97+
bool $globalScaleEnabled,
98+
bool $expected,
99+
bool $configValue,
100+
): void {
101+
$this->gsConfig->expects(self::once())
102+
->method('onlyInternalFederation')
103+
->willReturn($globalScaleEnabled);
104+
105+
if (!$globalScaleEnabled) {
106+
$this->appConfig->expects(self::once())
107+
->method('getValueBool')
108+
->with('files_sharing', 'incoming_server2server_share_enabled', true)
109+
->willReturn($configValue);
110+
} else {
111+
$this->appConfig->expects(self::never())
112+
->method('getValueBool');
113+
}
114+
115+
$this->assertEquals($expected, $this->config->isIncomingServer2serverShareEnabled());
116+
}
49117
}

‎apps/dav/tests/unit/CalDAV/Federation/CalendarFederationProviderTest.php‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,10 @@ public function testShareReceived(): void {
9191
->method('isFederationEnabled')
9292
->willReturn(true);
9393

94+
$this->calendarFederationConfig->expects(self::once())
95+
->method('isIncomingServer2serverShareEnabled')
96+
->willReturn(true);
97+
9498
$this->federatedCalendarMapper->expects(self::once())
9599
->method('deleteByUri')
96100
->with(
@@ -141,6 +145,10 @@ public function testShareReceivedWithInvalidProtocolVersion(): void {
141145
->method('isFederationEnabled')
142146
->willReturn(true);
143147

148+
$this->calendarFederationConfig->expects(self::once())
149+
->method('isIncomingServer2serverShareEnabled')
150+
->willReturn(true);
151+
144152
$this->federatedCalendarMapper->expects(self::never())
145153
->method('insert');
146154
$this->jobList->expects(self::never())
@@ -169,6 +177,10 @@ public function testShareReceivedWithoutProtocolVersion(): void {
169177
->method('isFederationEnabled')
170178
->willReturn(true);
171179

180+
$this->calendarFederationConfig->expects(self::once())
181+
->method('isIncomingServer2serverShareEnabled')
182+
->willReturn(true);
183+
172184
$this->federatedCalendarMapper->expects(self::never())
173185
->method('insert');
174186
$this->jobList->expects(self::never())
@@ -198,6 +210,30 @@ public function testShareReceivedWithDisabledConfig(): void {
198210
$this->calendarFederationProvider->shareReceived($share);
199211
}
200212

213+
public function testShareReceivedWithIncomingServer2serverShareDisabled(): void {
214+
$share = $this->createMock(ICloudFederationShare::class);
215+
$share->method('getShareType')
216+
->willReturn('user');
217+
218+
$this->calendarFederationConfig->expects(self::once())
219+
->method('isFederationEnabled')
220+
->willReturn(true);
221+
222+
$this->calendarFederationConfig->expects(self::once())
223+
->method('isIncomingServer2serverShareEnabled')
224+
->willReturn(false);
225+
226+
$this->federatedCalendarMapper->expects(self::never())
227+
->method('insert');
228+
$this->jobList->expects(self::never())
229+
->method('add');
230+
231+
$this->expectException(ProviderCouldNotAddShareException::class);
232+
$this->expectExceptionMessage('Instance does not support receiving federated calendar shares');
233+
$this->expectExceptionCode(503);
234+
$this->calendarFederationProvider->shareReceived($share);
235+
}
236+
201237
public function testShareReceivedWithUnsupportedShareType(): void {
202238
$share = $this->createMock(ICloudFederationShare::class);
203239
$share->method('getShareType')
@@ -207,6 +243,10 @@ public function testShareReceivedWithUnsupportedShareType(): void {
207243
->method('isFederationEnabled')
208244
->willReturn(true);
209245

246+
$this->calendarFederationConfig->expects(self::once())
247+
->method('isIncomingServer2serverShareEnabled')
248+
->willReturn(true);
249+
210250
$this->federatedCalendarMapper->expects(self::never())
211251
->method('insert');
212252
$this->jobList->expects(self::never())
@@ -259,6 +299,10 @@ public function testShareReceivedWithIncompleteProtocolData(array $protocol): vo
259299
->method('isFederationEnabled')
260300
->willReturn(true);
261301

302+
$this->calendarFederationConfig->expects(self::once())
303+
->method('isIncomingServer2serverShareEnabled')
304+
->willReturn(true);
305+
262306
$this->federatedCalendarMapper->expects(self::never())
263307
->method('insert');
264308
$this->jobList->expects(self::never())
@@ -296,6 +340,10 @@ public function testShareReceivedWithUnsupportedAccess(): void {
296340
->method('isFederationEnabled')
297341
->willReturn(true);
298342

343+
$this->calendarFederationConfig->expects(self::once())
344+
->method('isIncomingServer2serverShareEnabled')
345+
->willReturn(true);
346+
299347
$this->federatedCalendarMapper->expects(self::never())
300348
->method('insert');
301349
$this->jobList->expects(self::never())

‎apps/dav/tests/unit/CalDAV/Federation/FederationSharingServiceTest.php‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
namespace OCA\DAV\Tests\unit\CalDAV\Federation;
1111

1212
use OCA\DAV\CalDAV\Calendar;
13+
use OCA\DAV\CalDAV\Federation\CalendarFederationConfig;
1314
use OCA\DAV\CalDAV\Federation\FederationSharingService;
1415
use OCA\DAV\DAV\Sharing\IShareable;
1516
use OCA\DAV\DAV\Sharing\SharingMapper;
@@ -38,6 +39,7 @@ class FederationSharingServiceTest extends TestCase {
3839
private readonly LoggerInterface&MockObject $logger;
3940
private readonly ISecureRandom&MockObject $random;
4041
private readonly SharingMapper&MockObject $sharingMapper;
42+
private readonly CalendarFederationConfig&MockObject $config;
4143

4244
protected function setUp(): void {
4345
parent::setUp();
@@ -49,6 +51,7 @@ protected function setUp(): void {
4951
$this->logger = $this->createMock(LoggerInterface::class);
5052
$this->random = $this->createMock(ISecureRandom::class);
5153
$this->sharingMapper = $this->createMock(SharingMapper::class);
54+
$this->config = $this->createMock(CalendarFederationConfig::class);
5255

5356
$this->federationSharingService = new FederationSharingService(
5457
$this->federationManager,
@@ -58,6 +61,7 @@ protected function setUp(): void {
5861
$this->logger,
5962
$this->random,
6063
$this->sharingMapper,
64+
$this->config,
6165
);
6266
}
6367

@@ -83,6 +87,10 @@ public function testShareWith(): void {
8387
]
8488
});
8589

90+
$this->config->expects(self::once())
91+
->method('isOutgoingServer2serverShareEnabled')
92+
->willReturn(true);
93+
8694
$hostUser = $this->createMock(IUser::class);
8795
$hostUser->method('getCloudId')
8896
->willReturn('host1@nextcloud.host');
@@ -195,6 +203,10 @@ public function testShareWithWithFailingFederationManager(): void {
195203
]
196204
});
197205

206+
$this->config->expects(self::once())
207+
->method('isOutgoingServer2serverShareEnabled')
208+
->willReturn(true);
209+
198210
$hostUser = $this->createMock(IUser::class);
199211
$hostUser->method('getCloudId')
200212
->willReturn('host1@nextcloud.host');
@@ -299,6 +311,10 @@ public function testShareWithWithUnsuccessfulResponse(): void {
299311
]
300312
});
301313

314+
$this->config->expects(self::once())
315+
->method('isOutgoingServer2serverShareEnabled')
316+
->willReturn(true);
317+
302318
$hostUser = $this->createMock(IUser::class);
303319
$hostUser->method('getCloudId')
304320
->willReturn('host1@nextcloud.host');
@@ -381,6 +397,34 @@ public function testShareWithWithUnsuccessfulResponse(): void {
381397
);
382398
}
383399

400+
public function testShareWithWithOutgoingServer2serverShareDisabled(): void {
401+
$shareable = $this->createMock(Calendar::class);
402+
$shareable->method('getOwner')
403+
->willReturn('principals/users/host1');
404+
$shareable->method('getName')
405+
->willReturn('cal1');
406+
407+
$this->config->expects(self::once())
408+
->method('isOutgoingServer2serverShareEnabled')
409+
->willReturn(false);
410+
411+
$this->userManager->expects(self::never())
412+
->method('get');
413+
414+
$this->federationManager->expects(self::never())
415+
->method('sendCloudShare');
416+
$this->sharingMapper->expects(self::never())
417+
->method('deleteShare');
418+
$this->sharingMapper->expects(self::never())
419+
->method('shareWithToken');
420+
421+
$this->federationSharingService->shareWith(
422+
$shareable,
423+
'principals/remote-users/cmVtb3RlMUBuZXh0Y2xvdWQucmVtb3Rl',
424+
3, // Read-only
425+
);
426+
}
427+
384428
public static function provideInvalidRemoteUserPrincipalData(): array {
385429
return [
386430
['principals/users/foobar'],
@@ -418,6 +462,9 @@ public function testShareWithWithUnknownUser(): void {
418462
$shareable->method('getOwner')
419463
->willReturn('principals/users/host1');
420464

465+
$this->config->method('isOutgoingServer2serverShareEnabled')
466+
->willReturn(true);
467+
421468
$this->userManager->expects(self::once())
422469
->method('get')
423470
->with('host1')
@@ -442,6 +489,9 @@ public function testShareWithWithInvalidShareable(): void {
442489
$shareable->method('getOwner')
443490
->willReturn('principals/users/host1');
444491

492+
$this->config->method('isOutgoingServer2serverShareEnabled')
493+
->willReturn(true);
494+
445495
$this->userManager->expects(self::once())
446496
->method('get')
447497
->with('host1')

0 commit comments

Comments
 (0)